[Lldb-commits] [lldb] [lldb] Add support for updating string during debug process (PR #67782)

[via lldb-commits]

================
@@ -0,0 +1,171 @@
+#include "LibCxx.h"
+#include "LibCxxStringInfoExtractor.h"
+
+#include "lldb/DataFormatters/FormattersHelpers.h"
+#include <unordered_map>
+
+using namespace lldb;
+using namespace lldb_private;
+
+namespace {
+
+class StringFrontend : public SyntheticChildrenFrontEnd {
+
+public:
+  StringFrontend(ValueObject &valobj, const char *prefix = "")
+      : SyntheticChildrenFrontEnd(valobj), m_prefix(prefix) {}
+
+  llvm::Expected<uint32_t> CalculateNumChildren() override {
+    return m_size + m_special_members_count;
+  }
+
+  lldb::ValueObjectSP GetChildAtIndex(uint32_t idx) override {
+
+    if (idx < m_special_members_count) {
+      return m_backend.GetChildMemberWithName(ConstString("__r_"),
+                                              /*can_create=*/true);
+    }
+
+    idx -= m_special_members_count;
+
+    if (!m_str_data_ptr || idx > m_size || !m_element_size) {
+      return {};
+    }
+
+    auto char_it = m_chars.find(idx);
+    if (char_it != m_chars.end()) {
+      return char_it->second;
+    }
+
+    uint64_t offset = idx * m_element_size;
+    uint64_t address = m_str_data_ptr->GetValueAsUnsigned(0);
+
+    if (!address) {
+      return {};
+    }
+
+    StreamString name;
+    name.Printf("[%" PRIu64 "]", (uint64_t)idx);
+
+    m_chars[idx] = CreateValueObjectFromAddress(
+        name.GetString(), address + offset, m_backend.GetExecutionContextRef(),
+        m_element_type);
+
+    return m_chars[idx];
+  }
+
+  size_t GetIndexOfChildWithName(ConstString name) override {
+    if (name == "__r_") {
+      return 0;
+    }
+    return formatters::ExtractIndexFromString(name.GetCString()) +
+           m_special_members_count;
+  }
+
+  ChildCacheState Update() override {
+
+    clear();
+
+    auto string_info = ExtractLibcxxStringInfo(m_backend);
+    if (!string_info)
+      return ChildCacheState::eRefetch;
+    std::tie(m_size, m_str_data_ptr) = *string_info;
+
+    m_element_type = m_backend.GetCompilerType().GetTypeTemplateArgument(0);
+    m_element_size = m_element_type.GetByteSize(nullptr).value_or(0);
+
+    if (m_str_data_ptr->IsArrayType()) {
+      // this means the string is in short-mode and the
+      // data is stored inline in array,
+      // so we need address of this array
+      Status status;
+      m_str_data_ptr = m_str_data_ptr->AddressOf(status);
+    }
+
+    return ChildCacheState::eReuse;
+  }
+
+  bool MightHaveChildren() override { return true; }
+
+  bool SetValueFromCString(const char *value_str, Status &error) override {
+
+    ValueObjectSP expr_value_sp;
+
+    std::unique_lock<std::recursive_mutex> lock;
+    ExecutionContext exe_ctx(m_backend.GetExecutionContextRef(), lock);
+
+    Target *target = exe_ctx.GetTargetPtr();
+    StackFrame *frame = exe_ctx.GetFramePtr();
+
+    if (target && frame) {
----------------
jimingham wrote:

I don't think this is the right way to implement this.  By relying on the base name of the backend and using that directly you will only be able to handle cases where the std::string is a local variable or function argument.  It won't work on std::strings inside other objects - you would need the full path.  It also won't work for globals, which don't have frames, even though there's no reason it shouldn't.

A better approach would be to get the address of the object you are trying to change, and using that address properly cast to do the assignment.  That way you wouldn't have to care how that value got referred to.  

For the most part we try not to run expressions in the ValueObject code, but setting values is less likely to happen for large objects, or to lots of variables at once the way rendering their values/Summaries, etc does.  But you certainly shouldn't run expressions on a ValueObject's behalf that allow more than one thread to run.  Otherwise you get into the state where I stop in thread A, see something interesting, go to thread B and change a value and switch back to thread A and it's not where I left it!

https://github.com/llvm/llvm-project/pull/67782