Without this fix, strip -g misbehaves when stripping object files and other ELF objects containing debuginfo relocation sections, preserving all debuginfo sections in the output file because the .rela.debug* sections reference them.
Signed-off-by: Nick Alcock nick.alcock@oracle.com --- The selection of sections to RELAFY here is pretty much arbitrary, covering everything I've seen .rela.* created for and anything else that seems likely to need it.
Testing is trivial: try to strip -g a Linux kernel module (or any other object file containing debug info) and observe the .debug* and .rela.debug* sections turning up in the stripped output rather than (if specified) the -f file. binutils behaves correctly in this area.
I haven't considered DWARF 1 sections not shared with DWARF 2+ at all because I have no way to generate such sections for testing.
libebl/ChangeLog | 5 +++++ libebl/eblopenbackend.c | 36 +++++++++++++++++++----------------- 2 files changed, 24 insertions(+), 17 deletions(-)
diff --git a/libebl/ChangeLog b/libebl/ChangeLog index af819a2..c6204c6 100644 --- a/libebl/ChangeLog +++ b/libebl/ChangeLog @@ -1,3 +1,8 @@ +2012-09-18 Nick Alcock nick.alcock@oracle.com + + * eblopenbackend.c (default_debugscn_p): Add .rela.* versions of + many DWARF 2+ debuginfo sections. + 2012-08-22 Jeff Kenton jkenton@tilera.com
* eblopenbackend.c (machines): Add tilegx. diff --git a/libebl/eblopenbackend.c b/libebl/eblopenbackend.c index 89e5da5..d10b0d0 100644 --- a/libebl/eblopenbackend.c +++ b/libebl/eblopenbackend.c @@ -619,6 +619,7 @@ default_object_note (const char *name __attribute__ ((unused)), static bool default_debugscn_p (const char *name) { +#define RELAFY(x) x, ".rela" x /* We know by default only about the DWARF debug sections which have fixed names. */ static const char *dwarf_scn_names[] = @@ -630,31 +631,32 @@ default_debugscn_p (const char *name) ".debug_srcinfo", ".debug_sfnames", /* DWARF 1.1 and DWARF 2 */ - ".debug_aranges", - ".debug_pubnames", + RELAFY(".debug_aranges"), + RELAFY(".debug_pubnames"), /* DWARF 2 */ - ".debug_info", - ".debug_abbrev", - ".debug_line", - ".debug_frame", - ".debug_str", - ".debug_loc", - ".debug_macinfo", + RELAFY(".debug_info"), + RELAFY(".debug_abbrev"), + RELAFY(".debug_line"), + RELAFY(".debug_frame"), + RELAFY(".debug_str"), + RELAFY(".debug_loc"), + RELAFY(".debug_macinfo"), /* DWARF 3 */ - ".debug_ranges", - ".debug_pubtypes", + RELAFY(".debug_ranges"), + RELAFY(".debug_pubtypes"), /* DWARF 4 */ - ".debug_types", + RELAFY(".debug_types"), /* GDB DWARF 4 extension */ ".gdb_index", /* GNU/DWARF 5 extension/proposal */ - ".debug_macro", + RELAFY(".debug_macro"), /* SGI/MIPS DWARF 2 extensions */ - ".debug_weaknames", - ".debug_funcnames", - ".debug_typenames", - ".debug_varnames" + RELAFY(".debug_weaknames"), + RELAFY(".debug_funcnames"), + RELAFY(".debug_typenames"), + RELAFY(".debug_varnames") }; +#undef RELAFY const size_t ndwarf_scn_names = (sizeof (dwarf_scn_names) / sizeof (dwarf_scn_names[0])); for (size_t cnt = 0; cnt < ndwarf_scn_names; ++cnt)
This is not the right way to fix it. Instead, SHT_REL/SHT_RELA sections should be detected by sh_type and their disposition made to match that of the section their sh_info points to.
Thanks, Roland
On 18 Sep 2012, Roland McGrath outgrape:
This is not the right way to fix it. Instead, SHT_REL/SHT_RELA sections should be detected by sh_type and their disposition made to match that of the section their sh_info points to.
That's exactly the response I was hoping for: this felt sort of pig-ugly but I wasn't sure what the right fix was. It's obvious in hindsight, and looking at it quickly it will give precisely the right answer.
Patch forthcoming shortly, sorry for inflicting the ugly on you.
When deciding whether a section is strippable, ebl_section_strip_p() checks the section pointed to by the sh_info of relocation sections to see if the relocated section is strippable.
Unfortunately a one-character typo leads to this check always being skipped. (You can tell it's a typo because if the condition were correct, the next line would dereference a null pointer.)
Signed-off-by: Nick Alcock nick.alcock@oracle.com --- libebl/ChangeLog | 4 ++++ libebl/eblsectionstripp.c | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/libebl/ChangeLog b/libebl/ChangeLog index af819a2..28d6b10 100644 --- a/libebl/ChangeLog +++ b/libebl/ChangeLog @@ -1,3 +1,7 @@ +2012-09-18 Nick Alcock nick.alcock@oracle.com + + * eblsectionstripp.c (ebl_section_strip_p): Fix typo. + 2012-08-22 Jeff Kenton jkenton@tilera.com
* eblopenbackend.c (machines): Add tilegx. diff --git a/libebl/eblsectionstripp.c b/libebl/eblsectionstripp.c index 9497068..6455344 100644 --- a/libebl/eblsectionstripp.c +++ b/libebl/eblsectionstripp.c @@ -51,7 +51,7 @@ ebl_section_strip_p (Ebl *ebl, const GElf_Ehdr *ehdr, const GElf_Shdr *shdr, Elf_Scn *scn_l = elf_getscn (ebl->elf, (shdr)->sh_info); GElf_Shdr shdr_mem_l; GElf_Shdr *shdr_l = gelf_getshdr (scn_l, &shdr_mem_l); - if (shdr_l == NULL) + if (shdr_l != NULL) { const char *s_l = elf_strptr (ebl->elf, ehdr->e_shstrndx, shdr_l->sh_name);
That fix is fine. Would you like to get yourself set up with a fedoraproject.org and join the gitelfutils group so you can push your own commits?
Thanks, Roland
On Tue, 2012-09-18 at 20:23 +0100, Nick Alcock wrote:
When deciding whether a section is strippable, ebl_section_strip_p() checks the section pointed to by the sh_info of relocation sections to see if the relocated section is strippable.
Unfortunately a one-character typo leads to this check always being skipped. (You can tell it's a typo because if the condition were correct, the next line would dereference a null pointer.)
Signed-off-by: Nick Alcock nick.alcock@oracle.com
Things should have been setup so that you can push this yourself. But I can also do it for you.
Thanks,
Mark
On 10 Oct 2012, Mark Wielaard uttered the following:
On Tue, 2012-09-18 at 20:23 +0100, Nick Alcock wrote:
When deciding whether a section is strippable, ebl_section_strip_p() checks the section pointed to by the sh_info of relocation sections to see if the relocated section is strippable.
Unfortunately a one-character typo leads to this check always being skipped. (You can tell it's a typo because if the condition were correct, the next line would dereference a null pointer.)
Signed-off-by: Nick Alcock nick.alcock@oracle.com
Things should have been setup so that you can push this yourself. But I can also do it for you.
Hold off: work said yes about the contributor agreement and then as soon as I'd agreed to it, said 'no, wait for legal'. I'm now acting as if I hadn't agreed to it, and waiting for legal. For weeks and weeks. Even though dozens of people in my department have already signed it. Sigh.
Sorry about this :( I'll rebase against trunk and repost once I'm actually allowed to push this exciting one-character change.
Condolences on your corporate legal department's silliness. I don't think the paperwork really matters at all for a one-character change. I guess it would be improper to do a commit with your Signed-off-by line without the paperwork, but one of us (me if I were not so lazy) could just commit the fix.
Thanks, Roland
On Wed, Oct 10, 2012 at 03:33:06PM -0700, Roland McGrath wrote:
Condolences on your corporate legal department's silliness. I don't think the paperwork really matters at all for a one-character change. I guess it would be improper to do a commit with your Signed-off-by line without the paperwork, but one of us (me if I were not so lazy) could just commit the fix.
Grin, yeah. I have been straining myself to not just recreate the fix from scratch (writing this email is probably more work). Because I think it is better to have someone blocked on something this simple than have them being blocked in the future when they have something more substantial. Lets see if Nick can get something good out of his legal department. After that all future contributions should come "for free". Nick please let us know if things look hopeless then we can certainly just write a very similar fix ourselves. I have it on my list to double check before we do a new release.
Thansk,
Mark
On 11 Oct 2012, Mark Wielaard said:
Grin, yeah. I have been straining myself to not just recreate the fix from scratch (writing this email is probably more work). Because I think it is better to have someone blocked on something this simple than have them being blocked in the future when they have something more substantial. Lets see if Nick can get something good out of his legal department. After that all future contributions should come "for free". Nick please let us know if things look hopeless then we can certainly just write a very similar fix ourselves. I have it on my list to double check before we do a new release.
Things do definitely not look hopeless: there was a communications screwup and nobody had actually talked to the legal dept at all (everyone was deadlocked waiting for someone else). Given that numerous people in the same dept are Fedora maintainers of long standing, I'm reasonably certain that I will be allowed to sign the thing soon!
Hi Nick,
On Thu, 2012-10-11 at 13:19 +0100, Nick Alcock wrote:
On 11 Oct 2012, Mark Wielaard said:
Grin, yeah. I have been straining myself to not just recreate the fix from scratch (writing this email is probably more work). Because I think it is better to have someone blocked on something this simple than have them being blocked in the future when they have something more substantial. Lets see if Nick can get something good out of his legal department. After that all future contributions should come "for free". Nick please let us know if things look hopeless then we can certainly just write a very similar fix ourselves. I have it on my list to double check before we do a new release.
Things do definitely not look hopeless: there was a communications screwup and nobody had actually talked to the legal dept at all (everyone was deadlocked waiting for someone else). Given that numerous people in the same dept are Fedora maintainers of long standing, I'm reasonably certain that I will be allowed to sign the thing soon!
Sorry I rewrote the patch for now, since I didn't want to miss it going in before the next release (it missed the last one). So I pushed the attached.
Cheers,
Mark
elfutils-devel@lists.fedorahosted.org