[Lldb-commits] [PATCH] D134037: [lldb] Fix CommandInterpreter::DidProcessStopAbnormally() with multiple threads

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added inline comments.


================
Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:2443
     StopInfoSP stop_info = thread_sp->GetStopInfo();
     if (!stop_info)
+      continue;
----------------
DavidSpickett wrote:
> Please add a comment explaining why we walk all the threads.
> 
> (though in hindsight it does seem the obvious thing to do)
Sure, can do


================
Comment at: lldb/test/Shell/Driver/CommandOnCrashMultiThreaded.test:1
+# XFAIL: system-windows
+# REQUIRES: native && (target-x86 || target-x86_64)
----------------
DavidSpickett wrote:
> Isn't this fixing an issue that you saw on Windows as well?
Yes, that was the place where I observed the issue (as Windows apparently, under some circumstances, can have a couple of extra threads running that haven't been spawned by the code of the process itself), but it's easily reproducible on any platform as long as you make sure there's more than one thread running.

I copied the basis for the test from `Shell/Register/x86-multithreaded-read.test` - most of the tests there under `Shell/Register` seem to have such an XFAIL, not sure exactly why. (In this particular test, at least the `-pthread` linker flag might be a trivial platform specific difference that might break it, which would need to be abstracted somewhere.)


================
Comment at: lldb/test/Shell/Driver/Inputs/CommandOnCrashMultiThreaded.cpp:5
+  asm volatile(
+    "int3\n\t"
+  );
----------------
DavidSpickett wrote:
> This is an x86 breakpoint, right? Shame there is no `__builtin_break` (well, msvc has one apparently but no help for this).
> 
> I might add the AArch64 and Arm equivalent if I have some spare time.
Yes, afaik this is a regular x86 programmatic breakpoint.

About the portability of breakpoints on ARM btw; the exact `udf #xx` code for a programmatic breakpoint is OS dependent, and does differ between Linux and Windows at least - see e.g. https://github.com/mingw-w64/mingw-w64/commit/38c9f0318c6509110706afdc52adc12d8cc34ead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134037