Crash when building with Clang tip and thread-sanitizer

[GoogleCodeExporter]

When compile mozilla-central with Clang tip and thread-sanitizer enabled, Clang 
crashes during compilation of xpcom/io/nsStreamUtils.cpp. I verified that this 
only happens with thread-sanitizer. If I just build without sanitizers, this 
builds fine.

Attached is the log of the crash, as well as the cpp/sh files written by Clang.

I used LLVM r195890 on Ubuntu LTS 64 bit.

Original issue reported on code.google.com by decoder...@googlemail.com on 29 Nov 2013 at 12:14

Attachments:

• tsan-crash.txt
• nsStreamUtils-5a95b4.cpp
• nsStreamUtils-5a95b4.sh

[GoogleCodeExporter]

Original comment by konstant...@gmail.com on 29 Nov 2013 at 12:23

[GoogleCodeExporter]

I can reproduce this, stack trace is inside tsan phase: 

clang: /home/kcc/llvm/lib/IR/Instructions.cpp:2470: static llvm::CastInst 
*llvm::CastInst::CreatePointerCast(llvm::Value *, llvm::Type *, const 
llvm::Twine &, llvm::Instruction *): Assertion `Ty->isVectorTy() == 
S->getType()->isVectorTy() && "Invalid cast"' failed.

#0  0x00007ffff67d7425 in __GI_raise (sig=<optimized out>) at 
../nptl/sysdeps/unix/sysv/linux/raise.c:64
#1  0x00007ffff67dab8b in __GI_abort () at abort.c:91
#2  0x00007ffff67d00ee in __assert_fail_base (fmt=<optimized out>, 
assertion=0x25d9a9a "Ty->isVectorTy() == S->getType()->isVectorTy() && 
\"Invalid cast\"", 
    file=0x25d7653 "/home/kcc/llvm/lib/IR/Instructions.cpp", line=<optimized out>, function=<optimized out>) at assert.c:94
#3  0x00007ffff67d0192 in __GI___assert_fail (assertion=0x25d9a9a 
"Ty->isVectorTy() == S->getType()->isVectorTy() && \"Invalid cast\"", 
file=0x25d7653 "/home/kcc/llvm/lib/IR/Instructions.cpp", 
    line=2470, function=0x25d9b47 "static llvm::CastInst *llvm::CastInst::CreatePointerCast(llvm::Value *, llvm::Type *, const llvm::Twine &, llvm::Instruction *)") at assert.c:103
#4  0x000000000126b396 in llvm::CastInst::CreatePointerCast(llvm::Value*, 
llvm::Type*, llvm::Twine const&, llvm::Instruction*) ()
#5  0x0000000000ba0c96 in llvm::IRBuilder<true, llvm::ConstantFolder, 
llvm::IRBuilderDefaultInserter<true> >::CreatePointerCast(llvm::Value*, 
llvm::Type*, llvm::Twine const&) ()
#6  0x0000000001eb2626 in (anonymous 
namespace)::ThreadSanitizer::runOnFunction(llvm::Function&) ()
#7  0x000000000127c7bc in llvm::FPPassManager::runOnFunction(llvm::Function&) ()
#8  0x000000000127ca2b in llvm::FPPassManager::runOnModule(llvm::Module&) ()
#9  0x000000000127cfa5 in llvm::legacy::PassManagerImpl::run(llvm::Module&) ()
#10 0x000000000127d31a in llvm::legacy::PassManager::run(llvm::Module&) ()
#11 0x000000000137d1d0 in clang::EmitBackendOutput(clang::DiagnosticsEngine&, 
clang::CodeGenOptions const&, clang::TargetOptions const&, clang::LangOptions 
const&, llvm::Module*, clang::BackendAction, llvm::raw_ostream*) ()
#12 0x000000000137a60b in 
clang::BackendConsumer::HandleTranslationUnit(clang::ASTContext&) ()
#13 0x0000000001630fd3 in clang::ParseAST(clang::Sema&, bool, bool) ()
#14 0x0000000001379a82 in clang::CodeGenAction::ExecuteAction() ()
#15 0x00000000015783c1 in clang::FrontendAction::Execute() ()
#16 0x000000000155738d in 
clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) ()
#17 0x0000000001362453 in 
clang::ExecuteCompilerInvocation(clang::CompilerInstance*) ()
#18 0x000000000067d5cf in cc1_main(char const**, char const**, char const*, 
void*) ()
#19 0x000000000067bde5 in main ()

Original comment by konstant...@gmail.com on 29 Nov 2013 at 12:25

[GoogleCodeExporter]

Somewhat minimal reproducer. Ouch.
clang++  -std=c++11 -c -O2 bug3.cc -fsanitize=thread 

class nsISupports {
  virtual int AddRef();
};
enum nsAsyncCopyMode {};
class nsIRunnable : public nsISupports {};
class nsIInputStreamCallback : nsISupports {};
typedef enum tag_nsresult : int nsresult;
class nsAStreamCopier : nsIInputStreamCallback, public nsIRunnable {};
class nsStreamCopierIB : nsAStreamCopier {};
class nsStreamCopierOB : public nsAStreamCopier {};
nsresult fn1(nsAsyncCopyMode mode, nsISupports **aCopierCtx) {
  nsresult rv;
  nsAStreamCopier *copier;
  if (mode)
    new nsStreamCopierIB;
  else
    copier = new nsStreamCopierOB;
  *aCopierCtx = static_cast<nsISupports *>(static_cast<nsIRunnable *>(copier));
  return rv;
}

Original comment by konstant...@gmail.com on 29 Nov 2013 at 3:13

[GoogleCodeExporter]

a bit smaller:

class A {   virtual int v(); };
class B : public A {};
class C : A, public B {};
class D : C {};
class E : public C {}; 
int fn1(int b, A **a) {
  C *c;
  if (b)
    new D;
  else
    c = new E;
  *a = static_cast<A *>(static_cast<B *>(c));
  return 0;
} 

Original comment by konstant...@gmail.com on 29 Nov 2013 at 3:20

[GoogleCodeExporter]

The code that tsan gets as the input has a vector store marked as vtable update:

store <2 x i32 (...)**> %1, <2 x i32 (...)**>* %2, align 8, !tbaa !1
!1 = metadata !{metadata !2, metadata !2, i64 0}
!2 = metadata !{metadata !"vtable pointer", metadata !3, i64 0}

This is a bit strange IR, but probably still correct
(due to multiple inheritance we have multiple vtable pointers and their updates
get merged into a single vector instruction). 

decoder@, while I am looking for a proper fix, please try a workaround: 
Index: lib/Transforms/Instrumentation/ThreadSanitizer.cpp
===================================================================
--- lib/Transforms/Instrumentation/ThreadSanitizer.cpp  (revision 196066)
+++ lib/Transforms/Instrumentation/ThreadSanitizer.cpp  (working copy)
@@ -399,7 +399,9 @@
   int Idx = getMemoryAccessFuncIndex(Addr);
   if (Idx < 0)
     return false;
-  if (IsWrite && isVtableAccess(I)) {
+  if (IsWrite && isVtableAccess(I) &&
+      Addr->getType()->getPointerElementType()->canLosslesslyBitCastTo(
+          IRB.getInt8PtrTy())) {
     DEBUG(dbgs() << "  VPTR : " << *I << "\n");
     Value *StoredValue = cast<StoreInst>(I)->getValueOperand();
     // StoredValue does not necessary have a pointer type.

Original comment by konstant...@gmail.com on 2 Dec 2013 at 7:15

[GoogleCodeExporter]

This appears to be a more appropriate fix: 

Index: lib/Transforms/Instrumentation/ThreadSanitizer.cpp
===================================================================
--- lib/Transforms/Instrumentation/ThreadSanitizer.cpp  (revision 196066)
+++ lib/Transforms/Instrumentation/ThreadSanitizer.cpp  (working copy)
@@ -402,13 +402,15 @@
   if (IsWrite && isVtableAccess(I)) {
     DEBUG(dbgs() << "  VPTR : " << *I << "\n");
     Value *StoredValue = cast<StoreInst>(I)->getValueOperand();
-    // StoredValue does not necessary have a pointer type.
-    if (isa<IntegerType>(StoredValue->getType()))
-      StoredValue = IRB.CreateIntToPtr(StoredValue, IRB.getInt8PtrTy());
+    // StoredValue may be a vector type if we are storing several vptrs at 
once.
+    // In this case, just take the first element of the vector.
+    if (isa<VectorType>(StoredValue->getType()))
+      StoredValue = IRB.CreateExtractElement(
+          StoredValue, ConstantInt::get(IRB.getInt32Ty(), 0));
     // Call TsanVptrUpdate.
     IRB.CreateCall2(TsanVptrUpdate,
                     IRB.CreatePointerCast(Addr, IRB.getInt8PtrTy()),
-                    IRB.CreatePointerCast(StoredValue, IRB.getInt8PtrTy()));
+                    IRB.CreateBitCast(StoredValue, IRB.getInt8PtrTy()));
     NumInstrumentedVtableWrites++;
     return true;
   }

Original comment by konstant...@gmail.com on 2 Dec 2013 at 7:42

[GoogleCodeExporter]

Should be fixed by 
http://llvm.org/viewvc/llvm-project?view=revision&revision=196079

decoder@, please try. 

Original comment by konstant...@gmail.com on 2 Dec 2013 at 8:12

• Changed state: Fixed

[GoogleCodeExporter]

Confirmed fixed, thanks!

Original comment by decoder...@googlemail.com on 2 Dec 2013 at 3:39