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

Fri Sep 23 05:45:16 PDT 2022

DavidSpickett accepted this revision.
DavidSpickett added a comment.
This revision is now accepted and ready to land.

LGTM

================
Comment at: lldb/test/Shell/Driver/CommandOnCrashMultiThreaded.test:1
+# XFAIL: system-windows
+# REQUIRES: native && (target-x86 || target-x86_64)
----------------
mstorsjo wrote:
> 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.)
Yeah I didn't notice the pthread flag, that could be it. Makes sense to me.

================
Comment at: lldb/test/Shell/Driver/Inputs/CommandOnCrashMultiThreaded.cpp:5
+  asm volatile(
+    "int3\n\t"
+  );
----------------
mstorsjo wrote:
> 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.
Good point, definitely would have tripped over that. I'll double check arm64 as well.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134037