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