[PATCH] D121970: [Verify] check that BlockAddresses don't refer to functions with certain linkages which wont be emitted

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

I'm not sure I understand the issue you're trying to prevent here.

You can have basic blocks that won't be emitted into the object file for a bunch of reasons; it might not have any predecessors, it might be inside an unused internal function, or something else.  In all those cases, we use exactly the same strategy: replace the blockaddress with "1".  I don't see any reason to special-case available_externally.

Granted, available_externally functions that contain an indirectbr are currently pretty useless, since they almost always just get discarded.  But we could improve that situation if it mattered.

If you're dealing with weak symbols, there might be issues... but not because there's any fundamental semantic hole.  If a function definition gets discarded, the values of blockaddresses which refer to that definition don't matter.  But the way we currently emit blockaddresses referring to weak functions on ELF targets will lead to link errors, I think. We could solve that by changing the way we lower blockaddress for such functions.  For example, by using a relative offset instead of an absolute address.  I guess that only works for indirectbr, not callbr, but we're likely going to change the way callbr works anyway.

https://reviews.llvm.org/D120781#3383492 has an example of a blockaddress in a linkonce_odr global variable, but I'm pretty sure that has undefined behavior at the IR level.  A blockaddress in one translation unit is never equivalent to a blockaddress in another translation unit, so it breaks the rules for linkonce_odr.  It might make sense for clang to diagnose that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121970