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

[Rafael Avila de Espindola via llvm-commits]

Do you have commit access?

Cheers,
Rafael

Rafael Avila de Espindola <rafael.espindola at gmail.com> writes:

> 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;
>>  }
>>