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

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…

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

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

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

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.

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

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

> 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

done, r145463

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

–kcc