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]

Re: [patch] Correct `gcc -D' macros get ignored (PR 9873)


Hi Joel,

On Sat, 28 Feb 2009 08:15:14 +0100, Joel Brobecker wrote:
> > patched GCC + patched GDB:
> > (gdb) info macro __STDC__
> > Defined at ../.././gdb/testsuite/gdb.base/macscp1.c:0
> > #define __STDC__ 1
> 
> Before we go into the details of the patch itself, I don't think
> the behavior you are suggesting is right. Or at least I find it
> confusing. I was expecting GDB to say "Defined from the command line"
> or something like this.

I even had such patch I made first (attached - just FYI, it is not intended
for GDB inclusion).  Reasons why I rather chose the more simple line 0:

* Just some text like "command-line" would be hiding a DWARF-contained
  information from the user - each macro command-line macro is still
  associated with a compilation unit (CU).  In this context I find the Base
  Source (see below) file name as the appropriate identifier of this CU.

* At least the current testcase expects <filename>:<lineno> syntax.
  Changins this `\S+:\d+' pattern may break some frontends although I see no
  current MI way to access the macro information at all.

Should I resubmit the patch (using pmacroloc()) with this syntax?
(gdb) info macro __STDC__
Defined at ../.././gdb/testsuite/gdb.base/macscp1.c:command-line
#define __STDC__ 1


> Regarding the patch itself, I'm a little unsure.  Are you sure that
> the first start_file entry will necessarily be the actual source file?

According to http://dwarf.freestandards.org/Dwarf3.pdf page 118/267:
# 6.3.2 Base Source Entries
# In addition to producing a matched pair of DW_MACINFO_start_file and
# DW_MACINFO_end_file entries for each inclusion directive actually processed
# during compilation, a producer should generate such a matched pair also for
# the “base” source file submitted to the compiler for compilation.  [...]

I find it as defined each .debug_macro section must contain exactly one
level-0 pair of DW_MACINFO_start_file / DW_MACINFO_end_file entries - the pair
for Base Source.


> However, I wonder if this is the right approach.  What do you think
> if we had a special macro_source_file for the command line?

Yes although the field `filename' will still have to contain the same
`filename' as its child Base Source Entry.  As the command-line vs. in-file
entries are identifiable by their zero vs. non-zero line numbers respectively
(6.3.3) according to the standard I find such extra new `macro_source_file' as
technically excessive.

I am not against it, going to provide such patch if you still think the
separate command-line `macro_source_file' would make the code look better.


> > +# Workaround ccache making lineno non-zero for command-line definitions.
> > +if {[find_gcc] == "gcc" && [file executable "/usr/bin/gcc"]} {
> > +    set result [catch "exec which gcc" output]
> > +    if {$result == 0 && [string first "/ccache/" $output] >= -1} {
> > +	lappend options "compiler=/usr/bin/gcc"
> > +    }
> > +}
> 
> Can you explain what this problem is about?

Normal (non-ccache) compilation:
gcc -DFROM_COMMANDLINE -g3 -c -o g3testold.o g3.c
readelf -Wwm g3testold.o
 DW_MACINFO_define - lineno : 0 macro : __STDC__ 1
 DW_MACINFO_define - lineno : 0 macro : __STDC_HOSTED__ 1
[...]

Compilation through ccache-2.4-13.fc9.x86_64:
gcc -DFROM_COMMANDLINE -g3 -c -o g3testold.o g3.c
readelf -Wwm g3testold.o
 DW_MACINFO_define - lineno : 1 macro : __STDC__ 1
 DW_MACINFO_define - lineno : 2 macro : __STDC_HOSTED__ 1
[...]

__STDC__+__STDC_HOSTED__+others are definitely the command-line defined macros.
The possible DW_MACINFO_start_file entry was omitted above as its placement is
a completely different problem than the lineno problem.


> Basically, when the gcc used is "gcc" but you have a "/usr/bin/gcc" on your
> system, you check which "gcc" is on the path, and if the patch contains the
> ccache directory, then you force gcc to /usr/bin/gcc???

# yum -y install ccache; which gcc
/usr/lib64/ccache/gcc

Partially I think this workaround may be more appropriate as
distribution/vendor specific one so it may be dropped if you think so.


> I'll take a look again if we determine that they test the behavior
> we want.

Yes, I would also like to find a consensus on the requirements first.


Thanks for the review,
Jan


[ this specific patch is NOT intended for review/inclusion ]

