[Lldb-commits] [PATCH] D149641: [docs] Hide collaboration and include graphs in doxygen docs

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk accepted this revision.
kwk added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149641/new/

https://reviews.llvm.org/D149641

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D75750: [lldb] integrate debuginfod

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk abandoned this revision.
kwk added a comment.

There already is a Debuginfod implementation in LLVM by now. Abandoning 
revision.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75750/new/

https://reviews.llvm.org/D75750

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D75750: [lldb] integrate debuginfod

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a comment.

Looks like it  is already there: 
https://github.com/llvm/llvm-project/tree/main/llvm/include/llvm/Debuginfod


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75750/new/

https://reviews.llvm.org/D75750

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D75750: [lldb] integrate debuginfod

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a subscriber: phosek.
kwk added a comment.

@iridinite please see these:

- https://discourse.llvm.org/t/http-library-in-llvm/56317/10
- https://discourse.llvm.org/t/rfc-building-llvm-debuginfod/59011
- https://discourse.llvm.org/t/rfc-building-llvm-debuginfod/58994
- https://discourse.llvm.org/t/debuginfod-credential-helper-rfc/64092

I suggest, you contact @phosek on the status of debuginfod implementation in 
LLVM. I'd love to know where it is at, so please leave a trace ;)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75750/new/

https://reviews.llvm.org/D75750

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D97721: [lldb] Support DWARF-5 DW_FORM_line_strp (used by GCC)

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a comment.

@jankratochvil out of curiosity. Can you give a pointer to what the C program 
looks like that produces the object code? I know GCC might change and no longer 
produce this code so it's better to have the obj code instead. But still I'm 
not nearly close to understanding


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97721/new/

https://reviews.llvm.org/D97721

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D83180: Set generic error in SBError SetErrorToGenericError

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a comment.

LGTM but a test would indeed be nice.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83180/new/

https://reviews.llvm.org/D83180



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D83580: [lldb] on s390x fix override issue

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGecfa01e956a4: [lldb] on s390x fix override issue (authored 
by kwk).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83580/new/

https://reviews.llvm.org/D83580

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h


Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h
@@ -76,7 +76,7 @@
   Status WriteFPR() override;
 
   void *GetGPRBuffer() override { return &m_regs; }
-  size_t GetGPRSize() override { return sizeof(m_regs); }
+  size_t GetGPRSize() const override { return sizeof(m_regs); }
   void *GetFPRBuffer() override { return &m_fp_regs; }
   size_t GetFPRSize() override { return sizeof(m_fp_regs); }
 


Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h
@@ -76,7 +76,7 @@
   Status WriteFPR() override;
 
   void *GetGPRBuffer() override { return &m_regs; }
-  size_t GetGPRSize() override { return sizeof(m_regs); }
+  size_t GetGPRSize() const override { return sizeof(m_regs); }
   void *GetFPRBuffer() override { return &m_fp_regs; }
   size_t GetFPRSize() override { return sizeof(m_fp_regs); }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D83580: [lldb] on s390x fix override issue

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a comment.

In D83580#2144871 , @serge-sans-paille 
wrote:

> Possible followup: shouldn't all the other `Get*` be flagged as `const` too?


Thank you @serge-sans-paille . You're right but I'm unsure if all getters are 
actually const or not. And I think one needs to figure this out for all 
overrides as well. And right now I don't want to break another arch ;)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83580/new/

https://reviews.llvm.org/D83580



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D83580: [lldb] on s390x fix override issue

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk created this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This fixes an override issue by marking a function as const so that the
signature maps to the signature of the function in the base class.

This is the original error:

In file included from 
/root/llvm/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.cpp:11:
/root/llvm/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h:79:10:
 error: 'size_t 
lldb_private::process_linux::NativeRegisterContextLinux_s390x::GetGPRSize()' 
marked 'override', but does not override

  79 |   size_t GetGPRSize() override { return sizeof(m_regs); }
 |  ^~


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83580

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h


Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h
@@ -76,7 +76,7 @@
   Status WriteFPR() override;
 
   void *GetGPRBuffer() override { return &m_regs; }
-  size_t GetGPRSize() override { return sizeof(m_regs); }
+  size_t GetGPRSize() const override { return sizeof(m_regs); }
   void *GetFPRBuffer() override { return &m_fp_regs; }
   size_t GetFPRSize() override { return sizeof(m_fp_regs); }
 


Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h
===
--- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h
+++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_s390x.h
@@ -76,7 +76,7 @@
   Status WriteFPR() override;
 
   void *GetGPRBuffer() override { return &m_regs; }
-  size_t GetGPRSize() override { return sizeof(m_regs); }
+  size_t GetGPRSize() const override { return sizeof(m_regs); }
   void *GetFPRBuffer() override { return &m_fp_regs; }
   size_t GetFPRSize() override { return sizeof(m_fp_regs); }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 273637.
kwk added a comment.

- Simplify logic


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/API/functionalities/breakpoint/scripted_bkpt/TestScriptedResolver.py
  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 

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

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk marked 2 inline comments as done.
kwk added inline comments.



Comment at: lldb/source/Breakpoint/BreakpointResolverName.cpp:323
+  // passing.
+  remove_it = false;
+  }

jankratochvil wrote:
> Now `if (filter_by_function) {}` always overrides any result from `if 
> (filter_by_cu)`. So it would be faster + more clear to read as:
> ```
>   if (filter_by_function) {
> if (!sc.function || !filter.FunctionPasses(*sc.function))
>   remove_it = true;
>   } else if (filter_by_cu) {
> if (!sc.comp_unit || !filter.CompUnitPasses(*sc.comp_unit))
>   remove_it = true;
>   }
> ```
> 
Thanks @jankratochvil ! You were right about the logic. I must have thought to 
keep as much from the old code and only add to it. But in this case it makes 
much more sense to structure the logic the way that you've proposed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74136/new/

https://reviews.llvm.org/D74136



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 273307.
kwk added a comment.

- bring back logic to keep a symbol context when a function passes and add a 
comment as Jan suggested
- remove test from scripted resolver that calls 
SearchFilterByModulesAndSupportFiles::AddressPasses
  - before the test checked that a non existing file doesn't cause setting a 
breakpoint location
  - the logic in AddressPasses before was to identify the CU of a symbol 
context and check if it's file is in the list of files that are allowed to 
pass. We agreed to change this logic so that not only CU's are checked but also 
files in which a function is declared. 
SearchFilterByModulesAndSupportFiles::FunctionPasses now does exactly that but 
it is not called from the BreakPointResolverScripted class, only from 
BreakpointResolverName.
  - It is an open question how to deal with this and I hope Jim can help here.
  - Shall we maybe take another file from the SymbolContext to see if we can 
filter by that in AddressPasses? Here's a dump of the filter context for the 
removed test:

  0x7ffda14d94a0: SymbolContext
  Module   = 0x563169c60780 
/opt/notnfs/kkleine/llvm/build/lldb-test-build.noindex/functionalities/breakpoint/scripted_bkpt/TestScriptedResolver.test_scripted_resolver/a.out
  CompileUnit  = 0x56316a04c310 {} 
/opt/notnfs/kkleine/llvm/lldb/test/API/functionalities/breakpoint/scripted_bkpt/main.c
  Function = 0x56316a048c40 {7fff0043} break_on_me, 
address-range = a.out[0x00401150-0x00401167)
  Type = 0x56316a0220d0: Type{0x7fff0043} , name = 
"break_on_me", decl = main.c:10, compiler_type = 0x56316a0cbe80 void (void)
  
  Block= 0x56316a048c78 {7fff0043}
  LineEntry= a.out[0x00401150-0x00401154), file = 
/opt/notnfs/kkleine/llvm/lldb/test/API/functionalities/breakpoint/scripted_bkpt/main.c,
 line = 11, is_start_of_statement = TRUE
  Symbol   = 0x56316a037248
  Variable = 0x



- rename var


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/API/functionalities/breakpoint/scripted_bkpt/TestScriptedResolver.py
  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 s

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

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 273309.
kwk added a comment.

- Add newlines


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/API/functionalities/breakpoint/scripted_bkpt/TestScriptedResolver.py
  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 se

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

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk marked an inline comment as done.
kwk added inline comments.



Comment at: 
lldb/test/API/functionalities/breakpoint/scripted_bkpt/TestScriptedResolver.py:124
-file_list.Append(lldb.SBFileSpec("noFileOfThisName.xxx"))
-wrong.append(target.BreakpointCreateFromScript("resolver.Resolver", 
extra_args, module_list, file_list))
-

@jingham can you please let me know if this test serves any purpose with my 
patch? I mentioned in my last change log 
(https://reviews.llvm.org/D74136#2113899) that it calls `AddressPasses` and 
checked for the CU before. But since we're no longer doing this, this test no 
longer can pass. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74136/new/

https://reviews.llvm.org/D74136



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 271631.
kwk added a comment.

- Remove old logic that was no longer needed since my search filter now 
adaptively adds eSymbolContextCompUnit and not always returns it.


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 sho

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

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk marked 6 inline comments as done.
kwk added inline comments.



Comment at: lldb/source/Core/SearchFilter.cpp:713
+  if (!type)
+return SearchFilterByModuleList::FunctionPasses(function);
+

jankratochvil wrote:
> If we cannot determine which file the function is from then rather ignore it 
> (the current call returns `true`):
> ```
>   return false;
> ```
@jankratochvil so far I haven't addressed this on purpose to give @labath and 
@jingham time to say what they think about this.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74136/new/

https://reviews.llvm.org/D74136



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a comment.

@labath @jingham @jankratochvil The test change suggested by @labath is now in 
place and it works. @jankratochvil, I've removed the logic that checks 
seomthing with the CU from `AddressPasses`. That logic now lives in 
`FunctionPasses` where it logically makes more sense to me. The whole filtering 
by CU logic from `AddressPasses` is gone now and I wonder if @jingham has some 
thoughts on this.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74136/new/

https://reviews.llvm.org/D74136



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

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 

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

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk marked an inline comment as done.
kwk added inline comments.



Comment at: lldb/source/Core/SearchFilter.cpp:732
+FileSpec cu_spec;
+if (sym_ctx.comp_unit) {
+  cu_spec = sym_ctx.comp_unit->GetPrimaryFile();

jankratochvil wrote:
> This condition is always `true` as there is already above:
> ```
> if (!sym_ctx.comp_unit)
> ```
> 
That' incorrect. Only if the nested `if (m_support_file_list.GetSize() != 0)` 
is `false`, then `sym_ctx.comp_unit` is potentially `true`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74136/new/

https://reviews.llvm.org/D74136



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk planned changes to this revision.
kwk added a comment.

IMPORTANT: The behavior of `target.inline-breakpoint-strategy` when set to 
`headers` is still subject to change!

I think the setting is not respected correctly...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74136/new/

https://reviews.llvm.org/D74136



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a comment.

@labath I've applied all the ideas we ping-ponged yesterday and I decided to go 
with alternating the `target.inline-breakpoint-strategy` from `always` (the 
default) to `headers`. This way you can exactly see in the test file how things 
are behaving. So before going into the actual code review I'd like to ask you 
and @jingham to take a look at the test file. Is it the behavior described 
there the desired outcome? Then we can discuss the implementation.




Comment at: lldb/source/Breakpoint/BreakpointResolverName.cpp:320
+else
+  remove_it = false;
+  }

This is done on purpose to reverse the decision to remove a context for not 
passing a CU above.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74136/new/

https://reviews.llvm.org/D74136



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 269787.
kwk marked an inline comment as done.
kwk added a comment.

- remove debug output from test


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: no locations (pending).
+
+
+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_

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

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 269784.
kwk added a comment.

- Fix comment


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 --color --dump-input=always %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: no locations (pending).
+
+
+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.

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

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 269782.
kwk added a comment.

- remove commented out code


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 --color --dump-input=always %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: no locations (pending).
+
+
+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

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

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 269781.
kwk marked an inline comment as done.
kwk added a comment.

- Outsource parts into SearchFilterByModulesAndSupportFiles::FunctionPasses
- Tests with alternating setting target.inline-breakpoint-strategy between 
"always" and "headers"
- Respecting target.inline-breakpoint-strategy setting in 
SearchFilterByModulesAndSupportFiles and maded adjustments in 
BreakpointResolverName::SearchCallback
- Renamed SearchFilterByModuleListAndCUList to 
SearchFilterByModulesAndSupportFiles


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 --color --dump-input=always %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: no locations (pending).
+
+
+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).
+
+
+
+# 

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

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk marked an inline comment as done.
kwk added a comment.

@labath  please see my inline comment.




Comment at: lldb/source/Breakpoint/BreakpointResolverName.cpp:315
+  if (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

labath wrote:
> This mention of a "function call" is confusing. Either a function is in a 
> file or it isn't. Why do we care about who calls who?
@labath we have this `filter_by_function` as an indirection to let 
`BreakpointResolverName::SearchCallback` know that the `SearchFilter` needs a 
special handling. But while reviewing the code now I noticed that only the 
implementations of `SearchFilter::GetFilterRequiredItems` currently either 
return

  - `eSymbolContextModule` (for `SearchFilterByModule` and 
`SearchFilterByModuleList`) or  
  - `eSymbolContextModule | eSymbolContextCompUnit | eSymbolContextFunction` 
(for `SearchFilterByModuleListAndCU`). 

I thought this was clever at a time when the old 
`SearchFilterByModuleListAndCU` which then only returned `eSymbolContextModule 
| eSymbolContextCompUnit `. With my new search filter I wanted a special 
handling which is why I wrote a new search filter class and had it return 
`eSymbolContextModule | eSymbolContextCompUnit | eSymbolContextFunction`. But 
when you confirmed my question to change the default behavior 
(https://reviews.llvm.org/D74136#1869622), the new class became the default 
implementation and there no longer is a need to distinguish the required items 
for this search filter.

Makes sense?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74136/new/

https://reviews.llvm.org/D74136



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 266493.
kwk marked an inline comment as done.
kwk added a comment.

- don't hard-code --color and --dump-input on FileCheck invocation


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74136/new/

https://reviews.llvm.org/D74136

Files:
  lldb/source/Breakpoint/BreakpointResolverName.cpp
  lldb/source/Core/SearchFilter.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,46 @@
+# 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: %build %p/Inputs/search-support-files.cpp -o %t.out
+# RUN: %lldb -b -s %s %t.out | FileCheck %s 
+
+#Set breakpoint by function name.
+
+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
+
+breakpoint set -n main
+# CHECK: (lldb) breakpoint set -n main
+# CHECK-NEXT: Breakpoint 2: where = {{.*}}.out`main{{.*}} at search-support-files.cpp
+
+#   Set breakpoint by function name and filename (the one in which the function
+#   is inlined, aka the compilation unit).
+
+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 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
+# search by source files that are themselves no compilation units.
+
+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 4: where = {{.*}}.out`function_in_header(){{.*}} at search-support-files.h
+
+#   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 a file that doesn't exist, we should get no
+#   results.
+
+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 5: no locations (pending).
+# CHECK-NEXT: WARNING: Unable to resolve breakpoint to any actual locations.
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; }
Index: lldb/test/Shell/Breakpoint/Inputs/search-support-files.cpp
===
--- /dev/null
+++ lldb/test/Shell/Breakpoint/Inputs/search-support-files.cpp
@@ -0,0 +1,6 @@
+#include "search-support-files.h"
+
+int main(int argc, char *argv[]) {
+  int a = function_in_header();
+  return a;
+}
Index: lldb/source/Core/SearchFilter.cpp
===
--- lldb/source/Core/SearchFilter.cpp
+++ lldb/source/Core/SearchFilter.cpp
@@ -712,12 +712,7 @@
 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) {
@@ -811,7 +806,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
@@ -260,6 +260,8 @@
   SymbolContextList func_list;
   bool

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

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 266472.
kwk added a comment.

- make test CHECKs less strict


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74136/new/

https://reviews.llvm.org/D74136

Files:
  lldb/source/Breakpoint/BreakpointResolverName.cpp
  lldb/source/Core/SearchFilter.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,46 @@
+# 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: %build %p/Inputs/search-support-files.cpp -o %t.out
+# RUN: %lldb -b -s %s %t.out | FileCheck --color --dump-input=fail %s 
+
+#Set breakpoint by function name.
+
+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
+
+breakpoint set -n main
+# CHECK: (lldb) breakpoint set -n main
+# CHECK-NEXT: Breakpoint 2: where = {{.*}}.out`main{{.*}} at search-support-files.cpp
+
+#   Set breakpoint by function name and filename (the one in which the function
+#   is inlined, aka the compilation unit).
+
+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 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
+# search by source files that are themselves no compilation units.
+
+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 4: where = {{.*}}.out`function_in_header(){{.*}} at search-support-files.h
+
+#   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 a file that doesn't exist, we should get no
+#   results.
+
+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 5: no locations (pending).
+# CHECK-NEXT: WARNING: Unable to resolve breakpoint to any actual locations.
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; }
Index: lldb/test/Shell/Breakpoint/Inputs/search-support-files.cpp
===
--- /dev/null
+++ lldb/test/Shell/Breakpoint/Inputs/search-support-files.cpp
@@ -0,0 +1,6 @@
+#include "search-support-files.h"
+
+int main(int argc, char *argv[]) {
+  int a = function_in_header();
+  return a;
+}
Index: lldb/source/Core/SearchFilter.cpp
===
--- lldb/source/Core/SearchFilter.cpp
+++ lldb/source/Core/SearchFilter.cpp
@@ -712,12 +712,7 @@
 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) {
@@ -811,7 +806,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
@@ -260,6 +260,8 @@
   SymbolContextList func_list;
   bool filter_by_cu =
   (filter.GetFilterRequired

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

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk marked 2 inline comments as done.
kwk added inline comments.



Comment at: lldb/test/Shell/Breakpoint/Inputs/search-support-files.h:1
+int inlined_42() { return 42; }

labath wrote:
> Calling this `inlined` is misleading. The function won't get inlined anywhere 
> at -O0, and in fact your test would not work if it got inlined. Maybe just 
> call it `function_in_header` ?
Thank you for finding this. My understanding needs a lot of sharpening I guess.



Comment at: lldb/test/Shell/Breakpoint/search-support-files.test:15
+# 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{{.*}}
+

labath wrote:
> These check lines hardcode too much stuff. The `+4` thingy can easily change 
> due to unrelated codegen changes, and even the `:1:20` seems unnecessarily 
> strict.
> Maybe something like this would be enough:
> ```
> CHECK-NEXT: Breakpoint 1: where = {{.8}}`inlined_42{{.*}} at 
> search-support-files.h
> ```
Yes, you're right. Although I don't know what  `{{.8}}` does in this case. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74136/new/

https://reviews.llvm.org/D74136



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 266465.
kwk marked 3 inline comments as done.
kwk added a comment.

- use %t in files created in tests


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74136/new/

https://reviews.llvm.org/D74136

Files:
  lldb/source/Breakpoint/BreakpointResolverName.cpp
  lldb/source/Core/SearchFilter.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,46 @@
+# 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: %build %p/Inputs/search-support-files.cpp -o %t.out
+# RUN: %lldb -b -s %s %t.out | FileCheck --color --dump-input=fail %s 
+
+#Set breakpoint by function name.
+
+breakpoint set -n function_in_header
+# CHECK: (lldb) breakpoint set -n function_in_header
+# CHECK-NEXT: Breakpoint 1: where = {{.*}}.out`function_in_header() + 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 = {{.*}}.out`main + 22 at search-support-files.cpp:4: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 function_in_header -f search-support-files.cpp
+# CHECK: (lldb) breakpoint set -n function_in_header -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
+# search by source files that are themselves no compilation units.
+
+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 4: where = {{.*}}.out`function_in_header() + 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 a file that doesn't exist, we should get no
+#   results.
+
+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 5: no locations (pending).
+# CHECK-NEXT: WARNING: Unable to resolve breakpoint to any actual locations.
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; }
Index: lldb/test/Shell/Breakpoint/Inputs/search-support-files.cpp
===
--- /dev/null
+++ lldb/test/Shell/Breakpoint/Inputs/search-support-files.cpp
@@ -0,0 +1,6 @@
+#include "search-support-files.h"
+
+int main(int argc, char *argv[]) {
+  int a = function_in_header();
+  return a;
+}
Index: lldb/source/Core/SearchFilter.cpp
===
--- lldb/source/Core/SearchFilter.cpp
+++ lldb/source/Core/SearchFilter.cpp
@@ -712,12 +712,7 @@
 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) {
@@ -811,7 +806,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/BreakpointResolverN

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

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 266463.
kwk marked 3 inline comments as done.
kwk added a comment.
Herald added a subscriber: sstefan1.

- rebase
- Rename function in test from inlined_42 to function_in_header
- Typo: compulation -> compilation


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74136/new/

https://reviews.llvm.org/D74136

Files:
  lldb/source/Breakpoint/BreakpointResolverName.cpp
  lldb/source/Core/SearchFilter.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,46 @@
+# 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: %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 function_in_header
+# CHECK: (lldb) breakpoint set -n function_in_header
+# CHECK-NEXT: Breakpoint 1: where = dummy.out`function_in_header() + 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:4: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 function_in_header -f search-support-files.cpp
+# CHECK: (lldb) breakpoint set -n function_in_header -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
+# search by source files that are themselves no compilation units.
+
+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 4: where = dummy.out`function_in_header() + 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 a file that doesn't exist, we should get no
+#   results.
+
+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 5: no locations (pending).
+# CHECK-NEXT: WARNING: Unable to resolve breakpoint to any actual locations.
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; }
Index: lldb/test/Shell/Breakpoint/Inputs/search-support-files.cpp
===
--- /dev/null
+++ lldb/test/Shell/Breakpoint/Inputs/search-support-files.cpp
@@ -0,0 +1,6 @@
+#include "search-support-files.h"
+
+int main(int argc, char *argv[]) {
+  int a = function_in_header();
+  return a;
+}
Index: lldb/source/Core/SearchFilter.cpp
===
--- lldb/source/Core/SearchFilter.cpp
+++ lldb/source/Core/SearchFilter.cpp
@@ -712,12 +712,7 @@
 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) {
@@ -811,7 +806,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-commits] [PATCH] D80543: [lldb] Manual remove of DISALLOW_COPY_AND_ASSIGN def and one expansion

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk abandoned this revision.
kwk added a comment.

Sorry, wrong revisions uploaded


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80543/new/

https://reviews.llvm.org/D80543



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D72698: [lldb] Add method decls to a CXXRecordDecl only after all their properties are defined

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a comment.

> The only code that is currently after addDecl is changing whether the special 
> members are
>  defaulted/trivial. I'm not sure if this actually fixes anything but it's 
> more correct than what we
>  did before.

But at least you return immediately after calling `addDecl`.

When trying to see what `VerifyDecl(cxx_method_decl)` does, I noticed that I 
don't have a `ClangASTContext.cpp` in my source tree. Even grepping for the 
file in the last 400 revisions didn't show it: `git log --stat -n400 | grep 
ClangASTContext.cpp`. No luck with github as well: 
https://github.com/llvm/llvm-project/search?q=ClangASTContext.cpp&unscoped_q=ClangASTContext.cpp.
 Is that expected?


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72698/new/

https://reviews.llvm.org/D72698



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D78242: [lldb/Docs] Add some more info about the test suite layout

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a comment.

I've marked some mistakes that were not addressed yet but are marked as "Done".




Comment at: lldb/docs/resources/test.rst:86
+
+API tests are located under ``lldb/test/API``. Thy are run with the
+``dotest.py``. Tests are written in Python and test binaries (inferiors) are

kwk wrote:
> s/Thy/They
Not done yet.



Comment at: lldb/docs/resources/test.rst:94
+The test directory will always contain a python file, starting with ``Test``.
+Most of the tests are structured as a binary begin debugged, so there will be
+one or more sources file and a Makefile.

kwk wrote:
> s/begin/being
Not done yet.



Comment at: lldb/docs/resources/test.rst:95
+Most of the tests are structured as a binary begin debugged, so there will be
+one or more sources file and a Makefile.
 

kwk wrote:
> s/sources/source
> s/file/files
Not done yet.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78242/new/

https://reviews.llvm.org/D78242



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D75750: [lldb] integrate debuginfod

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a subscriber: fche.
kwk added a comment.

In D75750#1988330 , @labath wrote:

> lldb-dev is indeed a better place for the architectural discussion. However, 
> moving the discussion there does not automatically unblock this patch. "get 
> something in now and improve the architecture later" almost never works out 
> in practice. In fact I would say that adding debuginfod is a good way to 
> cement the status quo.


I get that, but hear me out...

> The situation around finding symbols is messy enough already

The way in which I integrated debuginfod for now is just to find source files 
and not yet symbols. That being said. I don't fear the status quo so much. The 
status quo is probably worse for symbols than it is for source files, don't you 
think? So with *all* the CMake integration, the hosting inside 
`lldb/include/lldb/Host/DebugInfoD.h` and your beloved test case,

testcase 


I think it is fair to say that at least some work is there that can be taken 
into LLDB. **As long** as I fix the retrieval of the module in 
`SourceManager::File::CommonInitializer`. As suggested by @jankratochvil either 
here or on IRC, I would like to give it a shot and try to pass down the correct 
module to this function. I'd say, let's see if this function can be passed a 
Module and if the changes are worth it. The whole part for retrieving debug 
information can come when the architectural changes are done. But then it's a 
piece of cake to extend `lldb/include/lldb/Host/DebugInfoD.h` with the right 
methods to call the debuginfod client lib.

> because one needs to understand the funky mac symbol searching mechanism, 
> which is pretty much impossible without a mac machine.

I'm setting up my old mac to compile LLDB and I guess @jankratochvil might soon 
also have his own Mac. This at least puts us in a position where we can verify 
some of our changes.

> After debuginfod, one will need to understand both, and have a linux machine 
> with some debuginfod setup. The set of such people is likely to be empty of a 
> long time...

I'm not sure if I understand you correctly but to me the *setup* is just to 
point to a machine with *your* or a hosted server. At least for OS binaries 
@fche2 @fche  (which is the correct one?) is making some effort to have those 
debuginfos and source files available and setup. That is a great start for most 
embedded systems with not much disk space to install all debug information I 
guess. Correct my if this is a wrong anticipation. Sure, I mean it will take a 
while before LLDB with debuginfod will make it into a distribution but hey, 
time flies by.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75750/new/

https://reviews.llvm.org/D75750



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D78337: [lldb/Host] Remove TaskPool and replace its uses with llvm::ThreadPool

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk accepted this revision.
kwk added a comment.
This revision is now accepted and ready to land.

All tests pass.  I first thought that the 
`lldb/test/Shell/Reproducer/TestGDBRemoteRepro.test` test didn't work but it 
seems to be flaky.




Comment at: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp:90
   // unit refers to another and the indexes accesses those DIEs.
-  TaskMapOverInt(0, units_to_index.size(), extract_fn);
+  TaskMapOverInt(units_to_index.size(), extract_fn);
 

labath wrote:
> All of this should use the same ThreadPool instance to avoid re-creating the 
> pool threads a couple of times.
@JDevlieghere I assume you've removed the first of the three parameters to 
`TaskMapOverInt` because it was always `0` anyways? If not, shouldn't this be 
easily changeable?

```lang=c++

static void TaskMapOverInt(size_t idx, size_t end,
   const llvm::function_ref &func) {
  llvm::ThreadPool pool;
  for (; idx < end; idx++)
pool.async(func, idx);
  pool.wait();
}
```


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78337/new/

https://reviews.llvm.org/D78337



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D78337: [lldb/Host] Remove TaskPool and replace its uses with llvm::ThreadPool

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk resigned from this revision.
kwk added a comment.
This revision now requires review to proceed.

I resign because I think @labath made some good points that I cannot argue 
about.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78337/new/

https://reviews.llvm.org/D78337



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D75750: [lldb] integrate debuginfod

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a comment.

In D75750#1971446 , @labath wrote:

> In D75750#1967019 , @fche2 wrote:
>
> > >> So it might be good to have the SymbolVendors use one or more 
> > >> SymbolServer plug-ins.
> > > 
> > > I don't believe we have anything that would require all modules in a 
> > > given target (or whatever) to use the same symbol vendor type.  [...]
> >
> > Just for clarity, is someone proposing to undertake such a rework of that 
> > infrastructure?  It sounds like this is becoming a prerequisite for 
> > Konrad's patch, but if no one's actually doing it, that means Konrad's work 
> > is on hold indefinitely.  Is that the intent?
>
>
> Yes, I believe that is becoming a prerequisite. I believe Konrad is willing 
> to try to implement that, but I have advised him to hold on a bit until the 
> exact details are hashed out.


@labath, I'm not really keen on implementing the architectural changes that you 
mentioned because it will take ages when I do that. And the cross-platform bit 
makes me nervous as well. Initially I hoped we might be able to integrate my 
work and improve on the architecture later. Then we're not fighting on too many 
fronts at the same time?

Shall we maybe move the discussion about the architectural changes to lldb-dev 
instead of this patch? @clayborg @labath @jingham @jankratochvil ?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75750/new/

https://reviews.llvm.org/D75750



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D78242: [lldb/Docs] Add some more info about the test suite layout

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a comment.

This looks good overall. I would add a section describing which test suite to 
use when you're interested in a particular DWARF feature for example. I heard 
from my GDB colleagues that the don't use a compiler, because that might change 
and produce a different DWARF. Instead they use a DWARF assembler (IIRC). I 
guess the analogy to LLDB would be yaml2obj which I see not mentioned here. 
Does it make sense to at least reference it in a section?

I forgot to mention that I'd love to have this at hand when I began developing 
for LLDB. So thank you very much for writing this up!!!




Comment at: lldb/docs/resources/test.rst:86
+
+API tests are located under ``lldb/test/API``. Thy are run with the
+``dotest.py``. Tests are written in Python and test binaries (inferiors) are

s/Thy/They



Comment at: lldb/docs/resources/test.rst:88
+``dotest.py``. Tests are written in Python and test binaries (inferiors) are
+compiled with Make. The majority of API tests are end-to-end tests that compile
+programs from source, run them, and debug the processes.

I personally struggled with the way inferiors are being build. The Makefile 
includes another Makefile and was more like a framework than simple make. You 
had to set special variables in order for the included Makefile to pick it up. 
That level of indirection made it quite complicated for me to get what I 
wanted. To put it differently, it would be nice if you could describe what the 
Makefile should look like or what is expected.



Comment at: lldb/docs/resources/test.rst:94
+The test directory will always contain a python file, starting with ``Test``.
+Most of the tests are structured as a binary begin debugged, so there will be
+one or more sources file and a Makefile.

s/begin/being



Comment at: lldb/docs/resources/test.rst:95
+Most of the tests are structured as a binary begin debugged, so there will be
+one or more sources file and a Makefile.
 

s/sources/source
s/file/files



Comment at: lldb/docs/resources/test.rst:122
+
+It's possible to skip or XFAIL tests using decorators. You'll see them a lot.
+The debugger can be sensitive to things like the architecture, the host and

Maybe link XFAIL to 
https://ftp.gnu.org/old-gnu/Manuals/dejagnu-1.3/html_node/dejagnu_6.html ? 



Comment at: lldb/docs/resources/test.rst:142
+building inferiors. Every test has its own Makefile, most of them only a few
+lines long. A shared Makefile (``Makefile.rules``) with about a thousand lines
+of rules takes care of most if not all of the boiler plate, while individual

Ah, there you're mentioning it.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78242/new/

https://reviews.llvm.org/D78242



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 258004.
kwk added a comment.

- Remove not needed include


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74136/new/

https://reviews.llvm.org/D74136

Files:
  lldb/source/Breakpoint/BreakpointResolverName.cpp
  lldb/source/Core/SearchFilter.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,46 @@
+# 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: %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:4: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
+# 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 a 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-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,6 @@
+#include "search-support-files.h"
+
+int main(int argc, char *argv[]) {
+  int a = inlined_42();
+  return a;
+}
Index: lldb/source/Core/SearchFilter.cpp
===
--- lldb/source/Core/SearchFilter.cpp
+++ lldb/source/Core/SearchFilter.cpp
@@ -712,12 +712,7 @@
 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) {
@@ -811,7 +806,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
@@ -260,6 +260,8 @@
   SymbolContextList func_list;
   bool filter_by_cu =
   (filter.GetFilterRequiredItems() & eSymbolContex

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

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 257691.
kwk added a comment.

- Revert "Honor the module"


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74136/new/

https://reviews.llvm.org/D74136

Files:
  lldb/source/Breakpoint/BreakpointResolverName.cpp
  lldb/source/Core/SearchFilter.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,46 @@
+# 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: %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:4: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
+# 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 a 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-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,6 @@
+#include "search-support-files.h"
+
+int main(int argc, char *argv[]) {
+  int a = inlined_42();
+  return a;
+}
Index: lldb/source/Core/SearchFilter.cpp
===
--- lldb/source/Core/SearchFilter.cpp
+++ lldb/source/Core/SearchFilter.cpp
@@ -712,12 +712,7 @@
 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) {
@@ -811,7 +806,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
@@ -246,6 +246,8 @@
 // accelerate function lookup.  At that point, we should switch the depth to
 // CompileUnit, and look in the

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

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk marked an inline comment as done.
kwk added a comment.

In D74136#1889066 , @labath wrote:

> Yes, I believe this matches the behavior we were talking about.
>
> I could make a bunch of comments on the implementation and the test, but I'm 
> not sure if we're at that stage yet...


I'd be happy to hear about your comments @labath because I'm kind of stuck in 
what I can think of. I will obviously rename `SearchFilterByModuleListAndCU` 
but that can come in a child revision.




Comment at: lldb/source/Core/SearchFilter.cpp:712
+  // the list of CU's or support files, that's enough.
+  // TODO(kwk): Is that enough?
+

labath wrote:
> I'm not sure -- it'd be good to check that we still honor the module (aka 
> --shlib) restriction.
@labath How do you want to honor it? I guess `ModulePasses(fileSpec)` as a 
check is wrong, isn't it?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74136/new/

https://reviews.llvm.org/D74136



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 257688.
kwk added a comment.

- Honor the module


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74136/new/

https://reviews.llvm.org/D74136

Files:
  lldb/source/Breakpoint/BreakpointResolverName.cpp
  lldb/source/Core/SearchFilter.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,46 @@
+# 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: %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:4: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
+# 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 a 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-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,6 @@
+#include "search-support-files.h"
+
+int main(int argc, char *argv[]) {
+  int a = inlined_42();
+  return a;
+}
Index: lldb/source/Core/SearchFilter.cpp
===
--- lldb/source/Core/SearchFilter.cpp
+++ lldb/source/Core/SearchFilter.cpp
@@ -712,16 +712,12 @@
 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;
+  return ModulePasses(fileSpec) &&
+ m_cu_spec_list.FindFileIndex(0, fileSpec, false) != UINT32_MAX;
 }
 
 bool SearchFilterByModuleListAndCU::CompUnitPasses(CompileUnit &compUnit) {
@@ -811,7 +807,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-commits] [PATCH] D74136: [LLDB] WIP: Follow DW_AT_decl_file when setting breakpoint

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 257682.
kwk added a comment.

- Modify SearchFilterByModuleListAndCU
- format


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74136/new/

https://reviews.llvm.org/D74136

Files:
  lldb/source/Breakpoint/BreakpointResolverName.cpp
  lldb/source/Core/SearchFilter.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,46 @@
+# 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: %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:4: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
+# 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 a 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-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,6 @@
+#include "search-support-files.h"
+
+int main(int argc, char *argv[]) {
+  int a = inlined_42();
+  return a;
+}
Index: lldb/source/Core/SearchFilter.cpp
===
--- lldb/source/Core/SearchFilter.cpp
+++ lldb/source/Core/SearchFilter.cpp
@@ -712,12 +712,7 @@
 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) {
@@ -811,7 +806,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
@@ -246,6 +246,8 @@
 // accelerate function lookup.  At that point, we should switch the depth to
 // CompileU

[Lldb-commits] [PATCH] D78109: Ran git clang-format

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk abandoned this revision.
kwk added a comment.

Sorry, wrong `arc diff` invocation.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78109/new/

https://reviews.llvm.org/D78109



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 257313.
kwk added a comment.

Ran git clang-format


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74136/new/

https://reviews.llvm.org/D74136

Files:
  lldb/source/Breakpoint/BreakpointResolverName.cpp
  lldb/source/Core/SearchFilter.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,46 @@
+# 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: %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:4: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
+# 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 a 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-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,6 @@
+#include "search-support-files.h"
+
+int main(int argc, char *argv[]) {
+  int a = inlined_42();
+  return a;
+}
Index: lldb/source/Core/SearchFilter.cpp
===
--- lldb/source/Core/SearchFilter.cpp
+++ lldb/source/Core/SearchFilter.cpp
@@ -715,9 +715,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) {
@@ -811,7 +815,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/BreakpointResolver

[Lldb-commits] [PATCH] D78109: Ran git clang-format

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk created this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Depends on D74136 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78109

Files:
  lldb/source/Core/SearchFilter.cpp


Index: lldb/source/Core/SearchFilter.cpp
===
--- lldb/source/Core/SearchFilter.cpp
+++ lldb/source/Core/SearchFilter.cpp
@@ -717,8 +717,8 @@
 cu_spec = sym_ctx.comp_unit->GetPrimaryFile();
   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.
+  // ^ 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);


Index: lldb/source/Core/SearchFilter.cpp
===
--- lldb/source/Core/SearchFilter.cpp
+++ lldb/source/Core/SearchFilter.cpp
@@ -717,8 +717,8 @@
 cu_spec = sym_ctx.comp_unit->GetPrimaryFile();
   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.
+  // ^ 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);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 255648.
kwk added a comment.

- Simplified test


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74136/new/

https://reviews.llvm.org/D74136

Files:
  lldb/source/Breakpoint/BreakpointResolverName.cpp
  lldb/source/Core/SearchFilter.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,46 @@
+# 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: %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:4: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
+# 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 a 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-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,6 @@
+#include "search-support-files.h"
+
+int main(int argc, char *argv[]) {
+  int a = inlined_42();
+  return a;
+}
Index: lldb/source/Core/SearchFilter.cpp
===
--- lldb/source/Core/SearchFilter.cpp
+++ lldb/source/Core/SearchFilter.cpp
@@ -715,9 +715,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) {
@@ -811,7 +815,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/BreakpointResolverNam

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

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 255339.
kwk added a comment.

- rebased onto master to get rid of NFC change


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74136/new/

https://reviews.llvm.org/D74136

Files:
  lldb/source/Breakpoint/BreakpointResolverName.cpp
  lldb/source/Core/SearchFilter.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/Core/SearchFilter.cpp
===
--- lldb/source/Core/SearchFilter.cpp
+++ lldb/source/Core/SearchFilter.cpp
@@ -715,9 +715,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
+ 

[Lldb-commits] [PATCH] D77376: [lldb][nfc] remove overriden funcs with default impl

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
kwk marked an inline comment as done.
Closed by commit rG9072df8ac143: [lldb][nfc] remove overriden funcs with 
default impl (authored by kwk).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77376/new/

https://reviews.llvm.org/D77376

Files:
  lldb/include/lldb/Core/SearchFilter.h
  lldb/source/Core/SearchFilter.cpp

Index: lldb/source/Core/SearchFilter.cpp
===
--- lldb/source/Core/SearchFilter.cpp
+++ lldb/source/Core/SearchFilter.cpp
@@ -417,12 +417,6 @@
   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;
@@ -543,14 +537,6 @@
   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;
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 the default implementation always returns \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 the default implementation always returns \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 the default implementation always returns \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 the default implementation always returns \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 the default implementation always returns \c true.
   virtual bool CompUnitPasses(CompileUnit &compUnit);
 
   /// Call this method with a Function to see if \a function passes the
@@ -321,10 +331,6 @@
 
   bool AddressPasses(Address &address) override;
 
-  bool CompUnitPasses(FileSpec &fileSpec) override;
-
-  bool CompUnitPasses(CompileUnit &compUnit) override;
-
   void GetDescription(Stream *s) override;
 
   uint32_t GetFilterRequiredItems() override;
@@ -372,10 +378,6 @@
 
   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

[Lldb-commits] [PATCH] D77376: [lldb][nfc] remove overriden funcs with default impl

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 255300.
kwk added a comment.

- Simplify comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77376/new/

https://reviews.llvm.org/D77376

Files:
  lldb/include/lldb/Core/SearchFilter.h
  lldb/source/Core/SearchFilter.cpp

Index: lldb/source/Core/SearchFilter.cpp
===
--- lldb/source/Core/SearchFilter.cpp
+++ lldb/source/Core/SearchFilter.cpp
@@ -417,12 +417,6 @@
   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;
@@ -543,14 +537,6 @@
   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;
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 the default implementation always returns \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 the default implementation always returns \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 the default implementation always returns \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 the default implementation always returns \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 the default implementation always returns \c true.
   virtual bool CompUnitPasses(CompileUnit &compUnit);
 
   /// Call this method with a Function to see if \a function passes the
@@ -321,10 +331,6 @@
 
   bool AddressPasses(Address &address) override;
 
-  bool CompUnitPasses(FileSpec &fileSpec) override;
-
-  bool CompUnitPasses(CompileUnit &compUnit) override;
-
   void GetDescription(Stream *s) override;
 
   uint32_t GetFilterRequiredItems() override;
@@ -372,10 +378,6 @@
 
   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

[Lldb-commits] [PATCH] D77376: [lldb][nfc] remove overriden funcs with default impl

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk marked 2 inline comments as done.
kwk added a comment.

@jankratochvil thanks for the tip.
@labath, addressed your comments and will now push it.




Comment at: lldb/include/lldb/Core/SearchFilter.h:102
+  ///
+  /// \note if not overriden, default implementation always \c true.
   virtual bool ModulePasses(const FileSpec &spec);

labath wrote:
> kwk wrote:
> > jankratochvil wrote:
> > > I am not against it but FYI in a paragraph above there is already 
> > > written: `The default implementation is "Everything Passes."`
> > > English: IMO missing "returns".
> > Thank you for the missing "returns". I'd like to keep the comment.
> You can keep _a_ comment if you want, but this comment is even internally 
> redundant. I don't see a need to mention overriding, saying "The default 
> (base?) implementation returns true" should be clear enough.
@labath. That is fine. I've changed the comment.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77376/new/

https://reviews.llvm.org/D77376



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D77376: [lldb][nfc] remove overriden funcs with default impl

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a comment.

@jankratochvil for now I've re-added the `AddressPasses` functions and the 
`FIXME` comment. Can you approve?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77376/new/

https://reviews.llvm.org/D77376



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D77376: [lldb][nfc] remove overriden funcs with default impl

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 255026.
kwk edited the summary of this revision.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77376/new/

https://reviews.llvm.org/D77376

Files:
  lldb/include/lldb/Core/SearchFilter.h
  lldb/source/Core/SearchFilter.cpp

Index: lldb/source/Core/SearchFilter.cpp
===
--- lldb/source/Core/SearchFilter.cpp
+++ lldb/source/Core/SearchFilter.cpp
@@ -417,12 +417,6 @@
   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;
@@ -543,14 +537,6 @@
   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;
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, the default implementation always returns \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, the default implementation always returns \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, the default implementation always returns \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, the default implementation always returns \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, the default implementation always returns \c true.
   virtual bool CompUnitPasses(CompileUnit &compUnit);
 
   /// Call this method with a Function to see if \a function passes the
@@ -321,10 +331,6 @@
 
   bool AddressPasses(Address &address) override;
 
-  bool CompUnitPasses(FileSpec &fileSpec) override;
-
-  bool CompUnitPasses(CompileUnit &compUnit) override;
-
   void GetDescription(Stream *s) override;
 
   uint32_t GetFilterRequiredItems() override;
@@ -372,10 +378,6 @@
 
   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

[Lldb-commits] [PATCH] D77376: [lldb][nfc] remove overriden funcs with default impl

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a subscriber: lattner.
kwk added a comment.

Had some problem with my `arc diff` reporting that another git was running 
which it wasn't so I had to update the diff by hand with a bit of trouble.




Comment at: lldb/include/lldb/Core/SearchFilter.h:102
+  ///
+  /// \note if not overriden, default implementation always \c true.
   virtual bool ModulePasses(const FileSpec &spec);

jankratochvil wrote:
> I am not against it but FYI in a paragraph above there is already written: 
> `The default implementation is "Everything Passes."`
> English: IMO missing "returns".
Thank you for the missing "returns". I'd like to keep the comment.



Comment at: lldb/source/Core/SearchFilter.cpp:416
-bool SearchFilterByModule::AddressPasses(Address &address) {
-  // FIXME: Not yet implemented
-  return true;

jankratochvil wrote:
> This comment will get lost. Maybe you could keep this override just due to 
> the FIXME comment. Or keep the FIXME comment some other way there. Or file a 
> Bugzilla tracking Bug instead.
> It is there since: [[ 
> https://github.com/llvm/llvm-project/commit/30fdc8d841c9d24ac5f3d452b6ece84ee0ac991c
>  | Initial checkin of lldb code from internal Apple repo. ]]
I've checked the code myself to find out where it was coming from and found out 
that the FIXME is there for almost 10 years. I wonder if @lattner still 
remembers why it was there or if it can be removed.  By the nature of the other 
default implementations below, I wonder if this was accidentally a left-over or 
should be put into `SearchFilterByModule::CompUnitPasses` as well? 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77376/new/

https://reviews.llvm.org/D77376



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D77376: [lldb][nfc] remove overriden funcs with default impl

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 255024.
kwk marked 3 inline comments as done.
kwk added a comment.

Re-added `AddressPasses` with FIXME comment


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77376/new/

https://reviews.llvm.org/D77376

Files:
  lldb/source/Core/SearchFilter.cpp


Index: lldb/source/Core/SearchFilter.cpp
===
--- lldb/source/Core/SearchFilter.cpp
+++ lldb/source/Core/SearchFilter.cpp
@@ -412,6 +412,11 @@
   return FileSpec::Match(m_module_spec, spec);
 }
 
+bool SearchFilterByModule::AddressPasses(Address &address) {
+  // FIXME: Not yet implemented
+  return true;
+}
+
 void SearchFilterByModule::Search(Searcher &searcher) {
   if (!m_target_sp)
 return;
@@ -527,6 +532,11 @@
   return m_module_spec_list.FindFileIndex(0, spec, true) != UINT32_MAX;
 }
 
+bool SearchFilterByModuleList::AddressPasses(Address &address) {
+  // FIXME: Not yet implemented
+  return true;
+}
+
 void SearchFilterByModuleList::Search(Searcher &searcher) {
   if (!m_target_sp)
 return;


Index: lldb/source/Core/SearchFilter.cpp
===
--- lldb/source/Core/SearchFilter.cpp
+++ lldb/source/Core/SearchFilter.cpp
@@ -412,6 +412,11 @@
   return FileSpec::Match(m_module_spec, spec);
 }
 
+bool SearchFilterByModule::AddressPasses(Address &address) {
+  // FIXME: Not yet implemented
+  return true;
+}
+
 void SearchFilterByModule::Search(Searcher &searcher) {
   if (!m_target_sp)
 return;
@@ -527,6 +532,11 @@
   return m_module_spec_list.FindFileIndex(0, spec, true) != UINT32_MAX;
 }
 
+bool SearchFilterByModuleList::AddressPasses(Address &address) {
+  // FIXME: Not yet implemented
+  return true;
+}
+
 void SearchFilterByModuleList::Search(Searcher &searcher) {
   if (!m_target_sp)
 return;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D77376: [lldb][nfc] remove overriden funcs with default impl

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 255022.
kwk added a comment.

- fix comment


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77376/new/

https://reviews.llvm.org/D77376

Files:
  lldb/include/lldb/Core/SearchFilter.h
  lldb/source/Core/SearchFilter.cpp

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;
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, the default implementation always returns \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, the default implementation always returns \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, the default implementation always returns \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, the default implementation always returns \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, the default implementation always returns \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

[Lldb-commits] [PATCH] D77377: [lldb][nfc] early exit/continue

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG107200ae0a03: [lldb][nfc] early exit/continue (authored by 
kwk).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77377/new/

https://reviews.llvm.org/D77377

Files:
  lldb/source/Breakpoint/BreakpointResolverName.cpp

Index: lldb/source/Breakpoint/BreakpointResolverName.cpp
===
--- lldb/source/Breakpoint/BreakpointResolverName.cpp
+++ lldb/source/Breakpoint/BreakpointResolverName.cpp
@@ -332,65 +332,66 @@
 
   // Remove any duplicates between the function list and the symbol list
   SymbolContext sc;
-  if (func_list.GetSize()) {
-for (uint32_t 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(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 =
-  breakpoint.GetTarget().GetArchitecturePlugin();
-  if (arch)
-arch->AdjustBreakpointAddress(*sc.symbol, break_addr);
-}
-  }
+  if (!func_list.GetSize())
+return Searcher::eCallbackReturnContinue;
+
+  for (uint32_t 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(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)) {
-bool new_location;
-BreakpointLocationSP bp_loc_sp(
-AddLocation(break_addr, &new_location));
-bp_loc_sp->SetIsReExported(is_reexported);
-if (bp_loc_sp && new_location && !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 =
+  breakpoint.GetTarget().GetArchitecturePlugin();
+  if (arch)
+arch->AdjustBreakpointAddress(*sc.symbol, break_addr);
 }
   }
 }
+
+if (!break_addr.IsValid())
+  continue;
+
+if (!filter.AddressPasses(break_addr))
+  continue;
+
+bool new_location;
+BreakpointLocationSP bp_loc_sp(AddLocation(break_addr, &new_location));
+bp_loc_sp->SetIsReExported(is_reexported);
+if (bp_loc_sp && new_location && !breakpoint.IsInternal()) {
+  if (log) {
+StreamString 

[Lldb-commits] [PATCH] D77377: [lldb][nfc] early exit/continue

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk created this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This commit just tries to invert some `if`'s logic to
`return`/`continue` early.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77377

Files:
  lldb/source/Breakpoint/BreakpointResolverName.cpp

Index: lldb/source/Breakpoint/BreakpointResolverName.cpp
===
--- lldb/source/Breakpoint/BreakpointResolverName.cpp
+++ lldb/source/Breakpoint/BreakpointResolverName.cpp
@@ -332,65 +332,66 @@
 
   // Remove any duplicates between the function list and the symbol list
   SymbolContext sc;
-  if (func_list.GetSize()) {
-for (uint32_t 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(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 =
-  breakpoint.GetTarget().GetArchitecturePlugin();
-  if (arch)
-arch->AdjustBreakpointAddress(*sc.symbol, break_addr);
-}
-  }
+  if (!func_list.GetSize())
+return Searcher::eCallbackReturnContinue;
+
+  for (uint32_t 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(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)) {
-bool new_location;
-BreakpointLocationSP bp_loc_sp(
-AddLocation(break_addr, &new_location));
-bp_loc_sp->SetIsReExported(is_reexported);
-if (bp_loc_sp && new_location && !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 =
+  breakpoint.GetTarget().GetArchitecturePlugin();
+  if (arch)
+arch->AdjustBreakpointAddress(*sc.symbol, break_addr);
 }
   }
 }
+
+if (!break_addr.IsValid())
+  continue;
+
+if (!filter.AddressPasses(break_addr))
+  continue;
+
+bool new_location;
+BreakpointLocationSP bp_loc_sp(AddLocation(break_addr, &new_location));
+bp_loc_sp->SetIsReExported(is_reexported);
+if (bp_loc_sp && new_location && !breakpoint.IsInternal()) {
+  if (log) {
+StreamString s;
+bp_loc_sp->GetDescription(&s, ll

[Lldb-commits] [PATCH] D77376: [lldb][nfc] remove overriden funcs with default impl

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk created this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

These three `SearchFilter` methods all return `true` by their default
implementation:

  virtual bool ModulePasses(const FileSpec &spec);
  virtual bool ModulePasses(const lldb::ModuleSP &module_sp);
  virtual bool AddressPasses(Address &addr);
  virtual bool CompUnitPasses(FileSpec &fileSpec);
  virtual bool CompUnitPasses(CompileUnit &compUnit);

That's why I've documented the default behavior and remove the overrides
in these `SearchFilter`-subclasses which all just repeated the default
implementation: `SearchFilterByModule`, `SearchFilterByModuleList`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77376

Files:
  lldb/include/lldb/Core/SearchFilter.h
  lldb/source/Core/SearchFilter.cpp

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;
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

[Lldb-commits] [PATCH] D77336: Findtypes -gmodules fix

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a comment.

Could you explain, why this was done please?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77336/new/

https://reviews.llvm.org/D77336



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D77326: 1/2: [nfc] [lldb] Unindent code

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk accepted this revision.
kwk added a comment.
This revision is now accepted and ready to land.

No need to split this into multiples revisions. It is just a rewrite of some 
odd nesting.




Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2943
+// Make sure the decl contexts match all the way up
+if (dwarf_decl_ctx != type_dwarf_decl_ctx)
+  continue;

jankratochvil wrote:
> kwk wrote:
> > This looks like a logic change and I wonder if it should go in a separate 
> > patch, maybe?
> This should be NFC, do I miss something?
> From:
> ```
> if (a == b) {
>   c;
> }
> ```
> The patch changes it to:
> ```
> if (a != b)
>   continue;
> c;
> ```
> 
Ah, now I see that it was previously a block on the right side of the diff :).




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77326/new/

https://reviews.llvm.org/D77326



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D77326: 1/2: [nfc] [lldb] Unindent code

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a comment.

Overall looks good to me except for one larger logic change. Maybe a your 
comment can clarify this. The code was in a very bad shape before, given the 
countless amounts of shortcuts you could take.




Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2014
 
-if (!method_die_offsets.empty()) {
-  DWARFDebugInfo &debug_info = dwarf->DebugInfo();

I assume this can be removed because you're iterating over `num_matches == 0` 
when it's empty. But I cannot tell about the `DWARFDebugInfo &debug_info = 
dwarf->DebugInfo();` and how costly this call is and if it makes sense to run 
when the offsets are empty.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp:524
   DIEInfoArray die_info_array;
-  if (FindByName(name, die_info_array))
-DWARFMappedHash::ExtractDIEArray(die_info_array, die_offsets);
+  FindByName(name, die_info_array);
+  DWARFMappedHash::ExtractDIEArray(die_info_array, die_offsets);

Why is the `if` no longer needed?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2061
+if (!die) {
+  m_index->ReportInvalidDIERef(die_ref, name.GetStringRef());
+  continue;

Nice shortcut.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2943
+// Make sure the decl contexts match all the way up
+if (dwarf_decl_ctx != type_dwarf_decl_ctx)
+  continue;

This looks like a logic change and I wonder if it should go in a separate 
patch, maybe?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77326/new/

https://reviews.llvm.org/D77326



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D77108: [lldb/DWARF] Fix evaluator crash when accessing empty stack

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a comment.

In D77108#1952039 , @labath wrote:

> In D77108#1951997 , @kwk wrote:
>
> > In D77108#1951879 , @labath wrote:
> >
> > > Most DW_OP cases check their stack, but it's quite possible that others 
> > > were missed too. It might be a nice cleanup to make a function like 
> > > (`getMinimalStackSize(op)`) and move this check up in front of the big 
> > > switch. That could not handle all operators, as for some of them, the 
> > > value is not statically known, but it would handle the vast majority of 
> > > them.
> >
> >
> > @labath I somewhat like that the logic for each `op` is close to the `case` 
> > statement. Creating the function that you suggested would separate this 
> > check out somewhere else and you would have to look at two places. At least 
> > for code review, the current solution is much clearer to me.
>
>
> I don't have a problem with the current patch (modulo the test tweak) -- it 
> fixes a real problem and it follows the style of the surrounding code. 
> However, DWARFExpression::Evaluate is gigantic (2600LOC), and nearly half of 
> that is error handling. Tastes may vary, but I think that's a bigger 
> readability problem than having to look at two places to understand an opcode.


@labath okay, that makes sense. Sorry.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77108/new/

https://reviews.llvm.org/D77108



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D77108: [lldb/DWARF] Fix evaluator crash when accessing empty stack

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a comment.

In D77108#1951879 , @labath wrote:

> In D77108#1951610 , @aprantl wrote:
>
> > This is obviously good! Do you think that a similar error handling bug 
> > might exist in other cases that depend top-of-stack?
>
>
> Most DW_OP cases check their stack, but it's quite possible that others were 
> missed too. It might be a nice cleanup to make a function like 
> (`getMinimalStackSize(op)`) and move this check up in front of the big 
> switch. That could not handle all operators, as for some of them, the value 
> is not statically known, but it would handle the vast majority of them.


@labath I somewhat like that the logic for each `op` is close to the `case` 
statement. Creating the function that you suggested would separate this check 
out somewhere else and you would have to look at two places. At least for code 
review, the current solution is much clearer to me.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77108/new/

https://reviews.llvm.org/D77108



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D75750: [lldb] integrate debuginfod

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 253531.
kwk added a comment.

- Use file:// and require debuginfod 0.179
- simplify FindDebuginfod.cmake


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75750/new/

https://reviews.llvm.org/D75750

Files:
  lldb/cmake/modules/FindDebuginfod.cmake
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/include/lldb/Host/Config.h.cmake
  lldb/include/lldb/Host/DebugInfoD.h
  lldb/source/Core/SourceManager.cpp
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/common/DebugInfoD.cpp
  lldb/test/CMakeLists.txt
  lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
  lldb/test/Shell/lit.cfg.py
  lldb/test/Shell/lit.site.cfg.py.in

Index: lldb/test/Shell/lit.site.cfg.py.in
===
--- lldb/test/Shell/lit.site.cfg.py.in
+++ lldb/test/Shell/lit.site.cfg.py.in
@@ -16,6 +16,7 @@
 config.python_executable = "@PYTHON_EXECUTABLE@"
 config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.lldb_enable_lzma = @LLDB_ENABLE_LZMA@
+config.lldb_enable_debuginfod = @LLDB_ENABLE_DEBUGINFOD@
 config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.lldb_bitness = 64 if @LLDB_IS_64_BITS@ else 32
 config.lldb_enable_python = @LLDB_ENABLE_PYTHON@
Index: lldb/test/Shell/lit.cfg.py
===
--- lldb/test/Shell/lit.cfg.py
+++ lldb/test/Shell/lit.cfg.py
@@ -117,6 +117,9 @@
 if config.lldb_enable_lzma:
 config.available_features.add('lzma')
 
+if config.lldb_enable_debuginfod:
+config.available_features.add('debuginfod')
+
 if find_executable('xz') != None:
 config.available_features.add('xz')
 
Index: lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
@@ -0,0 +1,114 @@
+// clang-format off
+// REQUIRES: debuginfod
+// UNSUPPORTED: darwin, windows
+
+//  Test that we can display the source of functions using debuginfod when the
+//  original source file is no longer present.
+//  
+//  The debuginfod client requires a buildid in the binary, so we compile one in.
+//  We can create the directory structure on disc that the client expects on a
+//  webserver that serves source files. We then set DEBUGINFOD_URLS to the mock
+//  directory using file://. This avoids the need for a
+//  debuginfod server to be run. 
+//  
+//  Go here to find more about debuginfod:
+//  https://sourceware.org/elfutils/Debuginfod.html
+
+
+//We copy this file because we want to compile and later move it away
+
+// RUN: mkdir -p %t.buildroot
+// RUN: cp %s %t.buildroot/test.cpp
+
+//We use the prefix map in order to overwrite all DW_AT_decl_file paths
+//in the DWARF. We cd into the directory before compiling to get
+//DW_AT_comp_dir pickup %t.buildroot as well so it will be replaced by
+///my/new/path.
+
+// RUN: cd %t.buildroot
+// RUN: %clang_host \
+// RUN:   -g \
+// RUN:   -Wl,--build-id="0xaabbccdd" \
+// RUN:   -fdebug-prefix-map=%t.buildroot=/my/new/path \
+// RUN:   -o %t \
+// RUN:   %t.buildroot/test.cpp
+
+
+//We move the original source file to a directory that looks like a debuginfod
+//URL part.
+
+// RUN: rm -rf %t.mock
+// RUN: mkdir -p   %t.mock/buildid/aabbccdd/source/my/new/path
+// RUN: mv%t.buildroot/test.cpp %t.mock/buildid/aabbccdd/source/my/new/path
+
+
+//Adjust where debuginfod stores cache files:
+
+// RUN: rm -rf %t.debuginfod_cache_path
+// RUN: mkdir -p %t.debuginfod_cache_path
+// RUN: export DEBUGINFOD_CACHE_PATH=%t.debuginfod_cache_path
+
+
+//-- TEST 1 --  No debuginfod awareness 
+
+
+// RUN: DEBUGINFOD_URLS="" \
+// RUN: %lldb -f %t -o 'source list -n main' | FileCheck --dump-input=fail %s --check-prefix=TEST-1
+
+// TEST-1: (lldb) source list -n main
+// TEST-1: File: /my/new/path/test.cpp
+// TEST-1-EMPTY:
+
+
+//-- TEST 2 -- debuginfod URL pointing in wrong place --
+
+
+// RUN: DEBUGINFOD_URLS="http://example.com/debuginfod"; \
+// RUN: %lldb -f %t -o 'source list -n main' | FileCheck --dump-input=fail %s --check-prefix=TEST-2
+
+// TEST-2: (lldb) source list -n main
+// TEST-2: File: /my/new/path/test.cpp
+// TEST-2-EMPTY:
+
+
+//-- TEST 3 -- debuginfod URL pointing corectly 
+
+
+// RUN: DEBUGINFOD_URLS="file://%t.mock/" \
+// RUN: %lldb -f %t -o 'source list -n main' | FileCheck --dump-input=fail %s --check-prefix=TEST-3
+
+// TEST-3: (lldb) source list -n main
+// TEST-3: File: /my/new/path/test.cpp
+// TEST-3: {{[0-9]+}}
+// TEST-3-NEXT:{{[0-9]+}}   // Some context lines before
+// TEST-3-NEXT:{{[0-9]+}}   // the function.
+// TEST-3-NEXT:{{[0-9]+}}
+// TEST-3-NEXT:{{[0-9]+}}
+// TEST-3-NEXT:{{[0-9]+}}   int main(int argc, char **argv) {
+// TEST-3-NEXT

[Lldb-commits] [PATCH] D76697: [lldb] Replace debug-only assert in AppleObjCTypeEncodingParser::BuildObjCObjectPointerType with lldbassert

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a comment.

Let's summarize this.

1. In the old and the new version, the program is aborted with an `assert()` if 
the configuration is `LLDB_CONFIGURATION_DEBUG`.
2. In the old version, nothing happens, when the configuration is non-Debug.
3. In the new version, the assert in non-Debug configurations will be turned 
into a warning and *print a warning and encourage the user to file a bug 
report* (@JDevlieghere that is the noise you're referring to, right?)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76697/new/

https://reviews.llvm.org/D76697



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D75750: [lldb] integrate debuginfod

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk marked 3 inline comments as done.
kwk added a comment.

@labath I made a signficant simplification of starting and killing the server. 
I hope you like that better.




Comment at: lldb/source/Core/SourceManager.cpp:408
+  if ((!file_spec.GetDirectory() && file_spec.GetFilename()) ||
+  !FileSystem::Instance().Exists(m_file_spec)) {
 // If this is just a file name, lets see if we can find it in the

labath wrote:
> jankratochvil wrote:
> > I do not like this extra line as it changes behavior of LLDB unrelated to 
> > `debuginfod`. IIUC if the source file with fully specified 
> > directory+filename in DWARF does not exist but the same filename exists in 
> > a different directory of the sourcetree LLDB will now quietly use the 
> > different file. That's a bug.
> > I think it is there as you needed to initialize `sc.module_sp`.
> Yes, that does not sound right. It may be good to break this function into 
> smaller pieces so you can invoke the thing you need when you need it.
My intention wasn't to leave this as is to be honest. I had comments in here 
that I removed upon request but they existed to remind myself that I haven't 
double checked the logic well enough. I just wanted access to the symbol 
context further down below and thought, that I can take it from up here.



Comment at: lldb/source/Host/common/DebugInfoD.cpp:50
+   "invalid build ID: %s",
+   buildID.GetAsString("").c_str());
+

labath wrote:
> kwk wrote:
> > jankratochvil wrote:
> > > It should not be an error:
> > > ```
> > > echo 'int main(void) { return 0; }' >/tmp/main2.c;gcc -o /tmp/main2 
> > > /tmp/main2.c -Wall -g -Wl,--build-id=none;rm 
> > > /tmp/main2.c;DEBUGINFOD_URLS=http://localhost:8002/ ./bin/lldb /tmp/main2 
> > > -o 'l main' -o q
> > > (lldb) target create "/tmp/main2"
> > > Current executable set to '/tmp/main2' (x86_64).
> > > (lldb) l main
> > > warning: (x86_64) /tmp/main2 An error occurred while finding the source 
> > > file /tmp/main2.c using debuginfod for build ID A9C3D738: invalid build 
> > > ID: A9C3D738
> > > File: /tmp/main2.c
> > > (lldb) q
> > > ```
> > > 
> > Okay, I'll have it return just an empty string. And adjust the comment on 
> > the empty string in findSource documentation. I fully understand that an 
> > error is undesirable in your test case. My question is if the caller should 
> > sanitize it's parameters passed to `findSource` of if the latter should 
> > silently ignore those wrong UUIDs. For now I silently ignore them and treat 
> > a wrong build ID like a not found (e.g. empty string is returned).
> It would be nice to make a test case out of that.
I agree, a test would be nice but not at this stage, where the whole patch 
seems to be at danger.



Comment at: lldb/test/Shell/SymbolFile/DWARF/source-list.cpp:57
+// RUN: timeout 5 python3 -u -m http.server 0 --directory %t.mock --bind 
"localhost" &> %t.server.log & export PID=$!
+// RUN: trap 'kill $PID;' EXIT INT
+

@labath My bad. I interpreted `timeout 5` wrongly. It will kill the python 
server after `5 seconds` no matter what. If we increase this time to `timeout 
5m` it will kill the server after 5 minutes and we don't need the bash trap. 
Does that sound better? At least the only ugly part would be done this way.  
The whole section would look like this:

```lang=yaml
// RUN: rm -f "%t.server.log"
// RUN: timeout 5m python3 -u -m http.server 0 --directory %t.mock --bind 
"localhost" &> %t.server.log &
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75750/new/

https://reviews.llvm.org/D75750



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D74187: [lldb] Add method Language::IsMangledName

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk requested changes to this revision.
kwk added a comment.
This revision now requires changes to proceed.

I agree with @friss that there is a change in behavior. I'm not sure it is 
fixable with what I wrote but I think so.




Comment at: lldb/include/lldb/Target/Language.h:191
 
+  virtual bool IsMangledName(llvm::StringRef name) const = 0;
+

In `Mangled::GuessLanguage()` you call this function with `if 
(lang->IsMangledName(mangled.GetCString()))`. `mangled` in that case is a 
`ConstString`. I wonder why you don't pass that to `IsMangledName`? Is a 
`StringRef` faster? I'm asking because I see  `GetMethodNameVariants` below and 
that also must be called with a `ConstString`.



Comment at: lldb/source/Core/Mangled.cpp:416
-  return lldb::eLanguageTypeC_plus_plus;
-else if (ObjCLanguage::IsPossibleObjCMethodName(mangled_name))
-  return lldb::eLanguageTypeObjC;

In the `ObjCLanguage`'s implementation of `IsMangledName()` I hope to see  a 
call to `sPossibleObjCMethodName`. I think that is what @friss is aiming for 
with D74187#1865342. 



Comment at: lldb/source/Plugins/Language/ObjC/ObjCLanguage.h:97
+  bool IsMangledName(llvm::StringRef name) const override {
+return false;
+  }

Essentially, just `return ObjCLanguage::IsPossibleObjCMethodName(name)`, no? It 
is defined only a few lines further down below.


@friss is that what you mean in order to restore the original behavior?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74187/new/

https://reviews.llvm.org/D74187



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D76736: [LLDB] Fix parsing of IPv6 host:port inside brackets

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a comment.

LGTM but I haven't tested it.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76736/new/

https://reviews.llvm.org/D76736



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D75750: [lldb] integrate debuginfod

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 252510.
kwk marked 10 inline comments as done.
kwk added a comment.

- Add newline to end of FindDebuginfod.cmake
- Describe empty string returned from debuginfod::findSource()
- Don't treat build IDs of len <= 8 as an error but simply as not found
- move inexpensive debuginfod::isAvailable() check to beginning of if-stmt
- Simplify line number check in test file to avoid adjusting the line number 
every time the test changes
- Add newline to source-list.cpp test file


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75750/new/

https://reviews.llvm.org/D75750

Files:
  lldb/cmake/modules/FindDebuginfod.cmake
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/include/lldb/Host/Config.h.cmake
  lldb/include/lldb/Host/DebugInfoD.h
  lldb/source/Core/SourceManager.cpp
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/common/DebugInfoD.cpp
  lldb/test/CMakeLists.txt
  lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
  lldb/test/Shell/lit.cfg.py
  lldb/test/Shell/lit.site.cfg.py.in

Index: lldb/test/Shell/lit.site.cfg.py.in
===
--- lldb/test/Shell/lit.site.cfg.py.in
+++ lldb/test/Shell/lit.site.cfg.py.in
@@ -16,6 +16,7 @@
 config.python_executable = "@PYTHON_EXECUTABLE@"
 config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.lldb_enable_lzma = @LLDB_ENABLE_LZMA@
+config.lldb_enable_debuginfod = @LLDB_ENABLE_DEBUGINFOD@
 config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.lldb_bitness = 64 if @LLDB_IS_64_BITS@ else 32
 config.lldb_enable_python = @LLDB_ENABLE_PYTHON@
Index: lldb/test/Shell/lit.cfg.py
===
--- lldb/test/Shell/lit.cfg.py
+++ lldb/test/Shell/lit.cfg.py
@@ -117,6 +117,9 @@
 if config.lldb_enable_lzma:
 config.available_features.add('lzma')
 
+if config.lldb_enable_debuginfod:
+config.available_features.add('debuginfod')
+
 if find_executable('xz') != None:
 config.available_features.add('xz')
 
Index: lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
@@ -0,0 +1,135 @@
+// clang-format off
+// REQUIRES: debuginfod
+// UNSUPPORTED: darwin, windows
+
+//  Test that we can display the source of functions using debuginfod when the
+//  original source file is no longer present.
+//  
+//  The debuginfod client requires a buildid in the binary, so we compile one in.
+//  We can create the directory structure on disc that the client expects on a
+//  webserver that serves source files. Then we fire up a python based http
+//  server in the root of that mock directory, set the DEBUGINFOD_URLS
+//  environment variable and let LLDB do the rest. 
+//  
+//  Go here to find more about debuginfod:
+//  https://sourceware.org/elfutils/Debuginfod.html
+
+
+//We copy this file because we want to compile and later move it away
+
+// RUN: mkdir -p %t.buildroot
+// RUN: cp %s %t.buildroot/test.cpp
+
+//We use the prefix map in order to overwrite all DW_AT_decl_file paths
+//in the DWARF. We cd into the directory before compiling to get
+//DW_AT_comp_dir pickup %t.buildroot as well so it will be replaced by
+///my/new/path.
+
+// RUN: cd %t.buildroot
+// RUN: %clang_host \
+// RUN:   -g \
+// RUN:   -Wl,--build-id="0xaabbccdd" \
+// RUN:   -fdebug-prefix-map=%t.buildroot=/my/new/path \
+// RUN:   -o %t \
+// RUN:   %t.buildroot/test.cpp
+
+
+//We move the original source file to a directory that looks like a debuginfod
+//URL part.
+
+// RUN: rm -rf %t.mock
+// RUN: mkdir -p   %t.mock/buildid/aabbccdd/source/my/new/path
+// RUN: mv%t.buildroot/test.cpp %t.mock/buildid/aabbccdd/source/my/new/path
+
+
+//Adjust where debuginfod stores cache files:
+
+// RUN: rm -rf %t.debuginfod_cache_path
+// RUN: mkdir -p %t.debuginfod_cache_path
+// RUN: export DEBUGINFOD_CACHE_PATH=%t.debuginfod_cache_path
+
+
+//Start HTTP file server on port picked by OS and wait until it is ready
+//The server will be closed on exit of the test.
+
+// RUN: rm -f "%t.server.log"
+// RUN: timeout 5 python3 -u -m http.server 0 --directory %t.mock --bind "localhost" &> %t.server.log & export PID=$!
+// RUN: trap 'kill $PID;' EXIT INT
+
+
+//Extract HTTP address from the first line of the server log
+//(e.g. "Serving HTTP on 127.0.0.1 port 40587 (http://127.0.0.1:40587/) ..")
+
+// RUN: echo -n "Waiting for server to be ready"
+// RUN: SERVER_ADDRESS=""
+// RUN: while [ -z "$SERVER_ADDRESS" ]; do \
+// RUN: echo -n "."; \
+// RUN: sleep 0.01; \
+// RUN: SERVER_ADDRESS=$(head -n1 %t.server.log | grep "http://.\+/\+"; -o); \
+// RUN: done
+// RUN: echo "DONE"
+
+
+//-- TEST 1 --  No debuginfod awareness 
+
+
+// RUN: DEB

[Lldb-commits] [PATCH] D75750: [lldb] integrate debuginfod

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a comment.

@jankratochvil thanks for this thorough review. I have to think about one 
comment more precisely but the rest was fixed.




Comment at: lldb/source/Host/common/DebugInfoD.cpp:50
+   "invalid build ID: %s",
+   buildID.GetAsString("").c_str());
+

jankratochvil wrote:
> It should not be an error:
> ```
> echo 'int main(void) { return 0; }' >/tmp/main2.c;gcc -o /tmp/main2 
> /tmp/main2.c -Wall -g -Wl,--build-id=none;rm 
> /tmp/main2.c;DEBUGINFOD_URLS=http://localhost:8002/ ./bin/lldb /tmp/main2 -o 
> 'l main' -o q
> (lldb) target create "/tmp/main2"
> Current executable set to '/tmp/main2' (x86_64).
> (lldb) l main
> warning: (x86_64) /tmp/main2 An error occurred while finding the source file 
> /tmp/main2.c using debuginfod for build ID A9C3D738: invalid build ID: 
> A9C3D738
> File: /tmp/main2.c
> (lldb) q
> ```
> 
Okay, I'll have it return just an empty string. And adjust the comment on the 
empty string in findSource documentation. I fully understand that an error is 
undesirable in your test case. My question is if the caller should sanitize 
it's parameters passed to `findSource` of if the latter should silently ignore 
those wrong UUIDs. For now I silently ignore them and treat a wrong build ID 
like a not found (e.g. empty string is returned).



Comment at: lldb/test/Shell/SymbolFile/DWARF/source-list.cpp:103
+// TEST-3: File: /my/new/path/test.cpp
+// TEST-3: 123
+// TEST-3-NEXT:{{[0-9]+}}   // Some context lines before

jankratochvil wrote:
> `s/123/{{[0-9]+}}/?`
Both are fine, but I'll go with your's if that helps. If you can tell me how to 
get a lit `CHECK` statement that checks for incremental numbers, that'll be 
awesome ;)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75750/new/

https://reviews.llvm.org/D75750



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D75750: [lldb] integrate debuginfod

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk marked 10 inline comments as done.
kwk added a comment.

@labath @jankratochvil @fche2 I've addressed all your comments and hope the 
patch is good to go now.




Comment at: lldb/source/Host/common/DebugInfoD.cpp:43
+  buildID.GetBytes().size() ==
+  sizeof(llvm::support::ulittle32_t)) // .gnu_debuglink crc32
+return llvm::createStringError(llvm::inconvertibleErrorCode(),

labath wrote:
> jankratochvil wrote:
> > If it is done this way (and not in `libdebuginfod.so`) I think there should 
> > be `<=8` because LLDB contains:
> > ```
> > if (gnu_debuglink_crc) {
> >   // Use 4 bytes of crc from the .gnu_debuglink section.
> >   u32le data(gnu_debuglink_crc);
> >   uuid = UUID::fromData(&data, sizeof(data));
> > } else if (core_notes_crc) {
> >   // Use 8 bytes - first 4 bytes for *magic* prefix, mainly to 
> > make
> >   // it look different form .gnu_debuglink crc followed by 4 
> > bytes
> >   // of note segments crc.
> >   u32le data[] = {u32le(g_core_uuid_magic), 
> > u32le(core_notes_crc)};
> >   uuid = UUID::fromData(data, sizeof(data));
> > }
> > ```
> > 
> 4 would have probably been fine too, as I don't think a core file "uuid" can 
> make its way into here. In either case, we should document what is this 
> working around, as 4 or 8 byte uuids are technically valid.
@labath. I've added a documentation for the workaround.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75750/new/

https://reviews.llvm.org/D75750



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D75750: WIP: [lldb] integrate debuginfod

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 252353.
kwk added a comment.

- Add documentation for workaround on rejecting special build UUIDs


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75750/new/

https://reviews.llvm.org/D75750

Files:
  lldb/cmake/modules/FindDebuginfod.cmake
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/include/lldb/Host/Config.h.cmake
  lldb/include/lldb/Host/DebugInfoD.h
  lldb/source/Core/SourceManager.cpp
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/common/DebugInfoD.cpp
  lldb/test/CMakeLists.txt
  lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
  lldb/test/Shell/lit.cfg.py
  lldb/test/Shell/lit.site.cfg.py.in

Index: lldb/test/Shell/lit.site.cfg.py.in
===
--- lldb/test/Shell/lit.site.cfg.py.in
+++ lldb/test/Shell/lit.site.cfg.py.in
@@ -16,6 +16,7 @@
 config.python_executable = "@PYTHON_EXECUTABLE@"
 config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.lldb_enable_lzma = @LLDB_ENABLE_LZMA@
+config.lldb_enable_debuginfod = @LLDB_ENABLE_DEBUGINFOD@
 config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.lldb_bitness = 64 if @LLDB_IS_64_BITS@ else 32
 config.lldb_enable_python = @LLDB_ENABLE_PYTHON@
Index: lldb/test/Shell/lit.cfg.py
===
--- lldb/test/Shell/lit.cfg.py
+++ lldb/test/Shell/lit.cfg.py
@@ -117,6 +117,9 @@
 if config.lldb_enable_lzma:
 config.available_features.add('lzma')
 
+if config.lldb_enable_debuginfod:
+config.available_features.add('debuginfod')
+
 if find_executable('xz') != None:
 config.available_features.add('xz')
 
Index: lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
@@ -0,0 +1,135 @@
+// clang-format off
+// REQUIRES: debuginfod
+// UNSUPPORTED: darwin, windows
+
+//  Test that we can display the source of functions using debuginfod when the
+//  original source file is no longer present.
+//  
+//  The debuginfod client requires a buildid in the binary, so we compile one in.
+//  We can create the directory structure on disc that the client expects on a
+//  webserver that serves source files. Then we fire up a python based http
+//  server in the root of that mock directory, set the DEBUGINFOD_URLS
+//  environment variable and let LLDB do the rest. 
+//  
+//  Go here to find more about debuginfod:
+//  https://sourceware.org/elfutils/Debuginfod.html
+
+
+//We copy this file because we want to compile and later move it away
+
+// RUN: mkdir -p %t.buildroot
+// RUN: cp %s %t.buildroot/test.cpp
+
+//We use the prefix map in order to overwrite all DW_AT_decl_file paths
+//in the DWARF. We cd into the directory before compiling to get
+//DW_AT_comp_dir pickup %t.buildroot as well so it will be replaced by
+///my/new/path.
+
+// RUN: cd %t.buildroot
+// RUN: %clang_host \
+// RUN:   -g \
+// RUN:   -Wl,--build-id="0xaabbccdd" \
+// RUN:   -fdebug-prefix-map=%t.buildroot=/my/new/path \
+// RUN:   -o %t \
+// RUN:   %t.buildroot/test.cpp
+
+
+//We move the original source file to a directory that looks like a debuginfod
+//URL part.
+
+// RUN: rm -rf %t.mock
+// RUN: mkdir -p   %t.mock/buildid/aabbccdd/source/my/new/path
+// RUN: mv%t.buildroot/test.cpp %t.mock/buildid/aabbccdd/source/my/new/path
+
+
+//Adjust where debuginfod stores cache files:
+
+// RUN: rm -rf %t.debuginfod_cache_path
+// RUN: mkdir -p %t.debuginfod_cache_path
+// RUN: export DEBUGINFOD_CACHE_PATH=%t.debuginfod_cache_path
+
+
+//Start HTTP file server on port picked by OS and wait until it is ready
+//The server will be closed on exit of the test.
+
+// RUN: rm -f "%t.server.log"
+// RUN: timeout 5 python3 -u -m http.server 0 --directory %t.mock --bind "localhost" &> %t.server.log & export PID=$!
+// RUN: trap 'kill $PID;' EXIT INT
+
+
+//Extract HTTP address from the first line of the server log
+//(e.g. "Serving HTTP on 127.0.0.1 port 40587 (http://127.0.0.1:40587/) ..")
+
+// RUN: echo -n "Waiting for server to be ready"
+// RUN: SERVER_ADDRESS=""
+// RUN: while [ -z "$SERVER_ADDRESS" ]; do \
+// RUN: echo -n "."; \
+// RUN: sleep 0.01; \
+// RUN: SERVER_ADDRESS=$(head -n1 %t.server.log | grep "http://.\+/\+"; -o); \
+// RUN: done
+// RUN: echo "DONE"
+
+
+//-- TEST 1 --  No debuginfod awareness 
+
+
+// RUN: DEBUGINFOD_URLS="" \
+// RUN: %lldb -f %t -o 'source list -n main' | FileCheck --dump-input=fail %s --check-prefix=TEST-1
+
+// TEST-1: (lldb) source list -n main
+// TEST-1: File: /my/new/path/test.cpp
+// TEST-1-EMPTY:
+
+
+//-- TEST 2 -- debuginfod URL pointing in wrong place --
+
+
+// RUN: DEBUGINFOD_URLS="http://example.com/debuginfod"; \
+// RUN

[Lldb-commits] [PATCH] D75750: WIP: [lldb] integrate debuginfod

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 252059.
kwk added a comment.

- Adjust buildID verification


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75750/new/

https://reviews.llvm.org/D75750

Files:
  lldb/cmake/modules/FindDebuginfod.cmake
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/include/lldb/Host/Config.h.cmake
  lldb/include/lldb/Host/DebugInfoD.h
  lldb/source/Core/SourceManager.cpp
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/common/DebugInfoD.cpp
  lldb/test/CMakeLists.txt
  lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
  lldb/test/Shell/lit.cfg.py
  lldb/test/Shell/lit.site.cfg.py.in

Index: lldb/test/Shell/lit.site.cfg.py.in
===
--- lldb/test/Shell/lit.site.cfg.py.in
+++ lldb/test/Shell/lit.site.cfg.py.in
@@ -16,6 +16,7 @@
 config.python_executable = "@PYTHON_EXECUTABLE@"
 config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.lldb_enable_lzma = @LLDB_ENABLE_LZMA@
+config.lldb_enable_debuginfod = @LLDB_ENABLE_DEBUGINFOD@
 config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.lldb_bitness = 64 if @LLDB_IS_64_BITS@ else 32
 config.lldb_enable_python = @LLDB_ENABLE_PYTHON@
Index: lldb/test/Shell/lit.cfg.py
===
--- lldb/test/Shell/lit.cfg.py
+++ lldb/test/Shell/lit.cfg.py
@@ -117,6 +117,9 @@
 if config.lldb_enable_lzma:
 config.available_features.add('lzma')
 
+if config.lldb_enable_debuginfod:
+config.available_features.add('debuginfod')
+
 if find_executable('xz') != None:
 config.available_features.add('xz')
 
Index: lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
@@ -0,0 +1,135 @@
+// clang-format off
+// REQUIRES: debuginfod
+// UNSUPPORTED: darwin, windows
+
+//  Test that we can display the source of functions using debuginfod when the
+//  original source file is no longer present.
+//  
+//  The debuginfod client requires a buildid in the binary, so we compile one in.
+//  We can create the directory structure on disc that the client expects on a
+//  webserver that serves source files. Then we fire up a python based http
+//  server in the root of that mock directory, set the DEBUGINFOD_URLS
+//  environment variable and let LLDB do the rest. 
+//  
+//  Go here to find more about debuginfod:
+//  https://sourceware.org/elfutils/Debuginfod.html
+
+
+//We copy this file because we want to compile and later move it away
+
+// RUN: mkdir -p %t.buildroot
+// RUN: cp %s %t.buildroot/test.cpp
+
+//We use the prefix map in order to overwrite all DW_AT_decl_file paths
+//in the DWARF. We cd into the directory before compiling to get
+//DW_AT_comp_dir pickup %t.buildroot as well so it will be replaced by
+///my/new/path.
+
+// RUN: cd %t.buildroot
+// RUN: %clang_host \
+// RUN:   -g \
+// RUN:   -Wl,--build-id="0xaabbccdd" \
+// RUN:   -fdebug-prefix-map=%t.buildroot=/my/new/path \
+// RUN:   -o %t \
+// RUN:   %t.buildroot/test.cpp
+
+
+//We move the original source file to a directory that looks like a debuginfod
+//URL part.
+
+// RUN: rm -rf %t.mock
+// RUN: mkdir -p   %t.mock/buildid/aabbccdd/source/my/new/path
+// RUN: mv%t.buildroot/test.cpp %t.mock/buildid/aabbccdd/source/my/new/path
+
+
+//Adjust where debuginfod stores cache files:
+
+// RUN: rm -rf %t.debuginfod_cache_path
+// RUN: mkdir -p %t.debuginfod_cache_path
+// RUN: export DEBUGINFOD_CACHE_PATH=%t.debuginfod_cache_path
+
+
+//Start HTTP file server on port picked by OS and wait until it is ready
+//The server will be closed on exit of the test.
+
+// RUN: rm -f "%t.server.log"
+// RUN: timeout 5 python3 -u -m http.server 0 --directory %t.mock --bind "localhost" &> %t.server.log & export PID=$!
+// RUN: trap 'kill $PID;' EXIT INT
+
+
+//Extract HTTP address from the first line of the server log
+//(e.g. "Serving HTTP on 127.0.0.1 port 40587 (http://127.0.0.1:40587/) ..")
+
+// RUN: echo -n "Waiting for server to be ready"
+// RUN: SERVER_ADDRESS=""
+// RUN: while [ -z "$SERVER_ADDRESS" ]; do \
+// RUN: echo -n "."; \
+// RUN: sleep 0.01; \
+// RUN: SERVER_ADDRESS=$(head -n1 %t.server.log | grep "http://.\+/\+"; -o); \
+// RUN: done
+// RUN: echo "DONE"
+
+
+//-- TEST 1 --  No debuginfod awareness 
+
+
+// RUN: DEBUGINFOD_URLS="" \
+// RUN: %lldb -f %t -o 'source list -n main' | FileCheck --dump-input=fail %s --check-prefix=TEST-1
+
+// TEST-1: (lldb) source list -n main
+// TEST-1: File: /my/new/path/test.cpp
+// TEST-1-EMPTY:
+
+
+//-- TEST 2 -- debuginfod URL pointing in wrong place --
+
+
+// RUN: DEBUGINFOD_URLS="http://example.com/debuginfod"; \
+// RUN: %lldb -f %t -o 'source list -n main'

[Lldb-commits] [PATCH] D75750: WIP: [lldb] integrate debuginfod

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 252054.
kwk marked 2 inline comments as done.
kwk added a comment.

- Remove commented out code
- Remove lldb/packages/Python/lldbsuite/test/httpserver.py in favor of lit test
- Removed commented out left-over code


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75750/new/

https://reviews.llvm.org/D75750

Files:
  lldb/cmake/modules/FindDebuginfod.cmake
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/include/lldb/Host/Config.h.cmake
  lldb/include/lldb/Host/DebugInfoD.h
  lldb/source/Core/SourceManager.cpp
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/common/DebugInfoD.cpp
  lldb/test/CMakeLists.txt
  lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
  lldb/test/Shell/lit.cfg.py
  lldb/test/Shell/lit.site.cfg.py.in

Index: lldb/test/Shell/lit.site.cfg.py.in
===
--- lldb/test/Shell/lit.site.cfg.py.in
+++ lldb/test/Shell/lit.site.cfg.py.in
@@ -16,6 +16,7 @@
 config.python_executable = "@PYTHON_EXECUTABLE@"
 config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.lldb_enable_lzma = @LLDB_ENABLE_LZMA@
+config.lldb_enable_debuginfod = @LLDB_ENABLE_DEBUGINFOD@
 config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.lldb_bitness = 64 if @LLDB_IS_64_BITS@ else 32
 config.lldb_enable_python = @LLDB_ENABLE_PYTHON@
Index: lldb/test/Shell/lit.cfg.py
===
--- lldb/test/Shell/lit.cfg.py
+++ lldb/test/Shell/lit.cfg.py
@@ -117,6 +117,9 @@
 if config.lldb_enable_lzma:
 config.available_features.add('lzma')
 
+if config.lldb_enable_debuginfod:
+config.available_features.add('debuginfod')
+
 if find_executable('xz') != None:
 config.available_features.add('xz')
 
Index: lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
@@ -0,0 +1,135 @@
+// clang-format off
+// REQUIRES: debuginfod
+// UNSUPPORTED: darwin, windows
+
+//  Test that we can display the source of functions using debuginfod when the
+//  original source file is no longer present.
+//  
+//  The debuginfod client requires a buildid in the binary, so we compile one in.
+//  We can create the directory structure on disc that the client expects on a
+//  webserver that serves source files. Then we fire up a python based http
+//  server in the root of that mock directory, set the DEBUGINFOD_URLS
+//  environment variable and let LLDB do the rest. 
+//  
+//  Go here to find more about debuginfod:
+//  https://sourceware.org/elfutils/Debuginfod.html
+
+
+//We copy this file because we want to compile and later move it away
+
+// RUN: mkdir -p %t.buildroot
+// RUN: cp %s %t.buildroot/test.cpp
+
+//We use the prefix map in order to overwrite all DW_AT_decl_file paths
+//in the DWARF. We cd into the directory before compiling to get
+//DW_AT_comp_dir pickup %t.buildroot as well so it will be replaced by
+///my/new/path.
+
+// RUN: cd %t.buildroot
+// RUN: %clang_host \
+// RUN:   -g \
+// RUN:   -Wl,--build-id="0xaabbccdd" \
+// RUN:   -fdebug-prefix-map=%t.buildroot=/my/new/path \
+// RUN:   -o %t \
+// RUN:   %t.buildroot/test.cpp
+
+
+//We move the original source file to a directory that looks like a debuginfod
+//URL part.
+
+// RUN: rm -rf %t.mock
+// RUN: mkdir -p   %t.mock/buildid/aabbccdd/source/my/new/path
+// RUN: mv%t.buildroot/test.cpp %t.mock/buildid/aabbccdd/source/my/new/path
+
+
+//Adjust where debuginfod stores cache files:
+
+// RUN: rm -rf %t.debuginfod_cache_path
+// RUN: mkdir -p %t.debuginfod_cache_path
+// RUN: export DEBUGINFOD_CACHE_PATH=%t.debuginfod_cache_path
+
+
+//Start HTTP file server on port picked by OS and wait until it is ready
+//The server will be closed on exit of the test.
+
+// RUN: rm -f "%t.server.log"
+// RUN: timeout 5 python3 -u -m http.server 0 --directory %t.mock --bind "localhost" &> %t.server.log & export PID=$!
+// RUN: trap 'kill $PID;' EXIT INT
+
+
+//Extract HTTP address from the first line of the server log
+//(e.g. "Serving HTTP on 127.0.0.1 port 40587 (http://127.0.0.1:40587/) ..")
+
+// RUN: echo -n "Waiting for server to be ready"
+// RUN: SERVER_ADDRESS=""
+// RUN: while [ -z "$SERVER_ADDRESS" ]; do \
+// RUN: echo -n "."; \
+// RUN: sleep 0.01; \
+// RUN: SERVER_ADDRESS=$(head -n1 %t.server.log | grep "http://.\+/\+"; -o); \
+// RUN: done
+// RUN: echo "DONE"
+
+
+//-- TEST 1 --  No debuginfod awareness 
+
+
+// RUN: DEBUGINFOD_URLS="" \
+// RUN: %lldb -f %t -o 'source list -n main' | FileCheck --dump-input=fail %s --check-prefix=TEST-1
+
+// TEST-1: (lldb) source list -n main
+// TEST-1: File: /my/new/path/test.cpp
+// TEST-1-EMPTY:
+
+
+//-- TEST 2 -- debuginfod URL point

[Lldb-commits] [PATCH] D75750: WIP: [lldb] integrate debuginfod

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a comment.

@labath @fche2 @jankratochvil I've implemented the logic to ignore invalid 
UUIDs and the ones that are too short. Can you have another look please?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75750/new/

https://reviews.llvm.org/D75750



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D75750: WIP: [lldb] integrate debuginfod

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 252026.
kwk added a comment.

- check for valid UUID
- less verbose mkdir and rm output
- More explicit test and documentation
- fixup


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75750/new/

https://reviews.llvm.org/D75750

Files:
  lldb/cmake/modules/FindDebuginfod.cmake
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/include/lldb/Host/Config.h.cmake
  lldb/include/lldb/Host/DebugInfoD.h
  lldb/packages/Python/lldbsuite/test/httpserver.py
  lldb/source/Core/SourceManager.cpp
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/common/DebugInfoD.cpp
  lldb/test/CMakeLists.txt
  lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
  lldb/test/Shell/lit.cfg.py
  lldb/test/Shell/lit.site.cfg.py.in

Index: lldb/test/Shell/lit.site.cfg.py.in
===
--- lldb/test/Shell/lit.site.cfg.py.in
+++ lldb/test/Shell/lit.site.cfg.py.in
@@ -16,6 +16,7 @@
 config.python_executable = "@PYTHON_EXECUTABLE@"
 config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.lldb_enable_lzma = @LLDB_ENABLE_LZMA@
+config.lldb_enable_debuginfod = @LLDB_ENABLE_DEBUGINFOD@
 config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.lldb_bitness = 64 if @LLDB_IS_64_BITS@ else 32
 config.lldb_enable_python = @LLDB_ENABLE_PYTHON@
Index: lldb/test/Shell/lit.cfg.py
===
--- lldb/test/Shell/lit.cfg.py
+++ lldb/test/Shell/lit.cfg.py
@@ -117,6 +117,9 @@
 if config.lldb_enable_lzma:
 config.available_features.add('lzma')
 
+if config.lldb_enable_debuginfod:
+config.available_features.add('debuginfod')
+
 if find_executable('xz') != None:
 config.available_features.add('xz')
 
Index: lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
@@ -0,0 +1,135 @@
+// clang-format off
+// REQUIRES: debuginfod
+// UNSUPPORTED: darwin, windows
+
+//  Test that we can display the source of functions using debuginfod when the
+//  original source file is no longer present.
+//  
+//  The debuginfod client requires a buildid in the binary, so we compile one in.
+//  We can create the directory structure on disc that the client expects on a
+//  webserver that serves source files. Then we fire up a python based http
+//  server in the root of that mock directory, set the DEBUGINFOD_URLS
+//  environment variable and let LLDB do the rest. 
+//  
+//  Go here to find more about debuginfod:
+//  https://sourceware.org/elfutils/Debuginfod.html
+
+
+//We copy this file because we want to compile and later move it away
+
+// RUN: mkdir -p %t.buildroot
+// RUN: cp %s %t.buildroot/test.cpp
+
+//We use the prefix map in order to overwrite all DW_AT_decl_file paths
+//in the DWARF. We cd into the directory before compiling to get
+//DW_AT_comp_dir pickup %t.buildroot as well so it will be replaced by
+///my/new/path.
+
+// RUN: cd %t.buildroot
+// RUN: %clang_host \
+// RUN:   -g \
+// RUN:   -Wl,--build-id="0xaabbccdd" \
+// RUN:   -fdebug-prefix-map=%t.buildroot=/my/new/path \
+// RUN:   -o %t \
+// RUN:   %t.buildroot/test.cpp
+
+
+//We move the original source file to a directory that looks like a debuginfod
+//URL part.
+
+// RUN: rm -rf %t.mock
+// RUN: mkdir -p   %t.mock/buildid/aabbccdd/source/my/new/path
+// RUN: mv%t.buildroot/test.cpp %t.mock/buildid/aabbccdd/source/my/new/path
+
+
+//Adjust where debuginfod stores cache files:
+
+// RUN: rm -rf %t.debuginfod_cache_path
+// RUN: mkdir -p %t.debuginfod_cache_path
+// RUN: export DEBUGINFOD_CACHE_PATH=%t.debuginfod_cache_path
+
+
+//Start HTTP file server on port picked by OS and wait until it is ready
+//The server will be closed on exit of the test.
+
+// RUN: rm -f "%t.server.log"
+// RUN: timeout 5 python3 -u -m http.server 0 --directory %t.mock --bind "localhost" &> %t.server.log & export PID=$!
+// RUN: trap 'kill $PID;' EXIT INT
+
+
+//Extract HTTP address from the first line of the server log
+//(e.g. "Serving HTTP on 127.0.0.1 port 40587 (http://127.0.0.1:40587/) ..")
+
+// RUN: echo -n "Waiting for server to be ready"
+// RUN: SERVER_ADDRESS=""
+// RUN: while [ -z "$SERVER_ADDRESS" ]; do \
+// RUN: echo -n "."; \
+// RUN: sleep 0.01; \
+// RUN: SERVER_ADDRESS=$(head -n1 %t.server.log | grep "http://.\+/\+"; -o); \
+// RUN: done
+// RUN: echo "DONE"
+
+
+//-- TEST 1 --  No debuginfod awareness 
+
+
+// RUN: DEBUGINFOD_URLS="" \
+// RUN: %lldb -f %t -o 'source list -n main' | FileCheck --dump-input=fail %s --check-prefix=TEST-1
+
+// TEST-1: (lldb) source list -n main
+// TEST-1: File: /my/new/path/test.cpp
+// TEST-1-EMPTY:
+
+
+//-- TEST 2 -- debuginfod URL pointing in wrong place -

[Lldb-commits] [PATCH] D75750: WIP: [lldb] integrate debuginfod

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 251383.
kwk added a comment.

- Validate that the server received the request from debuginfod client


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75750/new/

https://reviews.llvm.org/D75750

Files:
  lldb/cmake/modules/FindDebuginfod.cmake
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/include/lldb/Host/Config.h.cmake
  lldb/include/lldb/Host/DebugInfoD.h
  lldb/packages/Python/lldbsuite/test/httpserver.py
  lldb/source/Core/SourceManager.cpp
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/common/DebugInfoD.cpp
  lldb/test/CMakeLists.txt
  lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
  lldb/test/Shell/lit.cfg.py
  lldb/test/Shell/lit.site.cfg.py.in

Index: lldb/test/Shell/lit.site.cfg.py.in
===
--- lldb/test/Shell/lit.site.cfg.py.in
+++ lldb/test/Shell/lit.site.cfg.py.in
@@ -16,6 +16,7 @@
 config.python_executable = "@PYTHON_EXECUTABLE@"
 config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.lldb_enable_lzma = @LLDB_ENABLE_LZMA@
+config.lldb_enable_debuginfod = @LLDB_ENABLE_DEBUGINFOD@
 config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.lldb_bitness = 64 if @LLDB_IS_64_BITS@ else 32
 config.lldb_enable_python = @LLDB_ENABLE_PYTHON@
Index: lldb/test/Shell/lit.cfg.py
===
--- lldb/test/Shell/lit.cfg.py
+++ lldb/test/Shell/lit.cfg.py
@@ -117,6 +117,9 @@
 if config.lldb_enable_lzma:
 config.available_features.add('lzma')
 
+if config.lldb_enable_debuginfod:
+config.available_features.add('debuginfod')
+
 if find_executable('xz') != None:
 config.available_features.add('xz')
 
Index: lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
@@ -0,0 +1,134 @@
+// clang-format off
+// REQUIRES: debuginfod
+// UNSUPPORTED: darwin, windows
+
+//  Test that we can display the source of functions using debuginfod when the
+//  original source file is no longer present.
+//  
+//  The debuginfod client requires a buildid in the binary, so we compile one in.
+//  We can create the directory structure on disc that the client expects on a
+//  webserver that serves source files. Then we fire up a python based http
+//  server in the root of that mock directory, set the DEBUGINFOD_URLS
+//  environment variable and let LLDB do the rest. 
+//  
+//  Go here to find more about debuginfod:
+//  https://sourceware.org/elfutils/Debuginfod.html
+
+
+//We copy this file because we want to compile and later move it away
+
+// RUN: mkdir -p %t.buildroot
+// RUN: cp %s %t.buildroot/test.cpp
+
+
+//We cd into the directory before compiling to get DW_AT_comp_dir pickup
+//%t.buildroot as well so it will be replaced by /my/new/path.
+
+// RUN: cd %t.buildroot
+// RUN: %clang_host \
+// RUN:   -g \
+// RUN:   -Wl,--build-id="0xaabbccdd" \
+// RUN:   -fdebug-prefix-map=%t.buildroot=/my/new/path \
+// RUN:   -o %t \
+// RUN:   test.cpp
+
+
+//We move the original source file to a directory that looks like a debuginfod
+//URL part.
+
+// RUN: rm -rf %t.mock
+// RUN: mkdir -p   %t.mock/buildid/aabbccdd/source/my/new/path
+// RUN: mvtest.cpp %t.mock/buildid/aabbccdd/source/my/new/path
+
+
+//Adjust where debuginfod stores cache files:
+
+// RUN: rm -rfv %t.debuginfod_cache_path
+// RUN: mkdir -pv %t.debuginfod_cache_path
+// RUN: export DEBUGINFOD_CACHE_PATH=%t.debuginfod_cache_path
+
+
+//Start HTTP file server on port picked by OS and wait until it is ready
+//The server will be closed on exit of the test.
+
+// RUN: rm -fv "%t.server.log"
+// RUN: timeout 5 python3 -u -m http.server 0 --directory %t.mock --bind "localhost" &> %t.server.log & export PID=$!
+// RUN: trap 'kill $PID;' EXIT INT
+
+
+//Extract HTTP address from the first line of the server log
+//(e.g. "Serving HTTP on 127.0.0.1 port 40587 (http://127.0.0.1:40587/) ..")
+
+// RUN: echo -n "Waiting for server to be ready"
+// RUN: SERVER_ADDRESS=""
+// RUN: while [ -z "$SERVER_ADDRESS" ]; do \
+// RUN: echo -n "."; \
+// RUN: sleep 0.01; \
+// RUN: SERVER_ADDRESS=$(head -n1 %t.server.log | grep "http://.\+/\+"; -o); \
+// RUN: done
+// RUN: echo "DONE"
+
+
+//-- TEST 1 --  No debuginfod awareness 
+
+
+// RUN: DEBUGINFOD_URLS="" \
+// RUN: %lldb -f %t -o 'source list -n main' | FileCheck --dump-input=fail %s --check-prefix=TEST-1
+
+// TEST-1: (lldb) source list -n main
+// TEST-1: File: /my/new/path/test.cpp
+// TEST-1-EMPTY:
+
+
+//-- TEST 2 -- debuginfod URL pointing in wrong place --
+
+
+// RUN: DEBUGINFOD_URLS="http://example.com/debuginfod"; \
+// RUN: %lldb -f %t -o 'source list -n main' | FileCheck --dump-input=

[Lldb-commits] [PATCH] D75750: WIP: [lldb] integrate debuginfod

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 251376.
kwk marked 6 inline comments as done.
kwk added a comment.

- Removed not needed forward decl
- Format comments for better readability in my test


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75750/new/

https://reviews.llvm.org/D75750

Files:
  lldb/cmake/modules/FindDebuginfod.cmake
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/include/lldb/Host/Config.h.cmake
  lldb/include/lldb/Host/DebugInfoD.h
  lldb/packages/Python/lldbsuite/test/httpserver.py
  lldb/source/Core/SourceManager.cpp
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/common/DebugInfoD.cpp
  lldb/test/CMakeLists.txt
  lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
  lldb/test/Shell/lit.cfg.py
  lldb/test/Shell/lit.site.cfg.py.in

Index: lldb/test/Shell/lit.site.cfg.py.in
===
--- lldb/test/Shell/lit.site.cfg.py.in
+++ lldb/test/Shell/lit.site.cfg.py.in
@@ -16,6 +16,7 @@
 config.python_executable = "@PYTHON_EXECUTABLE@"
 config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.lldb_enable_lzma = @LLDB_ENABLE_LZMA@
+config.lldb_enable_debuginfod = @LLDB_ENABLE_DEBUGINFOD@
 config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.lldb_bitness = 64 if @LLDB_IS_64_BITS@ else 32
 config.lldb_enable_python = @LLDB_ENABLE_PYTHON@
Index: lldb/test/Shell/lit.cfg.py
===
--- lldb/test/Shell/lit.cfg.py
+++ lldb/test/Shell/lit.cfg.py
@@ -117,6 +117,9 @@
 if config.lldb_enable_lzma:
 config.available_features.add('lzma')
 
+if config.lldb_enable_debuginfod:
+config.available_features.add('debuginfod')
+
 if find_executable('xz') != None:
 config.available_features.add('xz')
 
Index: lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
@@ -0,0 +1,128 @@
+// clang-format off
+// REQUIRES: debuginfod
+// UNSUPPORTED: darwin, windows
+
+//  Test that we can display the source of functions using debuginfod when the
+//  original source file is no longer present.
+//  
+//  The debuginfod client requires a buildid in the binary, so we compile one in.
+//  We can create the directory structure on disc that the client expects on a
+//  webserver that serves source files. Then we fire up a python based http
+//  server in the root of that mock directory, set the DEBUGINFOD_URLS
+//  environment variable and let LLDB do the rest. 
+//  
+//  Go here to find more about debuginfod:
+//  https://sourceware.org/elfutils/Debuginfod.html
+
+
+//We copy this file because we want to compile and later move it away
+
+// RUN: mkdir -p %t.buildroot
+// RUN: cp %s %t.buildroot/test.cpp
+
+
+//We cd into the directory before compiling to get DW_AT_comp_dir pickup
+//%t.buildroot as well so it will be replaced by /my/new/path.
+
+// RUN: cd %t.buildroot
+// RUN: %clang_host \
+// RUN:   -g \
+// RUN:   -Wl,--build-id="0xaabbccdd" \
+// RUN:   -fdebug-prefix-map=%t.buildroot=/my/new/path \
+// RUN:   -o %t \
+// RUN:   test.cpp
+
+
+//We move the original source file to a directory that looks like a debuginfod
+//URL part.
+
+// RUN: rm -rf %t.mock
+// RUN: mkdir -p   %t.mock/buildid/aabbccdd/source/my/new/path
+// RUN: mvtest.cpp %t.mock/buildid/aabbccdd/source/my/new/path
+
+
+//Adjust where debuginfod stores cache files:
+
+// RUN: rm -rfv %t.debuginfod_cache_path
+// RUN: mkdir -pv %t.debuginfod_cache_path
+// RUN: export DEBUGINFOD_CACHE_PATH=%t.debuginfod_cache_path
+
+
+//Start HTTP file server on port picked by OS and wait until it is ready
+//The server will be closed on exit of the test.
+
+// RUN: rm -fv "%t.server.log"
+// RUN: timeout 5 python3 -u -m http.server 0 --directory %t.mock --bind "localhost" &> %t.server.log & export PID=$!
+// RUN: trap 'echo "SERVER LOG:"; cat %t.server.log; kill $PID;' EXIT INT
+
+
+//Extract HTTP address from the first line of the server log
+//(e.g. "Serving HTTP on 127.0.0.1 port 40587 (http://127.0.0.1:40587/) ..")
+
+// RUN: echo -n "Waiting for server to be ready"
+// RUN: SERVER_ADDRESS=""
+// RUN: while [ -z "$SERVER_ADDRESS" ]; do \
+// RUN: echo -n "."; \
+// RUN: sleep 0.01; \
+// RUN: SERVER_ADDRESS=$(head -n1 %t.server.log | grep "http://.\+/\+"; -o); \
+// RUN: done
+// RUN: echo "DONE"
+
+
+//-- TEST 1 --  No debuginfod awareness 
+
+
+// RUN: DEBUGINFOD_URLS="" \
+// RUN: %lldb -f %t -o 'source list -n main' | FileCheck --dump-input=fail %s --check-prefix=TEST-1
+
+// TEST-1: (lldb) source list -n main
+// TEST-1: File: /my/new/path/test.cpp
+// TEST-1-EMPTY:
+
+
+//-- TEST 2 -- debuginfod URL pointing in wrong place --
+
+
+// RUN: DEBUGINFOD_URLS="http://exampl

[Lldb-commits] [PATCH] D75750: WIP: [lldb] integrate debuginfod

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a comment.

@labath I've improved my test file for readability.




Comment at: lldb/include/lldb/Host/DebugInfoD.h:26
+
+llvm::Error findSource(UUID buildID, const std::string &path,
+   std::string &result_path);

labath wrote:
> Expected ?
Removed.



Comment at: lldb/include/lldb/Host/DebugInfoD.h:16-18
+namespace llvm {
+class Error;
+} // End of namespace llvm

labath wrote:
> I guess this is not needed now.
Right.



Comment at: lldb/packages/Python/lldbsuite/test/httpserver.py:75
+# Block only for 0.5 seconds max
+httpd.timeout = 0.5
+# Allow for reusing the address

labath wrote:
> What exactly is this timeout for? It seems rather small...
uff, I guess I had an idea when I wrote it but its lost now.  



Comment at: lldb/packages/Python/lldbsuite/test/lldbtest.py:4
 
-The concrete subclass can override lldbtest.TesBase in order to inherit the
+The concrete subclass can override lldbtest.TestBase in order to inherit the
 common behavior for unitest.TestCase.setUp/tearDown implemented in this file.

labath wrote:
> just commit this separately.  no review needed.
Done in 44361782e2c252c8886cd77f6b7d4ebe64fb6e8d.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75750/new/

https://reviews.llvm.org/D75750



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D75750: WIP: [lldb] integrate debuginfod

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 251297.
kwk added a comment.

- Fix NameError: name 'TRUE' is not defined


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75750/new/

https://reviews.llvm.org/D75750

Files:
  lldb/cmake/modules/FindDebuginfod.cmake
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/include/lldb/Host/Config.h.cmake
  lldb/include/lldb/Host/DebugInfoD.h
  lldb/packages/Python/lldbsuite/test/httpserver.py
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/source/Core/SourceManager.cpp
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/common/DebugInfoD.cpp
  lldb/test/CMakeLists.txt
  lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
  lldb/test/Shell/lit.cfg.py
  lldb/test/Shell/lit.site.cfg.py.in

Index: lldb/test/Shell/lit.site.cfg.py.in
===
--- lldb/test/Shell/lit.site.cfg.py.in
+++ lldb/test/Shell/lit.site.cfg.py.in
@@ -16,6 +16,7 @@
 config.python_executable = "@PYTHON_EXECUTABLE@"
 config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.lldb_enable_lzma = @LLDB_ENABLE_LZMA@
+config.lldb_enable_debuginfod = @LLDB_ENABLE_DEBUGINFOD@
 config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.lldb_bitness = 64 if @LLDB_IS_64_BITS@ else 32
 config.lldb_enable_python = @LLDB_ENABLE_PYTHON@
Index: lldb/test/Shell/lit.cfg.py
===
--- lldb/test/Shell/lit.cfg.py
+++ lldb/test/Shell/lit.cfg.py
@@ -117,6 +117,9 @@
 if config.lldb_enable_lzma:
 config.available_features.add('lzma')
 
+if config.lldb_enable_debuginfod:
+config.available_features.add('debuginfod')
+
 if find_executable('xz') != None:
 config.available_features.add('xz')
 
Index: lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
@@ -0,0 +1,121 @@
+// clang-format off
+// REQUIRES: debuginfod
+// UNSUPPORTED: darwin, windows
+
+//--
+// Test Purpose:
+// =
+// Test that we can display the source of functions using debuginfod when the
+// original source file is no longer present.
+// 
+// The debuginfod client requires a buildid in the binary, so we compile one in.
+// We can create the directory structure on disc that the client expects on a
+// webserver that serves source files. Then we fire up a python based http
+// server in the root of that mock directory, set the DEBUGINFOD_URLS
+// environment variable and let LLDB do the rest. 
+//
+// Go here to find more about debuginfod:
+// https://sourceware.org/elfutils/Debuginfod.html
+//--
+
+// We copy this file because we want to compile and later move it away
+// ===
+// RUN: mkdir -p %t.buildroot
+// RUN: cp %s %t.buildroot/test.cpp
+
+// We cd into the directory before compiling to get DW_AT_comp_dir pickup
+// %t.buildroot as well so it will be replaced by /my/new/path.
+// ===
+// RUN: cd %t.buildroot
+// RUN: %clang_host \
+// RUN:   -g \
+// RUN:   -Wl,--build-id="0xaabbccdd" \
+// RUN:   -fdebug-prefix-map=%t.buildroot=/my/new/path \
+// RUN:   -o %t \
+// RUN:   test.cpp
+
+// We move the original source file to a directory that looks like a debuginfod
+// URL part.
+// 
+// RUN: rm -rf %t.mock
+// RUN: mkdir -p   %t.mock/buildid/aabbccdd/source/my/new/path
+// RUN: mvtest.cpp %t.mock/buildid/aabbccdd/source/my/new/path
+
+// Adjust where debuginfod stores cache files:
+// ===
+// RUN: rm -rfv %t.debuginfod_cache_path
+// RUN: mkdir -pv %t.debuginfod_cache_path
+// RUN: export DEBUGINFOD_CACHE_PATH=%t.debuginfod_cache_path
+
+// Start HTTP file server on port picked by OS and wait until it is ready
+// The server will be closed on exit of the test.
+// ==
+// RUN: rm -fv "%t.server.log"
+// RUN: timeout 5 python3 -u -m http.server 0 --directory %t.mock --bind "localhost" &> %t.server.log & export PID=$!
+// RUN: trap 'echo "SERVER LOG:"; cat %t.server.log; kill $PID;' EXIT INT
+
+// Extract HTTP address from the first line of the server log
+// (e.g. "Serving HTTP on 127.0.0.1 port 40587 (http://127.0.0.1:40587/) ..")
+// ==
+// RUN: echo -n "Waiting for server to be ready"
+// RUN: SERVER_ADDRESS=""
+// RUN: while [ -z "$SERVER_ADDRESS" ]; do \
+// RUN: echo -n "."; \
+// RUN: sleep 0.01; \
+// RUN: SERVER_ADDRESS=$(hea

[Lldb-commits] [PATCH] D75750: WIP: [lldb] integrate debuginfod

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk planned changes to this revision.
kwk added a comment.

In D75750#1929967 , @jankratochvil 
wrote:

> On Fedora 31 x86_64 with LLDB using python3 I got:
>
>   llvm-lit: .../llvm-monorepo2/llvm/utils/lit/lit/TestingConfig.py:102: 
> fatal: unable to parse config file 
> '.../llvm-monorepo2-clangassert/tools/lldb/test/Shell/lit.site.cfg.py', 
> traceback: Traceback (most recent call last):
> File ".../llvm-monorepo2/llvm/utils/lit/lit/TestingConfig.py", line 89, 
> in load_from_path 
>   exec(compile(data, path, 'exec'), cfg_globals, None)
> File 
> ".../llvm-monorepo2-clangassert/tools/lldb/test/Shell/lit.site.cfg.py", line 
> 20, in 
>   config.lldb_enable_debuginfod = TRUE
>   NameError: name 'TRUE' is not defined
>   make[3]: *** [tools/lldb/test/CMakeFiles/check-lldb-lit.dir/build.make:58: 
> tools/lldb/test/CMakeFiles/check-lldb-lit] Error 2
>


Right, I manually fixed it locally and have forgotton to fix it. Thank you 
@jankratochvil for bringing it up.

> It helped to change:
> 
>   -  set(Debuginfod_FOUND TRUE)
>   +  set(Debuginfod_FOUND 1)

I'm sure this helps but we have a better way by using 
`llvm_canonicalize_cmake_booleans` in CMake. I did use this before for 
minidebuginfo and LZMA integration but the place in which I've put it was moved 
around which is why I needed some time. Expect a fix soon.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75750/new/

https://reviews.llvm.org/D75750



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D75750: WIP: [lldb] integrate debuginfod

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a comment.

@labath I've updated my patch and would love to hear your opinion on it. So far 
I've only written the python `ServeDirectoryWithHTTP()` function with proper 
doctest and documentation but since you mentioned the `0` port thingy I've 
tried that on the command line when using `python -m http.server 0` and it 
works smoothly. That's why I've included the `llvm-lit` test I was working on. 
Maybe `lldb/test/Shell/SymbolFile/DWARF/source-list.cpp` is the wrong file for 
this, but we can move it around if you like it so far.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75750/new/

https://reviews.llvm.org/D75750



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D75750: WIP: [lldb] integrate debuginfod

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 251086.
kwk added a comment.
Herald added a reviewer: jdoerfert.

- Added debuginfod2.py
- after running: autopep8 --in-place --aggressive --aggressive debuginfod2.py
- exponential backoff implemented
- Added http.py with doctests
- autopep8 --in-place  --aggressive http.py
- change import
- Removed simulated startup time
- fixup
- Changed wording
- Using os.path.abspath on directory before using it
- Fixups
- Fixups
- Fixups
- hide port and hostname from ServeDirectoryWithHTTP
- lit test working for debuginfod and source list


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75750/new/

https://reviews.llvm.org/D75750

Files:
  lldb/cmake/modules/FindDebuginfod.cmake
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/include/lldb/Host/Config.h.cmake
  lldb/include/lldb/Host/DebugInfoD.h
  lldb/packages/Python/lldbsuite/test/httpserver.py
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/source/Core/SourceManager.cpp
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/common/DebugInfoD.cpp
  lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
  lldb/test/Shell/lit.cfg.py
  lldb/test/Shell/lit.site.cfg.py.in

Index: lldb/test/Shell/lit.site.cfg.py.in
===
--- lldb/test/Shell/lit.site.cfg.py.in
+++ lldb/test/Shell/lit.site.cfg.py.in
@@ -16,6 +16,7 @@
 config.python_executable = "@PYTHON_EXECUTABLE@"
 config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.lldb_enable_lzma = @LLDB_ENABLE_LZMA@
+config.lldb_enable_debuginfod = @LLDB_ENABLE_DEBUGINFOD@
 config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.lldb_bitness = 64 if @LLDB_IS_64_BITS@ else 32
 config.lldb_enable_python = @LLDB_ENABLE_PYTHON@
Index: lldb/test/Shell/lit.cfg.py
===
--- lldb/test/Shell/lit.cfg.py
+++ lldb/test/Shell/lit.cfg.py
@@ -117,6 +117,9 @@
 if config.lldb_enable_lzma:
 config.available_features.add('lzma')
 
+if config.lldb_enable_debuginfod:
+config.available_features.add('debuginfod')
+
 if find_executable('xz') != None:
 config.available_features.add('xz')
 
Index: lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/source-list.cpp
@@ -0,0 +1,121 @@
+// clang-format off
+// REQUIRES: debuginfod
+// UNSUPPORTED: darwin, windows
+
+//--
+// Test Purpose:
+// =
+// Test that we can display the source of functions using debuginfod when the
+// original source file is no longer present.
+// 
+// The debuginfod client requires a buildid in the binary, so we compile one in.
+// We can create the directory structure on disc that the client expects on a
+// webserver that serves source files. Then we fire up a python based http
+// server in the root of that mock directory, set the DEBUGINFOD_URLS
+// environment variable and let LLDB do the rest. 
+//
+// Go here to find more about debuginfod:
+// https://sourceware.org/elfutils/Debuginfod.html
+//--
+
+// We copy this file because we want to compile and later move it away
+// ===
+// RUN: mkdir -p %t.buildroot
+// RUN: cp %s %t.buildroot/test.cpp
+
+// We cd into the directory before compiling to get DW_AT_comp_dir pickup
+// %t.buildroot as well so it will be replaced by /my/new/path.
+// ===
+// RUN: cd %t.buildroot
+// RUN: %clang_host \
+// RUN:   -g \
+// RUN:   -Wl,--build-id="0xaabbccdd" \
+// RUN:   -fdebug-prefix-map=%t.buildroot=/my/new/path \
+// RUN:   -o %t \
+// RUN:   test.cpp
+
+// We move the original source file to a directory that looks like a debuginfod
+// URL part.
+// 
+// RUN: rm -rf %t.mock
+// RUN: mkdir -p   %t.mock/buildid/aabbccdd/source/my/new/path
+// RUN: mvtest.cpp %t.mock/buildid/aabbccdd/source/my/new/path
+
+// Adjust where debuginfod stores cache files:
+// ===
+// RUN: rm -rfv %t.debuginfod_cache_path
+// RUN: mkdir -pv %t.debuginfod_cache_path
+// RUN: export DEBUGINFOD_CACHE_PATH=%t.debuginfod_cache_path
+
+// Start HTTP file server on port picked by OS and wait until it is ready
+// The server will be closed on exit of the test.
+// ==
+// RUN: rm -fv "%t.server.log"
+// RUN: timeout 5 python3 -u -m http.server 0 --directory %t.mock --bind "localhost" &> %t.server.log & export PID=$!
+// RUN: trap 'echo "SERVER LOG:"; cat %t.server.log; kill $PID;' EXIT INT
+

[Lldb-commits] [PATCH] D75750: WIP: [lldb] integrate debuginfod

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 249096.
kwk added a comment.

- Fix include ordering based on clang-format


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75750/new/

https://reviews.llvm.org/D75750

Files:
  lldb/cmake/modules/FindDebuginfod.cmake
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/include/lldb/Host/Config.h.cmake
  lldb/include/lldb/Host/DebugInfoD.h
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/source/Core/SourceManager.cpp
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/common/DebugInfoD.cpp

Index: lldb/source/Host/common/DebugInfoD.cpp
===
--- /dev/null
+++ lldb/source/Host/common/DebugInfoD.cpp
@@ -0,0 +1,96 @@
+//===-- DebugInfoD.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Host/DebugInfoD.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Host/Config.h"
+#include "lldb/Symbol/ObjectFile.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Errno.h"
+
+#if LLDB_ENABLE_DEBUGINFOD
+#include "elfutils/debuginfod.h"
+#endif
+
+namespace lldb_private {
+
+namespace debuginfod {
+
+using namespace lldb;
+using namespace lldb_private;
+
+#if !LLDB_ENABLE_DEBUGINFOD
+bool isAvailable() { return false; }
+
+llvm::Error findSource(UUID buildID, const std::string &path,
+   std::string &cache_path) {
+  llvm_unreachable("debuginfod::findSource is unavailable");
+}
+
+#else // LLDB_ENABLE_DEBUGINFOD
+
+bool isAvailable() { return true; }
+
+llvm::Expected findSource(UUID buildID, const std::string &path) {
+  if (!buildID.IsValid())
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "invalid build ID: %s",
+   buildID.GetAsString("").c_str());
+
+  debuginfod_client *client = debuginfod_begin();
+
+  if (!client)
+return llvm::createStringError(
+llvm::inconvertibleErrorCode(),
+"failed to create debuginfod connection handle: %s", strerror(errno));
+
+  // debuginfod_set_progressfn(client, [](debuginfod_client *client, long a,
+  // long b) -> int {
+  //   fprintf(stderr, "KWK === a: %ld b : %ld \n", a, b);
+  //   return 0; // continue
+  // });
+
+  char *cache_path = nullptr;
+  int rc = debuginfod_find_source(client, buildID.GetBytes().data(),
+  buildID.GetBytes().size(), path.c_str(),
+  &cache_path);
+
+  debuginfod_end(client);
+
+  std::string result_path;
+  if (cache_path) {
+result_path = std::string(cache_path);
+free(cache_path);
+  }
+
+  if (rc < 0) {
+if (rc == -ENOSYS) // No DEBUGINFO_URLS were specified
+  return result_path;
+else if (rc == -ENOENT) // No such file or directory, aka build-id not
+// available on servers.
+  return result_path;
+else
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "debuginfod_find_source query failed: %s",
+ llvm::sys::StrError(-rc).c_str());
+  }
+
+  if (close(rc) < 0) {
+return llvm::createStringError(
+llvm::inconvertibleErrorCode(),
+"failed to close result of call to debuginfo_find_source: %s",
+llvm::sys::StrError(errno).c_str());
+  }
+
+  return result_path;
+}
+
+#endif // LLDB_ENABLE_DEBUGINFOD
+
+} // end of namespace debuginfod
+} // namespace lldb_private
Index: lldb/source/Host/CMakeLists.txt
===
--- lldb/source/Host/CMakeLists.txt
+++ lldb/source/Host/CMakeLists.txt
@@ -30,6 +30,7 @@
   common/HostThread.cpp
   common/LockFileBase.cpp
   common/LZMA.cpp
