[Lldb-commits] [PATCH] D137873: [LLDB][Minidump] Set abi environment for windows.

[Alvin Wong via Phabricator via lldb-commits]

alvinhochun added a comment.

I think the module spec of the modules should reflect the value of the 
`plugin.object-file.pe-coff.abi` setting. Can ProcessMinidump take the module 
spec of the executable module to use its target environment?

Mixing MSVC and MinGW DLLs is a tricky case so I will not be surprised if 
something doesn't work correctly. It would be nice to have it working but that 
would be outside the scope of this review. (I don't yet have a use case for 
this but I am getting close with some COM / WinRT experiments.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137873

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

[Lldb-commits] [PATCH] D138181: [test] Allow skipTestIfFn to apply to entire classes for skipIfNoSBHeaders

[David Spickett via Phabricator via lldb-commits]

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

LGTM. Do what makes sense to you with the comment.




Comment at: lldb/packages/Python/lldbsuite/test/decorators.py:146
+# reason is used for both args.
+return unittest2.skipIf(reason, reason)(func)
 

This would make more immediate sense if you added the function prototype as 
well:
```
# skipIf(condition, reason)
```

Or if you can figure a way to do the C++ style `skipIf(/*condition=*/reason, 
reason)`.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138181

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

[Lldb-commits] [PATCH] D138197: [lldb] Fix bitfield incorrectly printing when field crosses a storage unit

[Charlie Keaney via Phabricator via lldb-commits]

CharKeaney created this revision.
CharKeaney added reviewers: DavidSpickett, simoncook, lewis-revill, 
edward-jones.
CharKeaney added a project: LLDB.
Herald added subscribers: JDevlieghere, kbarton, nemanjai.
Herald added a project: All.
CharKeaney requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch resolves an issue where bitfields will not be
printed correctly if a field crosses a storage unit.

The issue is caused by lldb internally storing bitfield
field's offsets as unsigned rather than signed values.
Negative offsets can be created when a field crosses a
storage unit.

The issue is resolved by changing lldb's internal representation
for bitfield field offsets to be stored using signed values.
Also, in the code responsible for extracting data for printing,
the mechanism must be changed such that it can handle negative
offsets appropriately.

This patch includes a test case to test that the issue is resolved.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138197

Files:
  lldb/include/lldb/Core/DumpDataExtractor.h
  lldb/include/lldb/Core/ValueObject.h
  lldb/include/lldb/Core/ValueObjectChild.h
  lldb/include/lldb/Symbol/CompilerType.h
  lldb/include/lldb/Symbol/TypeSystem.h
  lldb/include/lldb/Target/ProcessStructReader.h
  lldb/include/lldb/Utility/DataExtractor.h
  lldb/source/API/SBType.cpp
  lldb/source/Core/DumpDataExtractor.cpp
  lldb/source/Core/ValueObject.cpp
  lldb/source/Core/ValueObjectChild.cpp
  lldb/source/Core/ValueObjectConstResultImpl.cpp
  lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc64.cpp
  lldb/source/Plugins/ABI/X86/ABISysV_x86_64.cpp
  lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.cpp
  lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
  lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
  lldb/source/Symbol/CompilerType.cpp
  lldb/source/Utility/DataExtractor.cpp
  lldb/test/API/commands/expression/expr-bitfield-on-boundary/Makefile
  
lldb/test/API/commands/expression/expr-bitfield-on-boundary/TestExprBitfieldOnBoundary.py
  lldb/test/API/commands/expression/expr-bitfield-on-boundary/main.cpp
  lldb/unittests/Platform/PlatformSiginfoTest.cpp

Index: lldb/unittests/Platform/PlatformSiginfoTest.cpp
===
--- lldb/unittests/Platform/PlatformSiginfoTest.cpp
+++ lldb/unittests/Platform/PlatformSiginfoTest.cpp
@@ -59,7 +59,7 @@
 CompilerType field_type = siginfo_type;
 uint64_t total_offset = 0;
 for (auto field_name : llvm::split(path, '.')) {
-  uint64_t bit_offset;
+  int64_t bit_offset;
   ASSERT_NE(field_type.GetIndexOfFieldWithName(field_name.str().c_str(),
&field_type, &bit_offset),
 UINT32_MAX);
Index: lldb/test/API/commands/expression/expr-bitfield-on-boundary/main.cpp
===
--- /dev/null
+++ lldb/test/API/commands/expression/expr-bitfield-on-boundary/main.cpp
@@ -0,0 +1,11 @@
+struct __attribute__((packed)) Foo {
+  unsigned char a : 7;
+  unsigned char b : 4;
+};
+
+int main() {
+  volatile struct Foo f;
+  f.a = 1;
+  f.b = 2;
+  return 0; // Break here
+}
Index: lldb/test/API/commands/expression/expr-bitfield-on-boundary/TestExprBitfieldOnBoundary.py
===
--- /dev/null
+++ lldb/test/API/commands/expression/expr-bitfield-on-boundary/TestExprBitfieldOnBoundary.py
@@ -0,0 +1,12 @@
+"""Test that we are able to print bitfield fields that cross a byte boundary"""
+
+import lldb
+from lldbsuite.test.lldbtest import *
+
+class  BitFieldExprOnBoundaryTestCase(TestBase):
+
+def test_bitfield_expr_on_boundary(self):
+""" Test that we are able to print bitfield fields that cross a byte boundary """
+self.build()
+lldbutil.run_to_source_breakpoint(self, '// Break here', lldb.SBFileSpec("main.cpp"))
+self.expect_expr("f", result_type="volatile Foo", result_children=[ValueCheck(name='a', value=r"'\x01'"), ValueCheck(name='b', value=r"'\x02'")])
Index: lldb/test/API/commands/expression/expr-bitfield-on-boundary/Makefile
===
--- /dev/null
+++ lldb/test/API/commands/expression/expr-bitfield-on-boundary/Makefile
@@ -0,0 +1,5 @@
+CXX_SOURCES := main.cpp
+
+CXXFLAGS_EXTRAS := -gdwarf-2
+
+include Makefile.rules
Index: lldb/source/Utility/DataExtractor.cpp
===
--- lldb/source/Utility/DataExtractor.cpp
+++ lldb/source/Utility/DataExtractor.cpp
@@ -572,7 +572,7 @@
 
 uint64_t DataExtractor::GetMaxU64Bitfield(offset_t *offset_ptr, size_t size,
   uint32_t bitfield_bit_size,
- 

[Lldb-commits] [PATCH] D137247: [lldb] Allow plugins to extend DWARF expression parsing for vendor extensions

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

What was the decision on `m_dwarf_cu` possibly being nullptr? We at least want 
asserts in paths where we assume it'll not be null.

Beyond that this seems fine but I'm not really into DWARF, @labath any comments?

Seems like quite a cheap fallback to add, and my assumption is that we don't 
find many unknown opcodes in general use.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137247

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

[Lldb-commits] [lldb] 4113e98 - [LLDB][RISCV] Allow accessing registers through ABI names

[via lldb-commits]

Author: Emmmer
Date: 2022-11-17T19:39:06+08:00
New Revision: 4113e98ea78590eac217a8a76201120b57f9d39f

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

LOG: [LLDB][RISCV] Allow accessing registers through ABI names

This patch uses RISCV ABI register name as `alt_name` in `RegisterInfo` in 
`lldb-private-types.h`

Reviewed By: DavidSpickett

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

Added: 


Modified: 
lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h
lldb/source/Plugins/Process/Utility/lldb-riscv-register-enums.h
lldb/source/Utility/RISCV_DWARF_Registers.h

Removed: 




diff  --git a/lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h 
b/lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h
index ac1ec087e3760..2360b4356801e 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h
+++ b/lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h
@@ -67,36 +67,36 @@ static lldb_private::RegisterInfo 
g_register_infos_riscv64_le[] = {
 DEFINE_GPR64(pc, LLDB_REGNUM_GENERIC_PC),
 DEFINE_GPR64_ALT(ra, x1, LLDB_REGNUM_GENERIC_RA),
 DEFINE_GPR64_ALT(sp, x2, LLDB_REGNUM_GENERIC_SP),
-DEFINE_GPR64(x3, LLDB_INVALID_REGNUM),
-DEFINE_GPR64(x4, LLDB_INVALID_REGNUM),
-DEFINE_GPR64(x5, LLDB_INVALID_REGNUM),
-DEFINE_GPR64(x6, LLDB_INVALID_REGNUM),
-DEFINE_GPR64(x7, LLDB_INVALID_REGNUM),
+DEFINE_GPR64_ALT(gp, x3, LLDB_INVALID_REGNUM),
+DEFINE_GPR64_ALT(tp, x4, LLDB_INVALID_REGNUM),
+DEFINE_GPR64_ALT(t0, x5, LLDB_INVALID_REGNUM),
+DEFINE_GPR64_ALT(t1, x6, LLDB_INVALID_REGNUM),
+DEFINE_GPR64_ALT(t2, x7, LLDB_INVALID_REGNUM),
 DEFINE_GPR64_ALT(fp, x8, LLDB_REGNUM_GENERIC_FP),
-DEFINE_GPR64(x9, LLDB_INVALID_REGNUM),
-DEFINE_GPR64(x10, LLDB_REGNUM_GENERIC_ARG1),
-DEFINE_GPR64(x11, LLDB_REGNUM_GENERIC_ARG2),
-DEFINE_GPR64(x12, LLDB_REGNUM_GENERIC_ARG3),
-DEFINE_GPR64(x13, LLDB_REGNUM_GENERIC_ARG4),
-DEFINE_GPR64(x14, LLDB_REGNUM_GENERIC_ARG5),
-DEFINE_GPR64(x15, LLDB_REGNUM_GENERIC_ARG6),
-DEFINE_GPR64(x16, LLDB_REGNUM_GENERIC_ARG7),
-DEFINE_GPR64(x17, LLDB_REGNUM_GENERIC_ARG8),
-DEFINE_GPR64(x18, LLDB_INVALID_REGNUM),
-DEFINE_GPR64(x19, LLDB_INVALID_REGNUM),
-DEFINE_GPR64(x20, LLDB_INVALID_REGNUM),
-DEFINE_GPR64(x21, LLDB_INVALID_REGNUM),
-DEFINE_GPR64(x22, LLDB_INVALID_REGNUM),
-DEFINE_GPR64(x23, LLDB_INVALID_REGNUM),
-DEFINE_GPR64(x24, LLDB_INVALID_REGNUM),
-DEFINE_GPR64(x25, LLDB_INVALID_REGNUM),
-DEFINE_GPR64(x26, LLDB_INVALID_REGNUM),
-DEFINE_GPR64(x27, LLDB_INVALID_REGNUM),
-DEFINE_GPR64(x28, LLDB_INVALID_REGNUM),
-DEFINE_GPR64(x29, LLDB_INVALID_REGNUM),
-DEFINE_GPR64(x30, LLDB_INVALID_REGNUM),
-DEFINE_GPR64(x31, LLDB_INVALID_REGNUM),
-DEFINE_GPR64(x0, LLDB_INVALID_REGNUM),
+DEFINE_GPR64_ALT(s1, x9, LLDB_INVALID_REGNUM),
+DEFINE_GPR64_ALT(a0, x10, LLDB_REGNUM_GENERIC_ARG1),
+DEFINE_GPR64_ALT(a1, x11, LLDB_REGNUM_GENERIC_ARG2),
+DEFINE_GPR64_ALT(a2, x12, LLDB_REGNUM_GENERIC_ARG3),
+DEFINE_GPR64_ALT(a3, x13, LLDB_REGNUM_GENERIC_ARG4),
+DEFINE_GPR64_ALT(a4, x14, LLDB_REGNUM_GENERIC_ARG5),
+DEFINE_GPR64_ALT(a5, x15, LLDB_REGNUM_GENERIC_ARG6),
+DEFINE_GPR64_ALT(a6, x16, LLDB_REGNUM_GENERIC_ARG7),
+DEFINE_GPR64_ALT(a7, x17, LLDB_REGNUM_GENERIC_ARG8),
+DEFINE_GPR64_ALT(s2, x18, LLDB_INVALID_REGNUM),
+DEFINE_GPR64_ALT(s3, x19, LLDB_INVALID_REGNUM),
+DEFINE_GPR64_ALT(s4, x20, LLDB_INVALID_REGNUM),
+DEFINE_GPR64_ALT(s5, x21, LLDB_INVALID_REGNUM),
+DEFINE_GPR64_ALT(s6, x22, LLDB_INVALID_REGNUM),
+DEFINE_GPR64_ALT(s7, x23, LLDB_INVALID_REGNUM),
+DEFINE_GPR64_ALT(s8, x24, LLDB_INVALID_REGNUM),
+DEFINE_GPR64_ALT(s9, x25, LLDB_INVALID_REGNUM),
+DEFINE_GPR64_ALT(s10, x26, LLDB_INVALID_REGNUM),
+DEFINE_GPR64_ALT(s11, x27, LLDB_INVALID_REGNUM),
+DEFINE_GPR64_ALT(t3, x28, LLDB_INVALID_REGNUM),
+DEFINE_GPR64_ALT(t4, x29, LLDB_INVALID_REGNUM),
+DEFINE_GPR64_ALT(t5, x30, LLDB_INVALID_REGNUM),
+DEFINE_GPR64_ALT(t6, x31, LLDB_INVALID_REGNUM),
+DEFINE_GPR64_ALT(zero, x0, LLDB_INVALID_REGNUM),
 
 DEFINE_FPR64(f0, LLDB_INVALID_REGNUM),
 DEFINE_FPR64(f1, LLDB_INVALID_REGNUM),

diff  --git a/lldb/source/Plugins/Process/Utility/lldb-riscv-register-enums.h 
b/lldb/source/Plugins/Process/Utility/lldb-riscv-register-enums.h
index 820bf6aaf9888..b77dc1398f824 100644
--- a/lldb/source/Plugins/Process/Utility/lldb-riscv-register-enums.h
+++ b/lldb/source/Plugins/Process/Utility/lldb-riscv-register-enums.h
@@ -50,9 +50,38 @@ enum {
   gpr_x31_riscv,
   gpr_x0_riscv,
   gpr_last_riscv = gpr_x0_riscv,
+  gpr_zero_riscv = gpr_x0_riscv,
   gpr_ra_riscv = gpr_x1_riscv,
   gpr_sp_riscv = gpr_x2_riscv,
+  gpr_gp_riscv = 

[Lldb-commits] [PATCH] D137508: [LLDB][RISCV] Allow accessing registers through ABI names

[Emmmer S via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4113e98ea785: [LLDB][RISCV] Allow accessing registers 
through ABI names (authored by Emmmer).
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137508

Files:
  lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h
  lldb/source/Plugins/Process/Utility/lldb-riscv-register-enums.h
  lldb/source/Utility/RISCV_DWARF_Registers.h

Index: lldb/source/Utility/RISCV_DWARF_Registers.h
===
--- lldb/source/Utility/RISCV_DWARF_Registers.h
+++ lldb/source/Utility/RISCV_DWARF_Registers.h
@@ -118,10 +118,39 @@
   dwarf_first_csr = 4096,
   dwarf_last_csr = 8191,
 
-  // register name alias
+  // register ABI name
+  dwarf_gpr_zero = dwarf_gpr_x0,
   dwarf_gpr_ra = dwarf_gpr_x1,
   dwarf_gpr_sp = dwarf_gpr_x2,
+  dwarf_gpr_gp = dwarf_gpr_x3,
+  dwarf_gpr_tp = dwarf_gpr_x4,
+  dwarf_gpr_t0 = dwarf_gpr_x5,
+  dwarf_gpr_t1 = dwarf_gpr_x6,
+  dwarf_gpr_t2 = dwarf_gpr_x7,
   dwarf_gpr_fp = dwarf_gpr_x8,
+  dwarf_gpr_s1 = dwarf_gpr_x9,
+  dwarf_gpr_a0 = dwarf_gpr_x10,
+  dwarf_gpr_a1 = dwarf_gpr_x11,
+  dwarf_gpr_a2 = dwarf_gpr_x12,
+  dwarf_gpr_a3 = dwarf_gpr_x13,
+  dwarf_gpr_a4 = dwarf_gpr_x14,
+  dwarf_gpr_a5 = dwarf_gpr_x15,
+  dwarf_gpr_a6 = dwarf_gpr_x16,
+  dwarf_gpr_a7 = dwarf_gpr_x17,
+  dwarf_gpr_s2 = dwarf_gpr_x18,
+  dwarf_gpr_s3 = dwarf_gpr_x19,
+  dwarf_gpr_s4 = dwarf_gpr_x20,
+  dwarf_gpr_s5 = dwarf_gpr_x21,
+  dwarf_gpr_s6 = dwarf_gpr_x22,
+  dwarf_gpr_s7 = dwarf_gpr_x23,
+  dwarf_gpr_s8 = dwarf_gpr_x24,
+  dwarf_gpr_s9 = dwarf_gpr_x25,
+  dwarf_gpr_s10 = dwarf_gpr_x26,
+  dwarf_gpr_s11 = dwarf_gpr_x27,
+  dwarf_gpr_t3 = dwarf_gpr_x28,
+  dwarf_gpr_t4 = dwarf_gpr_x29,
+  dwarf_gpr_t5 = dwarf_gpr_x30,
+  dwarf_gpr_t6 = dwarf_gpr_x31,
 
   // mock pc regnum
   dwarf_gpr_pc = 11451,
Index: lldb/source/Plugins/Process/Utility/lldb-riscv-register-enums.h
===
--- lldb/source/Plugins/Process/Utility/lldb-riscv-register-enums.h
+++ lldb/source/Plugins/Process/Utility/lldb-riscv-register-enums.h
@@ -50,9 +50,38 @@
   gpr_x31_riscv,
   gpr_x0_riscv,
   gpr_last_riscv = gpr_x0_riscv,
+  gpr_zero_riscv = gpr_x0_riscv,
   gpr_ra_riscv = gpr_x1_riscv,
   gpr_sp_riscv = gpr_x2_riscv,
+  gpr_gp_riscv = gpr_x3_riscv,
+  gpr_tp_riscv = gpr_x4_riscv,
+  gpr_t0_riscv = gpr_x5_riscv,
+  gpr_t1_riscv = gpr_x6_riscv,
+  gpr_t2_riscv = gpr_x7_riscv,
   gpr_fp_riscv = gpr_x8_riscv,
+  gpr_s1_riscv = gpr_x9_riscv,
+  gpr_a0_riscv = gpr_x10_riscv,
+  gpr_a1_riscv = gpr_x11_riscv,
+  gpr_a2_riscv = gpr_x12_riscv,
+  gpr_a3_riscv = gpr_x13_riscv,
+  gpr_a4_riscv = gpr_x14_riscv,
+  gpr_a5_riscv = gpr_x15_riscv,
+  gpr_a6_riscv = gpr_x16_riscv,
+  gpr_a7_riscv = gpr_x17_riscv,
+  gpr_s2_riscv = gpr_x18_riscv,
+  gpr_s3_riscv = gpr_x19_riscv,
+  gpr_s4_riscv = gpr_x20_riscv,
+  gpr_s5_riscv = gpr_x21_riscv,
+  gpr_s6_riscv = gpr_x22_riscv,
+  gpr_s7_riscv = gpr_x23_riscv,
+  gpr_s8_riscv = gpr_x24_riscv,
+  gpr_s9_riscv = gpr_x25_riscv,
+  gpr_s10_riscv = gpr_x26_riscv,
+  gpr_s11_riscv = gpr_x27_riscv,
+  gpr_t3_riscv = gpr_x28_riscv,
+  gpr_t4_riscv = gpr_x29_riscv,
+  gpr_t5_riscv = gpr_x30_riscv,
+  gpr_t6_riscv = gpr_x31_riscv,
 
   fpr_first_riscv = 33,
   fpr_f0_riscv = fpr_first_riscv,
Index: lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h
===
--- lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h
+++ lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h
@@ -67,36 +67,36 @@
 DEFINE_GPR64(pc, LLDB_REGNUM_GENERIC_PC),
 DEFINE_GPR64_ALT(ra, x1, LLDB_REGNUM_GENERIC_RA),
 DEFINE_GPR64_ALT(sp, x2, LLDB_REGNUM_GENERIC_SP),
-DEFINE_GPR64(x3, LLDB_INVALID_REGNUM),
-DEFINE_GPR64(x4, LLDB_INVALID_REGNUM),
-DEFINE_GPR64(x5, LLDB_INVALID_REGNUM),
-DEFINE_GPR64(x6, LLDB_INVALID_REGNUM),
-DEFINE_GPR64(x7, LLDB_INVALID_REGNUM),
+DEFINE_GPR64_ALT(gp, x3, LLDB_INVALID_REGNUM),
+DEFINE_GPR64_ALT(tp, x4, LLDB_INVALID_REGNUM),
+DEFINE_GPR64_ALT(t0, x5, LLDB_INVALID_REGNUM),
+DEFINE_GPR64_ALT(t1, x6, LLDB_INVALID_REGNUM),
+DEFINE_GPR64_ALT(t2, x7, LLDB_INVALID_REGNUM),
 DEFINE_GPR64_ALT(fp, x8, LLDB_REGNUM_GENERIC_FP),
-DEFINE_GPR64(x9, LLDB_INVALID_REGNUM),
-DEFINE_GPR64(x10, LLDB_REGNUM_GENERIC_ARG1),
-DEFINE_GPR64(x11, LLDB_REGNUM_GENERIC_ARG2),
-DEFINE_GPR64(x12, LLDB_REGNUM_GENERIC_ARG3),
-DEFINE_GPR64(x13, LLDB_REGNUM_GENERIC_ARG4),
-DEFINE_GPR64(x14, LLDB_REGNUM_GENERIC_ARG5),
-DEFINE_GPR64(x15, LLDB_REGNUM_GENERIC_ARG6),
-DEFINE_GPR64(x16, LLDB_REGNUM_GENERIC_ARG7),
-DEFINE_GPR64(x17, LLDB_REGNUM_GENERIC_ARG8),
-DEFINE_GPR64(x18, LLDB_INVALID_REGNUM),
-DEFINE_GPR64(x19, LLDB_INVALID_REGNUM),
-DEFINE_GPR64(x20, 

[Lldb-commits] [PATCH] D137247: [lldb] Allow plugins to extend DWARF expression parsing for vendor extensions

[Philip Pfaffe via Phabricator via lldb-commits]

pfaffe added a comment.

I threaded dwarf_cu down to where it's really used and added the necessary 
check, so if it is nullptr, the fallback will be skipped.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137247

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

[Lldb-commits] [lldb] 201ed71 - [LLDB][RISCV] Allow accessing FPR registers through ABI names

[via lldb-commits]

Author: Emmmer
Date: 2022-11-17T19:58:21+08:00
New Revision: 201ed71cfd7e68f3925b1308e09f1a437f377080

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

LOG: [LLDB][RISCV] Allow accessing FPR registers through ABI names

Allow users to access FPR registers by names or ABI names.
PS: This patch should be merged after D137508

Reviewed By: DavidSpickett

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

Added: 


Modified: 
lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_riscv64.cpp
lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h
lldb/source/Plugins/Process/Utility/lldb-riscv-register-enums.h
lldb/source/Utility/RISCV_DWARF_Registers.h

Removed: 




diff  --git a/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_riscv64.cpp 
b/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_riscv64.cpp
index 619f3069..3819401c543b 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_riscv64.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_riscv64.cpp
@@ -104,7 +104,7 @@ RegisterInfoPOSIX_riscv64::RegisterInfoPOSIX_riscv64(
   m_register_info_count(GetRegisterInfoCount(target_arch)) {}
 
 uint32_t RegisterInfoPOSIX_riscv64::GetRegisterCount() const {
-  return k_num_gpr_registers;
+  return m_register_info_count;
 }
 
 size_t RegisterInfoPOSIX_riscv64::GetGPRSize() const {
@@ -121,7 +121,7 @@ RegisterInfoPOSIX_riscv64::GetRegisterInfo() const {
 }
 
 size_t RegisterInfoPOSIX_riscv64::GetRegisterSetCount() const {
-  return k_num_register_sets - 1;
+  return k_num_register_sets;
 }
 
 size_t RegisterInfoPOSIX_riscv64::GetRegisterSetFromRegisterIndex(

diff  --git a/lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h 
b/lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h
index 2360b4356801..331155eab32f 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h
+++ b/lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h
@@ -53,9 +53,13 @@ using namespace riscv_dwarf;
 GPR64_KIND(gpr_##reg, generic_kind), nullptr, nullptr  
\
   }
 
-#define DEFINE_FPR64(reg, generic_kind)
\
+#define DEFINE_FPR64(reg, generic_kind) DEFINE_FPR64_ALT(reg, reg, 
generic_kind)
+
+#define DEFINE_FPR64_ALT(reg, alt, generic_kind) DEFINE_FPR_ALT(reg, alt, 8, 
generic_kind)
+
+#define DEFINE_FPR_ALT(reg, alt, size, generic_kind)   
\
   {
\
-#reg, nullptr, 8, FPR_OFFSET(fpr_##reg##_riscv - fpr_first_riscv), 
\
+#reg, #alt, size, FPR_OFFSET(fpr_##reg##_riscv - fpr_first_riscv), 
\
 lldb::eEncodingUint, lldb::eFormatHex, 
\
 FPR64_KIND(fpr_##reg, generic_kind), nullptr, nullptr  
\
   }
@@ -98,38 +102,39 @@ static lldb_private::RegisterInfo 
g_register_infos_riscv64_le[] = {
 DEFINE_GPR64_ALT(t6, x31, LLDB_INVALID_REGNUM),
 DEFINE_GPR64_ALT(zero, x0, LLDB_INVALID_REGNUM),
 
-DEFINE_FPR64(f0, LLDB_INVALID_REGNUM),
-DEFINE_FPR64(f1, LLDB_INVALID_REGNUM),
-DEFINE_FPR64(f2, LLDB_INVALID_REGNUM),
-DEFINE_FPR64(f3, LLDB_INVALID_REGNUM),
-DEFINE_FPR64(f4, LLDB_INVALID_REGNUM),
-DEFINE_FPR64(f5, LLDB_INVALID_REGNUM),
-DEFINE_FPR64(f6, LLDB_INVALID_REGNUM),
-DEFINE_FPR64(f7, LLDB_INVALID_REGNUM),
-DEFINE_FPR64(f8, LLDB_INVALID_REGNUM),
-DEFINE_FPR64(f9, LLDB_INVALID_REGNUM),
-DEFINE_FPR64(f10, LLDB_INVALID_REGNUM),
-DEFINE_FPR64(f11, LLDB_INVALID_REGNUM),
-DEFINE_FPR64(f12, LLDB_INVALID_REGNUM),
-DEFINE_FPR64(f13, LLDB_INVALID_REGNUM),
-DEFINE_FPR64(f14, LLDB_INVALID_REGNUM),
-DEFINE_FPR64(f15, LLDB_INVALID_REGNUM),
-DEFINE_FPR64(f16, LLDB_INVALID_REGNUM),
-DEFINE_FPR64(f17, LLDB_INVALID_REGNUM),
-DEFINE_FPR64(f18, LLDB_INVALID_REGNUM),
-DEFINE_FPR64(f19, LLDB_INVALID_REGNUM),
-DEFINE_FPR64(f20, LLDB_INVALID_REGNUM),
-DEFINE_FPR64(f21, LLDB_INVALID_REGNUM),
-DEFINE_FPR64(f22, LLDB_INVALID_REGNUM),
-DEFINE_FPR64(f23, LLDB_INVALID_REGNUM),
-DEFINE_FPR64(f24, LLDB_INVALID_REGNUM),
-DEFINE_FPR64(f25, LLDB_INVALID_REGNUM),
-DEFINE_FPR64(f26, LLDB_INVALID_REGNUM),
-DEFINE_FPR64(f27, LLDB_INVALID_REGNUM),
-DEFINE_FPR64(f28, LLDB_INVALID_REGNUM),
-DEFINE_FPR64(f29, LLDB_INVALID_REGNUM),
-DEFINE_FPR64(f30, LLDB_INVALID_REGNUM),
-DEFINE_FPR64(f31, LLDB_INVALID_REGNUM),
+DEFINE_FPR64_ALT(ft0, f0, LLDB_INVALID_REGNUM),
+DEFINE_FPR64_ALT(ft1, f1, LLDB_INVALID_REGNUM),
+DEFINE_FPR64_ALT(ft2, f2, LLDB_INVALID_REGNUM),
+DEFINE_FPR64_ALT(ft3, f3, LLDB_INVALID_REGNUM),
+DEFINE_FPR64_ALT(ft4, f4, LLDB_INVALID_REGNUM),
+DEFINE_FPR64_

[Lldb-commits] [PATCH] D137761: [LLDB][RISCV] Allow accessing FPR registers through ABI names

[Emmmer S via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG201ed71cfd7e: [LLDB][RISCV] Allow accessing FPR registers 
through ABI names (authored by Emmmer).
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137761

Files:
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_riscv64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h
  lldb/source/Plugins/Process/Utility/lldb-riscv-register-enums.h
  lldb/source/Utility/RISCV_DWARF_Registers.h

Index: lldb/source/Utility/RISCV_DWARF_Registers.h
===
--- lldb/source/Utility/RISCV_DWARF_Registers.h
+++ lldb/source/Utility/RISCV_DWARF_Registers.h
@@ -116,6 +116,7 @@
   dwarf_v30,
   dwarf_v31 = 127,
   dwarf_first_csr = 4096,
+  dwarf_fpr_fcsr = dwarf_first_csr + 0x003,
   dwarf_last_csr = 8191,
 
   // register ABI name
@@ -152,6 +153,39 @@
   dwarf_gpr_t5 = dwarf_gpr_x30,
   dwarf_gpr_t6 = dwarf_gpr_x31,
 
+  dwarf_fpr_ft0 = dwarf_fpr_f0,
+  dwarf_fpr_ft1 = dwarf_fpr_f1,
+  dwarf_fpr_ft2 = dwarf_fpr_f2,
+  dwarf_fpr_ft3 = dwarf_fpr_f3,
+  dwarf_fpr_ft4 = dwarf_fpr_f4,
+  dwarf_fpr_ft5 = dwarf_fpr_f5,
+  dwarf_fpr_ft6 = dwarf_fpr_f6,
+  dwarf_fpr_ft7 = dwarf_fpr_f7,
+  dwarf_fpr_fs0 = dwarf_fpr_f8,
+  dwarf_fpr_fs1 = dwarf_fpr_f9,
+  dwarf_fpr_fa0 = dwarf_fpr_f10,
+  dwarf_fpr_fa1 = dwarf_fpr_f11,
+  dwarf_fpr_fa2 = dwarf_fpr_f12,
+  dwarf_fpr_fa3 = dwarf_fpr_f13,
+  dwarf_fpr_fa4 = dwarf_fpr_f14,
+  dwarf_fpr_fa5 = dwarf_fpr_f15,
+  dwarf_fpr_fa6 = dwarf_fpr_f16,
+  dwarf_fpr_fa7 = dwarf_fpr_f17,
+  dwarf_fpr_fs2 = dwarf_fpr_f18,
+  dwarf_fpr_fs3 = dwarf_fpr_f19,
+  dwarf_fpr_fs4 = dwarf_fpr_f20,
+  dwarf_fpr_fs5 = dwarf_fpr_f21,
+  dwarf_fpr_fs6 = dwarf_fpr_f22,
+  dwarf_fpr_fs7 = dwarf_fpr_f23,
+  dwarf_fpr_fs8 = dwarf_fpr_f24,
+  dwarf_fpr_fs9 = dwarf_fpr_f25,
+  dwarf_fpr_fs10 = dwarf_fpr_f26,
+  dwarf_fpr_fs11 = dwarf_fpr_f27,
+  dwarf_fpr_ft8 = dwarf_fpr_f28,
+  dwarf_fpr_ft9 = dwarf_fpr_f29,
+  dwarf_fpr_ft10 = dwarf_fpr_f30,
+  dwarf_fpr_ft11 = dwarf_fpr_f31,
+
   // mock pc regnum
   dwarf_gpr_pc = 11451,
 };
Index: lldb/source/Plugins/Process/Utility/lldb-riscv-register-enums.h
===
--- lldb/source/Plugins/Process/Utility/lldb-riscv-register-enums.h
+++ lldb/source/Plugins/Process/Utility/lldb-riscv-register-enums.h
@@ -49,7 +49,6 @@
   gpr_x30_riscv,
   gpr_x31_riscv,
   gpr_x0_riscv,
-  gpr_last_riscv = gpr_x0_riscv,
   gpr_zero_riscv = gpr_x0_riscv,
   gpr_ra_riscv = gpr_x1_riscv,
   gpr_sp_riscv = gpr_x2_riscv,
@@ -82,6 +81,7 @@
   gpr_t4_riscv = gpr_x29_riscv,
   gpr_t5_riscv = gpr_x30_riscv,
   gpr_t6_riscv = gpr_x31_riscv,
+  gpr_last_riscv = gpr_x0_riscv,
 
   fpr_first_riscv = 33,
   fpr_f0_riscv = fpr_first_riscv,
@@ -118,6 +118,38 @@
   fpr_f31_riscv,
 
   fpr_fcsr_riscv,
+  fpr_ft0_riscv = fpr_f0_riscv,
+  fpr_ft1_riscv = fpr_f1_riscv,
+  fpr_ft2_riscv = fpr_f2_riscv,
+  fpr_ft3_riscv = fpr_f3_riscv,
+  fpr_ft4_riscv = fpr_f4_riscv,
+  fpr_ft5_riscv = fpr_f5_riscv,
+  fpr_ft6_riscv = fpr_f6_riscv,
+  fpr_ft7_riscv = fpr_f7_riscv,
+  fpr_fs0_riscv = fpr_f8_riscv,
+  fpr_fs1_riscv = fpr_f9_riscv,
+  fpr_fa0_riscv = fpr_f10_riscv,
+  fpr_fa1_riscv = fpr_f11_riscv,
+  fpr_fa2_riscv = fpr_f12_riscv,
+  fpr_fa3_riscv = fpr_f13_riscv,
+  fpr_fa4_riscv = fpr_f14_riscv,
+  fpr_fa5_riscv = fpr_f15_riscv,
+  fpr_fa6_riscv = fpr_f16_riscv,
+  fpr_fa7_riscv = fpr_f17_riscv,
+  fpr_fs2_riscv = fpr_f18_riscv,
+  fpr_fs3_riscv = fpr_f19_riscv,
+  fpr_fs4_riscv = fpr_f20_riscv,
+  fpr_fs5_riscv = fpr_f21_riscv,
+  fpr_fs6_riscv = fpr_f22_riscv,
+  fpr_fs7_riscv = fpr_f23_riscv,
+  fpr_fs8_riscv = fpr_f24_riscv,
+  fpr_fs9_riscv = fpr_f25_riscv,
+  fpr_fs10_riscv = fpr_f26_riscv,
+  fpr_fs11_riscv = fpr_f27_riscv,
+  fpr_ft8_riscv = fpr_f28_riscv,
+  fpr_ft9_riscv = fpr_f29_riscv,
+  fpr_ft10_riscv = fpr_f30_riscv,
+  fpr_ft11_riscv = fpr_f31_riscv,
   fpr_last_riscv = fpr_fcsr_riscv,
 
   k_num_registers_riscv
Index: lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h
===
--- lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h
+++ lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h
@@ -53,9 +53,13 @@
 GPR64_KIND(gpr_##reg, generic_kind), nullptr, nullptr  \
   }
 
-#define DEFINE_FPR64(reg, generic_kind)\
+#define DEFINE_FPR64(reg, generic_kind) DEFINE_FPR64_ALT(reg, reg, generic_kind)
+
+#define DEFINE_FPR64_ALT(reg, alt, generic_kind) DEFINE_FPR_ALT(reg, alt, 8, generic_kind)
+
+#define DEFINE_FPR_ALT(reg, alt, size, generic_kind)   \
   {\
-#reg, nullptr, 8, FPR_OFFSET(fpr_##reg##_riscv - fpr_first_risc

[Lldb-commits] [PATCH] D137873: [LLDB][Minidump] Set abi environment for windows.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D137873#3933171 , @alvinhochun 
wrote:

> I think the module spec of the modules should reflect the value of the 
> `plugin.object-file.pe-coff.abi` setting. Can ProcessMinidump take the module 
> spec of the executable module to use its target environment?

That's kinda what `Target::MergeArchitecture` is for.

> Mixing MSVC and MinGW DLLs is a tricky case so I will not be surprised if 
> something doesn't work correctly. It would be nice to have it working but 
> that would be outside the scope of this review. (I don't yet have a use case 
> for this but I am getting close with some COM / WinRT experiments.)

Sounds fun. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137873

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

[Lldb-commits] [PATCH] D134033: [lldb/Plugins] Improve error reporting when reading memory in Scripted Process

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I kinda like it. One thing that I think would help with the readability is if 
the "transformation" methods were grouped according to the type of the object 
being transformed, rather than according to the direction. So something like:

  // general transform
  // general reverse
  // Status transform
  // Status reverse

instead of

  // general transform
  // Status transform
  // general reverse
  // Status reverse

Also I don't think that these structs (`transformation`, 
`reverse_transformation`) wrapping the transformation functions are really 
necessary. It's true that one cannot partially specialize functions, but one of 
the reasons for that is this is normally not necessary -- regular function 
overloading  can do most of that as well.




Comment at: lldb/bindings/python/python-swigsafecast.swig:36
 
+PythonObject ToSWIGWrapper(Status& status) {
+  return ToSWIGHelper(new lldb::SBError(status), SWIGTYPE_p_lldb__SBError);

add `const` ?



Comment at: lldb/source/API/SBError.cpp:29
+SBError::SBError(const lldb_private::Status &status)
+: m_opaque_up(new Status(status.ToError())) {
+  LLDB_INSTRUMENT_VA(this, status);

That's cute, but you can just call the copy constructor (`new Status(status)`)



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp:124
+  Status py_error;
+  // TODO: Log py_error after call if failed
+  return Dispatch("read_memory_at_address", py_error,

or assign it to the `error` argument instead?



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h:11-20
 
-#include "lldb/Host/Config.h"
-
-#if LLDB_ENABLE_PYTHON
+#include 
+#include 
+#include 
+#include 
 
+#include "lldb/Host/Config.h"

move this inside `#if LLDB_ENABLE_PYTHON`



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h:91-119
 if (sizeof...(Args) > 0) {
-  FormatArgs(format_buffer, args...);
+  std::apply(
+  [&format_buffer, this](auto... args) {
+this->FormatArgs(format_buffer, args...);
+  },
+  transformed_args);
   // TODO: make `const char *` when removing support for Python 2.

Can this be a call to `PythonObject::CallMethod` (via std::apply and 
everything). If not, why not?



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h:150
+// Call SWIG Wrapper function
+python::PythonObject py_obj = python::ToSWIGWrapper(arg);
+return py_obj.release();

PythonObject::CallMethod knows how to unwrap a PythonObject argument, so 
ideally, we'd return py_obj directly here.


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

https://reviews.llvm.org/D134033

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

[Lldb-commits] [PATCH] D138181: [test] Allow skipTestIfFn to apply to entire classes for skipIfNoSBHeaders

[Jordan Rupprecht via Phabricator via lldb-commits]

rupprecht updated this revision to Diff 476137.
rupprecht marked an inline comment as done.
rupprecht added a comment.

- Use named args to indicate reason is used as condition


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138181

Files:
  lldb/packages/Python/lldbsuite/test/decorators.py
  lldb/test/API/api/check_public_api_headers/TestPublicAPIHeaders.py
  lldb/test/API/api/multithreaded/TestMultithreaded.py
  lldb/test/API/functionalities/plugins/command_plugin/TestPluginCommands.py

Index: lldb/test/API/functionalities/plugins/command_plugin/TestPluginCommands.py
===
--- lldb/test/API/functionalities/plugins/command_plugin/TestPluginCommands.py
+++ lldb/test/API/functionalities/plugins/command_plugin/TestPluginCommands.py
@@ -12,7 +12,6 @@
 
 def setUp(self):
 TestBase.setUp(self)
-self.generateSource('plugin.cpp')
 
 @skipIfNoSBHeaders
 # Requires a compatible arch and platform to link against the host's built
@@ -22,6 +21,7 @@
 @no_debug_info_test
 def test_load_plugin(self):
 """Test that plugins that load commands work correctly."""
+self.generateSource('plugin.cpp')
 
 plugin_name = "plugin"
 if sys.platform.startswith("darwin"):
Index: lldb/test/API/api/multithreaded/TestMultithreaded.py
===
--- lldb/test/API/api/multithreaded/TestMultithreaded.py
+++ lldb/test/API/api/multithreaded/TestMultithreaded.py
@@ -9,6 +9,7 @@
 from lldbsuite.test import lldbutil
 
 
+@skipIfNoSBHeaders
 class SBBreakpointCallbackCase(TestBase):
 
 NO_DEBUG_INFO_TESTCASE = True
@@ -25,7 +26,6 @@
 self.generateSource('test_stop-hook.cpp')
 
 @skipIfRemote
-@skipIfNoSBHeaders
 # clang-cl does not support throw or catch (llvm.org/pr24538)
 @skipIfWindows
 def test_python_stop_hook(self):
@@ -34,7 +34,6 @@
 'test_python_stop_hook')
 
 @skipIfRemote
-@skipIfNoSBHeaders
 # clang-cl does not support throw or catch (llvm.org/pr24538)
 @skipIfWindows
 def test_breakpoint_callback(self):
@@ -43,7 +42,6 @@
 'test_breakpoint_callback')
 
 @skipIfRemote
-@skipIfNoSBHeaders
 # clang-cl does not support throw or catch (llvm.org/pr24538)
 @skipIfWindows
 def test_breakpoint_location_callback(self):
@@ -52,7 +50,6 @@
 'test_breakpoint_location_callback')
 
 @skipIfRemote
-@skipIfNoSBHeaders
 # clang-cl does not support throw or catch (llvm.org/pr24538)
 @skipIfWindows
 @expectedFlakeyFreeBSD
@@ -63,7 +60,6 @@
 'test_listener_event_description')
 
 @skipIfRemote
-@skipIfNoSBHeaders
 # clang-cl does not support throw or catch (llvm.org/pr24538)
 @skipIfWindows
 @expectedFlakeyFreeBSD
@@ -76,7 +72,6 @@
 'test_listener_event_process_state')
 
 @skipIfRemote
-@skipIfNoSBHeaders
 # clang-cl does not support throw or catch (llvm.org/pr24538)
 @skipIfWindows
 @expectedFlakeyFreeBSD
Index: lldb/test/API/api/check_public_api_headers/TestPublicAPIHeaders.py
===
--- lldb/test/API/api/check_public_api_headers/TestPublicAPIHeaders.py
+++ lldb/test/API/api/check_public_api_headers/TestPublicAPIHeaders.py
@@ -8,6 +8,7 @@
 from lldbsuite.test import lldbutil
 
 
+@skipIfNoSBHeaders
 class SBDirCheckerCase(TestBase):
 NO_DEBUG_INFO_TESTCASE = True
 
@@ -16,7 +17,6 @@
 self.source = 'main.cpp'
 self.generateSource(self.source)
 
-@skipIfNoSBHeaders
 def test_sb_api_directory(self):
 """Test the SB API directory and make sure there's no unwanted stuff."""
 
Index: lldb/packages/Python/lldbsuite/test/decorators.py
===
--- lldb/packages/Python/lldbsuite/test/decorators.py
+++ lldb/packages/Python/lldbsuite/test/decorators.py
@@ -140,8 +140,10 @@
 def skipTestIfFn(expected_fn, bugnumber=None):
 def skipTestIfFn_impl(func):
 if isinstance(func, type) and issubclass(func, unittest2.TestCase):
-raise Exception(
-"@skipTestIfFn can only be used to decorate a test method")
+reason = expected_fn()
+# The return value is the reason (or None if we don't skip), so
+# reason is used for both args.
+return unittest2.skipIf(condition=reason, reason=reason)(func)
 
 @wraps(func)
 def wrapper(*args, **kwargs):
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D138181: [test] Allow skipTestIfFn to apply to entire classes for skipIfNoSBHeaders

[Jordan Rupprecht via Phabricator via lldb-commits]

rupprecht added a comment.

Thanks!




Comment at: lldb/packages/Python/lldbsuite/test/decorators.py:146
+# reason is used for both args.
+return unittest2.skipIf(reason, reason)(func)
 

DavidSpickett wrote:
> This would make more immediate sense if you added the function prototype as 
> well:
> ```
> # skipIf(condition, reason)
> ```
> 
> Or if you can figure a way to do the C++ style `skipIf(/*condition=*/reason, 
> reason)`.
> 
We can use named args. Both the standardized `skipIf` [1] and LLDB's fork [2] 
use the same param names so we should be fine if we ever want to drop our fork 
and use standard python (I wonder if we should do that?)

I still left the comment because I feel like "condition=reason" is still 
atypical, but using named args makes it less load-bearing of a comment.

[1] 
https://docs.python.org/3/library/unittest.html?highlight=skipif#unittest.skipIf
[2] 
https://github.com/llvm/llvm-project/blob/0dff945bbca9e1c00ac76eafd1af61235272b7dd/lldb/third_party/Python/module/unittest2/unittest2/case.py#L86


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138181

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

[Lldb-commits] [lldb] 8be41c7 - [test] Allow skipTestIfFn to apply to entire classes for skipIfNoSBHeaders

[Jordan Rupprecht via lldb-commits]

Author: Jordan Rupprecht
Date: 2022-11-17T08:17:10-08:00
New Revision: 8be41c787f9e12a4eb101e2dcad67ddc2da7ec1f

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

LOG: [test] Allow skipTestIfFn to apply to entire classes for skipIfNoSBHeaders

Some test cases are already marked @skipIfNoSBHeaders, but they make use of 
SBAPI headers in test setup. The setup will fail if the headers are missing, so 
it is too late to wait until the test case to apply the skip annotation.

In addition to allowing this to apply to entire classes, I also changed all the 
existing annotations from test cases to test classes where 
necessary/appropriate.

Reviewed By: DavidSpickett

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

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/decorators.py
lldb/test/API/api/check_public_api_headers/TestPublicAPIHeaders.py
lldb/test/API/api/multithreaded/TestMultithreaded.py
lldb/test/API/functionalities/plugins/command_plugin/TestPluginCommands.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/decorators.py 
b/lldb/packages/Python/lldbsuite/test/decorators.py
index dd47f6845ef2f..0c5f55f66246a 100644
--- a/lldb/packages/Python/lldbsuite/test/decorators.py
+++ b/lldb/packages/Python/lldbsuite/test/decorators.py
@@ -140,8 +140,10 @@ def wrapper(*args, **kwargs):
 def skipTestIfFn(expected_fn, bugnumber=None):
 def skipTestIfFn_impl(func):
 if isinstance(func, type) and issubclass(func, unittest2.TestCase):
-raise Exception(
-"@skipTestIfFn can only be used to decorate a test method")
+reason = expected_fn()
+# The return value is the reason (or None if we don't skip), so
+# reason is used for both args.
+return unittest2.skipIf(condition=reason, reason=reason)(func)
 
 @wraps(func)
 def wrapper(*args, **kwargs):

diff  --git 
a/lldb/test/API/api/check_public_api_headers/TestPublicAPIHeaders.py 
b/lldb/test/API/api/check_public_api_headers/TestPublicAPIHeaders.py
index 1980b1434595f..b74e395b3e671 100644
--- a/lldb/test/API/api/check_public_api_headers/TestPublicAPIHeaders.py
+++ b/lldb/test/API/api/check_public_api_headers/TestPublicAPIHeaders.py
@@ -8,6 +8,7 @@
 from lldbsuite.test import lldbutil
 
 
+@skipIfNoSBHeaders
 class SBDirCheckerCase(TestBase):
 NO_DEBUG_INFO_TESTCASE = True
 
@@ -16,7 +17,6 @@ def setUp(self):
 self.source = 'main.cpp'
 self.generateSource(self.source)
 
-@skipIfNoSBHeaders
 def test_sb_api_directory(self):
 """Test the SB API directory and make sure there's no unwanted 
stuff."""
 

diff  --git a/lldb/test/API/api/multithreaded/TestMultithreaded.py 
b/lldb/test/API/api/multithreaded/TestMultithreaded.py
index 33759906243a4..c8e89b580c40e 100644
--- a/lldb/test/API/api/multithreaded/TestMultithreaded.py
+++ b/lldb/test/API/api/multithreaded/TestMultithreaded.py
@@ -9,6 +9,7 @@
 from lldbsuite.test import lldbutil
 
 
+@skipIfNoSBHeaders
 class SBBreakpointCallbackCase(TestBase):
 
 NO_DEBUG_INFO_TESTCASE = True
@@ -25,7 +26,6 @@ def setUp(self):
 self.generateSource('test_stop-hook.cpp')
 
 @skipIfRemote
-@skipIfNoSBHeaders
 # clang-cl does not support throw or catch (llvm.org/pr24538)
 @skipIfWindows
 def test_python_stop_hook(self):
@@ -34,7 +34,6 @@ def test_python_stop_hook(self):
 'test_python_stop_hook')
 
 @skipIfRemote
-@skipIfNoSBHeaders
 # clang-cl does not support throw or catch (llvm.org/pr24538)
 @skipIfWindows
 def test_breakpoint_callback(self):
@@ -43,7 +42,6 @@ def test_breakpoint_callback(self):
 'test_breakpoint_callback')
 
 @skipIfRemote
-@skipIfNoSBHeaders
 # clang-cl does not support throw or catch (llvm.org/pr24538)
 @skipIfWindows
 def test_breakpoint_location_callback(self):
@@ -52,7 +50,6 @@ def test_breakpoint_location_callback(self):
 'test_breakpoint_location_callback')
 
 @skipIfRemote
-@skipIfNoSBHeaders
 # clang-cl does not support throw or catch (llvm.org/pr24538)
 @skipIfWindows
 @expectedFlakeyFreeBSD
@@ -63,7 +60,6 @@ def test_sb_api_listener_event_description(self):
 'test_listener_event_description')
 
 @skipIfRemote
-@skipIfNoSBHeaders
 # clang-cl does not support throw or catch (llvm.org/pr24538)
 @skipIfWindows
 @expectedFlakeyFreeBSD
@@ -76,7 +72,6 @@ def test_sb_api_listener_event_process_state(self):
 'test_listener_event_process_state')
 
 @skipIfRemote
-@skipIfNoSBHeaders
 # clang-cl does not support throw or catch (llvm.org/pr24538)
 @skipIfWindows
 @expectedFlakeyFreeBSD

[Lldb-commits] [PATCH] D138181: [test] Allow skipTestIfFn to apply to entire classes for skipIfNoSBHeaders

[Jordan Rupprecht 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 rG8be41c787f9e: [test] Allow skipTestIfFn to apply to entire 
classes for skipIfNoSBHeaders (authored by rupprecht).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138181

Files:
  lldb/packages/Python/lldbsuite/test/decorators.py
  lldb/test/API/api/check_public_api_headers/TestPublicAPIHeaders.py
  lldb/test/API/api/multithreaded/TestMultithreaded.py
  lldb/test/API/functionalities/plugins/command_plugin/TestPluginCommands.py

Index: lldb/test/API/functionalities/plugins/command_plugin/TestPluginCommands.py
===
--- lldb/test/API/functionalities/plugins/command_plugin/TestPluginCommands.py
+++ lldb/test/API/functionalities/plugins/command_plugin/TestPluginCommands.py
@@ -12,7 +12,6 @@
 
 def setUp(self):
 TestBase.setUp(self)
-self.generateSource('plugin.cpp')
 
 @skipIfNoSBHeaders
 # Requires a compatible arch and platform to link against the host's built
@@ -22,6 +21,7 @@
 @no_debug_info_test
 def test_load_plugin(self):
 """Test that plugins that load commands work correctly."""
+self.generateSource('plugin.cpp')
 
 plugin_name = "plugin"
 if sys.platform.startswith("darwin"):
Index: lldb/test/API/api/multithreaded/TestMultithreaded.py
===
--- lldb/test/API/api/multithreaded/TestMultithreaded.py
+++ lldb/test/API/api/multithreaded/TestMultithreaded.py
@@ -9,6 +9,7 @@
 from lldbsuite.test import lldbutil
 
 
+@skipIfNoSBHeaders
 class SBBreakpointCallbackCase(TestBase):
 
 NO_DEBUG_INFO_TESTCASE = True
@@ -25,7 +26,6 @@
 self.generateSource('test_stop-hook.cpp')
 
 @skipIfRemote
-@skipIfNoSBHeaders
 # clang-cl does not support throw or catch (llvm.org/pr24538)
 @skipIfWindows
 def test_python_stop_hook(self):
@@ -34,7 +34,6 @@
 'test_python_stop_hook')
 
 @skipIfRemote
-@skipIfNoSBHeaders
 # clang-cl does not support throw or catch (llvm.org/pr24538)
 @skipIfWindows
 def test_breakpoint_callback(self):
@@ -43,7 +42,6 @@
 'test_breakpoint_callback')
 
 @skipIfRemote
-@skipIfNoSBHeaders
 # clang-cl does not support throw or catch (llvm.org/pr24538)
 @skipIfWindows
 def test_breakpoint_location_callback(self):
@@ -52,7 +50,6 @@
 'test_breakpoint_location_callback')
 
 @skipIfRemote
-@skipIfNoSBHeaders
 # clang-cl does not support throw or catch (llvm.org/pr24538)
 @skipIfWindows
 @expectedFlakeyFreeBSD
@@ -63,7 +60,6 @@
 'test_listener_event_description')
 
 @skipIfRemote
-@skipIfNoSBHeaders
 # clang-cl does not support throw or catch (llvm.org/pr24538)
 @skipIfWindows
 @expectedFlakeyFreeBSD
@@ -76,7 +72,6 @@
 'test_listener_event_process_state')
 
 @skipIfRemote
