[PATCH] D103562: [NFC][compiler-rt][hwasan] Refactor hwasan functions

[Florian Mayer via Phabricator via cfe-commits]

fmayer added a comment.

In D103562#3702962 <https://reviews.llvm.org/D103562#3702962>, @smd wrote:

> Hi folks,
>
> I've been working on support hwasan for risc-v and I believe I've found an issue with the existing lit tests this commit causes.
> Tests stack-{oob,uar,uas}.c check for correct backtrace being printed. From the code and comments the idea is to not to print any hwasan related frames(see the code and comments below).
>
>   void HandleTagMismatch(AccessInfo ai, uptr pc, uptr frame, void *uc,                                
>                          uptr *registers_frame) {                                                     
>     InternalMmapVector<BufferedStackTrace> stack_buffer(1);                                           
>     BufferedStackTrace *stack = stack_buffer.data();                                                  
>     stack->Reset();                                                                                   
>     stack->Unwind(pc, frame, uc, common_flags()->fast_unwind_on_fatal);                               
>                                                                                                       
>     // The second stack frame contains the failure __hwasan_check function, as                        
>     // we have a stack frame for the registers saved in __hwasan_tag_mismatch that                    
>     // we wish to ignore. This (currently) only occurs on AArch64, as x64                      
>     // implementations use SIGTRAP to implement the failure, and thus do not go                       
>     // through the stack saver.                                                                       
>     if (registers_frame && stack->trace && stack->size > 0) {                                         
>       stack->trace++;                                                                              
>       stack->size--;                                                                               
>     } 
>
> Before this commit the return address and frame pointer to were taken directly from **hwasan_tag_mismatch4**, while after the commit those addresses are calculated after another function being called from hwasan_tag_mismatch4 (the **HwasanTagMismatch** one).
> So, if I understand it correctly, now it looks like 2 stack frames must be ignored(for **hwasan_tag_mismatch4** and **HwasanTagMismatch**) to get a proper backtrace.
> What do you think?
>
> Thanks

Yes, but I am not sure we can *rely* on that being the case as is. LTO could conceivably inline this – in which case it would be one, right?



================
Comment at: compiler-rt/lib/hwasan/hwasan.cpp:205
+  if (registers_frame && stack->trace && stack->size > 0) {
+    stack->trace++;
+    stack->size--;
----------------
vitalybuka wrote:
> maybe we should pop everything up to "pc" to avoid issues with nested calls?
> 
> For most users hwasan frames are not very useful. However if you work on sanitizer, some frames can be a useful info. So I don't mind we just relax test cases to accommodate this nesting.
> 
> cc @smd 
This is probably for another patch though, right? This is already like this on the LHS.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103562/new/

https://reviews.llvm.org/D103562