From ec323e992bebd4300686bf5519f363b80176b7f0 Mon Sep 17 00:00:00 2001 From: Adrian Prantl Date: Tue, 28 Jan 2020 10:59:23 -0800 Subject: [PATCH 1/2] Delete dead code. (NFC) --- lldb/include/lldb/Symbol/SymbolFile.h | 7 --- .../SymbolFile/DWARF/SymbolFileDWARF.cpp | 54 ------------------- .../SymbolFile/DWARF/SymbolFileDWARF.h | 3 -- 3 files changed, 64 deletions(-) diff --git a/lldb/include/lldb/Symbol/SymbolFile.h b/lldb/include/lldb/Symbol/SymbolFile.h index 17a62452a7f0f..882831411446c 100644 --- a/lldb/include/lldb/Symbol/SymbolFile.h +++ b/lldb/include/lldb/Symbol/SymbolFile.h @@ -280,13 +280,6 @@ class SymbolFile : public PluginInterface { return false; } - virtual int GetCompileOptions(const char *option, - std::vector &values, - CompileUnit *cu = nullptr) { - values.clear(); - return false; - } - // Some symbol files might know if we should always check for inline // source file and line entries. This virtual function lets // SymbolFile subclasses control that, but a default implementation diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index 36f57e75624df..0e72eb4573480 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -3144,60 +3144,6 @@ bool SymbolFileDWARF::GetCompileOption(const char *option, std::string &value, return false; } -int SymbolFileDWARF::GetCompileOptions(const char *option, - std::vector &values, - CompileUnit *cu) { - DWARFDebugInfo *debug_info = DebugInfo(); - - if (debug_info) { - if (cu) { - DWARFUnit *dwarf_cu = GetDWARFCompileUnit(cu); - - if (dwarf_cu) { - const DWARFBaseDIE die = dwarf_cu->GetUnitDIEOnly(); - if (die) { - const char *flags = - die.GetAttributeValueAsString(DW_AT_APPLE_flags, NULL); - - if (flags) { - if (strstr(flags, option)) { - Args compiler_args(flags); - - return OptionParsing::GetOptionValuesAsStrings(compiler_args, - option, values); - } - } - } - } - } else { - const uint32_t num_compile_units = GetNumCompileUnits(); - - for (uint32_t cu_idx = 0; cu_idx < num_compile_units; ++cu_idx) { - DWARFUnit *dwarf_cu = debug_info->GetUnitAtIndex(cu_idx); - - if (dwarf_cu) { - const DWARFBaseDIE die = dwarf_cu->GetUnitDIEOnly(); - if (die) { - const char *flags = - die.GetAttributeValueAsString(DW_AT_APPLE_flags, NULL); - - if (flags) { - if (strstr(flags, option)) { - Args compiler_args(flags); - - return OptionParsing::GetOptionValuesAsStrings(compiler_args, - option, values); - } - } - } - } - } - } - } - - return 0; -} - TypeSP SymbolFileDWARF::ParseType(const SymbolContext &sc, const DWARFDIE &die, bool *type_is_new_ptr) { if (!die) diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h index e04ddbb15cc7e..2b6df03925099 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h @@ -223,9 +223,6 @@ class SymbolFileDWARF : public lldb_private::SymbolFile, bool GetCompileOption(const char *option, std::string &value, lldb_private::CompileUnit *cu = nullptr) override; - int GetCompileOptions(const char *option, std::vector &value, - lldb_private::CompileUnit *cu = nullptr) override; - void PreloadSymbols() override; std::recursive_mutex &GetModuleMutex() const override; From 7db154f355d235770f2fa33ea3b2118526ed158b Mon Sep 17 00:00:00 2001 From: Adrian Prantl Date: Tue, 28 Jan 2020 11:00:16 -0800 Subject: [PATCH 2/2] Implement private discriminator lookup for debug map symbol files. It turns out that this wasn;t implemented at all and the existing tests were working by coincidence because RegisterAllVariables() also injects local copies of global variables, which bypasses the discriminator matching. After properly implementing discriminator support a couple of additional bugs got uncovered which should all be fixed by this commit and the matching Swift compiler update to serialize the isTopLevelGlobal bit on VarDecls. --- .../test/lang/swift/private_var/main.swift | 7 +- .../Swift/SwiftASTManipulator.cpp | 4 ++ .../Swift/SwiftExpressionParser.cpp | 68 +++++++++---------- .../SymbolFile/DWARF/SymbolFileDWARF.cpp | 9 ++- .../DWARF/SymbolFileDWARFDebugMap.cpp | 11 +++ .../DWARF/SymbolFileDWARFDebugMap.h | 3 + 6 files changed, 65 insertions(+), 37 deletions(-) diff --git a/lldb/packages/Python/lldbsuite/test/lang/swift/private_var/main.swift b/lldb/packages/Python/lldbsuite/test/lang/swift/private_var/main.swift index 4b4244d9d745c..bbbb03c220565 100644 --- a/lldb/packages/Python/lldbsuite/test/lang/swift/private_var/main.swift +++ b/lldb/packages/Python/lldbsuite/test/lang/swift/private_var/main.swift @@ -16,4 +16,9 @@ func doSomething(b: Int) { a += b //% self.expect("expr a", substrs=['Int', '= 1']) } -doSomething(b:2) +func withLocalShadow() { + let a = 23 + doSomething(b: a) //% self.expect("log enable lldb expr");self.expect("expr a", substrs=['Int', '= 23']) +} + +withLocalShadow() diff --git a/lldb/source/Plugins/ExpressionParser/Swift/SwiftASTManipulator.cpp b/lldb/source/Plugins/ExpressionParser/Swift/SwiftASTManipulator.cpp index f52a663882d1d..360a7d585549e 100644 --- a/lldb/source/Plugins/ExpressionParser/Swift/SwiftASTManipulator.cpp +++ b/lldb/source/Plugins/ExpressionParser/Swift/SwiftASTManipulator.cpp @@ -1109,6 +1109,7 @@ bool SwiftASTManipulator::AddExternalVariables( swift::VarDecl(is_static, introducer, is_capture_list, loc, name, &m_source_file); redirected_var_decl->setInterfaceType(var_type); + redirected_var_decl->setTopLevelGlobal(true); swift::TopLevelCodeDecl *top_level_code = new (ast_context) swift::TopLevelCodeDecl(&m_source_file); @@ -1222,6 +1223,9 @@ bool SwiftASTManipulator::AddExternalVariables( redirected_var_decl->setInterfaceType(interface_type); redirected_var_decl->setDebuggerVar(true); redirected_var_decl->setImplicit(true); + // This avoids having local variables filtered out by + // swift::namelookup::filterForDiscriminator(). + redirected_var_decl->overwriteAccess(swift::AccessLevel::Public); swift::PatternBindingDecl *pattern_binding = GetPatternBindingForVarDecl(redirected_var_decl, containing_function); diff --git a/lldb/source/Plugins/ExpressionParser/Swift/SwiftExpressionParser.cpp b/lldb/source/Plugins/ExpressionParser/Swift/SwiftExpressionParser.cpp index 7440092fa99a5..d29c16d9ed53b 100644 --- a/lldb/source/Plugins/ExpressionParser/Swift/SwiftExpressionParser.cpp +++ b/lldb/source/Plugins/ExpressionParser/Swift/SwiftExpressionParser.cpp @@ -413,7 +413,6 @@ class LLDBExprNameLookup : public LLDBNameLookup { } } } - return swift::Identifier(); } }; @@ -742,45 +741,44 @@ static void RegisterAllVariables( // The module scoped variables are stored at the CompUnit level, so // after we go through the current context, then we have to take one // more pass through the variables in the CompUnit. - bool handling_globals = false; VariableList variables; // Proceed from the innermost scope outwards, adding all variables // not already shadowed by an inner declaration. llvm::SmallDenseSet processed_names; - while (true) { - if (!handling_globals) { - constexpr bool can_create = true; - constexpr bool get_parent_variables = false; - constexpr bool stop_if_block_is_inlined_function = true; - - block->AppendVariables( - can_create, get_parent_variables, stop_if_block_is_inlined_function, - [](Variable *) { return true; }, &variables); - } else { - if (sc.comp_unit) { - lldb::VariableListSP globals_sp = sc.comp_unit->GetVariableList(true); - if (globals_sp) - variables.AddVariables(globals_sp.get()); - } - } - - // Process all variables in this scope. - for (size_t vi = 0, ve = variables.GetSize(); vi != ve; ++vi) - AddVariableInfo({variables.GetVariableAtIndex(vi)}, stack_frame_sp, - ast_context, language_runtime, processed_names, - local_variables); - - if (!handling_globals) { - if (block == top_block) - // Now add the containing module block, that's what holds the - // module globals: - handling_globals = true; - else - block = block->GetParent(); - } else - break; - } + bool done = false; + do { + // Iterate over all parent contexts *including* the top_block. + if (block == top_block) + done = true; + bool can_create = true; + bool get_parent_variables = false; + bool stop_if_block_is_inlined_function = true; + + block->AppendVariables( + can_create, get_parent_variables, stop_if_block_is_inlined_function, + [](Variable *) { return true; }, &variables); + + if (!done) + block = block->GetParent(); + } while (block && !done); + + // Also add local copies of globals. This is in many cases redundant + // work because the globals would also be found in the expression + // context's Swift module, but it allows a limited form of + // expression evaluation to work even if the Swift module failed to + // load, as long as the module isn't necessary to resolve the type + // or aother symbols in the expression. + if (sc.comp_unit) { + lldb::VariableListSP globals_sp = sc.comp_unit->GetVariableList(true); + if (globals_sp) + variables.AddVariables(globals_sp.get()); + } + + for (size_t vi = 0, ve = variables.GetSize(); vi != ve; ++vi) + AddVariableInfo({variables.GetVariableAtIndex(vi)}, stack_frame_sp, + ast_context, language_runtime, processed_names, + local_variables); } static void ResolveSpecialNames( diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index 0e72eb4573480..d1cc98bb251fa 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -3099,9 +3099,16 @@ bool SymbolFileDWARF::GetCompileOption(const char *option, std::string &value, const uint32_t num_compile_units = GetNumCompileUnits(); if (cu) { - DWARFUnit *dwarf_cu = GetDWARFCompileUnit(cu); + auto *dwarf_cu = + llvm::dyn_cast_or_null(GetDWARFCompileUnit(cu)); if (dwarf_cu) { + // GetDWARFCompileUnit() only looks up by CU#. Make sure that + // this is actually the correct SymbolFile by converting it + // back to a CompileUnit. + if (GetCompUnitForDWARFCompUnit(*dwarf_cu) != cu) + return false; + const DWARFBaseDIE die = dwarf_cu->GetUnitDIEOnly(); if (die) { const char *flags = diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp index 4daa69cff9706..f38c460a07f32 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp @@ -1254,6 +1254,17 @@ CompilerDeclContext SymbolFileDWARFDebugMap::FindNamespace( return matching_namespace; } +bool SymbolFileDWARFDebugMap::GetCompileOption(const char *option, + std::string &value, + CompileUnit *cu) { + bool success = false; + ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) -> bool { + success |= oso_dwarf->GetCompileOption(option, value, cu); + return success; + }); + return success; +} + void SymbolFileDWARFDebugMap::DumpClangAST(Stream &s) { ForEachSymbolFile([&s](SymbolFileDWARF *oso_dwarf) -> bool { oso_dwarf->DumpClangAST(s); diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h index 720a8b711edb6..ce29eda9234fd 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h @@ -145,6 +145,9 @@ class SymbolFileDWARFDebugMap : public lldb_private::SymbolFile { void DumpClangAST(lldb_private::Stream &s) override; + bool GetCompileOption(const char *option, std::string &value, + lldb_private::CompileUnit *cu) override; + // PluginInterface protocol lldb_private::ConstString GetPluginName() override;