https://github.com/kastiglione updated https://github.com/llvm/llvm-project/pull/200084
>From 9fa77da8e42e4ebf693bcc54c6295880dfadc06c Mon Sep 17 00:00:00 2001 From: Dave Lee <[email protected]> Date: Wed, 27 May 2026 16:41:08 -0700 Subject: [PATCH 1/3] [lldb] Improve frame variable handling of recognizer arguments Fixes two bugs. The bug which prompted this change is that `frame variable name` would print all regcognizer arguments, even though only "name" was requested. While making a test for the first bug, I found that `frame variable recognizer_arg` will both successfully print the variable, and also report a false positive error: ``` error: <user expression>:1:1: use of undeclared identifier 'recognizer_arg' 1 | recognizer_arg `` This change fixes both bugs, including when using `--regex`. --- lldb/source/Commands/CommandObjectFrame.cpp | 106 ++++++++++-------- .../frame/recognizer/TestFrameRecognizer.py | 21 ++++ 2 files changed, 79 insertions(+), 48 deletions(-) diff --git a/lldb/source/Commands/CommandObjectFrame.cpp b/lldb/source/Commands/CommandObjectFrame.cpp index b9e6130831b92..7771dd67c64e6 100644 --- a/lldb/source/Commands/CommandObjectFrame.cpp +++ b/lldb/source/Commands/CommandObjectFrame.cpp @@ -642,10 +642,21 @@ may even involve JITing and running code in the target program.)"); if (sym_ctx.function && sym_ctx.function->IsTopLevelFunction()) m_option_variable.show_globals = true; - if (variable_list) { - const Format format = m_option_format.GetFormat(); - options.SetFormat(format); + ValueObjectListSP recognized_arg_list; + if (m_option_variable.show_recognized_args) + if (auto recognized_frame = frame->GetRecognizedFrame()) + recognized_arg_list = recognized_frame->GetRecognizedArguments(); + + const Format format = m_option_format.GetFormat(); + options.SetFormat(format); + + auto print_value = [&result, options](ValueObjectSP valobj_sp) { + result.GetValueObjectList().Append(valobj_sp); + if (auto error = valobj_sp->Dump(result.GetOutputStream(), options)) + result.AppendError(toString(std::move(error))); + }; + if (variable_list) { if (!command.empty()) { VariableList regex_var_list; @@ -659,9 +670,21 @@ may even involve JITing and running code in the target program.)"); std::optional<llvm::ArrayRef<VariableSP>> results = findUniqueRegexMatches(regex, regex_var_list, *variable_list); if (!results) { - result.AppendErrorWithFormat( - "no variables matched the regular expression '%s'", - entry.c_str()); + // No variables matched. Try recognized args as fallback. + bool found_recognized = false; + if (recognized_arg_list) { + for (auto &rec_value_sp : recognized_arg_list->GetObjects()) { + if (regex.Execute(rec_value_sp->GetName())) { + found_recognized = true; + print_value(rec_value_sp); + } + } + } + if (!found_recognized) { + result.AppendErrorWithFormat( + "no variables matched the regular expression '%s'", + entry.c_str()); + } continue; } for (const VariableSP &var_sp : *results) { @@ -710,7 +733,7 @@ may even involve JITing and running code in the target program.)"); valobj_sp = frame->GetValueForVariableExpressionPath( entry.ref(), m_varobj_options.use_dynamic, expr_path_options, var_sp, error); - if (valobj_sp) { + if (valobj_sp && error.Success()) { result.GetValueObjectList().Append(valobj_sp); std::string scope_string; @@ -732,29 +755,33 @@ may even involve JITing and running code in the target program.)"); Stream &output_stream = result.GetOutputStream(); options.SetRootValueObjectName( valobj_sp->GetParent() ? entry.c_str() : nullptr); - // Check only the `error` argument, because doing - // `valobj_sp->GetError()` will update the value and potentially - // return a new error that happens during the update, even if - // `GetValueForVariableExpressionPath` reported no errors. - if (error.Fail()) { - result.SetStatus(eReturnStatusFailed); - result.SetError(error.takeError()); - } else { - // If there is an error while updating the value, it will be - // printed here as the contents of the value, e.g. - // `(int) *((int*)0) = <parent is NULL>` - if (llvm::Error error = valobj_sp->Dump(output_stream, options)) - result.AppendError(toString(std::move(error))); - } + // If there is an error while updating the value, it will be + // printed here as the contents of the value, e.g. + // `(int) *((int*)0) = <parent is NULL>` + if (llvm::Error error = valobj_sp->Dump(output_stream, options)) + result.AppendError(toString(std::move(error))); } else { - if (auto error_cstr = error.AsCString(nullptr)) - result.AppendError(error_cstr); - else - result.AppendErrorWithFormat( - "unable to find any variable expression path that matches " - "'%s'", - entry.c_str()); + // Variable lookup failed. Check recognized args as a fallback. + bool found_recognized = false; + if (recognized_arg_list) { + for (auto &obj_sp : recognized_arg_list->GetObjects()) { + if (obj_sp->GetName() == entry.ref()) { + found_recognized = true; + print_value(obj_sp); + break; + } + } + } + if (!found_recognized) { + if (const auto *error_cstr = error.AsCString(nullptr)) + result.AppendError(error_cstr); + else + result.AppendErrorWithFormat( + "unable to find any variable expression path that " + "matches '%s'", + entry.c_str()); + } } } } @@ -811,26 +838,9 @@ may even involve JITing and running code in the target program.)"); result.SetStatus(eReturnStatusSuccessFinishResult); } - if (m_option_variable.show_recognized_args) { - auto recognized_frame = frame->GetRecognizedFrame(); - if (recognized_frame) { - ValueObjectListSP recognized_arg_list = - recognized_frame->GetRecognizedArguments(); - if (recognized_arg_list) { - for (auto &rec_value_sp : recognized_arg_list->GetObjects()) { - result.GetValueObjectList().Append(rec_value_sp); - options.SetFormat(m_option_format.GetFormat()); - options.SetVariableFormatDisplayLanguage( - rec_value_sp->GetPreferredDisplayLanguage()); - options.SetRootValueObjectName( - rec_value_sp->GetName().AsCString(nullptr)); - if (llvm::Error error = - rec_value_sp->Dump(result.GetOutputStream(), options)) - result.AppendError(toString(std::move(error))); - } - } - } - } + if (recognized_arg_list && (command.empty() || !variable_list)) + for (auto &rec_value_sp : recognized_arg_list->GetObjects()) + print_value(rec_value_sp); m_interpreter.PrintWarningsIfNecessary(result.GetOutputStream(), m_cmd_name); diff --git a/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py b/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py index e0c107fc51302..68df086143585 100644 --- a/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py +++ b/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py @@ -162,6 +162,27 @@ def test_frame_recognizer_1(self): substrs=['*a = 78']) """ + def test_recognized_args_filtered_by_name(self): + """Test that 'frame variable <name>' only prints matching recognized args.""" + self.build() + lldbutil.run_to_name_breakpoint(self, "foo") + + self.runCmd("frame recognizer clear") + self.runCmd("command script import recognizer.py") + self.runCmd( + "frame recognizer add -l recognizer.MyFrameRecognizer -s a.out -n foo" + ) + + # With no args, both recognized args are printed. + self.expect("frame variable", substrs=["(int) a = 42", "(int) b = 56"]) + + # With a specific name, only the matching recognized arg is printed. + self.expect("frame variable a", substrs=["(int) a = 42"]) + self.expect("frame variable a", matching=False, substrs=["b = 56"]) + + self.expect("frame variable b", substrs=["(int) b = 56"]) + self.expect("frame variable b", matching=False, substrs=["a = 42"]) + def test_frame_recognizer_hiding(self): self.build() >From 2dfff89c587d2adbf70f6b783e8922164274d5a2 Mon Sep 17 00:00:00 2001 From: Dave Lee <[email protected]> Date: Wed, 27 May 2026 17:13:09 -0700 Subject: [PATCH 2/3] Tweak error handling --- lldb/source/Commands/CommandObjectFrame.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lldb/source/Commands/CommandObjectFrame.cpp b/lldb/source/Commands/CommandObjectFrame.cpp index 7771dd67c64e6..0569ae91ed69d 100644 --- a/lldb/source/Commands/CommandObjectFrame.cpp +++ b/lldb/source/Commands/CommandObjectFrame.cpp @@ -774,8 +774,12 @@ may even involve JITing and running code in the target program.)"); } } if (!found_recognized) { - if (const auto *error_cstr = error.AsCString(nullptr)) - result.AppendError(error_cstr); + // Check only the `error` argument, because doing + // `valobj_sp->GetError()` will update the value and potentially + // return a new error that happens during the update, even if + // `GetValueForVariableExpressionPath` reported no errors. + if (error.Fail()) + result.SetError(error.takeError()); else result.AppendErrorWithFormat( "unable to find any variable expression path that " >From 04d9eeb5c6d0d135a01a856af6bc2db7ed41d5a3 Mon Sep 17 00:00:00 2001 From: Dave Lee <[email protected]> Date: Wed, 27 May 2026 18:23:06 -0700 Subject: [PATCH 3/3] Remove optional {} --- lldb/source/Commands/CommandObjectFrame.cpp | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/lldb/source/Commands/CommandObjectFrame.cpp b/lldb/source/Commands/CommandObjectFrame.cpp index 0569ae91ed69d..742f7026464f0 100644 --- a/lldb/source/Commands/CommandObjectFrame.cpp +++ b/lldb/source/Commands/CommandObjectFrame.cpp @@ -672,14 +672,12 @@ may even involve JITing and running code in the target program.)"); if (!results) { // No variables matched. Try recognized args as fallback. bool found_recognized = false; - if (recognized_arg_list) { - for (auto &rec_value_sp : recognized_arg_list->GetObjects()) { + if (recognized_arg_list) + for (auto &rec_value_sp : recognized_arg_list->GetObjects()) if (regex.Execute(rec_value_sp->GetName())) { found_recognized = true; print_value(rec_value_sp); } - } - } if (!found_recognized) { result.AppendErrorWithFormat( "no variables matched the regular expression '%s'", @@ -764,15 +762,13 @@ may even involve JITing and running code in the target program.)"); } else { // Variable lookup failed. Check recognized args as a fallback. bool found_recognized = false; - if (recognized_arg_list) { - for (auto &obj_sp : recognized_arg_list->GetObjects()) { + if (recognized_arg_list) + for (auto &obj_sp : recognized_arg_list->GetObjects()) if (obj_sp->GetName() == entry.ref()) { found_recognized = true; print_value(obj_sp); break; } - } - } if (!found_recognized) { // Check only the `error` argument, because doing // `valobj_sp->GetError()` will update the value and potentially _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
