<div class="gmail_quote">On 13 February 2012 09:58, Kostya Serebryany <span dir="ltr"><<a href="mailto:kcc@google.com" target="_blank">kcc@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">


<br><br><div class="gmail_quote"><div><div>On Fri, Feb 10, 2012 at 10:56 PM, Nick Lewycky <span dir="ltr"><<a href="mailto:nicholas@mxc.ca" target="_blank">nicholas@mxc.ca</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">



Thanks Kostya! This looks nearly ready to me.<br>
<br>
+; CHECK-NEXT:   call void @__tsan_func_exit()<br>
+; CHECK-ret:<br>
<br>
Err, that last one should just be "CHECK: ret void".<div><br>
<br>
Index: lib/Transforms/<u></u>Instrumentation/<u></u>ThreadSanitizer.cpp<br>
==============================<u></u>==============================<u></u>=======<br>
--- lib/Transforms/<u></u>Instrumentation/<u></u>ThreadSanitizer.cpp  (revision 0)<br>
+++ lib/Transforms/<u></u>Instrumentation/<u></u>ThreadSanitizer.cpp  (revision 0)<br></div>
@@ -0,0 +1,172 @@<div><br>
+//===-- ThreadSanitizer.cpp - race detector ---------------------*- C++ -*-===//<br>
<br></div>
No emacs mode marker on .cpp files, only on .h files.<br>
<br>
+  for (int i = 0; i < kNumberOfAccessSizes; i++) {<br>
<br>
Elsewhere in the file you use "++i" form. I suggest using it here too.<br>
<br>
+    TsanWrite[i] = M.getOrInsertFunction(<u></u>WriteName, IRB.getVoidTy(),<br>
+                                        IRB.getInt8PtrTy(), NULL);<br>
<br>
Indentation of the second line.<br>
<br>
+    Value *ReturnAddress = IRB.CreateCall(<br>
+        Intrinsic::getDeclaration(<u></u>CurrentModule, Intrinsic::returnaddress),<br>
+        IRB.getInt32(0));<br>
<br>
Please use F.getParent() instead of tracking CurrentModule.<br>
<br>
+bool ThreadSanitizer::<u></u>instrumentLoadOrStore(<u></u>Instruction *I) {<br>
+  IRBuilder<> IRB(I);<br>
+  bool IsWrite = isa<StoreInst>(*I);<br>
+  Value *Addr = IsWrite<br>
+      ? cast<StoreInst>(I)-><u></u>getPointerOperand()<br>
+      : cast<LoadInst>(I)-><u></u>getPointerOperand();<br>
+  Type *OrigPtrTy = Addr->getType();<br>
+  Type *OrigTy = cast<PointerType>(OrigPtrTy)-><u></u>getElementType();<br>
<br>
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.<br>
<br>
You may decide to bail on those, but please don't crash. :)<br><br></blockquote><div><br></div></div></div><div>I've fixed all except for the last bullet. </div><div>Could you please give an example where this may fail? </div>



<div>We have the exact same logic in AddressSanitizer and I have not seen any failures so far. </div></div></blockquote><div><br></div><div>My apologies, for some reason I thought we added support for:</div><div><br></div>


<div><div>define <4 x i8> @test(<4 x i8*> %vec_of_pointers) {</div><div>  %A = load <4 x i8*> %vec_of_pointers</div><div>  ret <4 x i8> %A</div><div>}</div></div><div><br></div><div>but we actually only added support for icmp and GEP of vector-of-pointer, not load and store of them. Carry on!</div>


<div><br></div><div>Nick</div><div><br></div></div>