[PATCH] D32397: [LLD] Avoid empty .eh_frame sections

[Rafael Avila de Espindola via llvm-commits]

LGTM

Thanks,
Rafael

Mark Kettenis via Phabricator <reviews at reviews.llvm.org> writes:

> kettenis created this revision.
>
> Since D30335: [LLD] Add terminator to .eh_frame sections <https://reviews.llvm.org/D30335> got reverted, here is a new attempt to address the issue that prompted me to look into the unterminated .eh_frame section issue. This is that strip on OpenBSD does not correctly handle an empty .eh_frame section and produces broken binaries in that case.  Currently lld creates such an empty .eh_frame section, despite the fact that the OpenBSD crtend.o explicitly inserts a terminator.  The Linux LSB "standard":
>
> https://refspecs.linuxfoundation.org/LSB_5.0.0/LSB-Core-generic/LSB-Core-generic/ehframechpt.html#EHFRAME
>
> explicitly says that
>
>>   The .eh_frame section shall contain 1 or more Call Frame Information (CFI) records.
>
> This diff includes a test that specifically tests the issue I'm seeing on OpenBSD.
>
>
> https://reviews.llvm.org/D32397
>
> Files:
>   lld/ELF/SyntheticSections.cpp
>   lld/test/ELF/Inputs/eh-frame-end.s
>   lld/test/ELF/eh-frame-begin-end.s
>
>
> Index: lld/test/ELF/eh-frame-begin-end.s
> ===================================================================
> --- lld/test/ELF/eh-frame-begin-end.s
> +++ lld/test/ELF/eh-frame-begin-end.s
> @@ -0,0 +1,17 @@
> +// REQUIRES: x86
> +// RUN: llvm-mc -filetype=obj -triple=amd64-unknown-openbsd %s -o %t.o
> +// RUN: llvm-mc -filetype=obj -triple=amd64-unknown-openbsd %p/Inputs/eh-frame-end.s -o %t2.o
> +// RUN: ld.lld %t.o %t2.o -o %t
> +// RUN: llvm-readobj -sections %t | FileCheck %s
> +	
> +// CHECK:      Name: .eh_frame
> +// CHECK-NEXT: Type: SHT_PROGBITS
> +// CHECK-NEXT: Flags [
> +// CHECK-NEXT:   SHF_ALLOC
> +// CHECK-NEXT: ]
> +// CHECK-NEXT: Address: 0x200120
> +// CHECK-NEXT: Offset: 0x120
> +// CHECK-NEXT: Size: 4
> +
> +	.section ".eh_frame", "a", @progbits
> +__EH_FRAME_BEGIN__:
> Index: lld/test/ELF/Inputs/eh-frame-end.s
> ===================================================================
> --- lld/test/ELF/Inputs/eh-frame-end.s
> +++ lld/test/ELF/Inputs/eh-frame-end.s
> @@ -0,0 +1,2 @@
> +	.section ".eh_frame", "a", @progbits
> +	.long 0
> Index: lld/ELF/SyntheticSections.cpp
> ===================================================================
> --- lld/ELF/SyntheticSections.cpp
> +++ lld/ELF/SyntheticSections.cpp
> @@ -541,6 +541,13 @@
>        Off += alignTo(Fde->size(), Config->Wordsize);
>      }
>    }
> +
> +  // The LSB standard does not allow a .eh_frame section with zero
> +  // Call Frame Information records. Therefore add a CIE record length
> +  // 0 as a terminator if this .eh_frame section is empty.
> +  if (Off == 0)
> +    Off = 4;
> +
>    this->Size = Off;
>  }
>  
>
>
> Index: lld/test/ELF/eh-frame-begin-end.s
> ===================================================================
> --- lld/test/ELF/eh-frame-begin-end.s
> +++ lld/test/ELF/eh-frame-begin-end.s
> @@ -0,0 +1,17 @@
> +// REQUIRES: x86
> +// RUN: llvm-mc -filetype=obj -triple=amd64-unknown-openbsd %s -o %t.o
> +// RUN: llvm-mc -filetype=obj -triple=amd64-unknown-openbsd %p/Inputs/eh-frame-end.s -o %t2.o
> +// RUN: ld.lld %t.o %t2.o -o %t
> +// RUN: llvm-readobj -sections %t | FileCheck %s
> +	
> +// CHECK:      Name: .eh_frame
> +// CHECK-NEXT: Type: SHT_PROGBITS
> +// CHECK-NEXT: Flags [
> +// CHECK-NEXT:   SHF_ALLOC
> +// CHECK-NEXT: ]
> +// CHECK-NEXT: Address: 0x200120
> +// CHECK-NEXT: Offset: 0x120
> +// CHECK-NEXT: Size: 4
> +
> +	.section ".eh_frame", "a", @progbits
> +__EH_FRAME_BEGIN__:
> Index: lld/test/ELF/Inputs/eh-frame-end.s
> ===================================================================
> --- lld/test/ELF/Inputs/eh-frame-end.s
> +++ lld/test/ELF/Inputs/eh-frame-end.s
> @@ -0,0 +1,2 @@
> +	.section ".eh_frame", "a", @progbits
> +	.long 0
> Index: lld/ELF/SyntheticSections.cpp
> ===================================================================
> --- lld/ELF/SyntheticSections.cpp
> +++ lld/ELF/SyntheticSections.cpp
> @@ -541,6 +541,13 @@
>        Off += alignTo(Fde->size(), Config->Wordsize);
>      }
>    }
> +
> +  // The LSB standard does not allow a .eh_frame section with zero
> +  // Call Frame Information records. Therefore add a CIE record length
> +  // 0 as a terminator if this .eh_frame section is empty.
> +  if (Off == 0)
> +    Off = 4;
> +
>    this->Size = Off;
>  }
>