[Lldb-commits] [lldb] [lldb] Improve error when redefining persistent vars (PR #227297)
Raphael Isemann via lldb-commits
lldb-commits at lists.llvm.org
Tue Sep 29 05:49:31 PDT 2026
https://github.com/Teemperor created https://github.com/llvm/llvm-project/pull/227297
A redefinition of a persistent variable currently yields the following error:
```
(lldb) expr int $var = 13423
error: redefinition of persistent variable '$var'
error: Internal error [IRForTarget]: Couldn't rewrite the creation of a persistent variable
```
The internal error makes this look like it's an LLDB bug, but this is really just a normal error. This patch is mostly just a bunch of error propagation logic so we actually know what went wrong in the calling logic.
This patch also adds some errors messages where we previously only returned `false` (as returning an actual error message is now mandatory for all code paths). I don't know if we can actually test any of these as most of this is only code that is reachable if there is an actual LLDB or system bug.
assisted-by: claude
>From 8b43985942c96da3931c85cd09063d43de867683 Mon Sep 17 00:00:00 2001
From: Raphael Isemann <rise at apple.com>
Date: Tue, 29 Sep 2026 13:13:13 +0100
Subject: [PATCH] [lldb] Improve error when redefining persistent vars
A redefinition of a persistent variable currently yields the following
error:
```
(lldb) expr int $var = 13423
error: redefinition of persistent variable '$var'
error: Internal error [IRForTarget]: Couldn't rewrite the creation of a persistent variable
```
The internal error makes this look like it's an LLDB bug, but this is
really just a normal error. This patch is mostly just a bunch of error
propagation logic so we actually know what went wrong in the calling
logic.
This patch also adds some errors messages where we previously only
returned `false` (as returning an actual error message is now mandatory
for all code paths). I don't know if we can actually test any of these
as most of this is only code that is reachable if there is an actual
LLDB or system bug.
assisted-by: claude
---
.../Python/lldbsuite/test/lldbutil.py | 21 +++++++++
.../Clang/ClangExpressionDeclMap.cpp | 44 ++++++++++---------
.../Clang/ClangExpressionDeclMap.h | 8 ++--
.../ExpressionParser/Clang/IRForTarget.cpp | 37 +++++++++-------
.../ExpressionParser/Clang/IRForTarget.h | 5 ++-
.../TestPersistentVariables.py | 14 +++---
6 files changed, 78 insertions(+), 51 deletions(-)
diff --git a/lldb/packages/Python/lldbsuite/test/lldbutil.py b/lldb/packages/Python/lldbsuite/test/lldbutil.py
index e78ef467f71d7..73e84bc05466e 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbutil.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbutil.py
@@ -872,6 +872,27 @@ def check_breakpoint(
test.assertEqual(expected_location_hit_count, loc_bkpt.GetHitCount())
+# ==================================================
+# Utility functions for expressions
+# ==================================================
+
+
+def get_expr_error_messages(test, frame, expr):
+ """Evaluates the given expression in the given frame, asserts that it
+ failed and returns the messages of all error diagnostics it produced."""
+ value = frame.EvaluateExpression(expr)
+ error = value.GetError()
+ test.assertTrue(error.Fail(), "Expression unexpectedly succeeded: " + expr)
+ diags = error.GetErrorData().GetValueForKey("errors").GetItemAtIndex(0)
+ details = diags.GetValueForKey("details")
+ messages = []
+ for i in range(details.GetSize()):
+ detail = details.GetItemAtIndex(i)
+ if str(detail.GetValueForKey("severity")) == "error":
+ messages.append(str(detail.GetValueForKey("message")))
+ return messages
+
+
# ==================================================
# Utility functions related to Threads and Processes
# ==================================================
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
index 351afdbbb2609..d54500d3bc830 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
@@ -218,24 +218,20 @@ TypeFromUser ClangExpressionDeclMap::DeportType(TypeSystemClang &target,
return TypeFromUser(m_ast_importer_sp->DeportType(target, parser_type));
}
-bool ClangExpressionDeclMap::AddPersistentVariable(const NamedDecl *decl,
- ConstString name,
- TypeFromParser parser_type,
- bool is_result,
- bool is_lvalue) {
+llvm::Error ClangExpressionDeclMap::AddPersistentVariable(
+ const NamedDecl *decl, ConstString name, TypeFromParser parser_type,
+ bool is_result, bool is_lvalue) {
assert(m_parser_vars.get());
auto ast = parser_type.GetTypeSystem<TypeSystemClang>();
if (ast == nullptr)
- return false;
+ return llvm::createStringError(
+ "persistent variable type is not a Clang type");
// Check if we already declared a persistent variable with the same name.
if (lldb::ExpressionVariableSP conflicting_var =
m_parser_vars->m_persistent_vars->GetVariable(name)) {
- std::string msg = llvm::formatv("redefinition of persistent variable '{0}'",
- name).str();
- m_parser_vars->m_diagnostics->AddDiagnostic(
- msg, lldb::eSeverityError, DiagnosticOrigin::eDiagnosticOriginLLDB);
- return false;
+ return llvm::createStringError(
+ llvm::formatv("redefinition of persistent variable '{0}'", name));
}
if (m_parser_vars->m_materializer && is_result) {
@@ -244,11 +240,13 @@ bool ClangExpressionDeclMap::AddPersistentVariable(const NamedDecl *decl,
ExecutionContext &exe_ctx = m_parser_vars->m_exe_ctx;
Target *target = exe_ctx.GetTargetPtr();
if (target == nullptr)
- return false;
+ return llvm::createStringError(
+ "no target to create the result variable in");
auto clang_ast_context = GetScratchContext(*target);
if (!clang_ast_context)
- return false;
+ return llvm::createStringError(
+ "couldn't get the scratch type system of the target");
TypeFromUser user_type = DeportType(*clang_ast_context, *ast, parser_type);
@@ -275,31 +273,34 @@ bool ClangExpressionDeclMap::AddPersistentVariable(const NamedDecl *decl,
jit_vars->m_offset = offset;
- return true;
+ return llvm::Error::success();
}
Log *log = GetLog(LLDBLog::Expressions);
ExecutionContext &exe_ctx = m_parser_vars->m_exe_ctx;
Target *target = exe_ctx.GetTargetPtr();
if (target == nullptr)
- return false;
+ return llvm::createStringError(
+ "no target to create the persistent variable in");
auto context = GetScratchContext(*target);
if (!context)
- return false;
+ return llvm::createStringError(
+ "couldn't get the scratch type system of the target");
TypeFromUser user_type = DeportType(*context, *ast, parser_type);
if (!user_type.GetOpaqueQualType()) {
LLDB_LOG(log, "Persistent variable's type wasn't copied successfully");
- return false;
+ return llvm::createStringError(
+ "persistent variable's type wasn't copied successfully");
}
if (!m_parser_vars->m_target_info.IsValid())
- return false;
+ return llvm::createStringError("invalid target information");
if (!m_parser_vars->m_persistent_vars)
- return false;
+ return llvm::createStringError("no persistent variable storage");
ClangExpressionVariable *var = llvm::cast<ClangExpressionVariable>(
m_parser_vars->m_persistent_vars
@@ -310,7 +311,8 @@ bool ClangExpressionDeclMap::AddPersistentVariable(const NamedDecl *decl,
.get());
if (!var)
- return false;
+ return llvm::createStringError(
+ llvm::formatv("couldn't create persistent variable '{0}'", name));
var->m_frozen_sp->SetHasCompleteType();
@@ -342,7 +344,7 @@ bool ClangExpressionDeclMap::AddPersistentVariable(const NamedDecl *decl,
parser_vars->m_named_decl = decl;
- return true;
+ return llvm::Error::success();
}
bool ClangExpressionDeclMap::AddValueToStruct(const NamedDecl *decl,
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
index 6573d1c875b3a..fd60222c2e5d2 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
@@ -129,10 +129,10 @@ class ClangExpressionDeclMap : public ClangASTSource {
/// The type of the variable, in the Clang parser's context.
///
/// \return
- /// True on success; false otherwise.
- bool AddPersistentVariable(const clang::NamedDecl *decl,
- ConstString name, TypeFromParser type,
- bool is_result, bool is_lvalue);
+ /// An error describing why the variable couldn't be added, or success.
+ llvm::Error AddPersistentVariable(const clang::NamedDecl *decl,
+ ConstString name, TypeFromParser type,
+ bool is_result, bool is_lvalue);
/// [Used by IRForTarget] Add a variable to the struct that needs to
/// be materialized each time the expression runs.
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp b/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
index f9ddce8e1be03..f83f939c6bb5a 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
@@ -394,9 +394,14 @@ bool IRForTarget::CreateResultVariable(llvm::Function &llvm_function) {
result_global->replaceAllUsesWith(new_result_global);
}
- if (!m_decl_map->AddPersistentVariable(
- result_decl, m_result_name, m_result_type, true, m_result_is_pointer))
+ if (llvm::Error err = m_decl_map->AddPersistentVariable(
+ result_decl, m_result_name, m_result_type, true,
+ m_result_is_pointer)) {
+ m_error_stream.Format("Internal error: Couldn't create the result "
+ "variable: {0}",
+ llvm::toString(std::move(err)));
return false;
+ }
result_global->eraseFromParent();
@@ -874,8 +879,8 @@ bool IRForTarget::RewriteObjCSelectors(BasicBlock &basic_block) {
return true;
}
-// This function does not report errors; its callers are responsible.
-bool IRForTarget::RewritePersistentAlloc(llvm::Instruction *persistent_alloc) {
+llvm::Error
+IRForTarget::RewritePersistentAlloc(llvm::Instruction *persistent_alloc) {
lldb_private::Log *log(GetLog(LLDBLog::Expressions));
AllocaInst *alloc = dyn_cast<AllocaInst>(persistent_alloc);
@@ -883,13 +888,15 @@ bool IRForTarget::RewritePersistentAlloc(llvm::Instruction *persistent_alloc) {
MDNode *alloc_md = alloc->getMetadata("clang.decl.ptr");
if (!alloc_md || !alloc_md->getNumOperands())
- return false;
+ return llvm::createStringError("Internal error: Couldn't rewrite the "
+ "creation of a persistent variable");
ConstantInt *constant_int =
mdconst::dyn_extract<ConstantInt>(alloc_md->getOperand(0));
if (!constant_int)
- return false;
+ return llvm::createStringError("Internal error: Couldn't rewrite the "
+ "creation of a persistent variable");
// We attempt to register this as a new persistent variable with the DeclMap.
@@ -902,9 +909,9 @@ bool IRForTarget::RewritePersistentAlloc(llvm::Instruction *persistent_alloc) {
StringRef decl_name(decl->getName());
lldb_private::ConstString persistent_variable_name(decl_name);
- if (!m_decl_map->AddPersistentVariable(decl, persistent_variable_name,
- result_decl_type, false, false))
- return false;
+ if (llvm::Error err = m_decl_map->AddPersistentVariable(
+ decl, persistent_variable_name, result_decl_type, false, false))
+ return err;
GlobalVariable *persistent_global = new GlobalVariable(
(*m_module), alloc->getType(), false, /* not constant */
@@ -939,7 +946,7 @@ bool IRForTarget::RewritePersistentAlloc(llvm::Instruction *persistent_alloc) {
alloc->replaceAllUsesWith(persistent_load);
alloc->eraseFromParent();
- return true;
+ return llvm::Error::success();
}
bool IRForTarget::RewritePersistentAllocs(llvm::BasicBlock &basic_block) {
@@ -972,12 +979,10 @@ bool IRForTarget::RewritePersistentAllocs(llvm::BasicBlock &basic_block) {
}
for (Instruction *inst : pvar_allocs) {
- if (!RewritePersistentAlloc(inst)) {
- m_error_stream.Printf("Internal error [IRForTarget]: Couldn't rewrite "
- "the creation of a persistent variable\n");
-
- LLDB_LOG(log, "Couldn't rewrite the creation of a persistent variable");
-
+ if (llvm::Error err = RewritePersistentAlloc(inst)) {
+ std::string msg = llvm::toString(std::move(err));
+ LLDB_LOG(log, "Couldn't rewrite a persistent variable: {0}", msg);
+ m_error_stream << msg;
return false;
}
}
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.h b/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.h
index 575d9f83b8c90..50f34c22c7a07 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.h
@@ -206,8 +206,9 @@ class IRForTarget {
/// The allocation of the persistent variable.
///
/// \return
- /// True on success; false otherwise
- bool RewritePersistentAlloc(llvm::Instruction *persistent_alloc);
+ /// An error describing why the allocation couldn't be rewritten, or
+ /// success.
+ llvm::Error RewritePersistentAlloc(llvm::Instruction *persistent_alloc);
/// The top-level pass implementation
///
diff --git a/lldb/test/API/commands/expression/persistent_variables/TestPersistentVariables.py b/lldb/test/API/commands/expression/persistent_variables/TestPersistentVariables.py
index d0d9358589a71..95ac188804e46 100644
--- a/lldb/test/API/commands/expression/persistent_variables/TestPersistentVariables.py
+++ b/lldb/test/API/commands/expression/persistent_variables/TestPersistentVariables.py
@@ -53,19 +53,17 @@ def test_persistent_variables(self):
# Try redeclaring the persistent variable with the same type.
# This should be rejected as we treat them as if they are globals.
- self.expect(
- "expr int $i = 123",
- error=True,
- substrs=["redefinition of persistent variable '$i'"],
+ self.assertEqual(
+ lldbutil.get_expr_error_messages(self, self.frame(), "int $i = 123"),
+ ["redefinition of persistent variable '$i'"],
)
self.expect_expr("$i", result_type="int", result_value="5")
# Try redeclaring the persistent variable with another type. Should
# also be rejected.
- self.expect(
- "expr long $i = 123",
- error=True,
- substrs=["redefinition of persistent variable '$i'"],
+ self.assertEqual(
+ lldbutil.get_expr_error_messages(self, self.frame(), "long $i = 123"),
+ ["redefinition of persistent variable '$i'"],
)
self.expect_expr("$i", result_type="int", result_value="5")
More information about the lldb-commits
mailing list