[Lldb-commits] [lldb] [lldb] Allow languages to filter breakpoints set by line (PR #83908)

[Felipe de Azevedo Piovezan via lldb-commits]

https://github.com/felipepiovezan closed 
https://github.com/llvm/llvm-project/pull/83908
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Allow languages to filter breakpoints set by line (PR #83908)

[Felipe de Azevedo Piovezan via lldb-commits]

https://github.com/felipepiovezan updated 
https://github.com/llvm/llvm-project/pull/83908

>From 51307b548d09c34ee06037ccf459110e451970a9 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan 
Date: Mon, 4 Mar 2024 09:56:18 -0800
Subject: [PATCH 1/7] [lldb] Allow languages to filter breakpoints set by line

Some languages may create artificial functions that have no real user code, even
though there is line table information for them. One such case is with coroutine
code that receives the CoroSplitter transformation in LLVM IR. That code
transformation creates many different Functions, cloning one Instruction into
many Instructions in many different Functions and copying the associated debug
locations.

It would be difficult to make that pass delete debug locations of cloned
instructions in a language agnostic way (is it even possible?), but LLDB can
ignore certain locations by querying its Language APIs and having it decide
based on, for example, mangling information.
---
 lldb/include/lldb/Target/Language.h   |  4 
 lldb/source/Breakpoint/BreakpointResolver.cpp | 12 
 2 files changed, 16 insertions(+)

diff --git a/lldb/include/lldb/Target/Language.h 
b/lldb/include/lldb/Target/Language.h
index 0cbd8a32dccd54..957c40eb7c0772 100644
--- a/lldb/include/lldb/Target/Language.h
+++ b/lldb/include/lldb/Target/Language.h
@@ -339,6 +339,10 @@ class Language : public PluginInterface {
 
   virtual llvm::StringRef GetInstanceVariableName() { return {}; }
 
+  virtual bool IsInterestingCtxForLineBreakpoint(const SymbolContext &) const {
+return true;
+  }
+
 protected:
   // Classes that inherit from Language can see and modify these
 
diff --git a/lldb/source/Breakpoint/BreakpointResolver.cpp 
b/lldb/source/Breakpoint/BreakpointResolver.cpp
index bc6348716ef418..876b30c6d76d55 100644
--- a/lldb/source/Breakpoint/BreakpointResolver.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolver.cpp
@@ -23,6 +23,7 @@
 #include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/Function.h"
 #include "lldb/Symbol/SymbolContext.h"
+#include "lldb/Target/Language.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Log.h"
@@ -199,6 +200,15 @@ bool operator<(const SourceLoc lhs, const SourceLoc rhs) {
 }
 } // namespace
 
+static void
+ApplyLanguageFilters(llvm::SmallVectorImpl _list) {
+  llvm::erase_if(sc_list, [](SymbolContext ) {
+if (Language *lang = Language::FindPlugin(sc.GetLanguage()))
+  return !lang->IsInterestingCtxForLineBreakpoint(sc);
+return false;
+  });
+}
+
 void BreakpointResolver::SetSCMatchesByLine(
 SearchFilter , SymbolContextList _list, bool skip_prologue,
 llvm::StringRef log_ident, uint32_t line, std::optional column) {
@@ -206,6 +216,8 @@ void BreakpointResolver::SetSCMatchesByLine(
   for (uint32_t i = 0; i < sc_list.GetSize(); ++i)
 all_scs.push_back(sc_list[i]);
 
+  ApplyLanguageFilters(all_scs);
+
   while (all_scs.size()) {
 uint32_t closest_line = UINT32_MAX;
 

>From 7ea76d6c04d063d50da52756179639f2037aa793 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan 
Date: Tue, 5 Mar 2024 18:47:51 -0800
Subject: [PATCH 2/7] fixup! add target option

---
 lldb/include/lldb/Target/Language.h   |  9 ++--
 lldb/include/lldb/Target/Target.h |  2 ++
 lldb/source/Breakpoint/BreakpointResolver.cpp | 23 +--
 lldb/source/Target/Target.cpp |  8 +++
 lldb/source/Target/TargetProperties.td|  3 +++
 5 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/lldb/include/lldb/Target/Language.h 
b/lldb/include/lldb/Target/Language.h
index 957c40eb7c0772..9db9f4e297e114 100644
--- a/lldb/include/lldb/Target/Language.h
+++ b/lldb/include/lldb/Target/Language.h
@@ -339,8 +339,13 @@ class Language : public PluginInterface {
 
   virtual llvm::StringRef GetInstanceVariableName() { return {}; }
 
-  virtual bool IsInterestingCtxForLineBreakpoint(const SymbolContext &) const {
-return true;
+  // Returns true if this SymbolContext should be used when setting breakpoints
+  // by line (number or regex). This is useful for languages that create
+  // artificial functions without any meaningful user code associated with them
+  // (e.g. code that gets expanded in late compilation stages, like by
+  // CoroSplitter).
+  virtual bool IsArtificialCtxForLineBreakpoint(const SymbolContext &) const {
+return false;
   }
 
 protected:
diff --git a/lldb/include/lldb/Target/Target.h 
b/lldb/include/lldb/Target/Target.h
index 8f57358981d4d2..ab04aa57f17300 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -258,6 +258,8 @@ class TargetProperties : public Properties {
 
   bool GetDebugUtilityExpression() const;
 
+  bool GetIgnoreBreakpointsFromLanguageArtificialLocations() const;
+
 private:
   // Callbacks for m_launch_info.
   void Arg0ValueChangedCallback();
diff --git 

[Lldb-commits] [lldb] [lldb] Allow languages to filter breakpoints set by line (PR #83908)

[Felipe de Azevedo Piovezan via lldb-commits]

https://github.com/felipepiovezan updated 
https://github.com/llvm/llvm-project/pull/83908

>From 51307b548d09c34ee06037ccf459110e451970a9 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan 
Date: Mon, 4 Mar 2024 09:56:18 -0800
Subject: [PATCH 1/6] [lldb] Allow languages to filter breakpoints set by line

Some languages may create artificial functions that have no real user code, even
though there is line table information for them. One such case is with coroutine
code that receives the CoroSplitter transformation in LLVM IR. That code
transformation creates many different Functions, cloning one Instruction into
many Instructions in many different Functions and copying the associated debug
locations.

It would be difficult to make that pass delete debug locations of cloned
instructions in a language agnostic way (is it even possible?), but LLDB can
ignore certain locations by querying its Language APIs and having it decide
based on, for example, mangling information.
---
 lldb/include/lldb/Target/Language.h   |  4 
 lldb/source/Breakpoint/BreakpointResolver.cpp | 12 
 2 files changed, 16 insertions(+)

diff --git a/lldb/include/lldb/Target/Language.h 
b/lldb/include/lldb/Target/Language.h
index 0cbd8a32dccd54..957c40eb7c0772 100644
--- a/lldb/include/lldb/Target/Language.h
+++ b/lldb/include/lldb/Target/Language.h
@@ -339,6 +339,10 @@ class Language : public PluginInterface {
 
   virtual llvm::StringRef GetInstanceVariableName() { return {}; }
 
+  virtual bool IsInterestingCtxForLineBreakpoint(const SymbolContext &) const {
+return true;
+  }
+
 protected:
   // Classes that inherit from Language can see and modify these
 
diff --git a/lldb/source/Breakpoint/BreakpointResolver.cpp 
b/lldb/source/Breakpoint/BreakpointResolver.cpp
index bc6348716ef418..876b30c6d76d55 100644
--- a/lldb/source/Breakpoint/BreakpointResolver.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolver.cpp
@@ -23,6 +23,7 @@
 #include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/Function.h"
 #include "lldb/Symbol/SymbolContext.h"
+#include "lldb/Target/Language.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Log.h"
@@ -199,6 +200,15 @@ bool operator<(const SourceLoc lhs, const SourceLoc rhs) {
 }
 } // namespace
 
+static void
+ApplyLanguageFilters(llvm::SmallVectorImpl _list) {
+  llvm::erase_if(sc_list, [](SymbolContext ) {
+if (Language *lang = Language::FindPlugin(sc.GetLanguage()))
+  return !lang->IsInterestingCtxForLineBreakpoint(sc);
+return false;
+  });
+}
+
 void BreakpointResolver::SetSCMatchesByLine(
 SearchFilter , SymbolContextList _list, bool skip_prologue,
 llvm::StringRef log_ident, uint32_t line, std::optional column) {
@@ -206,6 +216,8 @@ void BreakpointResolver::SetSCMatchesByLine(
   for (uint32_t i = 0; i < sc_list.GetSize(); ++i)
 all_scs.push_back(sc_list[i]);
 
+  ApplyLanguageFilters(all_scs);
+
   while (all_scs.size()) {
 uint32_t closest_line = UINT32_MAX;
 

>From 7ea76d6c04d063d50da52756179639f2037aa793 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan 
Date: Tue, 5 Mar 2024 18:47:51 -0800
Subject: [PATCH 2/6] fixup! add target option

---
 lldb/include/lldb/Target/Language.h   |  9 ++--
 lldb/include/lldb/Target/Target.h |  2 ++
 lldb/source/Breakpoint/BreakpointResolver.cpp | 23 +--
 lldb/source/Target/Target.cpp |  8 +++
 lldb/source/Target/TargetProperties.td|  3 +++
 5 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/lldb/include/lldb/Target/Language.h 
b/lldb/include/lldb/Target/Language.h
index 957c40eb7c0772..9db9f4e297e114 100644
--- a/lldb/include/lldb/Target/Language.h
+++ b/lldb/include/lldb/Target/Language.h
@@ -339,8 +339,13 @@ class Language : public PluginInterface {
 
   virtual llvm::StringRef GetInstanceVariableName() { return {}; }
 
-  virtual bool IsInterestingCtxForLineBreakpoint(const SymbolContext &) const {
-return true;
+  // Returns true if this SymbolContext should be used when setting breakpoints
+  // by line (number or regex). This is useful for languages that create
+  // artificial functions without any meaningful user code associated with them
+  // (e.g. code that gets expanded in late compilation stages, like by
+  // CoroSplitter).
+  virtual bool IsArtificialCtxForLineBreakpoint(const SymbolContext &) const {
+return false;
   }
 
 protected:
diff --git a/lldb/include/lldb/Target/Target.h 
b/lldb/include/lldb/Target/Target.h
index 8f57358981d4d2..ab04aa57f17300 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -258,6 +258,8 @@ class TargetProperties : public Properties {
 
   bool GetDebugUtilityExpression() const;
 
+  bool GetIgnoreBreakpointsFromLanguageArtificialLocations() const;
+
 private:
   // Callbacks for m_launch_info.
   void Arg0ValueChangedCallback();
diff --git 

[Lldb-commits] [lldb] [lldb] Allow languages to filter breakpoints set by line (PR #83908)

[via lldb-commits]

https://github.com/jimingham approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/83908
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Allow languages to filter breakpoints set by line (PR #83908)

[Felipe de Azevedo Piovezan via lldb-commits]

https://github.com/felipepiovezan updated 
https://github.com/llvm/llvm-project/pull/83908

>From 51307b548d09c34ee06037ccf459110e451970a9 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan 
Date: Mon, 4 Mar 2024 09:56:18 -0800
Subject: [PATCH 1/5] [lldb] Allow languages to filter breakpoints set by line

Some languages may create artificial functions that have no real user code, even
though there is line table information for them. One such case is with coroutine
code that receives the CoroSplitter transformation in LLVM IR. That code
transformation creates many different Functions, cloning one Instruction into
many Instructions in many different Functions and copying the associated debug
locations.

It would be difficult to make that pass delete debug locations of cloned
instructions in a language agnostic way (is it even possible?), but LLDB can
ignore certain locations by querying its Language APIs and having it decide
based on, for example, mangling information.
---
 lldb/include/lldb/Target/Language.h   |  4 
 lldb/source/Breakpoint/BreakpointResolver.cpp | 12 
 2 files changed, 16 insertions(+)

diff --git a/lldb/include/lldb/Target/Language.h 
b/lldb/include/lldb/Target/Language.h
index 0cbd8a32dccd54..957c40eb7c0772 100644
--- a/lldb/include/lldb/Target/Language.h
+++ b/lldb/include/lldb/Target/Language.h
@@ -339,6 +339,10 @@ class Language : public PluginInterface {
 
   virtual llvm::StringRef GetInstanceVariableName() { return {}; }
 
+  virtual bool IsInterestingCtxForLineBreakpoint(const SymbolContext &) const {
+return true;
+  }
+
 protected:
   // Classes that inherit from Language can see and modify these
 
diff --git a/lldb/source/Breakpoint/BreakpointResolver.cpp 
b/lldb/source/Breakpoint/BreakpointResolver.cpp
index bc6348716ef418..876b30c6d76d55 100644
--- a/lldb/source/Breakpoint/BreakpointResolver.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolver.cpp
@@ -23,6 +23,7 @@
 #include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/Function.h"
 #include "lldb/Symbol/SymbolContext.h"
+#include "lldb/Target/Language.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Log.h"
@@ -199,6 +200,15 @@ bool operator<(const SourceLoc lhs, const SourceLoc rhs) {
 }
 } // namespace
 
+static void
+ApplyLanguageFilters(llvm::SmallVectorImpl _list) {
+  llvm::erase_if(sc_list, [](SymbolContext ) {
+if (Language *lang = Language::FindPlugin(sc.GetLanguage()))
+  return !lang->IsInterestingCtxForLineBreakpoint(sc);
+return false;
+  });
+}
+
 void BreakpointResolver::SetSCMatchesByLine(
 SearchFilter , SymbolContextList _list, bool skip_prologue,
 llvm::StringRef log_ident, uint32_t line, std::optional column) {
@@ -206,6 +216,8 @@ void BreakpointResolver::SetSCMatchesByLine(
   for (uint32_t i = 0; i < sc_list.GetSize(); ++i)
 all_scs.push_back(sc_list[i]);
 
+  ApplyLanguageFilters(all_scs);
+
   while (all_scs.size()) {
 uint32_t closest_line = UINT32_MAX;
 

>From 7ea76d6c04d063d50da52756179639f2037aa793 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan 
Date: Tue, 5 Mar 2024 18:47:51 -0800
Subject: [PATCH 2/5] fixup! add target option

---
 lldb/include/lldb/Target/Language.h   |  9 ++--
 lldb/include/lldb/Target/Target.h |  2 ++
 lldb/source/Breakpoint/BreakpointResolver.cpp | 23 +--
 lldb/source/Target/Target.cpp |  8 +++
 lldb/source/Target/TargetProperties.td|  3 +++
 5 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/lldb/include/lldb/Target/Language.h 
b/lldb/include/lldb/Target/Language.h
index 957c40eb7c0772..9db9f4e297e114 100644
--- a/lldb/include/lldb/Target/Language.h
+++ b/lldb/include/lldb/Target/Language.h
@@ -339,8 +339,13 @@ class Language : public PluginInterface {
 
   virtual llvm::StringRef GetInstanceVariableName() { return {}; }
 
-  virtual bool IsInterestingCtxForLineBreakpoint(const SymbolContext &) const {
-return true;
+  // Returns true if this SymbolContext should be used when setting breakpoints
+  // by line (number or regex). This is useful for languages that create
+  // artificial functions without any meaningful user code associated with them
+  // (e.g. code that gets expanded in late compilation stages, like by
+  // CoroSplitter).
+  virtual bool IsArtificialCtxForLineBreakpoint(const SymbolContext &) const {
+return false;
   }
 
 protected:
diff --git a/lldb/include/lldb/Target/Target.h 
b/lldb/include/lldb/Target/Target.h
index 8f57358981d4d2..ab04aa57f17300 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -258,6 +258,8 @@ class TargetProperties : public Properties {
 
   bool GetDebugUtilityExpression() const;
 
+  bool GetIgnoreBreakpointsFromLanguageArtificialLocations() const;
+
 private:
   // Callbacks for m_launch_info.
   void Arg0ValueChangedCallback();
diff --git 

[Lldb-commits] [lldb] [lldb] Allow languages to filter breakpoints set by line (PR #83908)

[Felipe de Azevedo Piovezan via lldb-commits]

https://github.com/felipepiovezan updated 
https://github.com/llvm/llvm-project/pull/83908

>From 51307b548d09c34ee06037ccf459110e451970a9 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan 
Date: Mon, 4 Mar 2024 09:56:18 -0800
Subject: [PATCH 1/5] [lldb] Allow languages to filter breakpoints set by line

Some languages may create artificial functions that have no real user code, even
though there is line table information for them. One such case is with coroutine
code that receives the CoroSplitter transformation in LLVM IR. That code
transformation creates many different Functions, cloning one Instruction into
many Instructions in many different Functions and copying the associated debug
locations.

It would be difficult to make that pass delete debug locations of cloned
instructions in a language agnostic way (is it even possible?), but LLDB can
ignore certain locations by querying its Language APIs and having it decide
based on, for example, mangling information.
---
 lldb/include/lldb/Target/Language.h   |  4 
 lldb/source/Breakpoint/BreakpointResolver.cpp | 12 
 2 files changed, 16 insertions(+)

diff --git a/lldb/include/lldb/Target/Language.h 
b/lldb/include/lldb/Target/Language.h
index 0cbd8a32dccd54..957c40eb7c0772 100644
--- a/lldb/include/lldb/Target/Language.h
+++ b/lldb/include/lldb/Target/Language.h
@@ -339,6 +339,10 @@ class Language : public PluginInterface {
 
   virtual llvm::StringRef GetInstanceVariableName() { return {}; }
 
+  virtual bool IsInterestingCtxForLineBreakpoint(const SymbolContext &) const {
+return true;
+  }
+
 protected:
   // Classes that inherit from Language can see and modify these
 
diff --git a/lldb/source/Breakpoint/BreakpointResolver.cpp 
b/lldb/source/Breakpoint/BreakpointResolver.cpp
index bc6348716ef418..876b30c6d76d55 100644
--- a/lldb/source/Breakpoint/BreakpointResolver.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolver.cpp
@@ -23,6 +23,7 @@
 #include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/Function.h"
 #include "lldb/Symbol/SymbolContext.h"
+#include "lldb/Target/Language.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Log.h"
@@ -199,6 +200,15 @@ bool operator<(const SourceLoc lhs, const SourceLoc rhs) {
 }
 } // namespace
 
+static void
+ApplyLanguageFilters(llvm::SmallVectorImpl _list) {
+  llvm::erase_if(sc_list, [](SymbolContext ) {
+if (Language *lang = Language::FindPlugin(sc.GetLanguage()))
+  return !lang->IsInterestingCtxForLineBreakpoint(sc);
+return false;
+  });
+}
+
 void BreakpointResolver::SetSCMatchesByLine(
 SearchFilter , SymbolContextList _list, bool skip_prologue,
 llvm::StringRef log_ident, uint32_t line, std::optional column) {
@@ -206,6 +216,8 @@ void BreakpointResolver::SetSCMatchesByLine(
   for (uint32_t i = 0; i < sc_list.GetSize(); ++i)
 all_scs.push_back(sc_list[i]);
 
+  ApplyLanguageFilters(all_scs);
+
   while (all_scs.size()) {
 uint32_t closest_line = UINT32_MAX;
 

>From 7ea76d6c04d063d50da52756179639f2037aa793 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan 
Date: Tue, 5 Mar 2024 18:47:51 -0800
Subject: [PATCH 2/5] fixup! add target option

---
 lldb/include/lldb/Target/Language.h   |  9 ++--
 lldb/include/lldb/Target/Target.h |  2 ++
 lldb/source/Breakpoint/BreakpointResolver.cpp | 23 +--
 lldb/source/Target/Target.cpp |  8 +++
 lldb/source/Target/TargetProperties.td|  3 +++
 5 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/lldb/include/lldb/Target/Language.h 
b/lldb/include/lldb/Target/Language.h
index 957c40eb7c0772..9db9f4e297e114 100644
--- a/lldb/include/lldb/Target/Language.h
+++ b/lldb/include/lldb/Target/Language.h
@@ -339,8 +339,13 @@ class Language : public PluginInterface {
 
   virtual llvm::StringRef GetInstanceVariableName() { return {}; }
 
-  virtual bool IsInterestingCtxForLineBreakpoint(const SymbolContext &) const {
-return true;
+  // Returns true if this SymbolContext should be used when setting breakpoints
+  // by line (number or regex). This is useful for languages that create
+  // artificial functions without any meaningful user code associated with them
+  // (e.g. code that gets expanded in late compilation stages, like by
+  // CoroSplitter).
+  virtual bool IsArtificialCtxForLineBreakpoint(const SymbolContext &) const {
+return false;
   }
 
 protected:
diff --git a/lldb/include/lldb/Target/Target.h 
b/lldb/include/lldb/Target/Target.h
index 8f57358981d4d2..ab04aa57f17300 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -258,6 +258,8 @@ class TargetProperties : public Properties {
 
   bool GetDebugUtilityExpression() const;
 
+  bool GetIgnoreBreakpointsFromLanguageArtificialLocations() const;
+
 private:
   // Callbacks for m_launch_info.
   void Arg0ValueChangedCallback();
diff --git 

[Lldb-commits] [lldb] [lldb] Allow languages to filter breakpoints set by line (PR #83908)

[Felipe de Azevedo Piovezan via lldb-commits]

@@ -339,6 +339,15 @@ class Language : public PluginInterface {
 
   virtual llvm::StringRef GetInstanceVariableName() { return {}; }
 
+  /// Returns true if this SymbolContext should be ignored when setting
+  /// breakpoints by line (number or regex). This is useful for languages that
+  /// create artificial functions without any meaningful user code associated
+  /// with them (e.g. code that gets expanded in late compilation stages, like
+  /// by CoroSplitter).
+  virtual bool IgnoreForLineBreakpoints(const SymbolContext &) const {

felipepiovezan wrote:

brevity: 
This is useful for -> Helpful for
any meaningful -> meaningful

https://github.com/llvm/llvm-project/pull/83908
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Allow languages to filter breakpoints set by line (PR #83908)

[Felipe de Azevedo Piovezan via lldb-commits]

felipepiovezan wrote:

address review comments from Adrian.
I think I have a better name for the target option and language method now

https://github.com/llvm/llvm-project/pull/83908
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Allow languages to filter breakpoints set by line (PR #83908)

[Felipe de Azevedo Piovezan via lldb-commits]

https://github.com/felipepiovezan updated 
https://github.com/llvm/llvm-project/pull/83908

>From 51307b548d09c34ee06037ccf459110e451970a9 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan 
Date: Mon, 4 Mar 2024 09:56:18 -0800
Subject: [PATCH 1/4] [lldb] Allow languages to filter breakpoints set by line

Some languages may create artificial functions that have no real user code, even
though there is line table information for them. One such case is with coroutine
code that receives the CoroSplitter transformation in LLVM IR. That code
transformation creates many different Functions, cloning one Instruction into
many Instructions in many different Functions and copying the associated debug
locations.

It would be difficult to make that pass delete debug locations of cloned
instructions in a language agnostic way (is it even possible?), but LLDB can
ignore certain locations by querying its Language APIs and having it decide
based on, for example, mangling information.
---
 lldb/include/lldb/Target/Language.h   |  4 
 lldb/source/Breakpoint/BreakpointResolver.cpp | 12 
 2 files changed, 16 insertions(+)

diff --git a/lldb/include/lldb/Target/Language.h 
b/lldb/include/lldb/Target/Language.h
index 0cbd8a32dccd54..957c40eb7c0772 100644
--- a/lldb/include/lldb/Target/Language.h
+++ b/lldb/include/lldb/Target/Language.h
@@ -339,6 +339,10 @@ class Language : public PluginInterface {
 
   virtual llvm::StringRef GetInstanceVariableName() { return {}; }
 
+  virtual bool IsInterestingCtxForLineBreakpoint(const SymbolContext &) const {
+return true;
+  }
+
 protected:
   // Classes that inherit from Language can see and modify these
 
diff --git a/lldb/source/Breakpoint/BreakpointResolver.cpp 
b/lldb/source/Breakpoint/BreakpointResolver.cpp
index bc6348716ef418..876b30c6d76d55 100644
--- a/lldb/source/Breakpoint/BreakpointResolver.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolver.cpp
@@ -23,6 +23,7 @@
 #include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/Function.h"
 #include "lldb/Symbol/SymbolContext.h"
+#include "lldb/Target/Language.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Log.h"
@@ -199,6 +200,15 @@ bool operator<(const SourceLoc lhs, const SourceLoc rhs) {
 }
 } // namespace
 
+static void
+ApplyLanguageFilters(llvm::SmallVectorImpl _list) {
+  llvm::erase_if(sc_list, [](SymbolContext ) {
+if (Language *lang = Language::FindPlugin(sc.GetLanguage()))
+  return !lang->IsInterestingCtxForLineBreakpoint(sc);
+return false;
+  });
+}
+
 void BreakpointResolver::SetSCMatchesByLine(
 SearchFilter , SymbolContextList _list, bool skip_prologue,
 llvm::StringRef log_ident, uint32_t line, std::optional column) {
@@ -206,6 +216,8 @@ void BreakpointResolver::SetSCMatchesByLine(
   for (uint32_t i = 0; i < sc_list.GetSize(); ++i)
 all_scs.push_back(sc_list[i]);
 
+  ApplyLanguageFilters(all_scs);
+
   while (all_scs.size()) {
 uint32_t closest_line = UINT32_MAX;
 

>From 7ea76d6c04d063d50da52756179639f2037aa793 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan 
Date: Tue, 5 Mar 2024 18:47:51 -0800
Subject: [PATCH 2/4] fixup! add target option

---
 lldb/include/lldb/Target/Language.h   |  9 ++--
 lldb/include/lldb/Target/Target.h |  2 ++
 lldb/source/Breakpoint/BreakpointResolver.cpp | 23 +--
 lldb/source/Target/Target.cpp |  8 +++
 lldb/source/Target/TargetProperties.td|  3 +++
 5 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/lldb/include/lldb/Target/Language.h 
b/lldb/include/lldb/Target/Language.h
index 957c40eb7c0772..9db9f4e297e114 100644
--- a/lldb/include/lldb/Target/Language.h
+++ b/lldb/include/lldb/Target/Language.h
@@ -339,8 +339,13 @@ class Language : public PluginInterface {
 
   virtual llvm::StringRef GetInstanceVariableName() { return {}; }
 
-  virtual bool IsInterestingCtxForLineBreakpoint(const SymbolContext &) const {
-return true;
+  // Returns true if this SymbolContext should be used when setting breakpoints
+  // by line (number or regex). This is useful for languages that create
+  // artificial functions without any meaningful user code associated with them
+  // (e.g. code that gets expanded in late compilation stages, like by
+  // CoroSplitter).
+  virtual bool IsArtificialCtxForLineBreakpoint(const SymbolContext &) const {
+return false;
   }
 
 protected:
diff --git a/lldb/include/lldb/Target/Target.h 
b/lldb/include/lldb/Target/Target.h
index 8f57358981d4d2..ab04aa57f17300 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -258,6 +258,8 @@ class TargetProperties : public Properties {
 
   bool GetDebugUtilityExpression() const;
 
+  bool GetIgnoreBreakpointsFromLanguageArtificialLocations() const;
+
 private:
   // Callbacks for m_launch_info.
   void Arg0ValueChangedCallback();
diff --git 

[Lldb-commits] [lldb] [lldb] Allow languages to filter breakpoints set by line (PR #83908)

[Felipe de Azevedo Piovezan via lldb-commits]

felipepiovezan wrote:

I believe I've addressed all the feedback here.
Tried really hard to come up with a shorter name for that target option, but 
could not find something both short and descriptive. If you can think of 
anything better, please let me know

https://github.com/llvm/llvm-project/pull/83908
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Allow languages to filter breakpoints set by line (PR #83908)

[Felipe de Azevedo Piovezan via lldb-commits]

https://github.com/felipepiovezan updated 
https://github.com/llvm/llvm-project/pull/83908

>From 51307b548d09c34ee06037ccf459110e451970a9 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan 
Date: Mon, 4 Mar 2024 09:56:18 -0800
Subject: [PATCH 1/2] [lldb] Allow languages to filter breakpoints set by line

Some languages may create artificial functions that have no real user code, even
though there is line table information for them. One such case is with coroutine
code that receives the CoroSplitter transformation in LLVM IR. That code
transformation creates many different Functions, cloning one Instruction into
many Instructions in many different Functions and copying the associated debug
locations.

It would be difficult to make that pass delete debug locations of cloned
instructions in a language agnostic way (is it even possible?), but LLDB can
ignore certain locations by querying its Language APIs and having it decide
based on, for example, mangling information.
---
 lldb/include/lldb/Target/Language.h   |  4 
 lldb/source/Breakpoint/BreakpointResolver.cpp | 12 
 2 files changed, 16 insertions(+)

diff --git a/lldb/include/lldb/Target/Language.h 
b/lldb/include/lldb/Target/Language.h
index 0cbd8a32dccd54..957c40eb7c0772 100644
--- a/lldb/include/lldb/Target/Language.h
+++ b/lldb/include/lldb/Target/Language.h
@@ -339,6 +339,10 @@ class Language : public PluginInterface {
 
   virtual llvm::StringRef GetInstanceVariableName() { return {}; }
 
+  virtual bool IsInterestingCtxForLineBreakpoint(const SymbolContext &) const {
+return true;
+  }
+
 protected:
   // Classes that inherit from Language can see and modify these
 
diff --git a/lldb/source/Breakpoint/BreakpointResolver.cpp 
b/lldb/source/Breakpoint/BreakpointResolver.cpp
index bc6348716ef418..876b30c6d76d55 100644
--- a/lldb/source/Breakpoint/BreakpointResolver.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolver.cpp
@@ -23,6 +23,7 @@
 #include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/Function.h"
 #include "lldb/Symbol/SymbolContext.h"
+#include "lldb/Target/Language.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Log.h"
@@ -199,6 +200,15 @@ bool operator<(const SourceLoc lhs, const SourceLoc rhs) {
 }
 } // namespace
 
+static void
+ApplyLanguageFilters(llvm::SmallVectorImpl _list) {
+  llvm::erase_if(sc_list, [](SymbolContext ) {
+if (Language *lang = Language::FindPlugin(sc.GetLanguage()))
+  return !lang->IsInterestingCtxForLineBreakpoint(sc);
+return false;
+  });
+}
+
 void BreakpointResolver::SetSCMatchesByLine(
 SearchFilter , SymbolContextList _list, bool skip_prologue,
 llvm::StringRef log_ident, uint32_t line, std::optional column) {
@@ -206,6 +216,8 @@ void BreakpointResolver::SetSCMatchesByLine(
   for (uint32_t i = 0; i < sc_list.GetSize(); ++i)
 all_scs.push_back(sc_list[i]);
 
+  ApplyLanguageFilters(all_scs);
+
   while (all_scs.size()) {
 uint32_t closest_line = UINT32_MAX;
 

>From 7ea76d6c04d063d50da52756179639f2037aa793 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan 
Date: Tue, 5 Mar 2024 18:47:51 -0800
Subject: [PATCH 2/2] fixup! add target option

---
 lldb/include/lldb/Target/Language.h   |  9 ++--
 lldb/include/lldb/Target/Target.h |  2 ++
 lldb/source/Breakpoint/BreakpointResolver.cpp | 23 +--
 lldb/source/Target/Target.cpp |  8 +++
 lldb/source/Target/TargetProperties.td|  3 +++
 5 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/lldb/include/lldb/Target/Language.h 
b/lldb/include/lldb/Target/Language.h
index 957c40eb7c0772..9db9f4e297e114 100644
--- a/lldb/include/lldb/Target/Language.h
+++ b/lldb/include/lldb/Target/Language.h
@@ -339,8 +339,13 @@ class Language : public PluginInterface {
 
   virtual llvm::StringRef GetInstanceVariableName() { return {}; }
 
-  virtual bool IsInterestingCtxForLineBreakpoint(const SymbolContext &) const {
-return true;
+  // Returns true if this SymbolContext should be used when setting breakpoints
+  // by line (number or regex). This is useful for languages that create
+  // artificial functions without any meaningful user code associated with them
+  // (e.g. code that gets expanded in late compilation stages, like by
+  // CoroSplitter).
+  virtual bool IsArtificialCtxForLineBreakpoint(const SymbolContext &) const {
+return false;
   }
 
 protected:
diff --git a/lldb/include/lldb/Target/Target.h 
b/lldb/include/lldb/Target/Target.h
index 8f57358981d4d2..ab04aa57f17300 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -258,6 +258,8 @@ class TargetProperties : public Properties {
 
   bool GetDebugUtilityExpression() const;
 
+  bool GetIgnoreBreakpointsFromLanguageArtificialLocations() const;
+
 private:
   // Callbacks for m_launch_info.
   void Arg0ValueChangedCallback();
diff --git 

[Lldb-commits] [lldb] [lldb] Allow languages to filter breakpoints set by line (PR #83908)

[Felipe de Azevedo Piovezan via lldb-commits]

@@ -199,13 +200,24 @@ bool operator<(const SourceLoc lhs, const SourceLoc rhs) {
 }
 } // namespace
 
+static void
+ApplyLanguageFilters(llvm::SmallVectorImpl _list) {
+  llvm::erase_if(sc_list, [](SymbolContext ) {
+if (Language *lang = Language::FindPlugin(sc.GetLanguage()))
+  return !lang->IsInterestingCtxForLineBreakpoint(sc);
+return false;
+  });
+}
+
 void BreakpointResolver::SetSCMatchesByLine(
 SearchFilter , SymbolContextList _list, bool skip_prologue,
 llvm::StringRef log_ident, uint32_t line, std::optional column) {
   llvm::SmallVector all_scs;
   for (uint32_t i = 0; i < sc_list.GetSize(); ++i)
 all_scs.push_back(sc_list[i]);
 
+  ApplyLanguageFilters(all_scs);

felipepiovezan wrote:

Yup, I'll change that in the next iteration of this patch (it was uploaded 
before Adrian's comment)

https://github.com/llvm/llvm-project/pull/83908
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Allow languages to filter breakpoints set by line (PR #83908)

[via lldb-commits]

jimingham wrote:

We don't do the latter anywhere.  This is really equivalent to the: 
target.breakpoints-use-platform-avoid-list setting, which is also about "stop 
at places most normal human beings don't want to stop".  In the extant setting 
it's about whole libraries, the new setting would be about adding breakpoints 
on artificial line table entries or something like that.

Jim
 

> On Mar 4, 2024, at 3:45 PM, Felipe de Azevedo Piovezan ***@***.***> wrote:
> 
> 
> Also, are these 100% uninteresting line table entries? At present, there's no 
> way to get lldb to not filter them out, so they either really need to be 
> totally uninteresting, or we need a setting to turn them back on again.
> 
> They should be 100% uninteresting, but I agree that it would be nice to have 
> a mechanism allowing these to be re-enabled.
> Do you think it is better to have a setting for this, or could we perhaps add 
> the breakpoint but keep it disabled by default (is there precedent for this?) 
> ?
> 
> —
> Reply to this email directly, view it on GitHub 
> , or 
> unsubscribe 
> .
> You are receiving this because your review was requested.
> 



https://github.com/llvm/llvm-project/pull/83908
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Allow languages to filter breakpoints set by line (PR #83908)

[Felipe de Azevedo Piovezan via lldb-commits]

felipepiovezan wrote:

> Also, are these 100% uninteresting line table entries? At present, there's no 
> way to get lldb to not filter them out, so they either really need to be 
> totally uninteresting, or we need a setting to turn them back on again.

They should be 100% uninteresting, but I agree that it would be nice to have a 
mechanism allowing these to be re-enabled.
Do you think it is better to have a setting for this, or could we perhaps add 
the breakpoint but keep it disabled by default (is there precedent for this?) ?

https://github.com/llvm/llvm-project/pull/83908
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Allow languages to filter breakpoints set by line (PR #83908)

[via lldb-commits]

jimingham wrote:

LGTM except you should have a little bit of doc describing what "interesting" 
means.  On the face of it, this seems a little silly.  Why would the compiler 
emit "uninteresting" line table entries?  Your example motivation the PR makes 
that clear, that or something like in the docs would help motivate this.


Also, are these 100% uninteresting line table entries?  At present, there's no 
way to get lldb to not filter them out, so they either really need to be 
totally uninteresting, or we need a setting to turn them back on again.

https://github.com/llvm/llvm-project/pull/83908
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Allow languages to filter breakpoints set by line (PR #83908)

[via lldb-commits]

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Felipe de Azevedo Piovezan (felipepiovezan)


Changes

Some languages may create artificial functions that have no real user code, 
even though there is line table information for them. One such case is with 
coroutine code that receives the CoroSplitter transformation in LLVM IR. That 
code transformation creates many different Functions, cloning one Instruction 
into many Instructions in many different Functions and copying the associated 
debug locations.

It would be difficult to make that pass delete debug locations of cloned 
instructions in a language agnostic way (is it even possible?), but LLDB can 
ignore certain locations by querying its Language APIs and having it decide 
based on, for example, mangling information.

---
Full diff: https://github.com/llvm/llvm-project/pull/83908.diff


2 Files Affected:

- (modified) lldb/include/lldb/Target/Language.h (+4) 
- (modified) lldb/source/Breakpoint/BreakpointResolver.cpp (+12) 


``diff
diff --git a/lldb/include/lldb/Target/Language.h 
b/lldb/include/lldb/Target/Language.h
index 0cbd8a32dccd54..957c40eb7c0772 100644
--- a/lldb/include/lldb/Target/Language.h
+++ b/lldb/include/lldb/Target/Language.h
@@ -339,6 +339,10 @@ class Language : public PluginInterface {
 
   virtual llvm::StringRef GetInstanceVariableName() { return {}; }
 
+  virtual bool IsInterestingCtxForLineBreakpoint(const SymbolContext &) const {
+return true;
+  }
+
 protected:
   // Classes that inherit from Language can see and modify these
 
diff --git a/lldb/source/Breakpoint/BreakpointResolver.cpp 
b/lldb/source/Breakpoint/BreakpointResolver.cpp
index bc6348716ef418..876b30c6d76d55 100644
--- a/lldb/source/Breakpoint/BreakpointResolver.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolver.cpp
@@ -23,6 +23,7 @@
 #include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/Function.h"
 #include "lldb/Symbol/SymbolContext.h"
+#include "lldb/Target/Language.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Log.h"
@@ -199,6 +200,15 @@ bool operator<(const SourceLoc lhs, const SourceLoc rhs) {
 }
 } // namespace
 
+static void
+ApplyLanguageFilters(llvm::SmallVectorImpl _list) {
+  llvm::erase_if(sc_list, [](SymbolContext ) {
+if (Language *lang = Language::FindPlugin(sc.GetLanguage()))
+  return !lang->IsInterestingCtxForLineBreakpoint(sc);
+return false;
+  });
+}
+
 void BreakpointResolver::SetSCMatchesByLine(
 SearchFilter , SymbolContextList _list, bool skip_prologue,
 llvm::StringRef log_ident, uint32_t line, std::optional column) {
@@ -206,6 +216,8 @@ void BreakpointResolver::SetSCMatchesByLine(
   for (uint32_t i = 0; i < sc_list.GetSize(); ++i)
 all_scs.push_back(sc_list[i]);
 
+  ApplyLanguageFilters(all_scs);
+
   while (all_scs.size()) {
 uint32_t closest_line = UINT32_MAX;
 

``




https://github.com/llvm/llvm-project/pull/83908
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Allow languages to filter breakpoints set by line (PR #83908)

[Felipe de Azevedo Piovezan via lldb-commits]

https://github.com/felipepiovezan created 
https://github.com/llvm/llvm-project/pull/83908

Some languages may create artificial functions that have no real user code, 
even though there is line table information for them. One such case is with 
coroutine code that receives the CoroSplitter transformation in LLVM IR. That 
code transformation creates many different Functions, cloning one Instruction 
into many Instructions in many different Functions and copying the associated 
debug locations.

It would be difficult to make that pass delete debug locations of cloned 
instructions in a language agnostic way (is it even possible?), but LLDB can 
ignore certain locations by querying its Language APIs and having it decide 
based on, for example, mangling information.

>From 32ed57079eec7d33a87883e192cd928caa2f5f6d Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan 
Date: Mon, 4 Mar 2024 09:56:18 -0800
Subject: [PATCH] [lldb] Allow languages to filter breakpoints set by line

Some languages may create artificial functions that have no real user code, even
though there is line table information for them. One such case is with coroutine
code that receives the CoroSplitter transformation in LLVM IR. That code
transformation creates many different Functions, cloning one Instruction into
many Instructions in many different Functions and copying the associated debug
locations.

It would be difficult to make that pass delete debug locations of cloned
instructions in a language agnostic way (is it even possible?), but LLDB can
ignore certain locations by querying its Language APIs and having it decide
based on, for example, mangling information.
---
 lldb/include/lldb/Target/Language.h   |  4 
 lldb/source/Breakpoint/BreakpointResolver.cpp | 12 
 2 files changed, 16 insertions(+)

diff --git a/lldb/include/lldb/Target/Language.h 
b/lldb/include/lldb/Target/Language.h
index 0cbd8a32dccd54..957c40eb7c0772 100644
--- a/lldb/include/lldb/Target/Language.h
+++ b/lldb/include/lldb/Target/Language.h
@@ -339,6 +339,10 @@ class Language : public PluginInterface {
 
   virtual llvm::StringRef GetInstanceVariableName() { return {}; }
 
+  virtual bool IsInterestingCtxForLineBreakpoint(const SymbolContext &) const {
+return true;
+  }
+
 protected:
   // Classes that inherit from Language can see and modify these
 
diff --git a/lldb/source/Breakpoint/BreakpointResolver.cpp 
b/lldb/source/Breakpoint/BreakpointResolver.cpp
index bc6348716ef418..876b30c6d76d55 100644
--- a/lldb/source/Breakpoint/BreakpointResolver.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolver.cpp
@@ -23,6 +23,7 @@
 #include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/Function.h"
 #include "lldb/Symbol/SymbolContext.h"
+#include "lldb/Target/Language.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Log.h"
@@ -199,6 +200,15 @@ bool operator<(const SourceLoc lhs, const SourceLoc rhs) {
 }
 } // namespace
 
+static void
+ApplyLanguageFilters(llvm::SmallVectorImpl _list) {
+  llvm::erase_if(sc_list, [](SymbolContext ) {
+if (Language *lang = Language::FindPlugin(sc.GetLanguage()))
+  return !lang->IsInterestingCtxForLineBreakpoint(sc);
+return false;
+  });
+}
+
 void BreakpointResolver::SetSCMatchesByLine(
 SearchFilter , SymbolContextList _list, bool skip_prologue,
 llvm::StringRef log_ident, uint32_t line, std::optional column) {
@@ -206,6 +216,8 @@ void BreakpointResolver::SetSCMatchesByLine(
   for (uint32_t i = 0; i < sc_list.GetSize(); ++i)
 all_scs.push_back(sc_list[i]);
 
+  ApplyLanguageFilters(all_scs);
+
   while (all_scs.size()) {
 uint32_t closest_line = UINT32_MAX;
 

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