-@skipIfNoSBHeaders
 # clang-cl does not support throw or catch (llvm.org/pr24538)
 @skipIfWindows
 @expectedFlakeyFreeBSD
Index: lldb/test/API/api/check_public_api_headers/TestPublicAPIHeaders.py
===
--- lldb/test/API/api/check_public_api_headers/TestPublicAPIHeaders.py
+++ lldb/test/API/api/check_public_api_headers/TestPublicAPIHeaders.py
@@ -8,6 +8,7 @@
 from lldbsuite.test import lldbutil
 
 
+@skipIfNoSBHeaders
 class SBDirCheckerCase(TestBase):
 NO_DEBUG_INFO_TESTCASE = True
 
@@ -16,7 +17,6 @@
 self.source = 'main.cpp'
 self.generateSource(self.source)
 
-@skipIfNoSBHeaders
 def test_sb_api_directory(self):
 """Test the SB API directory and make sure there's no unwanted stuff."""
 
Index: lldb/packages/Python/lldbsuite/test/decorators.py
===
--- lldb/packages/Python/lldbsuite/test/decorators.py
+++ lldb/packages/Python/lldbsuite/test/decorators.py
@@ -140,8 +140,10 @@
 def skipTestIfFn(expected_fn, bugnumber=None):
 def skipTestIfFn_impl(func):
 if isinstance(func, type) and issubclass(func, unittest2.TestCase):
