[lldb] Enable "frame diagnose" on linux #123217
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
.. by changing the signal stop reason format 🤦 The reason this did not work is because the code in StopInfo::GetCrashingDereference was looking for the string "address=" to extract the address of the crash. Macos stop reason strings have the form EXC_BAD_ACCESS (code=1, address=0xdead) while on linux they look like: signal SIGSEGV: address not mapped to object (fault address: 0xdead) Extracting the address from a string sounds like a bad idea, but I suppose there's some value in using a consistent format across platforms, so this patch changes the signal format to use the equals sign as well. All of the diagnose tests pass except one, which appears to fail due to something similar llvm#115453 (disassembler reports unrelocated call targets). I've left the tests disabled on windows, as the stop reason reporting code works very differently there, and I suspect it won't work out of the box. If I'm wrong -- the XFAIL will let us know.
|
@llvm/pr-subscribers-lldb Author: Pavel Labath (labath) Changes.. by changing the signal stop reason format 🤦 The reason this did not work is because the code in while on linux they look like: Extracting the address from a string sounds like a bad idea, but I suppose there's some value in using a consistent format across platforms, so this patch changes the signal format to use the equals sign as well. All of the diagnose tests pass except one, which appears to fail due to something similar #115453 (disassembler reports unrelocated call targets). I've left the tests disabled on windows, as the stop reason reporting code works very differently there, and I suspect it won't work out of the box. If I'm wrong -- the XFAIL will let us know. Full diff: https://github.com/llvm/llvm-project/pull/123217.diff 15 Files Affected:
diff --git a/lldb/source/Target/UnixSignals.cpp b/lldb/source/Target/UnixSignals.cpp
index bee3a63818259e..da661003925c79 100644
--- a/lldb/source/Target/UnixSignals.cpp
+++ b/lldb/source/Target/UnixSignals.cpp
@@ -163,7 +163,7 @@ UnixSignals::GetSignalDescription(int32_t signo, std::optional<int32_t> code,
break;
case SignalCodePrintOption::Address:
if (addr)
- strm << " (fault address: 0x" << std::hex << *addr << ")";
+ strm << " (fault address=0x" << std::hex << *addr << ")";
break;
case SignalCodePrintOption::Bounds:
if (lower && upper && addr) {
@@ -172,9 +172,9 @@ UnixSignals::GetSignalDescription(int32_t signo, std::optional<int32_t> code,
else
strm << "upper bound violation ";
- strm << "(fault address: 0x" << std::hex << *addr;
- strm << ", lower bound: 0x" << std::hex << *lower;
- strm << ", upper bound: 0x" << std::hex << *upper;
+ strm << "(fault address=0x" << std::hex << *addr;
+ strm << ", lower bound=0x" << std::hex << *lower;
+ strm << ", upper bound=0x" << std::hex << *upper;
strm << ")";
} else
strm << sc.m_description.str();
diff --git a/lldb/test/API/commands/frame/diagnose/array/TestArray.py b/lldb/test/API/commands/frame/diagnose/array/TestArray.py
index 5de6f7b0aaa1c4..307e2cbca30228 100644
--- a/lldb/test/API/commands/frame/diagnose/array/TestArray.py
+++ b/lldb/test/API/commands/frame/diagnose/array/TestArray.py
@@ -10,7 +10,7 @@
class TestArray(TestBase):
- @skipUnlessDarwin
+ @expectedFailureAll(oslist=["windows"])
@skipIf(
archs=no_match(["x86_64"])
) # <rdar://problem/33842388> frame diagnose doesn't work for armv7 or arm64
diff --git a/lldb/test/API/commands/frame/diagnose/bad-reference/TestBadReference.py b/lldb/test/API/commands/frame/diagnose/bad-reference/TestBadReference.py
index 7a9498cab13766..8ded5e2ff55c21 100644
--- a/lldb/test/API/commands/frame/diagnose/bad-reference/TestBadReference.py
+++ b/lldb/test/API/commands/frame/diagnose/bad-reference/TestBadReference.py
@@ -10,7 +10,7 @@
class TestBadReference(TestBase):
- @skipUnlessDarwin
+ @expectedFailureAll(oslist=["windows"])
@skipIf(
archs=no_match(["x86_64"])
) # <rdar://problem/33842388> frame diagnose doesn't work for armv7 or arm64
diff --git a/lldb/test/API/commands/frame/diagnose/complicated-expression/TestComplicatedExpression.py b/lldb/test/API/commands/frame/diagnose/complicated-expression/TestComplicatedExpression.py
index eb1b556f0c408d..8ee254a28cb545 100644
--- a/lldb/test/API/commands/frame/diagnose/complicated-expression/TestComplicatedExpression.py
+++ b/lldb/test/API/commands/frame/diagnose/complicated-expression/TestComplicatedExpression.py
@@ -10,7 +10,7 @@
class TestDiagnoseDereferenceArgument(TestBase):
- @skipUnlessDarwin
+ @expectedFailureAll(oslist=["windows"])
@skipIf(
archs=no_match(["x86_64"])
) # <rdar://problem/33842388> frame diagnose doesn't work for armv7 or arm64
diff --git a/lldb/test/API/commands/frame/diagnose/dereference-argument/TestDiagnoseDereferenceArgument.py b/lldb/test/API/commands/frame/diagnose/dereference-argument/TestDiagnoseDereferenceArgument.py
index 4e867354343879..960dd99ff7f78b 100644
--- a/lldb/test/API/commands/frame/diagnose/dereference-argument/TestDiagnoseDereferenceArgument.py
+++ b/lldb/test/API/commands/frame/diagnose/dereference-argument/TestDiagnoseDereferenceArgument.py
@@ -10,7 +10,7 @@
class TestDiagnoseDereferenceArgument(TestBase):
- @skipUnlessDarwin
+ @expectedFailureAll(oslist=["windows"])
@skipIf(
archs=no_match(["x86_64"])
) # <rdar://problem/33842388> frame diagnose doesn't work for armv7 or arm64
diff --git a/lldb/test/API/commands/frame/diagnose/dereference-function-return/TestDiagnoseDereferenceFunctionReturn.py b/lldb/test/API/commands/frame/diagnose/dereference-function-return/TestDiagnoseDereferenceFunctionReturn.py
index 4d9b036f5102cb..d0f6ebefa334ad 100644
--- a/lldb/test/API/commands/frame/diagnose/dereference-function-return/TestDiagnoseDereferenceFunctionReturn.py
+++ b/lldb/test/API/commands/frame/diagnose/dereference-function-return/TestDiagnoseDereferenceFunctionReturn.py
@@ -10,7 +10,7 @@
class TestDiagnoseDereferenceFunctionReturn(TestBase):
- @skipUnlessDarwin
+ @expectedFailureAll(oslist=no_match(lldbplatformutil.getDarwinOSTriples()))
@skipIf(
archs=no_match(["x86_64"])
) # <rdar://problem/33842388> frame diagnose doesn't work for armv7 or arm64
diff --git a/lldb/test/API/commands/frame/diagnose/dereference-this/TestDiagnoseDereferenceThis.py b/lldb/test/API/commands/frame/diagnose/dereference-this/TestDiagnoseDereferenceThis.py
index fccba5ca116a9a..7a4d3fb2acb5c6 100644
--- a/lldb/test/API/commands/frame/diagnose/dereference-this/TestDiagnoseDereferenceThis.py
+++ b/lldb/test/API/commands/frame/diagnose/dereference-this/TestDiagnoseDereferenceThis.py
@@ -10,7 +10,7 @@
class TestDiagnoseDereferenceThis(TestBase):
- @skipUnlessDarwin
+ @expectedFailureAll(oslist=["windows"])
@skipIf(
archs=no_match(["x86_64"])
) # <rdar://problem/33842388> frame diagnose doesn't work for armv7 or arm64
diff --git a/lldb/test/API/commands/frame/diagnose/inheritance/TestDiagnoseInheritance.py b/lldb/test/API/commands/frame/diagnose/inheritance/TestDiagnoseInheritance.py
index 01245ff7608e1b..71a24002a06274 100644
--- a/lldb/test/API/commands/frame/diagnose/inheritance/TestDiagnoseInheritance.py
+++ b/lldb/test/API/commands/frame/diagnose/inheritance/TestDiagnoseInheritance.py
@@ -10,7 +10,7 @@
class TestDiagnoseInheritance(TestBase):
- @skipUnlessDarwin
+ @expectedFailureAll(oslist=["windows"])
@skipIf(
archs=no_match(["x86_64"])
) # <rdar://problem/33842388> frame diagnose doesn't work for armv7 or arm64
diff --git a/lldb/test/API/commands/frame/diagnose/local-variable/TestLocalVariable.py b/lldb/test/API/commands/frame/diagnose/local-variable/TestLocalVariable.py
index 9361d80367e128..2db054bec99190 100644
--- a/lldb/test/API/commands/frame/diagnose/local-variable/TestLocalVariable.py
+++ b/lldb/test/API/commands/frame/diagnose/local-variable/TestLocalVariable.py
@@ -10,7 +10,7 @@
class TestLocalVariable(TestBase):
- @skipUnlessDarwin
+ @expectedFailureAll(oslist=["windows"])
@skipIf(
archs=no_match(["x86_64"])
) # <rdar://problem/33842388> frame diagnose doesn't work for armv7 or arm64
diff --git a/lldb/test/API/commands/frame/diagnose/virtual-method-call/TestDiagnoseDereferenceVirtualMethodCall.py b/lldb/test/API/commands/frame/diagnose/virtual-method-call/TestDiagnoseDereferenceVirtualMethodCall.py
index 7a58203d8f2ed5..ef99b72f52afde 100644
--- a/lldb/test/API/commands/frame/diagnose/virtual-method-call/TestDiagnoseDereferenceVirtualMethodCall.py
+++ b/lldb/test/API/commands/frame/diagnose/virtual-method-call/TestDiagnoseDereferenceVirtualMethodCall.py
@@ -10,7 +10,7 @@
class TestDiagnoseVirtualMethodCall(TestBase):
- @skipUnlessDarwin
+ @expectedFailureAll(oslist=["windows"])
@skipIf(
archs=no_match(["x86_64"])
) # <rdar://problem/33842388> frame diagnose doesn't work for armv7 or arm64
diff --git a/lldb/test/API/linux/aarch64/mte_core_file/TestAArch64LinuxMTEMemoryTagCoreFile.py b/lldb/test/API/linux/aarch64/mte_core_file/TestAArch64LinuxMTEMemoryTagCoreFile.py
index 779050edb054a4..6309648819026a 100644
--- a/lldb/test/API/linux/aarch64/mte_core_file/TestAArch64LinuxMTEMemoryTagCoreFile.py
+++ b/lldb/test/API/linux/aarch64/mte_core_file/TestAArch64LinuxMTEMemoryTagCoreFile.py
@@ -215,7 +215,7 @@ def test_mte_tag_fault_reason(self):
self.expect(
"bt",
substrs=[
- "* thread #1, name = 'a.out.mte', stop reason = SIGSEGV: sync tag check fault (fault address: 0xffff82c74010)"
+ "* thread #1, name = 'a.out.mte', stop reason = SIGSEGV: sync tag check fault (fault address=0xffff82c74010)"
],
)
diff --git a/lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py b/lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py
index 668fca11903660..f27780358570bb 100644
--- a/lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py
+++ b/lldb/test/API/linux/aarch64/non_address_bit_memory_access/TestAArch64LinuxNonAddressBitMemoryAccess.py
@@ -202,7 +202,7 @@ def test_non_address_bit_memory_corefile(self):
"thread list",
substrs=[
"stopped",
- "stop reason = SIGSEGV: address not mapped to object (fault address: 0x0)",
+ "stop reason = SIGSEGV: address not mapped to object (fault address=0x0)",
],
)
diff --git a/lldb/test/Shell/Register/Core/x86-32-linux-multithread.test b/lldb/test/Shell/Register/Core/x86-32-linux-multithread.test
index eb0cf8708263cb..972e10844a5aad 100644
--- a/lldb/test/Shell/Register/Core/x86-32-linux-multithread.test
+++ b/lldb/test/Shell/Register/Core/x86-32-linux-multithread.test
@@ -1,7 +1,7 @@
# RUN: %lldb -b -s %s -c %p/Inputs/x86-32-linux-multithread.core | FileCheck %s
thread list
-# CHECK: * thread #1: tid = 330633, 0x080492d2, name = 'a.out', stop reason = SIGSEGV: address not mapped to object (fault address: 0x0)
+# CHECK: * thread #1: tid = 330633, 0x080492d2, name = 'a.out', stop reason = SIGSEGV: address not mapped to object (fault address=0x0)
# CHECK-NEXT: thread #2: tid = 330634, 0x080492dd, stop reason = signal 0
# CHECK-NEXT: thread #3: tid = 330635, 0x080492dd, stop reason = signal 0
# CHECK-NEXT: thread #4: tid = 330632, 0xf7f59549, stop reason = signal 0
diff --git a/lldb/test/Shell/Register/Core/x86-64-linux-multithread.test b/lldb/test/Shell/Register/Core/x86-64-linux-multithread.test
index a94a4de1c8080b..5bea84813b44fe 100644
--- a/lldb/test/Shell/Register/Core/x86-64-linux-multithread.test
+++ b/lldb/test/Shell/Register/Core/x86-64-linux-multithread.test
@@ -1,7 +1,7 @@
# RUN: %lldb -b -s %s -c %p/Inputs/x86-64-linux-multithread.core | FileCheck %s
thread list
-# CHECK: * thread #1: tid = 329384, 0x0000000000401262, name = 'a.out', stop reason = SIGSEGV: address not mapped to object (fault address: 0x0)
+# CHECK: * thread #1: tid = 329384, 0x0000000000401262, name = 'a.out', stop reason = SIGSEGV: address not mapped to object (fault address=0x0)
# CHECK-NEXT: thread #2: tid = 329385, 0x000000000040126d, stop reason = signal 0
# CHECK-NEXT: thread #3: tid = 329386, 0x000000000040126d, stop reason = signal 0
# CHECK-NEXT: thread #4: tid = 329383, 0x00007fcf5582f762, stop reason = signal 0
diff --git a/lldb/unittests/Signals/UnixSignalsTest.cpp b/lldb/unittests/Signals/UnixSignalsTest.cpp
index acd39286922501..9a7d9afc2b1859 100644
--- a/lldb/unittests/Signals/UnixSignalsTest.cpp
+++ b/lldb/unittests/Signals/UnixSignalsTest.cpp
@@ -119,7 +119,7 @@ TEST(UnixSignalsTest, GetAsString) {
ASSERT_EQ("SIG16: a specific type of SIG16",
signals.GetSignalDescription(16, 1, 0xCAFEF00D));
// Known code that should.
- ASSERT_EQ("SIG16: SIG16 with a fault address (fault address: 0xcafef00d)",
+ ASSERT_EQ("SIG16: SIG16 with a fault address (fault address=0xcafef00d)",
signals.GetSignalDescription(16, 2, 0xCAFEF00D));
// No address given just print the code description.
ASSERT_EQ("SIG16: SIG16 with a fault address",
@@ -131,11 +131,11 @@ TEST(UnixSignalsTest, GetAsString) {
ASSERT_EQ(expected, signals.GetSignalDescription(16, 3, 0xcafef00d));
ASSERT_EQ(expected, signals.GetSignalDescription(16, 3, 0xcafef00d, 0x1234));
- ASSERT_EQ("SIG16: upper bound violation (fault address: 0x5679, lower bound: "
- "0x1234, upper bound: 0x5678)",
+ ASSERT_EQ("SIG16: upper bound violation (fault address=0x5679, lower bound="
+ "0x1234, upper bound=0x5678)",
signals.GetSignalDescription(16, 3, 0x5679, 0x1234, 0x5678));
- ASSERT_EQ("SIG16: lower bound violation (fault address: 0x1233, lower bound: "
- "0x1234, upper bound: 0x5678)",
+ ASSERT_EQ("SIG16: lower bound violation (fault address=0x1233, lower bound="
+ "0x1234, upper bound=0x5678)",
signals.GetSignalDescription(16, 3, 0x1233, 0x1234, 0x5678));
}
|
...commands/frame/diagnose/dereference-function-return/TestDiagnoseDereferenceFunctionReturn.py
Show resolved
Hide resolved
|
We could make MacOS adopt the Linux format, right? Not that that changes much but just wondering if there's any reason you chose that direction. I see you changed the MTE core file case, there is a live process test case too, a search for |
The reason I chose this direction is because I still think of MacOS as the "reference platform" for lldb, but now that I've thought about it, there is a problem with doing the change in the other direction. This string comes from the debug/lldb-server, and MacOS has a... complicated compatibility story for the debugserver. That means we'd have to change receiving code to accept both versions at least as a transitional measure (which is something I considered already, but then decided that making things more consistent was a better approach). |
Done, and I've changed a couple of extra strings as well |
It's not to the same level, but doesn't everyone else have this issue, potentially? Though the consequence is just that if my lldb-server is old and my lldb is new, this command doesn't work. So I think the direction you've chosen is fine. So - shall we release note this and include that detail? |
Potentially yes, but I know for a fact debugserver comes with the iPhones and whatnot, which means it needs to keep working as long as the devices are supported. For example Android takes a different approach and always copies the "current" version of lldb-server for debugging (it wasn't the only factor, but not having to worry about compatibility definitely contributed to that decision). I expect most linux-y users will not have long support windows even if they're not always using matching binaries (the fact that you don't need to codesign lldb-server definitely helps with that)
SGTM |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
.. by changing the signal stop reason format 🤦
The reason this did not work is because the code in
StopInfo::GetCrashingDereferencewas looking for the string "address=" to extract the address of the crash. Macos stop reason strings have the formwhile on linux they look like:
Extracting the address from a string sounds like a bad idea, but I suppose there's some value in using a consistent format across platforms, so this patch changes the signal format to use the equals sign as well. All of the diagnose tests pass except one, which appears to fail due to something similar #115453 (disassembler reports unrelocated call targets).
I've left the tests disabled on windows, as the stop reason reporting code works very differently there, and I suspect it won't work out of the box. If I'm wrong -- the XFAIL will let us know.