aboutsummaryrefslogtreecommitdiff
path: root/lldb/source/Commands/CommandObjectWatchpoint.cpp
AgeCommit message (Collapse)AuthorFilesLines
2025-06-02[lldb] Refactor away UB in SBValue::GetLoadAddress (#141799)Pavel Labath1-14/+11
The problem was in calling GetLoadAddress on a value in the error state, where `ValueObject::GetLoadAddress` could end up accessing the uninitialized "address type" by-ref return value from `GetAddressOf`. This probably happened because each function expected the other to initialize it. We can guarantee initialization by turning this into a proper return value. I've added a test, but it only (reliably) crashes if lldb is built with ubsan.
2025-03-05[lldb] Upgrade CompilerType::GetBitSize to return llvm::Expected (#129601)Adrian Prantl1-4/+6
This patch pushes the error handling boundary for the GetBitSize() methods from Runtime into the Type and CompilerType APIs. This makes it easier to diagnose problems thanks to more meaningful error messages being available. GetBitSize() is often the first thing LLDB asks about a type, so this method is particularly important for a better user experience. rdar://145667239
2024-10-24[lldb] Move ValueObject into its own library (NFC) (#113393)Jonas Devlieghere1-1/+1
ValueObject is part of lldbCore for historical reasons, but conceptually it deserves to be its own library. This does introduce a (link-time) circular dependency between lldbCore and lldbValueObject, which is unfortunate but probably unavoidable because so many things in LLDB rely on ValueObject. We already have cycles and these libraries are never built as dylibs so while this doesn't improve the situation, it also doesn't make things worse. The header includes were updated with the following command: ``` find . -type f -exec sed -i.bak "s%include \"lldb/Core/ValueObject%include \"lldb/ValueObject/ValueObject%" '{}' \; ```
2024-08-27[lldb] Turn lldb_private::Status into a value type. (#106163)Adrian Prantl1-2/+2
This patch removes all of the Set.* methods from Status. This cleanup is part of a series of patches that make it harder use the anti-pattern of keeping a long-lives Status object around and updating it while dropping any errors it contains on the floor. This patch is largely NFC, the more interesting next steps this enables is to: 1. remove Status.Clear() 2. assert that Status::operator=() never overwrites an error 3. remove Status::operator=() Note that step (2) will bring 90% of the benefits for users, and step (3) will dramatically clean up the error handling code in various places. In the end my goal is to convert all APIs that are of the form ` ResultTy DoFoo(Status& error) ` to ` llvm::Expected<ResultTy> DoFoo() ` How to read this patch? The interesting changes are in Status.h and Status.cpp, all other changes are mostly ` perl -pi -e 's/\.SetErrorString/ = Status::FromErrorString/g' $(git grep -l SetErrorString lldb/source) ` plus the occasional manual cleanup.
2024-07-31[lldb] Use Target references instead of pointers in CommandObject (NFC)Jonas Devlieghere1-48/+48
The GetTarget helper returns a Target reference so there's reason to convert it to a pointer and check its validity.
2024-07-31[lldb] Unify the way we get the Target in CommandObject (#101208)Jonas Devlieghere1-8/+8
Currently, CommandObjects are obtaining a target in a variety of ways. Often the command incorrectly operates on the selected target. As an example, when a breakpoint command is running, the current target is passed into the command but the target that hit the breakpoint is not the selected target. In other places we use the CommandObject's execution context, which is frozen during the execution of the command, and comes with its own limitations. Finally, we often want to fall back to the dummy target if no real target is available. Instead of having to guess how to get the target, this patch introduces one helper function in CommandObject to get the most relevant target. In order of priority, that's the target from the command object's execution context, from the interpreter's execution context, the selected target or the dummy target. rdar://110846511
2024-02-27Start to clean up the process of defining command arguments. (#83097)jimingham1-60/+8
Partly, there's just a lot of unnecessary boiler plate. It's also possible to define combinations of arguments that make no sense (e.g. eArgRepeatPlus followed by eArgRepeatPlain...) but these are never checked since we just push_back directly into the argument definitions. This commit is step 1 of this cleanup - do the obvious stuff. In it, all the simple homogenous argument lists and the breakpoint/watchpoint ID/Range types, are set with common functions. This is an NFC change, it just centralizes boiler plate. There's no checking yet because you can't get a single argument wrong. The end goal is that all argument definition goes through functions and m_arguments is hidden so that you can't define inconsistent argument sets.
2024-02-20Add the RegisterCompleter to eArgTypeRegisterName in g_argument_table (#82428)jimingham1-10/+0
This is a follow-on to: https://github.com/llvm/llvm-project/pull/82085 The completer for register names was missing from the argument table. I somehow missed that the only register completer test was x86_64, so that test broke. I added the completer in to the right slot in the argument table, and added a small completions test that just uses the alias register names. If we end up having a platform that doesn't define register names, we'll have to skip this test there, but it should add a sniff test for register completion that will run most everywhere.
2024-02-19Revert "Centralize the handling of completion for simple argument lists. ↵Shubham Sandeep Rastogi1-0/+10
(#82085)" This reverts commit 21631494b068d9364b8dc8f18e59adee9131a0a5. Reverted because of greendragon failure: ******************** TEST 'lldb-api :: functionalities/completion/TestCompletion.py' FAILED ******************** Script:
2024-02-19Centralize the handling of completion for simple argument lists. (#82085)jimingham1-10/+0
Most commands were adding argument completion handling by themselves, resulting in a lot of unnecessary boilerplate. In many cases, this could be done generically given the argument definition and the entries in the g_argument_table. I'm going to address this in a couple passes. In this first pass, I added handling of commands that have only one argument list, with one argument type, either single or repeated, and changed all the commands that are of this sort (and don't have other bits of business in their completers.) I also added some missing connections between arg types and completions to the table, and added a RemoteFilename and RemotePath to use in places where we were using the Remote completers. Those arguments used to say they were "files" but they were in fact remote files. I also added a module arg type to use where we were using the module completer. In that case, we should call the argument module.
2024-01-31[lldb] Add support for large watchpoints in lldb (#79962)Jason Molenda1-1/+14
This patch is the next piece of work in my Large Watchpoint proposal, https://discourse.llvm.org/t/rfc-large-watchpoint-support-in-lldb/72116 This patch breaks a user's watchpoint into one or more WatchpointResources which reflect what the hardware registers can cover. This means we can watch objects larger than 8 bytes, and we can watched unaligned address ranges. On a typical 64-bit target with 4 watchpoint registers you can watch 32 bytes of memory if the start address is doubleword aligned. Additionally, if the remote stub implements AArch64 MASK style watchpoints (e.g. debugserver on Darwin), we can watch any power-of-2 size region of memory up to 2GB, aligned to that same size. I updated the Watchpoint constructor and CommandObjectWatchpoint to create a CompilerType of Array<UInt8> when the size of the watched region is greater than pointer-size and we don't have a variable type to use. For pointer-size and smaller, we can display the watched granule as an integer value; for larger-than-pointer-size we will display as an array of bytes. I have `watchpoint list` now print the WatchpointResources used to implement the watchpoint. I added a WatchpointAlgorithm class which has a top-level static method that takes an enum flag mask WatchpointHardwareFeature and a user address and size, and returns a vector of WatchpointResources covering the request. It does not take into account the number of watchpoint registers the target has, or the number still available for use. Right now there is only one algorithm, which monitors power-of-2 regions of memory. For up to pointer-size, this is what Intel hardware supports. AArch64 Byte Address Select watchpoints can watch any number of contiguous bytes in a pointer-size memory granule, that is not currently supported so if you ask to watch bytes 3-5, the algorithm will watch the entire doubleword (8 bytes). The newly default "modify" style means we will silently ignore modifications to bytes outside the watched range. I've temporarily skipped TestLargeWatchpoint.py for all targets. It was only run on Darwin when using the in-tree debugserver, which was a proxy for "debugserver supports MASK watchpoints". I'll be adding the aforementioned feature flag from the stub and enabling full mask watchpoints when a debugserver with that feature is enabled, and re-enable this test. I added a new TestUnalignedLargeWatchpoint.py which only has one test but it's a great one, watching a 22-byte range that is unaligned and requires four 8-byte watchpoints to cover. I also added a unit test, WatchpointAlgorithmsTests, which has a number of simple tests against WatchpointAlgorithms::PowerOf2Watchpoints. I think there's interesting possible different approaches to how we cover these; I note in the unit test that a user requesting a watch on address 0x12e0 of 120 bytes will be covered by two watchpoints today, a 128-bytes at 0x1280 and at 0x1300. But it could be done with a 16-byte watchpoint at 0x12e0 and a 128-byte at 0x1300, which would have fewer false positives/private stops. As we try refining this one, it's helpful to have a collection of tests to make sure things don't regress. I tested this on arm64 macOS, (genuine) x86_64 macOS, and AArch64 Ubuntu. I have not modifed the Windows process plugins yet, I might try that as a standalone patch, I'd be making the change blind, but the necessary changes (see ProcessGDBRemote::EnableWatchpoint) are pretty small so it might be obvious enough that I can change it and see what the Windows CI thinks. There isn't yet a packet (or a qSupported feature query) for the gdb remote serial protocol stub to communicate its watchpoint capabilities to lldb. I'll be doing that in a patch right after this is landed, having debugserver advertise its capability of AArch64 MASK watchpoints, and have ProcessGDBRemote add eWatchpointHardwareArmMASK to WatchpointAlgorithms so we can watch larger than 32-byte requests on Darwin. I haven't yet tackled WatchpointResource *sharing* by multiple Watchpoints. This is all part of the goal, especially when we may be watching a larger memory range than the user requested, if they then add another watchpoint next to their first request, it may be covered by the same WatchpointResource (hardware watchpoint register). Also one "read" watchpoint and one "write" watchpoint on the same memory granule need to be handled, making the WatchpointResource cover all requests. As WatchpointResources aren't shared among multiple Watchpoints yet, there's no handling of running the conditions/commands/etc on multiple Watchpoints when their shared WatchpointResource is hit. The goal beyond "large watchpoint" is to unify (much more) the Watchpoint and Breakpoint behavior and commands. I have a feeling I may be slowly chipping away at this for a while. Re-landing this patch after fixing two undefined behaviors in WatchpointAlgorithms found by UBSan and by failures on different CI bots. rdar://108234227
2024-01-31Revert "[lldb] Add support for large watchpoints in lldb (#79962)"Jason Molenda1-14/+1
This reverts commit 57c66b35a885b571f9897d75d18f1d974c29e533.
2024-01-31[lldb] Add support for large watchpoints in lldb (#79962)Jason Molenda1-1/+14
This patch is the next piece of work in my Large Watchpoint proposal, https://discourse.llvm.org/t/rfc-large-watchpoint-support-in-lldb/72116 This patch breaks a user's watchpoint into one or more WatchpointResources which reflect what the hardware registers can cover. This means we can watch objects larger than 8 bytes, and we can watched unaligned address ranges. On a typical 64-bit target with 4 watchpoint registers you can watch 32 bytes of memory if the start address is doubleword aligned. Additionally, if the remote stub implements AArch64 MASK style watchpoints (e.g. debugserver on Darwin), we can watch any power-of-2 size region of memory up to 2GB, aligned to that same size. I updated the Watchpoint constructor and CommandObjectWatchpoint to create a CompilerType of Array<UInt8> when the size of the watched region is greater than pointer-size and we don't have a variable type to use. For pointer-size and smaller, we can display the watched granule as an integer value; for larger-than-pointer-size we will display as an array of bytes. I have `watchpoint list` now print the WatchpointResources used to implement the watchpoint. I added a WatchpointAlgorithm class which has a top-level static method that takes an enum flag mask WatchpointHardwareFeature and a user address and size, and returns a vector of WatchpointResources covering the request. It does not take into account the number of watchpoint registers the target has, or the number still available for use. Right now there is only one algorithm, which monitors power-of-2 regions of memory. For up to pointer-size, this is what Intel hardware supports. AArch64 Byte Address Select watchpoints can watch any number of contiguous bytes in a pointer-size memory granule, that is not currently supported so if you ask to watch bytes 3-5, the algorithm will watch the entire doubleword (8 bytes). The newly default "modify" style means we will silently ignore modifications to bytes outside the watched range. I've temporarily skipped TestLargeWatchpoint.py for all targets. It was only run on Darwin when using the in-tree debugserver, which was a proxy for "debugserver supports MASK watchpoints". I'll be adding the aforementioned feature flag from the stub and enabling full mask watchpoints when a debugserver with that feature is enabled, and re-enable this test. I added a new TestUnalignedLargeWatchpoint.py which only has one test but it's a great one, watching a 22-byte range that is unaligned and requires four 8-byte watchpoints to cover. I also added a unit test, WatchpointAlgorithmsTests, which has a number of simple tests against WatchpointAlgorithms::PowerOf2Watchpoints. I think there's interesting possible different approaches to how we cover these; I note in the unit test that a user requesting a watch on address 0x12e0 of 120 bytes will be covered by two watchpoints today, a 128-bytes at 0x1280 and at 0x1300. But it could be done with a 16-byte watchpoint at 0x12e0 and a 128-byte at 0x1300, which would have fewer false positives/private stops. As we try refining this one, it's helpful to have a collection of tests to make sure things don't regress. I tested this on arm64 macOS, (genuine) x86_64 macOS, and AArch64 Ubuntu. I have not modifed the Windows process plugins yet, I might try that as a standalone patch, I'd be making the change blind, but the necessary changes (see ProcessGDBRemote::EnableWatchpoint) are pretty small so it might be obvious enough that I can change it and see what the Windows CI thinks. There isn't yet a packet (or a qSupported feature query) for the gdb remote serial protocol stub to communicate its watchpoint capabilities to lldb. I'll be doing that in a patch right after this is landed, having debugserver advertise its capability of AArch64 MASK watchpoints, and have ProcessGDBRemote add eWatchpointHardwareArmMASK to WatchpointAlgorithms so we can watch larger than 32-byte requests on Darwin. I haven't yet tackled WatchpointResource *sharing* by multiple Watchpoints. This is all part of the goal, especially when we may be watching a larger memory range than the user requested, if they then add another watchpoint next to their first request, it may be covered by the same WatchpointResource (hardware watchpoint register). Also one "read" watchpoint and one "write" watchpoint on the same memory granule need to be handled, making the WatchpointResource cover all requests. As WatchpointResources aren't shared among multiple Watchpoints yet, there's no handling of running the conditions/commands/etc on multiple Watchpoints when their shared WatchpointResource is hit. The goal beyond "large watchpoint" is to unify (much more) the Watchpoint and Breakpoint behavior and commands. I have a feeling I may be slowly chipping away at this for a while. rdar://108234227
2023-11-30[lldb] [mostly NFC] Large WP foundation: WatchpointResources (#68845)Jason Molenda1-4/+4
This patch is rearranging code a bit to add WatchpointResources to Process. A WatchpointResource is meant to represent a hardware watchpoint register in the inferior process. It has an address, a size, a type, and a list of Watchpoints that are using this WatchpointResource. This current patch doesn't add any of the features of WatchpointResources that make them interesting -- a user asking to watch a 24 byte object could watch this with three 8 byte WatchpointResources. Or a Watchpoint on 1 byte at 0x1002 and a second watchpoint on 1 byte at 0x1003, these must both be served by a single WatchpointResource on that doubleword at 0x1000 on a 64-bit target, if two hardware watchpoint registers were used to track these separately, one of them may not be hit. Or if you have one Watchpoint on a variable with a condition set, and another Watchpoint on that same variable with a command defined or different condition, or ignorecount, both of those Watchpoints need to evaluate their criteria/commands when their WatchpointResource has been hit. There's a bit of code movement to rearrange things in the direction I'll need for implementing this feature, so I want to start with reviewing & landing this mostly NFC patch and we can focus on the algorithmic choices about how WatchpointResources are shared and handled as they're triggeed, separately. This patch also stops printing "Watchpoint <n> hit: old value: <x>, new vlaue: <y>" for Read watchpoints. I could make an argument for print "Watchpoint <n> hit: current value <x>" but the current output doesn't make any sense, and the user can print the value if they are particularly interested. Read watchpoints are used primarily to understand what code is reading a variable. This patch adds more fallbacks for how to print the objects being watched if we have types, instead of assuming they are all integral values, so a struct will print its elements. As large watchpoints are added, we'll be doing a lot more of those. To track the WatchpointSP in the WatchpointResources, I changed the internal API which took a WatchpointSP and devolved it to a Watchpoint*, which meant touching several different Process files. I removed the watchpoint code in ProcessKDP which only reported that watchpoints aren't supported, the base class does that already. I haven't yet changed how we receive a watchpoint to identify the WatchpointResource responsible for the trigger, and identify all Watchpoints that are using this Resource to evaluate their conditions etc. This is the same work that a BreakpointSite needs to do when it has been tiggered, where multiple Breakpoints may be at the same address. There is not yet any printing of the Resources that a Watchpoint is implemented in terms of ("watchpoint list", or SBWatchpoint::GetDescription). "watchpoint set var" and "watchpoint set expression" take a size argument which was previously 1, 2, 4, or 8 (an enum). I've changed this to an unsigned int. Most hardware implementations can only watch 1, 2, 4, 8 byte ranges, but with Resources we'll allow a user to ask for different sized watchpoints and set them in hardware-expressble terms soon. I've annotated areas where I know there is work still needed with LWP_TODO that I'll be working on once this is landed. I've tested this on aarch64 macOS, aarch64 Linux, and Intel macOS. https://discourse.llvm.org/t/rfc-large-watchpoint-support-in-lldb/72116 (cherry picked from commit fc6b72523f3d73b921690a713e97a433c96066c6)
2023-11-28Revert "[lldb] [mostly NFC] Large WP foundation: WatchpointResources (#68845)"David Spickett1-4/+4
...and follow ups. As it has caused test failures on Linux Arm and AArch64: https://lab.llvm.org/buildbot/#/builders/96/builds/49126 https://lab.llvm.org/buildbot/#/builders/17/builds/45824 ``` lldb-shell :: Subprocess/clone-follow-child-wp.test lldb-shell :: Subprocess/fork-follow-child-wp.test lldb-shell :: Subprocess/vfork-follow-child-wp.test ``` This reverts commit a6c62bf1a4717accc852463b664cd1012237d334, commit a0a1ff3ab40e347589b4e27d8fd350c600526735 and commit fc6b72523f3d73b921690a713e97a433c96066c6.
2023-11-27[lldb] [mostly NFC] Large WP foundation: WatchpointResources (#68845)Jason Molenda1-4/+4
This patch is rearranging code a bit to add WatchpointResources to Process. A WatchpointResource is meant to represent a hardware watchpoint register in the inferior process. It has an address, a size, a type, and a list of Watchpoints that are using this WatchpointResource. This current patch doesn't add any of the features of WatchpointResources that make them interesting -- a user asking to watch a 24 byte object could watch this with three 8 byte WatchpointResources. Or a Watchpoint on 1 byte at 0x1002 and a second watchpoint on 1 byte at 0x1003, these must both be served by a single WatchpointResource on that doubleword at 0x1000 on a 64-bit target, if two hardware watchpoint registers were used to track these separately, one of them may not be hit. Or if you have one Watchpoint on a variable with a condition set, and another Watchpoint on that same variable with a command defined or different condition, or ignorecount, both of those Watchpoints need to evaluate their criteria/commands when their WatchpointResource has been hit. There's a bit of code movement to rearrange things in the direction I'll need for implementing this feature, so I want to start with reviewing & landing this mostly NFC patch and we can focus on the algorithmic choices about how WatchpointResources are shared and handled as they're triggeed, separately. This patch also stops printing "Watchpoint <n> hit: old value: <x>, new vlaue: <y>" for Read watchpoints. I could make an argument for print "Watchpoint <n> hit: current value <x>" but the current output doesn't make any sense, and the user can print the value if they are particularly interested. Read watchpoints are used primarily to understand what code is reading a variable. This patch adds more fallbacks for how to print the objects being watched if we have types, instead of assuming they are all integral values, so a struct will print its elements. As large watchpoints are added, we'll be doing a lot more of those. To track the WatchpointSP in the WatchpointResources, I changed the internal API which took a WatchpointSP and devolved it to a Watchpoint*, which meant touching several different Process files. I removed the watchpoint code in ProcessKDP which only reported that watchpoints aren't supported, the base class does that already. I haven't yet changed how we receive a watchpoint to identify the WatchpointResource responsible for the trigger, and identify all Watchpoints that are using this Resource to evaluate their conditions etc. This is the same work that a BreakpointSite needs to do when it has been tiggered, where multiple Breakpoints may be at the same address. There is not yet any printing of the Resources that a Watchpoint is implemented in terms of ("watchpoint list", or SBWatchpoint::GetDescription). "watchpoint set var" and "watchpoint set expression" take a size argument which was previously 1, 2, 4, or 8 (an enum). I've changed this to an unsigned int. Most hardware implementations can only watch 1, 2, 4, 8 byte ranges, but with Resources we'll allow a user to ask for different sized watchpoints and set them in hardware-expressble terms soon. I've annotated areas where I know there is work still needed with LWP_TODO that I'll be working on once this is landed. I've tested this on aarch64 macOS, aarch64 Linux, and Intel macOS. https://discourse.llvm.org/t/rfc-large-watchpoint-support-in-lldb/72116
2023-10-30[lldb] Part 2 of 2 - Refactor `CommandObject::DoExecute(...)` return `void` ↵Pete Lawrence1-50/+34
(not `bool`) (#69991) [lldb] Part 2 of 2 - Refactor `CommandObject::DoExecute(...)` to return `void` instead of ~~`bool`~~ Justifications: - The code doesn't ultimately apply the `true`/`false` return values. - The methods already pass around a `CommandReturnObject`, typically with a `result` parameter. - Each command return object already contains: - A more precise status - The error code(s) that apply to that status Part 1 refactors the `CommandObject::Execute(...)` method. - See [https://github.com/llvm/llvm-project/pull/69989](https://github.com/llvm/llvm-project/pull/69989) rdar://117378957
2023-09-21watch set expression's default type was wrong with new modify typeJason Molenda1-1/+17
`watch set expression` was passing the OptionGroupWatchpoint enum in to Target where the LLDB_WATCH_TYPE_* bitfield was expected. Modify matched READ|WRITE and resulted in a test failure in TestWatchTaggedAddress.py. David temporarily changed the test to expect this incorrect output; this fixes the bug and updates the test case to test it for correctness again.
2023-09-21Reland "[lldb] Add 'modify' type watchpoints, make it default (#66308)"David Spickett1-7/+23
This reverts commit a7b78cac9a77e3ef6bbbd8ab1a559891dc693401. With updates to the tests. TestWatchTaggedAddress.py: Updated the expected watchpoint types, though I'm not sure there should be a differnt default for the two ways of setting them, that needs to be confirmed. TestStepOverWatchpoint.py: Skipped this everywhere because I think what used to happen is you couldn't put 2 watchpoints on the same address (after alignment). I guess that this is now allowed because modify watchpoints aren't accounted for, but likely should be. Needs investigating.
2023-09-21Revert "[lldb] Add 'modify' type watchpoints, make it default (#66308)"David Spickett1-23/+7
This reverts commit 933ad5c897ee366759a54869b35b2d7285a92137. This caused 1 test failure and an unexpected pass on AArch64 Linux: https://lab.llvm.org/buildbot/#/builders/96/builds/45765 Wasn't reported because the bot was already red at the time.
2023-09-20[lldb] Add 'modify' type watchpoints, make it default (#66308)Jason Molenda1-7/+23
Watchpoints in lldb can be either 'read', 'write', or 'read/write'. This is exposing the actual behavior of hardware watchpoints. gdb has a different behavior: a "write" type watchpoint only stops when the watched memory region *changes*. A user is using a watchpoint for one of three reasons: 1. Want to find what is changing/corrupting this memory. 2. Want to find what is writing to this memory. 3. Want to find what is reading from this memory. I believe (1) is the most common use case for watchpoints, and it currently can't be done in lldb -- the user needs to continue every time the same value is written to the watched-memory manually. I think gdb's behavior is the correct one. There are some use cases where a developer wants to find every function that writes/reads to/from a memory region, regardless of value, I want to still allow that functionality. This is also a bit of groundwork for my large watchpoint support proposal https://discourse.llvm.org/t/rfc-large-watchpoint-support-in-lldb/72116 where I will be adding support for AArch64 MASK watchpoints which watch power-of-2 memory regions. A user might ask to watch 24 bytes, and a MASK watchpoint stub can do this with a 32-byte MASK watchpoint if it is properly aligned. And we need to ignore writes to the final 8 bytes of that watched region, and not show those hits to the user. This patch adds a new 'modify' watchpoint type and it is the default. Re-landing this patch after addressing testsuite failures found in CI on Linux, Intel machines, and windows. rdar://108234227
2023-09-18Revert "[lldb] Add 'modify' type watchpoints, make it default (#66308)"Jason Molenda1-23/+7
TestStepOverWatchpoint.py and TestUnalignedWatchpoint.py are failing on the ubuntu and debian bots https://lab.llvm.org/buildbot/#/builders/68/builds/60204 https://lab.llvm.org/buildbot/#/builders/96/builds/45623 and the newly added test TestModifyWatchpoint.py does not work on windows bot https://lab.llvm.org/buildbot/#/builders/219/builds/5708 I will debug tomorrow morning and reland. This reverts commit 3692267ca8f9c51cb55e4387283762d921fe2ae2.
2023-09-18[lldb] Add 'modify' type watchpoints, make it default (#66308)Jason Molenda1-7/+23
Watchpoints in lldb can be either 'read', 'write', or 'read/write'. This is exposing the actual behavior of hardware watchpoints. gdb has a different behavior: a "write" type watchpoint only stops when the watched memory region *changes*. A user is using a watchpoint for one of three reasons: 1. Want to find what is changing/corrupting this memory. 2. Want to find what is writing to this memory. 3. Want to find what is reading from this memory. I believe (1) is the most common use case for watchpoints, and it currently can't be done in lldb -- the user needs to continue every time the same value is written to the watched-memory manually. I think gdb's behavior is the correct one. There are some use cases where a developer wants to find every function that writes/reads to/from a memory region, regardless of value, I want to still allow that functionality. This is also a bit of groundwork for my large watchpoint support proposal https://discourse.llvm.org/t/rfc-large-watchpoint-support-in-lldb/72116 where I will be adding support for AArch64 MASK watchpoints which watch power-of-2 memory regions. A user might ask to watch 24 bytes, and a MASK watchpoint stub can do this with a 32-byte MASK watchpoint if it is properly aligned. And we need to ignore writes to the final 8 bytes of that watched region, and not show those hits to the user. This patch adds a new 'modify' watchpoint type and it is the default. rdar://108234227
2023-06-07[lldb] Disable variable watchpoints when going out of scopeMed Ismail Bennani1-15/+22
If we use a variable watchpoint with a condition using a scope variable, if we go out-of-scope, the watpoint remains active which can the expression evaluator to fail to parse the watchpoint condition (because of the missing varible bindings). This was discovered after `watchpoint_callback.test` started failing on the green dragon bot. This patch should address that issue by setting an internal breakpoint on the return addresss of the current frame when creating a variable watchpoint. The breakpoint has a callback that will disable the watchpoint if the the breakpoint execution context matches the watchpoint execution context. This is only enabled for local variables. This patch also re-enables the failing test following e1086384e584. rdar://109574319 Differential Revision: https://reviews.llvm.org/D151366 Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
2023-06-06[lldb/Commands] Add support to auto-completion for user commandsMed Ismail Bennani1-18/+18
This patch should allow the user to set specific auto-completion type for their custom commands. To do so, we had to hoist the `CompletionType` enum so the user can access it and add a new completion type flag to the CommandScriptAdd Command Object. So now, the user can specify which completion type will be used with their custom command, when they register it. This also makes the `crashlog` custom commands use disk-file completion type, to browse through the user file system and load the report. Differential Revision: https://reviews.llvm.org/D152011 Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
2023-05-26Revert "[lldb] Disable variable watchpoints when going out of scope"Jason Molenda1-21/+14
Reverting https://reviews.llvm.org/D151366 until Ismail has a chance to look at the ubuntu CI test failures and can reland. This reverts commit 7c847ac4bd1bd8a89c7fbb4581328fa8cb0498f1.
2023-05-25[lldb] Disable variable watchpoints when going out of scopeMed Ismail Bennani1-14/+21
If we use a variable watchpoint with a condition using a scope variable, if we go out-of-scope, the watpoint remains active which can the expression evaluator to fail to parse the watchpoint condition (because of the missing varible bindings). This was discovered after `watchpoint_callback.test` started failing on the green dragon bot. This patch should address that issue by setting an internal breakpoint on the return addresss of the current frame when creating a variable watchpoint. The breakpoint has a callback that will disable the watchpoint if the the breakpoint execution context matches the watchpoint execution context. This is only enabled for local variables. This patch also re-enables the failing test following e1086384e584. rdar://109574319 Differential Revision: https://reviews.llvm.org/D151366 Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
2023-04-26[lldb] Add support for specifying language when setting watchpoint by expressionAlex Langford1-2/+4
This is useful in contexts where you have multiple languages in play: You may be stopped in a frame for language A, but want to set a watchpoint with an expression using language B. The current way to do this is to use the setting `target.language` while setting the watchpoint and unset it after the watchpoint is set, but that's kind of clunky and somewhat error-prone. This should add a better way to do this. rdar://108202559 Differential Revision: https://reviews.llvm.org/D149111
2023-03-17[lldb] Unify WatchpointSP variable names (NFC)Jonas Devlieghere1-25/+25
LLDB uses `_up`, `_sp` and `_wp` suffixes for unique, shared and weak pointers respectively. This can become confusing in combination with watchpoints which are commonly abbreviated to `wp`. Update CommandObjectWatchpoint to use `watch_sp` for all `WatchpointSP` variables.
2023-03-17[lldb] Set the watchpoint spec for expression watchpointsJonas Devlieghere1-6/+5
When setting a variable watchpoint, the watchpoint stores the variable name in the watchpoint spec. For expression variables we should store the expression in the watchpoint spec. This patch adds that functionality. rdar://106096860 Differential revision: https://reviews.llvm.org/D146262
2023-02-14[LLDB] add arch-specific watchpoint behavior defaults to lldbJason Molenda1-5/+5
lldb was originally designed to get the watchpoint exception behavior from the gdb remote serial protocol stub -- exceptions are either received before the instruction executes, or after the instruction has executed. This behavior was reported via two lldb extensions to gdb RSP, so generic remote stubs like gdbserver or a JTAG stub, would not tell lldb which behavior was correct, and it would default to "exceptions are received after the instruction has executed". Two architectures hard coded their correct "exceptions before instruction" behavior, to work around this issue. Most architectures have a fixed behavior of watchpoint exceptions, and we can center that information in lldb. We can allow a remote stub to override the default behavior via our packet extensions if it's needed on a specific target. This patch also separates the fetching of the number of watchpoints from whether exceptions are before/after the insn. Currently if lldb couldn't fetch the number of watchpoints (not really needed), it also wouldn't get when exceptions are received, and watchpoint handling would fail. lldb doesn't actually use the number of watchpoints for anything beyond printing it to the user. Differential Revision: https://reviews.llvm.org/D143215 rdar://101426626
2023-01-10Move from llvm::makeArrayRef to ArrayRef deduction guides - last partserge-sans-paille1-4/+4
This is a follow-up to https://reviews.llvm.org/D140896, split into several parts as it touches a lot of files. Differential Revision: https://reviews.llvm.org/D141298
2022-12-04[lldb] Use std::nullopt instead of None (NFC)Kazu Hirata1-1/+1
This patch mechanically replaces None with std::nullopt where the compiler would warn if None were deprecated. The intent is to reduce the amount of manual work required in migrating from Optional to std::optional. This is part of an effort to migrate from llvm::Optional to std::optional: https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716
2022-07-14[lldb] Refactor command option enum values (NFC)Jonas Devlieghere1-0/+1
Refactor the command option enum values and the command argument table to connect the two. This has two benefits: - We guarantee that two options that use the same argument type have the same accepted values. - We can print the enum values and their description in the help output. (D129707) Differential revision: https://reviews.llvm.org/D129703
2022-06-19[lldb] Use value_or instead of getValueOr (NFC)Kazu Hirata1-1/+1
2022-03-31[LLDB] Applying clang-tidy modernize-use-equals-default over LLDBShafik Yaghmour1-4/+4
Applied modernize-use-equals-default clang-tidy check over LLDB. This check is already present in the lldb/.clang-tidy config. Differential Revision: https://reviews.llvm.org/D121844
2022-01-23[Commands] Remove redundant member initialization (NFC)Kazu Hirata1-16/+10
Identified with readability-redundant-member-init.
2021-10-23Use StringRef::contains (NFC)Kazu Hirata1-1/+1
2021-06-22[lldb] Remove more redundant SetStatus(eReturnStatusFailed)David Spickett1-16/+10
Mostly by converting uses of GetErrorStream to AppendError, so that the call to SetStatus is implicit. Some remain where it isn't certain that you'll have a message to set, or you want the output to be on stdout. One place in CommandObjectWatchpoint previously didn't set the status to failed at all. However it's pretty obvious that it should do so. Reviewed By: teemperor Differential Revision: https://reviews.llvm.org/D104697
2021-06-17[lldb] Remove redundant calls to set eReturnStatusFailedDavid Spickett1-22/+3
This is part 2, covering the commands source. Some uses remain where it's tricky to see what the logic is or they are not used with AppendError. Reviewed By: teemperor Differential Revision: https://reviews.llvm.org/D104448
2021-06-09[lldb] Use C++11 default member initializersJonas Devlieghere1-12/+8
This converts a default constructor's member initializers into C++11 default member initializers. This patch was automatically generated with clang-tidy and the modernize-use-default-member-init check. $ run-clang-tidy.py -header-filter='lldb' -checks='-*,modernize-use-default-member-init' -fix This is a mass-refactoring patch and this commit will be added to .git-blame-ignore-revs. Differential revision: https://reviews.llvm.org/D103483
2020-08-11[lldb] watchpoint ID common completion for commands `watchpoint ↵Gongyu Deng1-0/+40
delete/enable/disable/modify/ignore` 1. Added a common completion WatchPointIDs to complete with a list of the IDs of the current watchpoints; 2. Applied the completion to these commands: watchpoint delete/enable/disable/modify/ignore; 3. Added a correlated test case. Reviewed By: teemperor Differential Revision: https://reviews.llvm.org/D84104
2020-08-11[lldb] tab completion for `watchpoint set variable`Gongyu Deng1-0/+10
1. Applied the common completion `eVariablePathCompletion` to command `watchpoint set variable`; 2. Added a related test case. Reviewed By: teemperor, JDevlieghere Differential Revision: https://reviews.llvm.org/D84177
2020-07-27Unify the return value of GetByteSize to an llvm::Optional<uint64_t> (NFC-ish)Adrian Prantl1-1/+1
This cleanup patch unifies all methods called GetByteSize() in the ValueObject hierarchy to return an optional, like the methods in CompilerType do. This means fewer magic 0 values, which could fix bugs down the road in languages where types can have a size of zero, such as Swift and C (but not C++). Differential Revision: https://reviews.llvm.org/D84285 This re-lands the patch with bogus :m_byte_size(0) initalizations removed.
2020-07-25Temporarily Revert "Unify the return value of GetByteSize to an ↵Eric Christopher1-1/+1
llvm::Optional<uint64_t> (NFC-ish)" as it's causing numerous (176) test failures on linux. This reverts commit 1d9b860fb6a85df33fd52fcacc6a5efb421621bd.
2020-07-25Unify the return value of GetByteSize to an llvm::Optional<uint64_t> (NFC-ish)Adrian Prantl1-1/+1
This cleanup patch unifies all methods called GetByteSize() in the ValueObject hierarchy to return an optional, like the methods in CompilerType do. This means fewer magic 0 values, which could fix bugs down the road in languages where types can have a size of zero, such as Swift and C (but not C++). Differential Revision: https://reviews.llvm.org/D84285
2020-04-07[lldb][NFC] Fix typo in 'watchpoint delete' error messageRaphael Isemann1-1/+1
2020-04-07[lldb] NFC: Fix trivial typo in comments, documents, and messagesKazuaki Ishizaki1-1/+1
Differential Revision: https://reviews.llvm.org/D77460
2020-02-27[lldb] Show the actual error when 'watchpoint set expression' failedRaphael Isemann1-0/+2
Currently we only show the user that the expression failed but not what is actually wrong with it. This just dumps the error we get back alongside the other output to the error stream. This should also help with finding out with why sometimees the TestWatchLocationWithWatchSet.py test fails here on the LLDB incremental bot on Green Dragon.
2020-01-28Make llvm::StringRef to std::string conversions explicit.Benjamin Kramer1-2/+2
This is how it should've been and brings it more in line with std::string_view. There should be no functional change here. This is mostly mechanical from a custom clang-tidy check, with a lot of manual fixups. It uncovers a lot of minor inefficiencies. This doesn't actually modify StringRef yet, I'll do that in a follow-up.