+  common/DebugInfoD.cpp
   common/MainLoop.cpp
   common/MonitoringProcessLauncher.cpp
   common/NativeProcessProtocol.cpp
@@ -161,6 +162,9 @@
 if (LLDB_ENABLE_LZMA)
   list(APPEND EXTRA_LIBS ${LIBLZMA_LIBRARIES})
 endif()
+if (LLDB_ENABLE_DEBUGINFOD)
+  list(APPEND EXTRA_LIBS ${Debuginfod_LIBRARIES})
+endif()
 if (WIN32)
   list(APPEND LLDB_SYSTEM_LIBS psapi)
 endif ()
Index: lldb/source/Core/SourceManager.cpp
===
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -15,6 +15,7 @@
 #include "lldb/Core/Highlighter.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleList.h"
+#include "lldb/Host/DebugInfoD.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/Function.h"
@@ -402,7 +403,9 @@
 if (target) {
   m_source_map_m

[Lldb-commits] [PATCH] D75750: WIP: [lldb] integrate debuginfod

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk marked an inline comment as done.
kwk added a comment.

@labath thank you for your early feedback. It was helpful even though this is 
still a work in progress.




Comment at: lldb/source/Host/common/DebugInfoD.cpp:43-67
+UUID getBuildIDFromModule(const ModuleSP &module) {
+  UUID buildID;
+
+  if (!module)
+return buildID;
+
+  const FileSpec &moduleFileSpec = module->GetFileSpec();

labath wrote:
> How is all this different from `module->GetUUID()` ?
I didn't know about that :) . Thank you!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75750/new/

https://reviews.llvm.org/D75750



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D75750: WIP: [lldb] integrate debuginfod

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 249047.
kwk marked 3 inline comments as done.
kwk added a comment.

Changes suggested by elfutils maintainers:

- Silently ignore error when no DEBUGINFOD_URLS was given as an environment 
variable (ENOSYS).
- Silently ignore error when the build ID could not be found on any server 
(ENOENT).
- End debuginfod client before dealing with return code.

Applied review comments from labath:

- Removed getBuildIDFromModule because we have Module->GetUUID()
- Make findSource return an llvm::Expected instead of an error
- Various formatting issues with clang-format
- use llvm::sys::StrError instead of strerror directly

Other changes:

- Comments on functions in lldb_privat::debuginfod


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75750/new/

https://reviews.llvm.org/D75750

Files:
  lldb/cmake/modules/FindDebuginfod.cmake
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/include/lldb/Host/Config.h.cmake
  lldb/include/lldb/Host/DebugInfoD.h
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/source/Core/SourceManager.cpp
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/common/DebugInfoD.cpp

Index: lldb/source/Host/common/DebugInfoD.cpp
===
--- /dev/null
+++ lldb/source/Host/common/DebugInfoD.cpp
@@ -0,0 +1,96 @@
+//===-- DebugInfoD.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Host/DebugInfoD.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Host/Config.h"
+#include "lldb/Symbol/ObjectFile.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Errno.h"
+
+#if LLDB_ENABLE_DEBUGINFOD
+#include "elfutils/debuginfod.h"
+#endif
+
+namespace lldb_private {
+
+namespace debuginfod {
+
+using namespace lldb;
+using namespace lldb_private;
+
+#if !LLDB_ENABLE_DEBUGINFOD
+bool isAvailable() { return false; }
+
+llvm::Error findSource(UUID buildID, const std::string &path,
+   std::string &cache_path) {
+  llvm_unreachable("debuginfod::findSource is unavailable");
+}
+
+#else // LLDB_ENABLE_DEBUGINFOD
+
+bool isAvailable() { return true; }
+
+llvm::Expected findSource(UUID buildID, const std::string &path) {
+  if (!buildID.IsValid())
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "invalid build ID: %s",
+   buildID.GetAsString("").c_str());
+
+  debuginfod_client *client = debuginfod_begin();
+
+  if (!client)
+return llvm::createStringError(
+llvm::inconvertibleErrorCode(),
+"failed to create debuginfod connection handle: %s", strerror(errno));
+
+  // debuginfod_set_progressfn(client, [](debuginfod_client *client, long a,
+  // long b) -> int {
+  //   fprintf(stderr, "KWK === a: %ld b : %ld \n", a, b);
+  //   return 0; // continue
+  // });
+
+  char *cache_path = nullptr;
+  int rc = debuginfod_find_source(client, buildID.GetBytes().data(),
+  buildID.GetBytes().size(), path.c_str(),
+  &cache_path);
+
+  debuginfod_end(client);
+
+  std::string result_path;
+  if (cache_path) {
+result_path = std::string(cache_path);
+free(cache_path);
+  }
+
+  if (rc < 0) {
+if (rc == -ENOSYS) // No DEBUGINFO_URLS were specified
+  return result_path;
+else if (rc == -ENOENT) // No such file or directory, aka build-id not
+// available on servers.
+  return result_path;
+else
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "debuginfod_find_source query failed: %s",
+ llvm::sys::StrError(-rc).c_str());
+  }
+
+  if (close(rc) < 0) {
+return llvm::createStringError(
+llvm::inconvertibleErrorCode(),
+"failed to close result of call to debuginfo_find_source: %s",
+llvm::sys::StrError(errno).c_str());
+  }
+
+  return result_path;
+}
+
+#endif // LLDB_ENABLE_DEBUGINFOD
+
+} // end of namespace debuginfod
+} // namespace lldb_private
Index: lldb/source/Host/CMakeLists.txt
===
--- lldb/source/Host/CMakeLists.txt
+++ lldb/source/Host/CMakeLists.txt
@@ -30,6 +30,7 @@
   common/HostThread.cpp
   common/LockFileBase.cpp
   common/LZMA.cpp