-raise Exception(
-"@skipTestIfFn can only be used to decorate a test method")
+reason = expected_fn()
+# The return value is the reason (or None if we don't skip), so
+# reason is used for both args.
+return unittest2.skipIf(condition=reason, reason=reason)(func)
 
 @wraps(func)
 def wrapper(*args, **kwargs):
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D138176: [dsymutil] Fix assertion: (Ref > InputDIE.getOffset()), function cloneDieReferenceAttribute

[Alexey Lapshin via Phabricator via lldb-commits]

avl added inline comments.



Comment at: llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h:82
+/// Is this a reference to a DIE that hasn't been cloned yet?
+bool Reference : 1;
   };

Probably name it UnclonedReference ? or RefShouldBePatched?(or something like 
that)



Comment at: llvm/lib/DWARFLinker/DWARFLinkerCompileUnit.cpp:79
 
 /// Keep track of a forward cross-cu reference from this unit
 /// to \p Die that lives in \p RefUnit.

/// Keep track of a cross-cu reference from this unit


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

https://reviews.llvm.org/D138176

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

[Lldb-commits] [PATCH] D138176: [dsymutil] Fix assertion: (Ref > InputDIE.getOffset()), function cloneDieReferenceAttribute

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 476158.
JDevlieghere marked 2 inline comments as done.
JDevlieghere added a comment.

