[PATCH] D135737: [LLD][ELF] restore behaviour of __real_foo being a weak reference to a lazy foo

ben via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 13 10:45:21 PDT 2022


bd1976llvm added a comment.

In D135737#3854200 <https://reviews.llvm.org/D135737#3854200>, @MaskRay wrote:

> (--wrap is so complex that I can't come up with a list of rules to follow. I can only make sense by inspecting test output difference.)
>
> For `wrap-extract-real.s`, I don't think the new behavior improves semantics.
>
>   ld.lld %t/a.o --wrap foo  # error: undefined symbol: __real_foo
>   ld.lld %t/a.o --start-lib %t/b.o --end-lib --wrap foo  # ok, even if %t/b.o is not extracted. This is weird
>
> Generally, if an archive member is not extracted, we expect that there is no symbol resolution difference.

Thanks for the comments. I agree that the semantics are weird with this change in its current form. This is because I was trying to make as small a change as possible (because --wrap is hard I find!). We could improve this change so the "ld.lld %t/a.o --wrap foo" case is ok as well. However, I have discovered that:

1. We have a case where someone is referencing `__real_xxx` outside of `__wrap_xxx` (I wasn't able to find any documentation to state that this is disallowed and it seems useful) which means that this change (and LLD behaviour previous to https://github.com/llvm/llvm-project/commit/8b01b638d0145473d27a0dd99ded48cc5a8b85a1) leads to a reference patched to zero which could be a bug unless users are aware that `__real_xxx` references are treated as weak (easy to get wrong).

2. We can change the implementation for our library that inspired this so that we no longer have the same requirements and would be happy with the ld.bfd behaviour of `__real_xxx` references extracting `xxx` from an archive.

For those reasons I am going to abandon this revision.


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

https://reviews.llvm.org/D135737



More information about the llvm-commits mailing list