[PATCH] D88305: [COFF] Aliases resolve directly to defined external targets

[Eric Astor via Phabricator via llvm-commits]

epastor added a comment.

In D88305#2295996 <https://reviews.llvm.org/D88305#2295996>, @mstorsjo wrote:

> I tested this with an input file like this here:
> ...
>
> Maybe it'd be good to add more testcases out of these, in addition to the one added here (with verbose comments in the testcase about what's tested and what the expected outcome is), if some of the cases aren't exercised by the current tests. At least the second-level alias and alias against an undefined symbol (`undefalias`) are untested I think, and I don't see a case that really matches the `weakfunc` case either - so it'd be good to have those covered as well.

I've added a new test file covering (I think) all of your proposed cases except the `weakfunc` case. That one, I added to the `weak.s` test file instead.



================
Comment at: llvm/lib/MC/WinCOFFObjectWriter.cpp:359
+  else
     return nullptr;
 }
----------------
mstorsjo wrote:
> Can you come up with a scenario where you hit this else/return nullptr? Because in all of my testcases, I either hit the "return GetOr...", or return early at the first check (!Symbol.isVariable()).
> 
> I.e. it's hard to get a feel for whether this is the right condition here, as long as one could just as well remove the if statement altogether.
This scenario comes up when making a weak reference to a local label.

I've just added a new test file for most of the alias cases I could think of; the definition of t2 there (aliasee: proc2) triggers this scenario.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88305