[Lldb-commits] [PATCH] D74136: [LLDB] WIP: Follow DW_AT_decl_file when setting breakpoint

Jan Kratochvil via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Thu Jun 11 12:41:02 PDT 2020

jankratochvil added a comment.

I would separate/remove all [nfc] changes from this patch as they complicate it a bit. Such cleaned up patch I have prepared for myself: https://people.redhat.com/jkratoch/D74136-cleanup.patch

Then also did you notice there is a regression for (I do not know why):

  FAIL: LLDB (/home/jkratoch/redhat/llvm-monorepo-clangassert/bin/clang-11-x86_64) :: test_scripted_resolver (TestScriptedResolver.TestScriptedResolver)
    File "/home/jkratoch/redhat/llvm-monorepo/lldb/test/API/functionalities/breakpoint/scripted_bkpt/TestScriptedResolver.py", line 23, in test_scripted_resolver
    File "/home/jkratoch/redhat/llvm-monorepo/lldb/test/API/functionalities/breakpoint/scripted_bkpt/TestScriptedResolver.py", line 152, in do_test
      self.assertEqual(wrong[i].GetNumLocations(), 0, "Breakpoint %d has locations."%(i))
  AssertionError: 1 != 0 : Breakpoint 1 has locations.

Comment at: lldb/source/Breakpoint/BreakpointResolverName.cpp:292
+            filter_by_function), // include symbols only if we
+                                 // aren't filtering by CU or function
           include_inlines, func_list);
[nfc] It could use `include_symbols` here.

Comment at: lldb/source/Core/SearchFilter.cpp:713
+  if (!type)
+    return SearchFilterByModuleList::FunctionPasses(function);
If we cannot determine which file the function is from then rather ignore it (the current call returns `true`):
  return false;

Comment at: lldb/source/Core/SearchFilter.cpp:726
+  if (m_target_sp &&
+      m_target_sp->GetInlineStrategy() == eInlineBreakpointsHeaders) {
+    if (!sym_ctx.comp_unit) {
This value should be cached.
But besides that there should be `GetInlineStrategy() == eInlineBreakpointsNever || (GetInlineStrategy() == eInlineBreakpointsHeaders && cu_spec.IsSourceImplementationFile()modulosomeReverseRemapPath?`.
As for `eInlineBreakpointsAlways` the check is being delegated to `FunctionPasses()`.

Comment at: lldb/source/Core/SearchFilter.cpp:732
+    FileSpec cu_spec;
+    if (sym_ctx.comp_unit) {
+      cu_spec = sym_ctx.comp_unit->GetPrimaryFile();
This condition is always `true` as there is already above:
if (!sym_ctx.comp_unit)

Comment at: lldb/source/Core/SearchFilter.cpp:835
+  if (m_target_sp &&
+      m_target_sp->GetInlineStrategy() == eInlineBreakpointsHeaders)
+    return flags | eSymbolContextCompUnit;
Here should be `eInlineBreakpointsNever || eInlineBreakpointsHeaders` (and probably cache the `GetInlineStrategy()` value).
For `eInlineBreakpointsHeaders` we do not know for sure as we cannot call `IsSourceImplementationFile()` for `cu_spec` which is not known yet here.

  rG LLVM Github Monorepo



More information about the lldb-commits mailing list