[PATCH] D89004: [LLD] [COFF] Implement a GNU/ELF like -wrap option

Martin Storsjö via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 13 23:12:41 PDT 2020


mstorsjo added a comment.

In D89004#2329215 <https://reviews.llvm.org/D89004#2329215>, @MaskRay wrote:

> The testsuite is simply ported from test/ELF.

It's based on those tests yes, but some amount of modifications, and with a few new things. In particular, I added a negative test to see that we really do fail if referencing `__real_func` but it isn't available. (During development, while fiddling with different flags, I had one situation where such an error actually could slip through.)

> The way the tests are organized in ELF is actually not great. For the 3 symbols: `foo __wrap_foo __real_foo`. There are 8 states of their presence. The ELF naming is a bit arbitrary.

Any concrete suggestions on what to improve?



================
Comment at: lld/COFF/MinGW.cpp:194
+// that LTO won't eliminate them.
+std::vector<lld::coff::WrappedSymbol>
+lld::coff::addWrappedSymbols(opt::InputArgList &args) {
----------------
rnk wrote:
> orgads wrote:
> > You have using namespace lld::coff. Remove the qualifiers? (repeats below)
> True for WrappedSymbol, but for the function definition, this applies:
> https://llvm.org/docs/CodingStandards.html#use-namespace-qualifiers-to-implement-previously-declared-functions
Removing it from the WrappedSymbol references.


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

https://reviews.llvm.org/D89004



More information about the llvm-commits mailing list