Fixing a clang crash with duplicate mangled name

[ygao]

Summary

Hi,
The following test case crashes in different places depending on whether or not -femit-all-decls is passed
on the command-line. I am thinking that moving the err_duplicate_mangled_name diagnostic from
EmitGlobalFunctionDefinition() into EmitGlobal() should fix the issue.

$ clang -S -emit-llvm -o - test.cpp
$ clang -S -femit-all-decls -emit-llvm -o - test.cpp

// test.cpp
extern "C" int printf(const char *, ...);

void foo(void) __asm("_ZN1SC2Ev");
void foo(void) { }

struct S {

S() {
  printf("S constructor\n");
};

} var;
// end test.cpp

What do you think?

• Gao

[ygao]

A gentle ping.

[akyrtzi]

I don’t see a crash with your test case (TOT r233459), is it crashing with a different invocation ?
And after fully applying the patch the error is not emitted , and the test case fails.

Also FYI, from your test case, if I generate assembly file, I can see:

fatal error: error in backend: '__ZN1SC2Ev' label emitted multiple times to assembly file

But this is only if I add an extra underscore for the __asm label.

[ygao]

Hi Argyrios,
What target triple did you use to build clang? My configuration is as follows:

$ build/Debug+Asserts/bin/clang --version
clang version 3.7.0 (trunk 233464)
Target: x86_64-unknown-linux-gnu
Thread model: posix

$ build/Debug+Asserts/bin/clang -S -emit-llvm -o - D8405.cpp 
llvm/lib/IR/Value.cpp:325: void llvm::Value::replaceAllUsesWith(llvm::Value*): Assertion `New->getType() == getType() && "replaceAllUses of value with new value of different type!"' failed.

[ygao]

Re-based the patch to trunk r234180.
And a gentle ping.

[ygao]

A Darwin-targeted clang may not crash with the original test case, because the __asm() renaming on Darwin prepends
'\01' to the new function name. The following test case should crash on Darwin.

$ clang -S -emit-llvm -o - test.cpp
$ clang -S -femit-all-decls -emit-llvm -o - test.cpp

// test.cpp
extern "C" int printf(const char *, ...);

extern "C" {

void _ZN1SC2Ev(void) { }

}

struct S {

S() {
  printf("S constructor\n");
};

} var;

[akyrtzi]

Committed in r234767, thanks!

lib/CodeGen/CodeGenModule.cpp

                               /*DontDefer=*/false);
       return;
     }
+
+    if (llvm::GlobalValue *GV = GetGlobalValue(getMangledName(GD)))
+      if (!GV->isDeclaration()) {
+        getDiags().Report(FD->getLocation(), diag::err_duplicate_mangled_name);
+        GlobalDecl OldGD = Manglings.lookup(GV->getName());
+        if (auto *Prev = OldGD.getDecl())
+          getDiags().Report(Prev->getLocation(), diag::note_previous_definition);
+        return;
+      }
   } else {
     const auto *VD = cast<VarDecl>(Global);
     assert(VD->isFileVarDecl() && "Cannot emit local var decl as global.");

     }
   }
 
-  if (!GV->isDeclaration()) {
-    getDiags().Report(D->getLocation(), diag::err_duplicate_mangled_name);
-    GlobalDecl OldGD = Manglings.lookup(GV->getName());
-    if (auto *Prev = OldGD.getDecl())
-      getDiags().Report(Prev->getLocation(), diag::note_previous_definition);
-    return;
-  }
-
   if (GV->getType()->getElementType() != Ty) {
     // If the types mismatch then we have to rewrite the definition.
     assert(GV->isDeclaration() && "Shouldn't replace non-declaration");

test/CodeGenCXX/duplicate-mangled-name2.cpp

+// RUN: %clang_cc1 -triple i686-pc-linux-gnu -emit-llvm-only %s -verify
+// RUN: %clang_cc1 -triple i686-pc-linux-gnu -femit-all-decls -emit-llvm-only %s -verify
+
+void foo(void *p) __asm("_ZN1SC2Ev");
+void foo(void *p) { } // expected-note {{previous}}
+
+struct S {
+  S() {} // expected-error {{definition with same mangled name as another definition}}
+} s;

test/CodeGenCXX/duplicate-mangled-name3.cpp

+// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm-only %s -verify
+// RUN: %clang_cc1 -triple %itanium_abi_triple -femit-all-decls -emit-llvm-only %s -verify
+
+extern "C" {
+  void _ZN1SC2Ev(void *p) { } // expected-note {{previous}}
+}
+
+struct S {
+  S() {} // expected-error {{definition with same mangled name as another definition}}
+} s;