On Tue, Nov 19, 2013 at 07:14:03PM +0400, Alexey Samsonov wrote:
> > +#if SANITIZER_LIBBACKTRACE
> > +namespace {
> > +
> > +struct SymbolizeCodeData {
> > + AddressInfo *frames;
> > + uptr n_frames;
> > + uptr max_frames;
>
> max_frames is not used anywhere.
I guess
if (cdata->n_frames == cdata->max_frames)
return 1;
near the end of SymbolizeCodePCInfoCallback should do it (returning
non-zero from the callback I think means no further callbacks will be
called).
> > + const char *module_name;
> > + uptr module_offset;
> > +};
> > +
> > +extern "C" {
>
> why do you need extern "C" here?
Not for GCC, but generally I think the C++ standard requires that extern "C"
function taking function pointer argument implies the function must be
extern "C". At least AFAIK in SunPro overload resolution treats
extern "C" and extern "C++" function pointers differently.
> > +class LibbacktraceSymbolizer {
> > + public:
> > + static LibbacktraceSymbolizer *get(LowLevelAllocator *alloc) {
> > + backtrace_state *state
> > + = backtrace_create_state("/proc/self/exe", 1, ErrorCallback, NULL);
>
> You may want to use ReadBinaryName() here (in case it was cached
> earlier, e.g. before
> we create a sandbox).
As the class is constructed during getSymbolizer(), it is surely performed
before any getSymbolizer()->PrepareForSandboxing(); can be called, so using
/proc/self/exe is IMHO both right and faster.
>
> Looks like Libbacktrace symbolizer is now wrapped in a straightforward
> interface. Could you please
> move it it sanitizer_symbolizer_libbacktrace.{h,cc}, and keep changes
> to sanitizer_symbolizer_posix_libcdep.cc
> minimal?
I will try.
Jakub
