llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: None (royitaqi) <details> <summary>Changes</summary> # 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. ``` --- Full diff: https://github.com/llvm/llvm-project/pull/142224.diff 2 Files Affected: - (modified) lldb/source/Plugins/Process/Linux/Procfs.cpp (+1-1) - (modified) lldb/unittests/Process/Linux/ProcfsTests.cpp (+15-1) ``````````diff 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..a795fa4e019c5 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,20 @@ 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"); `````````` </details> https://github.com/llvm/llvm-project/pull/142224 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits