[Lldb-commits] [PATCH] D73303: [lldb/Target] Add Abort StackFrame Recognizer

[Frederic Riss via Phabricator via lldb-commits]

friss added inline comments.


================
Comment at: lldb/source/Target/AbortRecognizer.cpp:81
+    ThreadSP thread_sp, FileSpec module_spec, ConstString function_name) {
+  const uint32_t frames_to_fetch = 10;
+  StackFrameSP prev_frame_sp = nullptr;
----------------
mib wrote:
> JDevlieghere wrote:
> > Magic value? Why 10?
> Pretty much. In the beginning I was unwinding the entire stack. But if there is too much frames (i.e. recursion) this would take too much time. We agreed with @friss and @jingham to unwind up to 10 frames at most.
> 
> Can you think of a better way to do it ?
I don't think that's exactly what I said. My opinion was that asserts will have a pretty deterministic layout, and that we should know exactly what to unwind to determine whether we have an assert or not.

In your test, you actually check that the frame with the assert is frame 4. If it is anything other then 4 in any configuration, then the test will fail. You might as well just put 4 in the code here rather than a magical 10.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73303