Address @avl's feedback


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

https://reviews.llvm.org/D138176

Files:
  llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
  llvm/lib/DWARFLinker/DWARFLinker.cpp
  llvm/lib/DWARFLinker/DWARFLinkerCompileUnit.cpp

Index: llvm/lib/DWARFLinker/DWARFLinkerCompileUnit.cpp
===
--- llvm/lib/DWARFLinker/DWARFLinkerCompileUnit.cpp
+++ llvm/lib/DWARFLinker/DWARFLinkerCompileUnit.cpp
@@ -76,15 +76,15 @@
   return NextUnitOffset;
 }
 
-/// Keep track of a forward cross-cu reference from this unit
+/// Keep track of a cross-cu reference from this unit
 /// to \p Die that lives in \p RefUnit.
-void CompileUnit::noteForwardReference(DIE *Die, const CompileUnit *RefUnit,
-   DeclContext *Ctxt, PatchLocation Attr) {
-  ForwardDIEReferences.emplace_back(Die, RefUnit, Ctxt, Attr);
+void CompileUnit::noteReference(DIE *Die, const CompileUnit *RefUnit,
+DeclContext *Ctxt, PatchLocation Attr) {
+  DIEReferences.emplace_back(Die, RefUnit, Ctxt, Attr);
 }
 
-void CompileUnit::fixupForwardReferences() {
-  for (const auto &Ref : ForwardDIEReferences) {
+void CompileUnit::fixupReferences() {
+  for (const auto &Ref : DIEReferences) {
 DIE *RefDie;
 const CompileUnit *RefUnit;
 PatchLocation Attr;
Index: llvm/lib/DWARFLinker/DWARFLinker.cpp
===
--- llvm/lib/DWARFLinker/DWARFLinker.cpp
+++ llvm/lib/DWARFLinker/DWARFLinker.cpp
@@ -934,9 +934,9 @@
   }
 
   if (!RefInfo.Clone) {
-assert(Ref > InputDIE.getOffset());
 // We haven't cloned this DIE yet. Just create an empty one and
 // store it. It'll get really cloned when we process it.
+RefInfo.UnclonedReference = true;
 RefInfo.Clone = DIE::get(DIEAlloc, dwarf::Tag(RefDie.getTag()));
   }
   NewRefDie = RefInfo.Clone;
@@ -949,20 +949,20 @@
 // FIXME: we should be able to design DIEEntry reliance on
 // DwarfDebug away.
 uint64_t Attr;
-if (Ref < InputDIE.getOffset()) {
-  // We must have already cloned that DIE.
+if (Ref < InputDIE.getOffset() && !RefInfo.UnclonedReference) {
+  // We have already cloned that DIE.
   uint32_t NewRefOffset =
   RefUnit->getStartOffset() + NewRefDie->getOffset();
   Attr = NewRefOffset;
   Die.addValue(DIEAlloc, dwarf::Attribute(AttrSpec.Attr),
dwarf::DW_FORM_ref_addr, DIEInteger(Attr));
 } else {
-  // A forward reference. Note and fixup later.
+  // A forward or backward reference. Note and fixup later.
   Attr = 0xBADDEF;
-  Unit.noteForwardReference(
-  NewRefDie, RefUnit, RefInfo.Ctxt,
-  Die.addValue(DIEAlloc, dwarf::Attribute(AttrSpec.Attr),
-   dwarf::DW_FORM_ref_addr, DIEInteger(Attr)));
+  Unit.noteReference(NewRefDie, RefUnit, RefInfo.Ctxt,
+ Die.addValue(DIEAlloc, dwarf::Attribute(AttrSpec.Attr),
+  dwarf::DW_FORM_ref_addr,
+  DIEInteger(Attr)));
 }
 return U.getRefAddrByteSize();
   }
@@ -2237,7 +2237,7 @@
   if (LLVM_LIKELY(!Linker.Options.Update))
 Linker.generateUnitRanges(*CurrentUnit);
 
-  CurrentUnit->fixupForwardReferences();
+  CurrentUnit->fixupReferences();
 
   if (!CurrentUnit->getOutputUnitDIE())
 continue;
Index: llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
===
--- llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
+++ llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
@@ -77,6 +77,9 @@
 
 /// Is ODR marking done?
 bool ODRMarkingDone : 1;
+
+/// Is this a reference to a DIE that hasn't been cloned yet?
+bool UnclonedReference : 1;
   };
 
   CompileUnit(DWARFUnit &OrigUnit, unsigned ID, bool CanUseODR,
@@ -160,15 +163,15 @@
   /// current debug_info section size).
   uint64_t computeNextUnitOffset(uint16_t DwarfVersion);
 
-  /// Keep track of a forward reference to DIE \p Die in \p RefUnit by \p
-  /// Attr. The attribute should be fixed up later to point to the absolute
-  /// offset of \p Die in the debug_info section or to the canonical offset of
-  /// \p Ctxt if it is non-null.
-  void noteForwardReference(DIE *Die, const CompileUnit *RefUnit,
-DeclContext *Ctxt, PatchLocation Attr);
+  /// Keep track of a forward or backward reference to DIE \p Die in \p RefUnit
+  /// by \p Attr. The attribute should be fixed up later to point to the
+  /// absolute offset of \p Die in the debug_info section or to the canonical
+  /// offset of \p Ctxt if it is non-null.
+  void noteReference(DIE *Die, const CompileUnit *RefUnit, Decl

[Lldb-commits] [PATCH] D138060: Improve error logging when xcrun fails to execute successfully

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm:13
 #include "lldb/Host/macosx/HostInfoMacOSX.h"
+#include "lldb/Core/Debugger.h"
 #include "lldb/Utility/Args.h"

aprantl wrote:
> labath wrote:
> > We shouldn't have Host->Core dependencies. Could you report the error in 
> > the caller?
> How strong to you fell about this?
> It would be an odd interface. `GetXcodeSDKPath` also caches negative results 
> because negative lookups are really slow. This means we would only raise an 
> error on the first call and not on the subsequent ones. I can do that, but it 
> also feels weird. I guess we could cache the error string as well?
I would say pretty strongly. While we still have cycles in LLDB, we haven't 
introduced new one,  at least not that I'm aware off. The diagnostics can be 
fixed by percolating the error up further and dealing with the errors at the 
call site, which presumably can depend on Host. Caching the error sounds 
reasonable to me. 

That approach doesn't work for the progress updates though. I can imagine that 
there's more places in LLDB that don't rely on Core from where we'd like to 
report progress. One potential solution to that is moving Progress to Utility 
and replacing the dependency on Core with a callback. Similar to the 
diagnostics we could make it something that follows the Initialize/Terminate 
patter and then keeps a static list of callbacks for reporting progress. 
Definitely not the most elegant solution, but it could work for both Progress 
and Diagnostics if we wanted to. 


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

https://reviews.llvm.org/D138060

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

[Lldb-commits] [PATCH] D131075: [lldb] [gdb-remote] Send interrupt packets from async thread

[Ed Maste via Phabricator via lldb-commits]

emaste added a comment.

With D131159  and D131160 
 in, where does that leave us wrt this change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131075

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

[Lldb-commits] [PATCH] D135030: [lldb] [gdb-remote] Abstract sending ^c packet into SendCtrlC() method

[Ed Maste via Phabricator via lldb-commits]

emaste added a comment.

Maybe we want to call it SendInterrupt? Looks fine to me as a minor tidy-up 
either way.


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

https://reviews.llvm.org/D135030

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

[Lldb-commits] [lldb] 0acc08b - Attempt ro fix Windows build errors.

[Adrian Prantl via lldb-commits]

Author: Adrian Prantl
Date: 2022-11-17T10:50:34-08:00
New Revision: 0acc08b4eabdeb9b681259921df9e1b554a927e2

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

LOG: Attempt ro fix Windows build errors.

Added: 


Modified: 
lldb/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp

Removed: 




diff  --git a/lldb/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp 
b/lldb/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
index 7c7d1902eefb9..7957c83044c1b 100644
--- a/lldb/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
+++ b/lldb/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
@@ -391,7 +391,7 @@ TEST_F(SymbolFilePDBTests, TestNestedClassTypes) {
   ASSERT_THAT_EXPECTED(clang_ast_ctx_or_err, llvm::Succeeded());
 
   auto clang_ast_ctx =
-  llvm::dyn_cast_or_null(&clang_ast_ctx_or_err.get());
+  llvm::dyn_cast_or_null(clang_ast_ctx_or_err->get());
   EXPECT_NE(nullptr, clang_ast_ctx);
 
   symfile->FindTypes(ConstString("Class"), CompilerDeclContext(), 0,
@@ -445,7 +445,7 @@ TEST_F(SymbolFilePDBTests, TestClassInNamespace) {
   ASSERT_THAT_EXPECTED(clang_ast_ctx_or_err, llvm::Succeeded());
 
   auto clang_ast_ctx =
-  llvm::dyn_cast_or_null(&clang_ast_ctx_or_err.get());
+  llvm::dyn_cast_or_null(clang_ast_ctx_or_err->get());
   EXPECT_NE(nullptr, clang_ast_ctx);
 
   clang::ASTContext &ast_ctx = clang_ast_ctx->getASTContext();
@@ -540,8 +540,8 @@ TEST_F(SymbolFilePDBTests, TestTypedefs) {
 lldb::TypeSP typedef_type = results.GetTypeAtIndex(0);
 EXPECT_EQ(ConstString(Typedef), typedef_type->GetName());
 CompilerType compiler_type = typedef_type->GetFullCompilerType();
-TypeSystemClang *clang_type_system =
-llvm::dyn_cast_or_null(compiler_type.GetTypeSystem());
+auto clang_type_system =
+compiler_type.GetTypeSystem().dyn_cast_or_null();
 EXPECT_TRUE(
 clang_type_system->IsTypedefType(compiler_type.GetOpaqueQualType()));
 



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

[Lldb-commits] [PATCH] D136650: Make CompilerType safe [Was: Add a check for TypeSystem use-after-free problems]

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

In D136650#3932481 , 
@stella.stamenova wrote:

> Looks like this broke the windows lldb bot: 
> https://lab.llvm.org/buildbot/#/builders/83/builds/26042

I'm trying to fix this blindly. Should hopefully work after a few iterations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136650

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

[Lldb-commits] [PATCH] D136650: Make CompilerType safe [Was: Add a check for TypeSystem use-after-free problems]

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova added a comment.

Looks like the windows lldb bot is still broken: 
https://lab.llvm.org/buildbot/#/builders/83/builds/26083


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136650

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

[Lldb-commits] [PATCH] D137983: [lldb] Disable looking at pointee types to find synthetic value for non-ObjC

[Arthur Eubanks via Phabricator via lldb-commits]

aeubanks updated this revision to Diff 476199.
aeubanks added a comment.

add test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137983

Files:
  lldb/source/Core/ValueObject.cpp
  lldb/test/API/lang/cpp/incomplete-stl-types/Makefile
  lldb/test/API/lang/cpp/incomplete-stl-types/TestStlIncompleteTypes.py
  lldb/test/API/lang/cpp/incomplete-stl-types/f.cpp
  lldb/test/API/lang/cpp/incomplete-stl-types/main.cpp


Index: lldb/test/API/lang/cpp/incomplete-stl-types/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/incomplete-stl-types/main.cpp
@@ -0,0 +1,8 @@
+#include 
+
+void f(std::set &v);
+
+int main() {
+  std::set v;
+  f(v);
+}
Index: lldb/test/API/lang/cpp/incomplete-stl-types/f.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/incomplete-stl-types/f.cpp
@@ -0,0 +1,5 @@
+#include 
+
+void f(std::set &v) {
+  // break here
+}
Index: lldb/test/API/lang/cpp/incomplete-stl-types/TestStlIncompleteTypes.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/incomplete-stl-types/TestStlIncompleteTypes.py
@@ -0,0 +1,18 @@
+"""
+Test situations where the debug info only has a declaration, no definition, for
+an STL container with a formatter.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestStlIncompleteTypes(TestBase):
+def test(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here", 
lldb.SBFileSpec("f.cpp"))
+
+var = self.frame().GetValueForVariablePath("v")
+self.assertIn("set", var.GetDisplayTypeName())
Index: lldb/test/API/lang/cpp/incomplete-stl-types/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/cpp/incomplete-stl-types/Makefile
@@ -0,0 +1,9 @@
+CXX_SOURCES := main.cpp f.cpp
+
+include Makefile.rules
+
+# Force main.cpp to be built with no debug information
+main.o: CFLAGS = $(CFLAGS_NO_DEBUG)
+
+# And force -flimit-debug-info on the rest.
+f.o: CFLAGS_EXTRAS += $(LIMIT_DEBUG_INFO_FLAGS)
Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -2673,7 +2673,10 @@
 // In case of incomplete child compiler type, use the pointee type and try
 // to recreate a new ValueObjectChild using it.
 if (!m_deref_valobj) {
-  if (HasSyntheticValue()) {
+  // FIXME(#59012): C++ stdlib formatters break with incomplete types (e.g.
+  // `std::vector &`). Remove ObjC restriction once that's resolved.
+  if (Language::LanguageIsObjC(GetPreferredDisplayLanguage()) &&
+  HasSyntheticValue()) {
 child_compiler_type = compiler_type.GetPointeeType();
 
 if (child_compiler_type) {


Index: lldb/test/API/lang/cpp/incomplete-stl-types/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/incomplete-stl-types/main.cpp
@@ -0,0 +1,8 @@
+#include 
+
+void f(std::set &v);
+
+int main() {
+  std::set v;
+  f(v);
+}
Index: lldb/test/API/lang/cpp/incomplete-stl-types/f.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/incomplete-stl-types/f.cpp
@@ -0,0 +1,5 @@
+#include 
+
+void f(std::set &v) {
+  // break here
+}
Index: lldb/test/API/lang/cpp/incomplete-stl-types/TestStlIncompleteTypes.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/incomplete-stl-types/TestStlIncompleteTypes.py
@@ -0,0 +1,18 @@
+"""
+Test situations where the debug info only has a declaration, no definition, for
+an STL container with a formatter.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestStlIncompleteTypes(TestBase):
+def test(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here", lldb.SBFileSpec("f.cpp"))
+
+var = self.frame().GetValueForVariablePath("v")
+self.assertIn("set", var.GetDisplayTypeName())
Index: lldb/test/API/lang/cpp/incomplete-stl-types/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/cpp/incomplete-stl-types/Makefile
@@ -0,0 +1,9 @@
+CXX_SOURCES := main.cpp f.cpp
+
+include Makefile.rules
+
+# Force main.cpp to be built with no debug information
+main.o: CFLAGS = $(CFLAGS_NO_DEBUG)
+
+# And force -flimit-debug-info on the rest.
+f.o: CFLAGS_EXTRAS += $(LIMIT_DEBUG_INFO_FLAGS)
Index: lldb/source/Core/ValueObject.cpp
===

[Lldb-commits] [PATCH] D137983: [lldb] Disable looking at pointee types to find synthetic value for non-ObjC

[Arthur Eubanks via Phabricator via lldb-commits]

aeubanks added a comment.

In D137983#3930973 , @labath wrote:

> We have tests for shared libraries (grep for DYLIB_C(XX)_SOURCES)), but it's 
> easier to reproduce this problem by compiling one CU with -g0 -- see inline 
> comment. (If you are creating an "API" test for this, beware that their 
> default is to build everything with -fstandalone-debug, and you'll need to 
> explicitly change that (see LIMIT_DEBUG_INFO_FLAGS))

`lldb/test/API/lang/cpp/incomplete-types/` looks like exactly what I want, I've 
added a test based on that (and verified that lldb crashes in that test without 
the source change)

> This behavior (bug) is triggered by the `FrontEndWantsDereference` formatter 
> flag, which is why it manifests itself for (libc++) vector and friends. The 
> ObjC formatters don't set that flag so they should be safe. The libstdc++ 
> formatters don't set it either. Furthermore there doesn't seem to be a way to 
> set this flag by the user, so it is not possible to create a 
> libc++-independent reproducer (which is what I was going to suggest).
>
> I've been looking for a better way to fix this today (sorry about the delay), 
> but I haven't figured out anything better. There is this fundamental 
> recursion between the pretty printer (which wants to dereference a value) and 
> the dereferencing code (which wants to know if it has a pretty printer -- so 
> it can avoid dereferencing). Just removing the `HasSyntheticValue()`fixed the 
> bug for me -- but then we were able to "dereference" incomplete structs. It's 
> possible we may be able to allow the dereference to succeed here, but then 
> refuse to print the value somewhere down the line. I would like to hear what 
> @jingham things about all this.

Thanks for the investigation!
Actually some libstdc++ formatters do also set `FrontEndWantsDereference`, so 
this is reproable with

  $ cat /tmp/main.cc
  #include 
  
  void f(std::set& v);
  
  int main() {
  std::set v;
  f(v);
  }
  $ cat /tmp/a.cc
  #include 
  
  void f(std::set& v) {
  // break
  }
  $ bin/clang++ -g0 /tmp/main.cc -o /tmp/main.o  -c && bin/clang++ -g /tmp/a.cc 
-o /tmp/a.o  -c && bin/clang++ /tmp/a.o /tmp/main.o -o /tmp/a
  $ bin/lldb /tmp/a -b -o 'br set -n f'  -o run

with and without `-stdlib=libc++`

maybe a more targeted solution would be to change the added condition to when 
`FrontEndWantsDereference` is set, but that's more annoying to thread through 
the code. I think this should be ok as a temporary workaround if we don't have 
a proper fix soonish?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137983

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

[Lldb-commits] [PATCH] D138176: [dsymutil] Fix assertion: (Ref > InputDIE.getOffset()), function cloneDieReferenceAttribute

[Alexey Lapshin via Phabricator via lldb-commits]

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

LGTM


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

https://reviews.llvm.org/D138176

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

[Lldb-commits] [PATCH] D137873: [LLDB][Minidump] Set abi environment for windows.

[Zequan Wu via Phabricator via lldb-commits]

zequanwu updated this revision to Diff 476221.
zequanwu added a comment.

Use plugin.object-file.pe-coff.abi as minidump process abi.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137873

Files:
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
  lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
  lldb/test/Shell/Minidump/Windows/find-module.test

Index: lldb/test/Shell/Minidump/Windows/find-module.test
===
--- lldb/test/Shell/Minidump/Windows/find-module.test
+++ lldb/test/Shell/Minidump/Windows/find-module.test
@@ -4,7 +4,25 @@
 RUN: yaml2obj %S/Inputs/find-module.exe.yaml -o %T/find-module.exe
 RUN: yaml2obj %S/Inputs/find-module.dmp.yaml -o %T/find-module.dmp
 RUN: %lldb -O "settings set target.exec-search-paths %T" \
-RUN:   -c %T/find-module.dmp -o "image dump objfile" -o exit | FileCheck %s
+RUN:   -c %T/find-module.dmp -o "image dump objfile" -o "target list" -o exit \
+RUN:   | FileCheck --check-prefix=DEFAULT %s
 
-CHECK-LABEL: image dump objfile
-CHECK: ObjectFilePECOFF, file = '{{.*}}find-module.exe', arch = i386
+RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi msvc" \
+RUN:   -O "settings set target.exec-search-paths %T" -c %T/find-module.dmp \
+RUN:   -o "target list" -o exit | FileCheck --check-prefix=MSVC %s
+
+RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi gnu" \
+RUN:   -O "settings set target.exec-search-paths %T" -c %T/find-module.dmp \
+RUN:   -o "target list" -o exit | FileCheck --check-prefix=GNU %s
+
+DEFAULT-LABEL: image dump objfile
+DEFAULT: ObjectFilePECOFF, file = '{{.*}}find-module.exe', arch = i386
+
+DEFAULT-LABEL: target list
+DEFAULT: arch=i386-pc-windows-{{msvc|gnu}}
+
+MSVC-LABEL: target list
+MSVC: arch=i386-pc-windows-msvc
+
+GNU-LABEL: target list
+GNU: arch=i386-pc-windows-gnu
Index: lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
===
--- lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
+++ lldb/source/Plugins/Process/minidump/ProcessMinidump.cpp
@@ -8,6 +8,7 @@
 
 #include "ProcessMinidump.h"
 
+#include "Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h"
 #include "ThreadMinidump.h"
 
 #include "lldb/Core/DumpDataExtractor.h"
@@ -31,6 +32,7 @@
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/State.h"
 #include "llvm/BinaryFormat/Magic.h"
+#include "llvm/Support/Host.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Threading.h"
 
@@ -389,7 +391,23 @@
 
 ArchSpec ProcessMinidump::GetArchitecture() {
   if (!m_is_wow64) {
-return m_minidump_parser->GetArchitecture();
+ArchSpec arch = m_minidump_parser->GetArchitecture();
+if (arch.GetTriple().getOS() == llvm::Triple::OSType::Win32) {
+  static llvm::Triple::EnvironmentType default_env = [] {
+auto def_target = llvm::Triple(
+llvm::Triple::normalize(llvm::sys::getDefaultTargetTriple()));
+if (def_target.getOS() == llvm::Triple::Win32 &&
+def_target.getEnvironment() != llvm::Triple::UnknownEnvironment)
+  return def_target.getEnvironment();
+return llvm::Triple::MSVC;
+  }();
+  llvm::Triple::EnvironmentType env =
+  ObjectFilePECOFF::GetGlobalPluginEnvironment();
+  if (env == llvm::Triple::UnknownEnvironment)
+env = default_env;
+  arch.GetTriple().setEnvironment(env);
+}
+return arch;
   }
 
   llvm::Triple triple;
Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
@@ -88,6 +88,8 @@
 
   static lldb::SymbolType MapSymbolType(uint16_t coff_symbol_type);
 
+  static llvm::Triple::EnvironmentType GetGlobalPluginEnvironment();
+
   // LLVM RTTI support
   static char ID;
   bool isA(const void *ClassID) const override {
Index: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -389,6 +389,10 @@
   return lldb::eSymbolTypeInvalid;
 }
 
+llvm::Triple::EnvironmentType ObjectFilePECOFF::GetGlobalPluginEnvironment() {
+  return GetGlobalPluginProperties().ABI();
+}
+
 bool ObjectFilePECOFF::CreateBinary() {
   if (m_binary)
 return true;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D136650: Make CompilerType safe [Was: Add a check for TypeSystem use-after-free problems]

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added a comment.

I think it's fixed now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136650

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

[Lldb-commits] [PATCH] D138237: [lldb] Restore default setting of LLDB_INCLUDE_TESTS in standalone builds

[Alex Langford via Phabricator via lldb-commits]

bulbazord created this revision.
bulbazord added reviewers: labath, mgorny.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

In 52f39853abd46495a6d636c4b035e1b92cf4b833 
 the 
option `LLDB_INCLUDE_TESTS` was moved above the inclusion of `LLDBStandalone`. 
This isn't a problem per-se, but it changes the default value of 
`LLDB_INCLUDE_TESTS` in standalone builds. `LLDBStandalone` explicitly sets 
`LLVM_INCLUDE_TESTS` to true, indicating that for standalone builds this is 
considered the default behavior. This patch restores said default behavior.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138237

Files:
  lldb/CMakeLists.txt
  lldb/cmake/modules/LLDBStandalone.cmake


Index: lldb/cmake/modules/LLDBStandalone.cmake
===
--- lldb/cmake/modules/LLDBStandalone.cmake
+++ lldb/cmake/modules/LLDBStandalone.cmake
@@ -89,7 +89,6 @@
 include(LLVMDistributionSupport)
 
 set(PACKAGE_VERSION "${LLVM_PACKAGE_VERSION}")
-set(LLVM_INCLUDE_TESTS ON CACHE INTERNAL "")
 
 set(CMAKE_INCLUDE_CURRENT_DIR ON)
 include_directories(
Index: lldb/CMakeLists.txt
===
--- lldb/CMakeLists.txt
+++ lldb/CMakeLists.txt
@@ -18,6 +18,7 @@
 if (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
   project(lldb)
   set(LLDB_BUILT_STANDALONE TRUE)
+  set(LLVM_INCLUDE_TESTS ON CACHE INTERNAL "")
 endif()
 
 # Must go below project(..)


Index: lldb/cmake/modules/LLDBStandalone.cmake
===
--- lldb/cmake/modules/LLDBStandalone.cmake
+++ lldb/cmake/modules/LLDBStandalone.cmake
@@ -89,7 +89,6 @@
 include(LLVMDistributionSupport)
 
 set(PACKAGE_VERSION "${LLVM_PACKAGE_VERSION}")
-set(LLVM_INCLUDE_TESTS ON CACHE INTERNAL "")
 
 set(CMAKE_INCLUDE_CURRENT_DIR ON)
 include_directories(
Index: lldb/CMakeLists.txt
===
--- lldb/CMakeLists.txt
+++ lldb/CMakeLists.txt
@@ -18,6 +18,7 @@
 if (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
   project(lldb)
   set(LLDB_BUILT_STANDALONE TRUE)
+  set(LLVM_INCLUDE_TESTS ON CACHE INTERNAL "")
 endif()
 
 # Must go below project(..)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] d2b6b21 - Comment function

[Adrian Prantl via lldb-commits]

Author: Adrian Prantl
Date: 2022-11-17T15:24:48-08:00
New Revision: d2b6b2147b6123f7c470457ed3c06ffebcc9231d

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

LOG: Comment function

Added: 


Modified: 
lldb/include/lldb/Core/Module.h

Removed: 




diff  --git a/lldb/include/lldb/Core/Module.h b/lldb/include/lldb/Core/Module.h
index 87caf1cde0d36..f7c5971c2f1de 100644
--- a/lldb/include/lldb/Core/Module.h
+++ b/lldb/include/lldb/Core/Module.h
@@ -815,6 +815,8 @@ class Module : public std::enable_shared_from_this,
   llvm::Expected
   GetTypeSystemForLanguage(lldb::LanguageType language);
 
+  /// Call \p callback for each \p TypeSystem in this \p Module.
+  /// Return true from callback to keep iterating, false to stop iterating.
   void ForEachTypeSystem(llvm::function_ref 
callback);
 
   // Special error functions that can do printf style formatting that will



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

[Lldb-commits] [PATCH] D138248: [lldb/Python] Make use of PythonObject and PythonFormat in callbacks (NFC)

[Med Ismail Bennani via Phabricator via lldb-commits]

mib created this revision.
mib added reviewers: bulbazord, JDevlieghere.
mib added a project: LLDB.
Herald added a project: All.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch extends the template specialization of PythonFormat structs
and makes use of the pre-existing PythonObject class to format arguments
and pass them to the right method, before calling it.

This is a preparatory patch to merge PythonFormat with SWIGPythonBridge's
GetPythonValueFormatString methods.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138248

Files:
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -1501,50 +1501,27 @@
 StructuredData::ObjectSP os_plugin_object_sp) {
   Locker py_lock(this, Locker::AcquireLock | Locker::NoSTDIN, Locker::FreeLock);
 
-  static char callee_name[] = "get_register_info";
-
   if (!os_plugin_object_sp)
-return StructuredData::DictionarySP();
+return {};
 
   StructuredData::Generic *generic = os_plugin_object_sp->GetAsGeneric();
   if (!generic)
-return nullptr;
+return {};
 
   PythonObject implementor(PyRefType::Borrowed,
(PyObject *)generic->GetValue());
 
   if (!implementor.IsAllocated())
-return StructuredData::DictionarySP();
-
-  PythonObject pmeth(PyRefType::Owned,
- PyObject_GetAttrString(implementor.get(), callee_name));
-
-  if (PyErr_Occurred())
-PyErr_Clear();
-
-  if (!pmeth.IsAllocated())
-return StructuredData::DictionarySP();
+return {};
 
-  if (PyCallable_Check(pmeth.get()) == 0) {
-if (PyErr_Occurred())
-  PyErr_Clear();
+  llvm::Expected expected_py_return =
+  implementor.CallMethod("get_register_info");
 
-return StructuredData::DictionarySP();
-  }
-
-  if (PyErr_Occurred())
-PyErr_Clear();
+  if (!expected_py_return)
+return {};
 
-  // right now we know this function exists and is callable..
-  PythonObject py_return(
-  PyRefType::Owned,
-  PyObject_CallMethod(implementor.get(), callee_name, nullptr));
+  PythonObject py_return = expected_py_return.get();
 
-  // if it fails, print the error but otherwise go on
-  if (PyErr_Occurred()) {
-PyErr_Print();
-PyErr_Clear();
-  }
   if (py_return.get()) {
 PythonDictionary result_dict(PyRefType::Borrowed, py_return.get());
 return result_dict.CreateStructuredDictionary();
@@ -1555,51 +1532,26 @@
 StructuredData::ArraySP ScriptInterpreterPythonImpl::OSPlugin_ThreadsInfo(
 StructuredData::ObjectSP os_plugin_object_sp) {
   Locker py_lock(this, Locker::AcquireLock | Locker::NoSTDIN, Locker::FreeLock);
-
-  static char callee_name[] = "get_thread_info";
-
   if (!os_plugin_object_sp)
-return StructuredData::ArraySP();
+return {};
 
   StructuredData::Generic *generic = os_plugin_object_sp->GetAsGeneric();
   if (!generic)
-return nullptr;
+return {};
 
   PythonObject implementor(PyRefType::Borrowed,
(PyObject *)generic->GetValue());
 
   if (!implementor.IsAllocated())
-return StructuredData::ArraySP();
-
-  PythonObject pmeth(PyRefType::Owned,
- PyObject_GetAttrString(implementor.get(), callee_name));
-
-  if (PyErr_Occurred())
-PyErr_Clear();
-
-  if (!pmeth.IsAllocated())
-return StructuredData::ArraySP();
-
-  if (PyCallable_Check(pmeth.get()) == 0) {
-if (PyErr_Occurred())
-  PyErr_Clear();
+return {};
 
-return StructuredData::ArraySP();
-  }
+  llvm::Expected expected_py_return =
+  implementor.CallMethod("get_thread_info");
 
-  if (PyErr_Occurred())
-PyErr_Clear();
+  if (!expected_py_return)
+return {};
 
-  // right now we know this function exists and is callable..
-  PythonObject py_return(
-  PyRefType::Owned,
-  PyObject_CallMethod(implementor.get(), callee_name, nullptr));
-
-  // if it fails, print the error but otherwise go on
-  if (PyErr_Occurred()) {
-PyErr_Print();
-PyErr_Clear();
-  }
+  PythonObject py_return = expected_py_return.get();
 
   if (py_return.get()) {
 PythonList result_list(PyRefType::Borrowed, py_return.get());
@@ -1613,56 +1565,31 @@
 StructuredData::ObjectSP os_plugin_object_sp, lldb::tid_t tid) {
   Locker py_lock(this, Locker::AcquireLock | Locker::NoSTDIN, Locker::FreeLock);
 
-  static char callee_name[] = "get_register_data";
-  static char *param_format =
-  const_cast(GetPythonValueFormatString(tid));
-
   if (!os_plugin_object_sp)
-return StructuredData::StringSP();
+return {};
 
   StructuredData::Generic *generic = os_

[Lldb-commits] [PATCH] D134033: [lldb/Plugins] Improve error reporting when reading memory in Scripted Process

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 476292.
mib marked 8 inline comments as done.
mib edited the summary of this revision.
mib added a comment.

Address @labath comments.


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

https://reviews.llvm.org/D134033

Files:
  lldb/bindings/python/python-swigsafecast.swig
  lldb/examples/python/scripted_process/crashlog_scripted_process.py
  lldb/examples/python/scripted_process/scripted_process.py
  lldb/include/lldb/API/SBError.h
  lldb/source/API/SBError.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
  
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
  lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
  lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py
  lldb/test/API/functionalities/scripted_process/invalid_scripted_process.py
  lldb/test/API/functionalities/scripted_process/stack_core_scripted_process.py

Index: lldb/test/API/functionalities/scripted_process/stack_core_scripted_process.py
===
--- lldb/test/API/functionalities/scripted_process/stack_core_scripted_process.py
+++ lldb/test/API/functionalities/scripted_process/stack_core_scripted_process.py
@@ -65,9 +65,8 @@
 def get_registers_for_thread(self, tid: int):
 return {}
 
-def read_memory_at_address(self, addr: int, size: int) -> lldb.SBData:
+def read_memory_at_address(self, addr: int, size: int, error: lldb.SBError) -> lldb.SBData:
 data = lldb.SBData()
-error = lldb.SBError()
 bytes_read = self.corefile_process.ReadMemory(addr, size, error)
 
 if error.Fail():
Index: lldb/test/API/functionalities/scripted_process/invalid_scripted_process.py
===
--- lldb/test/API/functionalities/scripted_process/invalid_scripted_process.py
+++ lldb/test/API/functionalities/scripted_process/invalid_scripted_process.py
@@ -20,8 +20,9 @@
 def get_registers_for_thread(self, tid: int):
 return {}
 
-def read_memory_at_address(self, addr: int, size: int) -> lldb.SBData:
-return None
+def read_memory_at_address(self, addr: int, size: int, error: lldb.SBError) -> lldb.SBData:
+error.SetErrorString("This is an invalid scripted process!")
+return lldb.SBData()
 
 def get_loaded_images(self):
 return self.loaded_images
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
@@ -20,11 +20,12 @@
 def get_registers_for_thread(self, tid: int):
 return {}
 
-def read_memory_at_address(self, addr: int, size: int) -> lldb.SBData:
+def read_memory_at_address(self, addr: int, size: int, error: lldb.SBError) -> lldb.SBData:
 data = lldb.SBData().CreateDataFromCString(
 self.target.GetByteOrder(),
 self.target.GetCodeByteSize(),
 "Hello, world!")
+
 return data
 
 def get_loaded_images(self):
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
@@ -77,6 +77,12 @@
 self.assertEqual(process.GetProcessID(), 666)
 self.assertEqual(process.GetNumThreads(), 0)
 
+addr = 0x5
+buff = process.ReadMemory(addr, 4, error)
+self.assertEqual(buff, None)
+self.assertTrue(error.Fail())
+self.assertEqual(error.GetCString(), "This is an invalid scripted process!")
+
 with open(log_file, 'r') as f:
 log = f.read()
 
@@ -109,9 +115,14 @@
 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)
 
+addr = 0x5
+message = "Hello, world!"
+buff = process.ReadCStringFromMemory(addr, len(message) + 1, error)
+self.assertSuccess(error)
+self.assertEqual(buff, message)
+
 thread = process.GetSelectedThread()
 self.assertTrue(thread, "Invalid thread.")
 self.assertEqual(thread.GetThreadID(), 0x19)
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
===

[Lldb-commits] [PATCH] D134033: [lldb/Plugins] Improve error reporting when reading memory in Scripted Process

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added a comment.

In D134033#3933936 , @labath wrote:

> I kinda like it. One thing that I think would help with the readability is if 
> the "transformation" methods were grouped according to the type of the object 
> being transformed, rather than according to the direction. So something like:
>
>   // general transform
>   // general reverse
>   // Status transform
>   // Status reverse
>
> instead of
>
>   // general transform
>   // Status transform
>   // general reverse
>   // Status reverse
>
> Also I don't think that these structs (`transformation`, 
> `reverse_transformation`) wrapping the transformation functions are really 
> necessary. It's true that one cannot partially specialize functions, but one 
> of the reasons for that is this is normally not necessary -- regular function 
> overloading  can do most of that as well.

Thanks for the feedback! I really appreciate it :) For now I reordered the 
structs as you suggested but I'm thinking of moving all the transformation 
related code to an anonymous namespace above the class. I'll do that in a 
follow-up patch :)




Comment at: lldb/include/lldb/API/SBError.h:95
 private:
-  std::unique_ptr m_opaque_up;
+  std::shared_ptr m_opaque_sp;
 

labath wrote:
> This is technically an ABI break (changes `sizeof(SBError)`). I don't care, 
> but someone might.
Luckily, I didn't have to change this.


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

https://reviews.llvm.org/D134033

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

[Lldb-commits] [PATCH] D138250: [lldb/Python] Unify PythonFormat & GetPythonValueFormatString (NFC)

[Med Ismail Bennani via Phabricator via lldb-commits]

mib created this revision.
mib added reviewers: bulbazord, JDevlieghere.
mib added a project: LLDB.
Herald added a project: All.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch removes all occurences to GetPythonValueFormatString and
use the template specialization of PythonFormat structs instead.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138250

Files:
  lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
  lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h


Index: lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
===
--- lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
@@ -86,27 +86,6 @@
 
 } // namespace python
 
-// GetPythonValueFormatString provides a system independent type safe way to
-// convert a variable's type into a python value format. Python value formats
-// are defined in terms of builtin C types and could change from system to as
-// the underlying typedef for uint* types, size_t, off_t and other values
-// change.
-
-template  const char *GetPythonValueFormatString(T t);
-template <> const char *GetPythonValueFormatString(char *);
-template <> const char *GetPythonValueFormatString(char);
-template <> const char *GetPythonValueFormatString(unsigned char);
-template <> const char *GetPythonValueFormatString(short);
-template <> const char *GetPythonValueFormatString(unsigned short);
-template <> const char *GetPythonValueFormatString(int);
-template <> const char *GetPythonValueFormatString(unsigned int);
-template <> const char *GetPythonValueFormatString(long);
-template <> const char *GetPythonValueFormatString(unsigned long);
-template <> const char *GetPythonValueFormatString(long long);
-template <> const char *GetPythonValueFormatString(unsigned long long);
-template <> const char *GetPythonValueFormatString(float t);
-template <> const char *GetPythonValueFormatString(double t);
-
 void *LLDBSWIGPython_CastPyObjectToSBData(PyObject *data);
 void *LLDBSWIGPython_CastPyObjectToSBError(PyObject *data);
 void *LLDBSWIGPython_CastPyObjectToSBValue(PyObject *data);
Index: lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.cpp
+++ /dev/null
@@ -1,48 +0,0 @@
-//===-- SWIGPythonBridge.cpp 
--===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#include "lldb/Host/Config.h"
-#include "lldb/lldb-enumerations.h"
-
-#if LLDB_ENABLE_PYTHON
-
-// LLDB Python header must be included first
-#include "lldb-python.h"
-
-#include "SWIGPythonBridge.h"
-
-using namespace lldb;
-
-namespace lldb_private {
-
-template  const char *GetPythonValueFormatString(T t);
-template <> const char *GetPythonValueFormatString(char *) { return "s"; }
-template <> const char *GetPythonValueFormatString(char) { return "b"; }
-template <> const char *GetPythonValueFormatString(unsigned char) {
-  return "B";
-}
-template <> const char *GetPythonValueFormatString(short) { return "h"; }
-template <> const char *GetPythonValueFormatString(unsigned short) {
-  return "H";
-}
-template <> const char *GetPythonValueFormatString(int) { return "i"; }
-template <> const char *GetPythonValueFormatString(unsigned int) { return "I"; 
}
-template <> const char *GetPythonValueFormatString(long) { return "l"; }
-template <> const char *GetPythonValueFormatString(unsigned long) {
-  return "k";
-}
-template <> const char *GetPythonValueFormatString(long long) { return "L"; }
-template <> const char *GetPythonValueFormatString(unsigned long long) {
-  return "K";
-}
-template <> const char *GetPythonValueFormatString(float) { return "f"; }
-template <> const char *GetPythonValueFormatString(double) { return "d"; }
-
-} // namespace lldb_private
-
-#endif // LLDB_ENABLE_PYTHON
Index: lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
===
--- lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
+++ lldb/source/Plugins/ScriptInterpreter/Python/CMakeLists.txt
@@ -20,7 +20,6 @@
   ScriptedPythonInterface.cpp
   ScriptedProcessPythonInterface.cpp
   ScriptedThreadPythonInterface.cpp
-  SWIGPythonBridge.cpp
 
   LINK_LIBS
 lldbBreakpoint


Index: lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
===

[Lldb-commits] [PATCH] D134033: [lldb/Plugins] Improve error reporting when reading memory in Scripted Process

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 476299.
mib added a comment.

Fix failure when building PythonScriptInterpreter unit test libraries


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

https://reviews.llvm.org/D134033

Files:
  lldb/bindings/python/python-swigsafecast.swig
  lldb/examples/python/scripted_process/crashlog_scripted_process.py
  lldb/examples/python/scripted_process/scripted_process.py
  lldb/include/lldb/API/SBError.h
  lldb/source/API/SBError.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h
  
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
  lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py
  lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py
  lldb/test/API/functionalities/scripted_process/invalid_scripted_process.py
  lldb/test/API/functionalities/scripted_process/stack_core_scripted_process.py
  lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

Index: lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
===
--- lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
+++ lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp
@@ -270,3 +270,7 @@
 lldb::StreamSP stream) {
   return false;
 }
+
+python::PythonObject lldb_private::python::ToSWIGWrapper(const Status &status) {
+  return python::PythonObject();
+}
Index: lldb/test/API/functionalities/scripted_process/stack_core_scripted_process.py
===
--- lldb/test/API/functionalities/scripted_process/stack_core_scripted_process.py
+++ lldb/test/API/functionalities/scripted_process/stack_core_scripted_process.py
@@ -65,9 +65,8 @@
 def get_registers_for_thread(self, tid: int):
 return {}
 
-def read_memory_at_address(self, addr: int, size: int) -> lldb.SBData:
+def read_memory_at_address(self, addr: int, size: int, error: lldb.SBError) -> lldb.SBData:
 data = lldb.SBData()
-error = lldb.SBError()
 bytes_read = self.corefile_process.ReadMemory(addr, size, error)
 
 if error.Fail():
Index: lldb/test/API/functionalities/scripted_process/invalid_scripted_process.py
===
--- lldb/test/API/functionalities/scripted_process/invalid_scripted_process.py
+++ lldb/test/API/functionalities/scripted_process/invalid_scripted_process.py
@@ -20,8 +20,9 @@
 def get_registers_for_thread(self, tid: int):
 return {}
 
-def read_memory_at_address(self, addr: int, size: int) -> lldb.SBData:
-return None
+def read_memory_at_address(self, addr: int, size: int, error: lldb.SBError) -> lldb.SBData:
+error.SetErrorString("This is an invalid scripted process!")
+return lldb.SBData()
 
 def get_loaded_images(self):
 return self.loaded_images
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
@@ -20,11 +20,12 @@
 def get_registers_for_thread(self, tid: int):
 return {}
 
-def read_memory_at_address(self, addr: int, size: int) -> lldb.SBData:
+def read_memory_at_address(self, addr: int, size: int, error: lldb.SBError) -> lldb.SBData:
 data = lldb.SBData().CreateDataFromCString(
 self.target.GetByteOrder(),
 self.target.GetCodeByteSize(),
 "Hello, world!")
+
 return data
 
 def get_loaded_images(self):
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
@@ -77,6 +77,12 @@
 self.assertEqual(process.GetProcessID(), 666)
 self.assertEqual(process.GetNumThreads(), 0)
 
+addr = 0x5
+buff = process.ReadMemory(addr, 4, error)
+self.assertEqual(buff, None)
+self.assertTrue(error.Fail())
+self.assertEqual(error.GetCString(), "This is an invalid scripted process!")
+
 with open(log_file, 'r') as f:
 log = f.read()
 
@@ -109,9 +115,14 @@
 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)
 
+addr = 0x5000

[Lldb-commits] [PATCH] D138259: Add the ability to see when a type in incomplete.

[Greg Clayton via Phabricator via lldb-commits]

clayborg created this revision.
clayborg added reviewers: labath, JDevlieghere, jingham, yinghuitan, aprantl.
Herald added a subscriber: mstorsjo.
Herald added a project: All.
clayborg requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

-flimit-debug-info and other compiler options might end up removing debug info 
that is needed for debugging. LLDB marks these types as being forcefully 
completed in the metadata in the TypeSystem. These types should have been 
complete in the debug info but were not because the compiler omitted them to 
save space. When we can't find a suitable replacement for the type, we should 
let the user know that these types are incomplete to indicate there was an 
issue instead of just showing nothing for a type.

The solution is to display presented in this patch is to display "" as the summary for any incomplete types. If there is a summary string or 
function that is provided for a type, but the type is currently forcefully 
completed, the installed summary will be ignored and we will display 
"". This patch also exposes the ability to ask a SBType if it 
was forcefully completed with:

  bool SBType::IsTypeForcefullyCompleted();

This will allow the user interface for a debugger to also detect this issue and 
possibly mark the variable display up on some way to indicate to the user the 
type is incomplete.

To show how this is diplayed, we can look at the existing output first for the 
example source file from the file: 
lldb/test/API/functionalities/limit-debug-info/main.cpp

(lldb) frame variable inherits_from_one inherits_from_two one_as_member 
two_as_member array_of_one array_of_two shadowed_one
(InheritsFromOne) ::inherits_from_one = (member = 47)
(InheritsFromTwo) ::inherits_from_two = (member = 47)
(OneAsMember) ::one_as_member = (one = member::One @ 0x00018028, member 
= 47)
(TwoAsMember) ::two_as_member = (two = member::Two @ 0x00018040, member 
= 47)
(array::One [3]) ::array_of_one = ([0] = array::One @ 0x00018068, [1] = 
array::One @ 0x00018069, [2] = array::One @ 0x0001806a)
(array::Two [3]) ::array_of_two = ([0] = array::Two @ 0x00018098, [1] = 
array::Two @ 0x00018099, [2] = array::Two @ 0x0001809a)
(ShadowedOne) ::shadowed_one = (member = 47)
(lldb) frame variable --show-types inherits_from_one inherits_from_two 
one_as_member two_as_member array_of_one array_of_two shadowed_one
(InheritsFromOne) ::inherits_from_one = {

  (int) member = 47

}
(InheritsFromTwo) ::inherits_from_two = {

  (int) member = 47

}
(OneAsMember) ::one_as_member = {

  (member::One) one = {}
  (int) member = 47

}
(TwoAsMember) ::two_as_member = {

  (member::Two) two = {}
  (int) member = 47

}
(array::One [3]) ::array_of_one = {

  (array::One) [0] = {}
  (array::One) [1] = {}
  (array::One) [2] = {}

}
(array::Two [3]) ::array_of_two = {

  (array::Two) [0] = {}
  (array::Two) [1] = {}
  (array::Two) [2] = {}

}
(ShadowedOne) ::shadowed_one = {

  (int) member = 47

}

With this patch in place we can now see any classes that were forcefully 
completed to let us know that we are missing information:

(lldb) frame variable inherits_from_one inherits_from_two one_as_member 
two_as_member array_of_one array_of_two shadowed_one
(InheritsFromOne) ::inherits_from_one = (One = , member = 47)
(InheritsFromTwo) ::inherits_from_two = (Two = , member = 47)
(OneAsMember) ::one_as_member = (one = , member = 47)
(TwoAsMember) ::two_as_member = (two = , member = 47)
(array::One[3]) ::array_of_one = ([0] = , [1] = , [2] = )
(array::Two[3]) ::array_of_two = ([0] = , [1] = , [2] = )
(ShadowedOne) ::shadowed_one = (func_shadow::One = , member = 
47)
(lldb) frame variable --show-types inherits_from_one inherits_from_two 
one_as_member two_as_member array_of_one array_of_two shadowed_one
(InheritsFromOne) ::inherits_from_one = {

  (One) One =  {}
  (int) member = 47

}
(InheritsFromTwo) ::inherits_from_two = {

  (Two) Two =  {}
  (int) member = 47

}
(OneAsMember) ::one_as_member = {

  (member::One) one =  {}
  (int) member = 47

}
(TwoAsMember) ::two_as_member = {

  (member::Two) two =  {}
  (int) member = 47

}
(array::One[3]) ::array_of_one = {

  (array::One) [0] =  {}
  (array::One) [1] =  {}
  (array::One) [2] =  {}

}
(array::Two[3]) ::array_of_two = {

  (array::Two) [0] =  {}
  (array::Two) [1] =  {}
  (array::Two) [2] =  {}

}
(ShadowedOne) ::shadowed_one = {

  (func_shadow::One) func_shadow::One =  {}
  (int) member = 47

}


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138259

Files:
  lldb/bindings/interface/SBType.i
  lldb/include/lldb/API/SBType.h
  lldb/include/lldb/Symbol/CompilerType.h
  lldb/include/lldb/Symbol/TypeSystem.h
  lldb/source/API/SBType.cpp
  lldb/source/Core/ValueObject.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
  lldb/source/Symbol/CompilerType.cpp
  lldb

[Lldb-commits] [PATCH] D138259: Add the ability to see when a type in incomplete.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Forgot to mention that this patch will force any empty base classes that were 
forcefully completed to show up in the variable display so the user can know 
that the type was forcefully completed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138259

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

[Lldb-commits] [PATCH] D138259: Add the ability to see when a type in incomplete.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

We can see this in action with the before "var" output of:

  (InheritsFromOne) ::inherits_from_one = {
(int) member = 47
  }

And after, we see the "One" class definition and we can see that it is 
incomplete:

  (InheritsFromOne) ::inherits_from_one = {
(One) One =  {}
(int) member = 47
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138259

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

[Lldb-commits] [PATCH] D138248: [lldb/Python] Make use of PythonObject and PythonFormat in callbacks (NFC)

[Pavel Labath via Phabricator via lldb-commits]

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

Cool. Thanks for doing that.




Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h:191-194
+template <> struct PythonFormat {
+  static constexpr char format = 's';
+  static auto get(char *value) { return value; }
+};

Maybe something like:
```
template struct PassthroughFormat {
  static constexpr char format = F;
  static constexpr T get(T t) { return t; }
};

template<> struct PythonFormat : PassthroughFormat;
// etc.
```



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:1521
+  if (!expected_py_return)
+return {};
 

You need to do something with the error inside here. Log it perhaps (we have 
LLDB_LOG_ERROR for that)?



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:1552
+  if (!expected_py_return)
+return {};
 

same here (and elsewhere)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138248

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

[Lldb-commits] [PATCH] D138250: [lldb/Python] Unify PythonFormat & GetPythonValueFormatString (NFC)

[Pavel Labath via Phabricator via lldb-commits]

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

celebration_balloons 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138250

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

[Lldb-commits] [PATCH] D134033: [lldb/Plugins] Improve error reporting when reading memory in Scripted Process

[Pavel Labath via Phabricator via lldb-commits]

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

Cool. Thanks.

In D134033#3935424 , @mib wrote:

> In D134033#3933936 , @labath wrote:
>
>> I kinda like it. One thing that I think would help with the readability is 
>> if the "transformation" methods were grouped according to the type of the 
>> object being transformed, rather than according to the direction. So 
>> something like:
>>
>>   // general transform
>>   // general reverse
>>   // Status transform
>>   // Status reverse
>>
>> instead of
>>
>>   // general transform
>>   // Status transform
>>   // general reverse
>>   // Status reverse
>>
>> Also I don't think that these structs (`transformation`, 
>> `reverse_transformation`) wrapping the transformation functions are really 
>> necessary. It's true that one cannot partially specialize functions, but one 
>> of the reasons for that is this is normally not necessary -- regular 
>> function overloading  can do most of that 
>> as well.
>
> Thanks for the feedback! I really appreciate it :) For now I reordered the 
> structs as you suggested but I'm thinking of moving all the transformation 
> related code to an anonymous namespace above the class. I'll do that in a 
> follow-up patch :)

You can't have an anonymous namespace in a header (well.. you can, but it will 
break in very interesting ways). You can put it inside some `detail` or 
`internal` namespace, if you think it helps.




Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h:167-168
+  std::tuple &transformed_args) {
+if (sizeof...(Ts) != sizeof...(Us))
+  return false;
+

this could be a static_assert


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

https://reviews.llvm.org/D134033

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

[Lldb-commits] [PATCH] D138237: [lldb] Restore default setting of LLDB_INCLUDE_TESTS in standalone builds

[Michał Górny via Phabricator via lldb-commits]

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

Thanks, this makes sense. I just wish we could get D137035 
 in and avoid this whole complexity in the 
first place.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138237

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

[Lldb-commits] [PATCH] D138248: [lldb/Python] Make use of PythonObject and PythonFormat in callbacks (NFC)

[Med Ismail Bennani via Phabricator via lldb-commits]

mib marked 3 inline comments as done.
mib added inline comments.



Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h:191-194
+template <> struct PythonFormat {
+  static constexpr char format = 's';
+  static auto get(char *value) { return value; }
+};

labath wrote:
> Maybe something like:
> ```
> template struct PassthroughFormat {
>   static constexpr char format = F;
>   static constexpr T get(T t) { return t; }
> };
> 
> template<> struct PythonFormat : PassthroughFormat;
> // etc.
> ```
More templates <3 !


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138248

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

[Lldb-commits] [PATCH] D138248: [lldb/Python] Make use of PythonObject and PythonFormat in callbacks (NFC)

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 476361.
mib marked an inline comment as done.
mib added a comment.

Implement @labath suggestions


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

https://reviews.llvm.org/D138248

Files:
  lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -1501,50 +1501,29 @@
 StructuredData::ObjectSP os_plugin_object_sp) {
   Locker py_lock(this, Locker::AcquireLock | Locker::NoSTDIN, Locker::FreeLock);
 
-  static char callee_name[] = "get_register_info";
-
   if (!os_plugin_object_sp)
-return StructuredData::DictionarySP();
+return {};
 
   StructuredData::Generic *generic = os_plugin_object_sp->GetAsGeneric();
   if (!generic)
-return nullptr;
+return {};
 
   PythonObject implementor(PyRefType::Borrowed,
(PyObject *)generic->GetValue());
 
   if (!implementor.IsAllocated())
-return StructuredData::DictionarySP();
-
-  PythonObject pmeth(PyRefType::Owned,
- PyObject_GetAttrString(implementor.get(), callee_name));
-
-  if (PyErr_Occurred())
-PyErr_Clear();
-
-  if (!pmeth.IsAllocated())
-return StructuredData::DictionarySP();
+return {};
 
-  if (PyCallable_Check(pmeth.get()) == 0) {
-if (PyErr_Occurred())
-  PyErr_Clear();
+  llvm::Expected expected_py_return =
+  implementor.CallMethod("get_register_info");
 
-return StructuredData::DictionarySP();
+  if (!expected_py_return) {
+llvm::consumeError(expected_py_return.takeError());
+return {};
   }
 
-  if (PyErr_Occurred())
-PyErr_Clear();
-
-  // right now we know this function exists and is callable..
-  PythonObject py_return(
-  PyRefType::Owned,
-  PyObject_CallMethod(implementor.get(), callee_name, nullptr));
+  PythonObject py_return = std::move(expected_py_return.get());
 
-  // if it fails, print the error but otherwise go on
-  if (PyErr_Occurred()) {
-PyErr_Print();
-PyErr_Clear();
-  }
   if (py_return.get()) {
 PythonDictionary result_dict(PyRefType::Borrowed, py_return.get());
 return result_dict.CreateStructuredDictionary();
@@ -1555,51 +1534,28 @@
 StructuredData::ArraySP ScriptInterpreterPythonImpl::OSPlugin_ThreadsInfo(
 StructuredData::ObjectSP os_plugin_object_sp) {
   Locker py_lock(this, Locker::AcquireLock | Locker::NoSTDIN, Locker::FreeLock);
-
-  static char callee_name[] = "get_thread_info";
-
   if (!os_plugin_object_sp)
-return StructuredData::ArraySP();
+return {};
 
   StructuredData::Generic *generic = os_plugin_object_sp->GetAsGeneric();
   if (!generic)
-return nullptr;
+return {};
 
   PythonObject implementor(PyRefType::Borrowed,
(PyObject *)generic->GetValue());
 
   if (!implementor.IsAllocated())
-return StructuredData::ArraySP();
-
-  PythonObject pmeth(PyRefType::Owned,
- PyObject_GetAttrString(implementor.get(), callee_name));
-
-  if (PyErr_Occurred())
-PyErr_Clear();
-
-  if (!pmeth.IsAllocated())
-return StructuredData::ArraySP();
+return {};
 
-  if (PyCallable_Check(pmeth.get()) == 0) {
-if (PyErr_Occurred())
-  PyErr_Clear();
+  llvm::Expected expected_py_return =
+  implementor.CallMethod("get_thread_info");
 
-return StructuredData::ArraySP();
+  if (!expected_py_return) {
+llvm::consumeError(expected_py_return.takeError());
+return {};
   }
 
-  if (PyErr_Occurred())
-PyErr_Clear();
-
-  // right now we know this function exists and is callable..
-  PythonObject py_return(
-  PyRefType::Owned,
-  PyObject_CallMethod(implementor.get(), callee_name, nullptr));
-
-  // if it fails, print the error but otherwise go on
-  if (PyErr_Occurred()) {
-PyErr_Print();
-PyErr_Clear();
-  }
+  PythonObject py_return = std::move(expected_py_return.get());
 
   if (py_return.get()) {
 PythonList result_list(PyRefType::Borrowed, py_return.get());
@@ -1613,56 +1569,33 @@
 StructuredData::ObjectSP os_plugin_object_sp, lldb::tid_t tid) {
   Locker py_lock(this, Locker::AcquireLock | Locker::NoSTDIN, Locker::FreeLock);
 
-  static char callee_name[] = "get_register_data";
-  static char *param_format =
-  const_cast(GetPythonValueFormatString(tid));
-
   if (!os_plugin_object_sp)
-return StructuredData::StringSP();
+return {};
 
   StructuredData::Generic *generic = os_plugin_object_sp->GetAsGeneric();
   if (!generic)
-return nullptr;
+return {};
   PythonObject implementor(PyRefType::Borrowed,
(PyObject *)generic->GetValue());
 
   if (!implementor.IsAllocated())
-return Structu