[RFC] libdw: prepend current directory in read_srclines

[RFC] libdw: prepend current directory in read_srclines

[Torsten Polle]

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

Hi,

I observed that readelf and elfutils sometimes report different results. PFA a patch that corrects this. I’m not sure whether the way I tackled the problem is acceptable.

Kind Regards,
Torsten


[-- Attachment #2: 0001-libdw-prepend-current-directory-in-read_srclines.patch --]
[-- Type: application/octet-stream, Size: 2226 bytes --]

From dfd639e4e7377ce63d4e7296c0c139b7bf37167e Mon Sep 17 00:00:00 2001
From: Torsten Polle <Torsten.Polle@gmx.de>
Date: Sat, 18 Mar 2017 07:32:39 +0100
Subject: [PATCH] libdw: prepend current directory in read_srclines

read_srclines retrieves sometimes only relative paths for a source
file. The file entry in the line number program header refers to an
entry in the field "include_directories" that is a relative path. We
prepend the current directory of the compilation to make this an
absolute path.

Signed-off-by: Torsten Polle <Torsten.Polle@gmx.de>
---
 libdw/dwarf_getsrclines.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/libdw/dwarf_getsrclines.c b/libdw/dwarf_getsrclines.c
index d02c38d..83a511e 100644
--- a/libdw/dwarf_getsrclines.c
+++ b/libdw/dwarf_getsrclines.c
@@ -369,11 +369,28 @@ read_srclines (Dwarf *dbg,
 	new_file->info.name = fname;
       else
 	{
-	  new_file->info.name = libdw_alloc (dbg, char, 1,
-					     dirarray[diridx].len + 1
-					     + fnamelen + 1);
+	  size_t len = dirarray[diridx].len + 1 + fnamelen + 1;
+	  if (dirarray[diridx].dir != NULL
+	      && *dirarray[diridx].dir != '/'
+	      && dirarray[0].dir != NULL)
+	    {
+	      /* If the directory is a relative path, we need to make
+		 room for the compile directory. */
+	      len += dirarray[0].len + 1;
+	    }
+	  new_file->info.name = libdw_alloc (dbg, char, 1, len);
 	  char *cp = new_file->info.name;
 
+	  if (dirarray[diridx].dir != NULL
+	      && *dirarray[diridx].dir != '/'
+	      && dirarray[0].dir != NULL)
+	    {
+	      /* If the directory is a relative path, we prepend the
+		 compile directory. */
+	      cp = stpcpy (cp, dirarray[0].dir);
+	      *cp++ = '/';
+	    }
+
 	  if (dirarray[diridx].dir != NULL)
 	    {
 	      /* This value could be NULL in case the DW_AT_comp_dir
@@ -384,8 +401,7 @@ read_srclines (Dwarf *dbg,
 	    }
 	  *cp++ = '/';
 	  strcpy (cp, fname);
-	  assert (strlen (new_file->info.name)
-		  < dirarray[diridx].len + 1 + fnamelen + 1);
+	  assert (strlen (new_file->info.name) < len);
 	}
 
       /* Next comes the modification time.  */
-- 
2.7.4

Re: [RFC] libdw: prepend current directory in read_srclines

[Mark Wielaard]

Hi Torsten,

On Sun, Mar 26, 2017 at 08:35:50PM +0200, Torsten Polle wrote:
> I observed that readelf and elfutils sometimes report different results.

Do you have an example of this? It would be good to have a testcase.

> PFA a patch that corrects this. I’m not sure whether the way I tackled
> the problem is acceptable.

I see why you are proposing this. The DWARF spec does say about the
include_directories "Each path entry is either a full path name or is
relative to the current directory of the compilation". So your patch
does make sense.

But it does depend on what users of dwarf_getsrclines expect.
Or any use of Dwarf_Line/Dwarf_Files. I think those users expect that
the returned file names can be relative. And that they should make them
absolute using index zero or the comp_dir themselves.

So if you do have an example where the expected output isn't what you
believe it should be we should examine if there is some other way to
do the right thing.

Cheers,

Mark

Re: [RFC] libdw: prepend current directory in read_srclines

[Torsten Polle]

Hi Mark,

> Am 27.03.2017 um 22:45 schrieb Mark Wielaard <mark@klomp.org>:
> 
> Hi Torsten,
> 
> On Sun, Mar 26, 2017 at 08:35:50PM +0200, Torsten Polle wrote:
>> I observed that readelf and elfutils sometimes report different results.
> 
> Do you have an example of this? It would be good to have a testcase.
> 
>> PFA a patch that corrects this. I’m not sure whether the way I tackled
>> the problem is acceptable.
> 
> I see why you are proposing this. The DWARF spec does say about the
> include_directories "Each path entry is either a full path name or is
> relative to the current directory of the compilation". So your patch
> does make sense.
> 
> But it does depend on what users of dwarf_getsrclines expect.
> Or any use of Dwarf_Line/Dwarf_Files. I think those users expect that
> the returned file names can be relative. And that they should make them
> absolute using index zero or the comp_dir themselves.
> 
> So if you do have an example where the expected output isn't what you
> believe it should be we should examine if there is some other way to
> do the right thing.
> 
> Cheers,
> Mark

thanks for the answer. I’ll come back with an example, which is SystemTap based. :-)

Please allow for some delay. I expect to provide an example in about two weeks.

Regards,
Torsten

Re: [RFC] libdw: prepend current directory in read_srclines

[Torsten Polle]

Hi Mark,

> Am 29.03.2017 um 22:15 schrieb Torsten Polle <Torsten.Polle@gmx.de>:
> 
> Hi Mark,
> 
>> Am 27.03.2017 um 22:45 schrieb Mark Wielaard <mark@klomp.org>:
>> 
>> Hi Torsten,
>> 
>> On Sun, Mar 26, 2017 at 08:35:50PM +0200, Torsten Polle wrote:
>>> I observed that readelf and elfutils sometimes report different results.
>> 
>> Do you have an example of this? It would be good to have a testcase.
>> 
>>> PFA a patch that corrects this. I’m not sure whether the way I tackled
>>> the problem is acceptable.
>> 
>> I see why you are proposing this. The DWARF spec does say about the
>> include_directories "Each path entry is either a full path name or is
>> relative to the current directory of the compilation". So your patch
>> does make sense.
>> 
>> But it does depend on what users of dwarf_getsrclines expect.
>> Or any use of Dwarf_Line/Dwarf_Files. I think those users expect that
>> the returned file names can be relative. And that they should make them
>> absolute using index zero or the comp_dir themselves.
>> 
>> So if you do have an example where the expected output isn't what you
>> believe it should be we should examine if there is some other way to
>> do the right thing.
>> 
>> Cheers,
>> Mark
> 
> thanks for the answer. I’ll come back with an example, which is SystemTap based. :-)
> 
> Please allow for some delay. I expect to provide an example in about two weeks.
> 
> Regards,
> Torsten

Sorry for coming back so late. Please find attached my reproduction.

I compile the simple program relative.c:

int main()
{
  return 0;
}

with the command "gcc -g ../2017-05-03-elfutils/relative.c -o relative“.

When I run systemtap with the command
/opt/tooling/adit/systemtap/bin/stap -g -a i386 -B CROSS_COMPILE=i586-poky-linux- -r /home/polle/work/build/poky/build/tmp/work/qemux86-poky-linux/linux-yocto/4.8.12+gitAUTOINC+926c93ae07_021b4aef55-r0/build --sysroot=/home/polle/work/build/poky/build/tmp/work/qemux86-poky-linux/core-image-sato/1.0-r0/rootfs -L 'process("/bin/relative").function("main").call‘

I get the following output:
process("/bin/relative").function("main@../2017-05-03-elfutils/relative.c:1").call

But running nm returns the following.
nm -l | grep relative
080483eb T main	/home/polle/work/issues/2017-05-03-elfutils/../2017-05-03-elfutils/relative.c:1

I hope this helps.

Kind Regards,
Torsten

Re: [RFC] libdw: prepend current directory in read_srclines

[Mark Wielaard]

Hi Torsten,

On Wed, 2017-05-03 at 22:34 +0200, Torsten Polle wrote:
> I compile the simple program relative.c:
> 
> int main()
> {
>   return 0;
> }
> 
> with the command "gcc -g ../2017-05-03-elfutils/relative.c -o relative“.
> 
> When I run systemtap with the command
> /opt/tooling/adit/systemtap/bin/stap -g -a i386 -B CROSS_COMPILE=i586-poky-linux- -r /home/polle/work/build/poky/build/tmp/work/qemux86-poky-linux/linux-yocto/4.8.12+gitAUTOINC+926c93ae07_021b4aef55-r0/build --sysroot=/home/polle/work/build/poky/build/tmp/work/qemux86-poky-linux/core-image-sato/1.0-r0/rootfs -L 'process("/bin/relative").function("main").call‘
> 
> I get the following output:
> process("/bin/relative").function("main@../2017-05-03-elfutils/relative.c:1").call
> 
> But running nm returns the following.
> nm -l | grep relative
> 080483eb T main	/home/polle/work/issues/2017-05-03-elfutils/../2017-05-03-elfutils/relative.c:1
> 
> I hope this helps.

Yes. But I think it is an systemtap issue, not an elfutils issue.
So systemtap mailinglist CCed.

If I understand the systemtap sources correctly then the above comes
from either dwarf_decl_file (Dwarf_Die *). Which returns the file name
as recorded in the DIE. If that name doesn't start with '/' then if a
full path is needed then the compile unit comp_dir needs to be prefixed:

dwarf_formstring (dwarf_attr (dwarf_diecu (die, ...), DW_AT_comp_dir, ...));

If the name already started with a '/' then it is already a full path.

It might make the systemtap output more consistent if it did that. But
since the file names are also used to match against regular expressions
you might need a bit careful where you do or don't add the full path.

Cheers,

Mark