[commit] Handle files without DW_AT_comp_dir

[commit] Handle files without DW_AT_comp_dir

[Daniel Jacobowitz]

Joseph Myers discovered a case where GCC emits an absolute path in
DW_AT_name, no DW_AT_comp_dir, and relative pathnames in the directory
table.  GDB could not handle this, and failed to locate header files
in the correct paths.

The GCC behavior is a bug, and I think Joseph's working on a fix for
it.  But it's been around for a long time, so I think it's worthwhile
to handle it in GDB.  I tested the patch below on x86_64-linux and
checked it in.

-- 
Daniel Jacobowitz
CodeSourcery

2007-06-04  Daniel Jacobowitz  <dan@codesourcery.com>

	* defs.h (ldirname): New prototype.
	* dwarf2read.c (read_file_scope): Use DW_AT_name if DW_AT_comp_dir is
	missing.
	* utils.c (ldirname): New function.
	* xml-tdesc.c (file_read_description_xml): Use ldirname.

Index: defs.h
===================================================================
RCS file: /cvs/src/src/gdb/defs.h,v
retrieving revision 1.206
diff -u -p -r1.206 defs.h
--- defs.h	31 May 2007 17:00:06 -0000	1.206
+++ defs.h	4 Jun 2007 12:32:21 -0000
@@ -417,6 +417,8 @@ extern unsigned long gnu_debuglink_crc32
 
 ULONGEST strtoulst (const char *num, const char **trailer, int base);
 
+char *ldirname (const char *filename);
+
 /* From demangle.c */
 
 extern void set_demangling_style (char *);
Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.220
diff -u -p -r1.220 dwarf2read.c
--- dwarf2read.c	18 May 2007 19:42:42 -0000	1.220
+++ dwarf2read.c	4 Jun 2007 12:32:23 -0000
@@ -2783,7 +2783,7 @@ read_file_scope (struct die_info *die, s
   CORE_ADDR lowpc = ((CORE_ADDR) -1);
   CORE_ADDR highpc = ((CORE_ADDR) 0);
   struct attribute *attr;
-  char *name = "<unknown>";
+  char *name = NULL;
   char *comp_dir = NULL;
   struct die_info *child_die;
   bfd *abfd = objfile->obfd;
@@ -2806,21 +2806,29 @@ read_file_scope (struct die_info *die, s
     {
       name = DW_STRING (attr);
     }
+
   attr = dwarf2_attr (die, DW_AT_comp_dir, cu);
   if (attr)
+    comp_dir = DW_STRING (attr);
+  else if (name != NULL && IS_ABSOLUTE_PATH (name))
     {
-      comp_dir = DW_STRING (attr);
-      if (comp_dir)
-	{
-	  /* Irix 6.2 native cc prepends <machine>.: to the compilation
-	     directory, get rid of it.  */
-	  char *cp = strchr (comp_dir, ':');
+      comp_dir = ldirname (name);
+      if (comp_dir != NULL)
+	make_cleanup (xfree, comp_dir);
+    }
+  if (comp_dir != NULL)
+    {
+      /* Irix 6.2 native cc prepends <machine>.: to the compilation
+	 directory, get rid of it.  */
+      char *cp = strchr (comp_dir, ':');
 
-	  if (cp && cp != comp_dir && cp[-1] == '.' && cp[1] == '/')
-	    comp_dir = cp + 1;
-	}
+      if (cp && cp != comp_dir && cp[-1] == '.' && cp[1] == '/')
+	comp_dir = cp + 1;
     }
 
+  if (name == NULL)
+    name = "<unknown>";
+
   attr = dwarf2_attr (die, DW_AT_language, cu);
   if (attr)
     {
Index: utils.c
===================================================================
RCS file: /cvs/src/src/gdb/utils.c,v
retrieving revision 1.176
diff -u -p -r1.176 utils.c
--- utils.c	30 Mar 2007 09:31:31 -0000	1.176
+++ utils.c	4 Jun 2007 12:32:23 -0000
@@ -3195,3 +3195,31 @@ strtoulst (const char *num, const char *
   else
     return result;
 }
+
+/* Simple, portable version of dirname that does not modify its
+   argument.  */
+
+char *
+ldirname (const char *filename)
+{
+  const char *base = lbasename (filename);
+  char *dirname;
+
+  while (base > filename && IS_DIR_SEPARATOR (base[-1]))
+    --base;
+
+  if (base == filename)
+    return NULL;
+
+  dirname = xmalloc (base - filename + 2);
+  memcpy (dirname, filename, base - filename);
+
+  /* On DOS based file systems, convert "d:foo" to "d:.", so that we
+     create "d:./bar" later instead of the (different) "d:/bar".  */
+  if (base - filename == 2 && IS_ABSOLUTE_PATH (base)
+      && !IS_DIR_SEPARATOR (filename[0]))
+    dirname[base++ - filename] = '.';
+
+  dirname[base - filename] = '\0';
+  return dirname;
+}
Index: xml-tdesc.c
===================================================================
RCS file: /cvs/src/src/gdb/xml-tdesc.c,v
retrieving revision 1.5
diff -u -p -r1.5 xml-tdesc.c
--- xml-tdesc.c	13 Feb 2007 15:48:05 -0000	1.5
+++ xml-tdesc.c	4 Jun 2007 12:32:23 -0000
@@ -487,7 +487,6 @@ file_read_description_xml (const char *f
   struct target_desc *tdesc;
   char *tdesc_str;
   struct cleanup *back_to;
-  const char *base;
   char *dirname;
 
   tdesc_str = fetch_xml_from_file (filename, NULL);
@@ -499,28 +498,9 @@ file_read_description_xml (const char *f
 
   back_to = make_cleanup (xfree, tdesc_str);
 
-  /* Simple, portable version of dirname that does not modify its
-     argument.  */
-  base = lbasename (filename);
-  while (base > filename && IS_DIR_SEPARATOR (base[-1]))
-    --base;
-  if (base > filename)
-    {
-      dirname = xmalloc (base - filename + 2);
-      memcpy (dirname, filename, base - filename);
-
-      /* On DOS based file systems, convert "d:foo" to "d:.", so that
-	 we create "d:./bar" later instead of the (different)
-	 "d:/bar".  */
-      if (base - filename == 2 && IS_ABSOLUTE_PATH (base)
-	  && !IS_DIR_SEPARATOR (filename[0]))
-	dirname[base++ - filename] = '.';
-
-      dirname[base - filename] = '\0';
-      make_cleanup (xfree, dirname);
-    }
-  else
-    dirname = NULL;
+  dirname = ldirname (filename);
+  if (dirname != NULL)
+    make_cleanup (xfree, dirname);
 
   tdesc = tdesc_parse_xml (tdesc_str, fetch_xml_from_file, dirname);
   do_cleanups (back_to);

Re: [commit] Handle files without DW_AT_comp_dir

[Jan Kratochvil]

http://sourceware.org/ml/gdb-patches/2007-06/msg00031.html
On Mon, 04 Jun 2007 14:36:15 +0200, Daniel Jacobowitz wrote:
> Joseph Myers discovered a case where GCC emits an absolute path in
> DW_AT_name, no DW_AT_comp_dir, and relative pathnames in the directory
> table.

I see only absolute directory pathnames in .debug_line in such case.
If it really was a GCC bug it should be addressed by DW_AT_producer based
workaround limitation.


> GDB could not handle this, and failed to locate header files
> in the correct paths.
> 
> The GCC behavior is a bug, and I think Joseph's working on a fix for
> it.  But it's been around for a long time, so I think it's worthwhile
> to handle it in GDB.  I tested the patch below on x86_64-linux and
> checked it in.
[...]
>    attr = dwarf2_attr (die, DW_AT_comp_dir, cu);
>    if (attr)
> +    comp_dir = DW_STRING (attr);
> +  else if (name != NULL && IS_ABSOLUTE_PATH (name))
>      {
> +      comp_dir = ldirname (name);

This is affecting the long thread of patch
	[patch] GDB 7.2: new feature for "backtrace" that cuts path to file (remain filename)

DWARF considers the CU's DW_AT_comp_dir attribute optional.

I do not see any problem with it, compilation directory is not known in such
case.  I do not see any testcase in this patch which would show which GCC
versions under which conditions produced such buggy output.  These GCC
versions produce correct output, that is:
	cd /tmp; gcc -o ~/1 ~/1.c -g
	CU's DW_AT_name is absolute /root/1.c
	CU's DW_AT_comp_dir is not present
	.debug_line uses directory entry /root
	.debug_line uses filename entry 1.c
with:
	GNU C 2.96 20000731 (Red Hat Linux 7.2 2.96-116.7.2)  
	gcc (GCC) 3.3.5
	gcc (GCC) 4.4.7
	gcc (GCC) 4.8.0 20120406 (experimental)

Without known GCC buggy versions going to revert this patch as it breaks
correct DWARF.  The thread "that cuts path to file (remain filename)" tries to
(in some way) undo what this patch does.


Thanks,
Jan

Re: [commit] Handle files without DW_AT_comp_dir

[Daniel Jacobowitz]

On Fri, Apr 6, 2012 at 8:36 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> I do not see any problem with it, compilation directory is not known in such
> case.  I do not see any testcase in this patch which would show which GCC
> versions under which conditions produced such buggy output.  These GCC
> versions produce correct output, that is:

I'm sure it was the current stable GCC at the time.  That would be
4.1/4.2.  I'd try to find the fix, but picking out a patch by Joseph
is like hunting for a needle in a haystack :-)

> Without known GCC buggy versions going to revert this patch as it breaks
> correct DWARF.  The thread "that cuts path to file (remain filename)" tries to
> (in some way) undo what this patch does.

Can you explain the way in which this breaks correct DWARF?  I'm
confused since the description of the other thread sounds like an
output issue, not a debug reader issue...

-- 
Thanks,
Daniel

Re: [commit] Handle files without DW_AT_comp_dir

[Joseph S. Myers]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 885 bytes --]

On Sat, 7 Apr 2012, Daniel Jacobowitz wrote:

> On Fri, Apr 6, 2012 at 8:36 AM, Jan Kratochvil
> <jan.kratochvil@redhat.com> wrote:
> > I do not see any problem with it, compilation directory is not known in such
> > case.  I do not see any testcase in this patch which would show which GCC
> > versions under which conditions produced such buggy output.  These GCC
> > versions produce correct output, that is:
> 
> I'm sure it was the current stable GCC at the time.  That would be
> 4.1/4.2.  I'd try to find the fix, but picking out a patch by Joseph
> is like hunting for a needle in a haystack :-)

It may have been 
<http://gcc.gnu.org/ml/gcc-patches/2007-07/msg00404.html>.  The case of 
absolute paths to the source file and headers found by a relative -I path 
would probably have appeared when building glibc.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [commit] Handle files without DW_AT_comp_dir

[Jan Kratochvil]

On Sun, 08 Apr 2012 01:00:56 +0200, Joseph S. Myers wrote:
> It may have been 
> <http://gcc.gnu.org/ml/gcc-patches/2007-07/msg00404.html>.
 = GIT 5f1f2de5fe3f87b056e802102fcb975979845eff (gcc-4.3-pre)

BTW the GDB workaround does not work if the compilation directory was not the
directory of primary compiled file, not sure if it was expected:

cd; mkdir t t/inc.d; echo 'int j;' >t/inc.d/inc.h; echo -e '#include "inc.h"\nint i;' >t/inc.c; ~/redhat/gccgit-myers0-root/bin/gcc -c -o t/inc.o -Wall -g -It/inc.d $PWD/t/inc.c; readelf -wil t/inc.o 
     DW_AT_name        : /home/jkratoch/t/inc.c	
     ; no DW_AT_comp_dir
 The Directory Table:
  t/inc.d
  /home/jkratoch/t
 The File Name Table:
  Entry	Dir	Time	Size	Name
  1	1	0	0	inc.h
  2	2	0	0	inc.c
cd /tmp; gdb ~/t/inc.o
(gdb) list j
1	t/inc.d/inc.h: No such file or directory.


On Sun, 08 Apr 2012 00:36:51 +0200, Daniel Jacobowitz wrote:
> > Without known GCC buggy versions going to revert this patch as it breaks
> > correct DWARF.  The thread "that cuts path to file (remain filename)" tries to
> > (in some way) undo what this patch does.
> 
> Can you explain the way in which this breaks correct DWARF?

It pretends DW_AT_comp_dir was basename(DW_AT_name) so that the patch in
	Re: [patch] GDB 7.2: new feature for "backtrace" that cuts path to file (remain filename)
	http://sourceware.org/ml/gdb-patches/2012-03/msg00669.html
was removing it then.

With your patch and the iam ahal's patch above it will in fact do what
	set backtrace filename-display basename
does so I did not understand why the iam ahal's patch was more complicated and
I asked for more info, without reply yet.
	Re: [patch] GDB 7.2: new feature for "backtrace" that cuts path to file (remain filename)
	http://sourceware.org/ml/gdb-patches/2012-04/msg00106.html
	http://sourceware.org/ml/gdb-patches/2012-04/msg00147.html

In general DWARF fix ups should be IMO limited only for the buggy producers,
otherwise it is difficult do any work on top of it, as was proven by the
DW_AT_comp_dir weird removal in the patch above.

You are right I do not see any regression without considering the uncommitted
patch by iam ahal above.


As I have the problem reproducibel now I will limit the fix up to gcc <= 4.2
and write a DWARF testcase for it.  I guess the cases with gcc <= 4.2 where
DW_AT_comp_dir is equal to basename(DW_AT_name) are more common than the cases
where it is not equal.


Thanks,
Jan