[lldb-vscode] Add data breakpoint support

[cimacmillan]

Summary

Added support for setting data breakpoints to the LLDB vscode adapter. This is part of a workstream to improve the debugging support of the WebAssembly-Micro-Runtime, which uses lldb-vscode as it's debug adaptor: https://github.com/bytecodealliance/wasm-micro-runtime/issues/1810

Added an intergration test that runs two simple scenarios.

[cimacmillan]

This comment was removed by cimacmillan.

[cimacmillan]

This comment was removed by cimacmillan.

[clayborg]

lldb/test/API/tools/lldb-vscode/breakpoint_data/TestVSCode_setDataBreakpoints.py

75–82

This might fail on some systems if they put the value for the local variables "num_a" or "num_b" in a register. So you might need to modify this test to make sure it will run on any systems. The other thing you can do is to take the address of any local variable to ensure it is kept in memory, so adding "int *num_a_ptr = &num_a;" and "int *num_b_ptr = &num_b;" might help ensure this test always works.

96

I would suggest adding a test for a constant global value that appears in a section (like .rodata) that has no write permissions and verify that we correctly figure out that we can only do "read" access on such a variable (see code changes I suggest where we check the memory region).

We also need to test error conditions out when a variable is not in memory. If you compile some C code with optimizations, you can try to create a variable that is in a register with "int num_a = 1;" and in the test grab the SBValue for this local variable and test if the value has an invalid load address

lldb/tools/lldb-vscode/Watchpoint.cpp

35–37

no braces on single line if statements per llvm coding guidelines.

39–45

We should be using the "SBValue::Watch(...)" instead of SBTarget.WatchAddress(...). If you create a watchpoint for a local variable that is on the stack, this will cause all sorts of problems if the variable goes out of scope. SBValue::Watch(...) has the smarts to be able to figure out if a variable is local, and I seem to remember that it will disable this watchpoint as soon as a local variable goes out of scope, though I might not be remember things correctly. It can also figure out if a variable is currently in memory or not and it won't set a watchpoint if possible.

I might recommend that this Watchpoint class gets created with a SBValue, and then we use that for setting the watchpoint, the we set the watchpoint using:

lldb::SBWatchpoint lldb::SBValue::Watch(bool resolve_location, bool read, bool write, SBError &error);

lldb/tools/lldb-vscode/lldb-vscode.cpp

2431

Some variables don't have load addresses. This can happen when a variable is in a register. If a variable is in a register, then this function will return LLDB_INVALID_ADDRESS, and if that happens you will need to return something appropriate, like maybe returning "null" for the "dataId" in the response and then filling in an appropriate message in the "description" as it is documented above as "UI string that describes on what data the breakpoint is set on or why a data breakpoint is not available".

2441–2444

if "v_address != LLDB_INVALID_ADDRESS", then you can ask for the memory region for this address using:

lldb::SBError lldb::SBProcess::GetMemoryRegionInfo(lldb::addr_t load_addr, lldb::SBMemoryRegionInfo &region_info);

If you get an error back, then this memory address is unreadable and you should return an error. If no error is returned, then you should check if the memory region has read or write permissions with:

llvm::json::Array access_types;
lldb::SBMemoryRegionInfo region_info;
lldb::SBError error = g_vsc.target.GetProcess().GetMemoryRegionInfo(v_address, region_info);
if (error.Success()) {
  if (region_info.IsReadable()) {
    access_types.emplace_back("read");
    if (region_info.IsWritable())
      access_types.emplace_back("readWrite");
  }
  if (region_info. IsWritable())
    access_types.emplace_back("write");
}

[cimacmillan]

@clayborg Thanks for your feedback. I'm part the way through implementing your changes. Specifically about this point:

I seem to remember that it will disable this watchpoint as soon as a local variable goes out of scope, though I might not be remember things correctly

This is a behaviour I'd like to address, where for instance watchpoints are triggered in different functions because the stack frame addresses align. I have this example, I can add a test for:

int test_a() {
    int x = 10; <- watchpoint set here
    x = 20; <- watchpoint triggered here
}

int test_b() {
    int b = 10; <- watchpoint triggered here also
    b = 20;
}