+  common/DebugInfoD.cpp
   common/MainLoop.cpp
   common/MonitoringProcessLauncher.cpp
   common/NativeProcessProtocol.cpp
@@ -161,6 +162,9 @@
 if (LLDB_ENABLE_LZMA)
   list(APPEND EXTRA_LIBS ${LIBLZMA_LIBRARIES})
 endif()
+if (LLDB_ENABLE_DEBUGINFOD)
+  list(APPEND EXTRA

[Lldb-commits] [PATCH] D75753: Simplified return type of getBuildIDFromModule

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk abandoned this revision.
kwk added a comment.

I didn't intend to create a new revision.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75753/new/

https://reviews.llvm.org/D75753



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D75754: Fix typo

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk abandoned this revision.
kwk added a comment.

I didn't intend to create a new revision.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75754/new/

https://reviews.llvm.org/D75754



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D75754: Fix typo

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk created this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
kwk abandoned this revision.
kwk added a comment.

I didn't intend to create a new revision.


Depends on D75753 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75754

Files:
  lldb/source/Host/common/DebugInfoD.cpp


Index: lldb/source/Host/common/DebugInfoD.cpp
===
--- lldb/source/Host/common/DebugInfoD.cpp
+++ lldb/source/Host/common/DebugInfoD.cpp
@@ -93,7 +93,7 @@
 
   if (rc < 0)
 return llvm::createStringError(llvm::inconvertibleErrorCode(),
-   "buginfod_find_source query failed: %s",
+   "debuginfod_find_source query failed: %s",
strerror(-rc));
 
   if (cache_path) {


Index: lldb/source/Host/common/DebugInfoD.cpp
===
--- lldb/source/Host/common/DebugInfoD.cpp
+++ lldb/source/Host/common/DebugInfoD.cpp
@@ -93,7 +93,7 @@
 
   if (rc < 0)
 return llvm::createStringError(llvm::inconvertibleErrorCode(),
-   "buginfod_find_source query failed: %s",
+   "debuginfod_find_source query failed: %s",
strerror(-rc));
 
   if (cache_path) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D75750: WIP: [lldb] integrate debuginfod

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 248746.
kwk added a comment.

- Simplified return type of getBuildIDFromModule
- fixed typo


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75750/new/

https://reviews.llvm.org/D75750

Files:
  lldb/cmake/modules/FindDebuginfod.cmake
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/include/lldb/Host/Config.h.cmake
  lldb/include/lldb/Host/DebugInfoD.h
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/source/Core/SourceManager.cpp
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/common/DebugInfoD.cpp

Index: lldb/source/Host/common/DebugInfoD.cpp
===
--- /dev/null
+++ lldb/source/Host/common/DebugInfoD.cpp
@@ -0,0 +1,120 @@
+//===-- DebugInfoD.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Core/Module.h"
+#include "lldb/Host/Config.h"
+#include "lldb/Symbol/ObjectFile.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+#include "lldb/Host/DebugInfoD.h"
+
+#if LLDB_ENABLE_DEBUGINFOD
+#include "elfutils/debuginfod.h"
+#endif
+
+namespace lldb_private {
+
+namespace debuginfod {
+
+using namespace lldb;
+using namespace lldb_private;
+
+#if !LLDB_ENABLE_DEBUGINFOD
+bool isAvailable() { return false; }
+
+UUID getBuildIDFromModule(const ModuleSP &module) {
+  llvm_unreachable("debuginfod::getBuildIDFromModule is unavailable");
+};
+
+llvm::Error findSource(UUID buildID, const std::string &path,
+   std::string &cache_path, sys::TimePoint<> &mod_time) {
+  llvm_unreachable("debuginfod::findSource is unavailable");
+}
+
+#else // LLDB_ENABLE_DEBUGINFOD
+
+bool isAvailable() { return true; }
+
+UUID getBuildIDFromModule(const ModuleSP &module) {
+  UUID buildID;
+
+  if (!module)
+return buildID;
+
+  const FileSpec &moduleFileSpec = module->GetFileSpec();
+  ModuleSpecList specList;
+  size_t nSpecs =
+  ObjectFile::GetModuleSpecifications(moduleFileSpec, 0, 0, specList);
+
+  for (size_t i = 0; i < nSpecs; i++) {
+ModuleSpec spec;
+if (!specList.GetModuleSpecAtIndex(i, spec))
+  continue;
+
+const UUID &uuid = spec.GetUUID();
+if (!uuid.IsValid())
+  continue;
+
+buildID = uuid;
+break;
+  }
+  return buildID;
+}
+
+llvm::Error findSource(UUID buildID, const std::string &path,
+   std::string &result_path) {
+  if (!buildID.IsValid())
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "invalid build ID: %s",
+   buildID.GetAsString("").c_str());
+
+  debuginfod_client *client = debuginfod_begin();
+
+  if (!client)
+return llvm::createStringError(
+llvm::inconvertibleErrorCode(),
+"failed to create debuginfod connection handle: %s", strerror(errno));
+
+  // debuginfod_set_progressfn(client, [](debuginfod_client *client, long a,
+  // long b) -> int {
+  //   fprintf(stderr, "KWK === a: %ld b : %ld \n", a, b);
+  //   return 0; // continue
+  // });
+
+  char *cache_path = nullptr;
+  int rc = debuginfod_find_source(client, buildID.GetBytes().data(),
+  buildID.GetBytes().size(), path.c_str(),
+  &cache_path);
+
+  if (rc < 0)
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "debuginfod_find_source query failed: %s",
+   strerror(-rc));
+
+  if (cache_path) {
+result_path = std::string(cache_path);
+free(cache_path);
+  }
+
+  llvm::Error err = llvm::Error::success();
+  if (close(rc) < 0) {
+err = llvm::createStringError(
+llvm::inconvertibleErrorCode(),
+"failed to close result of call to debuginfo_find_source: %s",
+strerror(errno));
+  }
+
+  debuginfod_end(client);
+
+  return err;
+}
+
+#endif // LLDB_ENABLE_DEBUGINFOD
+
+} // end of namespace debuginfod
+} // namespace lldb_private
Index: lldb/source/Host/CMakeLists.txt
===
--- lldb/source/Host/CMakeLists.txt
+++ lldb/source/Host/CMakeLists.txt
@@ -30,6 +30,7 @@
   common/HostThread.cpp
   common/LockFileBase.cpp
   common/LZMA.cpp
