[llvm-commits] ThreadSanitizer, first patch. Please review.

[Kostya Serebryany]

On Fri, Feb 10, 2012 at 10:56 PM, Nick Lewycky <nicholas at mxc.ca> wrote:

> Thanks Kostya! This looks nearly ready to me.
>
> +; CHECK-NEXT:   call void @__tsan_func_exit()
> +; CHECK-ret:
>
> Err, that last one should just be "CHECK: ret void".
>
>
> Index: lib/Transforms/**Instrumentation/**ThreadSanitizer.cpp
> ==============================**==============================**=======
> --- lib/Transforms/**Instrumentation/**ThreadSanitizer.cpp  (revision 0)
> +++ lib/Transforms/**Instrumentation/**ThreadSanitizer.cpp  (revision 0)
> @@ -0,0 +1,172 @@
>
> +//===-- ThreadSanitizer.cpp - race detector ---------------------*- C++
> -*-===//
>
> No emacs mode marker on .cpp files, only on .h files.
>
> +  for (int i = 0; i < kNumberOfAccessSizes; i++) {
>
> Elsewhere in the file you use "++i" form. I suggest using it here too.
>
> +    TsanWrite[i] = M.getOrInsertFunction(**WriteName, IRB.getVoidTy(),
> +                                        IRB.getInt8PtrTy(), NULL);
>
> Indentation of the second line.
>
> +    Value *ReturnAddress = IRB.CreateCall(
> +        Intrinsic::getDeclaration(**CurrentModule,
> Intrinsic::returnaddress),
> +        IRB.getInt32(0));
>
> Please use F.getParent() instead of tracking CurrentModule.
>
> +bool ThreadSanitizer::**instrumentLoadOrStore(**Instruction *I) {
> +  IRBuilder<> IRB(I);
> +  bool IsWrite = isa<StoreInst>(*I);
> +  Value *Addr = IsWrite
> +      ? cast<StoreInst>(I)->**getPointerOperand()
> +      : cast<LoadInst>(I)->**getPointerOperand();
> +  Type *OrigPtrTy = Addr->getType();
> +  Type *OrigTy = cast<PointerType>(OrigPtrTy)->**getElementType();
>
> The element type of a pointer operand to a load or store instruction may
> also be a VectorType (whose elements are in turn pointers). The semantics
> of this are a scatter/gather operation.
>
> You may decide to bail on those, but please don't crash. :)
>
>
I've fixed all except for the last bullet.
Could you please give an example where this may fail?
We have the exact same logic in AddressSanitizer and I have not seen any
failures so far.

--kcc
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120213/ff1f254f/attachment.html>