Created attachment 33101 [details] heap-overflow-1 output I see some regressions (e.g. heap-overflow-1) in Asan testsuite on ARM Linux. It seems that libsanitizer couldn't unwind stack from __interceptor_malloc() function by using StackTrace::FastUnwindStack. Same test works fine with Clang on ARM. After some investigation, I noticed, that StackTrace::FastUnwindStack reads fp and lr from wrong stack slot. I suppose this related with different code producing by GCC and Clang in __interceptor_malloc() prologue. This is a small reprocase: extern int func2(int i); int func(int i) { return func2(i); } Clang builds such code in func prologue: .save {r11, lr} push {r11, lr} .setfp r11, sp mov r11, sp GCC builds another one: .save {fp, lr} str fp, [sp, #-8]! str lr, [sp, #4] .setfp fp, sp, #4 add fp, sp, #4 As we can see, the LLVM code results in the frame pointer pointing to just after the pushed R11(fp) register on the stack, while the GCC code results in the frame pointer pointing to just after the pushed LR register on the stack (related discussion in LLVM is http://comments.gmane.org/gmane.comp.compilers.llvm.devel/69514). This is not a bug in libsanitizer but rather an ABI mismatch between GCC and LLVM. Any ideas how to proceed with this? -Maxim
The ABI does not document a model for stack chains, so any use of a frame pointer is, by definition, purely private to that function.
So looks like fast unwinding in libsanitizer is not portable to GCC for ARM Linux target because of incompatible frame pointer value. But how is libsanitizer going to identify functions/object files compiled by GCC? Perhaps we should just disable fast unwind on ARM? -Maxim
Yes, FP on ARM is non-standard and differs in GCC and Clang implementations. Disabling fast unwind is not really an option, as you are looking at 10x, maybe 100x slowdown (depending of the app, of course). It should be possible to detect fp layout on the frame basis - there is a slot (don't know which one off the top of my head) that is FP in one compiler and return address in the other. Comparing its contents with the current stack limits (readily available in ASan) should do the trick. Of course, it would be awesome if we could synchronize (and document somewhere) FP stack layout between GCC and Clang - after all, there is no strong reason to do it one way or the other.
> It should be possible to detect fp layout on the frame basis - > there is a slot (don't know which one off the top of my head) > that is FP in one compiler and return address in the other. > Comparing its contents with the current stack limits > (readily available in ASan) should do the trick. Unless program uses nested functions (and so LR may point to trampoline which is located on stack). > Of course, it would be awesome if we could synchronize > (and document somewhere) FP stack layout between GCC and Clang Let me ping LLVM list for this.
> > Of course, it would be awesome if we could synchronize > > (and document somewhere) FP stack layout between GCC and Clang Isn't this what ARM should be doing? >> Perhaps we should just disable fast unwind on ARM? You will slowdown asan to the point where Valgrind will become preferable
(In reply to Kostya Serebryany from comment #5) > >> Perhaps we should just disable fast unwind on ARM? > You will slowdown asan to the point where Valgrind will become preferable Unless you choose to default-disable malloc contexts on platforms that don't really support them. Anyway, Evgeny's proposal sounds like a good WAR. We'll send something upstream.
(In reply to Evgeniy Stepanov from comment #3) > Yes, FP on ARM is non-standard and differs in GCC and Clang implementations. > Disabling fast unwind is not really an option, as you are looking at 10x, > maybe 100x slowdown (depending of the app, of course). > It should be possible to detect fp layout on the frame basis - there is a > slot (don't know which one off the top of my head) that is FP in one > compiler and return address in the other. Comparing its contents with the > current stack limits (readily available in ASan) should do the trick. > > Of course, it would be awesome if we could synchronize (and document > somewhere) FP stack layout between GCC and Clang - after all, there is no > strong reason to do it one way or the other. I think finding a fix in the run time will be better and probably more resilient across versions of GCC. In any case I think this is worthy of a work around in the sanitisers rather than in GCC itself. I don't know where the bugs for the sanitizer run time is tracked - so it maybe worth closing this with a link to the appropriate upstream bug report.
(In reply to Ramana Radhakrishnan from comment #7) > I think finding a fix in the run time will be better and probably more > resilient across versions of GCC. In any case I think this is worthy of a > work around in the sanitisers rather than in GCC itself. Ok, we are working on a libsanitizer workaround for this. > I don't know where the bugs for the sanitizer run time is tracked - so it > maybe worth closing this with a link to the appropriate upstream bug report. AFAIK we keep bugs open until they get fixed with next merge from LLVM.(In reply to Ramana Radhakrishnan from comment #7)
(In reply to Yury Gribov from comment #8) > (In reply to Ramana Radhakrishnan from comment #7) > > I think finding a fix in the run time will be better and probably more > > resilient across versions of GCC. In any case I think this is worthy of a > > work around in the sanitisers rather than in GCC itself. > > Ok, we are working on a libsanitizer workaround for this. > > > I don't know where the bugs for the sanitizer run time is tracked - so it > > maybe worth closing this with a link to the appropriate upstream bug report. > > AFAIK we keep bugs open until they get fixed with next merge from LLVM.(In > reply to Ramana Radhakrishnan from comment #7) That's ok - I didn't know how you handled issues for the sanitizers when you were working across the 2 projects. Anyway - confirmed.
This should be fixed since r229111 (and 229115). Close?
Yes, fixed.
(In reply to Maxim Ostapenko from comment #11) > Yes, fixed. Fixed for GCC 6.0 I suspect.