On Thu, Nov 22, 2012 at 12:38 PM, Evgeniy Stepanov <eugeni.stepa...@gmail.com> wrote: > Yes, it has 3 asan-rtl frames on top. I'm not sure why this does not > happen on ppc.
Different inlining inside run-time. I still prefer a simple PopFrames to anything else, I am sure we can make it reliable. the asan run-time should always be build with optimizations on and we can force or (better) forbid inlining for any function. --kcc > > #0 0x40122cdb in __asan::GetStackTrace(__sanitizer::StackTrace*, > unsigned long, unsigned long, unsigned long) > #1 0x40125167 in __asan_report_error > #2 0x40125af3 in __asan_report_load1 > > > On Thu, Nov 22, 2012 at 12:10 AM, Evgeniy Stepanov > <eugeni.stepa...@gmail.com> wrote: >> I'm looking into the empty stack issue, at this point it looks like a weird >> linker bug. But its completely orthogonal to this discussion. >> >> I recall that the stack trace of the offending memory access has in fact >> three extra frames on top. I'll verify tomorrow. If so, FP/SP matching >> solution is preferable. >> >> On Nov 21, 2012 9:08 PM, "Peter Bergner" <berg...@vnet.ibm.com> wrote: >>> >>> 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 >>> >>> >>> >>
