Kostya Serebryany
2011-Nov-17 02:24 UTC
[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>
Chandler Carruth
2011-Nov-17 02:43 UTC
[LLVMdev] AddressSanitizer run-time in tools/clang/runtime/compiler-rt
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>
Daniel Dunbar
2011-Nov-24 15:27 UTC
[LLVMdev] AddressSanitizer run-time in tools/clang/runtime/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). 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 > >
Kostya Serebryany
2011-Nov-28 21:15 UTC
[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>
Seemingly Similar Threads
- [LLVMdev] AddressSanitizer run-time in tools/clang/runtime/compiler-rt
- [LLVMdev] AddressSanitizer run-time in tools/clang/runtime/compiler-rt
- [LLVMdev] AddressSanitizer run-time in tools/clang/runtime/compiler-rt
- [LLVMdev] LLVMdev Digest, Vol 89, Issue 60
- [LLVMdev] LLVM-based address sanity checker