This is an archive of the discontinued LLVM Phabricator instance.

[LLD][ELF] Remove Duplicate .ARM.exidx sections
ClosedPublic

Authored by peter.smith on Dec 7 2017, 9:29 AM.

Details

Summary

The ARM.exidx section contains a table of 8-byte entries with the first word of each entry an offset to the function it describes and the second word instructions for unwinding if an exception is thrown from that function. The SHF_LINK_ORDER processing will order the table in ascending order of the functions described by the exception table entries. As the address range of an exception table entry is terminated by the next table entry, it is possible to merge consecutive table entries that have identical unwind instructions.

For this implementation we define a table entry to be identical if:

  • Both entries are the special EXIDX_CANTUNWIND.
  • Both entries have the same inline unwind instructions.

We do not attempt to establish if table entries that are references to
.ARM.extab sections are identical.

This implementation works at a granularity of a single .ARM.exidx InputSection. If all entries in the InputSection are identical to the previous table entry we can remove the InputSection. A more sophisticated but more complex implementation would rewrite InputSection contents so that duplicates within a .ARM.exidx InputSection can be merged. The implementation works really well for code compiled with -ffunction-sections and for code such as llvm that is compiled without exceptions (reduces to a single EXIDX_CANTUNWIND). For the real code I've tested it on it seems to be about half as effective as the fine grained approach, reducing the table size to approximately 70% of its original size. The fine grained approach as used by gold and bfd reduces table size to about 55% of its original value. If projects need the find grained approach I can keep working on this.

This is patch 3 of 3 in sequence:

  • D40964 Move SHF_LINK_ORDER processing earlier in Writer.cpp [NFC]
  • D40966 Refactor to remove loop copying all Sections in OS->finalize() [NFC]
  • D40967 Remove Duplicate .ARM.exidx sections

Diff Detail

Event Timeline

peter.smith created this revision.Dec 7 2017, 9:29 AM
ruiu added inline comments.Dec 11 2017, 9:52 PM
ELF/Driver.cpp
627

Can you also add -merge-exidx-entries as well and make it the primary option (i.e. add Config->MergeArmExidx instead of NoMergeArmExidx).

ELF/Writer.cpp
1162

Please mention that this is for --merge-exidx-entries and this is ARM only.

1200

nit: remove () after return.

1205

nit: you can remove {}

1241–1251

I wonder if you can use std::unique to remove consecutive duplicate list elements.

Thanks very much for the review comments. I've updated the diff to apply them. The one I haven't done yet is std::unique as I don't think I can make the predicate an equivalence relation without missing opportunities. For an example consider two consecutive (made up) unwind entries { 0xabababab, 0x1 } and { 0x1, 0x1, 0x1 }; I can merge the second into the first but not the first into the second. My guess is that the libstdc++ and libc++ implementations of std::unique will let us get away with the predicate not being symmetric but I have memories of the Visual Studio debug implementation actively checking all the preconditions required by the standard.

An alternative would be to write a version of std::unique inline, perhaps in the place of // Remove the Sections we marked as duplicate earlier. That would allow us to remove the middle chunk that sets duplicates to nullptr. Doing the unique check on the unified list does have the advantage of detecting duplicates across an InputSectionDescription boundary but I think that this wouldn't be common or significant in size. Let me know what you think?

Comments from Rafael via [llvm-commits]

+ struct ExidxEntry {
+ uint32_t Fn;
+ uint32_t Unwind;
+ };
+
+ // Get the last table Entry from the previous .ARM.exidx section.
+ const ExidxEntry PrevEntry = *reinterpret_cast<const ExidxEntry *>(
+ Prev->Data.data() + Prev->getSize() - sizeof(ExidxEntry));
+ if (IsExtabRef(PrevEntry.Unwind))
+ return false;

This is not safe in a big endian host, no?

I've updated the structure to contain ulittle32_t to match the original introduction in D25127. I'm happy to do it another way if you'd prefer.

Don't we have support for dumpling exidx in llvm-objdump? I was
surprised to not find it in Object/ELFTypes.h.

There is some support in llvm-readobj -u. Having given it a try it seems unreliable (segfaults frequently) when run on executables, and includes executables linked by .bfd and where gnu readelf -u has no problem with them. I've kept with the llvm-objdump -s for now, which also allows us to use fake .ARM.extab data that might also make llvm-readobj more prone to crash.

I think you forgot to upload th new patch.

Cheers,
Rafael

Sorry about that, I've uploaded it this time.

Thanks very much for the comments, I've applied the suggested changes.

+ RUN: llvm-mc -filetype=obj -triple=armv7a-none-linux-gnueabi %s -o %t
+
RUN: ld.lld %t --no-merge-exidx-entries -o %t2 2>&1

Why the 2>&1 ?

No good reason, I've taken them out.

+// CHECK-NEXT: 100f4 300f0000 01000000

Is it easy to check that this is the end of the section?

I've added a CHECK-NEXT for the line afterwords (.ARM.extab section name).

Cheers,
Rafael

This revision was not accepted when it landed; it landed in state Needs Review.Dec 15 2017, 3:10 AM
This revision was automatically updated to reflect the committed changes.