Page MenuHomePhabricator

[ELF] Drop .rel[a].debug_gnu_pub{names,types} for --gdb-index --emit-relocs
ClosedPublic

Authored by MaskRay on Fri, Jan 8, 5:50 PM.

Details

Summary

Fixes PR48693: --emit-relocs keeps relocation sections. --gdb-index drops
.debug_gnu_pubnames and .debug_gnu_pubtypes but not their relocation sections.
This can cause a null pointer dereference in getOutputSectionName.

Also delete debug-gnu-pubnames.s which is covered by gdb-index.s

Diff Detail

Event Timeline

MaskRay created this revision.Fri, Jan 8, 5:50 PM
MaskRay requested review of this revision.Fri, Jan 8, 5:50 PM
Herald added a project: Restricted Project. · View Herald TranscriptFri, Jan 8, 5:50 PM
MaskRay edited the summary of this revision. (Show Details)Fri, Jan 8, 5:51 PM

Does this behavior of -gdb-index --emit-relocs match GNU ld?

lld/ELF/SyntheticSections.cpp
2887
if (InputSectionBase *rel = isec->getRelocatedSection())
  return !rel->isLive();
lld/test/ELF/gdb-index.s
4

I think we should test --gdb-index independently too.

55–56

Perhaps, just

SECTION-NOT: .debug_gnu_pub

And move the comment from above here to clarify that this line checks we
don't emit .debug_gnu_pubnames, .debug_gnu_pubtypes and their relocation sections if --emit-relocs is specified.

Does this behavior of -gdb-index --emit-relocs match GNU ld?

GNU ld does not have --gdb-index. The behavior matches gold.

lld/ELF/SyntheticSections.cpp
2887
if (!rel->isLive())
   return true;

is deliberate.

The suggested version is less reliable: if .debug_pubnames erroneously relocates other sections, it may still trigger a crash...

MaskRay added inline comments.Mon, Jan 11, 9:20 AM
lld/ELF/SyntheticSections.cpp
2887

nvm. it is not needed.

MaskRay updated this revision to Diff 315833.Mon, Jan 11, 9:23 AM
MaskRay marked 3 inline comments as done.

comments

lld/test/ELF/gdb-index.s
55–56

Having both is probably clearer. Moved.

grimar accepted this revision.Mon, Jan 11, 11:49 PM

LGTM

This revision is now accepted and ready to land.Mon, Jan 11, 11:49 PM
MaskRay updated this revision to Diff 316002.Tue, Jan 12, 12:01 AM

improve a comment

MaskRay updated this revision to Diff 316003.Tue, Jan 12, 12:03 AM

drop an unneeded change

This revision was landed with ongoing or failed builds.Tue, Jan 12, 12:07 AM
This revision was automatically updated to reflect the committed changes.