This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[RFA] Fix breakpoint condition that use member variables.


Suppose that foo.cpp:10 is a location inside member function of a class,
and that said class has member variable i_, and that we've just started
a program and are in main. Then:

    break foo.cpp:10 if i_ == 10

will not work, claiming that i_ does not exist. The problem is that
lookup_symbol_aux uses value_of_this, which uses value_of_local and
that totally ignores the block that is passed to parse_exp_1 by
the breakpoint code and uses the block of the selected frame. Of course,
that either does not have "this", or has wrong "this".

This patch fixes the problem by looking in the right block directly,
and also by looking for the field in the type of "this", without
trying to get the value.

It should be noted that the existing check_field/check_field_in
functions are very similar in spirit to lookup_struct_elt_type,
so I've tried to kill check_field completely, but presently 
lookup_struct_elt_type does not handle member functions, so not
immediately suitable.

I've also changed language definitions for fortran and scheme not
to claim they have "this" pointer, as to the best of my knowledge
they don't.

No regressions on x86. OK?

- Volodya

	[gdb]
	Fix breakpoint condition that use member variables.
	* valops.c (check_field): Remove.
	(check_field_in): Rename to check_field.
	(value_of_this): Use la_name_of_this.
	* value.h (check_field): Adjust prototype.

	* language.h (la_value_of_this): Rename to la_name_of_this.
	* language.c (unknown_language_defn): Specify "this" for
	name_of_this.
	(auto_language_defn): Likewise.
	(local_language_defn): Likewise.
	* ada-lang.c (ada_language_defn): Adjust comment.
	* c-lang.c (c_language_defn): Adjust comment.
	(cplus_language_defn): Specify "this" for name_of_this.
	(asm_language_defn): Adjust comment.
	(minimal_language_defn): Adjust comment.
	* f-lang.c (f_language_defn): Specify NULL for name_of_this.
	* jv-lang.c (java_language_defn): Specify "this" for name_of_this.
	* m2-lang.c (m2_language_defn): Specify "this" for name_of_this.
	* objc-lang.c (objc_language_defn): Specify "self" for
	name_of_this.
	* p-lang.c (pascal_language_defn): Specify "this" for
	name_of_this.
	* scm-lang.c (scm_language_defn): Specify NULL for name_of_this.

	* symtab.c (lookup_symbol_aux): Lookup "this" in the
	proper scope, and check for field in type of "this", without
	trying to create a value.

	[gdb/testsuite]
	* gdb.cp/breakpoint.cc: New code to test conditions involving
	member variables.
	* gdb.cp/breakpoint.exp: Test condition involving member
	variables.
---
 gdb/ada-lang.c                      |    2 +-
 gdb/c-lang.c                        |    8 +++---
 gdb/f-lang.c                        |    2 +-
 gdb/jv-lang.c                       |    2 +-
 gdb/language.c                      |    6 ++--
 gdb/language.h                      |   10 ++-----
 gdb/m2-lang.c                       |    2 +-
 gdb/objc-lang.c                     |    2 +-
 gdb/p-lang.c                        |    2 +-
 gdb/scm-lang.c                      |    2 +-
 gdb/symtab.c                        |   35 +++++++++++++++++++------
 gdb/testsuite/gdb.cp/breakpoint.cc  |   16 +++++++++++-
 gdb/testsuite/gdb.cp/breakpoint.exp |    5 +++
 gdb/valops.c                        |   47 
