[lld] r249359 - [ELF2/AArch64] Read the right amount of bytes.

[Rafael Espíndola via llvm-commits]

I don't see how yaml can have anything to do with this.

-  write16le(Location, read32le(Location) | X);
+  write16le(Location, read16le(Location) | X);

The first one was reading too much. We can't put a relocation at the
very end of the file with llvm-mc or yaml2obj (the section table is
there).

I guess you could create one yourself with hexedit, but that really
doesn't look worth it.

-  write64le(Location, read32le(Location) | X);
+  write64le(Location, read64le(Location) | X);

This one was not reading enough. If this was Elf_Rel, you should be
able to create a test with llvm-mc. But with Elf_Rela, the addend is
in the relocation structure.

Which brings the question, Davide, are you sure this code should be
reading the contents? That would be very unusual for a Elf_Rela. Note,
for example, that X86_64 just calls write* and i386 calls add*.

Cheers,
Rafael



On 6 October 2015 at 02:52, Denis Protivensky via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
>
>
> On 10/06/2015 01:43 AM, Davide Italiano via llvm-commits wrote:
>>
>> Author: davide
>> Date: Mon Oct  5 17:43:42 2015
>> New Revision: 249359
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=249359&view=rev
>> Log:
>> [ELF2/AArch64] Read the right amount of bytes.
>>
>> This was clearly wrong (thanks Rui for spotting), and I honestly would
>> like to get this tested so such mistakes won't repeat. Unfortunately, I
>> wasn't (easily) able to craft a test that exposes the bad behavior.
>
> From my experience testing relocations for ARM, this should be possible with
> YAML scripts.
> Just craft a YAML with valid relocation, then manually change the bytes in
> the reloc to make them cause wrong behavior.
> That's of course possible if we decide to write YAML tests for some cases.
>
> - Denis.
>
>> Ideally, we would like to get tests of this kind for all relocations, but
>> at the time of writing, this is not true. So, for now just fix this bug
>> and try to re-evaluate a way to test this in the future.
>>
>> Modified:
>>      lld/trunk/ELF/Target.cpp
>>
>> Modified: lld/trunk/ELF/Target.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Target.cpp?rev=249359&r1=249358&r2=249359&view=diff
>>
>> ==============================================================================
>> --- lld/trunk/ELF/Target.cpp (original)
>> +++ lld/trunk/ELF/Target.cpp Mon Oct  5 17:43:42 2015
>> @@ -286,7 +286,7 @@ static void handle_ABS16(uint8_t *Locati
>>     uint64_t X = S + A;
>>     if (!isInt<16>(X)) // -2^15 <= X < 2^16
>>       error("Relocation R_AARCH64_ABS16 out of range");
>> -  write16le(Location, read32le(Location) | X);
>> +  write16le(Location, read16le(Location) | X);
>>   }
>>     static void handle_ABS32(uint8_t *Location, uint64_t S, int64_t A) {
>> @@ -299,7 +299,7 @@ static void handle_ABS32(uint8_t *Locati
>>   static void handle_ABS64(uint8_t *Location, uint64_t S, int64_t A) {
>>     uint64_t X = S + A;
>>     // No overflow check.
>> -  write64le(Location, read32le(Location) | X);
>> +  write64le(Location, read64le(Location) | X);
>>   }
>>     static void handle_ADD_ABS_LO12_NC(uint8_t *Location, uint64_t S,
>> int64_t A) {
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits