On Wed, 2012-11-21 at 20:22 +0400, Konstantin Serebryany wrote:
> On Wed, Nov 21, 2012 at 8:16 PM, Peter Bergner <berg...@vnet.ibm.com> wrote:
> > On Wed, 2012-11-21 at 13:46 +0400, Evgeniy Stepanov wrote:
> >> Matching FP or SP also sounds good, and perhaps more reliable than
> >> just popping 2 frames from the top of the stack.
> >
> > Agreed. Can you try my second patch that searches for the frame
> > address we want our backtrace to start with and see if that works
> > for ARM? The nice thing about that patch is that we won't have
> > to play any games with forcing or disabling inlining of any of
> > the ASAN functions which me might have to do if we always pop
> > 2 frames off the stack. It would also be tolerant of adding
> > any number of new function calls in between the current two
> > ASAN function at the top of the backtrace.
> >
> > http://gcc.gnu.org/ml/gcc-patches/2012-11/msg01711.html
> >
> > Bah, ignore that first diff of the LAST_UPDATED file. :(
>
> I'd actually prefer to keep the current code that pops two frames
> (if it works for you)
Well it does work for me, since I wrote it. :) That being said, the
change where I always pop two frames off of the backtrace was more of
a proof of concept that if I can remove those ASAN functions from the
backtrace, do we pass the test case in the testsuite. It did, but I
have to admit that code is extremely fragile. It is dependent not
only on the inlining heuristics of one compiler, but of two compilers!
Not to mention people building debugable compilers with -O0 -fno-inline,
etc. etc. We'd also have to make sure no one in the future adds any
ASAN function calls in between the report function and the GetBackTrace
calls. It just seems like there are so many things that could go wrong,
that something is bound to.
> Evgeniy seems to know how to fix the ARM case.
His fix was to do:
void StackTrace::PopStackFrames(uptr count) {
- CHECK(size > count);
+ CHECK(size >= count);
size -= count;
for (uptr i = 0; i < size; i++) {
trace[i] = trace[i + count];
Basically, that is allowing for us to pop off all of the frames from
the backtrace leaving an empty backtrace. That can't be helpful in
tracking down the address violation, can it? With my patch above,
either we find the frame we want to start our backtrace with, or
it returns the entire backtrace, ASAN functions and all. Isn't that
better from a diagnostic point of view?
That being said, I'd still like to hear from Evgeniy whether my
patch above helps ARM or not.
Peter
