Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* disable objective-c stuff when theres no objective-c cu.
@ 2010-10-05 22:37 Matt Rice
  2010-10-05 23:31 ` Michael Snyder
  0 siblings, 1 reply; 11+ messages in thread
From: Matt Rice @ 2010-10-05 22:37 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 655 bytes --]

this makes it so that a flag is set if either an objective-c
compilation unit is found,
or the user goes 'set language objective-c' in the case where debug
symbols are absent.

2010-10-05  Matt Rice  <ratmice@gmail.com>

        * defs.h: Add comment.
        * dwarf2read.c (set_cu_language): Notice that a language has been
        seen.
        * language.c (set_language): Ditto.
        (mask_for_language, language_has_cu_loaded): New Functions.
        (set_language_has_cu_loaded): Ditto.
        * language.h: Declare new functions.
        * linespec.c (decode_line_1): Don't lookup objective-c methods
        unless objective-c has been seen.

[-- Attachment #2: gdb-lang-objc.diff --]
[-- Type: application/octet-stream, Size: 5286 bytes --]

diff --git a/gdb/defs.h b/gdb/defs.h
index 9e4800c..304d1c9 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -185,7 +185,9 @@ extern void quit (void);
 /* Languages represented in the symbol table and elsewhere.
    This should probably be in language.h, but since enum's can't
    be forward declared to satisfy opaque references before their
-   actual definition, needs to be here. */
+   actual definition, needs to be here.
+
+   When adding a tangible language also update language.c:mask_for_language */
 
 enum language
   {
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index bf36e01..e0d4d52 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -9646,6 +9646,7 @@ set_cu_language (unsigned int lang, struct dwarf2_cu *cu)
       break;
     }
   cu->language_defn = language_def (cu->language);
+  set_language_has_cu_loaded (cu->language);
 }
 
 /* Return the named attribute or NULL if not there.  */
diff --git a/gdb/language.c b/gdb/language.c
index 3ce08b5..2a8c727 100644
--- a/gdb/language.c
+++ b/gdb/language.c
@@ -435,6 +435,7 @@ set_language (enum language lang)
       if (languages[i]->la_language == lang)
 	{
 	  current_language = languages[i];
+	  set_language_has_cu_loaded(current_language->la_language);
 	  set_type_range_case ();
 	  break;
 	}
@@ -1060,6 +1061,83 @@ default_get_string (struct value *value, gdb_byte **buffer, int *length,
   error (_("Getting a string is unsupported in this language."));
 }
 
+
+/* A mask for langauges that want to enable specific behaviours if (not when) 
+   a compilation unit of that language has been loaded. the only one currently
+   in use is objc but i figured its best to make it a generic api */
+#define CU_LOADED_C_LANG_MASK		0x1 << 0
+#define CU_LOADED_CPLUS_LANG_MASK	0x1 << 1
+#define CU_LOADED_D_LANG_MASK		0x1 << 2
+#define CU_LOADED_OBJC_LANG_MASK	0x1 << 3
+#define CU_LOADED_JAVA_LANG_MASK	0x1 << 4
+#define CU_LOADED_FORTRAN_LANG_MASK	0x1 << 5
+#define CU_LOADED_M2_LANG_MASK		0x1 << 6
+#define CU_LOADED_ASM_LANG_MASK		0x1 << 7
+#define CU_LOADED_PASCAL_LANG_MASK	0x1 << 8
+#define CU_LOADED_ADA_LANG_MASK		0x1 << 9
+#define CU_LOADED_SCM_LANG_MASK		0x1 << 10
+
+static unsigned int cu_languages_loaded_mask;
+
+static unsigned int
+mask_for_language(enum language lang)
+{
+  unsigned int lang_mask;
+  switch(lang)
+    {
+      case language_c:			/* C */
+        lang_mask = CU_LOADED_C_LANG_MASK;
+      break;
+      case language_cplus:		/* C++ */
+        lang_mask = CU_LOADED_CPLUS_LANG_MASK;
+      break;
+      case language_d:			/* D */
+        lang_mask = CU_LOADED_D_LANG_MASK;
+      break;
+      case language_objc:		/* Objective-C */
+        lang_mask = CU_LOADED_OBJC_LANG_MASK;
+      break;
+      case language_java:		/* Java */
+        lang_mask = CU_LOADED_JAVA_LANG_MASK;
+      break;
+      case language_fortran:		/* Fortran */
+        lang_mask = CU_LOADED_FORTRAN_LANG_MASK;
+      break;
+      case language_m2:		/* Modula-2 */
+        lang_mask = CU_LOADED_M2_LANG_MASK;
+      break;
+      case language_asm:		/* Assembly language */
+        lang_mask = CU_LOADED_ASM_LANG_MASK;
+      break;
+      case language_pascal:		/* Pascal */
+        lang_mask = CU_LOADED_PASCAL_LANG_MASK;
+      break;
+      case language_ada:		/* Pascal */
+        lang_mask = CU_LOADED_ADA_LANG_MASK;
+      break;
+      case language_scm:		/* Pascal */
+        lang_mask = CU_LOADED_SCM_LANG_MASK;
+      break;
+      default:
+	return 0;
+    }
+  return lang_mask;
+}
+
+unsigned int
+language_has_cu_loaded (enum language lang)
+{
+  unsigned int lang_mask = mask_for_language (lang);
+  return cu_languages_loaded_mask & lang_mask;
+}
+
+void
+set_language_has_cu_loaded (enum language lang)
+{
+  unsigned int lang_mask = mask_for_language (lang);
+  cu_languages_loaded_mask |= lang_mask; 
+}
+
 /* Define the language that is no language.  */
 
 static int
diff --git a/gdb/language.h b/gdb/language.h
index aa0523b..6fde401 100644
--- a/gdb/language.h
+++ b/gdb/language.h
@@ -523,4 +523,12 @@ void default_get_string (struct value *value, gdb_byte **buffer, int *length,
 void c_get_string (struct value *value, gdb_byte **buffer, int *length,
 		   struct type **char_type, const char **charset);
 
+/* Returns non-zero if cu of the language type has been previously loaded. */
+unsigned int
+language_has_cu_loaded (enum language lang);
+
+/* Sets that a cu of the language type has been previously loaded. */
+void
+set_language_has_cu_loaded (enum language lang);
+
 #endif /* defined (LANGUAGE_H) */
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 91c5b90..4c7197f 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -771,14 +771,15 @@ decode_line_1 (char **argptr, int funfirstline, struct symtab *default_symtab,
 
   /* Check if the symbol could be an Objective-C selector.  */
 
-  {
-    struct symtabs_and_lines values;
+  if (language_has_cu_loaded(language_objc))
+    {
+      struct symtabs_and_lines values;
 
-    values = decode_objc (argptr, funfirstline, NULL,
+      values = decode_objc (argptr, funfirstline, NULL,
 			  canonical, saved_arg);
-    if (values.sals != NULL)
-      return values;
-  }
+      if (values.sals != NULL)
+        return values;
+    }
 
   /* Does it look like there actually were two parts?  */
 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: disable objective-c stuff when theres no objective-c cu.
  2010-10-05 22:37 disable objective-c stuff when theres no objective-c cu Matt Rice
@ 2010-10-05 23:31 ` Michael Snyder
  2010-10-05 23:35   ` Matt Rice
  0 siblings, 1 reply; 11+ messages in thread
From: Michael Snyder @ 2010-10-05 23:31 UTC (permalink / raw)
  To: Matt Rice; +Cc: gdb-patches

Matt Rice wrote:
> this makes it so that a flag is set if either an objective-c
> compilation unit is found,
> or the user goes 'set language objective-c' in the case where debug
> symbols are absent.
> 
> 2010-10-05  Matt Rice  <ratmice@gmail.com>
> 
>         * defs.h: Add comment.
>         * dwarf2read.c (set_cu_language): Notice that a language has been
>         seen.
>         * language.c (set_language): Ditto.
>         (mask_for_language, language_has_cu_loaded): New Functions.
>         (set_language_has_cu_loaded): Ditto.
>         * language.h: Declare new functions.
>         * linespec.c (decode_line_1): Don't lookup objective-c methods
>         unless objective-c has been seen.

Hi Matt,

Do you have a  copyright assignment?

Thanks,
Michael


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: disable objective-c stuff when theres no objective-c cu.
  2010-10-05 23:31 ` Michael Snyder
