https://github.com/royitaqi created
https://github.com/llvm/llvm-project/pull/142224
# Symptom
We have seen SIGSEGV like this:
```
* thread #1, name = 'lldb-server', stop reason = SIGSEGV
frame #0: 0x00007f39e529c993 libc.so.6`__pthread_kill_internal(signo=11,
threadid=<unavailable>) at pthread_kill.c:46:37
...
* frame #5: 0x000056027c94fe48
lldb-server`lldb_private::process_linux::GetPtraceScope() + 72
frame #6: 0x000056027c92f94f
lldb-server`lldb_private::process_linux::NativeProcessLinux::Attach(int) + 1087
...
```
See [full stack trace](https://pastebin.com/X0d6QhYj).
A similar error (an unchecked `Error`) can be reproduced by running the newly
added unit test without the fix on a Linux machine which doesn't have
`/proc/sys/kernel/yama/ptrace_scope`. See the "Test" section below.
# Root cause
`GetPtraceScope()`
([code](https://github.com/llvm/llvm-project/blob/328f40f408c218f25695ea42c844e43bef38660b/lldb/source/Plugins/Process/Linux/Procfs.cpp#L77))
has the following `if` statement:
```
llvm::Expected<int> lldb_private::process_linux::GetPtraceScope() {
ErrorOr<std::unique_ptr<MemoryBuffer>> ptrace_scope_file =
getProcFile("sys/kernel/yama/ptrace_scope");
if (!*ptrace_scope_file)
return errorCodeToError(ptrace_scope_file.getError());
...
}
```
The intention of the `if` statement is to check whether the `ptrace_scope_file`
is an `Error` or not, and return the error if it is. However, the `operator*`
of `ErrorOr` returns the value that is stored (which is a
`std::unique_ptr<MemoryBuffer>`), so what the `if` condition actually do is to
check if the unique pointer is non-null.
Note that the method `ErrorOr::getStorage()` ([called
by](https://github.com/llvm/llvm-project/blob/328f40f408c218f25695ea42c844e43bef38660b/llvm/include/llvm/Support/ErrorOr.h#L162-L164)
`ErrorOr::operator *`) **does** assert on whether or not `HasError` has been
set (see
[ErrorOr.h](https://github.com/llvm/llvm-project/blob/328f40f408c218f25695ea42c844e43bef38660b/llvm/include/llvm/Support/ErrorOr.h#L235-L243)).
However, it seems this wasn't executed, probably because the LLDB was a
release build.
# Fix
The fix is simply remove the `*` in the said `if` statement.
# Test
Adding a new unit test: `RealPtraceScopeWhenNotExist`.
On a Linux machine which doesn't have `/proc/sys/kernel/yama/ptrace_scope`,
**run the unit test *without* the fix** reproduces this assertion error about
an unchecked `Error`:
```
$ tools/lldb/unittests/Process/Linux/ProcessLinuxTests
[==========] Running 5 tests from 1 test suite.
...
[ RUN ] Perf.RealPtraceScopeWhenNotExist
ProcessLinuxTests:
/home/royshi/llvm-sand/external/llvm-project/llvm/include/llvm/Support/ErrorOr.h:236:
storage_type *llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>>::getStorage()
[T = std::unique_ptr<llvm::MemoryBuffer>]: Assertion `!HasError && "Cannot get
value when an error exists!"' failed.
Stack dump without symbol names (ensure you have llvm-symbolizer in your PATH
or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it):
0 ProcessLinuxTests 0x0000560e7798d6e5
1 ProcessLinuxTests 0x0000560e7798ddf4
...
Aborted (core dumped)
```
**Run the unit test *with* the fix** show passed result:
```
[royshi@devvm24317.cln0 ~/llvm-sand/build/Debug/fbcode-x86_64/toolchain]$
tools/lldb/unittests/Process/Linux/ProcessLinuxTests
[==========] Running 5 tests from 1 test suite.
...
[ RUN ] Perf.RealPtraceScopeWhenNotExist
[ OK ] Perf.RealPtraceScopeWhenNotExist (0 ms)
...
[ PASSED ] 3 tests.
```
>From 58b8a322a8cc6e3cce3d3e2d405fb915c338791a Mon Sep 17 00:00:00 2001
From: Roy Shi <roy...@meta.com>
Date: Fri, 30 May 2025 14:21:06 -0700
Subject: [PATCH 1/2] Fix crash and add test
---
lldb/source/Plugins/Process/Linux/Procfs.cpp | 2 +-
lldb/unittests/Process/Linux/ProcfsTests.cpp | 14 +++++++++++++-
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/lldb/source/Plugins/Process/Linux/Procfs.cpp
b/lldb/source/Plugins/Process/Linux/Procfs.cpp
index d3bd396fbaeab..4349a2b1adb9d 100644
--- a/lldb/source/Plugins/Process/Linux/Procfs.cpp
+++ b/lldb/source/Plugins/Process/Linux/Procfs.cpp
@@ -74,7 +74,7 @@ lldb_private::process_linux::GetAvailableLogicalCoreIDs() {
llvm::Expected<int> lldb_private::process_linux::GetPtraceScope() {
ErrorOr<std::unique_ptr<MemoryBuffer>> ptrace_scope_file =
getProcFile("sys/kernel/yama/ptrace_scope");
- if (!*ptrace_scope_file)
+ if (!ptrace_scope_file)
return errorCodeToError(ptrace_scope_file.getError());
// The contents should be something like "1\n". Trim it so we get "1".
StringRef buffer = (*ptrace_scope_file)->getBuffer().trim();
diff --git a/lldb/unittests/Process/Linux/ProcfsTests.cpp
b/lldb/unittests/Process/Linux/ProcfsTests.cpp
index e7af1f469c2bf..cbddccb89a215 100644
--- a/lldb/unittests/Process/Linux/ProcfsTests.cpp
+++ b/lldb/unittests/Process/Linux/ProcfsTests.cpp
@@ -104,7 +104,7 @@ TEST(Perf, RealLogicalCoreIDs) {
ASSERT_GT((int)cpu_ids->size(), 0) << "We must see at least one core";
}
-TEST(Perf, RealPtraceScope) {
+TEST(Perf, RealPtraceScopeWhenExist) {
// We first check we can read /proc/sys/kernel/yama/ptrace_scope
auto buffer_or_error =
errorOrToExpected(getProcFile("sys/kernel/yama/ptrace_scope"));
@@ -120,6 +120,18 @@ TEST(Perf, RealPtraceScope) {
<< "Sensible values of ptrace_scope are between 0 and 3";
}
+TEST(Perf, RealPtraceScopeWhenNotExist) {
+ // We first check we can NOT read /proc/sys/kernel/yama/ptrace_scope
+ auto buffer_or_error =
+ errorOrToExpected(getProcFile("sys/kernel/yama/ptrace_scope"));
+ if (buffer_or_error)
+ GTEST_SKIP() << "In order for this test to run,
sys/kernel/yama/ptrace_scope should not exist";
+
+ // At this point we should fail parsing the ptrace_scope value.
+ Expected<int> ptrace_scope = GetPtraceScope();
+ ASSERT_FALSE((bool)ptrace_scope);
+}
+
#ifdef LLVM_ENABLE_THREADING
TEST(Support, getProcFile_Tid) {
auto BufferOrError = getProcFile(getpid(), llvm::get_threadid(), "comm");
>From 02a2ba2d67105b1fadec0a9e0c0ee7d914a24e72 Mon Sep 17 00:00:00 2001
From: Roy Shi <roy...@meta.com>
Date: Fri, 30 May 2025 14:39:46 -0700
Subject: [PATCH 2/2] Fix test
---
lldb/unittests/Process/Linux/ProcfsTests.cpp | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/lldb/unittests/Process/Linux/ProcfsTests.cpp
b/lldb/unittests/Process/Linux/ProcfsTests.cpp
index cbddccb89a215..a795fa4e019c5 100644
--- a/lldb/unittests/Process/Linux/ProcfsTests.cpp
+++ b/lldb/unittests/Process/Linux/ProcfsTests.cpp
@@ -125,11 +125,13 @@ TEST(Perf, RealPtraceScopeWhenNotExist) {
auto buffer_or_error =
errorOrToExpected(getProcFile("sys/kernel/yama/ptrace_scope"));
if (buffer_or_error)
- GTEST_SKIP() << "In order for this test to run,
sys/kernel/yama/ptrace_scope should not exist";
+ GTEST_SKIP() << "In order for this test to run,
/proc/sys/kernel/yama/ptrace_scope should not exist";
+ consumeError(buffer_or_error.takeError());
// At this point we should fail parsing the ptrace_scope value.
Expected<int> ptrace_scope = GetPtraceScope();
ASSERT_FALSE((bool)ptrace_scope);
+ consumeError(ptrace_scope.takeError());
}
#ifdef LLVM_ENABLE_THREADING
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
