[PATCH] D52145: lld-link: Also demangle undefined dllimported symbols.

[Nico Weber via Phabricator via llvm-commits]

thakis marked an inline comment as done.
thakis added a comment.

Thanks! Changed the comment; landing.



================
Comment at: Common/Strings.cpp:41
 Optional<std::string> lld::demangleMSVC(StringRef Name) {
+  // Note: This only works while the check for ? below is present, else a
+  // regular C function that happens to be called __imp__f would be considered
----------------
thakis wrote:
> ruiu wrote:
> > Is "__imp_" name mangling really only for C++ symbol names? I wonder if it's wrong to demangle __imp_f as "__declspec(dllimport) f".
> `__imp_` is also used for C names. But as far as I can tell, for C names a prefix of `__imp_` doesn't necessarily mean that this is an import thunk for whatever follows it: the thunk for `__declspec(dllimport) f`and the name of a regular function `void _imp__f()` both get the name `__imp__f`: https://godbolt.org/z/sbObpp So while this is likely the right thing to do in C, it would only be a heuristic there.
> 
> If you want, I can add this for C too -- we might want to also don't strip the leading `_`that the compiler adds for C functions and chop off the @correction suffix that's added for some calling conventions if we wan to "demangle" C names. Currently we only demangle C++ names, so I only added this for C++ symbols.
Somewhat entertainingly, the following program links and runs fine (with cl and link; 32-bit, for 64-bit it needs to be called `__imp_f` since no _ are inserted by the compiler there):

```
#include <stdio.h>

// Note: Declared only, not defined.
__declspec(dllimport) int f();

void g(void) { printf("hi\n"); }
void (*_imp__f)(void) = &g;

int main() { f(); }
```


https://reviews.llvm.org/D52145