@ 2010-10-05 23:35   ` Matt Rice
  2010-10-06  2:30     ` Matt Rice
  0 siblings, 1 reply; 11+ messages in thread
From: Matt Rice @ 2010-10-05 23:35 UTC (permalink / raw)
  To: Michael Snyder; +Cc: gdb-patches

On Tue, Oct 5, 2010 at 4:31 PM, Michael Snyder <msnyder@vmware.com> wrote:
> Matt Rice wrote:
>>
>> this makes it so that a flag is set if either an objective-c
>> compilation unit is found,
>> or the user goes 'set language objective-c' in the case where debug
>> symbols are absent.
>>
>> 2010-10-05  Matt Rice  <ratmice@gmail.com>
>>
>>        * defs.h: Add comment.
>>        * dwarf2read.c (set_cu_language): Notice that a language has been
>>        seen.
>>        * language.c (set_language): Ditto.
>>        (mask_for_language, language_has_cu_loaded): New Functions.
>>        (set_language_has_cu_loaded): Ditto.
>>        * language.h: Declare new functions.
>>        * linespec.c (decode_line_1): Don't lookup objective-c methods
>>        unless objective-c has been seen.
>
> Hi Matt,
>
> Do you have a  copyright assignment?

Yeah, I do

and I should have probably noted that this is a change of behaviour
so its kind of an RFC, and I didn't think about stabs, so w/ this they
probably require 'set language objective-c' 'set language auto' should
restore the old behaviour

