[PATCH] D34061: Fix weak symbols on arm and aarch64

[Peter Smith via llvm-commits]

Ok, I'll fill out the rest.

Peter

On 12 June 2017 at 19:00, Rafael Avila de Espindola
<rafael.espindola at gmail.com> wrote:
> Peter Smith via Phabricator <reviews at reviews.llvm.org> writes:
>
>> peter.smith accepted this revision.
>> peter.smith added a comment.
>> This revision is now accepted and ready to land.
>>
>> Looks good to me, thanks for spotting the mistake and apologies for making it in the first place. I've made a minor suggestion about making the default case unreachable but I don't think that this is that important.
>>
>>
>>
>> ================
>> Comment at: ELF/InputSection.cpp:409
>>    default:
>> -    return A;
>> +    return P + A;
>>    }
>> ----------------
>> Might be worth making the default case unreachable.
>>
>> The ABI (ARM ELF 4.5.1.1) says:
>> During linking, the value of an undefined weak reference is:
>> - Zero if the relocation type is absolute
>> - The address of the place if the relocation type is pc-relative
>> - The nominal base address if the relocation type is base-relative.
>>
>> The intention for just returning A was that default: would cover the absolute case. However as long as getARMUndefinedRelativeWeakVA is only called from pc-relative locations this wouldn't happen so I don't expect the default case to ever get hit.
>
> Making the default of both getARMUndefinedRelativeWeakVA and
> getAArch64UndefinedRelativeWeakVA unreachable is probably a good
> thing. Unfortunately some relocations (R_ARM_MOVT_PREL for example) get
> there.
>
> I will commit with the existing default values. Could you please then
> replace the default with the missing cases?
>
> Thanks,
> Rafael