date:20230206

[Lldb-commits] [PATCH] D143398: [WIP][lldb][DWARFASTParserClang] Correctly resolve imported namespaces during expression evaluation

2023-02-06 Thread Michael Buch via Phabricator via lldb-commits

Michael137 created this revision.
Michael137 added reviewers: aprantl, labath.
Herald added a reviewer: shafik.
Herald added a project: All.
Michael137 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

**Summary**

This patch makes the expression evaluator understand
namespace aliases.

This will become important once `std::ranges` become
more widespread since `std::views` is defined as:

  namespace std {
  namespace ranges::views {}
  
  namespace views = ranges::views;
  }

**Testing**

- Added API test


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D143398

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  lldb/test/API/commands/expression/namespace-alias/Makefile
  lldb/test/API/commands/expression/namespace-alias/TestInlineNamespaceAlias.py
  lldb/test/API/commands/expression/namespace-alias/main.cpp

Index: lldb/test/API/commands/expression/namespace-alias/main.cpp
===
--- /dev/null
+++ lldb/test/API/commands/expression/namespace-alias/main.cpp
@@ -0,0 +1,21 @@
+namespace A {
+inline namespace _A {
+namespace B {
+namespace C {
+int a = -1;
+
+int func() { return 0; }
+} // namespace C
+} // namespace B
+
+namespace C = B::C;
+namespace D = B::C;
+
+} // namespace _A
+} // namespace A
+
+namespace E = A;
+namespace F = E::C;
+namespace G = F;
+
+int main(int argc, char **argv) { return A::B::C::a; }
Index: lldb/test/API/commands/expression/namespace-alias/TestInlineNamespaceAlias.py
===
--- /dev/null
+++ lldb/test/API/commands/expression/namespace-alias/TestInlineNamespaceAlias.py
@@ -0,0 +1,29 @@
+"""
+Test that we correctly handle namespace
+expression evaluation through namespace
+aliases.
+"""
+
+import lldb
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestInlineNamespace(TestBase):
+def test(self):
+self.build()
+
+lldbutil.run_to_source_breakpoint(self,
+  "return A::B::C::a", lldb.SBFileSpec("main.cpp"))
+
+self.expect_expr("A::C::a", result_type="int", result_value="-1")
+self.expect_expr("A::D::a", result_type="int", result_value="-1")
+
+self.expect_expr("A::C::func()", result_type="int", result_value="0")
+self.expect_expr("A::D::func()", result_type="int", result_value="0")
+
+self.expect_expr("E::C::a", result_type="int", result_value="-1")
+self.expect_expr("E::D::a", result_type="int", result_value="-1")
+self.expect_expr("F::a", result_type="int", result_value="-1")
+self.expect_expr("G::a", result_type="int", result_value="-1")
Index: lldb/test/API/commands/expression/namespace-alias/Makefile
===
--- /dev/null
+++ lldb/test/API/commands/expression/namespace-alias/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -132,6 +132,17 @@
 
   clang::NamespaceDecl *ResolveNamespaceDIE(const DWARFDIE &die);
 
+  /// Returns the namespace decl that a DW_TAG_imported_declaration imports.
+  ///
+  /// \param[in] die The import declaration to resolve. If the DIE is not a
+  ///DW_TAG_imported_declaration the behaviour is undefined.
+  ///
+  /// \returns The decl corresponding to the namespace that the specified
+  ///  'die' imports. If the imported entity is not a namespace
+  ///  or another import declaration, returns nullptr. If an error
+  ///  occurs, returns nullptr.
+  clang::NamespaceDecl *ResolveImportedDeclarationDIE(const DWARFDIE &die);
+
   bool ParseTemplateDIE(const DWARFDIE &die,
 lldb_private::TypeSystemClang::TemplateParameterInfos
 &template_param_infos);
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -3304,6 +3304,11 @@
   try_parsing_type = false;
   break;
 
+case DW_TAG_imported_declaration:
+  decl_ctx = ResolveImportedDeclarationDIE(die);
+  try_parsing_type = false;
+  break;
+
 case DW_TAG_lexical_block:
   decl_ctx = GetDeclContextForBlock(die);
   try_parsing_type = false;
@@ -3465,6 +3470,41 @@
   return nullptr;
 }
 
+clang::NamespaceDecl *
+DWARFASTParserClang::ResolveImportedDeclarationD

[Lldb-commits] [lldb] 0b8eff1 - [LLDB][NFC] Fix a incorrect use of shared_ptr in RenderScriptRuntime.cpp

2023-02-06 Thread Shivam Gupta via lldb-commits


Author: Shivam Gupta
Date: 2023-02-06T22:07:43+05:30
New Revision: 0b8eff1f8724c6d8e890227597060109cb55e1ca

URL: 
https://github.com/llvm/llvm-project/commit/0b8eff1f8724c6d8e890227597060109cb55e1ca
DIFF: 
https://github.com/llvm/llvm-project/commit/0b8eff1f8724c6d8e890227597060109cb55e1ca.diff

LOG: [LLDB][NFC] Fix a incorrect use of shared_ptr in RenderScriptRuntime.cpp

Incorrect use of shared_ptr.

found by PVS-Studio https://pvs-studio.com/en/blog/posts/cpp/1003/, N8 & N9.

Differential Revision: https://reviews.llvm.org/D142309

Added: 


Modified: 

lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp

lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h

Removed: 




diff  --git 
a/lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
 
