[PATCH][lld] ELF/AArch64: Add support for checking for ABS32 overflow

[Will Newton]

On 26 January 2015 at 18:37, Shankar Easwaran <shankare at codeaurora.org> wrote:
> On 1/21/2015 12:03 PM, Will Newton wrote:
>>
>> Add support for checking overflow when applying a R_AARCH64_ABS32
>> relocation and add a test to ensure it behaves correctly.
>> ---
>>   .../ELF/AArch64/AArch64RelocationHandler.cpp       | 20 ++++++--
>>   test/elf/AArch64/rel-abs32-overflow.test           | 53
>> ++++++++++++++++++++++
>>   2 files changed, 68 insertions(+), 5 deletions(-)
>>   create mode 100644 test/elf/AArch64/rel-abs32-overflow.test
>>
>> diff --git a/lib/ReaderWriter/ELF/AArch64/AArch64RelocationHandler.cpp
>> b/lib/ReaderWriter/ELF/AArch64/AArch64RelocationHandler.cpp
>> index 924836d..85a3038 100644
>> --- a/lib/ReaderWriter/ELF/AArch64/AArch64RelocationHandler.cpp
>> +++ b/lib/ReaderWriter/ELF/AArch64/AArch64RelocationHandler.cpp
>> @@ -16,6 +16,13 @@ using namespace elf;
>>     #define PAGE(X) ((X) & ~0x0FFFL)
>>   +/// \brief Check X is in the interval (-2^(bits-1), 2^bits]
>> +bool withinSignedUnsignedRange(int64_t X, int bits) {
>> +  if (X < -(1L << (bits - 1)) || X >= (1L << bits))
>> +    return false;
>> +  return true;
>> +}
>> +
>
> Could you move this inside RelocationHelperFunctions.h ?

I had a look at this but decided it might be better to leave it where
it is for now. I'm not sure if other ports will end up using this
helper or whether it is purely a requirement of AArch64.

>>   /// \brief R_AARCH64_ABS64 - word64: S + A
>>   static void relocR_AARCH64_ABS64(uint8_t *location, uint64_t P, uint64_t
>> S,
>>                                    int64_t A) {
>> @@ -41,9 +48,11 @@ static void relocR_AARCH64_PREL32(uint8_t *location,
>> uint64_t P, uint64_t S,
>>   }
>>     /// \brief R_AARCH64_ABS32 - word32:  S + A
>> -static void relocR_AARCH64_ABS32(uint8_t *location, uint64_t P, uint64_t
>> S,
>> -                                 int64_t A) {
>> -  int32_t result = (int32_t)(S + A);
>> +static std::error_code relocR_AARCH64_ABS32(uint8_t *location, uint64_t
>> P,
>> +                                            uint64_t S, int64_t A) {
>> +  int64_t result = S + A;
>> +  if (!withinSignedUnsignedRange(result, 32))
>> +    return make_out_of_range_reloc_error();
>>     DEBUG_WITH_TYPE(
>>         "AArch64", llvm::dbgs() << "\t\tHandle " << LLVM_FUNCTION_NAME <<
>> " -";
>>         llvm::dbgs() << " S: 0x" << Twine::utohexstr(S);
>> @@ -53,6 +62,7 @@ static void relocR_AARCH64_ABS32(uint8_t *location,
>> uint64_t P, uint64_t S,
>>     *reinterpret_cast<llvm::support::ulittle32_t *>(location) =
>>         result |
>>         (int32_t) * reinterpret_cast<llvm::support::little32_t
>> *>(location);
>> +  return std::error_code();
>>   }
>>     /// \brief R_AARCH64_ADR_PREL_PG_HI21 - Page(S+A) - Page(P)
>> @@ -385,8 +395,8 @@ std::error_code
>> AArch64TargetRelocationHandler::applyRelocation(
>>                             ref.addend());
>>       break;
>>     case R_AARCH64_ABS32:
>> -    relocR_AARCH64_ABS32(location, relocVAddress, targetVAddress,
>> ref.addend());
>> -    break;
>> +    return relocR_AARCH64_ABS32(location, relocVAddress, targetVAddress,
>> +                                ref.addend());
>>     // Runtime only relocations. Ignore here.
>>     case R_AARCH64_RELATIVE:
>>     case R_AARCH64_IRELATIVE:
>> diff --git a/test/elf/AArch64/rel-abs32-overflow.test
>> b/test/elf/AArch64/rel-abs32-overflow.test
>> new file mode 100644
>> index 0000000..be65ebc
>> --- /dev/null
>> +++ b/test/elf/AArch64/rel-abs32-overflow.test
>> @@ -0,0 +1,53 @@
>> +# Check handling of R_AARCH64_ABS32 relocation overflow.
>> +# RUN: yaml2obj -format=elf %s > %t-obj
>> +# RUN: not lld -flavor gnu -target arm64 -o %t-exe %t-obj 2>&1 |
>> FileCheck %s
>> +
>> +# CHECK: Relocation out of range in file {{.*}}: reference from data1+0
>> to data2+34359738369 of type 258 (R_AARCH64_ABS32)
>> +# CHECK: Relocation out of range in file {{.*}}: reference from data2+0
>> to data1+34359738369 of type 258 (R_AARCH64_ABS32)
>> +
>> +!ELF
>> +FileHeader: !FileHeader
>> +  Class: ELFCLASS64
>> +  Data: ELFDATA2LSB
>> +  Type: ET_REL
>> +  Machine: EM_AARCH64
>> +
>> +Sections:
>> +- Name: .text
>> +  Type: SHT_PROGBITS
>> +  Content: "00000000"
>> +  AddressAlign: 16
>> +  Flags: [SHF_ALLOC, SHF_EXECINSTR]
>> +- Name: .data
>> +  Type: SHT_PROGBITS
>> +  Content: "0000000000000000"
>> +  AddressAlign: 16
>> +  Flags: [SHF_ALLOC, SHF_WRITE]
>> +
>> +- Name: .rela.data
>> +  Type: SHT_RELA
>> +  Info: .data
>> +  AddressAlign: 8
>> +  Relocations:
>> +    - Offset: 0x0
>> +      Symbol: data2
>> +      Type: R_AARCH64_ABS32
>> +      Addend: 0x800000001
>> +    - Offset: 0x4
>> +      Symbol: data1
>> +      Type: R_AARCH64_ABS32
>> +      Addend: 0x800000001
>> +
>> +Symbols:
>> +  Global:
>> +    - Name: _start
>> +      Section: .text
>> +      Value: 0x0
>> +      Size: 4
>> +    - Name: data1
>> +      Section: .data
>> +      Size: 4
>> +    - Name: data2
>> +      Section: .data
>> +      Value: 0x4
>> +      Size: 4
>
>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by
> the Linux Foundation
>



-- 
Will Newton
Toolchain Working Group, Linaro