[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
mstorsjo wrote: > > FYI, I also ran into failures due to this change, when debugging on Windows > > (with DWARF). See the run in > > https://github.com/mstorsjo/actions-test/actions/runs/10020326811 vs > > https://github.com/mstorsjo/actions-test/actions/runs/10020393197. Thanks > > for reverting; hopefully it’s the same root cause as on other platforms. > > oooh that 10020393197 is surprising. It looks like we hit a breakpoint, did > `finish` up until a recursive function and it resumed execution without > stopping. I haven't seen that one before, and not sure how I could have > introduced it. I'll def need to look at my ProcessWindows changes and again > see if I can't figure out what might be going on. Aleksandr ran a slightly > earlier version of the windows changes against the lldb testsuite and didn't > uncover any problems, but it didn't include this test file. > > @mstorsjo is this test source file on-line somewhere, or can you paste it > into a comment? I'll try to repo on linux or macos too, maybe it is a unique > corner case unrelated to Windows that is a problem. (unlikely, but you never > know if we might get lucky). Thanks for the heads-up. The source to this testcase is https://github.com/mstorsjo/llvm-mingw/blob/master/test/hello-exception.cpp, and I put up a prebuilt binary at https://martin.st/temp/hello-exception-dwarf.exe, if you want to have a look at it in that form. I'm running LLDB with the following input: https://github.com/mstorsjo/llvm-mingw/blob/master/run-lldb-tests.sh#L111-L120 ``` # Test setting a breakpoint in LLDB, checking the backtrace when we hit it, # stepping from the breakpoint, and running to completion. cat > $SCRIPT < $OUT 2>/dev/null ``` FWIW, interestingly, the issue did seem to appear on x86_64, but not on i386. This was with a non-optimized build of it, in case that makes any difference (as it sometimes does for how unwinding etc behaves). https://github.com/llvm/llvm-project/pull/96260 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
jasonmolenda wrote: > FYI, I also ran into failures due to this change, when debugging on Windows > (with DWARF). See the run in > https://github.com/mstorsjo/actions-test/actions/runs/10020326811 vs > https://github.com/mstorsjo/actions-test/actions/runs/10020393197. Thanks for > reverting; hopefully it’s the same root cause as on other platforms. oooh that 10020393197 is surprising. It looks like we hit a breakpoint, did `finish` up until a recursive function and it resumed execution without stopping. I haven't seen that one before, and not sure how I could have introduced it. I'll def need to look at my ProcessWindows changes and again see if I can't figure out what might be going on. Aleksandr ran a slightly earlier version of the windows changes against the lldb testsuite and didn't uncover any problems, but it didn't include this test file. @mstorsjo is this test source file on-line somewhere, or can you paste it into a comment? I'll try to repo on linux or macos too, maybe it is a unique corner case unrelated to Windows that is a problem. (unlikely, but you never know if we might get lucky). Thanks for the heads-up. https://github.com/llvm/llvm-project/pull/96260 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
mstorsjo wrote: FYI, I also ran into failures due to this change, when debugging on Windows (with DWARF). See the run in https://github.com/mstorsjo/actions-test/actions/runs/10020326811 vs https://github.com/mstorsjo/actions-test/actions/runs/10020393197. Thanks for reverting; hopefully it’s the same root cause as on other platforms. https://github.com/llvm/llvm-project/pull/96260 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
jasonmolenda wrote: I've reverted this change until I can debug the failures found on the CI bots. Two debuginfo dexter tests failed, likely because the way stepping and breakpoints works is now different. If you're sitting at a BreakpointSite which has not yet executed, previously lldb would overwrite the stop reason with "breakpoint hit" even though it had not been hit. Now, you'll have your actual stop reason -- and when you resume, you'll hit the breakpoint and stop again. If you 'continue', it stops again. If you 'next/step', it stops again and your next/step is still on the thread plan stack, so if you 'continue', you're going to first do the next/step that was on the thread stack, and then you can do your other command. I've spent a lot of time thinking about any other approach to this, and they all have bad outcomes that we don't want to add. In short, I expect the debuginfo tests are continuing from a breakpoint and not expecting the breakpoint to be hit. The ubuntu-arm bot failed TestConsecutiveBreakpoints.py and TestConsecutiveBreakpoints.py, but it seems like the only failure text was like "process exited -9" or something. I may need to re-land the patch with a lot more logging enabled on these two, to narrow down what this bot is seeing. I let it run an extra cycle and it failed in the same spot again, so it wasn't a oneoff error. https://github.com/llvm/llvm-project/pull/96260 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
llvm-ci wrote: 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/1091 Here is the relevant piece of the build log for the reference: ``` Step 6 (test) failure: build (failure) ... PASS: lldb-api :: symbol_ondemand/shared_library/TestSharedLibOnDemand.py (1098 of 2753) PASS: lldb-api :: terminal/TestSTTYBeforeAndAfter.py (1099 of 2753) PASS: lldb-api :: test_utils/TestDecorators.py (1100 of 2753) PASS: lldb-api :: test_utils/TestInlineTest.py (1101 of 2753) PASS: lldb-api :: test_utils/TestPExpectTest.py (1102 of 2753) PASS: lldb-api :: test_utils/base/TestBaseTest.py (1103 of 2753) PASS: lldb-api :: terminal/TestEditline.py (1104 of 2753) PASS: lldb-api :: tools/lldb-dap/attach/TestDAP_attach.py (1105 of 2753) UNSUPPORTED: lldb-api :: tools/lldb-dap/breakpoint-events/TestDAP_breakpointEvents.py (1106 of 2753) TIMEOUT: lldb-api :: functionalities/breakpoint/consecutive_breakpoints/TestConsecutiveBreakpoints.py (1107 of 2753) TEST 'lldb-api :: functionalities/breakpoint/consecutive_breakpoints/TestConsecutiveBreakpoints.py' FAILED Script: -- /usr/bin/python3.8 /home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/test/API/dotest.py -u CXXFLAGS -u CFLAGS --env ARCHIVER=/usr/local/bin/llvm-ar --env OBJCOPY=/usr/bin/llvm-objcopy --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 --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/functionalities/breakpoint/consecutive_breakpoints -p TestConsecutiveBreakpoints.py -- Exit Code: -9 Timeout: Reached timeout of 600 seconds Command Output (stdout): -- lldb version 19.0.0git (https://github.com/llvm/llvm-project.git revision 05f0e86cc895181b3d2210458c78938f83353002) clang revision 05f0e86cc895181b3d2210458c78938f83353002 llvm revision 05f0e86cc895181b3d2210458c78938f83353002 -- PASS: lldb-api :: tools/lldb-dap/breakpoint/TestDAP_logpoints.py (1108 of 2753) PASS: lldb-api :: tools/lldb-dap/breakpoint/TestDAP_setExceptionBreakpoints.py (1109 of 2753) PASS: lldb-api :: tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py (1110 of 2753) PASS: lldb-api :: tools/lldb-dap/commands/TestDAP_commands.py ( of 2753) PASS: lldb-api :: tools/lldb-dap/attach/TestDAP_attachByPortNum.py (1112 of 2753) PASS: lldb-api :: tools/lldb-dap/breakpoint/TestDAP_setFunctionBreakpoints.py (1113 of 2753) PASS: lldb-api :: tools/lldb-dap/console/TestDAP_redirection_to_console.py (1114 of 2753) PASS: lldb-api :: tools/lldb-dap/completions/TestDAP_completions.py (1115 of 2753) PASS: lldb-api :: tools/lldb-dap/coreFile/TestDAP_coreFile.py (1116 of 2753) PASS: lldb-api :: tools/lldb-dap/disassemble/TestDAP_disassemble.py (1117 of 2753) TIMEOUT: lldb-api :: functionalities/breakpoint/step_over_breakpoint/TestStepOverBreakpoint.py (1118 of 2753) TEST 'lldb-api :: functionalities/breakpoint/step_over_breakpoint/TestStepOverBreakpoint.py' FAILED Script: -- /usr/bin/python3.8 /home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/test/API/dotest.py -u CXXFLAGS -u CFLAGS --env ARCHIVER=/usr/local/bin/llvm-ar --env OBJCOPY=/usr/bin/llvm-objcopy --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
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
llvm-ci wrote: LLVM Buildbot has detected a new failure on builder `cross-project-tests-sie-ubuntu` running on `doug-worker-1a` while building `lldb` at step 6 "test-build-unified-tree-check-cross-project". Full details are available at: https://lab.llvm.org/buildbot/#/builders/181/builds/2053 Here is the relevant piece of the build log for the reference: ``` Step 6 (test-build-unified-tree-check-cross-project) failure: test (failure) TEST 'cross-project-tests :: debuginfo-tests/dexter/feature_tests/commands/perfect/dex_finish_test/default_conditional_hit_count.cpp' FAILED Exit Code: 2 Command Output (stderr): -- RUN: at line 10: clang++ -O0 -glldb -std=gnu++11 /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/llvm-project/cross-project-tests/debuginfo-tests/dexter/feature_tests/commands/perfect/dex_finish_test/default_conditional_hit_count.cpp -o /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/build/projects/cross-project-tests/debuginfo-tests/dexter/feature_tests/commands/perfect/dex_finish_test/Output/default_conditional_hit_count.cpp.tmp + clang++ -O0 -glldb -std=gnu++11 /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/llvm-project/cross-project-tests/debuginfo-tests/dexter/feature_tests/commands/perfect/dex_finish_test/default_conditional_hit_count.cpp -o /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/build/projects/cross-project-tests/debuginfo-tests/dexter/feature_tests/commands/perfect/dex_finish_test/Output/default_conditional_hit_count.cpp.tmp RUN: at line 11: "/usr/bin/python3.8" "/home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/llvm-project/cross-project-tests/debuginfo-tests/dexter/dexter.py" test --fail-lt 1.0 -w --debugger lldb --binary /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/build/projects/cross-project-tests/debuginfo-tests/dexter/feature_tests/commands/perfect/dex_finish_test/Output/default_conditional_hit_count.cpp.tmp -- /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/llvm-project/cross-project-tests/debuginfo-tests/dexter/feature_tests/commands/perfect/dex_finish_test/default_conditional_hit_count.cpp | /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/build/bin/FileCheck /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/llvm-project/cross-project-tests/debuginfo-tests/dexter/feature_tests/commands/perfect/dex_finish_test/default_conditional_hit_count.cpp + /usr/bin/python3.8 /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/llvm-project/cross-project-tests/debuginfo-tests/dexter/dexter.py test --fail-lt 1.0 -w --debugger lldb --binary /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/build/projects/cross-project-tests/debuginfo-tests/dexter/feature_tests/commands/perfect/dex_finish_test/Output/default_conditional_hit_count.cpp.tmp -- /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/llvm-project/cross-project-tests/debuginfo-tests/dexter/feature_tests/commands/perfect/dex_finish_test/default_conditional_hit_count.cpp + /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/build/bin/FileCheck /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/llvm-project/cross-project-tests/debuginfo-tests/dexter/feature_tests/commands/perfect/dex_finish_test/default_conditional_hit_count.cpp -- ``` https://github.com/llvm/llvm-project/pull/96260 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
llvm-ci wrote: LLVM Buildbot has detected a new failure on builder `cross-project-tests-sie-ubuntu-dwarf5` running on `doug-worker-1b` while building `lldb` at step 6 "test-build-unified-tree-check-cross-project". Full details are available at: https://lab.llvm.org/buildbot/#/builders/163/builds/2092 Here is the relevant piece of the build log for the reference: ``` Step 6 (test-build-unified-tree-check-cross-project) failure: test (failure) TEST 'cross-project-tests :: debuginfo-tests/dexter/feature_tests/subtools/test/address_printing.cpp' FAILED Exit Code: 1 Command Output (stderr): -- RUN: at line 14: clang++ -O0 -glldb -std=gnu++11 /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu-dwarf5/llvm-project/cross-project-tests/debuginfo-tests/dexter/feature_tests/subtools/test/address_printing.cpp -o /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu-dwarf5/build/projects/cross-project-tests/debuginfo-tests/dexter/feature_tests/subtools/test/Output/address_printing.cpp.tmp + clang++ -O0 -glldb -std=gnu++11 /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu-dwarf5/llvm-project/cross-project-tests/debuginfo-tests/dexter/feature_tests/subtools/test/address_printing.cpp -o /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu-dwarf5/build/projects/cross-project-tests/debuginfo-tests/dexter/feature_tests/subtools/test/Output/address_printing.cpp.tmp RUN: at line 15: not "/usr/bin/python3.10" "/home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu-dwarf5/llvm-project/cross-project-tests/debuginfo-tests/dexter/dexter.py" test --fail-lt 1.0 -w --debugger lldb --binary /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu-dwarf5/build/projects/cross-project-tests/debuginfo-tests/dexter/feature_tests/subtools/test/Output/address_printing.cpp.tmp -v -- /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu-dwarf5/llvm-project/cross-project-tests/debuginfo-tests/dexter/feature_tests/subtools/test/address_printing.cpp | /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu-dwarf5/build/bin/FileCheck /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu-dwarf5/llvm-project/cross-project-tests/debuginfo-tests/dexter/feature_tests/subtools/test/address_printing.cpp + /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu-dwarf5/build/bin/FileCheck /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu-dwarf5/llvm-project/cross-project-tests/debuginfo-tests/dexter/feature_tests/subtools/test/address_printing.cpp + not /usr/bin/python3.10 /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu-dwarf5/llvm-project/cross-project-tests/debuginfo-tests/dexter/dexter.py test --fail-lt 1.0 -w --debugger lldb --binary /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu-dwarf5/build/projects/cross-project-tests/debuginfo-tests/dexter/feature_tests/subtools/test/Output/address_printing.cpp.tmp -v -- /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu-dwarf5/llvm-project/cross-project-tests/debuginfo-tests/dexter/feature_tests/subtools/test/address_printing.cpp [1m/home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu-dwarf5/llvm-project/cross-project-tests/debuginfo-tests/dexter/feature_tests/subtools/test/address_printing.cpp:34:16: [0m[0;1;31merror: [0m[1mCHECK-NEXT: expected string not found in input [0m// CHECK-NEXT: (0x[[Y_VAL]]): step 4 [0;1;32m ^ [0m[1m:37:20: [0m[0;1;30mnote: [0m[1mscanning from here [0m misordered result: [0;1;32m ^ [0m[1m:37:20: [0m[0;1;30mnote: [0m[1mwith "Y_VAL" equal to "5556aed0" [0m misordered result: [0;1;32m ^ [0m[1m:38:16: [0m[0;1;30mnote: [0m[1mpossible intended match here [0m (0x5556aed0): step 6 [-3] [0;1;32m ^ [0m Input file: Check file: /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu-dwarf5/llvm-project/cross-project-tests/debuginfo-tests/dexter/feature_tests/subtools/test/address_printing.cpp -dump-input=help explains the following input dump. Input was: << [1m[0m[0;1;30m1: [0m[1m[0;1;46maddress_printing.cpp: (0.5714) [0m [0;1;30m2: [0m[1m[0;1;46m [0m [0;1;30m3: [0m[1m[0;1;46m## BEGIN ## [0m [0;1;30m4: [0m[1m[0;1;46m[1, "main", "/home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu-dwarf5/llvm-project/cross-project-tests/debuginfo-tests/dexter/feature_tests/subtools/test/address_printing.cpp", 38, 14, "StopReason.BREAKPOINT", "StepKind.FUNC", []] [0m [0;1;30m5: [0m[1m[0;1;46m[2, "main", "/home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu-dwarf5/llvm-project/cross-project-tests/debuginfo-tests/dexter/feature_tests/subtools/test/address_printing.cpp", 39, 14, "StopReason.STEP", "StepKind.VERTICAL_FORWARD", []] [0m [0;1;30m6: [0m[1m[0;1;46m[3, "main",
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
https://github.com/jasonmolenda closed https://github.com/llvm/llvm-project/pull/96260 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
jasonmolenda wrote: The pre-merge linux-x86 test builds both failed in TestConcurrentVFork.py after I updated the branch to current main. I don't know if there's a bot issue or a main issue. I tried repoing on aarch64 ubuntu; that test is skipped on aarch64 but I removed the skips and it runs without fails in my VM. I'm going to try landing this and see what happens; will revert if bots blow up. I've tried to test all the combinations I have access to here, and Aleksandr's testing on windows has been invaluable, but it's still possible it'll need a little reworking. https://github.com/llvm/llvm-project/pull/96260 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
https://github.com/jasonmolenda updated https://github.com/llvm/llvm-project/pull/96260 >From 9b541e6a035635e26c6a24eca022de8552fa4c17 Mon Sep 17 00:00:00 2001 From: Jason Molenda Date: Thu, 20 Jun 2024 17:53:17 -0700 Subject: [PATCH 1/8] [lldb] Change lldb's breakpoint handling behavior lldb today has two rules: When a thread stops at a BreakpointSite, we set the thread's StopReason to be "breakpoint hit" (regardless if we've actually hit the breakpoint, or if we've merely stopped *at* the breakpoint instruction/point and haven't tripped it yet). And second, when resuming a process, any thread sitting at a BreakpointSite is silently stepped over the BreakpointSite -- because we've already flagged the breakpoint hit when we stopped there originally. In this patch, I change lldb to only set a thread's stop reason to breakpoint-hit when we've actually executed the instruction/triggered the breakpoint. When we resume, we only silently step past a BreakpointSite that we've registered as hit. We preserve this state across inferior function calls that the user may do while stopped, etc. Also, when a user adds a new breakpoint at $pc while stopped, or changes $pc to be the address of a BreakpointSite, we will silently step past that breakpoint when the process resumes. This is purely a UX call, I don't think there's any person who wants to set a breakpoint at $pc and then hit it immediately on resuming. One non-intuitive UX from this change, but I'm convinced it is necessary: If you're stopped at a BreakpointSite that has not yet executed, you `stepi`, you will hit the breakpoint and the pc will not yet advance. This thread has not completed its stepi, and the thread plan is still on the stack. If you then `continue` the thread, lldb will now stop and say, "instruction step completed", one instruction past the BreakpointSite. You can continue a second time to resume execution. I discussed this with Jim, and trying to paper over this behavior will lead to more complicated scenarios behaving non-intuitively. And mostly it's the testsuite that was trying to instruction step past a breakpoint and getting thrown off -- and I changed those tests to expect the new behavior. The bugs driving this change are all from lldb dropping the real stop reason for a thread and setting it to breakpoint-hit when that was not the case. Jim hit one where we have an aarch64 watchpoint that triggers one instruction before a BreakpointSite. On this arch we are notified of the watchpoint hit after the instruction has been unrolled -- we disable the watchpoint, instruction step, re-enable the watchpoint and collect the new value. But now we're on a BreakpointSite so the watchpoint-hit stop reason is lost. Another was reported by ZequanWu in https://discourse.llvm.org/t/lldb-unable-to-break-at-start/78282 we attach to/launch a process with the pc at a BreakpointSite and misbehave. Caroline Tice mentioned it is also a problem they've had with putting a breakpoint on _dl_debug_state. The change to each Process plugin that does execution control is that 1. If we've stopped at a BreakpointSite (whether we hit it or not), we call Thread::SetThreadStoppedAtBreakpointSite(pc) to record the state at the point when the thread stopped. (so we can detect newly-added breakpoints, or when the pc is changed to an instruction that is a BreakpointSite) 2. When we have actually hit a breakpoint, and it is enabled for this thread, we call Thread::SetThreadHitBreakpointAtAddr(pc) so we know that it should be silently stepped past when we resume execution. When resuming, we silently step over a breakpoint if we've hit it, or if it is newly added (or the pc was changed to an existing BreakpointSite). The biggest set of changes is to StopInfoMachException where we translate a Mach Exception into a stop reason. The Mach exception codes differ in a few places depending on the target (unambiguously), and I didn't want to duplicate the new code for each target so I've tested what mach exceptions we get for each action on each target, and reorganized StopInfoMachException::CreateStopReasonWithMachException to document these possible values, and handle them without specializing based on the target arch. rdar://123942164 --- lldb/include/lldb/Target/Thread.h | 29 ++ .../Process/Utility/StopInfoMachException.cpp | 296 +++--- .../Process/Windows/Common/ProcessWindows.cpp | 16 +- .../Process/gdb-remote/ProcessGDBRemote.cpp | 118 +++ .../Process/scripted/ScriptedThread.cpp | 9 + lldb/source/Target/Thread.cpp | 17 +- .../TestConsecutiveBreakpoints.py | 26 +- .../TestStepOverBreakpoint.py | 6 +- 8 files changed, 235 insertions(+), 282 deletions(-) diff --git a/lldb/include/lldb/Target/Thread.h b/lldb/include/lldb/Target/Thread.h index c17bddf4d98b8..1e1aead896018 100644 --- a/lldb/include/lldb/Target/Thread.h +++
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
https://github.com/jasonmolenda updated https://github.com/llvm/llvm-project/pull/96260 >From 9b541e6a035635e26c6a24eca022de8552fa4c17 Mon Sep 17 00:00:00 2001 From: Jason Molenda Date: Thu, 20 Jun 2024 17:53:17 -0700 Subject: [PATCH 1/8] [lldb] Change lldb's breakpoint handling behavior lldb today has two rules: When a thread stops at a BreakpointSite, we set the thread's StopReason to be "breakpoint hit" (regardless if we've actually hit the breakpoint, or if we've merely stopped *at* the breakpoint instruction/point and haven't tripped it yet). And second, when resuming a process, any thread sitting at a BreakpointSite is silently stepped over the BreakpointSite -- because we've already flagged the breakpoint hit when we stopped there originally. In this patch, I change lldb to only set a thread's stop reason to breakpoint-hit when we've actually executed the instruction/triggered the breakpoint. When we resume, we only silently step past a BreakpointSite that we've registered as hit. We preserve this state across inferior function calls that the user may do while stopped, etc. Also, when a user adds a new breakpoint at $pc while stopped, or changes $pc to be the address of a BreakpointSite, we will silently step past that breakpoint when the process resumes. This is purely a UX call, I don't think there's any person who wants to set a breakpoint at $pc and then hit it immediately on resuming. One non-intuitive UX from this change, but I'm convinced it is necessary: If you're stopped at a BreakpointSite that has not yet executed, you `stepi`, you will hit the breakpoint and the pc will not yet advance. This thread has not completed its stepi, and the thread plan is still on the stack. If you then `continue` the thread, lldb will now stop and say, "instruction step completed", one instruction past the BreakpointSite. You can continue a second time to resume execution. I discussed this with Jim, and trying to paper over this behavior will lead to more complicated scenarios behaving non-intuitively. And mostly it's the testsuite that was trying to instruction step past a breakpoint and getting thrown off -- and I changed those tests to expect the new behavior. The bugs driving this change are all from lldb dropping the real stop reason for a thread and setting it to breakpoint-hit when that was not the case. Jim hit one where we have an aarch64 watchpoint that triggers one instruction before a BreakpointSite. On this arch we are notified of the watchpoint hit after the instruction has been unrolled -- we disable the watchpoint, instruction step, re-enable the watchpoint and collect the new value. But now we're on a BreakpointSite so the watchpoint-hit stop reason is lost. Another was reported by ZequanWu in https://discourse.llvm.org/t/lldb-unable-to-break-at-start/78282 we attach to/launch a process with the pc at a BreakpointSite and misbehave. Caroline Tice mentioned it is also a problem they've had with putting a breakpoint on _dl_debug_state. The change to each Process plugin that does execution control is that 1. If we've stopped at a BreakpointSite (whether we hit it or not), we call Thread::SetThreadStoppedAtBreakpointSite(pc) to record the state at the point when the thread stopped. (so we can detect newly-added breakpoints, or when the pc is changed to an instruction that is a BreakpointSite) 2. When we have actually hit a breakpoint, and it is enabled for this thread, we call Thread::SetThreadHitBreakpointAtAddr(pc) so we know that it should be silently stepped past when we resume execution. When resuming, we silently step over a breakpoint if we've hit it, or if it is newly added (or the pc was changed to an existing BreakpointSite). The biggest set of changes is to StopInfoMachException where we translate a Mach Exception into a stop reason. The Mach exception codes differ in a few places depending on the target (unambiguously), and I didn't want to duplicate the new code for each target so I've tested what mach exceptions we get for each action on each target, and reorganized StopInfoMachException::CreateStopReasonWithMachException to document these possible values, and handle them without specializing based on the target arch. rdar://123942164 --- lldb/include/lldb/Target/Thread.h | 29 ++ .../Process/Utility/StopInfoMachException.cpp | 296 +++--- .../Process/Windows/Common/ProcessWindows.cpp | 16 +- .../Process/gdb-remote/ProcessGDBRemote.cpp | 118 +++ .../Process/scripted/ScriptedThread.cpp | 9 + lldb/source/Target/Thread.cpp | 17 +- .../TestConsecutiveBreakpoints.py | 26 +- .../TestStepOverBreakpoint.py | 6 +- 8 files changed, 235 insertions(+), 282 deletions(-) diff --git a/lldb/include/lldb/Target/Thread.h b/lldb/include/lldb/Target/Thread.h index c17bddf4d98b8..1e1aead896018 100644 --- a/lldb/include/lldb/Target/Thread.h +++
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
https://github.com/jimingham approved this pull request. I had a couple trivial comment comments (and you didn't fix one typo). But this LGTM as well. https://github.com/llvm/llvm-project/pull/96260 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
@@ -577,15 +577,18 @@ StopInfoSP
StopInfoMachException::CreateStopReasonWithMachException(
ProcessSP process_sp(thread.GetProcess());
RegisterContextSP reg_ctx_sp(thread.GetRegisterContext());
- BreakpointSiteSP bp_site_sp;
- addr_t pc = LLDB_INVALID_ADDRESS;
- if (reg_ctx_sp) {
-pc = reg_ctx_sp->GetPC();
-BreakpointSiteSP bp_site_sp =
-process_sp->GetBreakpointSiteList().FindByAddress(pc);
-if (bp_site_sp && bp_site_sp->IsEnabled())
- thread.SetThreadStoppedAtUnexecutedBP(pc);
- }
+ // Caveat: with x86 KDP if we've hit a breakpoint, the pc we
+ // receive is past the breakpoint instruction.
+ // If we have a breakpoint at 0x100 (on a 1-byte original instruction)
jimingham wrote:
This construction tripped me up, though the content is clear. I'd either say:
If we have breakpoints at ...
or
If we have a breakpoint at... and ONE at 0x101
(not capitalized, I just did that so you could see the change.
https://github.com/llvm/llvm-project/pull/96260
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
https://github.com/jimingham deleted https://github.com/llvm/llvm-project/pull/96260 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
@@ -577,15 +577,18 @@ StopInfoSP
StopInfoMachException::CreateStopReasonWithMachException(
ProcessSP process_sp(thread.GetProcess());
RegisterContextSP reg_ctx_sp(thread.GetRegisterContext());
- BreakpointSiteSP bp_site_sp;
- addr_t pc = LLDB_INVALID_ADDRESS;
- if (reg_ctx_sp) {
-pc = reg_ctx_sp->GetPC();
-BreakpointSiteSP bp_site_sp =
-process_sp->GetBreakpointSiteList().FindByAddress(pc);
-if (bp_site_sp && bp_site_sp->IsEnabled())
- thread.SetThreadStoppedAtUnexecutedBP(pc);
- }
+ // Caveat: with x86 KDP if we've hit a breakpoint, the pc we
+ // receive is past the breakpoint instruction.
+ // If we have a breakpoint at 0x100 (on a 1-byte original instruction)
+ // and at 0x101, we hit the 0x100 breakpoint and the pc is
jimingham wrote:
and at 0x101 -> and are stopped at 0x101
https://github.com/llvm/llvm-project/pull/96260
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
https://github.com/jasonmolenda edited https://github.com/llvm/llvm-project/pull/96260 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
https://github.com/jasonmolenda updated https://github.com/llvm/llvm-project/pull/96260 >From 9b541e6a035635e26c6a24eca022de8552fa4c17 Mon Sep 17 00:00:00 2001 From: Jason Molenda Date: Thu, 20 Jun 2024 17:53:17 -0700 Subject: [PATCH 1/7] [lldb] Change lldb's breakpoint handling behavior lldb today has two rules: When a thread stops at a BreakpointSite, we set the thread's StopReason to be "breakpoint hit" (regardless if we've actually hit the breakpoint, or if we've merely stopped *at* the breakpoint instruction/point and haven't tripped it yet). And second, when resuming a process, any thread sitting at a BreakpointSite is silently stepped over the BreakpointSite -- because we've already flagged the breakpoint hit when we stopped there originally. In this patch, I change lldb to only set a thread's stop reason to breakpoint-hit when we've actually executed the instruction/triggered the breakpoint. When we resume, we only silently step past a BreakpointSite that we've registered as hit. We preserve this state across inferior function calls that the user may do while stopped, etc. Also, when a user adds a new breakpoint at $pc while stopped, or changes $pc to be the address of a BreakpointSite, we will silently step past that breakpoint when the process resumes. This is purely a UX call, I don't think there's any person who wants to set a breakpoint at $pc and then hit it immediately on resuming. One non-intuitive UX from this change, but I'm convinced it is necessary: If you're stopped at a BreakpointSite that has not yet executed, you `stepi`, you will hit the breakpoint and the pc will not yet advance. This thread has not completed its stepi, and the thread plan is still on the stack. If you then `continue` the thread, lldb will now stop and say, "instruction step completed", one instruction past the BreakpointSite. You can continue a second time to resume execution. I discussed this with Jim, and trying to paper over this behavior will lead to more complicated scenarios behaving non-intuitively. And mostly it's the testsuite that was trying to instruction step past a breakpoint and getting thrown off -- and I changed those tests to expect the new behavior. The bugs driving this change are all from lldb dropping the real stop reason for a thread and setting it to breakpoint-hit when that was not the case. Jim hit one where we have an aarch64 watchpoint that triggers one instruction before a BreakpointSite. On this arch we are notified of the watchpoint hit after the instruction has been unrolled -- we disable the watchpoint, instruction step, re-enable the watchpoint and collect the new value. But now we're on a BreakpointSite so the watchpoint-hit stop reason is lost. Another was reported by ZequanWu in https://discourse.llvm.org/t/lldb-unable-to-break-at-start/78282 we attach to/launch a process with the pc at a BreakpointSite and misbehave. Caroline Tice mentioned it is also a problem they've had with putting a breakpoint on _dl_debug_state. The change to each Process plugin that does execution control is that 1. If we've stopped at a BreakpointSite (whether we hit it or not), we call Thread::SetThreadStoppedAtBreakpointSite(pc) to record the state at the point when the thread stopped. (so we can detect newly-added breakpoints, or when the pc is changed to an instruction that is a BreakpointSite) 2. When we have actually hit a breakpoint, and it is enabled for this thread, we call Thread::SetThreadHitBreakpointAtAddr(pc) so we know that it should be silently stepped past when we resume execution. When resuming, we silently step over a breakpoint if we've hit it, or if it is newly added (or the pc was changed to an existing BreakpointSite). The biggest set of changes is to StopInfoMachException where we translate a Mach Exception into a stop reason. The Mach exception codes differ in a few places depending on the target (unambiguously), and I didn't want to duplicate the new code for each target so I've tested what mach exceptions we get for each action on each target, and reorganized StopInfoMachException::CreateStopReasonWithMachException to document these possible values, and handle them without specializing based on the target arch. rdar://123942164 --- lldb/include/lldb/Target/Thread.h | 29 ++ .../Process/Utility/StopInfoMachException.cpp | 296 +++--- .../Process/Windows/Common/ProcessWindows.cpp | 16 +- .../Process/gdb-remote/ProcessGDBRemote.cpp | 118 +++ .../Process/scripted/ScriptedThread.cpp | 9 + lldb/source/Target/Thread.cpp | 17 +- .../TestConsecutiveBreakpoints.py | 26 +- .../TestStepOverBreakpoint.py | 6 +- 8 files changed, 235 insertions(+), 282 deletions(-) diff --git a/lldb/include/lldb/Target/Thread.h b/lldb/include/lldb/Target/Thread.h index c17bddf4d98b8..1e1aead896018 100644 --- a/lldb/include/lldb/Target/Thread.h +++
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
@@ -229,6 +229,17 @@ bool ScriptedThread::CalculateStopInfo() {
LLVM_PRETTY_FUNCTION, "Failed to get scripted thread stop info.",
error,
LLDBLog::Thread);
+ // If we're at a BreakpointSite, mark that we stopped there and
jasonmolenda wrote:
This comment was meant to be on ProcessGDBRemote.cpp where I removed a section
of code for a thread that had no stop signal/reason, but is sitting at a
BreakpointSite. The old comment on this:
```
// If a thread is stopped at a breakpoint site, set that as the stop
// reason even if it hasn't executed the breakpoint instruction yet.
// We will silently step over the breakpoint when we resume execution
// and miss the fact that this thread hit the breakpoint.
```
That's the old stepping behavior. The new stepping behavior is if a thread
stops at a BreakpointSite but has no signal/breakpoint hit reason, we call
`Thread::SetThreadStoppedAtUnexecutedBP(pc)`. This is done at the top of
`ProcessGDBRemote::SetThreadStopInfo` already if the thread stops at a
BreakpointSite but hasn't hit it, so there's no need for this block to still be
around.
https://github.com/llvm/llvm-project/pull/96260
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
@@ -633,171 +613,142 @@ StopInfoSP
StopInfoMachException::CreateStopReasonWithMachException(
}
break;
+ // [exc_type, exc_code, exc_sub_code, exc_sub_sub_code]
+ //
+ // Instruction step:
+ // [6, 1, 0]
+ // Intel KDP [6, 3, ??]
+ // armv7 [6, 0x102, ] Same as software breakpoint!
+ //
+ // Software breakpoint:
+ // x86 [6, 2, 0]
+ // Intel KDP [6, 2, ]
+ // arm64 [6, 1, ]
+ // armv7 [6, 0x102, ] Same as instruction step!
+ //
+ // Hardware breakpoint:
+ // x86 [6, 1, , 0]
+ // x86/Rosetta not implemented, see software breakpoint
+ // arm64 [6, 1, ]
+ // armv7 not implemented, see software breakpoint
+ //
+ // Hardware watchpoint:
+ // x86 [6, 1, , 0] (both Intel hw and Rosetta)
+ // arm64 [6, 0x102, , 0]
+ // armv7 [6, 0x102, , 0]
+ //
+ // arm64 BRK instruction (imm arg not reflected in the ME)
+ // [ 6, 1, ]
+ //
+ // In order of codes mach exceptions:
+ // [6, 1, 0] - instruction step
+ // [6, 1, ] - hardware breakpoint or watchpoint
+ //
+ // [6, 2, 0] - software breakpoint
+ // [6, 2, ] - software breakpoint
+ //
+ // [6, 3] - instruction step
+ //
+ // [6, 0x102, ] armv7 instruction step
+ // [6, 0x102, ] armv7 software breakpoint
+ // [6, 0x102, , 0] arm64/armv7 watchpoint
case 6: // EXC_BREAKPOINT
{
-bool is_actual_breakpoint = false;
-bool is_trace_if_actual_breakpoint_missing = false;
-switch (cpu) {
-case llvm::Triple::x86:
-case llvm::Triple::x86_64:
- if (exc_code == 1) // EXC_I386_SGL
- {
-if (!exc_sub_code) {
- // This looks like a plain trap.
- // Have to check if there is a breakpoint here as well. When you
- // single-step onto a trap, the single step stops you not to trap.
- // Since we also do that check below, let's just use that logic.
- is_actual_breakpoint = true;
- is_trace_if_actual_breakpoint_missing = true;
-} else {
- if (StopInfoSP stop_info =
- GetStopInfoForHardwareBP(thread, target, exc_data_count,
- exc_sub_code, exc_sub_sub_code))
-return stop_info;
-}
- } else if (exc_code == 2 || // EXC_I386_BPT
- exc_code == 3) // EXC_I386_BPTFLT
- {
-// KDP returns EXC_I386_BPTFLT for trace breakpoints
-if (exc_code == 3)
- is_trace_if_actual_breakpoint_missing = true;
-
-is_actual_breakpoint = true;
-if (!pc_already_adjusted)
- pc_decrement = 1;
- }
- break;
-
-case llvm::Triple::arm:
-case llvm::Triple::thumb:
- if (exc_code == 0x102) // EXC_ARM_DA_DEBUG
- {
-// LWP_TODO: We need to find the WatchpointResource that matches
-// the address, and evaluate its Watchpoints.
-
-// It's a watchpoint, then, if the exc_sub_code indicates a
-// known/enabled data break address from our watchpoint list.
-lldb::WatchpointSP wp_sp;
-if (target)
- wp_sp = target->GetWatchpointList().FindByAddress(
- (lldb::addr_t)exc_sub_code);
-if (wp_sp && wp_sp->IsEnabled()) {
- return StopInfo::CreateStopReasonWithWatchpointID(thread,
-wp_sp->GetID());
-} else {
- is_actual_breakpoint = true;
- is_trace_if_actual_breakpoint_missing = true;
-}
- } else if (exc_code == 1) // EXC_ARM_BREAKPOINT
- {
-is_actual_breakpoint = true;
-is_trace_if_actual_breakpoint_missing = true;
- } else if (exc_code == 0) // FIXME not EXC_ARM_BREAKPOINT but a kernel
-// is currently returning this so accept it
-// as indicating a breakpoint until the
-// kernel is fixed
- {
-is_actual_breakpoint = true;
-is_trace_if_actual_breakpoint_missing = true;
- }
- break;
+bool stopped_by_hitting_breakpoint = false;
+bool stopped_by_completing_stepi = false;
+bool stopped_watchpoint = false;
+std::optional value;
+
+// exc_code 1
+if (exc_code == 1 && exc_sub_code == 0)
+ stopped_by_completing_stepi = true;
+if (exc_code == 1 && exc_sub_code != 0) {
+ stopped_by_hitting_breakpoint = true;
+ stopped_watchpoint = true;
+ value = exc_sub_code;
+}
-case llvm::Triple::aarch64_32:
-case llvm::Triple::aarch64: {
- // xnu describes three things with type EXC_BREAKPOINT:
- //
- // exc_code 0x102 [EXC_ARM_DA_DEBUG], exc_sub_code addr-of-insn
- // Watchpoint access. exc_sub_code is the address of the
- // instruction which trigged the watchpoint trap.
- // debugserver may add the watchpoint number that was triggered
- // in exc_sub_sub_code.
- //
- // exc_code 1
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
@@ -633,171 +613,142 @@ StopInfoSP
StopInfoMachException::CreateStopReasonWithMachException(
}
break;
+ // [exc_type, exc_code, exc_sub_code, exc_sub_sub_code]
+ //
+ // Instruction step:
+ // [6, 1, 0]
+ // Intel KDP [6, 3, ??]
+ // armv7 [6, 0x102, ] Same as software breakpoint!
+ //
+ // Software breakpoint:
+ // x86 [6, 2, 0]
+ // Intel KDP [6, 2, ]
+ // arm64 [6, 1, ]
+ // armv7 [6, 0x102, ] Same as instruction step!
+ //
+ // Hardware breakpoint:
+ // x86 [6, 1, , 0]
+ // x86/Rosetta not implemented, see software breakpoint
+ // arm64 [6, 1, ]
+ // armv7 not implemented, see software breakpoint
+ //
+ // Hardware watchpoint:
+ // x86 [6, 1, , 0] (both Intel hw and Rosetta)
+ // arm64 [6, 0x102, , 0]
+ // armv7 [6, 0x102, , 0]
+ //
+ // arm64 BRK instruction (imm arg not reflected in the ME)
+ // [ 6, 1, ]
+ //
+ // In order of codes mach exceptions:
+ // [6, 1, 0] - instruction step
+ // [6, 1, ] - hardware breakpoint or watchpoint
+ //
+ // [6, 2, 0] - software breakpoint
+ // [6, 2, ] - software breakpoint
+ //
+ // [6, 3] - instruction step
+ //
+ // [6, 0x102, ] armv7 instruction step
+ // [6, 0x102, ] armv7 software breakpoint
+ // [6, 0x102, , 0] arm64/armv7 watchpoint
case 6: // EXC_BREAKPOINT
{
-bool is_actual_breakpoint = false;
-bool is_trace_if_actual_breakpoint_missing = false;
-switch (cpu) {
-case llvm::Triple::x86:
-case llvm::Triple::x86_64:
- if (exc_code == 1) // EXC_I386_SGL
- {
-if (!exc_sub_code) {
- // This looks like a plain trap.
- // Have to check if there is a breakpoint here as well. When you
- // single-step onto a trap, the single step stops you not to trap.
- // Since we also do that check below, let's just use that logic.
- is_actual_breakpoint = true;
- is_trace_if_actual_breakpoint_missing = true;
-} else {
- if (StopInfoSP stop_info =
- GetStopInfoForHardwareBP(thread, target, exc_data_count,
- exc_sub_code, exc_sub_sub_code))
-return stop_info;
-}
- } else if (exc_code == 2 || // EXC_I386_BPT
- exc_code == 3) // EXC_I386_BPTFLT
- {
-// KDP returns EXC_I386_BPTFLT for trace breakpoints
-if (exc_code == 3)
- is_trace_if_actual_breakpoint_missing = true;
-
-is_actual_breakpoint = true;
-if (!pc_already_adjusted)
- pc_decrement = 1;
- }
- break;
-
-case llvm::Triple::arm:
-case llvm::Triple::thumb:
- if (exc_code == 0x102) // EXC_ARM_DA_DEBUG
- {
-// LWP_TODO: We need to find the WatchpointResource that matches
-// the address, and evaluate its Watchpoints.
-
-// It's a watchpoint, then, if the exc_sub_code indicates a
-// known/enabled data break address from our watchpoint list.
-lldb::WatchpointSP wp_sp;
-if (target)
- wp_sp = target->GetWatchpointList().FindByAddress(
- (lldb::addr_t)exc_sub_code);
-if (wp_sp && wp_sp->IsEnabled()) {
- return StopInfo::CreateStopReasonWithWatchpointID(thread,
-wp_sp->GetID());
-} else {
- is_actual_breakpoint = true;
- is_trace_if_actual_breakpoint_missing = true;
-}
- } else if (exc_code == 1) // EXC_ARM_BREAKPOINT
- {
-is_actual_breakpoint = true;
-is_trace_if_actual_breakpoint_missing = true;
- } else if (exc_code == 0) // FIXME not EXC_ARM_BREAKPOINT but a kernel
-// is currently returning this so accept it
-// as indicating a breakpoint until the
-// kernel is fixed
- {
-is_actual_breakpoint = true;
-is_trace_if_actual_breakpoint_missing = true;
- }
- break;
+bool stopped_by_hitting_breakpoint = false;
+bool stopped_by_completing_stepi = false;
+bool stopped_watchpoint = false;
+std::optional value;
+
+// exc_code 1
+if (exc_code == 1 && exc_sub_code == 0)
+ stopped_by_completing_stepi = true;
+if (exc_code == 1 && exc_sub_code != 0) {
+ stopped_by_hitting_breakpoint = true;
+ stopped_watchpoint = true;
+ value = exc_sub_code;
+}
-case llvm::Triple::aarch64_32:
-case llvm::Triple::aarch64: {
- // xnu describes three things with type EXC_BREAKPOINT:
- //
- // exc_code 0x102 [EXC_ARM_DA_DEBUG], exc_sub_code addr-of-insn
- // Watchpoint access. exc_sub_code is the address of the
- // instruction which trigged the watchpoint trap.
- // debugserver may add the watchpoint number that was triggered
- // in exc_sub_sub_code.
- //
- // exc_code 1
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
@@ -229,6 +229,17 @@ bool ScriptedThread::CalculateStopInfo() {
LLVM_PRETTY_FUNCTION, "Failed to get scripted thread stop info.",
error,
LLDBLog::Thread);
+ // If we're at a BreakpointSite, mark that we stopped there and
jimingham wrote:
Is this the code that's doing what the:
} else if (!signo) {
}
branch at 1883 in the old code was doing?
https://github.com/llvm/llvm-project/pull/96260
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
@@ -1880,60 +1849,40 @@ ThreadSP ProcessGDBRemote::SetThreadStopInfo(
StopInfo::CreateStopReasonVForkDone(*thread_sp));
handled = true;
}
- } else if (!signo) {
-addr_t pc = thread_sp->GetRegisterContext()->GetPC();
-lldb::BreakpointSiteSP bp_site_sp =
-thread_sp->GetProcess()->GetBreakpointSiteList().FindByAddress(pc);
-
-// If a thread is stopped at a breakpoint site, set that as the stop
-// reason even if it hasn't executed the breakpoint instruction yet.
-// We will silently step over the breakpoint when we resume execution
-// and miss the fact that this thread hit the breakpoint.
-if (bp_site_sp && bp_site_sp->ValidForThisThread(*thread_sp)) {
-
thread_sp->SetStopInfo(StopInfo::CreateStopReasonWithBreakpointSiteID(
- *thread_sp, bp_site_sp->GetID()));
- handled = true;
-}
}
if (!handled && signo && !did_exec) {
if (signo == SIGTRAP) {
// Currently we are going to assume SIGTRAP means we are either
// hitting a breakpoint or hardware single stepping.
- handled = true;
- addr_t pc =
- thread_sp->GetRegisterContext()->GetPC() +
m_breakpoint_pc_offset;
- lldb::BreakpointSiteSP bp_site_sp =
- thread_sp->GetProcess()->GetBreakpointSiteList().FindByAddress(
- pc);
- if (bp_site_sp) {
-// If the breakpoint is for this thread, then we'll report the hit,
-// but if it is for another thread, we can just report no reason.
-// We don't need to worry about stepping over the breakpoint here,
-// that will be taken care of when the thread resumes and notices
-// that there's a breakpoint under the pc.
-if (bp_site_sp->ValidForThisThread(*thread_sp)) {
- if (m_breakpoint_pc_offset != 0)
-thread_sp->GetRegisterContext()->SetPC(pc);
- thread_sp->SetStopInfo(
- StopInfo::CreateStopReasonWithBreakpointSiteID(
- *thread_sp, bp_site_sp->GetID()));
-} else {
- StopInfoSP invalid_stop_info_sp;
- thread_sp->SetStopInfo(invalid_stop_info_sp);
-}
+ // If we were stepping then assume the stop was the result of the
jimingham wrote:
The previous algorithm reversed this assumption. It first checked for the
breakpoint, and if a breakpoint was at the site, then returned either a
breakpoint hit (if it was for this thread) or no reason, and only if there
wasn't a breakpoint there did it return the trace result.
There was also another case that I don't see handled here anymore, if we
weren't at a breakpoint, and we weren't stepping, then we made a
"StopReasonSignal.
I don't understand why it was right to switch the order of attributing causes
here, or why we no longer need the Signal fallback.
https://github.com/llvm/llvm-project/pull/96260
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
@@ -633,171 +613,142 @@ StopInfoSP
StopInfoMachException::CreateStopReasonWithMachException(
}
break;
+ // [exc_type, exc_code, exc_sub_code, exc_sub_sub_code]
+ //
+ // Instruction step:
+ // [6, 1, 0]
+ // Intel KDP [6, 3, ??]
+ // armv7 [6, 0x102, ] Same as software breakpoint!
+ //
+ // Software breakpoint:
+ // x86 [6, 2, 0]
+ // Intel KDP [6, 2, ]
+ // arm64 [6, 1, ]
+ // armv7 [6, 0x102, ] Same as instruction step!
+ //
+ // Hardware breakpoint:
+ // x86 [6, 1, , 0]
+ // x86/Rosetta not implemented, see software breakpoint
+ // arm64 [6, 1, ]
+ // armv7 not implemented, see software breakpoint
+ //
+ // Hardware watchpoint:
+ // x86 [6, 1, , 0] (both Intel hw and Rosetta)
+ // arm64 [6, 0x102, , 0]
+ // armv7 [6, 0x102, , 0]
+ //
+ // arm64 BRK instruction (imm arg not reflected in the ME)
+ // [ 6, 1, ]
+ //
+ // In order of codes mach exceptions:
+ // [6, 1, 0] - instruction step
+ // [6, 1, ] - hardware breakpoint or watchpoint
+ //
+ // [6, 2, 0] - software breakpoint
+ // [6, 2, ] - software breakpoint
+ //
+ // [6, 3] - instruction step
+ //
+ // [6, 0x102, ] armv7 instruction step
+ // [6, 0x102, ] armv7 software breakpoint
+ // [6, 0x102, , 0] arm64/armv7 watchpoint
case 6: // EXC_BREAKPOINT
{
-bool is_actual_breakpoint = false;
-bool is_trace_if_actual_breakpoint_missing = false;
-switch (cpu) {
-case llvm::Triple::x86:
-case llvm::Triple::x86_64:
- if (exc_code == 1) // EXC_I386_SGL
- {
-if (!exc_sub_code) {
- // This looks like a plain trap.
- // Have to check if there is a breakpoint here as well. When you
- // single-step onto a trap, the single step stops you not to trap.
- // Since we also do that check below, let's just use that logic.
- is_actual_breakpoint = true;
- is_trace_if_actual_breakpoint_missing = true;
-} else {
- if (StopInfoSP stop_info =
- GetStopInfoForHardwareBP(thread, target, exc_data_count,
- exc_sub_code, exc_sub_sub_code))
-return stop_info;
-}
- } else if (exc_code == 2 || // EXC_I386_BPT
- exc_code == 3) // EXC_I386_BPTFLT
- {
-// KDP returns EXC_I386_BPTFLT for trace breakpoints
-if (exc_code == 3)
- is_trace_if_actual_breakpoint_missing = true;
-
-is_actual_breakpoint = true;
-if (!pc_already_adjusted)
- pc_decrement = 1;
- }
- break;
-
-case llvm::Triple::arm:
-case llvm::Triple::thumb:
- if (exc_code == 0x102) // EXC_ARM_DA_DEBUG
- {
-// LWP_TODO: We need to find the WatchpointResource that matches
-// the address, and evaluate its Watchpoints.
-
-// It's a watchpoint, then, if the exc_sub_code indicates a
-// known/enabled data break address from our watchpoint list.
-lldb::WatchpointSP wp_sp;
-if (target)
- wp_sp = target->GetWatchpointList().FindByAddress(
- (lldb::addr_t)exc_sub_code);
-if (wp_sp && wp_sp->IsEnabled()) {
- return StopInfo::CreateStopReasonWithWatchpointID(thread,
-wp_sp->GetID());
-} else {
- is_actual_breakpoint = true;
- is_trace_if_actual_breakpoint_missing = true;
-}
- } else if (exc_code == 1) // EXC_ARM_BREAKPOINT
- {
-is_actual_breakpoint = true;
-is_trace_if_actual_breakpoint_missing = true;
- } else if (exc_code == 0) // FIXME not EXC_ARM_BREAKPOINT but a kernel
-// is currently returning this so accept it
-// as indicating a breakpoint until the
-// kernel is fixed
- {
-is_actual_breakpoint = true;
-is_trace_if_actual_breakpoint_missing = true;
- }
- break;
+bool stopped_by_hitting_breakpoint = false;
+bool stopped_by_completing_stepi = false;
+bool stopped_watchpoint = false;
+std::optional value;
+
+// exc_code 1
+if (exc_code == 1 && exc_sub_code == 0)
+ stopped_by_completing_stepi = true;
+if (exc_code == 1 && exc_sub_code != 0) {
+ stopped_by_hitting_breakpoint = true;
+ stopped_watchpoint = true;
+ value = exc_sub_code;
+}
-case llvm::Triple::aarch64_32:
-case llvm::Triple::aarch64: {
- // xnu describes three things with type EXC_BREAKPOINT:
- //
- // exc_code 0x102 [EXC_ARM_DA_DEBUG], exc_sub_code addr-of-insn
- // Watchpoint access. exc_sub_code is the address of the
- // instruction which trigged the watchpoint trap.
- // debugserver may add the watchpoint number that was triggered
- // in exc_sub_sub_code.
- //
- // exc_code 1
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
https://github.com/jimingham edited https://github.com/llvm/llvm-project/pull/96260 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
@@ -633,171 +613,142 @@ StopInfoSP
StopInfoMachException::CreateStopReasonWithMachException(
}
break;
+ // [exc_type, exc_code, exc_sub_code, exc_sub_sub_code]
+ //
+ // Instruction step:
+ // [6, 1, 0]
+ // Intel KDP [6, 3, ??]
+ // armv7 [6, 0x102, ] Same as software breakpoint!
+ //
+ // Software breakpoint:
+ // x86 [6, 2, 0]
+ // Intel KDP [6, 2, ]
+ // arm64 [6, 1, ]
+ // armv7 [6, 0x102, ] Same as instruction step!
+ //
+ // Hardware breakpoint:
+ // x86 [6, 1, , 0]
+ // x86/Rosetta not implemented, see software breakpoint
+ // arm64 [6, 1, ]
+ // armv7 not implemented, see software breakpoint
+ //
+ // Hardware watchpoint:
+ // x86 [6, 1, , 0] (both Intel hw and Rosetta)
+ // arm64 [6, 0x102, , 0]
+ // armv7 [6, 0x102, , 0]
+ //
+ // arm64 BRK instruction (imm arg not reflected in the ME)
+ // [ 6, 1, ]
+ //
+ // In order of codes mach exceptions:
+ // [6, 1, 0] - instruction step
+ // [6, 1, ] - hardware breakpoint or watchpoint
+ //
+ // [6, 2, 0] - software breakpoint
+ // [6, 2, ] - software breakpoint
+ //
+ // [6, 3] - instruction step
+ //
+ // [6, 0x102, ] armv7 instruction step
+ // [6, 0x102, ] armv7 software breakpoint
+ // [6, 0x102, , 0] arm64/armv7 watchpoint
case 6: // EXC_BREAKPOINT
{
-bool is_actual_breakpoint = false;
-bool is_trace_if_actual_breakpoint_missing = false;
-switch (cpu) {
-case llvm::Triple::x86:
-case llvm::Triple::x86_64:
- if (exc_code == 1) // EXC_I386_SGL
- {
-if (!exc_sub_code) {
- // This looks like a plain trap.
- // Have to check if there is a breakpoint here as well. When you
- // single-step onto a trap, the single step stops you not to trap.
- // Since we also do that check below, let's just use that logic.
- is_actual_breakpoint = true;
- is_trace_if_actual_breakpoint_missing = true;
-} else {
- if (StopInfoSP stop_info =
- GetStopInfoForHardwareBP(thread, target, exc_data_count,
- exc_sub_code, exc_sub_sub_code))
-return stop_info;
-}
- } else if (exc_code == 2 || // EXC_I386_BPT
- exc_code == 3) // EXC_I386_BPTFLT
- {
-// KDP returns EXC_I386_BPTFLT for trace breakpoints
-if (exc_code == 3)
- is_trace_if_actual_breakpoint_missing = true;
-
-is_actual_breakpoint = true;
-if (!pc_already_adjusted)
- pc_decrement = 1;
- }
- break;
-
-case llvm::Triple::arm:
-case llvm::Triple::thumb:
- if (exc_code == 0x102) // EXC_ARM_DA_DEBUG
- {
-// LWP_TODO: We need to find the WatchpointResource that matches
-// the address, and evaluate its Watchpoints.
-
-// It's a watchpoint, then, if the exc_sub_code indicates a
-// known/enabled data break address from our watchpoint list.
-lldb::WatchpointSP wp_sp;
-if (target)
- wp_sp = target->GetWatchpointList().FindByAddress(
- (lldb::addr_t)exc_sub_code);
-if (wp_sp && wp_sp->IsEnabled()) {
- return StopInfo::CreateStopReasonWithWatchpointID(thread,
-wp_sp->GetID());
-} else {
- is_actual_breakpoint = true;
- is_trace_if_actual_breakpoint_missing = true;
-}
- } else if (exc_code == 1) // EXC_ARM_BREAKPOINT
- {
-is_actual_breakpoint = true;
-is_trace_if_actual_breakpoint_missing = true;
- } else if (exc_code == 0) // FIXME not EXC_ARM_BREAKPOINT but a kernel
-// is currently returning this so accept it
-// as indicating a breakpoint until the
-// kernel is fixed
- {
-is_actual_breakpoint = true;
-is_trace_if_actual_breakpoint_missing = true;
- }
- break;
+bool stopped_by_hitting_breakpoint = false;
+bool stopped_by_completing_stepi = false;
+bool stopped_watchpoint = false;
+std::optional value;
+
+// exc_code 1
+if (exc_code == 1 && exc_sub_code == 0)
+ stopped_by_completing_stepi = true;
+if (exc_code == 1 && exc_sub_code != 0) {
+ stopped_by_hitting_breakpoint = true;
+ stopped_watchpoint = true;
+ value = exc_sub_code;
+}
-case llvm::Triple::aarch64_32:
-case llvm::Triple::aarch64: {
- // xnu describes three things with type EXC_BREAKPOINT:
- //
- // exc_code 0x102 [EXC_ARM_DA_DEBUG], exc_sub_code addr-of-insn
- // Watchpoint access. exc_sub_code is the address of the
- // instruction which trigged the watchpoint trap.
- // debugserver may add the watchpoint number that was triggered
- // in exc_sub_sub_code.
- //
- // exc_code 1
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
@@ -607,6 +575,18 @@ StopInfoSP
StopInfoMachException::CreateStopReasonWithMachException(
target ? target->GetArchitecture().GetMachine()
: llvm::Triple::UnknownArch;
+ ProcessSP process_sp(thread.GetProcess());
+ RegisterContextSP reg_ctx_sp(thread.GetRegisterContext());
+ BreakpointSiteSP bp_site_sp;
+ addr_t pc = LLDB_INVALID_ADDRESS;
+ if (reg_ctx_sp) {
jimingham wrote:
When would you ever not have a register context?
https://github.com/llvm/llvm-project/pull/96260
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
@@ -633,171 +613,142 @@ StopInfoSP
StopInfoMachException::CreateStopReasonWithMachException(
}
break;
+ // [exc_type, exc_code, exc_sub_code, exc_sub_sub_code]
+ //
+ // Instruction step:
+ // [6, 1, 0]
+ // Intel KDP [6, 3, ??]
+ // armv7 [6, 0x102, ] Same as software breakpoint!
+ //
+ // Software breakpoint:
+ // x86 [6, 2, 0]
+ // Intel KDP [6, 2, ]
+ // arm64 [6, 1, ]
+ // armv7 [6, 0x102, ] Same as instruction step!
+ //
+ // Hardware breakpoint:
+ // x86 [6, 1, , 0]
+ // x86/Rosetta not implemented, see software breakpoint
+ // arm64 [6, 1, ]
+ // armv7 not implemented, see software breakpoint
+ //
+ // Hardware watchpoint:
+ // x86 [6, 1, , 0] (both Intel hw and Rosetta)
+ // arm64 [6, 0x102, , 0]
+ // armv7 [6, 0x102, , 0]
+ //
+ // arm64 BRK instruction (imm arg not reflected in the ME)
+ // [ 6, 1, ]
+ //
+ // In order of codes mach exceptions:
+ // [6, 1, 0] - instruction step
+ // [6, 1, ] - hardware breakpoint or watchpoint
+ //
+ // [6, 2, 0] - software breakpoint
+ // [6, 2, ] - software breakpoint
+ //
+ // [6, 3] - instruction step
+ //
+ // [6, 0x102, ] armv7 instruction step
+ // [6, 0x102, ] armv7 software breakpoint
+ // [6, 0x102, , 0] arm64/armv7 watchpoint
case 6: // EXC_BREAKPOINT
{
-bool is_actual_breakpoint = false;
-bool is_trace_if_actual_breakpoint_missing = false;
-switch (cpu) {
-case llvm::Triple::x86:
-case llvm::Triple::x86_64:
- if (exc_code == 1) // EXC_I386_SGL
- {
-if (!exc_sub_code) {
- // This looks like a plain trap.
- // Have to check if there is a breakpoint here as well. When you
- // single-step onto a trap, the single step stops you not to trap.
- // Since we also do that check below, let's just use that logic.
- is_actual_breakpoint = true;
- is_trace_if_actual_breakpoint_missing = true;
-} else {
- if (StopInfoSP stop_info =
- GetStopInfoForHardwareBP(thread, target, exc_data_count,
- exc_sub_code, exc_sub_sub_code))
-return stop_info;
-}
- } else if (exc_code == 2 || // EXC_I386_BPT
- exc_code == 3) // EXC_I386_BPTFLT
- {
-// KDP returns EXC_I386_BPTFLT for trace breakpoints
-if (exc_code == 3)
- is_trace_if_actual_breakpoint_missing = true;
-
-is_actual_breakpoint = true;
-if (!pc_already_adjusted)
- pc_decrement = 1;
- }
- break;
-
-case llvm::Triple::arm:
-case llvm::Triple::thumb:
- if (exc_code == 0x102) // EXC_ARM_DA_DEBUG
- {
-// LWP_TODO: We need to find the WatchpointResource that matches
-// the address, and evaluate its Watchpoints.
-
-// It's a watchpoint, then, if the exc_sub_code indicates a
-// known/enabled data break address from our watchpoint list.
-lldb::WatchpointSP wp_sp;
-if (target)
- wp_sp = target->GetWatchpointList().FindByAddress(
- (lldb::addr_t)exc_sub_code);
-if (wp_sp && wp_sp->IsEnabled()) {
- return StopInfo::CreateStopReasonWithWatchpointID(thread,
-wp_sp->GetID());
-} else {
- is_actual_breakpoint = true;
- is_trace_if_actual_breakpoint_missing = true;
-}
- } else if (exc_code == 1) // EXC_ARM_BREAKPOINT
- {
-is_actual_breakpoint = true;
-is_trace_if_actual_breakpoint_missing = true;
- } else if (exc_code == 0) // FIXME not EXC_ARM_BREAKPOINT but a kernel
-// is currently returning this so accept it
-// as indicating a breakpoint until the
-// kernel is fixed
- {
-is_actual_breakpoint = true;
-is_trace_if_actual_breakpoint_missing = true;
- }
- break;
+bool stopped_by_hitting_breakpoint = false;
+bool stopped_by_completing_stepi = false;
+bool stopped_watchpoint = false;
+std::optional value;
+
+// exc_code 1
+if (exc_code == 1 && exc_sub_code == 0)
+ stopped_by_completing_stepi = true;
+if (exc_code == 1 && exc_sub_code != 0) {
+ stopped_by_hitting_breakpoint = true;
+ stopped_watchpoint = true;
+ value = exc_sub_code;
+}
-case llvm::Triple::aarch64_32:
-case llvm::Triple::aarch64: {
- // xnu describes three things with type EXC_BREAKPOINT:
- //
- // exc_code 0x102 [EXC_ARM_DA_DEBUG], exc_sub_code addr-of-insn
- // Watchpoint access. exc_sub_code is the address of the
- // instruction which trigged the watchpoint trap.
- // debugserver may add the watchpoint number that was triggered
- // in exc_sub_sub_code.
- //
- // exc_code 1
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
@@ -633,171 +613,142 @@ StopInfoSP
StopInfoMachException::CreateStopReasonWithMachException(
}
break;
+ // [exc_type, exc_code, exc_sub_code, exc_sub_sub_code]
+ //
+ // Instruction step:
+ // [6, 1, 0]
+ // Intel KDP [6, 3, ??]
+ // armv7 [6, 0x102, ] Same as software breakpoint!
+ //
+ // Software breakpoint:
+ // x86 [6, 2, 0]
+ // Intel KDP [6, 2, ]
+ // arm64 [6, 1, ]
+ // armv7 [6, 0x102, ] Same as instruction step!
+ //
+ // Hardware breakpoint:
+ // x86 [6, 1, , 0]
+ // x86/Rosetta not implemented, see software breakpoint
+ // arm64 [6, 1, ]
+ // armv7 not implemented, see software breakpoint
+ //
+ // Hardware watchpoint:
+ // x86 [6, 1, , 0] (both Intel hw and Rosetta)
+ // arm64 [6, 0x102, , 0]
+ // armv7 [6, 0x102, , 0]
+ //
+ // arm64 BRK instruction (imm arg not reflected in the ME)
+ // [ 6, 1, ]
+ //
+ // In order of codes mach exceptions:
+ // [6, 1, 0] - instruction step
+ // [6, 1, ] - hardware breakpoint or watchpoint
+ //
+ // [6, 2, 0] - software breakpoint
+ // [6, 2, ] - software breakpoint
+ //
+ // [6, 3] - instruction step
+ //
+ // [6, 0x102, ] armv7 instruction step
+ // [6, 0x102, ] armv7 software breakpoint
+ // [6, 0x102, , 0] arm64/armv7 watchpoint
case 6: // EXC_BREAKPOINT
{
-bool is_actual_breakpoint = false;
-bool is_trace_if_actual_breakpoint_missing = false;
-switch (cpu) {
-case llvm::Triple::x86:
-case llvm::Triple::x86_64:
- if (exc_code == 1) // EXC_I386_SGL
- {
-if (!exc_sub_code) {
- // This looks like a plain trap.
- // Have to check if there is a breakpoint here as well. When you
- // single-step onto a trap, the single step stops you not to trap.
- // Since we also do that check below, let's just use that logic.
- is_actual_breakpoint = true;
- is_trace_if_actual_breakpoint_missing = true;
-} else {
- if (StopInfoSP stop_info =
- GetStopInfoForHardwareBP(thread, target, exc_data_count,
- exc_sub_code, exc_sub_sub_code))
-return stop_info;
-}
- } else if (exc_code == 2 || // EXC_I386_BPT
- exc_code == 3) // EXC_I386_BPTFLT
- {
-// KDP returns EXC_I386_BPTFLT for trace breakpoints
-if (exc_code == 3)
- is_trace_if_actual_breakpoint_missing = true;
-
-is_actual_breakpoint = true;
-if (!pc_already_adjusted)
- pc_decrement = 1;
- }
- break;
-
-case llvm::Triple::arm:
-case llvm::Triple::thumb:
- if (exc_code == 0x102) // EXC_ARM_DA_DEBUG
- {
-// LWP_TODO: We need to find the WatchpointResource that matches
-// the address, and evaluate its Watchpoints.
-
-// It's a watchpoint, then, if the exc_sub_code indicates a
-// known/enabled data break address from our watchpoint list.
-lldb::WatchpointSP wp_sp;
-if (target)
- wp_sp = target->GetWatchpointList().FindByAddress(
- (lldb::addr_t)exc_sub_code);
-if (wp_sp && wp_sp->IsEnabled()) {
- return StopInfo::CreateStopReasonWithWatchpointID(thread,
-wp_sp->GetID());
-} else {
- is_actual_breakpoint = true;
- is_trace_if_actual_breakpoint_missing = true;
-}
- } else if (exc_code == 1) // EXC_ARM_BREAKPOINT
- {
-is_actual_breakpoint = true;
-is_trace_if_actual_breakpoint_missing = true;
- } else if (exc_code == 0) // FIXME not EXC_ARM_BREAKPOINT but a kernel
-// is currently returning this so accept it
-// as indicating a breakpoint until the
-// kernel is fixed
- {
-is_actual_breakpoint = true;
-is_trace_if_actual_breakpoint_missing = true;
- }
- break;
+bool stopped_by_hitting_breakpoint = false;
+bool stopped_by_completing_stepi = false;
+bool stopped_watchpoint = false;
+std::optional value;
+
+// exc_code 1
+if (exc_code == 1 && exc_sub_code == 0)
+ stopped_by_completing_stepi = true;
+if (exc_code == 1 && exc_sub_code != 0) {
+ stopped_by_hitting_breakpoint = true;
+ stopped_watchpoint = true;
+ value = exc_sub_code;
+}
-case llvm::Triple::aarch64_32:
-case llvm::Triple::aarch64: {
- // xnu describes three things with type EXC_BREAKPOINT:
- //
- // exc_code 0x102 [EXC_ARM_DA_DEBUG], exc_sub_code addr-of-insn
- // Watchpoint access. exc_sub_code is the address of the
- // instruction which trigged the watchpoint trap.
- // debugserver may add the watchpoint number that was triggered
- // in exc_sub_sub_code.
- //
- // exc_code 1
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
@@ -633,171 +613,142 @@ StopInfoSP
StopInfoMachException::CreateStopReasonWithMachException(
}
break;
+ // [exc_type, exc_code, exc_sub_code, exc_sub_sub_code]
+ //
+ // Instruction step:
+ // [6, 1, 0]
+ // Intel KDP [6, 3, ??]
+ // armv7 [6, 0x102, ] Same as software breakpoint!
+ //
+ // Software breakpoint:
+ // x86 [6, 2, 0]
+ // Intel KDP [6, 2, ]
+ // arm64 [6, 1, ]
+ // armv7 [6, 0x102, ] Same as instruction step!
+ //
+ // Hardware breakpoint:
+ // x86 [6, 1, , 0]
+ // x86/Rosetta not implemented, see software breakpoint
+ // arm64 [6, 1, ]
+ // armv7 not implemented, see software breakpoint
+ //
+ // Hardware watchpoint:
+ // x86 [6, 1, , 0] (both Intel hw and Rosetta)
+ // arm64 [6, 0x102, , 0]
+ // armv7 [6, 0x102, , 0]
+ //
+ // arm64 BRK instruction (imm arg not reflected in the ME)
+ // [ 6, 1, ]
+ //
+ // In order of codes mach exceptions:
+ // [6, 1, 0] - instruction step
+ // [6, 1, ] - hardware breakpoint or watchpoint
+ //
+ // [6, 2, 0] - software breakpoint
+ // [6, 2, ] - software breakpoint
+ //
+ // [6, 3] - instruction step
+ //
+ // [6, 0x102, ] armv7 instruction step
+ // [6, 0x102, ] armv7 software breakpoint
+ // [6, 0x102, , 0] arm64/armv7 watchpoint
case 6: // EXC_BREAKPOINT
{
-bool is_actual_breakpoint = false;
-bool is_trace_if_actual_breakpoint_missing = false;
-switch (cpu) {
-case llvm::Triple::x86:
-case llvm::Triple::x86_64:
- if (exc_code == 1) // EXC_I386_SGL
- {
-if (!exc_sub_code) {
- // This looks like a plain trap.
- // Have to check if there is a breakpoint here as well. When you
- // single-step onto a trap, the single step stops you not to trap.
- // Since we also do that check below, let's just use that logic.
- is_actual_breakpoint = true;
- is_trace_if_actual_breakpoint_missing = true;
-} else {
- if (StopInfoSP stop_info =
- GetStopInfoForHardwareBP(thread, target, exc_data_count,
- exc_sub_code, exc_sub_sub_code))
-return stop_info;
-}
- } else if (exc_code == 2 || // EXC_I386_BPT
- exc_code == 3) // EXC_I386_BPTFLT
- {
-// KDP returns EXC_I386_BPTFLT for trace breakpoints
-if (exc_code == 3)
- is_trace_if_actual_breakpoint_missing = true;
-
-is_actual_breakpoint = true;
-if (!pc_already_adjusted)
- pc_decrement = 1;
- }
- break;
-
-case llvm::Triple::arm:
-case llvm::Triple::thumb:
- if (exc_code == 0x102) // EXC_ARM_DA_DEBUG
- {
-// LWP_TODO: We need to find the WatchpointResource that matches
-// the address, and evaluate its Watchpoints.
-
-// It's a watchpoint, then, if the exc_sub_code indicates a
-// known/enabled data break address from our watchpoint list.
-lldb::WatchpointSP wp_sp;
-if (target)
- wp_sp = target->GetWatchpointList().FindByAddress(
- (lldb::addr_t)exc_sub_code);
-if (wp_sp && wp_sp->IsEnabled()) {
- return StopInfo::CreateStopReasonWithWatchpointID(thread,
-wp_sp->GetID());
-} else {
- is_actual_breakpoint = true;
- is_trace_if_actual_breakpoint_missing = true;
-}
- } else if (exc_code == 1) // EXC_ARM_BREAKPOINT
- {
-is_actual_breakpoint = true;
-is_trace_if_actual_breakpoint_missing = true;
- } else if (exc_code == 0) // FIXME not EXC_ARM_BREAKPOINT but a kernel
-// is currently returning this so accept it
-// as indicating a breakpoint until the
-// kernel is fixed
- {
-is_actual_breakpoint = true;
-is_trace_if_actual_breakpoint_missing = true;
- }
- break;
+bool stopped_by_hitting_breakpoint = false;
+bool stopped_by_completing_stepi = false;
+bool stopped_watchpoint = false;
+std::optional value;
+
+// exc_code 1
+if (exc_code == 1 && exc_sub_code == 0)
+ stopped_by_completing_stepi = true;
+if (exc_code == 1 && exc_sub_code != 0) {
+ stopped_by_hitting_breakpoint = true;
+ stopped_watchpoint = true;
+ value = exc_sub_code;
+}
-case llvm::Triple::aarch64_32:
-case llvm::Triple::aarch64: {
- // xnu describes three things with type EXC_BREAKPOINT:
- //
- // exc_code 0x102 [EXC_ARM_DA_DEBUG], exc_sub_code addr-of-insn
- // Watchpoint access. exc_sub_code is the address of the
- // instruction which trigged the watchpoint trap.
- // debugserver may add the watchpoint number that was triggered
- // in exc_sub_sub_code.
- //
- // exc_code 1
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
@@ -633,171 +613,142 @@ StopInfoSP
StopInfoMachException::CreateStopReasonWithMachException(
}
break;
+ // [exc_type, exc_code, exc_sub_code, exc_sub_sub_code]
+ //
+ // Instruction step:
+ // [6, 1, 0]
+ // Intel KDP [6, 3, ??]
+ // armv7 [6, 0x102, ] Same as software breakpoint!
+ //
+ // Software breakpoint:
+ // x86 [6, 2, 0]
+ // Intel KDP [6, 2, ]
+ // arm64 [6, 1, ]
+ // armv7 [6, 0x102, ] Same as instruction step!
+ //
+ // Hardware breakpoint:
+ // x86 [6, 1, , 0]
+ // x86/Rosetta not implemented, see software breakpoint
+ // arm64 [6, 1, ]
+ // armv7 not implemented, see software breakpoint
+ //
+ // Hardware watchpoint:
+ // x86 [6, 1, , 0] (both Intel hw and Rosetta)
+ // arm64 [6, 0x102, , 0]
+ // armv7 [6, 0x102, , 0]
+ //
+ // arm64 BRK instruction (imm arg not reflected in the ME)
+ // [ 6, 1, ]
+ //
+ // In order of codes mach exceptions:
+ // [6, 1, 0] - instruction step
+ // [6, 1, ] - hardware breakpoint or watchpoint
+ //
+ // [6, 2, 0] - software breakpoint
+ // [6, 2, ] - software breakpoint
+ //
+ // [6, 3] - instruction step
+ //
+ // [6, 0x102, ] armv7 instruction step
+ // [6, 0x102, ] armv7 software breakpoint
+ // [6, 0x102, , 0] arm64/armv7 watchpoint
case 6: // EXC_BREAKPOINT
{
-bool is_actual_breakpoint = false;
-bool is_trace_if_actual_breakpoint_missing = false;
-switch (cpu) {
-case llvm::Triple::x86:
-case llvm::Triple::x86_64:
- if (exc_code == 1) // EXC_I386_SGL
- {
-if (!exc_sub_code) {
- // This looks like a plain trap.
- // Have to check if there is a breakpoint here as well. When you
- // single-step onto a trap, the single step stops you not to trap.
- // Since we also do that check below, let's just use that logic.
- is_actual_breakpoint = true;
- is_trace_if_actual_breakpoint_missing = true;
-} else {
- if (StopInfoSP stop_info =
- GetStopInfoForHardwareBP(thread, target, exc_data_count,
- exc_sub_code, exc_sub_sub_code))
-return stop_info;
-}
- } else if (exc_code == 2 || // EXC_I386_BPT
- exc_code == 3) // EXC_I386_BPTFLT
- {
-// KDP returns EXC_I386_BPTFLT for trace breakpoints
-if (exc_code == 3)
- is_trace_if_actual_breakpoint_missing = true;
-
-is_actual_breakpoint = true;
-if (!pc_already_adjusted)
- pc_decrement = 1;
- }
- break;
-
-case llvm::Triple::arm:
-case llvm::Triple::thumb:
- if (exc_code == 0x102) // EXC_ARM_DA_DEBUG
- {
-// LWP_TODO: We need to find the WatchpointResource that matches
-// the address, and evaluate its Watchpoints.
-
-// It's a watchpoint, then, if the exc_sub_code indicates a
-// known/enabled data break address from our watchpoint list.
-lldb::WatchpointSP wp_sp;
-if (target)
- wp_sp = target->GetWatchpointList().FindByAddress(
- (lldb::addr_t)exc_sub_code);
-if (wp_sp && wp_sp->IsEnabled()) {
- return StopInfo::CreateStopReasonWithWatchpointID(thread,
-wp_sp->GetID());
-} else {
- is_actual_breakpoint = true;
- is_trace_if_actual_breakpoint_missing = true;
-}
- } else if (exc_code == 1) // EXC_ARM_BREAKPOINT
- {
-is_actual_breakpoint = true;
-is_trace_if_actual_breakpoint_missing = true;
- } else if (exc_code == 0) // FIXME not EXC_ARM_BREAKPOINT but a kernel
-// is currently returning this so accept it
-// as indicating a breakpoint until the
-// kernel is fixed
- {
-is_actual_breakpoint = true;
-is_trace_if_actual_breakpoint_missing = true;
- }
- break;
+bool stopped_by_hitting_breakpoint = false;
+bool stopped_by_completing_stepi = false;
+bool stopped_watchpoint = false;
+std::optional value;
+
+// exc_code 1
+if (exc_code == 1 && exc_sub_code == 0)
+ stopped_by_completing_stepi = true;
+if (exc_code == 1 && exc_sub_code != 0) {
+ stopped_by_hitting_breakpoint = true;
+ stopped_watchpoint = true;
+ value = exc_sub_code;
+}
-case llvm::Triple::aarch64_32:
-case llvm::Triple::aarch64: {
- // xnu describes three things with type EXC_BREAKPOINT:
- //
- // exc_code 0x102 [EXC_ARM_DA_DEBUG], exc_sub_code addr-of-insn
- // Watchpoint access. exc_sub_code is the address of the
- // instruction which trigged the watchpoint trap.
- // debugserver may add the watchpoint number that was triggered
- // in exc_sub_sub_code.
- //
- // exc_code 1
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
@@ -633,171 +613,142 @@ StopInfoSP StopInfoMachException::CreateStopReasonWithMachException( } break; + // [exc_type, exc_code, exc_sub_code, exc_sub_sub_code] jimingham wrote: This table is great, but it deserves a line saying what it is. https://github.com/llvm/llvm-project/pull/96260 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
@@ -633,171 +613,142 @@ StopInfoSP
StopInfoMachException::CreateStopReasonWithMachException(
}
break;
+ // [exc_type, exc_code, exc_sub_code, exc_sub_sub_code]
+ //
+ // Instruction step:
+ // [6, 1, 0]
+ // Intel KDP [6, 3, ??]
+ // armv7 [6, 0x102, ] Same as software breakpoint!
+ //
+ // Software breakpoint:
+ // x86 [6, 2, 0]
+ // Intel KDP [6, 2, ]
+ // arm64 [6, 1, ]
+ // armv7 [6, 0x102, ] Same as instruction step!
+ //
+ // Hardware breakpoint:
+ // x86 [6, 1, , 0]
+ // x86/Rosetta not implemented, see software breakpoint
+ // arm64 [6, 1, ]
+ // armv7 not implemented, see software breakpoint
+ //
+ // Hardware watchpoint:
+ // x86 [6, 1, , 0] (both Intel hw and Rosetta)
+ // arm64 [6, 0x102, , 0]
+ // armv7 [6, 0x102, , 0]
+ //
+ // arm64 BRK instruction (imm arg not reflected in the ME)
+ // [ 6, 1, ]
+ //
+ // In order of codes mach exceptions:
+ // [6, 1, 0] - instruction step
+ // [6, 1, ] - hardware breakpoint or watchpoint
+ //
+ // [6, 2, 0] - software breakpoint
+ // [6, 2, ] - software breakpoint
+ //
+ // [6, 3] - instruction step
+ //
+ // [6, 0x102, ] armv7 instruction step
+ // [6, 0x102, ] armv7 software breakpoint
+ // [6, 0x102, , 0] arm64/armv7 watchpoint
case 6: // EXC_BREAKPOINT
{
-bool is_actual_breakpoint = false;
-bool is_trace_if_actual_breakpoint_missing = false;
-switch (cpu) {
-case llvm::Triple::x86:
-case llvm::Triple::x86_64:
- if (exc_code == 1) // EXC_I386_SGL
- {
-if (!exc_sub_code) {
- // This looks like a plain trap.
- // Have to check if there is a breakpoint here as well. When you
- // single-step onto a trap, the single step stops you not to trap.
- // Since we also do that check below, let's just use that logic.
- is_actual_breakpoint = true;
- is_trace_if_actual_breakpoint_missing = true;
-} else {
- if (StopInfoSP stop_info =
- GetStopInfoForHardwareBP(thread, target, exc_data_count,
- exc_sub_code, exc_sub_sub_code))
-return stop_info;
-}
- } else if (exc_code == 2 || // EXC_I386_BPT
- exc_code == 3) // EXC_I386_BPTFLT
- {
-// KDP returns EXC_I386_BPTFLT for trace breakpoints
-if (exc_code == 3)
- is_trace_if_actual_breakpoint_missing = true;
-
-is_actual_breakpoint = true;
-if (!pc_already_adjusted)
- pc_decrement = 1;
- }
- break;
-
-case llvm::Triple::arm:
-case llvm::Triple::thumb:
- if (exc_code == 0x102) // EXC_ARM_DA_DEBUG
- {
-// LWP_TODO: We need to find the WatchpointResource that matches
-// the address, and evaluate its Watchpoints.
-
-// It's a watchpoint, then, if the exc_sub_code indicates a
-// known/enabled data break address from our watchpoint list.
-lldb::WatchpointSP wp_sp;
-if (target)
- wp_sp = target->GetWatchpointList().FindByAddress(
- (lldb::addr_t)exc_sub_code);
-if (wp_sp && wp_sp->IsEnabled()) {
- return StopInfo::CreateStopReasonWithWatchpointID(thread,
-wp_sp->GetID());
-} else {
- is_actual_breakpoint = true;
- is_trace_if_actual_breakpoint_missing = true;
-}
- } else if (exc_code == 1) // EXC_ARM_BREAKPOINT
- {
-is_actual_breakpoint = true;
-is_trace_if_actual_breakpoint_missing = true;
- } else if (exc_code == 0) // FIXME not EXC_ARM_BREAKPOINT but a kernel
-// is currently returning this so accept it
-// as indicating a breakpoint until the
-// kernel is fixed
- {
-is_actual_breakpoint = true;
-is_trace_if_actual_breakpoint_missing = true;
- }
- break;
+bool stopped_by_hitting_breakpoint = false;
+bool stopped_by_completing_stepi = false;
+bool stopped_watchpoint = false;
+std::optional value;
+
+// exc_code 1
+if (exc_code == 1 && exc_sub_code == 0)
+ stopped_by_completing_stepi = true;
+if (exc_code == 1 && exc_sub_code != 0) {
+ stopped_by_hitting_breakpoint = true;
jimingham wrote:
On the face of it, it seems weird that you are asserting that this is both a
"breakpoint" and a "watchpoint" hit. Later on you'll tell the difference based
on the exe_sub_code stored in value. This would be easier to read if there
were a comment explaining that.
https://github.com/llvm/llvm-project/pull/96260
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
@@ -633,171 +613,142 @@ StopInfoSP
StopInfoMachException::CreateStopReasonWithMachException(
}
break;
+ // [exc_type, exc_code, exc_sub_code, exc_sub_sub_code]
+ //
+ // Instruction step:
+ // [6, 1, 0]
+ // Intel KDP [6, 3, ??]
+ // armv7 [6, 0x102, ] Same as software breakpoint!
+ //
+ // Software breakpoint:
+ // x86 [6, 2, 0]
+ // Intel KDP [6, 2, ]
+ // arm64 [6, 1, ]
+ // armv7 [6, 0x102, ] Same as instruction step!
+ //
+ // Hardware breakpoint:
+ // x86 [6, 1, , 0]
+ // x86/Rosetta not implemented, see software breakpoint
+ // arm64 [6, 1, ]
+ // armv7 not implemented, see software breakpoint
+ //
+ // Hardware watchpoint:
+ // x86 [6, 1, , 0] (both Intel hw and Rosetta)
+ // arm64 [6, 0x102, , 0]
+ // armv7 [6, 0x102, , 0]
+ //
+ // arm64 BRK instruction (imm arg not reflected in the ME)
+ // [ 6, 1, ]
+ //
+ // In order of codes mach exceptions:
+ // [6, 1, 0] - instruction step
+ // [6, 1, ] - hardware breakpoint or watchpoint
+ //
+ // [6, 2, 0] - software breakpoint
+ // [6, 2, ] - software breakpoint
+ //
+ // [6, 3] - instruction step
+ //
+ // [6, 0x102, ] armv7 instruction step
+ // [6, 0x102, ] armv7 software breakpoint
+ // [6, 0x102, , 0] arm64/armv7 watchpoint
case 6: // EXC_BREAKPOINT
{
-bool is_actual_breakpoint = false;
-bool is_trace_if_actual_breakpoint_missing = false;
-switch (cpu) {
-case llvm::Triple::x86:
-case llvm::Triple::x86_64:
- if (exc_code == 1) // EXC_I386_SGL
- {
-if (!exc_sub_code) {
- // This looks like a plain trap.
- // Have to check if there is a breakpoint here as well. When you
- // single-step onto a trap, the single step stops you not to trap.
- // Since we also do that check below, let's just use that logic.
- is_actual_breakpoint = true;
- is_trace_if_actual_breakpoint_missing = true;
-} else {
- if (StopInfoSP stop_info =
- GetStopInfoForHardwareBP(thread, target, exc_data_count,
- exc_sub_code, exc_sub_sub_code))
-return stop_info;
-}
- } else if (exc_code == 2 || // EXC_I386_BPT
- exc_code == 3) // EXC_I386_BPTFLT
- {
-// KDP returns EXC_I386_BPTFLT for trace breakpoints
-if (exc_code == 3)
- is_trace_if_actual_breakpoint_missing = true;
-
-is_actual_breakpoint = true;
-if (!pc_already_adjusted)
- pc_decrement = 1;
- }
- break;
-
-case llvm::Triple::arm:
-case llvm::Triple::thumb:
- if (exc_code == 0x102) // EXC_ARM_DA_DEBUG
- {
-// LWP_TODO: We need to find the WatchpointResource that matches
-// the address, and evaluate its Watchpoints.
-
-// It's a watchpoint, then, if the exc_sub_code indicates a
-// known/enabled data break address from our watchpoint list.
-lldb::WatchpointSP wp_sp;
-if (target)
- wp_sp = target->GetWatchpointList().FindByAddress(
- (lldb::addr_t)exc_sub_code);
-if (wp_sp && wp_sp->IsEnabled()) {
- return StopInfo::CreateStopReasonWithWatchpointID(thread,
-wp_sp->GetID());
-} else {
- is_actual_breakpoint = true;
- is_trace_if_actual_breakpoint_missing = true;
-}
- } else if (exc_code == 1) // EXC_ARM_BREAKPOINT
- {
-is_actual_breakpoint = true;
-is_trace_if_actual_breakpoint_missing = true;
- } else if (exc_code == 0) // FIXME not EXC_ARM_BREAKPOINT but a kernel
-// is currently returning this so accept it
-// as indicating a breakpoint until the
-// kernel is fixed
- {
-is_actual_breakpoint = true;
-is_trace_if_actual_breakpoint_missing = true;
- }
- break;
+bool stopped_by_hitting_breakpoint = false;
+bool stopped_by_completing_stepi = false;
+bool stopped_watchpoint = false;
+std::optional value;
+
+// exc_code 1
+if (exc_code == 1 && exc_sub_code == 0)
jimingham wrote:
Despite our fear of indentation, since the two if's exhaust the exe_code == 1
possibilities, this would be clearer as:
if (exc_code == 1) {
if (exc_sub_code == 0) {
} else {
}
}
https://github.com/llvm/llvm-project/pull/96260
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
@@ -633,171 +613,142 @@ StopInfoSP
StopInfoMachException::CreateStopReasonWithMachException(
}
break;
+ // [exc_type, exc_code, exc_sub_code, exc_sub_sub_code]
+ //
+ // Instruction step:
+ // [6, 1, 0]
+ // Intel KDP [6, 3, ??]
+ // armv7 [6, 0x102, ] Same as software breakpoint!
+ //
+ // Software breakpoint:
+ // x86 [6, 2, 0]
+ // Intel KDP [6, 2, ]
+ // arm64 [6, 1, ]
+ // armv7 [6, 0x102, ] Same as instruction step!
+ //
+ // Hardware breakpoint:
+ // x86 [6, 1, , 0]
+ // x86/Rosetta not implemented, see software breakpoint
+ // arm64 [6, 1, ]
+ // armv7 not implemented, see software breakpoint
+ //
+ // Hardware watchpoint:
+ // x86 [6, 1, , 0] (both Intel hw and Rosetta)
+ // arm64 [6, 0x102, , 0]
+ // armv7 [6, 0x102, , 0]
+ //
+ // arm64 BRK instruction (imm arg not reflected in the ME)
+ // [ 6, 1, ]
+ //
+ // In order of codes mach exceptions:
+ // [6, 1, 0] - instruction step
+ // [6, 1, ] - hardware breakpoint or watchpoint
+ //
+ // [6, 2, 0] - software breakpoint
+ // [6, 2, ] - software breakpoint
+ //
+ // [6, 3] - instruction step
+ //
+ // [6, 0x102, ] armv7 instruction step
+ // [6, 0x102, ] armv7 software breakpoint
+ // [6, 0x102, , 0] arm64/armv7 watchpoint
case 6: // EXC_BREAKPOINT
{
-bool is_actual_breakpoint = false;
-bool is_trace_if_actual_breakpoint_missing = false;
-switch (cpu) {
-case llvm::Triple::x86:
-case llvm::Triple::x86_64:
- if (exc_code == 1) // EXC_I386_SGL
- {
-if (!exc_sub_code) {
- // This looks like a plain trap.
- // Have to check if there is a breakpoint here as well. When you
- // single-step onto a trap, the single step stops you not to trap.
- // Since we also do that check below, let's just use that logic.
- is_actual_breakpoint = true;
- is_trace_if_actual_breakpoint_missing = true;
-} else {
- if (StopInfoSP stop_info =
- GetStopInfoForHardwareBP(thread, target, exc_data_count,
- exc_sub_code, exc_sub_sub_code))
-return stop_info;
-}
- } else if (exc_code == 2 || // EXC_I386_BPT
- exc_code == 3) // EXC_I386_BPTFLT
- {
-// KDP returns EXC_I386_BPTFLT for trace breakpoints
-if (exc_code == 3)
- is_trace_if_actual_breakpoint_missing = true;
-
-is_actual_breakpoint = true;
-if (!pc_already_adjusted)
- pc_decrement = 1;
- }
- break;
-
-case llvm::Triple::arm:
-case llvm::Triple::thumb:
- if (exc_code == 0x102) // EXC_ARM_DA_DEBUG
- {
-// LWP_TODO: We need to find the WatchpointResource that matches
-// the address, and evaluate its Watchpoints.
-
-// It's a watchpoint, then, if the exc_sub_code indicates a
-// known/enabled data break address from our watchpoint list.
-lldb::WatchpointSP wp_sp;
-if (target)
- wp_sp = target->GetWatchpointList().FindByAddress(
- (lldb::addr_t)exc_sub_code);
-if (wp_sp && wp_sp->IsEnabled()) {
- return StopInfo::CreateStopReasonWithWatchpointID(thread,
-wp_sp->GetID());
-} else {
- is_actual_breakpoint = true;
- is_trace_if_actual_breakpoint_missing = true;
-}
- } else if (exc_code == 1) // EXC_ARM_BREAKPOINT
- {
-is_actual_breakpoint = true;
-is_trace_if_actual_breakpoint_missing = true;
- } else if (exc_code == 0) // FIXME not EXC_ARM_BREAKPOINT but a kernel
-// is currently returning this so accept it
-// as indicating a breakpoint until the
-// kernel is fixed
- {
-is_actual_breakpoint = true;
-is_trace_if_actual_breakpoint_missing = true;
- }
- break;
+bool stopped_by_hitting_breakpoint = false;
+bool stopped_by_completing_stepi = false;
+bool stopped_watchpoint = false;
+std::optional value;
jimingham wrote:
Is there a better name for this than "value" - that's pretty generic.
https://github.com/llvm/llvm-project/pull/96260
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
@@ -1311,6 +1334,16 @@ class Thread : public std::enable_shared_from_this, bool m_should_run_before_public_stop; // If this thread has "stop others" // private work to do, then it will // set this. + lldb::addr_t + m_stopped_at_unexecuted_bp; // When the thread stops at a + // breakpoint instruction/BreakpointSite + // but has not yet hit/exectued it, jimingham wrote: executed https://github.com/llvm/llvm-project/pull/96260 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
@@ -128,6 +128,9 @@ class Thread : public std::enable_shared_from_this, register_backup_sp; // You need to restore the registers, of course... uint32_t current_inlined_depth; lldb::addr_t current_inlined_pc; +lldb::addr_t stopped_at_unexecuted_bp; // Set to the address of a breakpoint jimingham wrote: This is the stored version of the Thread ivar with roughly the same name. It's better documented there, maybe here you can just refer to that ivar for docs? https://github.com/llvm/llvm-project/pull/96260 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
@@ -377,6 +380,26 @@ class Thread : public std::enable_shared_from_this,
virtual void SetQueueLibdispatchQueueAddress(lldb::addr_t dispatch_queue_t)
{}
+ /// When a thread stops at an enabled BreakpointSite that has not exectued,
+ /// the Process plugin should call SetThreadStoppedAtUnexecutedBP(pc).
+ /// If that BreakpointSite was actually triggered (the instruction was
+ /// executed, for a software breakpoint), regardless of wheether the
+ /// breakpoint is valid for this thread, SetThreadHitBreakpointSite()
+ /// should be called to clear that state.
jimingham wrote:
"to clear that state" is confusing. A thread could stop because it executed
the breakpoint trap without ever having stopped "with the PC at the unexecuted
trap". That's actually the more common case. So you won't be clearing that
state usually. Maybe "to clear that state" -> "to record that fact".
https://github.com/llvm/llvm-project/pull/96260
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
@@ -377,6 +380,26 @@ class Thread : public std::enable_shared_from_this,
virtual void SetQueueLibdispatchQueueAddress(lldb::addr_t dispatch_queue_t)
{}
+ /// When a thread stops at an enabled BreakpointSite that has not exectued,
+ /// the Process plugin should call SetThreadStoppedAtUnexecutedBP(pc).
+ /// If that BreakpointSite was actually triggered (the instruction was
+ /// executed, for a software breakpoint), regardless of wheether the
jimingham wrote:
wheether -> whether
https://github.com/llvm/llvm-project/pull/96260
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
@@ -377,6 +380,26 @@ class Thread : public std::enable_shared_from_this,
virtual void SetQueueLibdispatchQueueAddress(lldb::addr_t dispatch_queue_t)
{}
+ /// When a thread stops at an enabled BreakpointSite that has not exectued,
jimingham wrote:
exectued -> executed
https://github.com/llvm/llvm-project/pull/96260
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
jasonmolenda wrote: Thanks again for all the help @AlexK0 I pushed an update that should compile correctly with my update. It's great to hear that the testsuite looks clean -- that's the part that worried me the most about these changes. I finished an aarch64 Ubuntu testsuite run and it is clean. I think this patch is about ready for review by @jimingham when he has time. https://github.com/llvm/llvm-project/pull/96260 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
https://github.com/jasonmolenda updated https://github.com/llvm/llvm-project/pull/96260 >From 9b541e6a035635e26c6a24eca022de8552fa4c17 Mon Sep 17 00:00:00 2001 From: Jason Molenda Date: Thu, 20 Jun 2024 17:53:17 -0700 Subject: [PATCH 1/6] [lldb] Change lldb's breakpoint handling behavior lldb today has two rules: When a thread stops at a BreakpointSite, we set the thread's StopReason to be "breakpoint hit" (regardless if we've actually hit the breakpoint, or if we've merely stopped *at* the breakpoint instruction/point and haven't tripped it yet). And second, when resuming a process, any thread sitting at a BreakpointSite is silently stepped over the BreakpointSite -- because we've already flagged the breakpoint hit when we stopped there originally. In this patch, I change lldb to only set a thread's stop reason to breakpoint-hit when we've actually executed the instruction/triggered the breakpoint. When we resume, we only silently step past a BreakpointSite that we've registered as hit. We preserve this state across inferior function calls that the user may do while stopped, etc. Also, when a user adds a new breakpoint at $pc while stopped, or changes $pc to be the address of a BreakpointSite, we will silently step past that breakpoint when the process resumes. This is purely a UX call, I don't think there's any person who wants to set a breakpoint at $pc and then hit it immediately on resuming. One non-intuitive UX from this change, but I'm convinced it is necessary: If you're stopped at a BreakpointSite that has not yet executed, you `stepi`, you will hit the breakpoint and the pc will not yet advance. This thread has not completed its stepi, and the thread plan is still on the stack. If you then `continue` the thread, lldb will now stop and say, "instruction step completed", one instruction past the BreakpointSite. You can continue a second time to resume execution. I discussed this with Jim, and trying to paper over this behavior will lead to more complicated scenarios behaving non-intuitively. And mostly it's the testsuite that was trying to instruction step past a breakpoint and getting thrown off -- and I changed those tests to expect the new behavior. The bugs driving this change are all from lldb dropping the real stop reason for a thread and setting it to breakpoint-hit when that was not the case. Jim hit one where we have an aarch64 watchpoint that triggers one instruction before a BreakpointSite. On this arch we are notified of the watchpoint hit after the instruction has been unrolled -- we disable the watchpoint, instruction step, re-enable the watchpoint and collect the new value. But now we're on a BreakpointSite so the watchpoint-hit stop reason is lost. Another was reported by ZequanWu in https://discourse.llvm.org/t/lldb-unable-to-break-at-start/78282 we attach to/launch a process with the pc at a BreakpointSite and misbehave. Caroline Tice mentioned it is also a problem they've had with putting a breakpoint on _dl_debug_state. The change to each Process plugin that does execution control is that 1. If we've stopped at a BreakpointSite (whether we hit it or not), we call Thread::SetThreadStoppedAtBreakpointSite(pc) to record the state at the point when the thread stopped. (so we can detect newly-added breakpoints, or when the pc is changed to an instruction that is a BreakpointSite) 2. When we have actually hit a breakpoint, and it is enabled for this thread, we call Thread::SetThreadHitBreakpointAtAddr(pc) so we know that it should be silently stepped past when we resume execution. When resuming, we silently step over a breakpoint if we've hit it, or if it is newly added (or the pc was changed to an existing BreakpointSite). The biggest set of changes is to StopInfoMachException where we translate a Mach Exception into a stop reason. The Mach exception codes differ in a few places depending on the target (unambiguously), and I didn't want to duplicate the new code for each target so I've tested what mach exceptions we get for each action on each target, and reorganized StopInfoMachException::CreateStopReasonWithMachException to document these possible values, and handle them without specializing based on the target arch. rdar://123942164 --- lldb/include/lldb/Target/Thread.h | 29 ++ .../Process/Utility/StopInfoMachException.cpp | 296 +++--- .../Process/Windows/Common/ProcessWindows.cpp | 16 +- .../Process/gdb-remote/ProcessGDBRemote.cpp | 118 +++ .../Process/scripted/ScriptedThread.cpp | 9 + lldb/source/Target/Thread.cpp | 17 +- .../TestConsecutiveBreakpoints.py | 26 +- .../TestStepOverBreakpoint.py | 6 +- 8 files changed, 235 insertions(+), 282 deletions(-) diff --git a/lldb/include/lldb/Target/Thread.h b/lldb/include/lldb/Target/Thread.h index c17bddf4d98b8..1e1aead896018 100644 --- a/lldb/include/lldb/Target/Thread.h +++
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
AlexK0 wrote: @jasonmolenda, I checked the tests with the latest fix. There is a minor compilation error: https://github.com/llvm/llvm-project/pull/96260#pullrequestreview-2161497469 If fix it (I just drop `pc` from the logging), all breakpoint tests work, and I don’t see any new failures compared to the main branch. https://github.com/llvm/llvm-project/pull/96260 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
https://github.com/AlexK0 edited https://github.com/llvm/llvm-project/pull/96260 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
@@ -377,24 +377,17 @@ void ProcessWindows::RefreshStateAfterStop() {
if (!stop_thread)
return;
- switch (active_exception->GetExceptionCode()) {
- case EXCEPTION_SINGLE_STEP: {
-RegisterContextSP register_context = stop_thread->GetRegisterContext();
+ // If we're at a BreakpointSite, mark this as an Unexecuted Breakpoint.
+ // We'll clear that state if we've actually executed the breakpoint.
+ if (RegisterContextSP register_context = stop_thread->GetRegisterContext()) {
AlexK0 wrote:
Moving `pc` into a scope causes a compilation error at the line with the logging
Here:
https://github.com/llvm/llvm-project/pull/96260/files#diff-3f6125dd89c50f3b1751b5a7d3270cdd93ad55eae76aef83deee563834c45888R399
https://github.com/llvm/llvm-project/pull/96260
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
https://github.com/jasonmolenda updated https://github.com/llvm/llvm-project/pull/96260 >From 9b541e6a035635e26c6a24eca022de8552fa4c17 Mon Sep 17 00:00:00 2001 From: Jason Molenda Date: Thu, 20 Jun 2024 17:53:17 -0700 Subject: [PATCH 1/5] [lldb] Change lldb's breakpoint handling behavior lldb today has two rules: When a thread stops at a BreakpointSite, we set the thread's StopReason to be "breakpoint hit" (regardless if we've actually hit the breakpoint, or if we've merely stopped *at* the breakpoint instruction/point and haven't tripped it yet). And second, when resuming a process, any thread sitting at a BreakpointSite is silently stepped over the BreakpointSite -- because we've already flagged the breakpoint hit when we stopped there originally. In this patch, I change lldb to only set a thread's stop reason to breakpoint-hit when we've actually executed the instruction/triggered the breakpoint. When we resume, we only silently step past a BreakpointSite that we've registered as hit. We preserve this state across inferior function calls that the user may do while stopped, etc. Also, when a user adds a new breakpoint at $pc while stopped, or changes $pc to be the address of a BreakpointSite, we will silently step past that breakpoint when the process resumes. This is purely a UX call, I don't think there's any person who wants to set a breakpoint at $pc and then hit it immediately on resuming. One non-intuitive UX from this change, but I'm convinced it is necessary: If you're stopped at a BreakpointSite that has not yet executed, you `stepi`, you will hit the breakpoint and the pc will not yet advance. This thread has not completed its stepi, and the thread plan is still on the stack. If you then `continue` the thread, lldb will now stop and say, "instruction step completed", one instruction past the BreakpointSite. You can continue a second time to resume execution. I discussed this with Jim, and trying to paper over this behavior will lead to more complicated scenarios behaving non-intuitively. And mostly it's the testsuite that was trying to instruction step past a breakpoint and getting thrown off -- and I changed those tests to expect the new behavior. The bugs driving this change are all from lldb dropping the real stop reason for a thread and setting it to breakpoint-hit when that was not the case. Jim hit one where we have an aarch64 watchpoint that triggers one instruction before a BreakpointSite. On this arch we are notified of the watchpoint hit after the instruction has been unrolled -- we disable the watchpoint, instruction step, re-enable the watchpoint and collect the new value. But now we're on a BreakpointSite so the watchpoint-hit stop reason is lost. Another was reported by ZequanWu in https://discourse.llvm.org/t/lldb-unable-to-break-at-start/78282 we attach to/launch a process with the pc at a BreakpointSite and misbehave. Caroline Tice mentioned it is also a problem they've had with putting a breakpoint on _dl_debug_state. The change to each Process plugin that does execution control is that 1. If we've stopped at a BreakpointSite (whether we hit it or not), we call Thread::SetThreadStoppedAtBreakpointSite(pc) to record the state at the point when the thread stopped. (so we can detect newly-added breakpoints, or when the pc is changed to an instruction that is a BreakpointSite) 2. When we have actually hit a breakpoint, and it is enabled for this thread, we call Thread::SetThreadHitBreakpointAtAddr(pc) so we know that it should be silently stepped past when we resume execution. When resuming, we silently step over a breakpoint if we've hit it, or if it is newly added (or the pc was changed to an existing BreakpointSite). The biggest set of changes is to StopInfoMachException where we translate a Mach Exception into a stop reason. The Mach exception codes differ in a few places depending on the target (unambiguously), and I didn't want to duplicate the new code for each target so I've tested what mach exceptions we get for each action on each target, and reorganized StopInfoMachException::CreateStopReasonWithMachException to document these possible values, and handle them without specializing based on the target arch. rdar://123942164 --- lldb/include/lldb/Target/Thread.h | 29 ++ .../Process/Utility/StopInfoMachException.cpp | 296 +++--- .../Process/Windows/Common/ProcessWindows.cpp | 16 +- .../Process/gdb-remote/ProcessGDBRemote.cpp | 118 +++ .../Process/scripted/ScriptedThread.cpp | 9 + lldb/source/Target/Thread.cpp | 17 +- .../TestConsecutiveBreakpoints.py | 26 +- .../TestStepOverBreakpoint.py | 6 +- 8 files changed, 235 insertions(+), 282 deletions(-) diff --git a/lldb/include/lldb/Target/Thread.h b/lldb/include/lldb/Target/Thread.h index c17bddf4d98b8..1e1aead896018 100644 --- a/lldb/include/lldb/Target/Thread.h +++
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
jasonmolenda wrote: Hi @AlexK0 I restructured this PR to track everything with a single piece of state in the Thread object, and redid all three Process plugins. I'm feeling a lot better about my changes to ProcessWindows now. I haven't tested the aarch64 ubuntu yet, I will do that tomorrow and if that looks good, I'm going to ask for more careful reviews of the PR. No rush at all, but if you have a chance to try this version on Windows I would really appreciate it. Thanks! https://github.com/llvm/llvm-project/pull/96260 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
https://github.com/jasonmolenda updated https://github.com/llvm/llvm-project/pull/96260 >From 9b541e6a035635e26c6a24eca022de8552fa4c17 Mon Sep 17 00:00:00 2001 From: Jason Molenda Date: Thu, 20 Jun 2024 17:53:17 -0700 Subject: [PATCH 1/4] [lldb] Change lldb's breakpoint handling behavior lldb today has two rules: When a thread stops at a BreakpointSite, we set the thread's StopReason to be "breakpoint hit" (regardless if we've actually hit the breakpoint, or if we've merely stopped *at* the breakpoint instruction/point and haven't tripped it yet). And second, when resuming a process, any thread sitting at a BreakpointSite is silently stepped over the BreakpointSite -- because we've already flagged the breakpoint hit when we stopped there originally. In this patch, I change lldb to only set a thread's stop reason to breakpoint-hit when we've actually executed the instruction/triggered the breakpoint. When we resume, we only silently step past a BreakpointSite that we've registered as hit. We preserve this state across inferior function calls that the user may do while stopped, etc. Also, when a user adds a new breakpoint at $pc while stopped, or changes $pc to be the address of a BreakpointSite, we will silently step past that breakpoint when the process resumes. This is purely a UX call, I don't think there's any person who wants to set a breakpoint at $pc and then hit it immediately on resuming. One non-intuitive UX from this change, but I'm convinced it is necessary: If you're stopped at a BreakpointSite that has not yet executed, you `stepi`, you will hit the breakpoint and the pc will not yet advance. This thread has not completed its stepi, and the thread plan is still on the stack. If you then `continue` the thread, lldb will now stop and say, "instruction step completed", one instruction past the BreakpointSite. You can continue a second time to resume execution. I discussed this with Jim, and trying to paper over this behavior will lead to more complicated scenarios behaving non-intuitively. And mostly it's the testsuite that was trying to instruction step past a breakpoint and getting thrown off -- and I changed those tests to expect the new behavior. The bugs driving this change are all from lldb dropping the real stop reason for a thread and setting it to breakpoint-hit when that was not the case. Jim hit one where we have an aarch64 watchpoint that triggers one instruction before a BreakpointSite. On this arch we are notified of the watchpoint hit after the instruction has been unrolled -- we disable the watchpoint, instruction step, re-enable the watchpoint and collect the new value. But now we're on a BreakpointSite so the watchpoint-hit stop reason is lost. Another was reported by ZequanWu in https://discourse.llvm.org/t/lldb-unable-to-break-at-start/78282 we attach to/launch a process with the pc at a BreakpointSite and misbehave. Caroline Tice mentioned it is also a problem they've had with putting a breakpoint on _dl_debug_state. The change to each Process plugin that does execution control is that 1. If we've stopped at a BreakpointSite (whether we hit it or not), we call Thread::SetThreadStoppedAtBreakpointSite(pc) to record the state at the point when the thread stopped. (so we can detect newly-added breakpoints, or when the pc is changed to an instruction that is a BreakpointSite) 2. When we have actually hit a breakpoint, and it is enabled for this thread, we call Thread::SetThreadHitBreakpointAtAddr(pc) so we know that it should be silently stepped past when we resume execution. When resuming, we silently step over a breakpoint if we've hit it, or if it is newly added (or the pc was changed to an existing BreakpointSite). The biggest set of changes is to StopInfoMachException where we translate a Mach Exception into a stop reason. The Mach exception codes differ in a few places depending on the target (unambiguously), and I didn't want to duplicate the new code for each target so I've tested what mach exceptions we get for each action on each target, and reorganized StopInfoMachException::CreateStopReasonWithMachException to document these possible values, and handle them without specializing based on the target arch. rdar://123942164 --- lldb/include/lldb/Target/Thread.h | 29 ++ .../Process/Utility/StopInfoMachException.cpp | 296 +++--- .../Process/Windows/Common/ProcessWindows.cpp | 16 +- .../Process/gdb-remote/ProcessGDBRemote.cpp | 118 +++ .../Process/scripted/ScriptedThread.cpp | 9 + lldb/source/Target/Thread.cpp | 17 +- .../TestConsecutiveBreakpoints.py | 26 +- .../TestStepOverBreakpoint.py | 6 +- 8 files changed, 235 insertions(+), 282 deletions(-) diff --git a/lldb/include/lldb/Target/Thread.h b/lldb/include/lldb/Target/Thread.h index c17bddf4d98b8..1e1aead896018 100644 --- a/lldb/include/lldb/Target/Thread.h +++
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
labath wrote: Thanks for the explanation, Jason. I believe I understand the problem now. Here, we're concerned with whether the physical breakpoint opcode got executed, and not about the higher level machinery (conditional breakpoints, ignore count, ...), which builds on top of that and can cause a breakpoint hit to "disappear". Since StopInfo tracks the "logical" stop reason of the process (after all of these filters are applied), using it to track the "physical" one would be tricky at best. > Looking at the total requirements, the rule can be condensed to: If this > thread stopped at a BreakpointSite which it did not execute, we should > execute the breakpoint on Resume. In any other case, a thread sitting at a > BreakpointSite should silently step past it and resume execution. So, IIUC, there would just be one field (the "unexecuted breakpoint site") instead of two. I'll take that, I think having just one field to think about would make it easier to understand the overall logic. https://github.com/llvm/llvm-project/pull/96260 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
jasonmolenda wrote: Thanks @AlexK0 ! After Pavel's suggestion, I need to do a minor bit of changes to the patch soon I think, for all platforms, so I'll see if this is easier to express in ProcessWindows that way. I've was thinking about @labath 's suggestion over the weekend and I agree that the way the patch is written today isn't ideal about tracking state at stop-time and resume-time, as I developed the change I had to change its behavior and I didn't rethink what I was doing. The original goal was: If we hit a breakpoint (whether it is enabled for this thread or not), when we Resume the thread, we silently instruction step past that breakpoint instruction before resuming. Then I realized the problem of "user sets a breakpoint at $pc, or changes $pc to a BreakpointSite" and I want to also silently step past the breakpoint in that case, so I added a separate variable to track that. Looking at the total requirements, the rule can be condensed to: If this thread stopped at a BreakpointSite which it did not execute, we should execute the breakpoint on Resume. In any other case, a thread sitting at a BreakpointSite should silently step past it and resume execution. So when a thread stops at a BreakpointSite that has executed (whether it is valid for this thread or not), we record nothing. When a thread stops at a BreakpointSite that has not executed, we need to record the address of that BreakpointSite. And on Resume, if the thread is still at this same address, we want to hit the breakpoint. I don't think we can store this information in the StopInfo easily, because a thread with no stop reason (e.g. a multi-threaded program that hits a breakpoint on one thread, but the others were executing normally) wouldn't have a way to record that they were sitting at a BreakpointSite that needed to be hit still. I outlined the idea of storing this data in the StopInfo to @jimingham earlier today briefly, and he agreed that we should have StopInfos around until we Resume, but I hadn't thought this through enough to account for threads with no stop reason. I'll check in with him about the details on this before I make the change, but I think I need to keep tracking this in the Thread object. https://github.com/llvm/llvm-project/pull/96260 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
AlexK0 wrote:
> Anyway, tl;dr, I believe this will fix it:
>
> ```
> --- a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
> +++ b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
> @@ -442,6 +442,7 @@ void ProcessWindows::RefreshStateAfterStop() {
> m_session_data->m_debugger->GetProcess().GetProcessId(), pc,
> site->GetID());
>
> + stop_thread->SetThreadHitBreakpointAtAddr(pc);
>if (site->ValidForThisThread(*stop_thread)) {
> LLDB_LOG(log,
> "Breakpoint site {0} is valid for this thread ({1:x}), "
> ```
>
> I'll push it right now. Sorry for my confusion, and thanks again for looking
> at it, this was a big help.
@jasonmolenda unfortunately, the test still fails :(
I did a little investigation, here is a possible patch that fixes the test:
```
diff --git a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
index 231b22f5f189..fb0404f1c4b9 100644
--- a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
+++ b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
@@ -382,7 +382,7 @@ void ProcessWindows::RefreshStateAfterStop() {
RegisterContextSP register_context = stop_thread->GetRegisterContext();
const uint64_t pc = register_context->GetPC();
BreakpointSiteSP site(GetBreakpointSiteList().FindByAddress(pc));
-if (site)
+if (site && site->ValidForThisThread(*stop_thread))
stop_thread->SetThreadStoppedAtBreakpointSite(pc);
auto *reg_ctx = static_cast(
stop_thread->GetRegisterContext().get());
```
https://github.com/llvm/llvm-project/pull/96260
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
https://github.com/jasonmolenda updated https://github.com/llvm/llvm-project/pull/96260 >From 9b541e6a035635e26c6a24eca022de8552fa4c17 Mon Sep 17 00:00:00 2001 From: Jason Molenda Date: Thu, 20 Jun 2024 17:53:17 -0700 Subject: [PATCH 1/3] [lldb] Change lldb's breakpoint handling behavior lldb today has two rules: When a thread stops at a BreakpointSite, we set the thread's StopReason to be "breakpoint hit" (regardless if we've actually hit the breakpoint, or if we've merely stopped *at* the breakpoint instruction/point and haven't tripped it yet). And second, when resuming a process, any thread sitting at a BreakpointSite is silently stepped over the BreakpointSite -- because we've already flagged the breakpoint hit when we stopped there originally. In this patch, I change lldb to only set a thread's stop reason to breakpoint-hit when we've actually executed the instruction/triggered the breakpoint. When we resume, we only silently step past a BreakpointSite that we've registered as hit. We preserve this state across inferior function calls that the user may do while stopped, etc. Also, when a user adds a new breakpoint at $pc while stopped, or changes $pc to be the address of a BreakpointSite, we will silently step past that breakpoint when the process resumes. This is purely a UX call, I don't think there's any person who wants to set a breakpoint at $pc and then hit it immediately on resuming. One non-intuitive UX from this change, but I'm convinced it is necessary: If you're stopped at a BreakpointSite that has not yet executed, you `stepi`, you will hit the breakpoint and the pc will not yet advance. This thread has not completed its stepi, and the thread plan is still on the stack. If you then `continue` the thread, lldb will now stop and say, "instruction step completed", one instruction past the BreakpointSite. You can continue a second time to resume execution. I discussed this with Jim, and trying to paper over this behavior will lead to more complicated scenarios behaving non-intuitively. And mostly it's the testsuite that was trying to instruction step past a breakpoint and getting thrown off -- and I changed those tests to expect the new behavior. The bugs driving this change are all from lldb dropping the real stop reason for a thread and setting it to breakpoint-hit when that was not the case. Jim hit one where we have an aarch64 watchpoint that triggers one instruction before a BreakpointSite. On this arch we are notified of the watchpoint hit after the instruction has been unrolled -- we disable the watchpoint, instruction step, re-enable the watchpoint and collect the new value. But now we're on a BreakpointSite so the watchpoint-hit stop reason is lost. Another was reported by ZequanWu in https://discourse.llvm.org/t/lldb-unable-to-break-at-start/78282 we attach to/launch a process with the pc at a BreakpointSite and misbehave. Caroline Tice mentioned it is also a problem they've had with putting a breakpoint on _dl_debug_state. The change to each Process plugin that does execution control is that 1. If we've stopped at a BreakpointSite (whether we hit it or not), we call Thread::SetThreadStoppedAtBreakpointSite(pc) to record the state at the point when the thread stopped. (so we can detect newly-added breakpoints, or when the pc is changed to an instruction that is a BreakpointSite) 2. When we have actually hit a breakpoint, and it is enabled for this thread, we call Thread::SetThreadHitBreakpointAtAddr(pc) so we know that it should be silently stepped past when we resume execution. When resuming, we silently step over a breakpoint if we've hit it, or if it is newly added (or the pc was changed to an existing BreakpointSite). The biggest set of changes is to StopInfoMachException where we translate a Mach Exception into a stop reason. The Mach exception codes differ in a few places depending on the target (unambiguously), and I didn't want to duplicate the new code for each target so I've tested what mach exceptions we get for each action on each target, and reorganized StopInfoMachException::CreateStopReasonWithMachException to document these possible values, and handle them without specializing based on the target arch. rdar://123942164 --- lldb/include/lldb/Target/Thread.h | 29 ++ .../Process/Utility/StopInfoMachException.cpp | 296 +++--- .../Process/Windows/Common/ProcessWindows.cpp | 16 +- .../Process/gdb-remote/ProcessGDBRemote.cpp | 118 +++ .../Process/scripted/ScriptedThread.cpp | 9 + lldb/source/Target/Thread.cpp | 17 +- .../TestConsecutiveBreakpoints.py | 26 +- .../TestStepOverBreakpoint.py | 6 +- 8 files changed, 235 insertions(+), 282 deletions(-) diff --git a/lldb/include/lldb/Target/Thread.h b/lldb/include/lldb/Target/Thread.h index c17bddf4d98b8..1e1aead896018 100644 --- a/lldb/include/lldb/Target/Thread.h +++
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
jasonmolenda wrote:
> > @AlexK0 if you have a setup to build and run the testsuite on windows,
> > could you try it with this patch some time when you're able? I can't see
> > this being merged for at least another 5-6 days, there's no rush. You can
> > download the unidiff style diff of the patch from
> > https://patch-diff.githubusercontent.com/raw/llvm/llvm-project/pull/96260.diff
>
> @jasonmolenda I checked the tests with VS2022/x86-64/win11 in a debug build
> with assertions enabled. Unfortunately, the main branch is a bit unstable,
> and some tests constantly fail :( . Nonetheless, I noticed one new failure
> with the applied patch:
>
> `functionalities/breakpoint/consecutive_breakpoints/TestConsecutiveBreakpoints.py`
>
These TestConcurrent / TestConsecutiveBreakpoints tests are great for finding
corner cases with these changes, thanks so much. I was a little uncertain
about one part of my ProcessWindows change, where the pc is *past* the
breakpoint when a software breakpoint instruction is used, on Windows. For a
moment, I thought, "oh, I don't need to record that we hit the breakpoint
because we're already past it and we don't need to instruction past it" but of
course that was wrong -- ProcessWindows::RefreshStateAfterStop decrements the
$pc value by the size of its breakpoint instruction, which is necessary because
e.g. the size of an x86 breakpoint instruction is 1 byte (0xcc) but x86_64
instructions can be between 1 to 15 bytes long, so stopping 1 byte after the
BreakpointSite means we are possibly in the middle of the real instruction. We
must set the pc to the BreakpointSite address and use lldb's normal logic.
Anyway, tl;dr, I believe this will fix it:
```
--- a/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
+++ b/lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
@@ -442,6 +442,7 @@ void ProcessWindows::RefreshStateAfterStop() {
m_session_data->m_debugger->GetProcess().GetProcessId(), pc,
site->GetID());
+ stop_thread->SetThreadHitBreakpointAtAddr(pc);
if (site->ValidForThisThread(*stop_thread)) {
LLDB_LOG(log,
"Breakpoint site {0} is valid for this thread ({1:x}), "
```
I'll push it right now. Sorry for my confusion, and thanks again for looking
at it, this was a big help.
https://github.com/llvm/llvm-project/pull/96260
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
AlexK0 wrote: > @AlexK0 if you have a setup to build and run the testsuite on windows, could > you try it with this patch some time when you're able? I can't see this being > merged for at least another 5-6 days, there's no rush. You can download the > unidiff style diff of the patch from > https://patch-diff.githubusercontent.com/raw/llvm/llvm-project/pull/96260.diff @jasonmolenda I checked the tests with VS2022/x86-64/win11 in a debug build with assertions enabled. Unfortunately, the main branch is a bit unstable, and some tests constantly fail :( . Nonetheless, I noticed one new failure with the applied patch: `functionalities/breakpoint/consecutive_breakpoints/TestConsecutiveBreakpoints.py` backtrace: ``` FAIL: test_single_step_thread_specific (TestConsecutiveBreakpoints.ConsecutiveBreakpointsTestCase) Test that single step stops, even though the second breakpoint is not valid. -- Traceback (most recent call last): File "D:\Projects\github\llvm-project-jasonmolenda\lldb\packages\Python\lldbsuite\test\decorators.py", line 451, in wrapper return func(self, *args, **kwargs) File "D:\Projects\github\llvm-project-jasonmolenda\lldb\test\API\functionalities\breakpoint\consecutive_breakpoints\TestConsecutiveBreakpoints.py", line 121, in test_single_step_thread_specific self.finish_test() File "D:\Projects\github\llvm-project-jasonmolenda\lldb\test\API\functionalities\breakpoint\consecutive_breakpoints\TestConsecutiveBreakpoints.py", line 42, in finish_test self.assertState(self.process.GetState(), lldb.eStateExited) File "D:\Projects\github\llvm-project-jasonmolenda\lldb\packages\Python\lldbsuite\test\lldbtest.py", line 2578, in assertState self.fail(self._formatMessage(msg, error)) AssertionError: stopped (5) != exited (10) Config=x86_64-D:\Projects\github\llvm-project-jasonmolenda\build\bin\clang.exe -- ``` https://github.com/llvm/llvm-project/pull/96260 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
@@ -377,6 +382,19 @@ class Thread : public std::enable_shared_from_this,
virtual void SetQueueLibdispatchQueueAddress(lldb::addr_t dispatch_queue_t)
{}
+ /// When a thread has executed/trapped a breakpoint, set the address of that
+ /// breakpoint so we know it has been hit already, and should be silently
+ /// stepped past on resume.
+ void SetThreadHitBreakpointAtAddr(lldb::addr_t pc) { m_hit_bp_at_addr = pc; }
+
+ /// When a thread stops at a breakpoint instruction/address, but has not yet
+ /// executed/triggered it, record that so we can detect when a user adds a
+ /// breakpoint (or changes a thread to a breakpoint site) and we need to
+ /// silently step past that when resuming.
+ void SetThreadStoppedAtBreakpointSite(lldb::addr_t pc) {
+m_bpsite_at_stop_pc = pc;
+ }
labath wrote:
I'm also finding it hard to wrap my head around the meaning of this variable.
If I understand correctly it tells us: the pc that we've stopped at; that there
was a breakpoint site there at the time of the stop; and we did not hit that
site.
I'm wondering if it would be clearer if we unpacked that. What if we:
- unconditionally stored the PC of the last stop. Maybe this could be even a
part of the StopInfo class, as I think it could be useful to see the PC value
at the time of the stop, even if the user changed the PC afterwards.
- a flag indicating whether we stopped at a breakpoint site, regardless of
whether we've hit it or not (per the previous comment, that could be indicated
by the stop reason). This doesn't really look like it belongs to StopInfo
class, but I think I'd be fine with putting it there for collocation purposes.
https://github.com/llvm/llvm-project/pull/96260
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
https://github.com/labath commented: I like this change a lot, but I'm wondering if there's a clearer way to store the intermediate state. It seems like this info could go into the StopInfo class, which would have the benefit of automatically being backed up when doing expression evaluation. Is there a reason to not do that? If the reason is that the StopInfo of the thread has been cleared by the time we call SetupForResume (and there's no good way to change that), maybe we could pass the old stop info to the function as an argument, since that's effectively what the function is doing -- deciding how to resume the thread based on how it was previously stopped. Regarding the scenarios you mention in the description (moving the PC to a breakpoint site, setting a breakpoint site at the current pc), are these tested by any of the existing tests? > Caroline Tice mentioned it is also a problem they've had with putting a > breakpoint on _dl_debug_state I believe this was fixed by 60b90b523323f8196a9e4a68b1f33358624c09eb. https://github.com/llvm/llvm-project/pull/96260 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
@@ -377,6 +382,19 @@ class Thread : public std::enable_shared_from_this,
virtual void SetQueueLibdispatchQueueAddress(lldb::addr_t dispatch_queue_t)
{}
+ /// When a thread has executed/trapped a breakpoint, set the address of that
+ /// breakpoint so we know it has been hit already, and should be silently
+ /// stepped past on resume.
labath wrote:
I'm a little unclear as to why do we need to store this separately. Shouldn't
this already be stored in the stop reason of the thread (i.e.,
StopInfoBreakpoint implies we've hit a breakpoint, and the breakpoint site
within it should give us the PC value)?
https://github.com/llvm/llvm-project/pull/96260
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
https://github.com/labath edited https://github.com/llvm/llvm-project/pull/96260 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
https://github.com/jasonmolenda updated https://github.com/llvm/llvm-project/pull/96260 >From 9b541e6a035635e26c6a24eca022de8552fa4c17 Mon Sep 17 00:00:00 2001 From: Jason Molenda Date: Thu, 20 Jun 2024 17:53:17 -0700 Subject: [PATCH 1/2] [lldb] Change lldb's breakpoint handling behavior lldb today has two rules: When a thread stops at a BreakpointSite, we set the thread's StopReason to be "breakpoint hit" (regardless if we've actually hit the breakpoint, or if we've merely stopped *at* the breakpoint instruction/point and haven't tripped it yet). And second, when resuming a process, any thread sitting at a BreakpointSite is silently stepped over the BreakpointSite -- because we've already flagged the breakpoint hit when we stopped there originally. In this patch, I change lldb to only set a thread's stop reason to breakpoint-hit when we've actually executed the instruction/triggered the breakpoint. When we resume, we only silently step past a BreakpointSite that we've registered as hit. We preserve this state across inferior function calls that the user may do while stopped, etc. Also, when a user adds a new breakpoint at $pc while stopped, or changes $pc to be the address of a BreakpointSite, we will silently step past that breakpoint when the process resumes. This is purely a UX call, I don't think there's any person who wants to set a breakpoint at $pc and then hit it immediately on resuming. One non-intuitive UX from this change, but I'm convinced it is necessary: If you're stopped at a BreakpointSite that has not yet executed, you `stepi`, you will hit the breakpoint and the pc will not yet advance. This thread has not completed its stepi, and the thread plan is still on the stack. If you then `continue` the thread, lldb will now stop and say, "instruction step completed", one instruction past the BreakpointSite. You can continue a second time to resume execution. I discussed this with Jim, and trying to paper over this behavior will lead to more complicated scenarios behaving non-intuitively. And mostly it's the testsuite that was trying to instruction step past a breakpoint and getting thrown off -- and I changed those tests to expect the new behavior. The bugs driving this change are all from lldb dropping the real stop reason for a thread and setting it to breakpoint-hit when that was not the case. Jim hit one where we have an aarch64 watchpoint that triggers one instruction before a BreakpointSite. On this arch we are notified of the watchpoint hit after the instruction has been unrolled -- we disable the watchpoint, instruction step, re-enable the watchpoint and collect the new value. But now we're on a BreakpointSite so the watchpoint-hit stop reason is lost. Another was reported by ZequanWu in https://discourse.llvm.org/t/lldb-unable-to-break-at-start/78282 we attach to/launch a process with the pc at a BreakpointSite and misbehave. Caroline Tice mentioned it is also a problem they've had with putting a breakpoint on _dl_debug_state. The change to each Process plugin that does execution control is that 1. If we've stopped at a BreakpointSite (whether we hit it or not), we call Thread::SetThreadStoppedAtBreakpointSite(pc) to record the state at the point when the thread stopped. (so we can detect newly-added breakpoints, or when the pc is changed to an instruction that is a BreakpointSite) 2. When we have actually hit a breakpoint, and it is enabled for this thread, we call Thread::SetThreadHitBreakpointAtAddr(pc) so we know that it should be silently stepped past when we resume execution. When resuming, we silently step over a breakpoint if we've hit it, or if it is newly added (or the pc was changed to an existing BreakpointSite). The biggest set of changes is to StopInfoMachException where we translate a Mach Exception into a stop reason. The Mach exception codes differ in a few places depending on the target (unambiguously), and I didn't want to duplicate the new code for each target so I've tested what mach exceptions we get for each action on each target, and reorganized StopInfoMachException::CreateStopReasonWithMachException to document these possible values, and handle them without specializing based on the target arch. rdar://123942164 --- lldb/include/lldb/Target/Thread.h | 29 ++ .../Process/Utility/StopInfoMachException.cpp | 296 +++--- .../Process/Windows/Common/ProcessWindows.cpp | 16 +- .../Process/gdb-remote/ProcessGDBRemote.cpp | 118 +++ .../Process/scripted/ScriptedThread.cpp | 9 + lldb/source/Target/Thread.cpp | 17 +- .../TestConsecutiveBreakpoints.py | 26 +- .../TestStepOverBreakpoint.py | 6 +- 8 files changed, 235 insertions(+), 282 deletions(-) diff --git a/lldb/include/lldb/Target/Thread.h b/lldb/include/lldb/Target/Thread.h index c17bddf4d98b8..1e1aead896018 100644 --- a/lldb/include/lldb/Target/Thread.h +++
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
https://github.com/jasonmolenda edited https://github.com/llvm/llvm-project/pull/96260 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
jasonmolenda wrote: This patch definitely needs review from Jim Ingham, who is out this week. @ZequanWu and @cmtice FYI. I tested this on arm64 macOS and on aarch64 Ubuntu - the important Process changes are were in ProcessGDBRemote, StopInfoMachException, and in ProcessWindows, I believe my testing will have exercised the first two set of changes (the largest and most complex). I updated ProcessWindows as I believe it needs to be updated, but don't have a way of testing it short of merging the PR and seeing what happens to the windows CI bot. @AlexK0 if you have a setup to build and run the testsuite on windows, could you try it with this patch some time when you're able? I can't see this being merged for at least another 5-6 days, there's no rush. You can download the unidiff style diff of the patch from https://patch-diff.githubusercontent.com/raw/llvm/llvm-project/pull/96260.diff https://github.com/llvm/llvm-project/pull/96260 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Jason Molenda (jasonmolenda) Changes lldb today has two rules: When a thread stops at a BreakpointSite, we set the thread's StopReason to be "breakpoint hit" (regardless if we've actually hit the breakpoint, or if we've merely stopped *at* the breakpoint instruction/point and haven't tripped it yet). And second, when resuming a process, any thread sitting at a BreakpointSite is silently stepped over the BreakpointSite -- because we've already flagged the breakpoint hit when we stopped there originally. In this patch, I change lldb to only set a thread's stop reason to breakpoint-hit when we've actually executed the instruction/triggered the breakpoint. When we resume, we only silently step past a BreakpointSite that we've registered as hit. We preserve this state across inferior function calls that the user may do while stopped, etc. Also, when a user adds a new breakpoint at $pc while stopped, or changes $pc to be the address of a BreakpointSite, we will silently step past that breakpoint when the process resumes. This is purely a UX call, I don't think there's any person who wants to set a breakpoint at $pc and then hit it immediately on resuming. One non-intuitive UX from this change, but I'm convinced it is necessary: If you're stopped at a BreakpointSite that has not yet executed, you `stepi`, you will hit the breakpoint and the pc will not yet advance. This thread has not completed its stepi, and the thread plan is still on the stack. If you then `continue` the thread, lldb will now stop and say, "instruction step completed", one instruction past the BreakpointSite. You can continue a second time to resume execution. I discussed this with Jim, and trying to paper over this behavior will lead to more complicated scenarios behaving non-intuitively. And mostly it's the testsuite that was trying to instruction step past a breakpoint and getting thrown off -- and I changed those tests to expect the new behavior. The bugs driving this change are all from lldb dropping the real stop reason for a thread and setting it to breakpoint-hit when that was not the case. Jim hit one where we have an aarch64 watchpoint that triggers one instruction before a BreakpointSite. On this arch we are notified of the watchpoint hit after the instruction has been unrolled -- we disable the watchpoint, instruction step, re-enable the watchpoint and collect the new value. But now we're on a BreakpointSite so the watchpoint-hit stop reason is lost. Another was reported by ZequanWu in https://discourse.llvm.org/t/lldb-unable-to-break-at-start/78282 we attach to/launch a process with the pc at a BreakpointSite and misbehave. Caroline Tice mentioned it is also a problem they've had with putting a breakpoint on _dl_debug_state. The change to each Process plugin that does execution control is that 1. If we've stopped at a BreakpointSite (whether we hit it or not), we call Thread::SetThreadStoppedAtBreakpointSite(pc) to record the state at the point when the thread stopped. (so we can detect newly-added breakpoints, or when the pc is changed to an instruction that is a BreakpointSite) 2. When we have actually hit a breakpoint, and it is enabled for this thread, we call Thread::SetThreadHitBreakpointAtAddr(pc) so we know that it should be silently stepped past when we resume execution. When resuming, we silently step over a breakpoint if we've hit it, or if it is newly added (or the pc was changed to an existing BreakpointSite). The biggest set of changes is to StopInfoMachException where we translate a Mach Exception into a stop reason. The Mach exception codes differ in a few places depending on the target (unambiguously), and I didn't want to duplicate the new code for each target so I've tested what mach exceptions we get for each action on each target, and reorganized StopInfoMachException::CreateStopReasonWithMachException to document these possible values, and handle them without specializing based on the target arch. rdar://123942164 --- Patch is 33.48 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/96260.diff 8 Files Affected: - (modified) lldb/include/lldb/Target/Thread.h (+29) - (modified) lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp (+118-178) - (modified) lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp (+3-13) - (modified) lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (+36-82) - (modified) lldb/source/Plugins/Process/scripted/ScriptedThread.cpp (+9) - (modified) lldb/source/Target/Thread.cpp (+16-1) - (modified) lldb/test/API/functionalities/breakpoint/consecutive_breakpoints/TestConsecutiveBreakpoints.py (+19-7) - (modified) lldb/test/API/functionalities/breakpoint/step_over_breakpoint/TestStepOverBreakpoint.py (+5-1) ``diff diff --git a/lldb/include/lldb/Target/Thread.h
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
https://github.com/jasonmolenda created https://github.com/llvm/llvm-project/pull/96260 lldb today has two rules: When a thread stops at a BreakpointSite, we set the thread's StopReason to be "breakpoint hit" (regardless if we've actually hit the breakpoint, or if we've merely stopped *at* the breakpoint instruction/point and haven't tripped it yet). And second, when resuming a process, any thread sitting at a BreakpointSite is silently stepped over the BreakpointSite -- because we've already flagged the breakpoint hit when we stopped there originally. In this patch, I change lldb to only set a thread's stop reason to breakpoint-hit when we've actually executed the instruction/triggered the breakpoint. When we resume, we only silently step past a BreakpointSite that we've registered as hit. We preserve this state across inferior function calls that the user may do while stopped, etc. Also, when a user adds a new breakpoint at $pc while stopped, or changes $pc to be the address of a BreakpointSite, we will silently step past that breakpoint when the process resumes. This is purely a UX call, I don't think there's any person who wants to set a breakpoint at $pc and then hit it immediately on resuming. One non-intuitive UX from this change, but I'm convinced it is necessary: If you're stopped at a BreakpointSite that has not yet executed, you `stepi`, you will hit the breakpoint and the pc will not yet advance. This thread has not completed its stepi, and the thread plan is still on the stack. If you then `continue` the thread, lldb will now stop and say, "instruction step completed", one instruction past the BreakpointSite. You can continue a second time to resume execution. I discussed this with Jim, and trying to paper over this behavior will lead to more complicated scenarios behaving non-intuitively. And mostly it's the testsuite that was trying to instruction step past a breakpoint and getting thrown off -- and I changed those tests to expect the new behavior. The bugs driving this change are all from lldb dropping the real stop reason for a thread and setting it to breakpoint-hit when that was not the case. Jim hit one where we have an aarch64 watchpoint that triggers one instruction before a BreakpointSite. On this arch we are notified of the watchpoint hit after the instruction has been unrolled -- we disable the watchpoint, instruction step, re-enable the watchpoint and collect the new value. But now we're on a BreakpointSite so the watchpoint-hit stop reason is lost. Another was reported by ZequanWu in https://discourse.llvm.org/t/lldb-unable-to-break-at-start/78282 we attach to/launch a process with the pc at a BreakpointSite and misbehave. Caroline Tice mentioned it is also a problem they've had with putting a breakpoint on _dl_debug_state. The change to each Process plugin that does execution control is that 1. If we've stopped at a BreakpointSite (whether we hit it or not), we call Thread::SetThreadStoppedAtBreakpointSite(pc) to record the state at the point when the thread stopped. (so we can detect newly-added breakpoints, or when the pc is changed to an instruction that is a BreakpointSite) 2. When we have actually hit a breakpoint, and it is enabled for this thread, we call Thread::SetThreadHitBreakpointAtAddr(pc) so we know that it should be silently stepped past when we resume execution. When resuming, we silently step over a breakpoint if we've hit it, or if it is newly added (or the pc was changed to an existing BreakpointSite). The biggest set of changes is to StopInfoMachException where we translate a Mach Exception into a stop reason. The Mach exception codes differ in a few places depending on the target (unambiguously), and I didn't want to duplicate the new code for each target so I've tested what mach exceptions we get for each action on each target, and reorganized StopInfoMachException::CreateStopReasonWithMachException to document these possible values, and handle them without specializing based on the target arch. rdar://123942164 >From 9b541e6a035635e26c6a24eca022de8552fa4c17 Mon Sep 17 00:00:00 2001 From: Jason Molenda Date: Thu, 20 Jun 2024 17:53:17 -0700 Subject: [PATCH] [lldb] Change lldb's breakpoint handling behavior lldb today has two rules: When a thread stops at a BreakpointSite, we set the thread's StopReason to be "breakpoint hit" (regardless if we've actually hit the breakpoint, or if we've merely stopped *at* the breakpoint instruction/point and haven't tripped it yet). And second, when resuming a process, any thread sitting at a BreakpointSite is silently stepped over the BreakpointSite -- because we've already flagged the breakpoint hit when we stopped there originally. In this patch, I change lldb to only set a thread's stop reason to breakpoint-hit when we've actually executed the instruction/triggered the breakpoint. When we resume, we only silently step past a BreakpointSite