--- gdb/defs.h	6 Feb 2009 22:50:52 -0000	1.245
+++ gdb/defs.h	12 Feb 2009 22:14:01 -0000
@@ -409,6 +409,8 @@ char *ldirname (const char *filename);
 
 char **gdb_buildargv (const char *);
 
+char *pmacroloc (const char *filename, int line);
+
 /* From demangle.c */
 
 extern void set_demangling_style (char *);
--- gdb/dwarf2read.c	12 Feb 2009 09:15:06 -0000	1.295
+++ gdb/dwarf2read.c	12 Feb 2009 22:14:05 -0000
@@ -9756,15 +9756,26 @@ file_full_name (int file, struct line_he
     }
 }
 
+/* Create new MACRO_SOURCE_FILE.  FILE_PTR can be NULL to prepare for
+   command-line macros specified on the compiler command-line (before first
+   DW_MACINFO_start_file).  */
 
 static struct macro_source_file *
-macro_start_file (int file, int line,
+macro_start_file (const int *file_ptr, int line,
                   struct macro_source_file *current_file,
                   const char *comp_dir,
                   struct line_header *lh, struct objfile *objfile)
 {
   /* The full name of this source file.  */
-  char *full_name = file_full_name (file, lh, comp_dir);
+  char *full_name;
+  
+  if (file_ptr)
+    full_name = file_full_name (*file_ptr, lh, comp_dir);
+  else
+    {
+      full_name = NULL;
+      gdb_assert (! pending_macros);
+    }
 
   /* We don't create a macro table for this compilation unit
      at all until we actually get a filename.  */
@@ -10008,12 +10019,18 @@ dwarf_decode_macros (struct line_header 
             mac_ptr += bytes_read;
 
             if (! current_file)
-	      complaint (&symfile_complaints,
-			 _("debug info gives macro %s outside of any file: %s"),
-			 macinfo_type ==
-			 DW_MACINFO_define ? "definition" : macinfo_type ==
-			 DW_MACINFO_undef ? "undefinition" :
-			 "something-or-other", body);
+	      {
+	        if (line != 0)
+		  complaint (&symfile_complaints,
+			     _("debug info gives command-line macro %s "
+			       "with non-zero line %d: %s"),
+			     macinfo_type ==
+			     DW_MACINFO_define ? "definition" : macinfo_type ==
+			     DW_MACINFO_undef ? "undefinition" :
+			     "something-or-other", line, body);
+		current_file = macro_start_file (NULL, 0, current_file,
+						 comp_dir, lh, cu->objfile);
+	      }
             else
               {
                 if (macinfo_type == DW_MACINFO_define)
@@ -10034,9 +10051,8 @@ dwarf_decode_macros (struct line_header 
             file = read_unsigned_leb128 (abfd, mac_ptr, &bytes_read);
             mac_ptr += bytes_read;
 
-            current_file = macro_start_file (file, line,
-                                             current_file, comp_dir,
-                                             lh, cu->objfile);
+            current_file = macro_start_file (&file, line, current_file,
+					     comp_dir, lh, cu->objfile);
           }
           break;
 
--- gdb/macrocmd.c	3 Jan 2009 05:57:52 -0000	1.19
+++ gdb/macrocmd.c	12 Feb 2009 22:14:06 -0000
@@ -121,13 +121,13 @@ show_pp_source_pos (struct ui_file *stre
                     struct macro_source_file *file,
                     int line)
 {
-  fprintf_filtered (stream, "%s:%d\n", file->filename, line);
+  fprintf_filtered (stream, "%s\n", pmacroloc (file->filename, line));
 
   while (file->included_by)
     {
-      fprintf_filtered (gdb_stdout, "  included at %s:%d\n",
-                        file->included_by->filename,
-                        file->included_at_line);
+      fprintf_filtered (gdb_stdout, "  included at %s\n",
+                        pmacroloc (file->included_by->filename,
+				   file->included_at_line));
       file = file->included_by;
     }
 }
--- gdb/macrotab.c	3 Jan 2009 05:57:52 -0000	1.18
+++ gdb/macrotab.c	12 Feb 2009 22:14:06 -0000
@@ -383,7 +383,8 @@ new_source_file (struct macro_table *t,
 
   memset (f, 0, sizeof (*f));
   f->table = t;
-  f->filename = macro_bcache_str (t, filename);
+  if (filename)
+    f->filename = macro_bcache_str (t, filename);
   f->includes = 0;
 
   return f;
@@ -403,7 +404,8 @@ free_macro_source_file (struct macro_sou
       free_macro_source_file (child);
     }
 
-  macro_bcache_free (src->table, (char *) src->filename);
+  if (src->filename)
+    macro_bcache_free (src->table, (char *) src->filename);
   macro_free (src, src->table);
 }
 
@@ -467,8 +469,11 @@ macro_include (struct macro_source_file 
 
          First, squawk.  */
       complaint (&symfile_complaints,
-		 _("both `%s' and `%s' allegedly #included at %s:%d"), included,
-		 (*link)->filename, source->filename, line);
+		 _("both `%s' and `%s' allegedly #included at %s"),
+		 /* NULL command-line cannot reach it here.  */
+		 included ? included : "(nil)",
+		 (*link)->filename ? (*link)->filename : "(nil)",
+		 pmacroloc (source->filename, line));
 
       /* Now, choose a new, unoccupied line number for this
          #inclusion, after the alleged #inclusion line.  */
@@ -497,23 +502,24 @@ struct macro_source_file *
 macro_lookup_inclusion (struct macro_source_file *source, const char *name)
 {
   /* Is SOURCE itself named NAME?  */
-  if (strcmp (name, source->filename) == 0)
+  if (source->filename && strcmp (name, source->filename) == 0)
     return source;
 
   /* The filename in the source structure is probably a full path, but
      NAME could be just the final component of the name.  */
-  {
-    int name_len = strlen (name);
-    int src_name_len = strlen (source->filename);
+  if (source->filename)
+    {
+      int name_len = strlen (name);
+      int src_name_len = strlen (source->filename);
 
-    /* We do mean < here, and not <=; if the lengths are the same,
-       then the strcmp above should have triggered, and we need to
-       check for a slash here.  */
-    if (name_len < src_name_len
-        && source->filename[src_name_len - name_len - 1] == '/'
-        && strcmp (name, source->filename + src_name_len - name_len) == 0)
-      return source;
-  }
+      /* We do mean < here, and not <=; if the lengths are the same,
+	 then the strcmp above should have triggered, and we need to
+	 check for a slash here.  */
+      if (name_len < src_name_len
+	  && source->filename[src_name_len - name_len - 1] == '/'
+	  && strcmp (name, source->filename + src_name_len - name_len) == 0)
+	return source;
+    }
 
   /* It's not us.  Try all our children, and return the lowest.  */
   {
@@ -726,9 +732,10 @@ check_for_redefinition (struct macro_sou
       if (! same)
         {
 	  complaint (&symfile_complaints,
-		     _("macro `%s' redefined at %s:%d; original definition at %s:%d"),
-		     name, source->filename, line,
-		     found_key->start_file->filename, found_key->start_line);
+		     _("macro `%s' redefined at %s; original definition at %s"),
+		     name, pmacroloc (source->filename, line),
+		     pmacroloc (found_key->start_file->filename,
+				found_key->start_line));
         }
 
       return found_key;
@@ -828,11 +835,9 @@ macro_undef (struct macro_source_file *s
           if (key->end_file)
             {
               complaint (&symfile_complaints,
-                         _("macro '%s' is #undefined twice,"
-                           " at %s:%d and %s:%d"),
-                         name,
-                         source->filename, line,
-                         key->end_file->filename, key->end_line);
+                         _("macro '%s' is #undefined twice, at %s and %s"),
+                         name, pmacroloc (source->filename, line),
+                         pmacroloc (key->end_file->filename, key->end_line));
             }
 
           /* Whether or not we've seen a prior #undefinition, wipe out
@@ -848,8 +853,8 @@ macro_undef (struct macro_source_file *s
          ignore it too.  */
 #if 0
       complaint (&symfile_complaints,
-		 _("no definition for macro `%s' in scope to #undef at %s:%d"),
-		 name, source->filename, line);
+		 _("no definition for macro `%s' in scope to #undef at %s"),
+		 name, pmacroloc (source->filename, line));
 #endif
     }
 }
--- gdb/utils.c	26 Jan 2009 16:24:27 -0000	1.205
+++ gdb/utils.c	12 Feb 2009 22:14:07 -0000
@@ -3541,6 +3541,19 @@ gdb_buildargv (const char *s)
   return argv;
 }
 
+char *
+pmacroloc (const char *filename, int line)
+{
+  char *str;
+
+  if (!filename)
+    return _("command-line");
+
+  str = get_cell ();
+  xsnprintf (str, CELLSIZE, "%s:%d", filename, line);
+  return str;
+}
+
 void
 _initialize_utils (void)
 {


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