[PATCH] D97726: [sanitizers] [windows] Use InternalMmapVector instead of silencing -Wframe-larger-than

[Reid Kleckner via Phabricator via llvm-commits]

rnk added inline comments.


================
Comment at: compiler-rt/lib/sanitizer_common/sanitizer_unwind_win.cpp:50
+  InternalMmapVector<CONTEXT> ctx_buf(1);
+  CONTEXT &ctx = ctx_buf[0];
+  ctx = *(CONTEXT *)context;
----------------
vitalybuka wrote:
> This called unwind slow and usually called infrequently, but user can 
> set fast_unwind_on_malloc=false and make it frequent.
> So it's not nice that we make it slower.
> 
> You can try:
> 
> ```
> NOINLINE
> static void UnwindSlowInternal(uptr pc, void *context, u32 max_depth, STACKFRAME64*) {
> }
> 
> void BufferedStackTrace::UnwindSlow(uptr pc, void *context, u32 max_depth) {
>   CONTEXT ctx = *(CONTEXT *)context;
>   STACKFRAME64 stack_frame;
>   UnwindSlowInternal(...., &stack_frame)
> }
> ```
> 
> 
> And also I don't know if all these are worth of hassle. Maybe we can just raise limit for -Wframe-larger-than=
Windows doesn't use the fast unwind path at all, so actually we come here every time we capture a stack trace for a heap event. See:
https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/sanitizer_common/sanitizer_stacktrace.h#L26
```
#elif SANITIZER_WINDOWS
# define SANITIZER_CAN_FAST_UNWIND 0
```

There are probably much faster ways to walk the stack, if someone was really motivated to maintain Windows ASan.

For now we should probably just revert this change. All the other changes seem like they should happen at most once: either during startup or during error reporting, so I'd keep them.



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

https://reviews.llvm.org/D97726