Author: royitaqi Date: 2025-06-02T10:43:58-07:00 New Revision: 8e8da88d46dcb1ba6ffe266779dd047fa772497e
URL: https://github.com/llvm/llvm-project/commit/8e8da88d46dcb1ba6ffe266779dd047fa772497e DIFF: https://github.com/llvm/llvm-project/commit/8e8da88d46dcb1ba6ffe266779dd047fa772497e.diff LOG: [lldb] Fix SIGSEGV in `GetPtraceScope()` in `Procfs.cpp` (#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). This happens on Linux where LLDB doesn't have access to `/proc/sys/kernel/yama/ptrace_scope`. A similar error (an unchecked `Error`) can be reproduced by running the newly added unit test without the fix. 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. Added: Modified: lldb/source/Plugins/Process/Linux/Procfs.cpp lldb/unittests/Process/Linux/ProcfsTests.cpp Removed: ################################################################################ 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..534e3e132ec62 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,21 @@ 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, " + "/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 TEST(Support, getProcFile_Tid) { auto BufferOrError = getProcFile(getpid(), llvm::get_threadid(), "comm"); _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
