The clang part looks good to me, but you will need some one else's review. <br><br><div class="gmail_quote">On Thu, Aug 16, 2012 at 12:05 PM, 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">+llvm-commits<div><br></div><div>Reid, </div><div><br></div><div>The LLVM and compiler-rt patches look good. </div><div>
Please fix the remaining small issues (see my code review comments) and commit. </div><div>Hold on with the output tests for a bit since Alexey Samsonov is migrating them to cmake (please coordinate with him and commit as a separate patch). </div>
<div><br></div><div>The stress test should contain X files, Y linker initialized globals and Z dynamically initialized globals. </div><div>Such test only makes sense where all 3 numbers are large. </div><div>I guess you can commit a single .sh script into compiler-rt/lib/asan/scripts</div>
<div><br></div><div>Thanks! </div><div><br></div><div>--kcc <br><br><div class="gmail_quote"><div><div class="h5">On Wed, Aug 15, 2012 at 9:04 PM, Reid Watson <span dir="ltr"><<a href="mailto:reidw@google.com" target="_blank">reidw@google.com</a>></span> wrote:<br>
</div></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5">Hello,<br>
<br>
This patch extends AddressSanitizer to include checking for the<br>
initialization order fiascos in C++.<br>
Specifically, this will cause AddressSanitizer to crash when it<br>
encounters an example of access to a global object or its members<br>
before it's (non-trivial) constructor runs.<br>
This is undefined behavior by sections 12.7.1 and 3.8.1 of the C++11 standard.<br>
Real world testing has shown initialization order checking has been<br>
finding plenty of examples of undefined behavior with no currently<br>
known false positives.<br>
<br>
This patch includes a few components:<br>
1. Clang patch<br>
- Small patch to add metadata identifying dynamically initialized<br>
globals for AddressSanitizer to instrument.<br>
2. LLVM patch<br>
- Changes to the AddressSanitizer instrumentation pass to<br>
instrument initializers.<br>
- Tests<br>
3. Compiler-RT patch<br>
- Changes to AddressSanitizer runtime library to display info<br>
about an initialization order fiasco crash.<br>
- Output test, and a small patch to output_tests.sh to support<br>
multiple files in compiling a test (necessary for testing initializers<br>
in separate TUs).<br>
4. Stress test<br>
- I'm not sure if there's a good home for this, but I've attached<br>
a small shell/C++ program to benchmark this.<br>
- This patch adds a ~0.1 second overhead to initialization of a<br>
program which contains 40,000 (!) dynamically initialized int size<br>
globals and 40,000 statically initialized globals.<br>
- Performance of initialization order checking is independent of<br>
the number of statically initialized globals<br>
<br>
I'd appreciate review of this patch. I've also updated the issue on Rietveld:<br>
<br>
LLVM: <a href="http://codereview.appspot.com/6432065/" target="_blank">http://codereview.appspot.com/6432065/</a><br>
Compiler-RT: <a href="http://codereview.appspot.com/6419070/" target="_blank">http://codereview.appspot.com/6419070/</a><br>
Clang: <a href="http://codereview.appspot.com/6440051/" target="_blank">http://codereview.appspot.com/6440051/</a><br>
<br>
All the best,<br>
Reid<br>
<br></div></div>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br></blockquote></div><br></div>
</blockquote></div><br>