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

[Rafael Avila de Espindola via llvm-commits]

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