llvm dev - Nov 2011 - [LLVMdev] AddressSanitizer run-time in tools/clang/runtime/compiler-rt

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.
>
> > - 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?

>
> >    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
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20111128/11592242/attachment.html>

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

done, r145463

Will continue working on the integration with the build system, etc.
Expect more commits :)

--kcc


On Tue, Nov 29, 2011 at 12:32 PM, Daniel Dunbar <daniel at zuster.org>
wrote:
> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20111129/8e56d419/attachment.html>