[lld] r288581 - [ELF] - Implemented R_386_16 and R_386PC16 relocations

George Rimar via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 8 00:45:20 PST 2016


>> @@ -437,11 +438,13 @@ uint64_t X86TargetInfo::getImplicitAdden
>>    switch (Type) {
>>    default:
>>      return 0;
>> +  case R_386_16:
>>    case R_386_32:
>>    case R_386_GOT32:
>>    case R_386_GOT32X:
>>    case R_386_GOTOFF:
>>    case R_386_GOTPC:
>> +  case R_386_PC16:
>>    case R_386_PC32:
>>    case R_386_PLT32:
>>    case R_386_TLS_LE:
>
>This reads a 32 bit value for the addend, which is almost certainly
>incorrect.

ABI says these relocations truncates the result. But nothing about truncation of addend.
Imagine next code:

.global _start
.code16
_start:
 movw $foo,%si
foo:
 .quad 0x12345678

It still has 4 bytes addend truncated to word size:
Disassembly of section .text:
0000000000000000 <_start>:
   0:	be 00 00 78 56       	mov    $0x56780000,%esi
			1: R_X86_64_16	.text+0x3
0000000000000003 <foo>:
   3:	78 56                	js     5b <foo+0x58>
   5:	34 12                	xor    $0x12,%al
   7:	00 00                	add    %al,(%rax)

Then logic implemented in the patch will work anyways as it truncates
the output to 16 bits.

At least that was my logic when I implemented that. Was I wrong ?

>
>> +# RUN: yaml2obj %s -o %t.o
>
>Why do you need to use yaml2obj?
>

First version of patch has code instead. It was 2 files in test which did not clearly showed
how implementation truncates the relocation (https://reviews.llvm.org/D27303?id=79921).

I found then that with obj2yaml it is possible to show that in a pretty obvious way.
There relocations are bit special, so I assumed it is fine to have special test for them.

>Cheers,
>Rafael


George.


More information about the llvm-commits mailing list