kwk updated this revision to Diff 271583.
kwk added a comment.
- Align tests with reviewer expectations
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/include/lldb/Target/Target.h
lldb/source/Breakpoint/BreakpointResolverName.cpp
lldb/source/Core/SearchFilter.cpp
lldb/source/Target/Target.cpp
lldb/test/Shell/Breakpoint/Inputs/search-support-files-func.cpp
lldb/test/Shell/Breakpoint/Inputs/search-support-files.cpp
lldb/test/Shell/Breakpoint/Inputs/search-support-files.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,123 @@
+# In these tests we will set breakpoints on a function by name with the
+# target.inline-breakpoint-strategy setting alternating between set "always" and
+# "headers".
+
+# RUN: %build %p/Inputs/search-support-files.cpp -o %t.out
+# RUN: %lldb -b -s %s %t.out | FileCheck %s
+
+
+# Set breakpoint by function name.
+
+
+settings set target.inline-breakpoint-strategy always
+breakpoint set -n function_in_header
+# CHECK: (lldb) breakpoint set -n function_in_header
+# CHECK-NEXT: Breakpoint 1: where = {{.*}}.out`function_in_header(){{.*}} at search-support-files.h
+
+settings set target.inline-breakpoint-strategy headers
+breakpoint set -n function_in_header
+# CHECK: (lldb) breakpoint set -n function_in_header
+# CHECK-NEXT: Breakpoint 2: where = {{.*}}.out`function_in_header(){{.*}} at search-support-files.h
+
+
+
+settings set target.inline-breakpoint-strategy always
+breakpoint set -n main
+# CHECK: (lldb) breakpoint set -n main
+# CHECK-NEXT: Breakpoint 3: where = {{.*}}.out`main{{.*}} at search-support-files.cpp
+
+settings set target.inline-breakpoint-strategy headers
+breakpoint set -n main
+# CHECK: (lldb) breakpoint set -n main
+# CHECK-NEXT: Breakpoint 4: where = {{.*}}.out`main{{.*}} at search-support-files.cpp
+
+
+
+settings set target.inline-breakpoint-strategy always
+breakpoint set -n func
+# CHECK: (lldb) breakpoint set -n func
+# CHECK-NEXT: Breakpoint 5: where = {{.*}}.out`func{{.*}} at search-support-files-func.cpp
+
+settings set target.inline-breakpoint-strategy headers
+breakpoint set -n func
+# CHECK: (lldb) breakpoint set -n func
+# CHECK-NEXT: Breakpoint 6: where = {{.*}}.out`func{{.*}} at search-support-files-func.cpp
+
+
+
+# Set breakpoint by function name and filename (here: the compilation unit).
+
+
+
+settings set target.inline-breakpoint-strategy always
+breakpoint set -n function_in_header -f search-support-files.cpp
+# CHECK: (lldb) breakpoint set -n function_in_header -f search-support-files.cpp
+# CHECK-NEXT: Breakpoint 7: no locations (pending).
+
+settings set target.inline-breakpoint-strategy headers
+breakpoint set -n function_in_header -f search-support-files.cpp
+# CHECK: (lldb) breakpoint set -n function_in_header -f search-support-files.cpp
+# CHECK-NEXT: Breakpoint 8: 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
+# search by source files that are themselves no compilation units.
+
+
+
+settings set target.inline-breakpoint-strategy always
+breakpoint set -n function_in_header -f search-support-files.h
+# CHECK: (lldb) breakpoint set -n function_in_header -f search-support-files.h
+# CHECK-NEXT: Breakpoint 9: where = {{.*}}.out`function_in_header(){{.*}} at search-support-files.h
+
+settings set target.inline-breakpoint-strategy headers
+breakpoint set -n function_in_header -f search-support-files.h
+# CHECK: (lldb) breakpoint set -n function_in_header -f search-support-files.h
+# CHECK-NEXT: Breakpoint 10: where = {{.*}}.out`function_in_header(){{.*}} at search-support-files.h
+
+
+settings set target.inline-breakpoint-strategy always
+breakpoint set -n func -f search-support-files-func.cpp
+# CHECK: (lldb) breakpoint set -n func -f search-support-files-func.cpp
+# CHECK-NEXT: Breakpoint 11: where = {{.*}}.out`func(){{.*}} at search-support-files-func.cpp
+
+settings set target.inline-breakpoint-strategy headers
+breakpoint set -n func -f search-support-files-func.cpp
+# CHECK: (lldb) breakpoint set -n func -f search-support-files-func.cpp
+# CHECK-NEXT: Breakpoint 12: no locations (pending).
+
+
+
+# Set breakpoint by function name and source filename. This time the file
+# doesn't exist or is not the file in which the function is declared or
+# defined. This is to prove that we haven't widen the search space too much.
+# When we search for a function in a file that doesn't exist, we should get no
+# results.
+
+
+
+settings set target.inline-breakpoint-strategy always
+breakpoint set -n function_in_header -f file-not-existing.h
+# CHECK: (lldb) breakpoint set -n function_in_header -f file-not-existing.h
+# CHECK-NEXT: Breakpoint 13: no locations (pending).
+
+settings set target.inline-breakpoint-strategy headers
+breakpoint set -n function_in_header -f file-not-existing.h
+# CHECK: (lldb) breakpoint set -n function_in_header -f file-not-existing.h
+# CHECK-NEXT: Breakpoint 14: no locations (pending).
+
+
+
+settings set target.inline-breakpoint-strategy always
+breakpoint set -n func -f search-support-files.cpp
+# CHECK: (lldb) breakpoint set -n func -f search-support-files.cpp
+# CHECK-NEXT: Breakpoint 15: no locations (pending).
+
+settings set target.inline-breakpoint-strategy headers
+breakpoint set -n func -f search-support-files.cpp
+# CHECK: (lldb) breakpoint set -n func -f search-support-files.cpp
+# CHECK-NEXT: Breakpoint 16: no locations (pending).
\ 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 function_in_header() { return 42; }
\ No newline at end of file
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-files-func.cpp"
+
+int main(int argc, char *argv[]) {
+ int a = func() - function_in_header();
+ return a;
+}
Index: lldb/test/Shell/Breakpoint/Inputs/search-support-files-func.cpp
===================================================================
--- /dev/null
+++ lldb/test/Shell/Breakpoint/Inputs/search-support-files-func.cpp
@@ -0,0 +1,3 @@
+int func() {
+ return 84;
+}
\ No newline at end of file
Index: lldb/source/Target/Target.cpp
===================================================================
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -309,7 +309,7 @@
const std::unordered_set<std::string> &function_names,
RegularExpression source_regex, bool internal, bool hardware,
LazyBool move_to_nearest_code) {
- SearchFilterSP filter_sp(GetSearchFilterForModuleAndCUList(
+ SearchFilterSP filter_sp(GetSearchFilterForModulesAndSupportFiles(
containingModules, source_file_spec_list));
if (move_to_nearest_code == eLazyBoolCalculate)
move_to_nearest_code = GetMoveToNearestCode() ? eLazyBoolYes : eLazyBoolNo;
@@ -355,8 +355,8 @@
// Not checking for inlines, we are looking only for matching compile units
FileSpecList compile_unit_list;
compile_unit_list.Append(remapped_file);
- filter_sp = GetSearchFilterForModuleAndCUList(containingModules,
- &compile_unit_list);
+ filter_sp = GetSearchFilterForModulesAndSupportFiles(containingModules,
+ &compile_unit_list);
} else {
filter_sp = GetSearchFilterForModuleList(containingModules);
}
@@ -419,7 +419,7 @@
lldb::addr_t offset, LazyBool skip_prologue, bool internal, bool hardware) {
BreakpointSP bp_sp;
if (func_name) {
- SearchFilterSP filter_sp(GetSearchFilterForModuleAndCUList(
+ SearchFilterSP filter_sp(GetSearchFilterForModulesAndSupportFiles(
containingModules, containingSourceFiles));
if (skip_prologue == eLazyBoolCalculate)
@@ -445,7 +445,7 @@
BreakpointSP bp_sp;
size_t num_names = func_names.size();
if (num_names > 0) {
- SearchFilterSP filter_sp(GetSearchFilterForModuleAndCUList(
+ SearchFilterSP filter_sp(GetSearchFilterForModulesAndSupportFiles(
containingModules, containingSourceFiles));
if (skip_prologue == eLazyBoolCalculate)
@@ -470,7 +470,7 @@
LazyBool skip_prologue, bool internal, bool hardware) {
BreakpointSP bp_sp;
if (num_names > 0) {
- SearchFilterSP filter_sp(GetSearchFilterForModuleAndCUList(
+ SearchFilterSP filter_sp(GetSearchFilterForModulesAndSupportFiles(
containingModules, containingSourceFiles));
if (skip_prologue == eLazyBoolCalculate) {
@@ -527,7 +527,7 @@
return filter_sp;
}
-SearchFilterSP Target::GetSearchFilterForModuleAndCUList(
+SearchFilterSP Target::GetSearchFilterForModulesAndSupportFiles(
const FileSpecList *containingModules,
const FileSpecList *containingSourceFiles) {
if (containingSourceFiles == nullptr || containingSourceFiles->GetSize() == 0)
@@ -538,10 +538,10 @@
// We could make a special "CU List only SearchFilter". Better yet was if
// these could be composable, but that will take a little reworking.
- filter_sp = std::make_shared<SearchFilterByModuleListAndCU>(
+ filter_sp = std::make_shared<SearchFilterByModulesAndSupportFiles>(
shared_from_this(), FileSpecList(), *containingSourceFiles);
} else {
- filter_sp = std::make_shared<SearchFilterByModuleListAndCU>(
+ filter_sp = std::make_shared<SearchFilterByModulesAndSupportFiles>(
shared_from_this(), *containingModules, *containingSourceFiles);
}
return filter_sp;
@@ -552,7 +552,7 @@
const FileSpecList *containingSourceFiles, RegularExpression func_regex,
lldb::LanguageType requested_language, LazyBool skip_prologue,
bool internal, bool hardware) {
- SearchFilterSP filter_sp(GetSearchFilterForModuleAndCUList(
+ SearchFilterSP filter_sp(GetSearchFilterForModulesAndSupportFiles(
containingModules, containingSourceFiles));
bool skip = (skip_prologue == eLazyBoolCalculate)
? GetSkipPrologue()
@@ -594,11 +594,11 @@
bool has_modules = containingModules && containingModules->GetSize() > 0;
if (has_files && has_modules) {
- filter_sp = GetSearchFilterForModuleAndCUList(containingModules,
- containingSourceFiles);
+ filter_sp = GetSearchFilterForModulesAndSupportFiles(containingModules,
+ containingSourceFiles);
} else if (has_files) {
- filter_sp =
- GetSearchFilterForModuleAndCUList(nullptr, containingSourceFiles);
+ filter_sp = GetSearchFilterForModulesAndSupportFiles(nullptr,
+ containingSourceFiles);
} else if (has_modules) {
filter_sp = GetSearchFilterForModuleList(containingModules);
} else {
Index: lldb/source/Core/SearchFilter.cpp
===================================================================
--- lldb/source/Core/SearchFilter.cpp
+++ lldb/source/Core/SearchFilter.cpp
@@ -121,7 +121,7 @@
target_sp, *subclass_options, error);
break;
case ByModulesAndCU:
- result_sp = SearchFilterByModuleListAndCU::CreateFromStructuredData(
+ result_sp = SearchFilterByModulesAndSupportFiles::CreateFromStructuredData(
target_sp, *subclass_options, error);
break;
case Exception:
@@ -637,22 +637,23 @@
return WrapOptionsDict(options_dict_sp);
}
-// SearchFilterByModuleListAndCU:
+// SearchFilterByModulesAndSupportFiles:
// Selects a shared library matching a given file spec
-SearchFilterByModuleListAndCU::SearchFilterByModuleListAndCU(
+SearchFilterByModulesAndSupportFiles::SearchFilterByModulesAndSupportFiles(
const lldb::TargetSP &target_sp, const FileSpecList &module_list,
- const FileSpecList &cu_list)
+ const FileSpecList &support_file_list)
: SearchFilterByModuleList(target_sp, module_list,
FilterTy::ByModulesAndCU),
- m_cu_spec_list(cu_list) {}
+ m_support_file_list(support_file_list) {}
-SearchFilterByModuleListAndCU::~SearchFilterByModuleListAndCU() = default;
+SearchFilterByModulesAndSupportFiles::~SearchFilterByModulesAndSupportFiles() =
+ default;
-lldb::SearchFilterSP SearchFilterByModuleListAndCU::CreateFromStructuredData(
- const lldb::TargetSP& target_sp,
- const StructuredData::Dictionary &data_dict,
- Status &error) {
+lldb::SearchFilterSP
+SearchFilterByModulesAndSupportFiles::CreateFromStructuredData(
+ const lldb::TargetSP &target_sp,
+ const StructuredData::Dictionary &data_dict, Status &error) {
StructuredData::Array *modules_array = nullptr;
SearchFilterSP result_sp;
bool success = data_dict.GetValueForKeyAsArray(GetKey(OptionNames::ModList),
@@ -693,40 +694,51 @@
cus.EmplaceBack(cu);
}
- return std::make_shared<SearchFilterByModuleListAndCU>(
- target_sp, modules, cus);
+ return std::make_shared<SearchFilterByModulesAndSupportFiles>(target_sp,
+ modules, cus);
}
StructuredData::ObjectSP
-SearchFilterByModuleListAndCU::SerializeToStructuredData() {
+SearchFilterByModulesAndSupportFiles::SerializeToStructuredData() {
auto options_dict_sp = std::make_shared<StructuredData::Dictionary>();
SearchFilterByModuleList::SerializeUnwrapped(options_dict_sp);
- SerializeFileSpecList(options_dict_sp, OptionNames::CUList, m_cu_spec_list);
+ SerializeFileSpecList(options_dict_sp, OptionNames::CUList,
+ m_support_file_list);
return WrapOptionsDict(options_dict_sp);
}
-bool SearchFilterByModuleListAndCU::AddressPasses(Address &address) {
+bool SearchFilterByModulesAndSupportFiles::FunctionPasses(Function &function) {
+ Type *type = function.GetType();
+ if (!type)
+ return SearchFilterByModuleList::FunctionPasses(function);
+
+ Declaration &decl = const_cast<Declaration &>(type->GetDeclaration());
+ FileSpec &source_file = decl.GetFile();
+
+ if (m_target_sp &&
+ m_target_sp->GetInlineStrategy() == eInlineBreakpointsHeaders) {
+ if (source_file.IsSourceImplementationFile())
+ return false;
+ }
+
+ return m_support_file_list.FindFileIndex(0, source_file, false) != UINT32_MAX;
+}
+
+bool SearchFilterByModulesAndSupportFiles::AddressPasses(Address &address) {
SymbolContext sym_ctx;
address.CalculateSymbolContext(&sym_ctx, eSymbolContextEverything);
- if (!sym_ctx.comp_unit) {
- if (m_cu_spec_list.GetSize() != 0)
- return false; // Has no comp_unit so can't pass the file check.
- }
- 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);
+
+ return SearchFilterByModuleList::ModulePasses(sym_ctx.module_sp);
}
-bool SearchFilterByModuleListAndCU::CompUnitPasses(FileSpec &fileSpec) {
- return m_cu_spec_list.FindFileIndex(0, fileSpec, false) != UINT32_MAX;
+bool SearchFilterByModulesAndSupportFiles::CompUnitPasses(FileSpec &fileSpec) {
+ return m_support_file_list.FindFileIndex(0, fileSpec, false) != UINT32_MAX;
}
-bool SearchFilterByModuleListAndCU::CompUnitPasses(CompileUnit &compUnit) {
- bool in_cu_list = m_cu_spec_list.FindFileIndex(0, compUnit.GetPrimaryFile(),
- false) != UINT32_MAX;
+bool SearchFilterByModulesAndSupportFiles::CompUnitPasses(
+ CompileUnit &compUnit) {
+ bool in_cu_list = m_support_file_list.FindFileIndex(
+ 0, compUnit.GetPrimaryFile(), false) != UINT32_MAX;
if (!in_cu_list)
return false;
@@ -737,7 +749,7 @@
return SearchFilterByModuleList::ModulePasses(module_sp);
}
-void SearchFilterByModuleListAndCU::Search(Searcher &searcher) {
+void SearchFilterByModulesAndSupportFiles::Search(Searcher &searcher) {
if (!m_target_sp)
return;
@@ -780,7 +792,7 @@
matchingContext.comp_unit = cu_sp.get();
if (!matchingContext.comp_unit)
continue;
- if (m_cu_spec_list.FindFileIndex(
+ if (m_support_file_list.FindFileIndex(
0, matchingContext.comp_unit->GetPrimaryFile(), false) ==
UINT32_MAX)
continue;
@@ -791,7 +803,7 @@
}
}
-void SearchFilterByModuleListAndCU::GetDescription(Stream *s) {
+void SearchFilterByModulesAndSupportFiles::GetDescription(Stream *s) {
size_t num_modules = m_module_spec_list.GetSize();
if (num_modules == 1) {
s->Printf(", module = ");
@@ -810,12 +822,16 @@
}
}
-uint32_t SearchFilterByModuleListAndCU::GetFilterRequiredItems() {
- return eSymbolContextModule | eSymbolContextCompUnit;
+uint32_t SearchFilterByModulesAndSupportFiles::GetFilterRequiredItems() {
+ uint32_t flags = eSymbolContextModule | eSymbolContextFunction;
+ if (m_target_sp &&
+ m_target_sp->GetInlineStrategy() == eInlineBreakpointsHeaders)
+ return flags | eSymbolContextCompUnit;
+ return flags;
}
-void SearchFilterByModuleListAndCU::Dump(Stream *s) const {}
+void SearchFilterByModulesAndSupportFiles::Dump(Stream *s) const {}
-SearchFilterSP SearchFilterByModuleListAndCU::DoCreateCopy() {
- return std::make_shared<SearchFilterByModuleListAndCU>(*this);
+SearchFilterSP SearchFilterByModulesAndSupportFiles::DoCreateCopy() {
+ return std::make_shared<SearchFilterByModulesAndSupportFiles>(*this);
}
Index: lldb/source/Breakpoint/BreakpointResolverName.cpp
===================================================================
--- lldb/source/Breakpoint/BreakpointResolverName.cpp
+++ lldb/source/Breakpoint/BreakpointResolverName.cpp
@@ -260,8 +260,10 @@
SymbolContextList func_list;
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_symbols = !(filter_by_cu || filter_by_function);
const bool include_inlines = true;
switch (m_match_type) {
@@ -285,7 +287,8 @@
if (context.module_sp) {
context.module_sp->FindFunctions(
m_regex,
- !filter_by_cu, // include symbols only if we aren't filtering by CU
+ include_symbols, // include symbols only if we aren't filtering by CU
+ // or function
include_inlines, func_list);
}
break;
@@ -297,18 +300,26 @@
// If the filter specifies a Compilation Unit, remove the ones that don't
// pass at this point.
- if (filter_by_cu || filter_by_language) {
+ if (filter_by_cu || filter_by_language || filter_by_function) {
uint32_t num_functions = func_list.GetSize();
for (size_t idx = 0; idx < num_functions; idx++) {
bool remove_it = false;
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.function || !filter.FunctionPasses(*sc.function))
+ remove_it = true;
+ else
+ remove_it = false;
+ }
+
if (filter_by_language) {
LanguageType sym_language = sc.GetLanguage();
if ((Language::GetPrimaryLanguage(sym_language) !=
Index: lldb/include/lldb/Target/Target.h
===================================================================
--- lldb/include/lldb/Target/Target.h
+++ lldb/include/lldb/Target/Target.h
@@ -1241,9 +1241,9 @@
lldb::SearchFilterSP
GetSearchFilterForModuleList(const FileSpecList *containingModuleList);
- lldb::SearchFilterSP
- GetSearchFilterForModuleAndCUList(const FileSpecList *containingModules,
- const FileSpecList *containingSourceFiles);
+ lldb::SearchFilterSP GetSearchFilterForModulesAndSupportFiles(
+ const FileSpecList *containingModules,
+ const FileSpecList *containingSourceFiles);
lldb::REPLSP GetREPL(Status &err, lldb::LanguageType language,
const char *repl_options, bool can_create);
Index: lldb/include/lldb/Core/SearchFilter.h
===================================================================
--- lldb/include/lldb/Core/SearchFilter.h
+++ lldb/include/lldb/Core/SearchFilter.h
@@ -402,15 +402,23 @@
FileSpecList m_module_spec_list;
};
-class SearchFilterByModuleListAndCU : public SearchFilterByModuleList {
+class SearchFilterByModulesAndSupportFiles : public SearchFilterByModuleList {
public:
/// The basic constructor takes a Target, which gives the space to search,
/// and the module list to restrict the search to.
- SearchFilterByModuleListAndCU(const lldb::TargetSP &targetSP,
- const FileSpecList &module_list,
- const FileSpecList &cu_list);
+ SearchFilterByModulesAndSupportFiles(const lldb::TargetSP &targetSP,
+ const FileSpecList &module_list,
+ const FileSpecList &support_file_list);
- ~SearchFilterByModuleListAndCU() override;
+ ~SearchFilterByModulesAndSupportFiles() override;
+
+ /// \returns \c true if the file in which \p function was declared is part of
+ /// the file list given for this filter; otherwise we fall back to
+ ///
+ /// \code{.cpp}
+ /// SearchFilterByModuleList::FunctionPasses(function)
+ /// \endcode
+ bool FunctionPasses(Function &function) override;
bool AddressPasses(Address &address) override;
@@ -427,7 +435,7 @@
void Search(Searcher &searcher) override;
static lldb::SearchFilterSP
- CreateFromStructuredData(const lldb::TargetSP& target_sp,
+ CreateFromStructuredData(const lldb::TargetSP &target_sp,
const StructuredData::Dictionary &data_dict,
Status &error);
@@ -437,7 +445,7 @@
lldb::SearchFilterSP DoCreateCopy() override;
private:
- FileSpecList m_cu_spec_list;
+ FileSpecList m_support_file_list;
};
} // namespace lldb_private
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
