This is an archive of the discontinued LLVM Phabricator instance.

Mark ELF sections whose name start with .note as note
ClosedPublic

Authored by phosek on Sep 16 2016, 5:44 PM.

Details

Summary

Previously, such sections would be marked as SHT_PROGBITS which makes it impossible to use an initialized C variable declaration to emit an (allocated) ELF note. The new behavior is also consistent with the ELF assembly parser.

There is a related GCC bug 77609.

Diff Detail

Repository
rL LLVM

Event Timeline

phosek updated this revision to Diff 71716.Sep 16 2016, 5:44 PM
phosek retitled this revision from to Mark ELF sections whose name start with .note as note.
phosek updated this object.
phosek set the repository for this revision to rL LLVM.
phosek added subscribers: llvm-commits, phosek.
ruiu added a subscriber: ruiu.Sep 16 2016, 5:47 PM
ruiu added inline comments.
lib/CodeGen/TargetLoweringObjectFileImpl.cpp
154 ↗(On Diff #71716)

Shouldn't this be Name == ".note" || Name.startswith(".note.")?

phosek added inline comments.Sep 16 2016, 6:05 PM
lib/CodeGen/TargetLoweringObjectFileImpl.cpp
154 ↗(On Diff #71716)

That's definitely more strict so it's probably a better solution. The logic in lib/MC/MCParser/ELFAsmParser.cpp uses Name.startswith(".note"), shall I update that as well?

ruiu added inline comments.Sep 16 2016, 6:07 PM
lib/CodeGen/TargetLoweringObjectFileImpl.cpp
154 ↗(On Diff #71716)

I don't know the answer, but I *think* that's the right thing to do.

phosek added inline comments.Sep 18 2016, 6:46 PM
lib/CodeGen/TargetLoweringObjectFileImpl.cpp
154 ↗(On Diff #71716)

I checked the behavior of GNU assembler and it's equivalent to the current implementation, i.e. even .notefoo would end up with SHT_NOTE type.

davide added a subscriber: davide.Sep 18 2016, 9:23 PM

Side note, I almost always dislike relying on section name specific logic (in the assembler and in the linker), but this is the way things are. LGTM, modulo one comment.

lib/CodeGen/TargetLoweringObjectFileImpl.cpp
154 ↗(On Diff #71716)

Please add a comment here to explain what's going on (maybe add the link to the gcc bug, if you feel it's OK).

davide accepted this revision.Sep 18 2016, 9:23 PM
davide added a reviewer: davide.
This revision is now accepted and ready to land.Sep 18 2016, 9:23 PM
phosek updated this revision to Diff 71974.Sep 20 2016, 1:18 PM
phosek edited edge metadata.
phosek removed rL LLVM as the repository for this revision.
phosek marked an inline comment as done.
This revision was automatically updated to reflect the committed changes.