+++-------------------------------
 gdb/value.h                         |    2 +-
 15 files changed, 69 insertions(+), 74 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 2f0f55f..8cfc2c8 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -10999,7 +10999,7 @@ const struct language_defn ada_language_defn = {
   ada_val_print,                /* Print a value using appropriate 
syntax */
   ada_value_print,              /* Print a top-level value */
   NULL,                         /* Language specific skip_trampoline */
-  NULL,                         /* value_of_this */
+  NULL,                         /* name_of_this */
   ada_lookup_symbol_nonlocal,   /* Looking up non-local symbols.  */
   basic_lookup_transparent_type,        /* lookup_transparent_type */
   ada_la_decode,                /* Language specific symbol demangler 
*/
diff --git a/gdb/c-lang.c b/gdb/c-lang.c
index c4ef9d6..29aa765 100644
--- a/gdb/c-lang.c
+++ b/gdb/c-lang.c
@@ -414,7 +414,7 @@ const struct language_defn c_language_defn =
   c_val_print,			/* Print a value using appropriate syntax */
   c_value_print,		/* Print a top-level value */
   NULL,				/* Language specific skip_trampoline */
-  NULL,				/* value_of_this */
+  NULL,				/* name_of_this */
   basic_lookup_symbol_nonlocal,	/* lookup_symbol_nonlocal */
   basic_lookup_transparent_type,/* lookup_transparent_type */
   NULL,				/* Language specific symbol demangler */
@@ -527,7 +527,7 @@ const struct language_defn cplus_language_defn =
   c_val_print,			/* Print a value using appropriate syntax */
   c_value_print,		/* Print a top-level value */
   cplus_skip_trampoline,	/* Language specific skip_trampoline */
-  value_of_this,		/* value_of_this */
+  "this",                       /* name_of_this */
   cp_lookup_symbol_nonlocal,	/* lookup_symbol_nonlocal */
   cp_lookup_transparent_type,   /* lookup_transparent_type */
   cplus_demangle,		/* Language specific symbol demangler */
@@ -562,7 +562,7 @@ const struct language_defn asm_language_defn =
   c_val_print,			/* Print a value using appropriate syntax */
   c_value_print,		/* Print a top-level value */
   NULL,				/* Language specific skip_trampoline */
-  NULL,				/* value_of_this */
+  NULL,				/* name_of_this */
   basic_lookup_symbol_nonlocal,	/* lookup_symbol_nonlocal */
   basic_lookup_transparent_type,/* lookup_transparent_type */
   NULL,				/* Language specific symbol demangler */
@@ -602,7 +602,7 @@ const struct language_defn minimal_language_defn =
   c_val_print,			/* Print a value using appropriate syntax */
   c_value_print,		/* Print a top-level value */
   NULL,				/* Language specific skip_trampoline */
-  NULL,				/* value_of_this */
+  NULL,				/* name_of_this */
   basic_lookup_symbol_nonlocal,	/* lookup_symbol_nonlocal */
   basic_lookup_transparent_type,/* lookup_transparent_type */
   NULL,				/* Language specific symbol demangler */
diff --git a/gdb/f-lang.c b/gdb/f-lang.c
index 038126c..5dcbd33 100644
--- a/gdb/f-lang.c
+++ b/gdb/f-lang.c
@@ -324,7 +324,7 @@ const struct language_defn f_language_defn =
   f_val_print,			/* Print a value using appropriate syntax */
   c_value_print,		/* FIXME */
   NULL,				/* Language specific skip_trampoline */
-  value_of_this,		/* value_of_this */
+  NULL,                    	/* name_of_this */
   basic_lookup_symbol_nonlocal,	/* lookup_symbol_nonlocal */
   basic_lookup_transparent_type,/* lookup_transparent_type */
   NULL,				/* Language specific symbol demangler */
diff --git a/gdb/jv-lang.c b/gdb/jv-lang.c
index e6c6f7d..b72b90c 100644
--- a/gdb/jv-lang.c
+++ b/gdb/jv-lang.c
@@ -1070,7 +1070,7 @@ const struct language_defn java_language_defn =
   java_val_print,		/* Print a value using appropriate syntax */
   java_value_print,		/* Print a top-level value */
   NULL,				/* Language specific skip_trampoline */
-  value_of_this,		/* value_of_this */
+  "this",	                /* name_of_this */
   basic_lookup_symbol_nonlocal,	/* lookup_symbol_nonlocal */
   basic_lookup_transparent_type,/* lookup_transparent_type */
   java_demangle,		/* Language specific symbol demangler */
diff --git a/gdb/language.c b/gdb/language.c
index a26218d..80f6961 100644
--- a/gdb/language.c
+++ b/gdb/language.c
@@ -1192,7 +1192,7 @@ const struct language_defn unknown_language_defn =
   unk_lang_val_print,		/* Print a value using appropriate syntax */
   unk_lang_value_print,		/* Print a top-level value */
   unk_lang_trampoline,		/* Language specific skip_trampoline */
-  value_of_this,		/* value_of_this */
+  "this",        	    	/* name_of_this */
   basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */
   basic_lookup_transparent_type,/* lookup_transparent_type */
   unk_lang_demangle,		/* Language specific symbol demangler */
@@ -1228,7 +1228,7 @@ const struct language_defn auto_language_defn =
   unk_lang_val_print,		/* Print a value using appropriate syntax */
   unk_lang_value_print,		/* Print a top-level value */
   unk_lang_trampoline,		/* Language specific skip_trampoline */
-  value_of_this,		/* value_of_this */
+  "this",		        /* name_of_this */
   basic_lookup_symbol_nonlocal,	/* lookup_symbol_nonlocal */
   basic_lookup_transparent_type,/* lookup_transparent_type */
   unk_lang_demangle,		/* Language specific symbol demangler */
@@ -1263,7 +1263,7 @@ const struct language_defn local_language_defn =
   unk_lang_val_print,		/* Print a value using appropriate syntax */
   unk_lang_value_print,		/* Print a top-level value */
   unk_lang_trampoline,		/* Language specific skip_trampoline */
-  value_of_this,		/* value_of_this */
+  "this", 		        /* name_of_this */
   basic_lookup_symbol_nonlocal,	/* lookup_symbol_nonlocal */
   basic_lookup_transparent_type,/* lookup_transparent_type */
   unk_lang_demangle,		/* Language specific symbol demangler */
diff --git a/gdb/language.h b/gdb/language.h
index f7e654d..233a5a3 100644
--- a/gdb/language.h
+++ b/gdb/language.h
@@ -206,14 +206,10 @@ struct language_defn
 
     /* Now come some hooks for lookup_symbol.  */
 
-    /* If this is non-NULL, lookup_symbol will do the 'field_of_this'
-       check, using this function to find the value of this.  */
+    /* If this is non-NULL, specifies the name that of the implicit
+       local variable that refers to the current object instance.  */
 
-    /* FIXME: carlton/2003-05-19: Audit all the language_defn structs
-       to make sure we're setting this appropriately: I'm sure it
-       could be NULL in more languages.  */
-
-    struct value *(*la_value_of_this) (int complain);
+    char *la_name_of_this;
 
     /* This is a function that lookup_symbol will call when it gets to
        the part of symbol lookup where C looks up static and global
diff --git a/gdb/m2-lang.c b/gdb/m2-lang.c
index 6b51fd5..400338e 100644
--- a/gdb/m2-lang.c
+++ b/gdb/m2-lang.c
@@ -375,7 +375,7 @@ const struct language_defn m2_language_defn =
   m2_val_print,			/* Print a value using appropriate syntax */
   c_value_print,		/* Print a top-level value */
   NULL,				/* Language specific skip_trampoline */
-  value_of_this,		/* value_of_this */
+  "this",		        /* name_of_this */
   basic_lookup_symbol_nonlocal,	/* lookup_symbol_nonlocal */
   basic_lookup_transparent_type,/* lookup_transparent_type */
   NULL,				/* Language specific symbol demangler */
diff --git a/gdb/objc-lang.c b/gdb/objc-lang.c
index ccf8068..08a6bb8 100644
--- a/gdb/objc-lang.c
+++ b/gdb/objc-lang.c
@@ -509,7 +509,7 @@ const struct language_defn objc_language_defn = {
   c_val_print,			/* Print a value using appropriate syntax */
   c_value_print,		/* Print a top-level value */
   objc_skip_trampoline, 	/* Language specific skip_trampoline */
-  value_of_this,		/* value_of_this */
+  "self",		        /* name_of_this */
   basic_lookup_symbol_nonlocal,	/* lookup_symbol_nonlocal */
   basic_lookup_transparent_type,/* lookup_transparent_type */
   objc_demangle,		/* Language specific symbol demangler */
diff --git a/gdb/p-lang.c b/gdb/p-lang.c
index f7b901f..2accf35 100644
--- a/gdb/p-lang.c
+++ b/gdb/p-lang.c
@@ -414,7 +414,7 @@ const struct language_defn pascal_language_defn =
   pascal_val_print,		/* Print a value using appropriate syntax */
   pascal_value_print,		/* Print a top-level value */
   NULL,				/* Language specific skip_trampoline */
-  value_of_this,		/* value_of_this */
+  "this",		        /* name_of_this */
   basic_lookup_symbol_nonlocal,	/* lookup_symbol_nonlocal */
   basic_lookup_transparent_type,/* lookup_transparent_type */
   NULL,				/* Language specific symbol demangler */
diff --git a/gdb/scm-lang.c b/gdb/scm-lang.c
index 339c614..0692d43 100644
--- a/gdb/scm-lang.c
+++ b/gdb/scm-lang.c
@@ -253,7 +253,7 @@ const struct language_defn scm_language_defn =
   scm_val_print,		/* Print a value using appropriate syntax */
   scm_value_print,		/* Print a top-level value */
   NULL,				/* Language specific skip_trampoline */
-  value_of_this,		/* value_of_this */
+  NULL,	                        /* name_of_this */
   basic_lookup_symbol_nonlocal,	/* lookup_symbol_nonlocal */
   basic_lookup_transparent_type,/* lookup_transparent_type */
   NULL,				/* Language specific symbol demangler */
diff --git a/gdb/symtab.c b/gdb/symtab.c
index ddd2310..7d9b4ea 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -1198,17 +1198,34 @@ lookup_symbol_aux (const char *name, const char 
*linkage_name,
 
   langdef = language_def (language);
 
-  if (langdef->la_value_of_this != NULL
-      && is_a_field_of_this != NULL)
+  if (langdef->la_name_of_this != NULL && is_a_field_of_this != NULL
+      && block != NULL && !dict_empty (BLOCK_DICT (block)))
     {
-      struct value *v = langdef->la_value_of_this (0);
-
-      if (v && check_field (v, name))
+      struct symbol *sym = lookup_block_symbol (block, 
langdef->la_name_of_this,
+						NULL, VAR_DOMAIN);
+      if (sym)
 	{
-	  *is_a_field_of_this = 1;
-	  if (symtab != NULL)
-	    *symtab = NULL;
-	  return NULL;
+	  struct type *t = sym->type;
+
+	  /* I'm not really sure that type of this can ever
+	     be typedefed; just be safe.  */
+	  CHECK_TYPEDEF (t);
+	  if (TYPE_CODE (t) == TYPE_CODE_PTR
+	      || TYPE_CODE (t) == TYPE_CODE_REF)
+	    t = TYPE_TARGET_TYPE (t);
+
+	  if (TYPE_CODE (t) != TYPE_CODE_STRUCT
+	      && TYPE_CODE (t) != TYPE_CODE_UNION)
+	    error (_("Internal error: `%s' is not an aggregate"), 
+		   langdef->la_name_of_this);
+	  
+	  if (check_field (t, name))
+	    {
+	      *is_a_field_of_this = 1;
+	      if (symtab != NULL)
+		*symtab = NULL;
+	      return NULL;
+	    }
 	}
     }
 
diff --git a/gdb/testsuite/gdb.cp/breakpoint.cc 
b/gdb/testsuite/gdb.cp/breakpoint.cc
index c719af2..e479efd 100644
--- a/gdb/testsuite/gdb.cp/breakpoint.cc
+++ b/gdb/testsuite/gdb.cp/breakpoint.cc
@@ -19,6 +19,13 @@
 
 class C1 {
 public:
+  C1(int i) : i_(i) {}
+
+  int foo ()
+  {
+    return 1; // conditional breakpoint in method
+  }
+
   class Nested {
   public:
     int
@@ -27,13 +34,20 @@ public:
       return 1;
     }
   };
+
+private:
+  int i_;
 };
 
 int main ()
 {
   C1::Nested c1;
 
-  c1.foo();
+  c1.foo ();
+  
+  C1 c2 (2), c3 (3);
+  c2.foo ();
+  c3.foo ();
 
   return 0;
 }
diff --git a/gdb/testsuite/gdb.cp/breakpoint.exp 
b/gdb/testsuite/gdb.cp/breakpoint.exp
index d75e0f7..3c176e9 100644
--- a/gdb/testsuite/gdb.cp/breakpoint.exp
+++ b/gdb/testsuite/gdb.cp/breakpoint.exp
@@ -61,5 +61,10 @@ proc test_breakpoint {name} {
 
 test_breakpoint "C1::Nested::foo"
 
+set bp_location1 [gdb_get_line_number "conditional breakpoint in 
method"]
+gdb_test "break $bp_location1 if i_==3" ".*Breakpoint.*" "conditional 
breakpoint in method"
+gdb_test "continue" ".*Breakpoint.*C1::foo.*" "continue to breakpoint"
+gdb_test "print i_" "\\\$1 = 3" "check the member variable"
+
 gdb_exit
 return 0
diff --git a/gdb/valops.c b/gdb/valops.c
index 80bee1e..0ef8df4 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -80,8 +80,6 @@ static enum
 oload_classification classify_oload_match (struct badness_vector *,
 					   int, int);
 
-static int check_field_in (struct type *, const char *);
-
 static struct value *value_struct_elt_for_reference (struct type *,
 						     int, struct type *,
 						     char *,
@@ -2296,12 +2294,12 @@ destructor_name_p (const char *name, const 
struct type *type)
   return 0;
 }
 
-/* Helper function for check_field: Given TYPE, a structure/union,
+/* Given TYPE, a structure/union,
    return 1 if the component named NAME from the ultimate target
    structure/union is defined, otherwise, return 0.  */
 
-static int
-check_field_in (struct type *type, const char *name)
+int
+check_field (struct type *type, const char *name)
 {
   int i;
 
@@ -2330,44 +2328,12 @@ check_field_in (struct type *type, const char 
*name)
     }
 
   for (i = TYPE_N_BASECLASSES (type) - 1; i >= 0; i--)
-    if (check_field_in (TYPE_BASECLASS (type, i), name))
+    if (check_field (TYPE_BASECLASS (type, i), name))
       return 1;
 
   return 0;
 }
 
-
-/* C++: Given ARG1, a value of type (pointer to a)* structure/union,
-   return 1 if the component named NAME from the ultimate target
-   structure/union is defined, otherwise, return 0.  */
-
-int
-check_field (struct value *arg1, const char *name)
-{
-  struct type *t;
-
-  arg1 = coerce_array (arg1);
-
-  t = value_type (arg1);
-
-  /* Follow pointers until we get to a non-pointer.  */
-
-  for (;;)
-    {
-      CHECK_TYPEDEF (t);
-      if (TYPE_CODE (t) != TYPE_CODE_PTR 
-	  && TYPE_CODE (t) != TYPE_CODE_REF)
-	break;
-      t = TYPE_TARGET_TYPE (t);
-    }
-
-  if (TYPE_CODE (t) != TYPE_CODE_STRUCT
-      && TYPE_CODE (t) != TYPE_CODE_UNION)
-    error (_("Internal error: `this' is not an aggregate"));
-
-  return check_field_in (t, name);
-}
-
 /* C++: Given an aggregate type CURTYPE, and a member name NAME,
    return the appropriate member (or the address of the member, if
    WANT_ADDRESS).  This function is used to resolve user expressions
@@ -2773,10 +2739,7 @@ value_of_local (const char *name, int complain)
 struct value *
 value_of_this (int complain)
 {
-  if (current_language->la_language == language_objc)
-    return value_of_local ("self", complain);
-  else
-    return value_of_local ("this", complain);
+  return value_of_local (current_language->la_name_of_this, complain);
 }
 
 /* Create a slice (sub-string, sub-array) of ARRAY, that is LENGTH
diff --git a/gdb/value.h b/gdb/value.h
index ba226e5..5f8fe58 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -530,7 +530,7 @@ extern void print_variable_value (struct symbol 
*var,
 				  struct frame_info *frame,
 				  struct ui_file *stream);
 
-extern int check_field (struct value *, const char *);
+extern int check_field (struct type *, const char *);
 
 extern void typedef_print (struct type *type, struct symbol *news,
 			   struct ui_file *stream);
-- 
1.5.3.5


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]