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

[Richard Smith]

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 <kcc at google.com> 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 <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
>>>
>>>
>>
>
> _______________________________________________
> 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/5f5d1cad/attachment.html>