LGTM, modulo some trivial things on rietvald (on the assumption that the metadata format has already been approved on the LLVM side).
On Thu, Aug 16, 2012 at 1:06 AM, Kostya Serebryany <[email protected]> wrote: > 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 > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
