From: Raphael Isemann Date: Wed, 14 Oct 2020 07:50:59 +0000 (+0200) Subject: [lldb] Reject redefinitions of persistent variables X-Git-Tag: llvmorg-13-init~9330 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=cb81e662a58908913f342520e4c010564a68126a;p=platform%2Fupstream%2Fllvm.git [lldb] Reject redefinitions of persistent variables Currently one can redefine a persistent variable and LLDB will just silently ignore the second definition: ``` (lldb) expr int $i = 1 (lldb) expr int $i = 2 (lldb) expr $i (int) $i = 1 ``` This patch makes this an error and rejects the expression with the second definition. A nice follow up would be to refactor LLDB's persistent variables to not just be a pair of type and name, but also contain some way to obtain the original declaration and source code that declared the variable. That way we could actually make a full diagnostic as we would get from redefining a variable twice in the same expression. Reviewed By: labath, shafik, JDevlieghere Differential Revision: https://reviews.llvm.org/D89310 --- diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp index 8c49898..7d8cd85 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp @@ -19,6 +19,7 @@ #include "lldb/Core/ModuleSpec.h" #include "lldb/Core/ValueObjectConstResult.h" #include "lldb/Core/ValueObjectVariable.h" +#include "lldb/Expression/DiagnosticManager.h" #include "lldb/Expression/Materializer.h" #include "lldb/Symbol/CompileUnit.h" #include "lldb/Symbol/CompilerDecl.h" @@ -125,6 +126,12 @@ void ClangExpressionDeclMap::InstallCodeGenerator( m_parser_vars->m_code_gen = code_gen; } +void ClangExpressionDeclMap::InstallDiagnosticManager( + DiagnosticManager &diag_manager) { + assert(m_parser_vars); + m_parser_vars->m_diagnostics = &diag_manager; +} + void ClangExpressionDeclMap::DidParse() { if (m_parser_vars && m_parser_vars->m_persistent_vars) { for (size_t entity_index = 0, num_entities = m_found_entities.GetSize(); @@ -196,6 +203,17 @@ bool ClangExpressionDeclMap::AddPersistentVariable(const NamedDecl *decl, if (ast == nullptr) return false; + // 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, DiagnosticSeverity::eDiagnosticSeverityError, + DiagnosticOrigin::eDiagnosticOriginLLDB); + return false; + } + if (m_parser_vars->m_materializer && is_result) { Status err; diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h index 6974535..0c81d46 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h @@ -102,6 +102,8 @@ public: void InstallCodeGenerator(clang::ASTConsumer *code_gen); + void InstallDiagnosticManager(DiagnosticManager &diag_manager); + /// Disable the state needed for parsing and IR transformation. void DidParse(); @@ -330,6 +332,8 @@ private: clang::ASTConsumer *m_code_gen = nullptr; ///< If non-NULL, a code generator ///that receives new top-level ///functions. + DiagnosticManager *m_diagnostics = nullptr; + private: ParserVars(const ParserVars &) = delete; const ParserVars &operator=(const ParserVars &) = delete; diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp index 8ad39ec..bd0d831 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp @@ -1078,6 +1078,7 @@ ClangExpressionParser::ParseInternal(DiagnosticManager &diagnostic_manager, ClangExpressionDeclMap *decl_map = type_system_helper->DeclMap(); if (decl_map) { decl_map->InstallCodeGenerator(&m_compiler->getASTConsumer()); + decl_map->InstallDiagnosticManager(diagnostic_manager); clang::ExternalASTSource *ast_source = decl_map->CreateProxy(); diff --git a/lldb/test/API/commands/expression/persistent_variables/TestPersistentVariables.py b/lldb/test/API/commands/expression/persistent_variables/TestPersistentVariables.py index ebe1809..bdd1ccc 100644 --- a/lldb/test/API/commands/expression/persistent_variables/TestPersistentVariables.py +++ b/lldb/test/API/commands/expression/persistent_variables/TestPersistentVariables.py @@ -41,3 +41,19 @@ class PersistentVariablesTestCase(TestBase): # Test that $200 wasn't created by the previous expression. self.expect("expr $200", error=True, substrs=["use of undeclared identifier '$200'"]) + + # 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=["error: 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=["error: redefinition of persistent variable '$i'"]) + self.expect_expr("$i", result_type="int", result_value="5") + + # Try assigning the persistent variable a new value. + self.expect("expr $i = 55") + self.expect_expr("$i", result_type="int", result_value="55")