+  common/DebugInfoD.cpp
   common/MainLoop.cpp
   common/MonitoringProcessLauncher.cpp
   common/NativeProcessProtocol.cpp
@@ -161,6 +162,9 @@
 if (LLDB_ENABLE_LZMA)
   list(APPEND EXTRA_LIBS ${LIBLZMA_LIBRARIES})
 endif()
+if (LLDB_ENABLE_DEBUGINFOD)
+  list(APPEND EXTRA_LIBS ${Debuginfod_LIBRARIES})
+endif()
 if (WIN32)
   list(APPEND LLDB_SYSTEM_LIBS psapi)
 endif ()
Index: lld

[Lldb-commits] [PATCH] D75753: Simplified return type of getBuildIDFromModule

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk created this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
kwk added a child revision: D75754: Fix typo.
kwk abandoned this revision.
kwk added a comment.

I didn't intend to create a new revision.


Depends on D75750 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75753

Files:
  lldb/include/lldb/Host/DebugInfoD.h
  lldb/source/Core/SourceManager.cpp
  lldb/source/Host/common/DebugInfoD.cpp


Index: lldb/source/Host/common/DebugInfoD.cpp
===
--- lldb/source/Host/common/DebugInfoD.cpp
+++ lldb/source/Host/common/DebugInfoD.cpp
@@ -11,6 +11,7 @@
 #include "lldb/Symbol/ObjectFile.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
