[llvm-commits] [cfe-commits] [PATCH] AddressSanitizer Initialization Order Checking

Kostya Serebryany kcc at google.com
Thu Aug 16 01:06:21 PDT 2012


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 <kcc at google.com> 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 <reidw at google.com> 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
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120816/bb88ce46/attachment.html>


More information about the llvm-commits mailing list