waiting on the testsuite to run, but it looks like gdb.objc/nondebug.exp
is going to require 'set language objective-c'.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: disable objective-c stuff when theres no objective-c cu.
  2010-10-05 23:35   ` Matt Rice
@ 2010-10-06  2:30     ` Matt Rice
  2010-10-06  8:51       ` Jan Kratochvil
  0 siblings, 1 reply; 11+ messages in thread
From: Matt Rice @ 2010-10-06  2:30 UTC (permalink / raw)
  To: Michael Snyder; +Cc: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 3131 bytes --]

On Tue, Oct 5, 2010 at 4:35 PM, Matt Rice <ratmice@gmail.com> wrote:
> On Tue, Oct 5, 2010 at 4:31 PM, Michael Snyder <msnyder@vmware.com> wrote:
>> Matt Rice wrote:
>>>
>>> this makes it so that a flag is set if either an objective-c
>>> compilation unit is found,
>>> or the user goes 'set language objective-c' in the case where debug
>>> symbols are absent.
>>>
>>> 2010-10-05  Matt Rice  <ratmice@gmail.com>
>>>
>>>        * defs.h: Add comment.
>>>        * dwarf2read.c (set_cu_language): Notice that a language has been
>>>        seen.
>>>        * language.c (set_language): Ditto.
>>>        (mask_for_language, language_has_cu_loaded): New Functions.
>>>        (set_language_has_cu_loaded): Ditto.
>>>        * language.h: Declare new functions.
>>>        * linespec.c (decode_line_1): Don't lookup objective-c methods
>>>        unless objective-c has been seen.
>>
>> Hi Matt,
>>
>> Do you have a  copyright assignment?
>
> Yeah, I do
>
> and I should have probably noted that this is a change of behaviour
> so its kind of an RFC, and I didn't think about stabs, so w/ this they
> probably require 'set language objective-c' 'set language auto' should
> restore the old behaviour
>
> waiting on the testsuite to run, but it looks like gdb.objc/nondebug.exp
> is going to require 'set language objective-c'.
>

attached is a new patch, the old one had some typos/pasteos in
comments, and i noticed
that we can lessen the impact by checking for is_objc_method, or for
objective-c compilation units. (is_objc_method doesn't neccesarily
affect future lookups should it call set_language_has_cu_loaded?)

added a NEWS entry which at least explains the current impact of the change.
which may or may not be acceptable.

I asked on the gnustep-dev list a while ago about removing this
feature all together, they weren't opposed to replacing it with a
different but similar feature,
so they're probably not opposed to keeping the feature but having to
manually enable it if it cannot be lazily enabled.

here is when gcc started to emit DW_LANG_ObjC, maybe we could fall
back to source files with the .m extension.
http://gcc.gnu.org/ml/gcc-patches/2006-03/msg01515.html


another patch here which i'll look into tomorrow to see whatever
happened with it,
might be helpful in squaring out support for other debug info formats
also probably looking for .m sources.
http://sourceware.org/ml/gdb-patches/2004-08/msg00138.html

the impetus for this is that Objective-c matches normal c functions as methods,
so when you set a breakpoint on a c-function it tries to match it to
an objective-c method and churns through all the symbols, then finally
falls back to looking for a c function.


-PASS: gdb.cp/psmang.exp: break s::method2
+FAIL: gdb.cp/psmang.exp: break s::method2 (got interactive prompt)

-FAIL: gdb.threads/attachstop-mt.exp: attach1, post-gdb sanity check
of the sleeping state - Red Hat BZ 197584
+PASS: gdb.threads/attachstop-mt.exp: attach1, post-gdb sanity check
of the sleeping state - Red Hat BZ 197584

at least it broke even.

[-- Attachment #2: gdb-lang-objc.diff --]
[-- Type: application/octet-stream, Size: 6448 bytes --]

diff --git a/gdb/NEWS b/gdb/NEWS
index 85059c6..01b0a3c 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -23,6 +23,25 @@
      feature requires proper debuginfo support from the compiler; it
      was added to GCC 4.5.
 
+* Objective-C changes:
+
+  ** GDB Objective-C language support has changed so that lookups of
+     objective-c methods is now done if any of the following conditions is true:
+
+       * GDB has identified objective-c code as having been loaded.
+
+       * The user has specified an objective-c method explicitly in the form of
+         `break +[Class aMethod]' or `break -[Class aMethod]' as opposed to
+         the short form `break aMethod'.
+ 
+       * The user has called 'set language objective-c' at least once;
+         though it need not be the current language.
+
+     For GDB to identify an objective-c library debug symbols must be present
+     and contain language information.  Users wanting to retain the old behaviour
+     can add `set language objective-c` followed by `set language auto' lines
+     to their .gdbinit file.
+
 * GDB now has some support for using labels in the program's source in
   linespecs.  For instance, you can use "advance label" to continue
   execution to a label.
diff --git a/gdb/defs.h b/gdb/defs.h
index 9e4800c..304d1c9 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -185,7 +185,9 @@ extern void quit (void);
 /* Languages represented in the symbol table and elsewhere.
    This should probably be in language.h, but since enum's can't
    be forward declared to satisfy opaque references before their
-   actual definition, needs to be here. */
+   actual definition, needs to be here.
+
+   When adding a tangible language also update language.c:mask_for_language */
 
 enum language
   {
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index bf36e01..e0d4d52 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -9646,6 +9646,7 @@ set_cu_language (unsigned int lang, struct dwarf2_cu *cu)
       break;
     }
   cu->language_defn = language_def (cu->language);
+  set_language_has_cu_loaded (cu->language);
 }
 
 /* Return the named attribute or NULL if not there.  */
diff --git a/gdb/language.c b/gdb/language.c
index 3ce08b5..cdc69c9 100644
--- a/gdb/language.c
+++ b/gdb/language.c
@@ -435,6 +435,7 @@ set_language (enum language lang)
       if (languages[i]->la_language == lang)
 	{
 	  current_language = languages[i];
+	  set_language_has_cu_loaded(current_language->la_language);
 	  set_type_range_case ();
 	  break;
 	}
@@ -1060,6 +1061,82 @@ default_get_string (struct value *value, gdb_byte **buffer, int *length,
   error (_("Getting a string is unsupported in this language."));
 }
 
