[PATCH] D88625: [MC] Redirect fixup symbol to the aliasee (if exists)

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 13 12:15:03 PDT 2020


MaskRay added a comment.

In D88625#2328335 <https://reviews.llvm.org/D88625#2328335>, @jyknight wrote:

>> I have tried adding the expression folding stuff to`MCExpr::evaluateAsRelocatableImpl` but I abandoned that idea because evaluateAsRelocatableImpl does not like a modifier different from VK_None (@PLT). (What does a at plt + 1 do? Relaxing the condition (== VK_None) may work... but the semantics are not very clear.
>
> `a at plt + 1` is perfectly fine -- that turns into a R_X86_64_PLT32 relocation with symbol "a" and addend 1.
>
> The interesting question is what this should do:
>
>   b=a+1 # no @plt!
>   ... b at plt
>
> It's certainly impossible to create a PLT referring to a function at address "a+1", in current object formats. So, probably this should be invalid. However, GAS seems to consider the modifier as more of an overall-modifier, resulting in the same thing as `a at plt + 1`. I agree that's probably not something we ought to support, since it seems wrong.
>
> But, it does still seem reasonable to put the !VK_None support into evaluateAsRelocatableImpl -- as long as the resolved MCValue has only a single symbol, no offset and no kind.

The current way the patch implements the "if a fixup symbol is equated to an expression with an undefined symbol" logic is consistent with the step GNU as implements the check.
GNU as implements the logic when it is about to emit a relocation, not when folding expressions.

One major problem by moving the condition to MCExpr::evaluateAsRelocatableImpl is that we may do `isUndefined()` check too early (this special case does not kick in when the symbol is defined).
The second problem is indeed the cooperation with the `VK_None` special rule on MCExpr.cpp:805. Another minor problem is that this will add another rule to the existing `if (!IsMachO)` hack on 810.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88625/new/

https://reviews.llvm.org/D88625



More information about the llvm-commits mailing list