|
From: L. D. B. <db...@db...> - 2006-04-02 01:28:29
Attachments:
FC5-debuginfo.4.patch
|
I've been looking into why valgrind doesn't pick up information from the
*-debuginfo packages on Fedora Core 5, and have a patch against valgrind
3.1.1 that fixes the problems I'm seeing. I'm willing to port this to
the trunk (which is probably where I should have started, although I
have looked at the code there to see that the same problems are present
there), but I have a few questions first.
There are two basic problems that prevent the information in the
debuginfo file reached through the .gnu_debuglink section from working
correctly:
1. Presumably because of prelinking (although I haven't verified this),
the symbol table in the library is based on a non-zero offset, but in
the debuginfo file it's based on a zero offset. The current code
assumes that the offset (stored in the offset member of struct _SegInfo)
is constant across the .dynsym, .strtab, and .debug_* sections.
2. Some of the libraries shipped with Fedora Core 5 (e.g., gtk+/gdk
libraries) have been stripped. This means that the .strtab and .symtab
sections must be read from the debuginfo file.
My current patch takes the approach of passing the appropriate offset
around with the symbol table or debug data, rather than changing the one
offset stored in struct _SegInfo into three separate offsets. But
either way there are three different offsets:
1. the one used for the data from the .dynsym sections, which always
comes from the library
2. the one used for the data from the .symtab sections, which comes
from the library unless the library is stripped and external
debuginfo is found
3. the one used for the data from the .debug_* or .stab sections, which
comes from the debuginfo file if external debuginfo is found
This seems fine, except that there's what looks to me like a public API,
seginfo_sym_offset, that returns that information (currently #1 above).
I see three things that could be done with the public API:
1. remove it (which the attached patch does)
2. preserve its behavior and either document it better or deprecate it
3. extend it so that all three offsets can be obtained
Since I have no idea what, if anything, uses this API, I'm in no
position to judge which of these is the best course.
My current patch (against 3.1.1) is attached, although it's not ready
for inclusion.
-David
--
L. David Baron <URL: http://dbaron.org/ >
Technical Lead, Layout & CSS, Mozilla Corporation
|
|
From: Julian S. <js...@ac...> - 2006-04-05 11:00:14
|
David
You write at an opportune time; I've been cleaning up the debuginfo
reader recently, and so have it at least partially paged in.
> I'm willing to port this to the trunk
That would be a good first step - please do. You'll find m_debuginfo
has been restructured since you looked at it (see m_debuginfo/README.txt).
> My current patch takes the approach of passing the appropriate offset
> around with the symbol table or debug data, rather than changing the one
> offset stored in struct _SegInfo into three separate offsets. But
> either way there are three different offsets:
> 1. the one used for the data from the .dynsym sections, which always
> comes from the library
> 2. the one used for the data from the .symtab sections, which comes
> from the library unless the library is stripped and external
> debuginfo is found
> 3. the one used for the data from the .debug_* or .stab sections, which
> comes from the debuginfo file if external debuginfo is found
>
> This seems fine, except that there's what looks to me like a public API,
> seginfo_sym_offset, that returns that information (currently #1 above).
> I see three things that could be done with the public API:
> 1. remove it (which the attached patch does)
> 2. preserve its behavior and either document it better or deprecate it
> 3. extend it so that all three offsets can be obtained
> Since I have no idea what, if anything, uses this API, I'm in no
> position to judge which of these is the best course.
The more I look at what's already there the less I understand it.
Removing seginfo_sym_offset will break callgrind, but my view is
the ->offset field is something used whilst reading ELF/DWARF/stabs
stuff, and shouldn't be publically visible.
What seems to me to be the right thing is that the addresses added
to the symbol tables/line number tables/CFI tables in storage.c
should be correct for the running image, and should not need to be
interpreted together with any other offset to make sense of.
Assuming that's a correct interpretation, .offset should not be part
of struct _SegInfo, but instead be a local variable established by
readelf.c and handed to read{dwarf,stabs}.c.
So I think you can remove this API.
Josef: what is it that callgrind uses VG_(seginfo_sym_offset) for?
I looked at obj_of_address() in bb.c but didn't understand it.
J
|
|
From: Josef W. <Jos...@gm...> - 2006-04-05 12:04:46
|
On Wednesday 05 April 2006 13:00, Julian Seward wrote: > Josef: what is it that callgrind uses VG_(seginfo_sym_offset) for? > I looked at obj_of_address() in bb.c but didn't understand it. There are two uses: 1) I store addresses int callgrinds structures as (object name, address offset). As you know, I keep cost counters after unmap(), and if the same object is mapped again, the old counters are also used. This works automatically because the counters are accessed by the above tuple and not the absolute address 2) When callgrind is asked to write profile info at instruction granularity, I write out the address offset instead of an absolute address. This is needed to get disassembler with "objdump -d <elf object>" for the assembler annotation. Absolute addresses is useless here. Regarding (1), keeping counters after unmapping is kind of a design bug, and the best way is to trigger a dump of profile data every time an unmap happens. Using this fix, there is no need to use offset addresses any longer. Regarding (2), offsets really are needed for the visualization to be able to relate costs with instructions from the binary, ie. I need an offset to pass to --start-address/--end-address options of objdump. Josef |
|
From: Josef W. <Jos...@gm...> - 2006-04-05 13:09:33
|
On Wednesday 05 April 2006 14:04, Josef Weidendorfer wrote: > On Wednesday 05 April 2006 13:00, Julian Seward wrote: > > Josef: what is it that callgrind uses VG_(seginfo_sym_offset) for? > > I looked at obj_of_address() in bb.c but didn't understand it. > Regarding (2), offsets really are needed for the visualization to be able to > relate costs with instructions from the binary, ie. I need an offset to pass > to --start-address/--end-address options of objdump. I just checked how objdump behaves with a prelinked binary: the output of the disassembler output shows the already relocated/prelinked addresses. Still, --start-address wants to an unrelocated addresses (ie. an offset). Somehow inconsistent. Thus, obviously, callgrind always wants to have address offsets as they are used in debug info. Josef |
|
From: L. D. B. <db...@db...> - 2006-04-06 06:28:24
|
On Wednesday 2006-04-05 14:04 +0200, Josef Weidendorfer wrote: > On Wednesday 05 April 2006 13:00, Julian Seward wrote: > > Josef: what is it that callgrind uses VG_(seginfo_sym_offset) for? > > I looked at obj_of_address() in bb.c but didn't understand it. So I'm hoping that the uses of VG_(seginfo_sym_offset) can be replaced by VG_(seginfo_start). The result of the latter (which I'll call start, since that's the name of the member of struct _obj_node) gives the address in memory at which the library is mapped; the result of the former (which I'll call offset, for the same reason) gives the difference between the address at which the library is mapped and the base address of the internal addresses used within the library. For a non-prelinked library, that base address is typically zero, so start =3D=3D offset. For a prelinked library loaded at the address for which it was prelinked, start is equal to the target base address and offset is zero. > There are two uses: > 1) I store addresses int callgrinds structures as (object name, address o= ffset). > As you know, I keep cost counters after unmap(), and if the same object i= s mapped > again, the old counters are also used. This works automatically because t= he > counters are accessed by the above tuple and not the absolute address Using VG_(seginfo_start) instead of VG_(seginfo_sym_offset) should work fine for this purpose; for any given library, they always differ by a constant (the base address for addresses within symbol tables, etc.). > 2) When callgrind is asked to write profile info at instruction granulari= ty, I > write out the address offset instead of an absolute address. This is need= ed > to get disassembler with "objdump -d <elf object>" for the assembler anno= tation. > Absolute addresses is useless here. > Regarding (2), offsets really are needed for the visualization to be able= to > relate costs with instructions from the binary, ie. I need an offset to p= ass > to --start-address/--end-address options of objdump. On Wednesday 2006-04-05 15:09 +0200, Josef Weidendorfer wrote: > I just checked how objdump behaves with a prelinked binary: the output of > the disassembler output shows the already relocated/prelinked addresses. > Still, --start-address wants to an unrelocated addresses (ie. an offset). > Somehow inconsistent. >=20 > Thus, obviously, callgrind always wants to have address offsets as they a= re > used in debug info. If what you want is always an offset relative to the library's base address, and you're starting with an address in memory, then I'd think you want to be subtracting start rather than offset. I suppose I should figure out how to test callgrind so I can test this theory. -David --=20 L. David Baron <URL: http://dbaron.org/ > Technical Lead, Layout & CSS, Mozilla Corporation |
|
From: L. D. B. <db...@db...> - 2006-04-06 06:56:54
|
On Wednesday 2006-04-05 23:28 -0700, L. David Baron wrote: > On Wednesday 2006-04-05 14:04 +0200, Josef Weidendorfer wrote: > > On Wednesday 05 April 2006 13:00, Julian Seward wrote: > > > Josef: what is it that callgrind uses VG_(seginfo_sym_offset) for? > > > I looked at obj_of_address() in bb.c but didn't understand it. >=20 > So I'm hoping that the uses of VG_(seginfo_sym_offset) can be replaced > by VG_(seginfo_start). So, I tried this, and the only difference I could see between callgrind output with and without my patch is that I'm seeing the same external debuginfo that wasn't working without my patch. Perhaps obj->offset was added and subtracted in so many places that it comes out even? With and without my patch, I ran callgrind on a program that uses a whole bunch of shared libraries, some of which are prelinked, and then ran callgrind_annotate --threshold=3D100 on the output, and compared a few features of the results. As I said, the only difference I noticed was the effect of the external debuginfo working correctly where it didn't before. I attached a patch aganst HEAD to http://bugs.kde.org/show_bug.cgi?id=3D124949 -David --=20 L. David Baron <URL: http://dbaron.org/ > Technical Lead, Layout & CSS, Mozilla Corporation |
|
From: Julian S. <js...@ac...> - 2006-04-06 11:15:55
|
Thanks for updating this to the head. What platforms did you test it on? In particular, do you know if the nasty hacks needed for reading ELF symbols on ppc64-linux still work? On Thursday 06 April 2006 07:56, L. David Baron wrote: > On Wednesday 2006-04-05 23:28 -0700, L. David Baron wrote: > > On Wednesday 2006-04-05 14:04 +0200, Josef Weidendorfer wrote: > > > On Wednesday 05 April 2006 13:00, Julian Seward wrote: > > > > Josef: what is it that callgrind uses VG_(seginfo_sym_offset) for? > > > > I looked at obj_of_address() in bb.c but didn't understand it. > > > > So I'm hoping that the uses of VG_(seginfo_sym_offset) can be replaced > > by VG_(seginfo_start). > > So, I tried this, and the only difference I could see between callgrind > output with and without my patch is that I'm seeing the same external > debuginfo that wasn't working without my patch. Perhaps obj->offset was > added and subtracted in so many places that it comes out even? Josef, does this look OK for you? J |
|
From: Josef W. <Jos...@gm...> - 2006-04-06 17:36:49
|
On Thursday 06 April 2006 13:15, Julian Seward wrote: > Josef, does this look OK for you? The patch looks fine. Josef |
|
From: Josef W. <Jos...@gm...> - 2006-04-06 17:35:25
|
On Thursday 06 April 2006 08:28, L. David Baron wrote: > On Wednesday 2006-04-05 14:04 +0200, Josef Weidendorfer wrote: > > On Wednesday 05 April 2006 13:00, Julian Seward wrote: > > > Josef: what is it that callgrind uses VG_(seginfo_sym_offset) for? > > > I looked at obj_of_address() in bb.c but didn't understand it. > > So I'm hoping that the uses of VG_(seginfo_sym_offset) can be replaced > by VG_(seginfo_start). The result of the latter (which I'll call start, > since that's the name of the member of struct _obj_node) gives the > address in memory at which the library is mapped; the result of the > former (which I'll call offset, for the same reason) gives the > difference between the address at which the library is mapped and the > base address of the internal addresses used within the library. So using this definition, callgrind did the right thing: I wanted code addresses in memory mapped to the internal addresses used within the library... > For a > non-prelinked library, that base address is typically zero, Are there other cases than prelink, where this base address is != 0 ? > so start == > offset. For a prelinked library loaded at the address for which it was > prelinked, start is equal to the target base address and offset is zero. Hmmm... So subtracting the offset "0", as done in callgrind previously, seemed to be wrong? As far as I see it, objdumps --start-addr option always wants file offsets, starting at 0 (what my check from the last mail revealed). I'll think about a regression test for this... > > There are two uses: > > 1) I store addresses int callgrinds structures as (object name, address offset). > > As you know, I keep cost counters after unmap(), and if the same object is mapped > > again, the old counters are also used. This works automatically because the > > counters are accessed by the above tuple and not the absolute address > > Using VG_(seginfo_start) instead of VG_(seginfo_sym_offset) should work > fine for this purpose; for any given library, they always differ by a > constant (the base address for addresses within symbol tables, etc.). Yes. > > 2) When callgrind is asked to write profile info at instruction granularity, I > > write out the address offset instead of an absolute address. This is needed > > to get disassembler with "objdump -d <elf object>" for the assembler annotation. > > Absolute addresses is useless here. > > > Regarding (2), offsets really are needed for the visualization to be able to > > relate costs with instructions from the binary, ie. I need an offset to pass > > to --start-address/--end-address options of objdump. > > On Wednesday 2006-04-05 15:09 +0200, Josef Weidendorfer wrote: > > I just checked how objdump behaves with a prelinked binary: the output of > > the disassembler output shows the already relocated/prelinked addresses. > > Still, --start-address wants to an unrelocated addresses (ie. an offset). > > Somehow inconsistent. > > > > Thus, obviously, callgrind always wants to have address offsets as they are > > used in debug info. > > If what you want is always an offset relative to the library's base > address, and you're starting with an address in memory, then I'd think > you want to be subtracting start rather than offset. Probably. On Thursday 06 April 2006 08:56, L. David Baron wrote: > So, I tried this, and the only difference I could see between callgrind > output with and without my patch is that I'm seeing the same external > debuginfo that wasn't working without my patch. Perhaps obj->offset was > added and subtracted in so many places that it comes out even? Symbol addresses are only put into the output of callgrind with "--dump-instr=yes", and unfortunately, callgrind_annotate can not parse this extended format at the moment. So you'll have to check it out with KCachegrind for now, whether assembler annotation is working. Or look at the addresses in the callgrind.out.*, and run objdump on them to see if you get a matching disassembler output. > With and without my patch, I ran callgrind on a program that uses a > whole bunch of shared libraries, some of which are prelinked, and then > ran callgrind_annotate --threshold=100 on the output, and compared a few > features of the results. As I said, the only difference I noticed was > the effect of the external debuginfo working correctly where it didn't > before. You didn't run callgrind with the option where this would be relevant. Source annotation always should work. Josef |
|
From: L. D. B. <db...@db...> - 2006-04-08 17:38:05
|
On Thursday 2006-04-06 19:35 +0200, Josef Weidendorfer wrote:
> On Thursday 06 April 2006 08:28, L. David Baron wrote:
> > So I'm hoping that the uses of VG_(seginfo_sym_offset) can be replaced
> > by VG_(seginfo_start). The result of the latter (which I'll call start,
> > since that's the name of the member of struct _obj_node) gives the
> > address in memory at which the library is mapped; the result of the
> > former (which I'll call offset, for the same reason) gives the
> > difference between the address at which the library is mapped and the
> > base address of the internal addresses used within the library.
>=20
> So using this definition, callgrind did the right thing: I wanted
> code addresses in memory mapped to the internal addresses used within
> the library...
>=20
> > For a=20
> > non-prelinked library, that base address is typically zero,
>=20
> Are there other cases than prelink, where this base address is !=3D 0 ?
I don't know.
> > so start =3D=3D=20
> > offset. For a prelinked library loaded at the address for which it was
> > prelinked, start is equal to the target base address and offset is zero.
>=20
> Hmmm... So subtracting the offset "0", as done in callgrind previously,
> seemed to be wrong?
>=20
> As far as I see it, objdumps --start-addr option always wants file offset=
s,
> starting at 0 (what my check from the last mail revealed).
Actually, at least for the prelinked libraries on my FC5 system, it
doesn't. On a profile generated without my patch, kcachegrind (the one
that shipped with FC5; I didn't rebuild it) issues the command
objdump -C -d --start-address=3D0xC16C3C --stop-address=3D0xC16EF7 "/usr/=
lib/libfontconfig.so.1.0.4"
which works correctly, but with my patch it issues the command
objdump -C -d --start-address=3D0x1DC3C --stop-address=3D0x1DEF7 "/usr/li=
b/libfontconfig.so.1.0.4"
The difference between these two exactly matches the prelinked base
address, which is 0x00bf9000:
$ readelf -l /usr/lib/libfontconfig.so.1.0.4 | grep "^ LOAD" | head -1
LOAD 0x000000 0x00bf9000 0x00bf9000 0x37d0c 0x37d0c R E 0x1000
(At least objdump is consistent; addr2line wants absolute addresses for
executables but library-relative addresses for shared libraries.)
So in this case what cachegrind does want what it was currently getting,
and it wants the offset for the library and not for any external
debuginfo.
I'll attach a new patch (sometime later today) that restores
seginfo_sym_offset.
-David
--=20
L. David Baron <URL: http://dbaron.org/ >
Technical Lead, Layout & CSS, Mozilla Corporation
|
|
From: Josef W. <Jos...@gm...> - 2006-04-09 00:53:26
|
On Saturday 08 April 2006 19:37, L. David Baron wrote: > > > offset. For a prelinked library loaded at the address for which it was > > > prelinked, start is equal to the target base address and offset is zero. > > > > Hmmm... So subtracting the offset "0", as done in callgrind previously, > > seemed to be wrong? > > > > As far as I see it, objdumps --start-addr option always wants file offsets, > > starting at 0 (what my check from the last mail revealed). Oops. My check was bogus. And, yes, you are right: > Actually, at least for the prelinked libraries on my FC5 system, it > doesn't. On a profile generated without my patch, kcachegrind (the one > that shipped with FC5; I didn't rebuild it) issues the command > objdump -C -d --start-address=0xC16C3C --stop-address=0xC16EF7 "/usr/lib/libfontconfig.so.1.0.4" Yes, objdump wants the prelinked addresses. It's the same on Suse. > which works correctly, but with my patch it issues the command > objdump -C -d --start-address=0x1DC3C --stop-address=0x1DEF7 "/usr/lib/libfontconfig.so.1.0.4" > The difference between these two exactly matches the prelinked base > address, which is 0x00bf9000: > $ readelf -l /usr/lib/libfontconfig.so.1.0.4 | grep "^ LOAD" | head -1 > LOAD 0x000000 0x00bf9000 0x00bf9000 0x37d0c 0x37d0c R E 0x1000 > (At least objdump is consistent; addr2line wants absolute addresses for > executables but library-relative addresses for shared libraries.) > > So in this case what cachegrind does want what it was currently getting, > and it wants the offset for the library and not for any external > debuginfo. Yes. > I'll attach a new patch (sometime later today) that restores > seginfo_sym_offset. That would be nice, thanks. That said, it looks wrong that callgrinds output is dependent on the prelink status of the shared libraries. So if libraries are relayouted by a newer prelink run afterwards, the assembler annotation isn't working any longer for an old callgrind output... Hmmm... but I do not see a better way; KCachegrind would have to look up the prelink offset and do address correction itself if we wanted to store the file offset in callgrinds output. That seems ugly, and changes the format semantics in a subtle way. Josef > > -David > |
|
From: L. D. B. <db...@db...> - 2006-04-12 03:11:42
|
On Sunday 2006-04-09 02:53 +0200, Josef Weidendorfer wrote:
> On Saturday 08 April 2006 19:37, L. David Baron wrote:
> > I'll attach a new patch (sometime later today) that restores
> > seginfo_sym_offset.
>=20
> That would be nice, thanks.
In any case, what needs to happen for the patch to get in?
> That said, it looks wrong that callgrinds output is dependent on the
> prelink status of the shared libraries. So if libraries are relayouted
> by a newer prelink run afterwards, the assembler annotation isn't
> working any longer for an old callgrind output...
> Hmmm... but I do not see a better way; KCachegrind would have to look
> up the prelink offset and do address correction itself if we wanted to
> store the file offset in callgrinds output. That seems ugly, and changes
> the format semantics in a subtle way.
It probably wouldn't be too hard to do -- you can get that information
with readelf if necessary: it's essentially
readelf -l $LIBRARY | grep -m 1 LOAD | awk '{ print $3 }'
But it would require a change in the file format. Perhaps it's worth
adding to a list of things to do if it becomes worthwhile to change the
file format.
-David
--=20
L. David Baron <URL: http://dbaron.org/ >
Technical Lead, Layout & CSS, Mozilla Corporation
|
|
From: Julian S. <js...@ac...> - 2006-04-12 10:11:28
|
On Wednesday 12 April 2006 04:11, L. David Baron wrote: > In any case, what needs to happen for the patch to get in? Committed. Let's see what the overnight builds say. I might add some parts of your original posting in this thread as comments. J |
|
From: Josef W. <Jos...@gm...> - 2006-04-12 11:27:19
|
On Wednesday 12 April 2006 05:11, L. David Baron wrote:
> > That said, it looks wrong that callgrinds output is dependent on the
> > prelink status of the shared libraries. So if libraries are relayouted
> > by a newer prelink run afterwards, the assembler annotation isn't
> > working any longer for an old callgrind output...
> > Hmmm... but I do not see a better way; KCachegrind would have to look
> > up the prelink offset and do address correction itself if we wanted to
> > store the file offset in callgrinds output. That seems ugly, and changes
> > the format semantics in a subtle way.
>
> It probably wouldn't be too hard to do -- you can get that information
> with readelf if necessary: it's essentially
> readelf -l $LIBRARY | grep -m 1 LOAD | awk '{ print $3 }'
>
> But it would require a change in the file format. Perhaps it's worth
> adding to a list of things to do if it becomes worthwhile to change the
> file format.
After some thought, I think it is better to keep it as it is now.
In my reply above I forgot that a ELF binary itself is always already
relocated to the load address.
Josef
>
> -David
>
|
|
From: Nicholas N. <nj...@cs...> - 2006-04-12 11:09:19
|
On Tue, 11 Apr 2006, L. David Baron wrote: > In any case, what needs to happen for the patch to get in? Generally speaking, Julian, Tom or I need to look at it and decide to check it in. And our time is limited, so not everything that is posted gets considered, unfortunately. Nick |
|
From: Julian S. <js...@ac...> - 2006-04-12 11:21:15
|
> On Tue, 11 Apr 2006, L. David Baron wrote: > > In any case, what needs to happen for the patch to get in? > > Generally speaking, Julian, Tom or I need to look at it and decide to check > it in. And our time is limited, so not everything that is posted gets > considered, unfortunately. I had been following the David - Josef discussion on this. My main concern with this one is that it doesn't break the debuginfo reader in some subtle way, but so far it looks OK on the 4 machines I tested it on. J |
|
From: Julian S. <js...@ac...> - 2006-04-16 01:03:35
|
David, are you able to check out the trunk and verify that it now works as you want? J On Wednesday 12 April 2006 12:20, Julian Seward wrote: > > On Tue, 11 Apr 2006, L. David Baron wrote: > > > In any case, what needs to happen for the patch to get in? > > > > Generally speaking, Julian, Tom or I need to look at it and decide to > > check it in. And our time is limited, so not everything that is posted > > gets considered, unfortunately. > > I had been following the David - Josef discussion on this. My main > concern with this one is that it doesn't break the debuginfo reader > in some subtle way, but so far it looks OK on the 4 machines I > tested it on. > > J |
|
From: L. D. B. <db...@db...> - 2006-04-16 01:58:29
|
On Sunday 2006-04-16 02:03 +0100, Julian Seward wrote: > David, are you able to check out the trunk and verify that it > now works as you want? It does. -David > On Wednesday 12 April 2006 12:20, Julian Seward wrote: > > > On Tue, 11 Apr 2006, L. David Baron wrote: > > > > In any case, what needs to happen for the patch to get in? > > > > > > Generally speaking, Julian, Tom or I need to look at it and decide to > > > check it in. And our time is limited, so not everything that is post= ed > > > gets considered, unfortunately. > > > > I had been following the David - Josef discussion on this. My main > > concern with this one is that it doesn't break the debuginfo reader > > in some subtle way, but so far it looks OK on the 4 machines I > > tested it on. --=20 L. David Baron <URL: http://dbaron.org/ > Technical Lead, Layout & CSS, Mozilla Corporation |