+
+/* A mask for languages that want to enable specific behaviours if (not when)
+   a compilation unit of that language has been loaded. */
+#define CU_LOADED_C_LANG_MASK		0x1 << 0
+#define CU_LOADED_CPLUS_LANG_MASK	0x1 << 1
+#define CU_LOADED_D_LANG_MASK		0x1 << 2
+#define CU_LOADED_OBJC_LANG_MASK	0x1 << 3
+#define CU_LOADED_JAVA_LANG_MASK	0x1 << 4
+#define CU_LOADED_FORTRAN_LANG_MASK	0x1 << 5
+#define CU_LOADED_M2_LANG_MASK		0x1 << 6
+#define CU_LOADED_ASM_LANG_MASK		0x1 << 7
+#define CU_LOADED_PASCAL_LANG_MASK	0x1 << 8
+#define CU_LOADED_ADA_LANG_MASK		0x1 << 9
+#define CU_LOADED_SCM_LANG_MASK		0x1 << 10
+
+static unsigned int cu_languages_loaded_mask;
+
+static unsigned int
+mask_for_language(enum language lang)
+{
+  unsigned int lang_mask;
+  switch(lang)
+    {
+      case language_c:			/* C */
+        lang_mask = CU_LOADED_C_LANG_MASK;
+      break;
+      case language_cplus:		/* C++ */
+        lang_mask = CU_LOADED_CPLUS_LANG_MASK;
+      break;
+      case language_d:			/* D */
+        lang_mask = CU_LOADED_D_LANG_MASK;
+      break;
+      case language_objc:		/* Objective-C */
+        lang_mask = CU_LOADED_OBJC_LANG_MASK;
+      break;
+      case language_java:		/* Java */
+        lang_mask = CU_LOADED_JAVA_LANG_MASK;
+      break;
+      case language_fortran:		/* Fortran */
+        lang_mask = CU_LOADED_FORTRAN_LANG_MASK;
+      break;
+      case language_m2:			/* Modula-2 */
+        lang_mask = CU_LOADED_M2_LANG_MASK;
+      break;
+      case language_asm:		/* Assembly language */
+        lang_mask = CU_LOADED_ASM_LANG_MASK;
+      break;
+      case language_pascal:		/* Pascal */
+        lang_mask = CU_LOADED_PASCAL_LANG_MASK;
+      break;
+      case language_ada:		/* Ada */
+        lang_mask = CU_LOADED_ADA_LANG_MASK;
+      break;
+      case language_scm:		/* Guile Scheme*/
+        lang_mask = CU_LOADED_SCM_LANG_MASK;
+      break;
+      default:
+	return 0;
+    }
+  return lang_mask;
+}
+
+unsigned int
+language_has_cu_loaded (enum language lang)
+{
+  unsigned int lang_mask = mask_for_language (lang);
+  return cu_languages_loaded_mask & lang_mask;
+}
+
+void
+set_language_has_cu_loaded (enum language lang)
+{
+  unsigned int lang_mask = mask_for_language (lang);
+  cu_languages_loaded_mask |= lang_mask;
+}
+
 /* Define the language that is no language.  */
 
 static int
