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

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

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

Thanks,

--kcc
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20111116/ad18f82f/attachment.html>

On Wed, Nov 16, 2011 at 6:24 PM, Kostya Serebryany <kcc at google.com>
wrote:
> - test-specific: can I rely on gtest being installed? (fresh version is
> required).
>
Just to quickly clarify this point, if compiler-rt is moving its build
system to have it build in a sub-tree of LLVM, it should definitely be able
to use GoogleTest. LLVM has unit tests using it, as does Clang. I would
expect it to be straight forward to extend it to compiler-rt...
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20111116/3c72556d/attachment.html>

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

 - Daniel
> Thanks,
> --kcc
>
>

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>