llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
26.77k stars 10.97k forks source link

[ms] __declspec(dllexport) causes redefinition with same mangled name as another definition #27440

Open llvmbot opened 8 years ago

llvmbot commented 8 years ago
Bugzilla Link 27066
Version trunk
OS Windows NT
Reporter LLVM Bugzilla Contributor
CC @majnemer,@dmpolukhin,@nico,@pogo59,@rnk,@zahiraam

Extended Description

!- This problem is related to dllexport. MSVC compiles sample described below.

Note that even if clang's behavior is correct, compiler's diagnostic is not usable: error: definition with same mangled name as another definition class A {}; (why class A is redefined?)

note: previous definition is here: (where is the line with the previous definition?) -!

==========Small Reproducer============== typedef struct _GUID { int i; } ID;

template <const ID* piid> class A {};

struct __declspec(uuid("{00000000-0000-0000-c000-000000000046}")) S1 {};

struct __declspec(uuid("{00000000-0000-0000-c000-000000000046}")) S2 {};

struct declspec(dllexport) C1 : public A <&uuidof(S1)> {};

struct declspec(dllexport) C2 : public A<&uuidof(S2)> {};

==============Error=====================

clang: test.cpp(4,7) : error: definition with same mangled name as another definition class A {}; ^ test.cpp(4,7) : note: previous definition is here 1 error generated.

MSVC: Creating library test.lib and object test.exp

Andrey Kuleshov

Software Engineer Intel Compiler Team

llvmbot commented 2 years ago

mentioned in issue llvm/llvm-bugzilla-archive#37481

991901f3-cc14-4404-b340-165691b62a58 commented 6 years ago

I hit similar error but without __uuidof nor dllimport, though I'm not sure they are same root cause.

---- foo.cc struct Foo { virtual void f(); virtual void g(); };

void Foo::f() {} void Foo::g() {}

template <void (Foo::*)()> static void h() {}

int main() { h<&Foo::f>(); h<&Foo::g>(); return 0; }

$ clang-cl /c /std:c++17 foo.cc foo.cc(11,13): error: definition with same mangled name as another definition static void h() {} ^ foo.cc(11,13): note: previous definition is here 1 error generated.

This is a different issue, please file a new bug. Thanks!

llvmbot commented 6 years ago

I hit similar error but without __uuidof nor dllimport, though I'm not sure they are same root cause.

---- foo.cc struct Foo { virtual void f(); virtual void g(); };

void Foo::f() {} void Foo::g() {}

template <void (Foo::*)()> static void h() {}

int main() { h<&Foo::f>(); h<&Foo::g>(); return 0; }

$ clang-cl /c /std:c++17 foo.cc foo.cc(11,13): error: definition with same mangled name as another definition static void h() {} ^ foo.cc(11,13): note: previous definition is here 1 error generated.

nico commented 6 years ago

There's an in-progress (but stalled) patch for this at https://reviews.llvm.org/D43576

llvmbot commented 7 years ago

Ok, I guess I should send another bug, but I don't understand why this code compiled and linked fine with clang-3.8 and not trunk?

rnk commented 7 years ago

Hey guys, I get a link error for __declspec(selectany). The project I am trying to build is ChakraCore, and here is the use of it that fails:

https://github.com/Microsoft/ChakraCore/blob/master/lib/Common/ DataStructures/UnitBitVector.h#L491

I doubt that's related. This is about an error that happens during compilation due to mangling collision, not linking. We probably don't honor selectany on Linux.

llvmbot commented 7 years ago

Let me add that I am compiling on linux. It happens with gold and LLD. The interesting thing is that it doesn't happen with the clang-3.8

llvmbot commented 7 years ago

Hey guys, I get a link error for __declspec(selectany). The project I am trying to build is ChakraCore, and here is the use of it that fails:

https://github.com/Microsoft/ChakraCore/blob/master/lib/Common/DataStructures/UnitBitVector.h#L491

991901f3-cc14-4404-b340-165691b62a58 commented 8 years ago

There is another solution which might be better: desugar declspec(uuid). If we invent an extern "C", declspec(selectany) variable called GUID.... we will get the correct semantics. This would also allow us to remove the weird declspec(uuid) handling from the mangler.

rnk commented 8 years ago

I think the problem is that A<&uuidof(S1)> and A<&uuidof(S2)> should be the same instantiation, but they are not.

The solution is probably to add a new kind of TemplateArgument for UUIDs. We shouldn't be doing this kind of expression profiling to deduce whether two template arguments are equivalent: if (Y.getKind() == TemplateArgument::Expression) { // Compare the expressions for equality llvm::FoldingSetNodeID ID1, ID2; X.getAsExpr()->Profile(ID1, Context, true); Y.getAsExpr()->Profile(ID2, Context, true); if (ID1 == ID2) return X; }

pogo59 commented 8 years ago

The "previous definition" actually points to the same location as the original error message; see bug 25343 for another example of this. It's because Clang is using the location of the template definition rather than the instantiation as "the definition." Definitely not optimal diagnostic reporting.

I don't know what uuid() does so no idea whether Clang is correct about mangling S1 and S2 the same way. If they do mangle the same way then MSVC might just be silently allowing the duplication; Clang used to do that too.