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

samsonov at google.com samsonov at google.com
Thu May 31 06:22:12 PDT 2012


Reviewers: llvm-commits_cs.uiuc.edu, dvyukov,


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

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

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

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?



Please review this at http://codereview.appspot.com/6258065/

Affected files:
   M     sanitizer_common/mini_libc.h
   A     sanitizer_common/symbolizer.cc
   A     sanitizer_common/symbolizer.h





More information about the llvm-commits mailing list