+#include "lldb/Host/DebugInfoD.h"
 
 #if LLDB_ENABLE_DEBUGINFOD
 #include "elfutils/debuginfod.h"
@@ -26,7 +27,7 @@
 #if !LLDB_ENABLE_DEBUGINFOD
 bool isAvailable() { return false; }
 
-llvm::Expected getBuildIDFromModule(const ModuleSP &module) {
+UUID getBuildIDFromModule(const ModuleSP &module) {
   llvm_unreachable("debuginfod::getBuildIDFromModule is unavailable");
 };
 
@@ -39,7 +40,7 @@
 
 bool isAvailable() { return true; }
 
-llvm::Expected getBuildIDFromModule(const ModuleSP &module) {
+UUID getBuildIDFromModule(const ModuleSP &module) {
   UUID buildID;
 
   if (!module)
Index: lldb/source/Core/SourceManager.cpp
===
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -460,25 +460,18 @@
   // Try finding the file using elfutils' debuginfod
   if (!FileSystem::Instance().Exists(m_file_spec) &&
   debuginfod::isAvailable() && sc.module_sp) {
-llvm::Expected buildID =
-debuginfod::getBuildIDFromModule(sc.module_sp);
-if (auto err = buildID.takeError()) {
-  sc.module_sp->ReportWarning("An error occurred while getting the "
-  "build ID from the module: %s",
+UUID buildID = debuginfod::getBuildIDFromModule(sc.module_sp);
+std::string cache_path;
+llvm::Error err =
+debuginfod::findSource(buildID, file_spec.GetCString(), 
cache_path);
+if (err) {
+  sc.module_sp->ReportWarning("An error occurred while finding the "
+  "source file %s using debuginfod: %s",
+  file_spec.GetCString(),
   llvm::toString(std::move(err)).c_str());
 } else {
-  std::string cache_path;
-  err = debuginfod::findSource(*buildID, file_spec.GetCString(),
-   cache_path);
-  if (err) {
-sc.module_sp->ReportWarning("An error occurred while finding the "
-"source file %s using debuginfod: %s",
-file_spec.GetCString(),
-
llvm::toString(std::move(err)).c_str());
-  } else {
-m_file_spec = FileSpec(cache_path);
-m_mod_time = 
FileSystem::Instance().GetModificationTime(cache_path);
-  }
+  m_file_spec = FileSpec(cache_path);
+  m_mod_time = FileSystem::Instance().GetModificationTime(cache_path);
 }
   }
 }
Index: lldb/include/lldb/Host/DebugInfoD.h
===
--- lldb/include/lldb/Host/DebugInfoD.h
+++ lldb/include/lldb/Host/DebugInfoD.h
@@ -21,7 +21,7 @@

 bool isAvailable();
 
-llvm::Expected getBuildIDFromModule(const lldb::ModuleSP &module);
+UUID getBuildIDFromModule(const lldb::ModuleSP &module);
 
 llvm::Error findSource(UUID buildID, const std::string &path,
std::string &result_path);


Index: lldb/source/Host/common/DebugInfoD.cpp
===
--- lldb/source/Host/common/DebugInfoD.cpp
+++ lldb/source/Host/common/DebugInfoD.cpp
@@ -11,6 +11,7 @@
 #include "lldb/Symbol/ObjectFile.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
+#include "lldb/Host/DebugInfoD.h"
 
 #if LLDB_ENABLE_DEBUGINFOD
 #include "elfutils/debuginfod.h"
@@ -26,7 +27,7 @@
 #if !LLDB_ENABLE_DEBUGINFOD
 bool isAvailable() { return false; }
 
-llvm::Expected getBuildIDFromModule(const ModuleSP &module) {
+UUID getBuildIDFromModule(const ModuleSP &module) {
   llvm_unreachable("debuginfod::getBuildIDFromModule is unavailable");
 };
 
@@ -39,7 +40,7 @@
 
 bool isAvailable() { return true; }
 
-llvm::Expected getBuildIDFromModule(const ModuleSP &module) {
+UUID getBuildIDFromModule(const ModuleSP &module) {
   UUID buildID;
 
   if (!module)
Index: lldb/source/Core/SourceManager.cpp
===
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/

[Lldb-commits] [PATCH] D75750: WIP: [lldb] integrate debuginfod

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk created this revision.
Herald added subscribers: lldb-commits, mgorny.
Herald added a project: LLDB.
kwk planned changes to this revision.

This first patch does the heavy lifting of bootstrapping debuginfod with
CMake and integrating it to find a source file using debuginfod when
using `(lldb) source list` and the file cannot be found locally.

TODOs:

- tests are missing
- fetching debuginfo from debuginfod is missing

Read more about debuginfod here:
https://sourceware.org/elfutils/Debuginfod.html


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75750

Files:
  lldb/cmake/modules/FindDebuginfod.cmake
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/include/lldb/Host/Config.h.cmake
  lldb/include/lldb/Host/DebugInfoD.h
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/source/Core/SourceManager.cpp
  lldb/source/Host/CMakeLists.txt
  lldb/source/Host/common/DebugInfoD.cpp

Index: lldb/source/Host/common/DebugInfoD.cpp
===
--- /dev/null
+++ lldb/source/Host/common/DebugInfoD.cpp
@@ -0,0 +1,119 @@
+//===-- DebugInfoD.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Core/Module.h"
+#include "lldb/Host/Config.h"
+#include "lldb/Symbol/ObjectFile.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+
+#if LLDB_ENABLE_DEBUGINFOD
+#include "elfutils/debuginfod.h"
+#endif
+
+namespace lldb_private {
+
+namespace debuginfod {
+
+using namespace lldb;
+using namespace lldb_private;
+
+#if !LLDB_ENABLE_DEBUGINFOD
+bool isAvailable() { return false; }
+
+llvm::Expected getBuildIDFromModule(const ModuleSP &module) {
+  llvm_unreachable("debuginfod::getBuildIDFromModule is unavailable");
+};
+
+llvm::Error findSource(UUID buildID, const std::string &path,
+   std::string &cache_path, sys::TimePoint<> &mod_time) {
+  llvm_unreachable("debuginfod::findSource is unavailable");
+}
+
+#else // LLDB_ENABLE_DEBUGINFOD
+
+bool isAvailable() { return true; }
+
+llvm::Expected getBuildIDFromModule(const ModuleSP &module) {
+  UUID buildID;
+
+  if (!module)
+return buildID;
+
+  const FileSpec &moduleFileSpec = module->GetFileSpec();
+  ModuleSpecList specList;
+  size_t nSpecs =
+  ObjectFile::GetModuleSpecifications(moduleFileSpec, 0, 0, specList);
+
+  for (size_t i = 0; i < nSpecs; i++) {
+ModuleSpec spec;
+if (!specList.GetModuleSpecAtIndex(i, spec))
+  continue;
+
+const UUID &uuid = spec.GetUUID();
+if (!uuid.IsValid())
+  continue;
+
+buildID = uuid;
+break;
+  }
+  return buildID;
+}
+
+llvm::Error findSource(UUID buildID, const std::string &path,
+   std::string &result_path) {
+  if (!buildID.IsValid())
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "invalid build ID: %s",
+   buildID.GetAsString("").c_str());
+
+  debuginfod_client *client = debuginfod_begin();
+
+  if (!client)
+return llvm::createStringError(
+llvm::inconvertibleErrorCode(),
+"failed to create debuginfod connection handle: %s", strerror(errno));
+
+  // debuginfod_set_progressfn(client, [](debuginfod_client *client, long a,
+  // long b) -> int {
+  //   fprintf(stderr, "KWK === a: %ld b : %ld \n", a, b);
+  //   return 0; // continue
+  // });
+
+  char *cache_path = nullptr;
+  int rc = debuginfod_find_source(client, buildID.GetBytes().data(),
+  buildID.GetBytes().size(), path.c_str(),
+  &cache_path);
+
+  if (rc < 0)
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "buginfod_find_source query failed: %s",
+   strerror(-rc));
+
+  if (cache_path) {
+result_path = std::string(cache_path);
+free(cache_path);
+  }
+
+  llvm::Error err = llvm::Error::success();
+  if (close(rc) < 0) {
+err = llvm::createStringError(
+llvm::inconvertibleErrorCode(),
+"failed to close result of call to debuginfo_find_source: %s",
+strerror(errno));
+  }
+
+  debuginfod_end(client);
+
+  return err;
+}
+
+#endif // LLDB_ENABLE_DEBUGINFOD
+
+} // end of namespace debuginfod
+} // namespace lldb_private
Index: lldb/source/Host/CMakeLists.txt
===
--- lldb/source/Host/CMakeLists.txt
+++ lldb/source/Host/CMakeLists.txt
@@ -30,6 +30,7 @@
   common/HostThread.cpp
   common/LockFileBase.cpp
   common/LZMA.cpp
+  common/DebugInfoD.cpp
   common/MainLoop.cpp
   common/MonitoringProcessLauncher

[Lldb-commits] [PATCH] D74650: [lldb] WIP: idea for keeping filenames in raw form when coming from DWARF

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk abandoned this revision.
kwk added a comment.

I abandon this revision because @labath mentioned that in the past we must have 
had something similar: two file specs where one was normalized and one doesn't 
and then the behavior was not very predictable. I hope this recaptures our 
conversation on IRC well enough.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74650/new/

https://reviews.llvm.org/D74650



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a comment.

I figured it might be the easiest to re-use `SearchFilterByModuleListAndCU` but 
it needs to be renamed still. If you have a good suggestion, please let me know.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74136/new/

https://reviews.llvm.org/D74136



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk added a comment.

@labath @jingham Can you please have a look at the 
`lldb/test/Shell/Breakpoint/search-support-files.test` to see if the test 
reflects the desired behavior?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74136/new/

https://reviews.llvm.org/D74136



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

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) {
+ 

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

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

kwk updated this revision to Diff 246160.
kwk added a comment.

Updated tests and code to remove the --search-source-files flag and make it the 
default behavior to also search source files.

TODO: rename class SearchFilterByModuleListAndCU to something more meaningful 
when an agreement on the behavior was made.


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/Commands/CommandObjectBreakpoint.cpp
  lldb/source/Core/SearchFilter.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.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 @@
   

[Lldb-commits] [PATCH] D74895: [lldb]: fix typo in lldb-gdb-remote.txt

[Konrad Wilhelm Kleine via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc7b7f76ae6ae: [lldb]: fix typo in lldb-gdb-remote.txt 
(authored by kwk).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74895/new/

https://reviews.llvm.org/D74895

Files:
  lldb/docs/lldb-gdb-remote.txt


Index: lldb/docs/lldb-gdb-remote.txt
===
--- lldb/docs/lldb-gdb-remote.txt
+++ lldb/docs/lldb-gdb-remote.txt
@@ -3,7 +3,7 @@
 correct GDB server when debugging. If you have mismatch, then things go wrong
 very quickly. LLDB makes extensive use of the GDB remote protocol and we
 wanted to make sure that the experience was a bit more dynamic where we can
-discover information about a remote target with having to know anything up
+discover information about a remote target without having to know anything up
 front. We also ran into performance issues with the existing GDB remote
 protocol that can be overcome when using a reliable communications layer.
 Some packets improve performance, others allow for remote process launching


Index: lldb/docs/lldb-gdb-remote.txt
===
--- lldb/docs/lldb-gdb-remote.txt
+++ lldb/docs/lldb-gdb-remote.txt
@@ -3,7 +3,7 @@
 correct GDB server when debugging. If you have mismatch, then things go wrong
 very quickly. LLDB makes extensive use of the GDB remote protocol and we
 wanted to make sure that the experience was a bit more dynamic where we can
-discover information about a remote target with having to know anything up
+discover information about a remote target without having to know anything up
 front. We also ran into performance issues with the existing GDB remote
 protocol that can be overcome when using a reliable communications layer.
 Some packets improve performance, others allow for remote process launching
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits