Skip to content

[lldb] Avoid Function::GetAddressRange in ThreadPlanStepRange::InSymbol #128515

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 25, 2025

Conversation

labath
Copy link
Collaborator

@labath labath commented Feb 24, 2025

The existing implementation would probably produce false positives for discontinuous functions. I haven't tried reproducing it because setting up discontinuous functions (and executing them, in particular) is pretty complex and there's nothing particularly interesting happening here.

The existing implementation would probably produce false positives for
discontinuous functions. I haven't tried reproducing it because setting
up discontinuous functions (and executing them, in particular) is pretty
complex and there's nothing particularly interesting happening here.
@labath labath requested a review from jimingham February 24, 2025 14:12
@labath labath requested a review from JDevlieghere as a code owner February 24, 2025 14:12
@llvmbot llvmbot added the lldb label Feb 24, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 24, 2025

@llvm/pr-subscribers-lldb

Author: Pavel Labath (labath)

Changes

The existing implementation would probably produce false positives for discontinuous functions. I haven't tried reproducing it because setting up discontinuous functions (and executing them, in particular) is pretty complex and there's nothing particularly interesting happening here.


Full diff: https://github.com/llvm/llvm-project/pull/128515.diff

1 Files Affected:

  • (modified) lldb/source/Target/ThreadPlanStepRange.cpp (+5-3)
diff --git a/lldb/source/Target/ThreadPlanStepRange.cpp b/lldb/source/Target/ThreadPlanStepRange.cpp
index de4cd5995f695..78e1270edcdab 100644
--- a/lldb/source/Target/ThreadPlanStepRange.cpp
+++ b/lldb/source/Target/ThreadPlanStepRange.cpp
@@ -197,9 +197,11 @@ bool ThreadPlanStepRange::InRange() {
 bool ThreadPlanStepRange::InSymbol() {
   lldb::addr_t cur_pc = GetThread().GetRegisterContext()->GetPC();
   if (m_addr_context.function != nullptr) {
-    return m_addr_context.function->GetAddressRange().ContainsLoadAddress(
-        cur_pc, &GetTarget());
-  } else if (m_addr_context.symbol && m_addr_context.symbol->ValueIsAddress()) {
+    AddressRange unused_range;
+    return m_addr_context.function->GetRangeContainingLoadAddress(
+        cur_pc, GetTarget(), unused_range);
+  }
+  if (m_addr_context.symbol && m_addr_context.symbol->ValueIsAddress()) {
     AddressRange range(m_addr_context.symbol->GetAddressRef(),
                        m_addr_context.symbol->GetByteSize());
     return range.ContainsLoadAddress(cur_pc, &GetTarget());

Copy link
Collaborator

@jimingham jimingham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGMT

@labath labath merged commit 5088e1b into llvm:main Feb 25, 2025
12 checks passed
@labath labath deleted the insymbol branch February 25, 2025 08:47
@llvm-ci
Copy link
Collaborator

llvm-ci commented Feb 25, 2025

LLVM Buildbot has detected a new failure on builder lldb-arm-ubuntu running on linaro-lldb-arm-ubuntu while building lldb at step 6 "test".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/18/builds/11957

Here is the relevant piece of the build log for the reference
Step 6 (test) failure: build (failure)
...
UNSUPPORTED: lldb-api :: tools/lldb-dap/exception/objc/TestDAP_exception_objc.py (1153 of 2914)
PASS: lldb-api :: tools/lldb-dap/disconnect/TestDAP_disconnect.py (1154 of 2914)
PASS: lldb-api :: tools/lldb-dap/exception/cpp/TestDAP_exception_cpp.py (1155 of 2914)
UNSUPPORTED: lldb-api :: tools/lldb-dap/extendedStackTrace/TestDAP_extendedStackTrace.py (1156 of 2914)
PASS: lldb-api :: tools/lldb-dap/instruction-breakpoint/TestDAP_instruction_breakpoint.py (1157 of 2914)
PASS: lldb-api :: tools/lldb-dap/locations/TestDAP_locations.py (1158 of 2914)
PASS: lldb-api :: tools/lldb-dap/memory/TestDAP_memory.py (1159 of 2914)
PASS: lldb-api :: tools/lldb-dap/evaluate/TestDAP_evaluate.py (1160 of 2914)
PASS: lldb-api :: tools/lldb-dap/optimized/TestDAP_optimized.py (1161 of 2914)
PASS: lldb-api :: tools/lldb-dap/module/TestDAP_module.py (1162 of 2914)
FAIL: lldb-api :: tools/lldb-dap/output/TestDAP_output.py (1163 of 2914)
******************** TEST 'lldb-api :: tools/lldb-dap/output/TestDAP_output.py' FAILED ********************
Script:
--
/usr/bin/python3.10 /home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/test/API/dotest.py -u CXXFLAGS -u CFLAGS --env LLVM_LIBS_DIR=/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./lib --env LLVM_INCLUDE_DIR=/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/include --env LLVM_TOOLS_DIR=/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./bin --arch armv8l --build-dir /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/lldb-test-build.noindex --lldb-module-cache-dir /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/lldb-test-build.noindex/module-cache-lldb/lldb-api --clang-module-cache-dir /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/lldb-test-build.noindex/module-cache-clang/lldb-api --executable /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./bin/lldb --compiler /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./bin/clang --dsymutil /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./bin/dsymutil --make /usr/bin/gmake --llvm-tools-dir /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./bin --lldb-obj-root /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/tools/lldb --lldb-libs-dir /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./lib /home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/test/API/tools/lldb-dap/output -p TestDAP_output.py
--
Exit Code: 1

Command Output (stdout):
--
lldb version 21.0.0git (https://github.com/llvm/llvm-project.git revision 5088e1b435fd06de2bfccd3894dcc2f2c326630f)
  clang revision 5088e1b435fd06de2bfccd3894dcc2f2c326630f
  llvm revision 5088e1b435fd06de2bfccd3894dcc2f2c326630f
Skipping the following test categories: ['libc++', 'dsym', 'gmodules', 'debugserver', 'objc']
========= DEBUG ADAPTER PROTOCOL LOGS =========
1740474211.981302261 stdin/stdout --> 
Content-Length: 344

{
  "arguments": {
    "adapterID": "lldb-native",
    "clientID": "vscode",
    "columnsStartAt1": true,
    "linesStartAt1": true,
    "locale": "en-us",
    "pathFormat": "path",
    "sourceInitFile": false,
    "supportsRunInTerminalRequest": true,
    "supportsStartDebuggingRequest": true,
    "supportsVariablePaging": true,
    "supportsVariableType": true
  },
  "command": "initialize",
  "seq": 1,
  "type": "request"
}
1740474211.984862804 stdin/stdout <-- 
Content-Length: 1631


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants