[PATCH] D40967: [LLD][ELF] Remove Duplicate .ARM.exidx sections

[Rafael Avila de Espindola via llvm-commits]

Peter Smith via Phabricator <reviews at reviews.llvm.org> writes:


> Index: test/ELF/arm-exidx-dedup.s
> ===================================================================
> --- /dev/null
> +++ test/ELF/arm-exidx-dedup.s
> @@ -0,0 +1,124 @@
> +// 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 ?

> +// RUN: llvm-objdump -s %t2 | FileCheck --check-prefix CHECK-DUPS %s
> +// RUN: ld.lld %t -o %t3 2>&1

Same here

> +// RUN: llvm-objdump -s %t3 | FileCheck %s
> +// REQUIRES: arm
> +// Test that lld can at least remove duplicate .ARM.exidx sections. A more
> +// fine grained implementation will be able to remove duplicate entries within
> +// a .ARM.exidx section.
> +
> +// With duplicate entries
> +// CHECK-DUPS: Contents of section .ARM.exidx:
> +// CHECK-DUPS-NEXT:  100d4 2c0f0000 01000000 280f0000 01000000
> +// CHECK-DUPS-NEXT:  100e4 240f0000 01000000 200f0000 01000000
> +// CHECK-DUPS-NEXT:  100f4 1c0f0000 08849780 180f0000 08849780
> +// CHECK-DUPS-NEXT:  10104 140f0000 08849780 100f0000 14000000
> +// CHECK-DUPS-NEXT:  10114 0c0f0000 18000000 080f0000 01000000
> +
> +// After duplicate entry removal
> +// CHECK: Contents of section .ARM.exidx:
> +// CHECK-NEXT:  100d4 2c0f0000 01000000 340f0000 08849780
> +// CHECK-NEXT:  100e4 380f0000 14000000 340f0000 18000000
> +// CHECK-NEXT:  100f4 300f0000 01000000

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

> +  // References to .ARM.Extab Sections have bit 31 clear and are not the
> +  // special EXIDX_CANTUNWIND bit-pattern.
> +  auto IsExtabRef = [](ulittle32_t Unwind) {

This should take a uint32_t. In general the endian specific types should
be used only to form pointers. Once read it is a plain uint32_t.

> +    return (Unwind & 0x80000000) == 0 && Unwind != 0x1;
> +  };
> +
> +  struct ExidxEntry {
> +    ulittle32_t Fn;
> +    ulittle32_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));

Make PrevEntry a pointer or a reference.

> +  for (const ExidxEntry Entry : Entries)

Make entry a reference.


Cheers,
Rafael