diff --git a/gdb/language.h b/gdb/language.h
index aa0523b..6fde401 100644
--- a/gdb/language.h
+++ b/gdb/language.h
@@ -523,4 +523,12 @@ void default_get_string (struct value *value, gdb_byte **buffer, int *length,
 void c_get_string (struct value *value, gdb_byte **buffer, int *length,
 		   struct type **char_type, const char **charset);
 
+/* Returns non-zero if cu of the language type has been previously loaded. */
+unsigned int
+language_has_cu_loaded (enum language lang);
+
+/* Sets that a cu of the language type has been previously loaded. */
+void
+set_language_has_cu_loaded (enum language lang);
+
 #endif /* defined (LANGUAGE_H) */
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 91c5b90..b74c997 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -771,14 +771,15 @@ decode_line_1 (char **argptr, int funfirstline, struct symtab *default_symtab,
 
   /* Check if the symbol could be an Objective-C selector.  */
 
-  {
-    struct symtabs_and_lines values;
+  if (is_objc_method || language_has_cu_loaded(language_objc))
+    {
+      struct symtabs_and_lines values;
 
-    values = decode_objc (argptr, funfirstline, NULL,
+      values = decode_objc (argptr, funfirstline, NULL,
 			  canonical, saved_arg);
-    if (values.sals != NULL)
-      return values;
-  }
+      if (values.sals != NULL)
+        return values;
+    }
 
   /* Does it look like there actually were two parts?  */
 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: disable objective-c stuff when theres no objective-c cu.
  2010-10-06  2:30     ` Matt Rice
@ 2010-10-06  8:51       ` Jan Kratochvil
  2010-10-06 10:03         ` Matt Rice
  0 siblings, 1 reply; 11+ messages in thread
From: Jan Kratochvil @ 2010-10-06  8:51 UTC (permalink / raw)
  To: Matt Rice; +Cc: Michael Snyder, gdb-patches

On Wed, 06 Oct 2010 04:30:33 +0200, Matt Rice wrote:
> that we can lessen the impact by checking for is_objc_method, or for
> objective-c compilation units. (is_objc_method doesn't neccesarily
> affect future lookups should it call set_language_has_cu_loaded?)

Wouldn't be enough to replace the bitmask just by?
	if (current_language->la_language == language_objc)


> here is when gcc started to emit DW_LANG_ObjC, maybe we could fall
> back to source files with the .m extension.
> http://gcc.gnu.org/ml/gcc-patches/2006-03/msg01515.html

For current_language it works even without DW_LANG_ObjC due to:
	init_filename_language_table (void)
	      add_filename_language (".m", language_objc);


> -PASS: gdb.cp/psmang.exp: break s::method2
> +FAIL: gdb.cp/psmang.exp: break s::method2 (got interactive prompt)

There is also a regression for:
	gdb.ada/frame_args.exp
	gdb.ada/ptype_tagged_param.exp
	gdb.ada/ref_param.exp


> -FAIL: gdb.threads/attachstop-mt.exp: attach1, post-gdb sanity check of the sleeping state - Red Hat BZ 197584
> +PASS: gdb.threads/attachstop-mt.exp: attach1, post-gdb sanity check of the sleeping state - Red Hat BZ 197584

This one is racy, I should fix it.


Thanks,
Jan


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: disable objective-c stuff when theres no objective-c cu.
  2010-10-06  8:51       ` Jan Kratochvil
@ 2010-10-06 10:03         ` Matt Rice
  2010-10-06 13:12           ` Matt Rice
  0 siblings, 1 reply; 11+ messages in thread
From: Matt Rice @ 2010-10-06 10:03 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: Michael Snyder, gdb-patches

On Wed, Oct 6, 2010 at 1:51 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Wed, 06 Oct 2010 04:30:33 +0200, Matt Rice wrote:
>> that we can lessen the impact by checking for is_objc_method, or for
>> objective-c compilation units. (is_objc_method doesn't neccesarily
>> affect future lookups should it call set_language_has_cu_loaded?)
>
> Wouldn't be enough to replace the bitmask just by?
>        if (current_language->la_language == language_objc)

objective-c uses mixed compilation units possibly more than most other
languages,
e.g. objc++ can contain c++ and objc compilation units in the same file
which we don't emit dwarf for yet, but there has been some recent work for lto
I was watching which looks to make this possibly fixable, so basing it
off of 'current_language'
means you may have to either use the explicit long form (discussed in
news), or conversely
it is possible that the short form would be able to both work/not work
at different times when debugging

currently you are able to set a breakpoint from languages regardless
of the current one
everywhere else in the decode_line_1 path, e.g. i can set a c++ one
with the current language objective-c.
and vice versa (well maybe not the decode_variable line with
'(implicit this->)foo'

personally I think that having break work consistently across
compilation units is nice,
and tried to get somewhere in the middle between working all the time everywhere
and working based on current language.

>> here is when gcc started to emit DW_LANG_ObjC, maybe we could fall
>> back to source files with the .m extension.
>> http://gcc.gnu.org/ml/gcc-patches/2006-03/msg01515.html
>
> For current_language it works even without DW_LANG_ObjC due to:
>        init_filename_language_table (void)
>              add_filename_language (".m", language_objc);

maybe the dwarf2read call to set_language_has_cu_loaded is redundant
then since i added the call from set_language


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: disable objective-c stuff when theres no objective-c cu.
  2010-10-06 10:03         ` Matt Rice
@ 2010-10-06 13:12           ` Matt Rice
  2010-10-06 17:06             ` Joel Brobecker
  2010-10-17 20:13             ` Jan Kratochvil
  0 siblings, 2 replies; 11+ messages in thread
From: Matt Rice @ 2010-10-06 13:12 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: Michael Snyder, gdb-patches

[-- Attachment #1: Type: text/plain, Size: 2111 bytes --]

On Wed, Oct 6, 2010 at 3:03 AM, Matt Rice <ratmice@gmail.com> wrote:
> On Wed, Oct 6, 2010 at 1:51 AM, Jan Kratochvil
> <jan.kratochvil@redhat.com> wrote:
>> On Wed, 06 Oct 2010 04:30:33 +0200, Matt Rice wrote:
>>> that we can lessen the impact by checking for is_objc_method, or for
>>> objective-c compilation units. (is_objc_method doesn't neccesarily
>>> affect future lookups should it call set_language_has_cu_loaded?)
>>
>> Wouldn't be enough to replace the bitmask just by?
>>        if (current_language->la_language == language_objc)
>
> objective-c uses mixed compilation units possibly more than most other
> languages,

<snip>
i could have argued this more concisely,
what I mean is that 'break' is not related to the current language,
but the language which we want to be the current language
when the breakpoint is hit.  and so 'set language' to use language
specific breakpoints is arguably wrong because the current language
may not be the language of the breakpoint we want set.


>>> here is when gcc started to emit DW_LANG_ObjC, maybe we could fall
>>> back to source files with the .m extension.
>>> http://gcc.gnu.org/ml/gcc-patches/2006-03/msg01515.html
>>
>> For current_language it works even without DW_LANG_ObjC due to:
>>        init_filename_language_table (void)
>>              add_filename_language (".m", language_objc);
>
> maybe the dwarf2read call to set_language_has_cu_loaded is redundant
> then since i added the call from set_language
>

doesn't seem as though reading language symfiles frobs the current
language afaict.
attached should fix GDB's pre-DW_LANG_ObjC, tested it with stabs,
don't have an old
enough compiler to test the others easily though.

attached calls set_language_has_cu_loaded when calling
deduce_language_from_filename.
except for the symtab case, i dont think we need it there since we
should already have called it
when making psytmabs.  Thanks for the pointer.

for some reason i installed gnat/libgnat-devel but still not getting
ada testsuite :/
i'll get started looking at the new failures.

[-- Attachment #2: gdb-lang-objc.diff --]
[-- Type: application/octet-stream, Size: 8690 bytes --]

diff --git a/gdb/NEWS b/gdb/NEWS
index 85059c6..2369752 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -23,6 +23,25 @@
      feature requires proper debuginfo support from the compiler; it
      was added to GCC 4.5.
 
+* Objective-C changes:
+
+  ** GDB Objective-C language support has changed so that lookups of
+     Objective-c methods is now done if any of the following conditions is true:
+
+       * GDB has identified Objective-C code as having been loaded.
+
+       * The user has specified an Objective-c method explicitly in the form of
+         `break +[Class aMethod]' or `break -[Class aMethod]' as opposed to
+         the short form `break aMethod'.
+
+       * The user has called 'set language objective-c' at least once;
+         though it need not be the current language.
+
+     For GDB to identify an Objective-C library debug symbols must be present.
+     Users wanting to retain the old behaviour can add
+     `set language objective-c` followed by `set language auto' lines
+     to their .gdbinit file.
+
 * GDB now has some support for using labels in the program's source in
   linespecs.  For instance, you can use "advance label" to continue
   execution to a label.
diff --git a/gdb/buildsym.c b/gdb/buildsym.c
index 4a76e3e..0c9186d 100644
--- a/gdb/buildsym.c
+++ b/gdb/buildsym.c
@@ -44,6 +44,7 @@
 #include "cp-support.h"
 #include "dictionary.h"
 #include "addrmap.h"
+#include "language.h"
 
 /* Ask buildsym.h to define the vars it normally declares `extern'.  */
 #define	EXTERN
@@ -621,6 +622,9 @@ start_subfile (const char *name, const char *dirname)
     {
       subfile->language = subfile->next->language;
     }
+
+  set_language_has_cu_loaded(subfile->language);
+
 }
 
 /* For stabs readers, the first N_SO symbol is assumed to be the
diff --git a/gdb/dbxread.c b/gdb/dbxread.c
index af94659..57e9b29 100644
--- a/gdb/dbxread.c
+++ b/gdb/dbxread.c
@@ -56,6 +56,7 @@
 #include "cp-abi.h"
 #include "cp-support.h"
 #include "psympriv.h"
+#include "language.h"
 
 #include "gdb_assert.h"
 #include "gdb_string.h"
@@ -1564,6 +1565,7 @@ read_dbx_symtab (struct objfile *objfile)
 
 	    namestring = set_namestring (objfile, &nlist);
 	    tmp_language = deduce_language_from_filename (namestring);
+	    set_language_has_cu_loaded (tmp_language);
 
 	    /* Only change the psymtab's language if we've learned
 	       something useful (eg. tmp_language is not language_unknown).
diff --git a/gdb/defs.h b/gdb/defs.h
index 9e4800c..304d1c9 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -185,7 +185,9 @@ extern void quit (void);
 /* Languages represented in the symbol table and elsewhere.
    This should probably be in language.h, but since enum's can't
    be forward declared to satisfy opaque references before their
-   actual definition, needs to be here. */
+   actual definition, needs to be here.
+
+   When adding a tangible language also update language.c:mask_for_language */
 
 enum language
   {
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index bf36e01..e0d4d52 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -9646,6 +9646,7 @@ set_cu_language (unsigned int lang, struct dwarf2_cu *cu)
       break;
     }
   cu->language_defn = language_def (cu->language);
+  set_language_has_cu_loaded (cu->language);
 }
 
 /* Return the named attribute or NULL if not there.  */
diff --git a/gdb/language.c b/gdb/language.c
index 3ce08b5..cdc69c9 100644
--- a/gdb/language.c
+++ b/gdb/language.c
@@ -435,6 +435,7 @@ set_language (enum language lang)
       if (languages[i]->la_language == lang)
 	{
 	  current_language = languages[i];
+	  set_language_has_cu_loaded(current_language->la_language);
 	  set_type_range_case ();
 	  break;
 	}
@@ -1060,6 +1061,82 @@ default_get_string (struct value *value, gdb_byte **buffer, int *length,
   error (_("Getting a string is unsupported in this language."));
 }
 
+
+/* A mask for languages that want to enable specific behaviours if (not when)
+   a compilation unit of that language has been loaded. */
+#define CU_LOADED_C_LANG_MASK		0x1 << 0
+#define CU_LOADED_CPLUS_LANG_MASK	0x1 << 1
+#define CU_LOADED_D_LANG_MASK		0x1 << 2
+#define CU_LOADED_OBJC_LANG_MASK	0x1 << 3
+#define CU_LOADED_JAVA_LANG_MASK	0x1 << 4
+#define CU_LOADED_FORTRAN_LANG_MASK	0x1 << 5
+#define CU_LOADED_M2_LANG_MASK		0x1 << 6
+#define CU_LOADED_ASM_LANG_MASK		0x1 << 7
+#define CU_LOADED_PASCAL_LANG_MASK	0x1 << 8
+#define CU_LOADED_ADA_LANG_MASK		0x1 << 9
+#define CU_LOADED_SCM_LANG_MASK		0x1 << 10
+
+static unsigned int cu_languages_loaded_mask;
+
+static unsigned int
+mask_for_language(enum language lang)
+{
+  unsigned int lang_mask;
+  switch(lang)
+    {
+      case language_c:			/* C */
+        lang_mask = CU_LOADED_C_LANG_MASK;
+      break;
+      case language_cplus:		/* C++ */
+        lang_mask = CU_LOADED_CPLUS_LANG_MASK;
+      break;
+      case language_d:			/* D */
+        lang_mask = CU_LOADED_D_LANG_MASK;
+      break;
+      case language_objc:		/* Objective-C */
+        lang_mask = CU_LOADED_OBJC_LANG_MASK;
+      break;
+      case language_java:		/* Java */
+        lang_mask = CU_LOADED_JAVA_LANG_MASK;
+      break;
+      case language_fortran:		/* Fortran */
+        lang_mask = CU_LOADED_FORTRAN_LANG_MASK;
+      break;
+      case language_m2:			/* Modula-2 */
+        lang_mask = CU_LOADED_M2_LANG_MASK;
+      break;
+      case language_asm:		/* Assembly language */
+        lang_mask = CU_LOADED_ASM_LANG_MASK;
+      break;
+      case language_pascal:		/* Pascal */
+        lang_mask = CU_LOADED_PASCAL_LANG_MASK;
+      break;
+      case language_ada:		/* Ada */
+        lang_mask = CU_LOADED_ADA_LANG_MASK;
+      break;
+      case language_scm:		/* Guile Scheme*/
+        lang_mask = CU_LOADED_SCM_LANG_MASK;
+      break;
+      default:
+	return 0;
+    }
+  return lang_mask;
+}
+
+unsigned int
+language_has_cu_loaded (enum language lang)
+{
+  unsigned int lang_mask = mask_for_language (lang);
+  return cu_languages_loaded_mask & lang_mask;
+}
+
+void
+set_language_has_cu_loaded (enum language lang)
+{
+  unsigned int lang_mask = mask_for_language (lang);
+  cu_languages_loaded_mask |= lang_mask;
+}
+
 /* Define the language that is no language.  */
 
 static int
diff --git a/gdb/language.h b/gdb/language.h
index aa0523b..6fde401 100644
--- a/gdb/language.h
+++ b/gdb/language.h
@@ -523,4 +523,12 @@ void default_get_string (struct value *value, gdb_byte **buffer, int *length,
 void c_get_string (struct value *value, gdb_byte **buffer, int *length,
 		   struct type **char_type, const char **charset);
 
+/* Returns non-zero if cu of the language type has been previously loaded. */
+unsigned int
+language_has_cu_loaded (enum language lang);
+
+/* Sets that a cu of the language type has been previously loaded. */
+void
+set_language_has_cu_loaded (enum language lang);
+
 #endif /* defined (LANGUAGE_H) */
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 91c5b90..b74c997 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -771,14 +771,15 @@ decode_line_1 (char **argptr, int funfirstline, struct symtab *default_symtab,
 
   /* Check if the symbol could be an Objective-C selector.  */
 
-  {
-    struct symtabs_and_lines values;
+  if (is_objc_method || language_has_cu_loaded(language_objc))
+    {
+      struct symtabs_and_lines values;
 
-    values = decode_objc (argptr, funfirstline, NULL,
+      values = decode_objc (argptr, funfirstline, NULL,
 			  canonical, saved_arg);
-    if (values.sals != NULL)
-      return values;
-  }
+      if (values.sals != NULL)
+        return values;
+    }
 
   /* Does it look like there actually were two parts?  */
 
diff --git a/gdb/mdebugread.c b/gdb/mdebugread.c
index 5ce5db2..3136940 100644
--- a/gdb/mdebugread.c
+++ b/gdb/mdebugread.c
@@ -58,6 +58,7 @@
 #include "gdb_stat.h"
 #include "gdb_string.h"
 #include "psympriv.h"
+#include "language.h"
 
 #include "bfd.h"
 
@@ -2700,6 +2701,9 @@ parse_partial_symbols (struct objfile *objfile)
 	  psymtab_language = deduce_language_from_filename (fdr_name (fh));
 	  break;
 	}
+
+      set_language_has_cu_loaded (psymtab_language);
+
       if (psymtab_language == language_unknown)
 	psymtab_language = prev_language;
       PST_PRIVATE (pst)->pst_language = psymtab_language;
diff --git a/gdb/xcoffread.c b/gdb/xcoffread.c
index 902d48f..541ec22 100644
--- a/gdb/xcoffread.c
+++ b/gdb/xcoffread.c
@@ -1973,6 +1973,7 @@ xcoff_start_psymtab (struct objfile *objfile, char *filename, int first_symnum,
 
   /* Deduce the source language from the filename for this psymtab. */
   psymtab_language = deduce_language_from_filename (filename);
+  set_language_has_cu_loaded (psymtab_language);
 
   return result;
 }

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: disable objective-c stuff when theres no objective-c cu.
  2010-10-06 13:12           ` Matt Rice
@ 2010-10-06 17:06             ` Joel Brobecker
  2010-10-06 17:54               ` Matt Rice
  2010-10-17 20:13             ` Jan Kratochvil
  1 sibling, 1 reply; 11+ messages in thread
From: Joel Brobecker @ 2010-10-06 17:06 UTC (permalink / raw)
  To: Matt Rice; +Cc: Jan Kratochvil, Michael Snyder, gdb-patches

> i could have argued this more concisely,
> what I mean is that 'break' is not related to the current language,
> but the language which we want to be the current language
> when the breakpoint is hit.  and so 'set language' to use language
> specific breakpoints is arguably wrong because the current language
> may not be the language of the breakpoint we want set.

But at the same time, I don't think we want to be able to support
all languages at the same time. If we can guess the language of
the location, great, but I think that's sort of a chicken-and-egg
problem. You need to parse the location to determine the language,
but you need to determine which language to use in order to parse it.

IMO, it's much cleaner to follow the current-language when parsing
the breakpoint location.  Various languages may want to provide
bridges to other syntaxes (for instance, the Obj-C language might
want to provide c-like breakpoint expressions), but I don't think
that this should be part of the general code.  More particularly,
I don't think that linespect should have to handle "break [foo]"
when not in objc mode.

-- 
Joel


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: disable objective-c stuff when theres no objective-c cu.
  2010-10-06 17:06             ` Joel Brobecker
@ 2010-10-06 17:54               ` Matt Rice
  0 siblings, 0 replies; 11+ messages in thread
From: Matt Rice @ 2010-10-06 17:54 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Jan Kratochvil, Michael Snyder, gdb-patches

On Wed, Oct 6, 2010 at 10:06 AM, Joel Brobecker <brobecker@adacore.com> wrote:
>> i could have argued this more concisely,
>> what I mean is that 'break' is not related to the current language,
>> but the language which we want to be the current language
>> when the breakpoint is hit.  and so 'set language' to use language
>> specific breakpoints is arguably wrong because the current language
>> may not be the language of the breakpoint we want set.
>
> But at the same time, I don't think we want to be able to support
> all languages at the same time.

yeah, thats what we currently do though, so this is a much larger
change of rewriting linespec i suppose.

> If we can guess the language of
> the location, great, but I think that's sort of a chicken-and-egg
> problem. You need to parse the location to determine the language,
> but you need to determine which language to use in order to parse it.
>
> IMO, it's much cleaner to follow the current-language when parsing
> the breakpoint location.  Various languages may want to provide
> bridges to other syntaxes (for instance, the Obj-C language might
> want to provide c-like breakpoint expressions), but I don't think
> that this should be part of the general code.  More particularly,
> I don't think that linespect should have to handle "break [foo]"
> when not in objc mode.

I'm alright with 'inferring' the linespec parsing based on the current
language I suppose, I just don't think that the user should have to
'set language objective-c' to set a breakpoint in a language which is
*not* the current language, because if the user were to subsequently
do
p foo
and the scope was a c++ class,
then it would look for self->foo instead of this->foo.

so if that means something like 'break objc:[something orOther]'
and being able to drop the objc: if the current language is objc thats ok IMO.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: disable objective-c stuff when theres no objective-c cu.
  2010-10-06 13:12           ` Matt Rice
  2010-10-06 17:06             ` Joel Brobecker
@ 2010-10-17 20:13             ` Jan Kratochvil
  2010-10-17 21:57               ` Matt Rice
  1 sibling, 1 reply; 11+ messages in thread
From: Jan Kratochvil @ 2010-10-17 20:13 UTC (permalink / raw)
  To: Matt Rice; +Cc: Michael Snyder, gdb-patches

On Wed, 06 Oct 2010 15:12:06 +0200, Matt Rice wrote:
> what I mean is that 'break' is not related to the current language,
> but the language which we want to be the current language
> when the breakpoint is hit.  and so 'set language' to use language
> specific breakpoints is arguably wrong because the current language
> may not be the language of the breakpoint we want set.

OK, I understand that Obj-C is special that it (a) is mixed with C/C++ and
even possibly having C/C++ main(), (b) requiring Obj-C specific support to
just figure out where to place a breakpoint.

That `objc:' breakpoint prefix etc. could be nice but a more conservative
change IMO also makes sense.  I can imagine it could be inconvenient for Obj-C
development (which I do not know myself, though).


> doesn't seem as though reading language symfiles frobs the current
> language afaict.

It really does not.


+  set_language_has_cu_loaded(subfile->language);

here should be a space: `...loaded (subfile->...'


+/* A mask for languages that want to enable specific behaviours if (not when)
+   a compilation unit of that language has been loaded. */
+#define CU_LOADED_C_LANG_MASK		0x1 << 0
+#define CU_LOADED_CPLUS_LANG_MASK	0x1 << 1

Do I miss something why don't you use enum language like (1 << language_objc)?
nr_languages == 14 so it fits fine into a bitmask.  BTW such #define right
hand sides should be in parentheses (for operator priority surprises during
their use).


+static unsigned int cu_languages_loaded_mask;

One can imagine this mask may (possibly in the future) modify user-visible
behavior due to a different code paths being executed due to it - which is
also its purpose.

Loading program A, kill, loading program B would behave differently than just
loading program B with fresh GDB.

I would place such mask into `struct objfile'.  Checking its content then
means iterating ALL_OBJFILES but I would find it acceptable.  (Some
accelerations of such scheme are also possible - when performance is a goal of
this patch anyway.)  This would make the GDB behavior more deterministic IMO.


+unsigned int
+language_has_cu_loaded (enum language lang)
+{
+  unsigned int lang_mask = mask_for_language (lang);
+  return cu_languages_loaded_mask & lang_mask;
+}

As it returns boolean it should be just `int' and I would make it normalized
to 0-or-1.


Thanks,
Jan


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: disable objective-c stuff when theres no objective-c cu.
  2010-10-17 20:13             ` Jan Kratochvil
@ 2010-10-17 21:57               ` Matt Rice
  0 siblings, 0 replies; 11+ messages in thread
From: Matt Rice @ 2010-10-17 21:57 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: Michael Snyder, gdb-patches

On Sun, Oct 17, 2010 at 1:12 PM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Wed, 06 Oct 2010 15:12:06 +0200, Matt Rice wrote:
>> what I mean is that 'break' is not related to the current language,
>> but the language which we want to be the current language
>> when the breakpoint is hit.  and so 'set language' to use language
>> specific breakpoints is arguably wrong because the current language
>> may not be the language of the breakpoint we want set.
>
> OK, I understand that Obj-C is special that it (a) is mixed with C/C++ and
> even possibly having C/C++ main(), (b) requiring Obj-C specific support to
> just figure out where to place a breakpoint.
>
> That `objc:' breakpoint prefix etc. could be nice but a more conservative
> change IMO also makes sense.  I can imagine it could be inconvenient for Obj-C
> development (which I do not know myself, though).

I tend to agree, with the conservative approach,
requiring objc: sometimes could lead to an inconsistent user experience
I prefer this approach because of the minimal user impact, yet we get
the bulk of objc_decode
out of the way of non-objc users.

>> doesn't seem as though reading language symfiles frobs the current
>> language afaict.
>
> It really does not.
>
>
> +  set_language_has_cu_loaded(subfile->language);
>
> here should be a space: `...loaded (subfile->...'

k, will fix that

> +/* A mask for languages that want to enable specific behaviours if (not when)
> +   a compilation unit of that language has been loaded. */
> +#define CU_LOADED_C_LANG_MASK          0x1 << 0
> +#define CU_LOADED_CPLUS_LANG_MASK      0x1 << 1
>
> Do I miss something why don't you use enum language like (1 << language_objc)?
> nr_languages == 14 so it fits fine into a bitmask.  BTW such #define right
> hand sides should be in parentheses (for operator priority surprises during
> their use).

before i spin a new patch, lets resolve this, the reason I used a
define instead of an enum
was because I vaguely reember weird differences wrt c and c++ enum use
as numerical values.
 Not sure if this case is actually hit by that, I just figured that
define was relatively future-proof,

if a bit ugly, i should probably actually try it with c++.
I definitely prefer enum to what i did here.  so yeah, there was a
reason but its possibly
irrelevent and/or unecessary.

> +static unsigned int cu_languages_loaded_mask;
>
> One can imagine this mask may (possibly in the future) modify user-visible
> behavior due to a different code paths being executed due to it - which is
> also its purpose.
>
> Loading program A, kill, loading program B would behave differently than just
> loading program B with fresh GDB.
>
> I would place such mask into `struct objfile'.  Checking its content then
> means iterating ALL_OBJFILES but I would find it acceptable.  (Some
> accelerations of such scheme are also possible - when performance is a goal of
> this patch anyway.)  This would make the GDB behavior more deterministic IMO.

ahh, I had totally spaced on this and the multi-process impact of this
change, thanks
will look into this.

> +unsigned int
> +language_has_cu_loaded (enum language lang)
> +{
> +  unsigned int lang_mask = mask_for_language (lang);
> +  return cu_languages_loaded_mask & lang_mask;
> +}
>
> As it returns boolean it should be just `int' and I would make it normalized
> to 0-or-1.

k


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2010-10-17 21:57 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-10-05 22:37 disable objective-c stuff when theres no objective-c cu Matt Rice
2010-10-05 23:31 ` Michael Snyder
2010-10-05 23:35   ` Matt Rice
2010-10-06  2:30     ` Matt Rice
2010-10-06  8:51       ` Jan Kratochvil
2010-10-06 10:03         ` Matt Rice
2010-10-06 13:12           ` Matt Rice
2010-10-06 17:06             ` Joel Brobecker
2010-10-06 17:54               ` Matt Rice
2010-10-17 20:13             ` Jan Kratochvil
2010-10-17 21:57               ` Matt Rice

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox