kwk updated this revision to Diff 246162. kwk added a comment. - Clear formatting - Make list private again - Remove open from CommandObjectBreakpoint.cpp - Remove change in unrelated file
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74136/new/ https://reviews.llvm.org/D74136 Files: lldb/include/lldb/Core/SearchFilter.h lldb/source/Breakpoint/Breakpoint.cpp lldb/source/Breakpoint/BreakpointResolverName.cpp lldb/source/Core/SearchFilter.cpp lldb/source/Target/Target.cpp lldb/test/Shell/Breakpoint/Inputs/search-support-files.cpp lldb/test/Shell/Breakpoint/Inputs/search-support-files.h lldb/test/Shell/Breakpoint/Inputs/search-support-files2.h lldb/test/Shell/Breakpoint/search-support-files.test
Index: lldb/test/Shell/Breakpoint/search-support-files.test =================================================================== --- /dev/null +++ lldb/test/Shell/Breakpoint/search-support-files.test @@ -0,0 +1,51 @@ +# In these tests we will set breakpoints on a function by name. That function +# is defined in a header file (search-support-files.h) and will therefore be +# inlined into the file that includes it (search-support-files.cpp). +# +# TODO(kwk): Check that we can also do the same with C++ methods in files? +# (See https://lldb.llvm.org/use/tutorial.html and look for --method.) + +# RUN: mkdir -p %t +# RUN: cd %t +# RUN: %build %p/Inputs/search-support-files.cpp -o dummy.out +# RUN: %lldb -b -s %s dummy.out | FileCheck --color --dump-input=fail %s + +#------------------------------------------------------------------------------- +# Set breakpoint by function name. + +breakpoint set -n inlined_42 +# CHECK: (lldb) breakpoint set -n inlined_42 +# CHECK-NEXT: Breakpoint 1: where = dummy.out`inlined_42() + 4 at search-support-files.h:1:20, address = 0x0{{.*}} + +breakpoint set -n main +# CHECK: (lldb) breakpoint set -n main +# CHECK-NEXT: Breakpoint 2: where = dummy.out`main + 22 at search-support-files.cpp:5:11, address = 0x0{{.*}} + +#------------------------------------------------------------------------------- +# Set breakpoint by function name and filename (the one in which the function is +# inlined, aka the compilation unit). + +breakpoint set -n inlined_42 -f search-support-files.cpp +# CHECK: (lldb) breakpoint set -n inlined_42 -f search-support-files.cpp +# CHECK-NEXT: Breakpoint 3: no locations (pending). + +#------------------------------------------------------------------------------- +# Set breakpoint by function name and source filename (the file in which the +# function is defined). +# +# NOTE: This test is the really interesting one as it shows that we can now +# search by source files that are themselves no compulation units. + +breakpoint set -n inlined_42 -f search-support-files.h +# CHECK: (lldb) breakpoint set -n inlined_42 -f search-support-files.h +# CHECK-NEXT: Breakpoint 4: where = dummy.out`inlined_42() + 4 at search-support-files.h:1:20, address = 0x0{{.*}} + +#------------------------------------------------------------------------------- +# Set breakpoint by function name and source filename. This time the file +# doesn't exist to prove that we haven't widen the search space too much. When +# we search for a function in file that doesn't exist, we should get no results. + +breakpoint set -n inlined_42 -f file-not-existing.h +# CHECK: (lldb) breakpoint set -n inlined_42 -f file-not-existing.h +# CHECK-NEXT: Breakpoint 5: no locations (pending). +# CHECK-NEXT: WARNING: Unable to resolve breakpoint to any actual locations. Index: lldb/test/Shell/Breakpoint/Inputs/search-support-files2.h =================================================================== --- /dev/null +++ lldb/test/Shell/Breakpoint/Inputs/search-support-files2.h @@ -0,0 +1 @@ +int return_zero() { return 0; } \ No newline at end of file Index: lldb/test/Shell/Breakpoint/Inputs/search-support-files.h =================================================================== --- /dev/null +++ lldb/test/Shell/Breakpoint/Inputs/search-support-files.h @@ -0,0 +1 @@ +int inlined_42() { return 42; } Index: lldb/test/Shell/Breakpoint/Inputs/search-support-files.cpp =================================================================== --- /dev/null +++ lldb/test/Shell/Breakpoint/Inputs/search-support-files.cpp @@ -0,0 +1,7 @@ +#include "search-support-files.h" +#include "search-support-files2.h" + +int main(int argc, char *argv[]) { + int a = inlined_42(); + return return_zero() + a; +} Index: lldb/source/Target/Target.cpp =================================================================== --- lldb/source/Target/Target.cpp +++ lldb/source/Target/Target.cpp @@ -324,7 +324,7 @@ LazyBool check_inlines, LazyBool skip_prologue, bool internal, bool hardware, - LazyBool move_to_nearest_code) { + LazyBool move_to_nearest_code) { FileSpec remapped_file; if (!GetSourcePathMap().ReverseRemapPath(file, remapped_file)) remapped_file = file; Index: lldb/source/Core/SearchFilter.cpp =================================================================== --- lldb/source/Core/SearchFilter.cpp +++ lldb/source/Core/SearchFilter.cpp @@ -412,17 +412,6 @@ return FileSpec::Match(m_module_spec, spec); } -bool SearchFilterByModule::AddressPasses(Address &address) { - // FIXME: Not yet implemented - return true; -} - -bool SearchFilterByModule::CompUnitPasses(FileSpec &fileSpec) { return true; } - -bool SearchFilterByModule::CompUnitPasses(CompileUnit &compUnit) { - return true; -} - void SearchFilterByModule::Search(Searcher &searcher) { if (!m_target_sp) return; @@ -538,19 +527,6 @@ return m_module_spec_list.FindFileIndex(0, spec, true) != UINT32_MAX; } -bool SearchFilterByModuleList::AddressPasses(Address &address) { - // FIXME: Not yet implemented - return true; -} - -bool SearchFilterByModuleList::CompUnitPasses(FileSpec &fileSpec) { - return true; -} - -bool SearchFilterByModuleList::CompUnitPasses(CompileUnit &compUnit) { - return true; -} - void SearchFilterByModuleList::Search(Searcher &searcher) { if (!m_target_sp) return; @@ -729,9 +705,13 @@ FileSpec cu_spec; if (sym_ctx.comp_unit) cu_spec = sym_ctx.comp_unit->GetPrimaryFile(); - if (m_cu_spec_list.FindFileIndex(0, cu_spec, false) == UINT32_MAX) - return false; // Fails the file check - return SearchFilterByModuleList::ModulePasses(sym_ctx.module_sp); + if (m_cu_spec_list.FindFileIndex(0, cu_spec, false) != UINT32_MAX) + return true; + // ^ If the primary source file associated with the symbol's compile unit is in + // the list of CU's or support files, that's enough. + // TODO(kwk): Is that enough? + + return SearchFilterByModuleList::ModulePasses(sym_ctx.module_sp); } bool SearchFilterByModuleListAndCU::CompUnitPasses(FileSpec &fileSpec) { @@ -825,7 +805,7 @@ } uint32_t SearchFilterByModuleListAndCU::GetFilterRequiredItems() { - return eSymbolContextModule | eSymbolContextCompUnit; + return eSymbolContextModule | eSymbolContextCompUnit | eSymbolContextFunction; } void SearchFilterByModuleListAndCU::Dump(Stream *s) const {} Index: lldb/source/Breakpoint/BreakpointResolverName.cpp =================================================================== --- lldb/source/Breakpoint/BreakpointResolverName.cpp +++ lldb/source/Breakpoint/BreakpointResolverName.cpp @@ -248,6 +248,8 @@ // accelerate function lookup. At that point, we should switch the depth to // CompileUnit, and look in these tables. +#include "lldb/Symbol/CompileUnit.h" + Searcher::CallbackReturn BreakpointResolverName::SearchCallback(SearchFilter &filter, SymbolContext &context, Address *addr) { @@ -268,6 +270,8 @@ } bool filter_by_cu = (filter.GetFilterRequiredItems() & eSymbolContextCompUnit) != 0; + bool filter_by_function = + (filter.GetFilterRequiredItems() & eSymbolContextFunction) != 0; bool filter_by_language = (m_language != eLanguageTypeUnknown); const bool include_symbols = !filter_by_cu; const bool include_inlines = true; @@ -313,8 +317,26 @@ SymbolContext sc; func_list.GetContextAtIndex(idx, sc); if (filter_by_cu) { - if (!sc.comp_unit || !filter.CompUnitPasses(*sc.comp_unit)) - remove_it = true; + if (!filter_by_function) { + if (!sc.comp_unit || !filter.CompUnitPasses(*sc.comp_unit)) + remove_it = true; + } else { + // TODO(kwk): Currently this appears somewhat like a hack because only + // the SearchFilterByModuleListAndCUOrSupportFile search filter + // requires eSymbolContextCompUnit and eSymbolContextFunction + // (filter_by_cu && filter_by_function). + + // Keep this symbol context if it is a function call to a function + // whose declaration is located in a file that passes. This is needed + // for inlined functions (e.g. when LTO is enabled) and templates. + if (Type *type = sc.function->GetType()) { + Declaration &decl = + const_cast<Declaration &>(type->GetDeclaration()); + FileSpec &source_file = decl.GetFile(); + if (!filter.CompUnitPasses(source_file)) + remove_it = true; + } + } } if (filter_by_language) { @@ -336,64 +358,67 @@ // Remove any duplicates between the function list and the symbol list SymbolContext sc; - if (func_list.GetSize()) { - for (i = 0; i < func_list.GetSize(); i++) { - if (func_list.GetContextAtIndex(i, sc)) { - bool is_reexported = false; - - if (sc.block && sc.block->GetInlinedFunctionInfo()) { - if (!sc.block->GetStartAddress(break_addr)) - break_addr.Clear(); - } else if (sc.function) { - break_addr = sc.function->GetAddressRange().GetBaseAddress(); - if (m_skip_prologue && break_addr.IsValid()) { - const uint32_t prologue_byte_size = - sc.function->GetPrologueByteSize(); - if (prologue_byte_size) - break_addr.SetOffset(break_addr.GetOffset() + prologue_byte_size); - } - } else if (sc.symbol) { - if (sc.symbol->GetType() == eSymbolTypeReExported) { - const Symbol *actual_symbol = - sc.symbol->ResolveReExportedSymbol(m_breakpoint->GetTarget()); - if (actual_symbol) { - is_reexported = true; - break_addr = actual_symbol->GetAddress(); - } - } else { - break_addr = sc.symbol->GetAddress(); - } - - if (m_skip_prologue && break_addr.IsValid()) { - const uint32_t prologue_byte_size = - sc.symbol->GetPrologueByteSize(); - if (prologue_byte_size) - break_addr.SetOffset(break_addr.GetOffset() + prologue_byte_size); - else { - const Architecture *arch = - m_breakpoint->GetTarget().GetArchitecturePlugin(); - if (arch) - arch->AdjustBreakpointAddress(*sc.symbol, break_addr); - } - } + if (!func_list.GetSize()) + return Searcher::eCallbackReturnContinue; + + for (i = 0; i < func_list.GetSize(); i++) { + if (!func_list.GetContextAtIndex(i, sc)) + continue; + + bool is_reexported = false; + + if (sc.block && sc.block->GetInlinedFunctionInfo()) { + if (!sc.block->GetStartAddress(break_addr)) + break_addr.Clear(); + } else if (sc.function) { + break_addr = sc.function->GetAddressRange().GetBaseAddress(); + if (m_skip_prologue && break_addr.IsValid()) { + const uint32_t prologue_byte_size = + sc.function->GetPrologueByteSize(); + if (prologue_byte_size) + break_addr.SetOffset(break_addr.GetOffset() + prologue_byte_size); + } + } else if (sc.symbol) { + if (sc.symbol->GetType() == eSymbolTypeReExported) { + const Symbol *actual_symbol = + sc.symbol->ResolveReExportedSymbol(m_breakpoint->GetTarget()); + if (actual_symbol) { + is_reexported = true; + break_addr = actual_symbol->GetAddress(); } + } else { + break_addr = sc.symbol->GetAddress(); + } - if (break_addr.IsValid()) { - if (filter.AddressPasses(break_addr)) { - BreakpointLocationSP bp_loc_sp( - AddLocation(break_addr, &new_location)); - bp_loc_sp->SetIsReExported(is_reexported); - if (bp_loc_sp && new_location && !m_breakpoint->IsInternal()) { - if (log) { - StreamString s; - bp_loc_sp->GetDescription(&s, lldb::eDescriptionLevelVerbose); - LLDB_LOGF(log, "Added location: %s\n", s.GetData()); - } - } - } + if (m_skip_prologue && break_addr.IsValid()) { + const uint32_t prologue_byte_size = sc.symbol->GetPrologueByteSize(); + if (prologue_byte_size) + break_addr.SetOffset(break_addr.GetOffset() + prologue_byte_size); + else { + const Architecture *arch = + m_breakpoint->GetTarget().GetArchitecturePlugin(); + if (arch) + arch->AdjustBreakpointAddress(*sc.symbol, break_addr); } } } + + if (!break_addr.IsValid()) + continue; + + if (!filter.AddressPasses(break_addr)) + continue; + + BreakpointLocationSP bp_loc_sp( + AddLocation(break_addr, &new_location)); + bp_loc_sp->SetIsReExported(is_reexported); + if (bp_loc_sp && new_location && !m_breakpoint->IsInternal()) { + if (log) { + StreamString s; + bp_loc_sp->GetDescription(&s, lldb::eDescriptionLevelVerbose); + LLDB_LOGF(log, "Added location: %s\n", s.GetData()); + } + } } return Searcher::eCallbackReturnContinue; Index: lldb/source/Breakpoint/Breakpoint.cpp =================================================================== --- lldb/source/Breakpoint/Breakpoint.cpp +++ lldb/source/Breakpoint/Breakpoint.cpp @@ -193,7 +193,7 @@ return result_sp; } } - + bool hardware = false; success = breakpoint_dict->GetValueForKeyAsBoolean( Breakpoint::GetKey(OptionNames::Hardware), hardware); Index: lldb/include/lldb/Core/SearchFilter.h =================================================================== --- lldb/include/lldb/Core/SearchFilter.h +++ lldb/include/lldb/Core/SearchFilter.h @@ -98,6 +98,8 @@ /// The file spec to check against the filter. /// \return /// \b true if \a spec passes, and \b false otherwise. + /// + /// \note if not overriden, default implementation always \c true. virtual bool ModulePasses(const FileSpec &spec); /// Call this method with a Module to see if that module passes the filter. @@ -107,6 +109,8 @@ /// /// \return /// \b true if \a module passes, and \b false otherwise. + /// + /// \note if not overriden, default implementation always \c true. virtual bool ModulePasses(const lldb::ModuleSP &module_sp); /// Call this method with a Address to see if \a address passes the filter. @@ -116,6 +120,8 @@ /// /// \return /// \b true if \a address passes, and \b false otherwise. + /// + /// \note if not overriden, default implementation always \c true. virtual bool AddressPasses(Address &addr); /// Call this method with a FileSpec to see if \a file spec passes the @@ -126,6 +132,8 @@ /// /// \return /// \b true if \a file spec passes, and \b false otherwise. + /// + /// \note if not overriden, default implementation always \c true. virtual bool CompUnitPasses(FileSpec &fileSpec); /// Call this method with a CompileUnit to see if \a comp unit passes the @@ -136,6 +144,8 @@ /// /// \return /// \b true if \a Comp Unit passes, and \b false otherwise. + /// + /// \note if not overriden, default implementation always \c true. virtual bool CompUnitPasses(CompileUnit &compUnit); /// Call this method with a Function to see if \a function passes the @@ -319,12 +329,6 @@ bool ModulePasses(const FileSpec &spec) override; - bool AddressPasses(Address &address) override; - - bool CompUnitPasses(FileSpec &fileSpec) override; - - bool CompUnitPasses(CompileUnit &compUnit) override; - void GetDescription(Stream *s) override; uint32_t GetFilterRequiredItems() override; @@ -370,12 +374,6 @@ bool ModulePasses(const FileSpec &spec) override; - bool AddressPasses(Address &address) override; - - bool CompUnitPasses(FileSpec &fileSpec) override; - - bool CompUnitPasses(CompileUnit &compUnit) override; - void GetDescription(Stream *s) override; uint32_t GetFilterRequiredItems() override;
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