int main() {
    test_a();
    test_b();

I've refactored the Watchpoint class to use "SBValue::Watch", and I can still reproduce this behaviour. I also can't find where this watchpoint disable on scope change might be implemented. Do you have any suggestions for this? Thanks

[clayborg]

In D140630#4056641, @cimacmillan wrote:

@clayborg Thanks for your feedback. I'm part the way through implementing your changes. Specifically about this point:

I seem to remember that it will disable this watchpoint as soon as a local variable goes out of scope, though I might not be remember things correctly

This is a behaviour I'd like to address, where for instance watchpoints are triggered in different functions because the stack frame addresses align. I have this example, I can add a test for:

int test_a() {
    int x = 10; <- watchpoint set here
    x = 20; <- watchpoint triggered here
}

int test_b() {
    int b = 10; <- watchpoint triggered here also
    b = 20;
}

int main() {
    test_a();
    test_b();

I've refactored the Watchpoint class to use "SBValue::Watch", and I can still reproduce this behaviour. I also can't find where this watchpoint disable on scope change might be implemented. Do you have any suggestions for this? Thanks

Upon further examination it doesn't seem like this is happening, though it should be possible to add some things to the watchpoint classes to make this happen. This is beyond the scope of this fix though, so no need to do it unless you have time. What we would need to do is store a StackID (see "lldb/Target/StackID.h") and thread ID with the watchpoint. When the watchpoint triggers, we would need to check if the StackID + thread ID is valid (we should store this info with the watchpoint when setting it from a SBValue::Watch(...), or not if we just watch the address) still exists in the thread's stack somewhere. If it does still exist then stop and report, else clear the watchpoint and don't report anything. If the stack ID isn't valid then we always stop.

[cimacmillan]

Add handling for const and register cases. Setting watchpoint on SBValue rather than the
address.

[clayborg]

lldb/test/API/tools/lldb-vscode/breakpoint_data/TestVSCode_setDataBreakpoints.py

74

might be better as suggested?

lldb/tools/lldb-vscode/Watchpoint.cpp

16

We should either rename the argument variables so they don't match the member variable names or if we add the "m_" prefix this all just will work.

17

remove "this->"

21–22

28

Remove all "this->" from this and all changes in this patch.

37–42

If we use two bools instead of using WatchPointType enum, this is much easier to code.

47–49

We should probably just leave the error inside of this object and then test if the watchpoint was set correctly and report it back in the response to "setDataBreakpoints" for each breakpoint instead of printing to console?

lldb/tools/lldb-vscode/Watchpoint.h

21

We could get rid of this enum and just store two bools and avoid the ReadWrite case. See comment below where WatchpointType is used for member variable

30–35

For classes we prefer things to have a "m_" prefix for all member variables. I know other parts of this code for breakpoints use "struct" instead. It helps keep code readable and avoids having variables and member variables with the same name. See next inline comment.

32

Maybe just use two bools and get rid of WatchPointType enum since we really want a bitfield for read and write?

lldb/tools/lldb-vscode/lldb-vscode.cpp

2125–2138

We shouldn't be logging to the debug console if the setDataBreakpoint packet is not valid, but we should return an error for that packet. Might be better to have this return a bool to indicate success or failure and have the prototype be:

static bool get_watchpoint_type(const std::string &type, bool &read, bool &write);

If this caller gets false returned from this, then we return an error to the packet with an appropriate error message.

2188

This function should probably be able to return an error for each watchpoint if anything fails so this can be reported back as a description or error response in the "setDataBreakpoints" response? We shouldn't be printing to the console for errors in the "breakpoint" object arguments or if the watchpoint actually fails to resolve (was in a register). An error message should be returned somehow if this is possible?

2192–2193

We should return an error for this breakpoint if the watchpoint type is not supported. Is there a way to return an error for a specific data breakpoint instead of us printing to the console?

2317–2318

You have to return a valid "Breakpoint" object for each item in "breakpoints". If we fail, it looks like you can put any error into the "Breakpoint.message" field and set "Breakpoint.verified = false".

[cimacmillan]

lldb/tools/lldb-vscode/lldb-vscode.cpp

2188

Thanks for your feedback, working through comments now. About this and

Is there a way to return an error for a specific data breakpoint instead of us printing to the console?

We can return whether the breakpoint is verified, which displays a greyed dot similar to when a line breakpoint can't be set. There doesn't seem to be a way to display the error in VS code with the message. I think the logging would be useful in that case, but I can also include it in the message response in case this is added to VS code.

[cimacmillan]

Refactored Watchpoint class and addressed comments

[cimacmillan]

@clayborg Thanks for your feedback. I've refactored the Watchpoint class slightly so that it encapsulates the parsing of the setDataBreakpoints request and be used to get the Breakpoint info to attach to the response. This is more similar to how the other breakpoint classes are written, and encapsulates things a bit better.

lldb/test/API/tools/lldb-vscode/breakpoint_data/TestVSCode_setDataBreakpoints.py

74

There isn't an IsValid function in this case as it's not the SBValue type. I suppose is the variable was invalid, the rest of the test would fail.

[cimacmillan]

Ping.

[clayborg]

Sorry for delay, I have been on vacation for the last two weeks. I will be back next Monday and get to this soon. Feel free to ping again next week if I don't get to it!

[cimacmillan]

Ping.

[cimacmillan]

Ping.

[jingham]

In D140630#4276855, @cimacmillan wrote:

Ping.

Greg's been out since late March, and isn't expected back for a while still. I am entirely unfamiliar with the lldb-vscode part of lldb, so I don't feel comfortable reviewing this.

[cimacmillan]

Ping.

[wallace]

This seems reasonable to me and I'm accepting it to unblock this feature. When Greg gets back he should be able to provide some additional comments.

[cimacmillan]

Ping

@clayborg would you mind having another look? Thanks

[wallace]

Greg hasn't been working for a while. You can go ahead and land this, and when he gets back he can share some feedback.

[clayborg]

Sorry this fell off my radar. I was on medical leave for a surgery for 3 months. Back now

lldb/tools/lldb-vscode/Watchpoint.cpp

72

90

lldb/tools/lldb-vscode/Watchpoint.h

1

Fix width of this comment to match line 7

lldb/tools/lldb-vscode/lldb-vscode.cpp

2449

Any line over 80 chars should be formatted to not exceed. Best to try and. use clang-format

lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py

def verify_breakpoint_hit(self, breakpoint_ids):
                 # location.
                 description = body['description']
                 for breakpoint_id in breakpoint_ids:
                     match_desc = 'breakpoint %s.' % (breakpoint_id)
                     if match_desc in description:
                         return
         self.assertTrue(False, "breakpoint not hit")
 
+    def verify_watchpoint_hit(self, line_number):
+        '''Wait for the process we are debugging to stop, and verify we hit
+           any watchpoint at the line number.'''
+        stopped_events = self.vscode.wait_for_stopped()
+        for stopped_event in stopped_events:
+            if 'body' in stopped_event:
+                body = stopped_event['body']
+                if 'reason' not in body:
+                    continue
+                if body['reason'] != 'breakpoint':
+                    continue
+                if 'description' not in body:
+                    continue
+                description = body['description']
+                if 'watchpoint' not in description:
+                    continue
+
+                (_, line) = self.get_source_and_line(threadId=body['threadId'])
+                if line == line_number:
+                    return
+
+        self.assertTrue(False, "watchpoint not hit")
+
     def verify_stop_exception_info(self, expected_description):
         '''Wait for the process we are debugging to stop, and verify the stop
            reason is 'exception' and that the description matches
            'expected_description'
         '''
         stopped_events = self.vscode.wait_for_stopped()
         for stopped_event in stopped_events:
             if 'body' in stopped_event:

lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py

def request_setFunctionBreakpoints(self, names, condition=None,
         args_dict = {'breakpoints': breakpoints}
         command_dict = {
             'command': 'setFunctionBreakpoints',
             'type': 'request',
             'arguments': args_dict
         }
         return self.send_recv(command_dict)
 
+    def request_setDataBreakpoints(self, id, address, accessType):
+        breakpoint = {
+            'id': id,
+            'dataId': address,
+            'enabled': True,
+            'accessType': accessType
+        }
+        args_dict = {
+            'breakpoints': [ breakpoint ]
+        }
+        command_dict = {
+            'command': 'setDataBreakpoints',
+            'type': 'request',
+            'arguments': args_dict
+        }
+        return self.send_recv(command_dict)
+
+    def request_dataBreakpointInfo(self, name, variables_reference):
+        args_dict = {
+            'name': name,
+            'variablesReference': variables_reference
+        }
+        command_dict = {
+            'command': 'dataBreakpointInfo',
+            'type': 'request',
+            'arguments': args_dict
+        }
+        return self.send_recv(command_dict)
+
     def request_compileUnits(self, moduleId):
         args_dict = {'moduleId': moduleId}
         command_dict = {
             'command': 'compileUnits',
             'type': 'request',
             'arguments': args_dict
         }
         response = self.send_recv(command_dict)

lldb/test/API/tools/lldb-vscode/breakpoint_data/Makefile

+CXX_SOURCES := main-copy.cpp
+LD_EXTRAS := -Wl,-rpath "-Wl,$(shell pwd)"
+USE_LIBDL :=1
+
+include Makefile.rules
+
+main-copy.cpp: main.cpp
+  cp -f $< $@

lldb/test/API/tools/lldb-vscode/breakpoint_data/TestVSCode_setDataBreakpoints.py

+"""
+Test lldb-vscode setDataBreakpoints request
+"""
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+import lldbvscode_testcase
+import os
+
+# Return first element in the array where func is true
+def array_find(array, func):
+    for i in array:
+        if func(i):
+            return i
+    return None
+
+class TestVSCode_setDataBreakpoints(lldbvscode_testcase.VSCodeTestCaseBase):
+
+    def setUp(self):
+        lldbvscode_testcase.VSCodeTestCaseBase.setUp(self)
+        self.main_basename = 'main-copy.cpp'
+        self.main_path = os.path.realpath(self.getBuildArtifact(self.main_basename))
+
+    @skipIfWindows
+    @skipIfRemote
+    def test_global_var(self):
+        program = self.getBuildArtifact("a.out")
+        self.build_and_launch(program)
+        
+        # Break on the main function so we can see the variable scopes
+        response = self.vscode.request_setFunctionBreakpoints(['main'])
+        breakpoints = response['body']['breakpoints']
+        breakpoint_id = breakpoints[0]['id']
+        self.vscode.request_continue()
+        self.verify_breakpoint_hit([breakpoint_id])
+
+        # Get and verify that the global_num is in scope
+        globals = self.vscode.get_global_variables()
+        global_num = array_find(globals, lambda x: x['name'] == 'global_num')
+        self.assertIsNotNone(global_num)
+
+        # Get and verify the data breakpoint info
+        data_breakpoint_info = self.vscode.request_dataBreakpointInfo(
+            global_num['name'], 2
+        )
+        self.assertEqual(data_breakpoint_info['body']['accessTypes'], ['read', 'readWrite', 'write'])
+
+        # Set the data breakpoint and verify watchpoint hit after continue
+        self.vscode.request_setDataBreakpoints(
+            'mockId',
+            data_breakpoint_info['body']['dataId'],
+            'readWrite'
+        )
+        self.vscode.request_continue()
+        expected_line = line_number('main.cpp', '// global_num')
+        self.verify_watchpoint_hit(expected_line)
+
+    @skipIfWindows
+    @skipIfRemote
+    def test_local_var(self):
+        program = self.getBuildArtifact("a.out")
+        self.build_and_launch(program)
+        
+        # Break on the main function so we can see the variable scopes
+        response = self.vscode.request_setFunctionBreakpoints(['main'])
+        breakpoints = response['body']['breakpoints']
+        breakpoint_id = breakpoints[0]['id']
+        self.vscode.request_continue()
+        self.verify_breakpoint_hit([breakpoint_id])
+
+        # Get and verify the locals
+        locals = self.vscode.get_local_variables()
+        num_a = array_find(locals, lambda x: x['name'] == 'num_a')
+        self.assertIsNotNone(num_a)
+
+        # Get and verify the data breakpoint info
+        data_breakpoint_info = self.vscode.request_dataBreakpointInfo(
+            num_a['name'], 1
+        )
+        self.assertEqual(data_breakpoint_info['body']['accessTypes'], ['read', 'readWrite', 'write'])
+
+        # Set the data breakpoint and verify breakpoint hit after continue
+        self.vscode.request_setDataBreakpoints(
+            'mockId',
+            data_breakpoint_info['body']['dataId'],
+            'readWrite'
+        )
+        self.vscode.request_continue()
+        expected_line = line_number('main.cpp', '// num_a first')
+        self.verify_watchpoint_hit(expected_line)
+
+        # In this example there is a second watchpoint to hit
+        self.vscode.request_continue()
+        expected_line = line_number('main.cpp', '// num_a second')
+        self.verify_watchpoint_hit(expected_line)
+
+    @skipIfWindows
+    @skipIfRemote
+    def global_const_num(self):
+        program = self.getBuildArtifact("a.out")
+        self.build_and_launch(program)
+        
+        # Break on the main function so we can see the variable scopes
+        response = self.vscode.request_setFunctionBreakpoints(['main'])
+        breakpoints = response['body']['breakpoints']
+        breakpoint_id = breakpoints[0]['id']
+        self.vscode.request_continue()
+        self.verify_breakpoint_hit([breakpoint_id])
+
+        # Get and verify that the global_num is in scope
+        globals = self.vscode.get_global_variables()
+        global_const_num = array_find(globals, lambda x: x['name'] == 'global_const_num')
+        self.assertIsNotNone(global_const_num)
+
+        # Get and verify the data breakpoint info
+        data_breakpoint_info = self.vscode.request_dataBreakpointInfo(
+            global_const_num['name'], 2
+        )
+        self.assertEqual(data_breakpoint_info['body']['accessTypes'], ['read'])
+
+        # Set the data breakpoint and verify watchpoint hit after continue
+        self.vscode.request_setDataBreakpoints(
+            'mockId',
+            data_breakpoint_info['body']['dataId'],
+            'read'
+        )
+        self.vscode.request_continue()
+        expected_line = line_number('main.cpp', '// global_const_num')
+        self.verify_watchpoint_hit(expected_line)

lldb/test/API/tools/lldb-vscode/breakpoint_data/main.cpp

+#include "string.h"
+
+static int global_num = 20;
+const char *global_str = "hello world";
+const int global_const_num = 123;
+
+int main() {
+  int num_a = 10;
+  int num_b = 20; // num_a first
+  int *num_a_ptr = &num_a;
+  int *num_b_ptr = &num_b;
+
+  *num_a_ptr = *num_b_ptr * 20;
+  *num_b_ptr = *num_a_ptr / 10; // num_a second
+  global_num = 30;
+
+  int string_sum = 0; // global_num
+  for (int i = 0; i < strlen(global_str); i++)
+    string_sum += global_const_num; // global_const_num
+}

lldb/test/API/tools/lldb-vscode/breakpoint_data_optimized/Makefile

+CXX_SOURCES := main-copy.cpp
+LD_EXTRAS := -Wl,-rpath "-Wl,$(shell pwd)"
+USE_LIBDL :=1
+CXXFLAGS_EXTRAS := -O3
+
+include Makefile.rules
+
+main-copy.cpp: main.cpp
+  cp -f $< $@

lldb/test/API/tools/lldb-vscode/breakpoint_data_optimized/TestVSCode_setDataBreakpoints.py

+"""
+Test lldb-vscode setDataBreakpoints request
+"""
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+import lldbvscode_testcase
+import os
+
+# Return first element in the array where func is true
+def array_find(array, func):
+    for i in array:
+        if func(i):
+            return i
+    return None
+
+class TestVSCode_setDataBreakpoints(lldbvscode_testcase.VSCodeTestCaseBase):
+
+    def setUp(self):
+        lldbvscode_testcase.VSCodeTestCaseBase.setUp(self)
+        self.main_basename = 'main-copy.cpp'
+        self.main_path = os.path.realpath(self.getBuildArtifact(self.main_basename))
+
+    @skipIfWindows
+    @skipIfRemote
+    def test_local_var(self):
+        program = self.getBuildArtifact("a.out")
+        self.build_and_launch(program)
+        
+        # Break on the main function so we can see the variable scopes
+        response = self.vscode.request_setFunctionBreakpoints(['main'])
+        breakpoints = response['body']['breakpoints']
+        breakpoint_id = breakpoints[0]['id']
+        self.vscode.request_continue()
+        self.verify_breakpoint_hit([breakpoint_id])
+
+        # Get and verify the locals
+        locals = self.vscode.get_local_variables()
+        num_a = array_find(locals, lambda x: x['name'] == 'num_a')
+        self.assertIsNotNone(num_a)
+
+        # Get and verify the data breakpoint info
+        data_breakpoint_info = self.vscode.request_dataBreakpointInfo(
+            num_a['name'], 1
+        )
+        # num_a has no watchpoint access as it's stored in a register
+        self.assertEqual(data_breakpoint_info['body']['accessTypes'], [])

lldb/test/API/tools/lldb-vscode/breakpoint_data_optimized/main.cpp

+#include "stdio.h"
+
+int main() {
+  int num_a = 10;
+  int num_b = 20;
+  printf("%d", num_a + num_b);
+}

lldb/tools/lldb-vscode/CMakeLists.txt

add_lldb_tool(lldb-vscode
   FunctionBreakpoint.cpp
   IOStream.cpp
   JSONUtils.cpp
   LLDBUtils.cpp
   OutputRedirector.cpp
   ProgressEvent.cpp
   RunInTerminal.cpp
   SourceBreakpoint.cpp
+  Watchpoint.cpp
   VSCode.cpp
 
   LINK_LIBS
     liblldb
     ${extra_libs}
 
   LINK_COMPONENTS
     Option

lldb/tools/lldb-vscode/VSCode.h

 #include "llvm/Config/llvm-config.h" // for LLVM_ON_UNIX
 
 #include <condition_variable>
 #include <cstdio>
 #include <iosfwd>
 #include <map>
 #include <set>
 #include <thread>
+#include <unordered_map>
 
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/JSON.h"
 #include "llvm/Support/raw_ostream.h"
 

 
 #include "ExceptionBreakpoint.h"
 #include "FunctionBreakpoint.h"
 #include "IOStream.h"
 #include "ProgressEvent.h"
 #include "RunInTerminal.h"
 #include "SourceBreakpoint.h"
 #include "SourceReference.h"
+#include "Watchpoint.h"
 
 #define VARREF_LOCALS (int64_t)1
 #define VARREF_GLOBALS (int64_t)2
 #define VARREF_REGS (int64_t)3
 #define VARREF_FIRST_VAR_IDX (int64_t)4
 #define NO_TYPENAME "<no-type>"
 
 namespace lldb_vscode {
struct VSCode {
   std::thread event_thread;
   std::thread progress_event_thread;
   std::unique_ptr<std::ofstream> log;
   llvm::DenseMap<lldb::addr_t, int64_t> addr_to_source_ref;
   llvm::DenseMap<int64_t, SourceReference> source_map;
   llvm::StringMap<SourceBreakpointMap> source_breakpoints;
   FunctionBreakpointMap function_breakpoints;
   std::vector<ExceptionBreakpoint> exception_breakpoints;
+  std::unordered_map<std::string, Watchpoint> watchpoints;
   std::vector<std::string> init_commands;
   std::vector<std::string> pre_run_commands;
   std::vector<std::string> exit_commands;
   std::vector<std::string> stop_commands;
   std::vector<std::string> terminate_commands;
   lldb::tid_t focus_tid;
   bool sent_terminated_event;
   bool stop_at_entry;

lldb/tools/lldb-vscode/Watchpoint.h

+//===-- Watchpoint.h --------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_TOOLS_LLDB_VSCODE_WATCHPOINT_H
+#define LLDB_TOOLS_LLDB_VSCODE_WATCHPOINT_H
+
+#include "lldb/API/SBError.h"
+#include "lldb/API/SBValue.h"
+#include "lldb/API/SBWatchpoint.h"
+#include "llvm/Support/JSON.h"
+
+#include <string>
+
+namespace lldb_vscode {
+
+typedef lldb::SBValue (*VariableGetter)(std::string variable_name,
+                                        uint32_t variables_reference);
+
+class Watchpoint {
+public:
+  // Get a watchpoint ID from request
+  static std::string GetId(const llvm::json::Object &obj);
+
+  // Create a watchpoint from a debug adapter protocol request
+  Watchpoint(const llvm::json::Object &obj, VariableGetter variable_getter);
+
+  // Update existing watchpoint with new debug adapter protocol request
+  void Update(const llvm::json::Object &obj);
+
+  // Get watchpoint state for data breakpoint response
+  llvm::json::Object GetResponse();
+
+  // Remove any existing watchpoint
+  void RemoveExisting();
+
+private:
+  VariableGetter m_variable_getter;
+  std::string m_id;
+  bool m_enabled;
+  bool m_read;
+  bool m_write;
+  bool m_verified;
+  std::string m_error;
+  lldb::SBWatchpoint m_watchpoint;
+
+  void OnError(std::string reason);
+};
+
+} // namespace lldb_vscode
+
+#endif

lldb/tools/lldb-vscode/Watchpoint.cpp

+//===-- Watchpoint.cpp ------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "Watchpoint.h"
+#include "VSCode.h"
+
+namespace lldb_vscode {
+
+std::string Watchpoint::GetId(const llvm::json::Object &obj) {
+  return GetString(obj, "id").str();
+}
+
+Watchpoint::Watchpoint(const llvm::json::Object &obj,
+                       VariableGetter variable_getter)
+    : m_variable_getter(variable_getter), m_verified(false) {
+  Update(obj);
+}
+
+void Watchpoint::Update(const llvm::json::Object &obj) {
+  RemoveExisting();
+
+  m_id = GetId(obj);
+  m_enabled = GetBoolean(obj, "enabled", false);
+  if (!m_enabled)
+    return;
+
+  std::string access_type = GetString(obj, "accessType").str();
+  m_read = access_type == "read" || access_type == "readWrite";
+  m_write = access_type == "write" || access_type == "readWrite";
+  if (m_read == false && m_write == false) {
+    OnError("accessType is invalid. It should be read/write/readWrite.");
+    return;
+  }
+
+  std::string data_id = GetString(obj, "dataId").str();
+  auto delimiter = data_id.find('/');
+  if (delimiter == std::string::npos) {
+    OnError(
+        "data_id should be of the format 'variable_name/variable_reference'");
+    return;
+  }
+
+  auto variable_name = data_id.substr(0, delimiter);
+  auto variable_index_str = data_id.substr(delimiter + 1, data_id.length());
+  if (variable_name.size() == 0 || variable_index_str.size() == 0) {
+    OnError(
+        "data_id should be of the format 'variable_name/variable_reference'");
+    return;
+  }
+
+  uint32_t variables_reference = stoul(variable_index_str);
+  lldb::SBValue variable =
+      m_variable_getter(variable_name, variables_reference);
+  if (!variable.IsValid()) {
+    OnError("referenced variable is invalid");
+    return;
+  }
+
+  lldb::SBError error;
+  m_watchpoint = variable.Watch(true, m_read, m_write, error);
+  if (!error.Success()) {
+    OnError(error.GetCString());
+    return;
+  };
+
+  m_verified = true;
+  m_error = "";
+}
+
+llvm::json::Object Watchpoint::GetResponse() {
+  llvm::json::Object watchpoint_info;
+  watchpoint_info.try_emplace("id", m_id);
+  watchpoint_info.try_emplace("verified", m_verified);
+  if (m_error.size() > 0)
+    watchpoint_info.try_emplace("message", m_error);
+
+  return watchpoint_info;
+}
+
+void Watchpoint::RemoveExisting() {
+  if (m_verified)
+    g_vsc.target.DeleteWatchpoint(m_watchpoint.GetID());
+
+  m_verified = false;
+  m_error = "";
+}
+
+void Watchpoint::OnError(std::string error) {
+  m_verified = false;
+  m_error = error;
+  g_vsc.SendOutput(OutputType::Console, error);
+}
+
+} // namespace lldb_vscode

lldb/tools/lldb-vscode/lldb-vscode.cpp

 #include <chrono>
 #include <fstream>
 #include <map>
 #include <memory>
 #include <mutex>
 #include <set>
 #include <sstream>
 #include <thread>
+#include <unordered_set>
 #include <vector>
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Option/Option.h"
 #include "llvm/Support/Errno.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/raw_ostream.h"
 
+#include "lldb/API/SBMemoryRegionInfo.h"
+
 #include "JSONUtils.h"
 #include "LLDBUtils.h"
 #include "OutputRedirector.h"
 
 #if defined(_WIN32)
 #ifndef PATH_MAX
 #define PATH_MAX MAX_PATH
 #endif
void request_initialize(const llvm::json::Object &request) {
   // 'totalFrames' result of the 'StackTrace' request are supported.
   body.try_emplace("supportsDelayedStackTraceLoading", true);
   // The debug adapter supports the 'loadedSources' request.
   body.try_emplace("supportsLoadedSourcesRequest", false);
   // The debug adapter supports sending progress reporting events.
   body.try_emplace("supportsProgressReporting", true);
   // The debug adapter supports 'logMessage' in breakpoint.
   body.try_emplace("supportsLogPoints", true);
+  // The debug adapter supports data breakpoints
+  body.try_emplace("supportsDataBreakpoints", true);
 
   response.try_emplace("body", std::move(body));
   g_vsc.SendJSON(llvm::json::Value(std::move(response)));
 }
 
 llvm::Error request_runInTerminal(const llvm::json::Object &launch_request) {
   g_vsc.is_attach = true;
   lldb::SBAttachInfo attach_info;
void request_setBreakpoints(const llvm::json::Object &request) {
   }
 
   llvm::json::Object body;
   body.try_emplace("breakpoints", std::move(response_breakpoints));
   response.try_emplace("body", std::move(body));
   g_vsc.SendJSON(llvm::json::Value(std::move(response)));
 }
 
+static lldb::SBValue get_variable(std::string variable_name,
+                                  uint32_t variables_reference) {
+  bool is_duplicated_variable_name =
+      variable_name.find(" @") != llvm::StringRef::npos;
+  lldb::SBValue variable;
+
+  if (lldb::SBValueList *top_scope = GetTopLevelScope(variables_reference)) {
+    // variablesReference is one of our scopes, not an actual variable it is
+    // asking for a variable in locals or globals or registers
+    int64_t end_idx = top_scope->GetSize();
+    // Searching backward so that we choose the variable in closest scope
+    // among variables of the same name.
+    for (int64_t i = end_idx - 1; i >= 0; --i) {
+      lldb::SBValue curr_variable = top_scope->GetValueAtIndex(i);
+      std::string local_variable = CreateUniqueVariableNameForDisplay(
+          curr_variable, is_duplicated_variable_name);
+      if (variable_name == local_variable) {
+        variable = curr_variable;
+        break;
+      }
+    }
+  } else {
+    // This is not under the globals or locals scope, so there are no duplicated
+    // names.
+
+    // We have a named item within an actual variable so we need to find it
+    // withing the container variable by name.
+    lldb::SBValue container = g_vsc.variables.GetVariable(variables_reference);
+    if (!container.IsValid()) {
+      return variable;
+    }
+
+    variable = container.GetChildMemberWithName(variable_name.data());
+    if (!variable.IsValid()) {
+      if (variable_name.size() > 0 && variable_name[0] == '[') {
+        llvm::StringRef index_str(std::move(variable_name.substr(1)));
+        uint64_t index = 0;
+        if (!index_str.consumeInteger(0, index)) {
+          if (index_str == "]")
+            variable = container.GetChildAtIndex(index);
+        }
+      }
+    }
+  }
+
+  return variable;
+}
+
+// "SetDataBreakpointsRequest": {
+//   "allOf": [ { "$ref": "#/definitions/Request" }, {
+//     "type": "object",
+//     "description": "Replaces all existing data breakpoints with new data
+//     breakpoints.\nTo clear all data breakpoints, specify an empty
+//     array.\nWhen a data breakpoint is hit, a `stopped` event (with reason
+//     `data breakpoint`) is generated.\nClients should only call this request
+//     if the corresponding capability `supportsDataBreakpoints` is true.",
+//     "properties": {
+//       "command": {
+//         "type": "string",
+//         "enum": [ "setDataBreakpoints" ]
+//       },
+//       "arguments": {
+//         "$ref": "#/definitions/SetDataBreakpointsArguments"
+//       }
+//     },
+//     "required": [ "command", "arguments"  ]
+//   }]
+// },
+// "SetDataBreakpointsArguments": {
+//   "type": "object",
+//   "description": "Arguments for `setDataBreakpoints` request.",
+//   "properties": {
+//     "breakpoints": {
+//       "type": "array",
+//       "items": {
+//         "$ref": "#/definitions/DataBreakpoint"
+//       },
+//       "description": "The contents of this array replaces all existing data
+//       breakpoints. An empty array clears all data breakpoints."
+//     }
+//   },
+//   "required": [ "breakpoints" ]
+// },
+// "SetDataBreakpointsResponse": {
+//   "allOf": [ { "$ref": "#/definitions/Response" }, {
+//     "type": "object",
+//     "description": "Response to `setDataBreakpoints` request.\nReturned is
+//     information about each breakpoint created by this request.",
+//     "properties": {
+//       "body": {
+//         "type": "object",
+//         "properties": {
+//           "breakpoints": {
+//             "type": "array",
+//             "items": {
+//               "$ref": "#/definitions/Breakpoint"
+//             },
+//             "description": "Information about the data breakpoints. The array
+//             elements correspond to the elements of the input argument
+//             `breakpoints` array."
+//           }
+//         },
+//         "required": [ "breakpoints" ]
+//       }
+//     },
+//     "required": [ "body" ]
+//   }]
+// }
+// "DataBreakpoint": {
+//   "type": "object",
+//   "description": "Properties of a data breakpoint passed to the
+//   `setDataBreakpoints` request.", "properties": {
+//     "dataId": {
+//       "type": "string",
+//       "description": "An id representing the data. This id is returned from
+//       the `dataBreakpointInfo` request."
+//     },
+//     "accessType": {
+//       "$ref": "#/definitions/DataBreakpointAccessType",
+//       "description": "The access type of the data."
+//     },
+//     "condition": {
+//       "type": "string",
+//       "description": "An expression for conditional breakpoints."
+//     },
+//     "hitCondition": {
+//       "type": "string",
+//       "description": "An expression that controls how many hits of the
+//       breakpoint are ignored.\nThe debug adapter is expected to interpret the
+//       expression as needed."
+//     }
+//   },
+//   "required": [ "dataId" ]
+// }
+void request_setDataBreakpoints(const llvm::json::Object &request) {
+  llvm::json::Object response;
+  lldb::SBError error;
+  FillResponse(request, response);
+  auto arguments = request.getObject("arguments");
+  auto breakpoints = arguments->getArray("breakpoints");
+
+  std::unordered_set<std::string> breakpoint_ids;
+  for (const auto &breakpoint : *breakpoints) {
+    auto breakpoint_obj = *breakpoint.getAsObject();
+    auto breakpoint_id = Watchpoint::GetId(breakpoint_obj);
+    breakpoint_ids.emplace(breakpoint_id);
+
+    // Update watchpoint if it already exists
+    if (g_vsc.watchpoints.find(breakpoint_id) != g_vsc.watchpoints.end()) {
+      auto existing_watchpoint = g_vsc.watchpoints.at(breakpoint_id);
+      existing_watchpoint.Update(breakpoint_obj);
+      continue;
+    }
+
+    g_vsc.watchpoints.emplace(breakpoint_id, Watchpoint(breakpoint_obj, get_variable));
+  }
+
+  // If the record of watchpoints has entries that aren't present in the
+  // request, it means it's been deleted
+  std::vector<std::string> breakpoints_to_delete;
+  for (auto entry : g_vsc.watchpoints) {
+    auto id = entry.first;
+    if (breakpoint_ids.find(id) == breakpoint_ids.end()) {
+      breakpoints_to_delete.push_back(id);
+    }
+  }
+  for (auto entry : breakpoints_to_delete) {
+    g_vsc.watchpoints.at(entry).RemoveExisting();
+    g_vsc.watchpoints.erase(entry);
+  }
+
+  llvm::json::Object body;
+  llvm::json::Array breakpoints_to_return;
+  for (const auto &breakpoint_id: breakpoint_ids) {
+    breakpoints_to_return.emplace_back(g_vsc.watchpoints.at(breakpoint_id).GetResponse());
+  }
+
+  body.try_emplace("breakpoints", std::move(breakpoints_to_return));
+  response.try_emplace("body", std::move(body));
+  g_vsc.SendJSON(llvm::json::Value(std::move(response)));
+}
+
+void populate_data_breakpoint_info(llvm::json::Object &response, std::optional<std::string> data_id, std::string description, bool read, bool write) {
+    llvm::json::Object body;
+    llvm::json::Array access_types;
+    if (data_id.has_value())
+      body.try_emplace("dataId", data_id.value());
+    else
+      body.try_emplace("dataId", nullptr);
+
+    body.try_emplace("description", description);
+    body.try_emplace("canPersist", false);
+
+    if (read) {
+      access_types.emplace_back("read");
+      if (write)
+        access_types.emplace_back("readWrite");
+    }
+    if (write)
+      access_types.emplace_back("write");
+
+    body.try_emplace("accessTypes", std::move(access_types));
+    response.try_emplace("body", std::move(body));
+}
+
+// "DataBreakpointInfoRequest": {
+//   "allOf": [ { "$ref": "#/definitions/Request" }, {
+//     "type": "object",
+//     "description": "Obtains information on a possible data breakpoint that
+//     could be set on an expression or variable.\nClients should only call this
+//     request if the corresponding capability `supportsDataBreakpoints` is
+//     true.", "properties": {
+//       "command": {
+//         "type": "string",
+//         "enum": [ "dataBreakpointInfo" ]
+//       },
+//       "arguments": {
+//         "$ref": "#/definitions/DataBreakpointInfoArguments"
+//       }
+//     },
+//     "required": [ "command", "arguments"  ]
+//   }]
+// },
+// "DataBreakpointInfoArguments": {
+//   "type": "object",
+//   "description": "Arguments for `dataBreakpointInfo` request.",
+//   "properties": {
+//     "variablesReference": {
+//       "type": "integer",
+//       "description": "Reference to the variable container if the data
+//       breakpoint is requested for a child of the container. The
+//       `variablesReference` must have been obtained in the current suspended
+//       state. See 'Lifetime of Object References' in the Overview section for
+//       details."
+//     },
+//     "name": {
+//       "type": "string",
+//       "description": "The name of the variable's child to obtain data
+//       breakpoint information for.\nIf `variablesReference` isn't specified,
+//       this can be an expression."
+//     },
+//     "frameId": {
+//       "type": "integer",
+//       "description": "When `name` is an expression, evaluate it in the scope
+//       of this stack frame. If not specified, the expression is evaluated in
+//       the global scope. When `variablesReference` is specified, this property
+//       has no effect."
+//     }
+//   },
+//   "required": [ "name" ]
+// },
+// "DataBreakpointInfoResponse": {
+//   "allOf": [ { "$ref": "#/definitions/Response" }, {
+//     "type": "object",
+//     "description": "Response to `dataBreakpointInfo` request.",
+//     "properties": {
+//       "body": {
+//         "type": "object",
+//         "properties": {
+//           "dataId": {
+//             "type": [ "string", "null" ],
+//             "description": "An identifier for the data on which a data
+//             breakpoint can be registered with the `setDataBreakpoints`
+//             request or null if no data breakpoint is available."
+//           },
+//           "description": {
+//             "type": "string",
+//             "description": "UI string that describes on what data the
+//             breakpoint is set on or why a data breakpoint is not available."
+//           },
+//           "accessTypes": {
+//             "type": "array",
+//             "items": {
+//               "$ref": "#/definitions/DataBreakpointAccessType"
+//             },
+//             "description": "Attribute lists the available access types for a
+//             potential data breakpoint. A UI client could surface this
+//             information."
+//           },
+//           "canPersist": {
+//             "type": "boolean",
+//             "description": "Attribute indicates that a potential data
+//             breakpoint could be persisted across sessions."
+//           }
+//         },
+//         "required": [ "dataId", "description" ]
+//       }
+//     },
+//     "required": [ "body" ]
+//   }]
+// }
+void request_dataBreakpointInfo(const llvm::json::Object &request) {
+  llvm::json::Object response;
+  lldb::SBError error;
+  std::stringstream data;
+  std::stringstream description;
+  FillResponse(request, response);
+  auto arguments = request.getObject("arguments");
+  auto name = GetString(arguments, "name");
+  auto variables_reference = arguments->getInteger("variablesReference");
+  lldb::SBValue variable =
+      get_variable(name.str(), variables_reference.value());
+
+  if (!variable.IsValid()) {
+    description << "variable '" << name.data() << "' is not a valid variable.";
+    populate_data_breakpoint_info(response, std::nullopt, description.str(), false, false);
+    g_vsc.SendJSON(llvm::json::Value(std::move(response)));
+    return;
+  }
+
+  auto v_address = variable.GetLoadAddress();
+  auto v_size = variable.GetByteSize();
+
+  if (v_address == LLDB_INVALID_ADDRESS) {
+    populate_data_breakpoint_info(response, std::nullopt, "Variable has an invalid load address. This could be because it is being stored in a register. Try recompiling your program without optimizations.", false, false);
+    g_vsc.SendJSON(llvm::json::Value(std::move(response)));
+    return;
+  }
+
+  lldb::SBMemoryRegionInfo region_info;
+  error = g_vsc.target.GetProcess().GetMemoryRegionInfo(v_address, region_info);
+  data << name.str() << "/" << variables_reference.value();
+  description << name.data() << ": at address " << std::hex << "0x" << v_address
+              << " with size " << std::dec << v_size;
+  populate_data_breakpoint_info(response, data.str(), description.str(), error.Success() && region_info.IsReadable(), error.Success() && region_info.IsWritable());
+  g_vsc.SendJSON(llvm::json::Value(std::move(response)));
+}
+
 // "SetExceptionBreakpointsRequest": {
 //   "allOf": [ { "$ref": "#/definitions/Request" }, {
 //     "type": "object",
 //     "description": "SetExceptionBreakpoints request; value of command field
 //     is 'setExceptionBreakpoints'. The request configures the debuggers
 //     response to thrown exceptions. If an exception is configured to break, a
 //     StoppedEvent is fired (event type 'exception').", "properties": {
 //       "command": {
void request_setVariable(const llvm::json::Object &request) {
   FillResponse(request, response);
   llvm::json::Array variables;
   llvm::json::Object body;
   auto arguments = request.getObject("arguments");
   // This is a reference to the containing variable/scope
   const auto variablesReference =
       GetUnsigned(arguments, "variablesReference", 0);
   llvm::StringRef name = GetString(arguments, "name");
-  bool is_duplicated_variable_name = name.contains(" @");
 
   const auto value = GetString(arguments, "value");
   // Set success to false just in case we don't find the variable by name
   response.try_emplace("success", false);
 
   lldb::SBValue variable;
   int64_t newVariablesReference = 0;
 
   // The "id" is the unique integer ID that is unique within the enclosing
   // variablesReference. It is optionally added to any "interface Variable"
   // objects to uniquely identify a variable within an enclosing
   // variablesReference. It helps to disambiguate between two variables that
   // have the same name within the same scope since the "setVariables" request
   // only specifies the variable reference of the enclosing scope/variable, and
   // the name of the variable. We could have two shadowed variables with the
   // same name in "Locals" or "Globals". In our case the "id" absolute index
   // of the variable within the g_vsc.variables list.
   const auto id_value = GetUnsigned(arguments, "id", UINT64_MAX);
   if (id_value != UINT64_MAX) {
     variable = g_vsc.variables.GetVariable(id_value);
-  } else if (lldb::SBValueList *top_scope =
-                 GetTopLevelScope(variablesReference)) {
-    // variablesReference is one of our scopes, not an actual variable it is
-    // asking for a variable in locals or globals or registers
-    int64_t end_idx = top_scope->GetSize();
-    // Searching backward so that we choose the variable in closest scope
-    // among variables of the same name.
-    for (int64_t i = end_idx - 1; i >= 0; --i) {
-      lldb::SBValue curr_variable = top_scope->GetValueAtIndex(i);
-      std::string variable_name = CreateUniqueVariableNameForDisplay(
-          curr_variable, is_duplicated_variable_name);
-      if (variable_name == name) {
-        variable = curr_variable;
-        break;
-      }
-    }
   } else {
-    // This is not under the globals or locals scope, so there are no duplicated
-    // names.
-
-    // We have a named item within an actual variable so we need to find it
-    // withing the container variable by name.
-    lldb::SBValue container = g_vsc.variables.GetVariable(variablesReference);
-    variable = container.GetChildMemberWithName(name.data());
-    if (!variable.IsValid()) {
-      if (name.startswith("[")) {
-        llvm::StringRef index_str(name.drop_front(1));
-        uint64_t index = 0;
-        if (!index_str.consumeInteger(0, index)) {
-          if (index_str == "]")
-            variable = container.GetChildAtIndex(index);
-        }
-      }
-    }
+    variable = get_variable(name.str(), variablesReference);
   }
 
   if (variable.IsValid()) {
     lldb::SBError error;
     bool success = variable.SetValueFromCString(value.data(), error);
     if (success) {
       SetValueForKey(variable, body, "value");
       EmplaceSafeString(body, "type", variable.GetType().GetDisplayTypeName());
void RegisterRequestCallbacks() {
   g_vsc.RegisterRequestCallback("pause", request_pause);
   g_vsc.RegisterRequestCallback("scopes", request_scopes);
   g_vsc.RegisterRequestCallback("setBreakpoints", request_setBreakpoints);
   g_vsc.RegisterRequestCallback("setExceptionBreakpoints",
                                 request_setExceptionBreakpoints);
   g_vsc.RegisterRequestCallback("setFunctionBreakpoints",
                                 request_setFunctionBreakpoints);
   g_vsc.RegisterRequestCallback("setVariable", request_setVariable);
+  g_vsc.RegisterRequestCallback("setDataBreakpoints",
+                                request_setDataBreakpoints);
+  g_vsc.RegisterRequestCallback("dataBreakpointInfo",
+                                request_dataBreakpointInfo);
   g_vsc.RegisterRequestCallback("source", request_source);
   g_vsc.RegisterRequestCallback("stackTrace", request_stackTrace);
   g_vsc.RegisterRequestCallback("stepIn", request_stepIn);
   g_vsc.RegisterRequestCallback("stepOut", request_stepOut);
   g_vsc.RegisterRequestCallback("threads", request_threads);
   g_vsc.RegisterRequestCallback("variables", request_variables);
   // Custom requests
   g_vsc.RegisterRequestCallback("compileUnits", request_compileUnits);