[LLVMdev] LLVM-based address sanity checker

Mon Aug 1 05:49:57 PDT 2011

My first patch: http://codereview.appspot.com/4842041/ (the diff file is in
attachment).
This patch adds the instrumentation pass only (no clang driver changes, no
run-time, no tests).
The instrumentation pass is shorter than the original one (does not handle
globals, stack and black list).
Please comment.

The next step would be to add instrumentation-only tests (something like an
input llvm file and an expected output llvm file checked with FileCheck,
right?).
Please suggest where to add such tests and how to run them.

Thanks,

--kcc

On Mon, Aug 1, 2011 at 12:58 PM, Kostya Serebryany <kcc at google.com> wrote:

>
>
> On Mon, Aug 1, 2011 at 12:01 PM, Chandler Carruth <chandlerc at google.com>wrote:
>
>> Any updates on this?
>>
>> In particular, I'd like to see concrete patches proposed for review and
>> inclusion into LLVM. I think having actual patches on the table and under
>> review will help a great deal. Kostya, let me know if I can help prepare
>> them.
>>
>
> Ok, I'll send the first (small) patch shortly.
>
>
>>  A few general comments as well inline...
>>
>> On Tue, Jul 26, 2011 at 1:57 AM, Kostya Serebryany <kcc at google.com>wrote:
>>
>>> On Tue, Jul 26, 2011 at 10:20 AM, Chris Lattner <clattner at apple.com>wrote:
>>>
>>>>
>>>> On Jun 21, 2011, at 8:05 AM, Kostya Serebryany wrote:
>>>>
>>>> > Hi,
>>>> > What would be our next steps in getting ASan into the LLVM trunk?
>>>> > I'd like to do it in two steps, first for the LLVM part with minimal
>>>> tests and then for the run-time library and all tests.
>>>> > The current ASan's source repository will probably stay the primary
>>>> home for the run-time library and tests as we plan to use it in non-LLVM
>>>> environments.
>>>> >
>>>>
>>>> Hi Kostya,
>>>>
>>>> I haven't had a chance to look at your patch yet, I'm backed up on "big
>>>> patches".  Did you see my review of the safecode patch here?
>>>>
>>>> http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20110718/124515.html
>>>>
>>>> I expect to have similar concerns and suggestions for your patch,
>>>>
>>>
>>> Hi Chris,
>>>
>>> Thanks for the reply.
>>> Indeed, some of your comments to safecode patch apply here.
>>>
>>> Codingstyle: I'll try to cleanup as much as I can today.
>>> Do you have any lint-like tool that checks for llvm coding style (similar
>>> to cpplint <http://google-styleguide.googlecode.com/svn/trunk/cpplint/>
>>> )?
>>>
>>
>> I don't know of any effective ones. Kostya, maybe you can send the code by
>> myself and/or Nick to help walk through it for style issues?
>>
>>  Documentation: everything is in the wiki. The main pages are:
>>> http://code.google.com/p/address-sanitizer/wiki/AddressSanitizer
>>> http://code.google.com/p/address-sanitizer/wiki/AddressSanitizerAlgorithm
>>>
>>
>> It would be good to have these written up in HTML for easy inclusion in
>> the existing LLVM documentation tree. This tree is checked in along side the
>> code itself. However, contributing snapshots of the documentation from the
>> wiki page could likely occur in follow-up patches. Alternatively, maybe
>> Chris would be OK linking to these wiki pages from the primary LLVM
>> documentation.
>>
>>
>>> "who is going to maintain": my team at Google (in particular, myself and
>>> Alexander, in CC) are highly motivated to keep this working.
>>>  "Do you have particular clients": the Chromium project is a very active<http://blog.chromium.org/2011/06/testing-chromium-addresssanitizer-fast.html>client.
>>
>>
>> For reference, myself and others on my team at Google are working actively
>> with Clang and LLVM and would be able to maintain, fix bugs, and update
>> this. We also have a *very* large base of users that would be interested in
>> the functionality.
>>
>>
>>> "The work can be decomposed into small and incremental pieces":
>>>    the LLVM part<http://code.google.com/p/address-sanitizer/source/browse/trunk/llvm/AddressSanitizer.cpp>is just 1000 LOC. If you still like it to be decomposed, we can do it in 3
>>> parts: a) general instrumentation, b) redzones for stack, c) redzones for
>>> globals.
>>>
>>
>> I think breaking up the LLVM part would be very helpful. Can you break out
>> the first part, and after a style cleanup mail the attached patch? Then the
>> review for that can overlap with preparing the other 2 parts.
>>
>>
>> How do you plan to test this? It's important that there are regression
>> tests in the LLVM suite that exercise the functionality independent of any
>> runtime so that other developers can catch regressions. Also, unittests in
>> the LLVM unittest tree would be nice as well.
>>
>
> Currently, I have tests<http://code.google.com/p/address-sanitizer/source/browse/#svn%2Ftrunk%2Fasan%2Ftests>that work only with the run-time library.
> I will definitely need tests that don't require run-time support.
>
>
>>
>>
>> Have you written a Clang patch to turn this functionality on and off?
>> Looking at the wiki documentation shows one thing that gives me pause:
>> you're using the -mllvm flag in Clang to directly pass options down to the
>> LLVM layer. Also, it indicates the asan functionality defaults to on.
>>
>
> Yes, I have clang patch that adds -fasan flag (off by default) to Linux and
> Mac (the run-time library doesn't work on other OSes yet).
> The patch<http://code.google.com/p/address-sanitizer/source/browse/trunk/llvm/clang.patch>changes tools/clang/lib/Driver/Tools.cpp and
> tools/clang/include/clang/Driver/Options.td
>
> The instrumentation pass has a bunch of flags which one can use via "-mllvm
> -asan-flag", but most of these flags should not be user-visible.
>
>
>
>>
>> Ideally all of this functionality would default to off, and be enabled via
>> '-fasan' or even better '-faddress-sanitizer' in Clang.
>>
>
> That's what I have now (-fasan). I slightly prefer -fasan
> over -faddress-sanitizer because the former is shorter.
>
>
>
>> That would match the behavior of '-fcatch-undefined-behavior',
>> '-fmudflap', etc. If you want to expose the more fine grained flags to users
>> that are mentioned on the wiki page, they could also have '-f...' Clang
>> flags, but it seems unlikely that those are important.
>>
>
> I will probably need a few more user-visible flags: [don't]instrument
> stack, [don't]instrument globals, [don't]instrument reads.
>
>
>
>>
>>
>> What's your expected plan for the runtime library? Is that something you
>> would be interested in contributing to the LLVM project proper? If so, I'd
>> be interested in where Chris thinks that should go... in the LLVM runtimes
>> tree? In a separate LLVM project?
>>
>
> I'd like to be able to keep the run-time completely separate from the rest
> of LLVM so that the same library could be used in other environments.
> This of course does not prevent us from having the library sources in the
> LLVM tree.
>
> --kcc
>
>
>
>>
>> Anyways, thanks for working on integrating this back into LLVM!
>> -Chandler
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20110801/fd1affea/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: issue4842041_2001.diff
Type: text/x-patch
Size: 27614 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20110801/fd1affea/attachment.bin>