[PATCH] Use libbacktrace as libsanitizer's symbolizer

Jakub Jelinek jakub@redhat.com
Tue Nov 19 16:27:00 GMT 2013


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



More information about the Gcc-patches mailing list