[llvm-commits] Stub for LLVM-based sanitizer for ASan/TSan tools. (issue 6258065)

[Dmitry Vyukov]

On Thu, May 31, 2012 at 5:22 PM, <samsonov at google.com> wrote:

> Reviewers: llvm-commits_cs.uiuc.edu, dvyukov,
>
>
>
> http://codereview.appspot.com/**6258065/diff/1/lib/sanitizer_**
> common/symbolizer.h<http://codereview.appspot.com/6258065/diff/1/lib/sanitizer_common/symbolizer.h>
> File lib/sanitizer_common/**symbolizer.h (right):
>
> http://codereview.appspot.com/**6258065/diff/1/lib/sanitizer_**
> common/symbolizer.h#newcode1<http://codereview.appspot.com/6258065/diff/1/lib/sanitizer_common/symbolizer.h#newcode1>
> lib/sanitizer_common/**symbolizer.h:1: //===-- symbolizer.h
> ------------------------------**--------------*- C++ -*-===//
> On 2012/05/31 12:24:58, dvyukov wrote:
>
>> Let's prefix files in sanitizer_common with some prefixes.
>>
> Isn't directory name enough? What is your suggestion: sanitizer_? san_?


Object files sometimes clash.
sanitizer_


>
>
> http://codereview.appspot.com/**6258065/diff/1/lib/sanitizer_**
> common/symbolizer.h#newcode12<http://codereview.appspot.com/6258065/diff/1/lib/sanitizer_common/symbolizer.h#newcode12>
> lib/sanitizer_common/**symbolizer.h:12: #ifndef SYMBOLIZER_H
> On 2012/05/31 12:24:58, dvyukov wrote:
>
>> I would put the prefix here as well.
>>
> ditto.
>
>
> http://codereview.appspot.com/**6258065/diff/1/lib/sanitizer_**
> common/symbolizer.h#newcode19<http://codereview.appspot.com/6258065/diff/1/lib/sanitizer_common/symbolizer.h#newcode19>
> lib/sanitizer_common/**symbolizer.h:19: typedef unsigned long uptr;
> On 2012/05/31 12:24:58, dvyukov wrote:
>
>> Move this to another file.
>>
>
> Done.
>
>
> http://codereview.appspot.com/**6258065/diff/1/lib/sanitizer_**
> common/symbolizer.h#newcode23<http://codereview.appspot.com/6258065/diff/1/lib/sanitizer_common/symbolizer.h#newcode23>
> lib/sanitizer_common/**symbolizer.h:23: char *module;
> On 2012/05/31 12:24:58, dvyukov wrote:
>
>> What are these pointers? Who frees them? What is the lifetime?
>>
> On a second thought, I though that Symbolizer should better return a
> pointer to AddressInfo and transfer ownership.


What about strings?



> http://codereview.appspot.com/**6258065/diff/1/lib/sanitizer_**
> common/symbolizer.h#newcode31<http://codereview.appspot.com/6258065/diff/1/lib/sanitizer_common/symbolizer.h#newcode31>
> lib/sanitizer_common/**symbolizer.h:31: class Symbolizer {
> On 2012/05/31 12:24:58, dvyukov wrote:
>
>> Are you going to use pimpl?
>>
> Hm-m, I hoped to get away with OS-specific files with implementation
> details (as we do in ASan now).


The header will have a lot of unnecessary details.
Actually, what you need is a single function SymbolizeCode(...). You don't
even need Initialize(), it won't do anything useful and won't return a
useful return value.


>
>
> http://codereview.appspot.com/**6258065/diff/1/lib/sanitizer_**
> common/symbolizer.h#newcode36<http://codereview.appspot.com/6258065/diff/1/lib/sanitizer_common/symbolizer.h#newcode36>
> lib/sanitizer_common/**symbolizer.h:36: bool SymbolizeCode(uptr address,
> AddressInfo* info);
> On 2012/05/31 12:24:58, dvyukov wrote:
>
>> It must return a set of AddressInfo's with inlining info.
>>
> Can a FIXME be fine for now?
>

Let's include it into the interface, but leave implementation as FIXME.


Please review this at
http://codereview.appspot.com/**6258065/<http://codereview.appspot.com/6258065/>
>
> Affected files:
>  M     sanitizer_common/mini_libc.h
>  A     sanitizer_common/symbolizer.cc
>  A     sanitizer_common/symbolizer.h
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120531/ccfe632a/attachment.html>