b/lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
index e168241fb2c97..cc43ecb901038 100644
--- 
a/lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
+++ 
b/lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
@@ -2346,7 +2346,7 @@ void RenderScriptRuntime::SetElementSize(Element &elem) {
 // Given an allocation, this function copies the allocation contents from
 // device into a buffer on the heap. Returning a shared pointer to the buffer
 // containing the data.
-std::shared_ptr
+std::shared_ptr
 RenderScriptRuntime::GetAllocationData(AllocationDetails *alloc,
StackFrame *frame_ptr) {
   Log *log = GetLog(LLDBLog::Language);
@@ -2368,7 +2368,7 @@ RenderScriptRuntime::GetAllocationData(AllocationDetails 
*alloc,
 
   // Allocate a buffer to copy data into
   const uint32_t size = *alloc->size.get();
-  std::shared_ptr buffer(new uint8_t[size]);
+  std::shared_ptr buffer(new uint8_t[size]);
   if (!buffer) {
 LLDB_LOGF(log, "%s - couldn't allocate a %" PRIu32 " byte buffer",
   __FUNCTION__, size);
@@ -2557,7 +2557,7 @@ bool RenderScriptRuntime::LoadAllocation(Stream &strm, 
const uint32_t alloc_id,
 // saved to the file as the ElementHeader struct followed by offsets to the
 // structs of all the element's children.
 size_t RenderScriptRuntime::PopulateElementHeaders(
-const std::shared_ptr header_buffer, size_t offset,
+const std::shared_ptr header_buffer, size_t offset,
 const Element &elem) {
   // File struct for an element header with all the relevant details copied
   // from elem. We assume members are valid already.
@@ -2661,7 +2661,7 @@ bool RenderScriptRuntime::SaveAllocation(Stream &strm, 
const uint32_t alloc_id,
   }
 
   // Read allocation into buffer of heap memory
-  const std::shared_ptr buffer = GetAllocationData(alloc, frame_ptr);
+  const std::shared_ptr buffer = GetAllocationData(alloc, 
frame_ptr);
   if (!buffer) {
 strm.Printf("Error: Couldn't read allocation data into buffer");
 strm.EOL();
@@ -2695,7 +2695,7 @@ bool RenderScriptRuntime::SaveAllocation(Stream &strm, 
const uint32_t alloc_id,
   }
 
   // Create the headers describing the element type of the allocation.
-  std::shared_ptr element_header_buffer(
+  std::shared_ptr element_header_buffer(
   new uint8_t[element_header_size]);
   if (element_header_buffer == nullptr) {
 strm.Printf("Internal Error: Couldn't allocate %" PRIu64
@@ -3214,7 +3214,7 @@ bool RenderScriptRuntime::DumpAllocation(Stream &strm, 
StackFrame *frame_ptr,
 __FUNCTION__, data_size);
 
   // Allocate a buffer to copy data into
-  std::shared_ptr buffer = GetAllocationData(alloc, frame_ptr);
+  std::shared_ptr buffer = GetAllocationData(alloc, frame_ptr);
   if (!buffer) {
 strm.Printf("Error: Couldn't read allocation data");
 strm.EOL();

diff  --git 
a/lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h
 
b/lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h
index bc460706fd295..336058ede9b96 100644
--- 
a/lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h
+++ 
b/lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h
@@ -528,7 +528,7 @@ class RenderScriptRuntime : public 
lldb_private::CPPLanguageRuntime {
 
   AllocationDetails *FindAllocByID(Stream &strm, const uint32_t alloc_id);
 
-  std::shared_ptr GetAllocationData(AllocationDetails *alloc,
+  std::shared_ptr GetAllocationData(AllocationDetails *alloc,
  StackFrame *frame_ptr);
 
   void SetElementSize(Element &elem);
@@ -538,7 +538,7 @@ class RenderScriptRuntime : public 
lldb_private::CPPLanguageRuntime {
 
   void FindStructTypeName(Element &elem, StackFrame *frame_ptr);
 
-  size_t PopulateElementHeaders(const std::sh

[Lldb-commits] [PATCH] D142309: [LLDB][NFC] Fix a incorrect use of shared_ptr in RenderScriptRuntime.cpp

2023-02-06 Thread Shivam Gupta via Phabricator via lldb-commits

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0b8eff1f8724: [LLDB][NFC] Fix a incorrect use of shared_ptr 
in RenderScriptRuntime.cpp (authored by xgupta).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142309

Files:
  
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
  
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h


Index: 
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h
===
--- 
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h
+++ 
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h
@@ -528,7 +528,7 @@
 
   AllocationDetails *FindAllocByID(Stream &strm, const uint32_t alloc_id);
 
-  std::shared_ptr GetAllocationData(AllocationDetails *alloc,
+  std::shared_ptr GetAllocationData(AllocationDetails *alloc,
  StackFrame *frame_ptr);
 
   void SetElementSize(Element &elem);
@@ -538,7 +538,7 @@
 
   void FindStructTypeName(Element &elem, StackFrame *frame_ptr);
 
-  size_t PopulateElementHeaders(const std::shared_ptr header_buffer,
+  size_t PopulateElementHeaders(const std::shared_ptr 
header_buffer,
 size_t offset, const Element &elem);
 
   size_t CalculateElementHeaderSize(const Element &elem);
Index: 
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
===
--- 
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
+++ 
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
@@ -2346,7 +2346,7 @@
 // Given an allocation, this function copies the allocation contents from
 // device into a buffer on the heap. Returning a shared pointer to the buffer
 // containing the data.
-std::shared_ptr
+std::shared_ptr
 RenderScriptRuntime::GetAllocationData(AllocationDetails *alloc,
StackFrame *frame_ptr) {
   Log *log = GetLog(LLDBLog::Language);
@@ -2368,7 +2368,7 @@
 
   // Allocate a buffer to copy data into
   const uint32_t size = *alloc->size.get();
-  std::shared_ptr buffer(new uint8_t[size]);
+  std::shared_ptr buffer(new uint8_t[size]);
   if (!buffer) {
 LLDB_LOGF(log, "%s - couldn't allocate a %" PRIu32 " byte buffer",
   __FUNCTION__, size);
@@ -2557,7 +2557,7 @@
 // saved to the file as the ElementHeader struct followed by offsets to the
 // structs of all the element's children.
 size_t RenderScriptRuntime::PopulateElementHeaders(
-const std::shared_ptr header_buffer, size_t offset,
+const std::shared_ptr header_buffer, size_t offset,
 const Element &elem) {
   // File struct for an element header with all the relevant details copied
   // from elem. We assume members are valid already.
@@ -2661,7 +2661,7 @@
   }
 
   // Read allocation into buffer of heap memory
-  const std::shared_ptr buffer = GetAllocationData(alloc, frame_ptr);
+  const std::shared_ptr buffer = GetAllocationData(alloc, 
frame_ptr);
   if (!buffer) {
 strm.Printf("Error: Couldn't read allocation data into buffer");
 strm.EOL();
@@ -2695,7 +2695,7 @@
   }
 
   // Create the headers describing the element type of the allocation.
-  std::shared_ptr element_header_buffer(
+  std::shared_ptr element_header_buffer(
   new uint8_t[element_header_size]);
   if (element_header_buffer == nullptr) {
 strm.Printf("Internal Error: Couldn't allocate %" PRIu64
@@ -3214,7 +3214,7 @@
 __FUNCTION__, data_size);
 
   // Allocate a buffer to copy data into
-  std::shared_ptr buffer = GetAllocationData(alloc, frame_ptr);
+  std::shared_ptr buffer = GetAllocationData(alloc, frame_ptr);
   if (!buffer) {
 strm.Printf("Error: Couldn't read allocation data");
 strm.EOL();


Index: lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h
===
--- lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h
+++ lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.h
@@ -528,7 +528,7 @@
 
   AllocationDetails *FindAllocByID(Stream &strm, const uint32_t alloc_id);
 
-  std::shared_ptr GetAllocationData(AllocationDetails *alloc,
+  std::shared_ptr GetAllocationData(AllocationDetails *alloc,
  StackFrame *frame_ptr);
 
   void SetElementSize(Element &elem);
@@ -538,7 +538,7 @@
 
   void FindStructTypeName(Element &elem, S

[Lldb-commits] [PATCH] D142309: [LLDB][NFC] Fix a incorrect use of shared_ptr in RenderScriptRuntime.cpp

2023-02-06 Thread Shivam Gupta via Phabricator via lldb-commits

xgupta added a comment.

In D142309#4102284 , @labath wrote:

> In D142309#4075745 , @JDevlieghere 
> wrote:
>
>> LGTM, but I wonder when we'll be able to get rid of RenderScript completely 
>> (CC @labath)
>
> I am not going to stand in your way. I usually redirect these questions to 
> @srhines, but I understand he's moved on to other things as well.

Thanks, no problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142309

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

[Lldb-commits] [PATCH] D142309: [LLDB][NFC] Fix a incorrect use of shared_ptr in RenderScriptRuntime.cpp

2023-02-06 Thread Stephen Hines via Phabricator via lldb-commits

srhines added a comment.

In D142309#4107170 , @xgupta wrote:

> In D142309#4102284 , @labath wrote:
>
>> In D142309#4075745 , @JDevlieghere 
>> wrote:
>>
>>> LGTM, but I wonder when we'll be able to get rid of RenderScript completely 
>>> (CC @labath)
>>
>> I am not going to stand in your way. I usually redirect these questions to 
>> @srhines, but I understand he's moved on to other things as well.
>
> Thanks, no problem.

RenderScript support can be removed. I don't necessarily have the bandwidth to 
do it immediately, but if someone wants to clean that up, I'd help with 
reviewing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142309

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

[Lldb-commits] [PATCH] D143308: [lldb/Plugins] Fix method dispatch bug when using multiple scripted processes

2023-02-06 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

Makes sense. LGTM.


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

https://reviews.llvm.org/D143308

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

[Lldb-commits] [PATCH] D141658: [lldb/crashlog] Make interactive mode the new default

2023-02-06 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added inline comments.



Comment at: lldb/examples/python/crashlog.py:1148
+class CrashLogLoadingMode(str, enum.Enum):
+legacy = 'legacy'
+interactive = 'interactive'

This can be interpreted as the textual crashlogs going away which isn't the 
case or the goal. I think this should say "textual" (although that might be 
confusing with the JSON/text input) or maybe "batch".


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

https://reviews.llvm.org/D141658

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

[Lldb-commits] [PATCH] D143398: [WIP][lldb][DWARFASTParserClang] Correctly resolve imported namespaces during expression evaluation

2023-02-06 Thread Michael Buch via Phabricator via lldb-commits

Michael137 updated this revision to Diff 495230.
Michael137 added a comment.

- Reword commit message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143398

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  lldb/test/API/commands/expression/namespace-alias/Makefile
  lldb/test/API/commands/expression/namespace-alias/TestInlineNamespaceAlias.py
  lldb/test/API/commands/expression/namespace-alias/main.cpp

Index: lldb/test/API/commands/expression/namespace-alias/main.cpp
===
--- /dev/null
+++ lldb/test/API/commands/expression/namespace-alias/main.cpp
@@ -0,0 +1,21 @@
+namespace A {
+inline namespace _A {
+namespace B {
+namespace C {
+int a = -1;
+
+int func() { return 0; }
+} // namespace C
+} // namespace B
+
+namespace C = B::C;
+namespace D = B::C;
+
+} // namespace _A
+} // namespace A
+
+namespace E = A;
+namespace F = E::C;
+namespace G = F;
+
+int main(int argc, char **argv) { return A::B::C::a; }
Index: lldb/test/API/commands/expression/namespace-alias/TestInlineNamespaceAlias.py
===
--- /dev/null
+++ lldb/test/API/commands/expression/namespace-alias/TestInlineNamespaceAlias.py
@@ -0,0 +1,29 @@
+"""
+Test that we correctly handle namespace
+expression evaluation through namespace
+aliases.
+"""
+
+import lldb
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestInlineNamespace(TestBase):
+def test(self):
+self.build()
+
+lldbutil.run_to_source_breakpoint(self,
+  "return A::B::C::a", lldb.SBFileSpec("main.cpp"))
+
+self.expect_expr("A::C::a", result_type="int", result_value="-1")
+self.expect_expr("A::D::a", result_type="int", result_value="-1")
+
+self.expect_expr("A::C::func()", result_type="int", result_value="0")
+self.expect_expr("A::D::func()", result_type="int", result_value="0")
+
+self.expect_expr("E::C::a", result_type="int", result_value="-1")
+self.expect_expr("E::D::a", result_type="int", result_value="-1")
+self.expect_expr("F::a", result_type="int", result_value="-1")
+self.expect_expr("G::a", result_type="int", result_value="-1")
Index: lldb/test/API/commands/expression/namespace-alias/Makefile
===
--- /dev/null
+++ lldb/test/API/commands/expression/namespace-alias/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -132,6 +132,17 @@
 
   clang::NamespaceDecl *ResolveNamespaceDIE(const DWARFDIE &die);
 
+  /// Returns the namespace decl that a DW_TAG_imported_declaration imports.
+  ///
+  /// \param[in] die The import declaration to resolve. If the DIE is not a
+  ///DW_TAG_imported_declaration the behaviour is undefined.
+  ///
+  /// \returns The decl corresponding to the namespace that the specified
+  ///  'die' imports. If the imported entity is not a namespace
+  ///  or another import declaration, returns nullptr. If an error
+  ///  occurs, returns nullptr.
+  clang::NamespaceDecl *ResolveImportedDeclarationDIE(const DWARFDIE &die);
+
   bool ParseTemplateDIE(const DWARFDIE &die,
 lldb_private::TypeSystemClang::TemplateParameterInfos
 &template_param_infos);
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -3304,6 +3304,11 @@
   try_parsing_type = false;
   break;
 
+case DW_TAG_imported_declaration:
+  decl_ctx = ResolveImportedDeclarationDIE(die);
+  try_parsing_type = false;
+  break;
+
 case DW_TAG_lexical_block:
   decl_ctx = GetDeclContextForBlock(die);
   try_parsing_type = false;
@@ -3465,6 +3470,41 @@
   return nullptr;
 }
 
+clang::NamespaceDecl *
+DWARFASTParserClang::ResolveImportedDeclarationDIE(const DWARFDIE &die) {
+  assert(die && die.Tag() == DW_TAG_imported_declaration);
+
+  // See if we cached a NamespaceDecl for this imported declaration
+  // already
+  clang::NamespaceDecl *namespace_decl =
+  static_cast(m_die_to_decl_ctx[die.GetDIE()]);
+  if (namespace_decl)
+return namespace_decl;
+
+  const DWARFDIE imported_uid =
+  die.GetAttributeValueAsReferenceDIE(DW_AT

[Lldb-commits] [PATCH] D143347: [lldb][DWARF] Infer no_unique_address attribute

2023-02-06 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

Changes look fine to me. I would like someone that specializes in the 
expression parser to give the final ok though.




Comment at: lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp:551
 const LayoutInfo &layout) {
+
   m_record_decl_to_layout_map.insert(std::make_pair(decl, layout));

revert whitespace only changes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143347

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

[Lldb-commits] [PATCH] D143232: Return an error when the CFA resolves to no known register, instead of segfaulting

2023-02-06 Thread Jason Molenda via Phabricator via lldb-commits

jasonmolenda added a comment.

In D143232#4101396 , @bulbazord wrote:

> LGTM. I wonder if there's a good way to exercise this with a test? Like maybe 
> we can create some bogus unwind information and see if LLDB falls over when 
> consuming it?

I'm not sure how we could do that tbh - the currently executing frame uses an 
unwind plan sourced from the assembly instructions, and I can't construct 
assembly language that lldb would parse as using an invalid register number to 
calculate the canonical frame address.  (I genuinely have no idea how we're 
hitting this codepath; it should not be possible.)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143232

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

[Lldb-commits] [PATCH] D143232: Return an error when the CFA resolves to no known register, instead of segfaulting

2023-02-06 Thread Jason Molenda via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG701030c3ecba: In InitializeZerothFrame check for a CFA/AFA 
or error out (authored by jasonmolenda).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143232

Files:
  lldb/source/Target/RegisterContextUnwind.cpp


Index: lldb/source/Target/RegisterContextUnwind.cpp
===
--- lldb/source/Target/RegisterContextUnwind.cpp
+++ lldb/source/Target/RegisterContextUnwind.cpp
@@ -37,6 +37,8 @@
 #include "lldb/Utility/RegisterValue.h"
 #include "lldb/Utility/VASPrintf.h"
 #include "lldb/lldb-private.h"
+
+#include 
 #include 
 
 using namespace lldb;
@@ -289,6 +291,13 @@
   } else
 ReadFrameAddress(row_register_kind, active_row->GetAFAValue(), m_afa);
 
+  if (m_cfa == LLDB_INVALID_ADDRESS && m_afa == LLDB_INVALID_ADDRESS) {
+UnwindLogMsg(
+"could not read CFA or AFA values for first frame, not valid.");
+m_frame_type = eNotAValidFrame;
+return;
+  }
+
   UnwindLogMsg("initialized frame current pc is 0x%" PRIx64 " cfa is 0x%" 
PRIx64
" afa is 0x%" PRIx64 " using %s UnwindPlan",
(uint64_t)m_current_pc.GetLoadAddress(exe_ctx.GetTargetPtr()),
@@ -2116,6 +2125,14 @@
   }
 
   const RegisterInfo *reg_info = GetRegisterInfoAtIndex(lldb_regnum);
+  assert(reg_info);
+  if (!reg_info) {
+UnwindLogMsg(
+"Could not find RegisterInfo definition for lldb register number %d",
+lldb_regnum);
+return false;
+  }
+
   RegisterValue reg_value;
   // if this is frame 0 (currently executing frame), get the requested reg
   // contents from the actual thread registers


Index: lldb/source/Target/RegisterContextUnwind.cpp
===
--- lldb/source/Target/RegisterContextUnwind.cpp
+++ lldb/source/Target/RegisterContextUnwind.cpp
@@ -37,6 +37,8 @@
 #include "lldb/Utility/RegisterValue.h"
 #include "lldb/Utility/VASPrintf.h"
 #include "lldb/lldb-private.h"
+
+#include 
 #include 
 
 using namespace lldb;
@@ -289,6 +291,13 @@
   } else
 ReadFrameAddress(row_register_kind, active_row->GetAFAValue(), m_afa);
 
+  if (m_cfa == LLDB_INVALID_ADDRESS && m_afa == LLDB_INVALID_ADDRESS) {
+UnwindLogMsg(
+"could not read CFA or AFA values for first frame, not valid.");
+m_frame_type = eNotAValidFrame;
+return;
+  }
+
   UnwindLogMsg("initialized frame current pc is 0x%" PRIx64 " cfa is 0x%" PRIx64
" afa is 0x%" PRIx64 " using %s UnwindPlan",
(uint64_t)m_current_pc.GetLoadAddress(exe_ctx.GetTargetPtr()),
@@ -2116,6 +2125,14 @@
   }
 
   const RegisterInfo *reg_info = GetRegisterInfoAtIndex(lldb_regnum);
+  assert(reg_info);
+  if (!reg_info) {
+UnwindLogMsg(
+"Could not find RegisterInfo definition for lldb register number %d",
+lldb_regnum);
+return false;
+  }
+
   RegisterValue reg_value;
   // if this is frame 0 (currently executing frame), get the requested reg
   // contents from the actual thread registers
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 701030c - In InitializeZerothFrame check for a CFA/AFA or error out

2023-02-06 Thread Jason Molenda via lldb-commits


Author: Jason Molenda
Date: 2023-02-06T14:53:50-08:00
New Revision: 701030c3ecba0621ce5d325667fb75b73cf1532b

URL: 
https://github.com/llvm/llvm-project/commit/701030c3ecba0621ce5d325667fb75b73cf1532b
DIFF: 
https://github.com/llvm/llvm-project/commit/701030c3ecba0621ce5d325667fb75b73cf1532b.diff

LOG: In InitializeZerothFrame check for a CFA/AFA or error out

There is a failure where we somehow get an invalid register
number being used to calculate the canonical frame address,
and this ends up with lldb crashing with a null deref because it
assumes that it is always able to find information about that
register.

This patch adds a check for a failure to get a register, and
declares the frame invalid in that case, with some additional
logging or an assert for debug builds.

Differential Revision: https://reviews.llvm.org/D143232
rdar://104428038

Added: 


Modified: 
lldb/source/Target/RegisterContextUnwind.cpp

Removed: 




diff  --git a/lldb/source/Target/RegisterContextUnwind.cpp 
b/lldb/source/Target/RegisterContextUnwind.cpp
index 2da40ba2bf61e..bf31ebbd858ae 100644
--- a/lldb/source/Target/RegisterContextUnwind.cpp
+++ b/lldb/source/Target/RegisterContextUnwind.cpp
@@ -37,6 +37,8 @@
 #include "lldb/Utility/RegisterValue.h"
 #include "lldb/Utility/VASPrintf.h"
 #include "lldb/lldb-private.h"
+
+#include 
 #include 
 
 using namespace lldb;
@@ -289,6 +291,13 @@ void RegisterContextUnwind::InitializeZerothFrame() {
   } else
 ReadFrameAddress(row_register_kind, active_row->GetAFAValue(), m_afa);
 
+  if (m_cfa == LLDB_INVALID_ADDRESS && m_afa == LLDB_INVALID_ADDRESS) {
+UnwindLogMsg(
+"could not read CFA or AFA values for first frame, not valid.");
+m_frame_type = eNotAValidFrame;
+return;
+  }
+
   UnwindLogMsg("initialized frame current pc is 0x%" PRIx64 " cfa is 0x%" 
PRIx64
" afa is 0x%" PRIx64 " using %s UnwindPlan",
(uint64_t)m_current_pc.GetLoadAddress(exe_ctx.GetTargetPtr()),
@@ -2116,6 +2125,14 @@ bool 
RegisterContextUnwind::ReadGPRValue(lldb::RegisterKind register_kind,
   }
 
   const RegisterInfo *reg_info = GetRegisterInfoAtIndex(lldb_regnum);
+  assert(reg_info);
+  if (!reg_info) {
+UnwindLogMsg(
+"Could not find RegisterInfo definition for lldb register number %d",
+lldb_regnum);
+return false;
+  }
+
   RegisterValue reg_value;
   // if this is frame 0 (currently executing frame), get the requested reg
   // contents from the actual thread registers



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

[Lldb-commits] [PATCH] D143012: Add a bit of error checking to SBProcess::ReadMemory to check if a nullptr destination buffer was provided, return error

2023-02-06 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

LGTM modulo inline comments




Comment at: lldb/source/API/SBProcess.cpp:807-811
+  if (!dst) {
+sb_error.SetErrorStringWithFormat(
+"no buffer provided to read %zu bytes into", dst_len);
+return 0;
+  }

Nit: this isn't using `bytes_read` so I'd move the new code above it.



Comment at: lldb/test/API/python_api/process/TestProcessAPI.py:70-72
+if error.Success():
+self.fail("SBProcessReadMemory claims to have "
+  "successfully read 0xffe8 bytes")

`self.assertFalse(error.Success(), "SBProcessReadMemory claims to have 
successfully read 0xffe8 bytes")`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143012

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

[Lldb-commits] [PATCH] D143012: Add a bit of error checking to SBProcess::ReadMemory to check if a nullptr destination buffer was provided, return error

2023-02-06 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib accepted this revision.
mib added a comment.

LGTM with @JDevlieghere comments.




Comment at: lldb/source/API/SBProcess.cpp:807-811
+  if (!dst) {
+sb_error.SetErrorStringWithFormat(
+"no buffer provided to read %zu bytes into", dst_len);
+return 0;
+  }

JDevlieghere wrote:
> Nit: this isn't using `bytes_read` so I'd move the new code above it.
+1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143012

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

[Lldb-commits] [lldb] a3d4f73 - [lldb/Plugins] Fix method dispatch bug when using multiple scripted processes

2023-02-06 Thread Med Ismail Bennani via lldb-commits


Author: Med Ismail Bennani
Date: 2023-02-06T16:02:51-08:00
New Revision: a3d4f739eea357c702754246442a2568f2bace81

URL: 
https://github.com/llvm/llvm-project/commit/a3d4f739eea357c702754246442a2568f2bace81
DIFF: 
https://github.com/llvm/llvm-project/commit/a3d4f739eea357c702754246442a2568f2bace81.diff

LOG: [lldb/Plugins] Fix method dispatch bug when using multiple scripted 
processes

This patch should address a bug when a user have multiple scripted
processes in the same debugging session.

In order for the scripted process plugin to be able to call into the
scripted object instance methods to fetch the necessary data to
reconstruct its state, the scripted process plugin calls into a
scripted process interface, that has a reference to the created script
object instance.

However, prior to this patch, we only had a single instance of the
scripted process interface, living the script interpreter. So every time
a new scripted process plugin was created, it would overwrite the script
object instance that was held by the single scripted process interface
in the script interpreter.

That would cause all the method calls made to the scripted process
interface to be dispatched by the last instanciated script object
instance, which is wrong.

In order to prevent that, this patch moves the scripted process
interface reference to be help by the scripted process plugin itself.

rdar://104882562

Differential Revision: https://reviews.llvm.org/D143308

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 
lldb/include/lldb/Interpreter/ScriptInterpreter.h
lldb/include/lldb/Interpreter/ScriptedInterface.h
lldb/source/Interpreter/ScriptInterpreter.cpp
lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
lldb/source/Plugins/Process/scripted/ScriptedProcess.h
lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py

Removed: 




diff  --git a/lldb/include/lldb/Interpreter/ScriptInterpreter.h 
b/lldb/include/lldb/Interpreter/ScriptInterpreter.h
index 4d073995defb2..4917b7a508aed 100644
--- a/lldb/include/lldb/Interpreter/ScriptInterpreter.h
+++ b/lldb/include/lldb/Interpreter/ScriptInterpreter.h
@@ -147,8 +147,6 @@ class ScriptInterpreter : public PluginInterface {
 
   ScriptInterpreter(
   Debugger &debugger, lldb::ScriptLanguage script_lang,
-  lldb::ScriptedProcessInterfaceUP scripted_process_interface_up =
-  std::make_unique(),
   lldb::ScriptedPlatformInterfaceUP scripted_platform_interface_up =
   std::make_unique());
 
@@ -570,8 +568,8 @@ class ScriptInterpreter : public PluginInterface {
 
   lldb::ScriptLanguage GetLanguage() { return m_script_lang; }
 
-  ScriptedProcessInterface &GetScriptedProcessInterface() {
-return *m_scripted_process_interface_up;
+  virtual lldb::ScriptedProcessInterfaceUP CreateScriptedProcessInterface() {
+return std::make_unique();
   }
 
   ScriptedPlatformInterface &GetScriptedPlatformInterface() {
@@ -589,7 +587,6 @@ class ScriptInterpreter : public PluginInterface {
 protected:
   Debugger &m_debugger;
   lldb::ScriptLanguage m_script_lang;
-  lldb::ScriptedProcessInterfaceUP m_scripted_process_interface_up;
   lldb::ScriptedPlatformInterfaceUP m_scripted_platform_interface_up;
 };
 

diff  --git a/lldb/include/lldb/Interpreter/ScriptedInterface.h 
b/lldb/include/lldb/Interpreter/ScriptedInterface.h
index da4977af123da..31064de7b765f 100644
--- a/lldb/include/lldb/Interpreter/ScriptedInterface.h
+++ b/lldb/include/lldb/Interpreter/ScriptedInterface.h
@@ -30,6 +30,10 @@ class ScriptedInterface {
  StructuredData::DictionarySP args_sp,
  StructuredData::Generic *script_obj = nullptr) = 0;
 
+  StructuredData::GenericSP GetScriptObjectInstance() {
+return m_object_instance_sp;
+  }
+
   template 
   static Ret ErrorWithMessage(llvm::StringRef caller_name,
   llvm::StringRef error_msg, Status &error,

diff  --git a/lldb/source/Interpreter/ScriptInterpreter.cpp 
b/lldb/source/Interpreter/ScriptInterpreter.cpp
index 2722666439bff..60b676d2a5cd0 100644
--- a/lldb/source/Interpreter/ScriptInterpreter.cpp
+++ b/lldb/source/Interpreter/ScriptInterpreter.cpp
@@ -29,10 +29,8 @@ using namespace lldb_private;
 
 ScriptInterpreter::ScriptInterpreter(
 Debugger &debugger, lldb::ScriptLanguage script_lang,
-lldb::ScriptedProcessInterfaceUP scripted_process_interface_up,
 lldb::ScriptedPlatformInterfaceUP scripted_platform_interface_up)
 : m_debugger(debugger), m_script_lang(script_lang),
-  
m_scripted_process_interface_up(std::move(scripted_process_int

[Lldb-commits] [PATCH] D143308: [lldb/Plugins] Fix method dispatch bug when using multiple scripted processes

2023-02-06 Thread Med Ismail Bennani via Phabricator via lldb-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa3d4f739eea3: [lldb/Plugins] Fix method dispatch bug when 
using multiple scripted processes (authored by mib).

Changed prior to commit:
  https://reviews.llvm.org/D143308?vs=494779&id=495313#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143308

Files:
  lldb/include/lldb/Interpreter/ScriptInterpreter.h
  lldb/include/lldb/Interpreter/ScriptedInterface.h
  lldb/source/Interpreter/ScriptInterpreter.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.h
  lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
  lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
  lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py

Index: lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py
===
--- lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py
+++ lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py
@@ -21,10 +21,12 @@
 return {}
 
 def read_memory_at_address(self, addr: int, size: int, error: lldb.SBError) -> lldb.SBData:
+debugger = self.target.GetDebugger()
+index = debugger.GetIndexOfTarget(self.target)
 data = lldb.SBData().CreateDataFromCString(
 self.target.GetByteOrder(),
 self.target.GetCodeByteSize(),
-"Hello, world!")
+"Hello, target " + str(index))
 
 return data
 
Index: lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
===
--- lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
+++ lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
@@ -98,8 +98,8 @@
 id, name stop reason and register context.
 """
 self.build()
-target = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
-self.assertTrue(target, VALID_TARGET)
+target_0 = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+self.assertTrue(target_0, VALID_TARGET)
 
 os.environ['SKIP_SCRIPTED_PROCESS_LAUNCH'] = '1'
 def cleanup():
@@ -115,12 +115,12 @@
 launch_info.SetScriptedProcessClassName("dummy_scripted_process.DummyScriptedProcess")
 
 error = lldb.SBError()
-process = target.Launch(launch_info, error)
-self.assertTrue(process and process.IsValid(), PROCESS_IS_VALID)
-self.assertEqual(process.GetProcessID(), 42)
-self.assertEqual(process.GetNumThreads(), 1)
+process_0 = target_0.Launch(launch_info, error)
+self.assertTrue(process_0 and process_0.IsValid(), PROCESS_IS_VALID)
+self.assertEqual(process_0.GetProcessID(), 42)
+self.assertEqual(process_0.GetNumThreads(), 1)
 
-py_impl = process.GetScriptedImplementation()
+py_impl = process_0.GetScriptedImplementation()
 self.assertTrue(py_impl)
 self.assertTrue(isinstance(py_impl, dummy_scripted_process.DummyScriptedProcess))
 self.assertFalse(hasattr(py_impl, 'my_super_secret_member'))
@@ -128,13 +128,37 @@
 self.assertTrue(hasattr(py_impl, 'my_super_secret_member'))
 self.assertEqual(py_impl.my_super_secret_method(), 42)
 
+# Try reading from target #0 process ...
+addr = 0x5
+message = "Hello, target 0"
+buff = process_0.ReadCStringFromMemory(addr, len(message) + 1, error)
+self.assertSuccess(error)
+self.assertEqual(buff, message)
+
+target_1 = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+self.assertTrue(target_1, VALID_TARGET)
+error = lldb.SBError()
+process_1 = target_1.Launch(launch_info, error)
+self.assertTrue(process_1 and process_1.IsValid(), PROCESS_IS_VALID)
+self.assertEqual(process_1.GetProcessID(), 42)
+self.assertEqual(process_1.GetNumThreads(), 1)
+
+# ... then try reading from target #1 process ...
+addr = 0x5
+message = "Hello, target 1"
+buff = process_1.ReadCStringFromMemory(addr, len(message) + 1, error)
+self.assertSuccess(error)
+self.assertEqual(buff, message)
+
+# ... now, reading again from target #0 process to make sure the call
+# gets dispatched to the right target.
 addr = 0x5
-message = "Hello, world!"
-buff = process.ReadCStri

[Lldb-commits] [PATCH] D143398: [lldb][DWARFASTParserClang] Correctly resolve imported namespaces during expression evaluation

2023-02-06 Thread Adrian Prantl via Phabricator via lldb-commits

aprantl added a comment.

SGTM.




Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:3480
+  clang::NamespaceDecl *namespace_decl =
+  static_cast(m_die_to_decl_ctx[die.GetDIE()]);
+  if (namespace_decl)

The common pattern in LLVM (and thinking of it, I'm surprised we don't have 
some kind of helper template for it) is to use find() since operator[] inserts 
a default-constructed element into the list, which will screw with anyone who 
concurrently searches using find().


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143398

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

[Lldb-commits] [PATCH] D143215: Separate Process::GetWatchpointSupportInfo into two methods to get the two separate pieces of information

2023-02-06 Thread Jason Molenda via Phabricator via lldb-commits

jasonmolenda updated this revision to Diff 495385.
jasonmolenda edited the summary of this revision.
jasonmolenda added a comment.

David's feedback was spot on, overhaul patch to address.

Change Process::GetWatchpointSlotCount to return an optional count, for when it 
is available, instead of depending on a magic number.

I've been unhappy with how we calculate if a target reports watchpoint 
exceptions before or after the instruction has executed.  I believe it is 
target arch specific, I don't think we have any processors that change behavior 
at runtime. We originally homed this bit of information in debugserver and had 
it report it in qHostInfo, but it was one of those early decisions that was not 
correct IMO, and I'm not in favor of perpetuating it.  This update to the patch 
makes a concrete Process::GetWatchpointReportedAfter method that uses the 
Target architecture to determine it.  It also has a call to a 
`DoGetWatchpointReportedAfter` method that subclasses can override if they are 
an environment where the target arch determination is not sufficient - who 
knows.

I changed the gdb-remote plugin to only return the override when the qHostInfo 
key is


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143215

Files:
  lldb/include/lldb/Target/Process.h
  lldb/source/API/SBProcess.cpp
  lldb/source/Commands/CommandObjectWatchpoint.cpp
  lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
  lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h
  lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/source/Target/Process.cpp
  lldb/source/Target/StopInfo.cpp
  lldb/source/Target/Target.cpp

Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -790,19 +790,18 @@
 }
 
 static bool CheckIfWatchpointsSupported(Target *target, Status &error) {
-  uint32_t num_supported_hardware_watchpoints;
-  Status rc = target->GetProcessSP()->GetWatchpointSupportInfo(
-  num_supported_hardware_watchpoints);
+  std::optional num_supported_hardware_watchpoints =
+  target->GetProcessSP()->GetWatchpointSlotCount();
 
   // If unable to determine the # of watchpoints available,
   // assume they are supported.
-  if (rc.Fail())
+  if (!num_supported_hardware_watchpoints)
 return true;
 
   if (num_supported_hardware_watchpoints == 0) {
 error.SetErrorStringWithFormat(
 "Target supports (%u) hardware watchpoint slots.\n",
-num_supported_hardware_watchpoints);
+*num_supported_hardware_watchpoints);
 return false;
   }
   return true;
Index: lldb/source/Target/StopInfo.cpp
===
--- lldb/source/Target/StopInfo.cpp
+++ lldb/source/Target/StopInfo.cpp
@@ -823,16 +823,8 @@
 // stop
 
 ProcessSP process_sp = exe_ctx.GetProcessSP();
-uint32_t num;
-bool wp_triggers_after;
+bool wp_triggers_after = process_sp->GetWatchpointReportedAfter();
 
-if (!process_sp->GetWatchpointSupportInfo(num, wp_triggers_after)
-.Success()) {
-  m_should_stop_is_valid = true;
-  m_should_stop = true;
-  return m_should_stop;
-}
-
 if (!wp_triggers_after) {
   // We have to step over the watchpoint before we know what to do:   
   StopInfoWatchpointSP me_as_siwp_sp 
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -2355,6 +2355,24 @@
   return error;
 }
 
+bool Process::GetWatchpointReportedAfter() {
+  std::optional subclass_override = DoGetWatchpointReportedAfter();
+  if (subclass_override) {
+return *subclass_override;
+  }
+  bool reported_after = true;
+  const ArchSpec &arch = GetTarget().GetArchitecture();
+  if (!arch.IsValid()) {
+return reported_after;
+  }
+  llvm::Triple triple = arch.GetTriple();
+  if (triple.isMIPS() || triple.isPPC64())
+reported_after = false;
+  if (triple.isAArch64() || triple.isArmMClass() || triple.isARM())
+reported_after = false;
+  return reported_after;
+}
+
 ModuleSP Process::ReadModuleFromMemory(const FileSpec &file_spec,
lldb::addr_t header_addr,
size_t size_to_read) {
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remot

[Lldb-commits] [lldb] 5273168 - [lldb] Fix typo in LoongArch unittest

2023-02-06 Thread Weining Lu via lldb-commits


Author: Weining Lu
Date: 2023-02-07T15:09:04+08:00
New Revision: 52731683c9a2f4f8138c317201ea68eaedca7225

URL: 
https://github.com/llvm/llvm-project/commit/52731683c9a2f4f8138c317201ea68eaedca7225
DIFF: 
https://github.com/llvm/llvm-project/commit/52731683c9a2f4f8138c317201ea68eaedca7225.diff

LOG: [lldb] Fix typo in LoongArch unittest

Added: 


Modified: 
lldb/unittests/Instruction/LoongArch/TestLoongArchEmulator.cpp

Removed: 




diff  --git a/lldb/unittests/Instruction/LoongArch/TestLoongArchEmulator.cpp 
b/lldb/unittests/Instruction/LoongArch/TestLoongArchEmulator.cpp
index 376af1f015905..f9372ded0133b 100644
--- a/lldb/unittests/Instruction/LoongArch/TestLoongArchEmulator.cpp
+++ b/lldb/unittests/Instruction/LoongArch/TestLoongArchEmulator.cpp
@@ -170,7 +170,7 @@ TEST_F(LoongArch64EmulatorTester, testJIRL) {
   addr_t old_pc = 0x12000600;
   WritePC(old_pc);
   // JIRL r1, r12, 0x10
-  // | 31   26 | 25   15 | 9   5 | 4   0 |
+  // | 31   26 | 25   10 | 9   5 | 4   0 |
   // | 0 1 0 0 1 1 | 0 0 0 0 0 0 0 0 0 0 0 1 0 0 0 0 | 0 1 1 0 0 | 0 0 0 0 1 |
   uint32_t inst = 0b0100110001011001;
   uint32_t offs16 = 0x10;



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

[Lldb-commits] [PATCH] D143215: Separate Process::GetWatchpointSupportInfo into two methods to get the two separate pieces of information

2023-02-06 Thread Jason Molenda via Phabricator via lldb-commits

jasonmolenda added a comment.

I phabracated wrong and the end of the last msg was truncated.  I meant to end 
it with this:

If the number of watchpoints could not be fetched with an lldb-extension 
packet, Process:GetWatchpointSupportInfo would not report whether breakpoints 
are received before or after the instruction has executed. This latter is 
important to know to handle watchpoints; the former doesn't really matter. So 
when we connect to a non-lldb-server/debugserver stub, and we are targetting an 
arm cpu (where watchpoint exceptions are received before the instruction 
executes), lldb would not correctly disable the watchpoint/insn-step/re-enable 
the watchpoint (v. StopInfoWatchpoint::ShouldStopSynchronous in StopInfo.cpp), 
you'd hit the watchpoint exception over and over without advancing.

This patch separates these into two different Process methods, and sets a 
default of "before" for arm targets if the qHostInfo packet didn't provide any 
hints (e.g. `watchpoint_exceptions_received:after;`), otherwise it returns 
nullopt.

I removed the before/after calculation from the ProcessWindows plugin because 
it always returned after; the concrete method in Process already behaves that 
way for intel.

I need to review & test this patch - it's closer to a WIP at this instant, but 
I wanted to checkpoint the updates I made so far, and where I'm heading with it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143215

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

[Lldb-commits] [PATCH] D143398: [WIP][lldb][DWARFASTParserClang] Correctly resolve imported namespaces during expression evaluation

[Lldb-commits] [lldb] 0b8eff1 - [LLDB][NFC] Fix a incorrect use of shared_ptr in RenderScriptRuntime.cpp

[Lldb-commits] [PATCH] D142309: [LLDB][NFC] Fix a incorrect use of shared_ptr in RenderScriptRuntime.cpp

[Lldb-commits] [PATCH] D142309: [LLDB][NFC] Fix a incorrect use of shared_ptr in RenderScriptRuntime.cpp

[Lldb-commits] [PATCH] D142309: [LLDB][NFC] Fix a incorrect use of shared_ptr in RenderScriptRuntime.cpp

[Lldb-commits] [PATCH] D143308: [lldb/Plugins] Fix method dispatch bug when using multiple scripted processes

[Lldb-commits] [PATCH] D141658: [lldb/crashlog] Make interactive mode the new default

[Lldb-commits] [PATCH] D143398: [WIP][lldb][DWARFASTParserClang] Correctly resolve imported namespaces during expression evaluation

[Lldb-commits] [PATCH] D143347: [lldb][DWARF] Infer no_unique_address attribute

[Lldb-commits] [PATCH] D143232: Return an error when the CFA resolves to no known register, instead of segfaulting

[Lldb-commits] [PATCH] D143232: Return an error when the CFA resolves to no known register, instead of segfaulting

[Lldb-commits] [lldb] 701030c - In InitializeZerothFrame check for a CFA/AFA or error out

[Lldb-commits] [PATCH] D143012: Add a bit of error checking to SBProcess::ReadMemory to check if a nullptr destination buffer was provided, return error

[Lldb-commits] [PATCH] D143012: Add a bit of error checking to SBProcess::ReadMemory to check if a nullptr destination buffer was provided, return error

[Lldb-commits] [lldb] a3d4f73 - [lldb/Plugins] Fix method dispatch bug when using multiple scripted processes

[Lldb-commits] [PATCH] D143308: [lldb/Plugins] Fix method dispatch bug when using multiple scripted processes

[Lldb-commits] [PATCH] D143398: [lldb][DWARFASTParserClang] Correctly resolve imported namespaces during expression evaluation

[Lldb-commits] [PATCH] D143215: Separate Process::GetWatchpointSupportInfo into two methods to get the two separate pieces of information

[Lldb-commits] [lldb] 5273168 - [lldb] Fix typo in LoongArch unittest

[Lldb-commits] [PATCH] D143215: Separate Process::GetWatchpointSupportInfo into two methods to get the two separate pieces of information

20 matches

Site Navigation

Mail list logo

Footer information