[PATCH] D31659: [LLD][ELF] Be more precise about Thumb state bit in ARM thunks

[Peter Smith via llvm-commits]

The particular problem that this fixes is only visible when there are
range extension Thunks, in particular a Thumb to Thumb call. It isn't
worth applying this patch unless as part of the overall range
extension work. I've separated out the test cases for all of the range
extension Thunks in D31665 (non-script) and D31666 (script).

At this stage it is worth me breaking down the reviews that exist for
range extension thunks as there are in effect two branches depending
on whether we insert Thunks into OutputSections::Sections() or
InputSectionDescriptions::Sections().

The first branch of reviews D31656 - D31666 are the first attempt
inserting into OutputSections::Sections and synchronizing the changes
into InputSectionDescriptions::Sections. Some of the early parts of
the review such as D31656 are now covering ground that intersects with
the recent addition of LinkerScript::synchronize(). These reviews are
quite old now and will need rebasing.

The second branch of reviews starts with D32799 this adds Thunks to
InputSectionDescriptions::Sections and synchronizes with
OutputSections::Sections at the end in order to be compatible with
LinkerScript::synchronize(). I've not posted the remainder of the
range extension Thunk patches on this approach, they will look very
much like D31656 - D31666 but with small implementation details
changed.

What I think I really need to know right now is which approach to take
D31656 - D31666 or D32799, once I know that I can update or abandon
the reviews using the alternative approach. It would be great if you
could take a look at D32799 and let me know your opinion?



On 5 May 2017 at 19:18, Rafael Avila de Espindola
<rafael.espindola at gmail.com> wrote:
> Testcase?
> Peter Smith via Phabricator <reviews at reviews.llvm.org> writes:
>
>> peter.smith created this revision.
>> Herald added subscribers: rengolin, aemerson.
>>
>> When we are able to reuse a Thumb thunk from ARM and vice versa the thumb state bit needs to be set to make sure that the bl is converted to a blx.
>>
>>
>> https://reviews.llvm.org/D31659
>>
>> Files:
>>   ELF/Thunks.cpp
>>
>>
>> Index: ELF/Thunks.cpp
>> ===================================================================
>> --- ELF/Thunks.cpp
>> +++ ELF/Thunks.cpp
>> @@ -157,7 +157,7 @@
>>  void ThumbV7ABSLongThunk<ELFT>::addSymbols(ThunkSection &IS) {
>>    this->ThunkSym = addSyntheticLocal<ELFT>(
>>        Saver.save("__Thumbv7ABSLongThunk_" + this->Destination.getName()),
>> -      STT_FUNC, this->Offset, size(), &IS);
>> +      STT_FUNC, this->Offset | 1, size(), &IS);
>>    addSyntheticLocal<ELFT>("$t", STT_NOTYPE, this->Offset, 0, &IS);
>>  }
>>
>> @@ -206,7 +206,7 @@
>>        0x60, 0x47,             //     bx   r12
>>    };
>>    uint64_t S = getARMThunkDestVA(this->Destination);
>> -  uint64_t P = this->ThunkSym->getVA();
>> +  uint64_t P = this->ThunkSym->getVA() & ~0x1;
>>    memcpy(Buf, Data, sizeof(Data));
>>    Target->relocateOne(Buf, R_ARM_THM_MOVW_PREL_NC, S - P - 12);
>>    Target->relocateOne(Buf + 4, R_ARM_THM_MOVT_PREL, S - P - 8);
>> @@ -216,7 +216,7 @@
>>  void ThumbV7PILongThunk<ELFT>::addSymbols(ThunkSection &IS) {
>>    this->ThunkSym = addSyntheticLocal<ELFT>(
>>        Saver.save("__ThumbV7PILongThunk_" + this->Destination.getName()),
>> -      STT_FUNC, this->Offset, size(), &IS);
>> +      STT_FUNC, this->Offset | 1, size(), &IS);
>>    addSyntheticLocal<ELFT>("$t", STT_NOTYPE, this->Offset, 0, &IS);
>>  }
>>
>>
>>
>> Index: ELF/Thunks.cpp
>> ===================================================================
>> --- ELF/Thunks.cpp
>> +++ ELF/Thunks.cpp
>> @@ -157,7 +157,7 @@
>>  void ThumbV7ABSLongThunk<ELFT>::addSymbols(ThunkSection &IS) {
>>    this->ThunkSym = addSyntheticLocal<ELFT>(
>>        Saver.save("__Thumbv7ABSLongThunk_" + this->Destination.getName()),
>> -      STT_FUNC, this->Offset, size(), &IS);
>> +      STT_FUNC, this->Offset | 1, size(), &IS);
>>    addSyntheticLocal<ELFT>("$t", STT_NOTYPE, this->Offset, 0, &IS);
>>  }
>>
>> @@ -206,7 +206,7 @@
>>        0x60, 0x47,             //     bx   r12
>>    };
>>    uint64_t S = getARMThunkDestVA(this->Destination);
>> -  uint64_t P = this->ThunkSym->getVA();
>> +  uint64_t P = this->ThunkSym->getVA() & ~0x1;
>>    memcpy(Buf, Data, sizeof(Data));
>>    Target->relocateOne(Buf, R_ARM_THM_MOVW_PREL_NC, S - P - 12);
>>    Target->relocateOne(Buf + 4, R_ARM_THM_MOVT_PREL, S - P - 8);
>> @@ -216,7 +216,7 @@
>>  void ThumbV7PILongThunk<ELFT>::addSymbols(ThunkSection &IS) {
>>    this->ThunkSym = addSyntheticLocal<ELFT>(
>>        Saver.save("__ThumbV7PILongThunk_" + this->Destination.getName()),
>> -      STT_FUNC, this->Offset, size(), &IS);
>> +      STT_FUNC, this->Offset | 1, size(), &IS);
>>    addSyntheticLocal<ELFT>("$t", STT_NOTYPE, this->Offset, 0, &IS);
>>  }
>>