Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

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

Open Quuxplusone opened 8 years ago

Quuxplusone commented 8 years ago
Bugzilla Link PR27066
Status NEW
Importance P normal
Reported by Andrey Kuleshov (kul92@list.ru)
Reported on 2016-03-25 06:44:17 -0700
Last modified on 2018-05-15 13:55:08 -0700
Version trunk
Hardware PC Windows NT
CC andrew.v.tischenko@gmail.com, david.majnemer@gmail.com, dmitry.polukhin@gmail.com, kul92@list.ru, llvm-bugs@lists.llvm.org, mail+llvm@tzik.jp, nicolasweber@gmx.de, paul_robinson@playstation.sony.com, piotr.padlewski@gmail.com, rnk@google.com, zahira.ammarguellat@intel.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also
!- 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: <no line> (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
Quuxplusone 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.
Quuxplusone 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;
    }
Quuxplusone 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.
Quuxplusone 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
Quuxplusone 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
Quuxplusone commented 7 years ago
(In reply to Piotr Padlewski from comment #4)
> 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.
Quuxplusone 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?
Quuxplusone commented 6 years ago

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

Quuxplusone 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.
Quuxplusone commented 6 years ago
(In reply to Taiju Tsuiki from comment #9)
> 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!