[LLVMdev] AddressSanitizer run-time in tools/clang/runtime/compiler-rt

[Daniel Dunbar]

On Mon, Nov 28, 2011 at 1:15 PM, Kostya Serebryany <kcc at google.com> wrote:
>
>
> On Thu, Nov 24, 2011 at 7:27 AM, Daniel Dunbar <daniel at zuster.org> wrote:
>>
>> Quick answers, I'm on txgiving break this week and not doing any real
>> work, but I will be doing more compiler-rt work when I get back
>> (initially focused at getting profile libs to come from compiler-rt on
>> Linux et al).
>>
>> On Wed, Nov 16, 2011 at 9:24 PM, Kostya Serebryany <kcc at google.com> wrote:
>> > Hi Daniel,
>> > Chris suggested to talk to you about committing the AddressSanitizer
>> > (asan)
>> > run-time into the llvm tree (llvm-project/compiler-rt).
>> > Questions:
>> > - What is the preferred name for the directory? (asan? libasan?
>> > address_sanitizer? AdressSanitizer?)
>>
>> I don't care. lib/asan seems perfectly reasonable to me, with a README
>> explaining what the module is for.
>
>
> So, it will be a sibling of lib/profile. Makes sense.
>
>>
>>
>> > - Should the asan run-time use cmake, or just make, or what? The build
>> > is a
>> > bit tricky, especially for tests. We currently use make.
>>
>> The only build system I care about for compiler-rt is the one in make.
>> It's quite a crazy set up, although there is a reason for it to be as
>> complicated as it is. I can help (and/or) do the asan integration into
>> the compiler-rt make build.
>
>
> Yea, I may need your help, probably after the files are committed.

No problem!

>>
>>
>> > - How would you suggest to do the code review?
>> >    The code of the run-time is ~5
>> >
>> > KLOC. http://code.google.com/p/address-sanitizer/source/browse/#svn%2Ftrunk%2Fasan
>> >    The Apple-specific part may make some Apple experts cry (or maybe
>> > not).
>> >    The code uses google's coding style, which is similar, but not
>> > equivalent
>> > to the LLVM's one. We check it using cpplint before commits.
>>
>> I personally would like to see it be in LLVM style, but the rest of
>> compiler-rt isn't really that way, so I don't think this is a blocker.
>>
>> Unless anyone objects, I think we should just bring the code in as is
>> so that ASAN works, and if people want to do more thorough review that
>> can happen post commit.
>
>
> Do I just start committing or someone still wants to make a pre-review?

No one has objected, so I vote to just commit it into lib/asan and
work from there.

 - Daniel

>>
>>
>> >    LLVM license is used.
>> >    The tests are ~2.5 KLOC; most of the tests require the clang compiler
>> > with -faddress-sanitizer switch.
>> >    If possible, I'd prefer not to review the patch in a usual way (too
>> > big),
>> > but instead have the comments to the files
>> >
>> > in http://code.google.com/p/address-sanitizer/source/browse/#svn%2Ftrunk%2Fasan
>> >    Or, alternatively, commit everything as is and then iterate over
>> > individual files.
>> > - The run-time library uses a couple of third_party files (BSD license)
>> > to
>> > parse process mapping.
>> >   W/o these files asan will work, but will not produce symbolizable
>> > traces.
>> >   Can we include these files as as in a separate directory? (They may be
>> > useful for other tools as well)
>>
>> Let's try to include the bits with extra licenses in subdirectories of
>> lib/asan. If someone wants to reuse them later we can find a different
>> way to factor things, I don't see a reason to worry about it now.
>
>
> Ok.
>
> On a related note: lib/asan might benefit from reusing some of the lldb code
> (symbolizer).
>
>>
>>
>> >
>> >   http://code.google.com/p/address-sanitizer/source/browse/trunk/asan/sysinfo.cc
>> > - Yet another piece of third-party code (mit license) is used for
>> > Apple-specific work (function overriding). Same question as above apply.
>> >
>> >
>> >  http://code.google.com/p/address-sanitizer/source/browse/trunk/asan/mach_override.c
>> > - test-specific: can I rely on gtest being installed? (fresh version is
>> > required).
>>
>> Compiler-rt doesn't currently have a very good test set up. You'll
>> probably have to find a way to shoehorn this in for your own testing
>> initially, but we can try and work out a way to integrate it more
>> properly...
>
>
> ok.
>
>
> Thanks!
>
> --kcc