from:"Jonas Devlieghere via Phabricator via lldb\-commits"

[Lldb-commits] [PATCH] D158785: [lldb] Add a "thread extrainfo" LC_NOTE for Mach-O corefiles, to store the thread IDs of the threads

2023-09-11 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

I really dislike all the manual memory management that's going on in this file, 
but I realize that's not the result of this patch, but rather because of the 
code that was factored out. I'd love if someone would replace the mallo'c 
buffers with `SmallString<0>`s but that's orthogonal to this patch. LGTM.




Comment at: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:73
 #include 
+#include 
 

I don't think this is needed (anymore)? 



Comment at: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h:124
+  std::vector>
+  FindLC_NOTEByName(std::string name);
+

Thanks for factoring this out!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158785

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

[Lldb-commits] [PATCH] D157609: [lldb] Add more ways to find split DWARF files

2023-09-07 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

Looks like this broke the X86 bot: 
https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/59815/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157609

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

[Lldb-commits] [PATCH] D159408: Switch over to using the LLVM archive parser for BSD archives.

2023-09-05 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159408

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

[Lldb-commits] [PATCH] D159408: Switch over to using the LLVM archive parser for BSD archives.

2023-09-05 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

Overall this looks good, but instead of consuming all the errors, let's log 
them with `LLDB_LOG_ERROR`. Note that the API is a bit tricky and still 
requires you to provide a format string:

  LLDB_LOG_ERROR(GetLog(LLDBLog::Object), something.takeError(), "context 
before printing the actual error: {0}");


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159408

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

[Lldb-commits] [PATCH] D159387: [lldb][NFCI] Remove unused method TypeCategoryMap::Get(uint32_t, ValueSP &)

2023-09-01 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

LGTM 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159387

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

[Lldb-commits] [PATCH] D159164: [lldb] Add assembly syntax highlighting

2023-09-01 Thread Jonas Devlieghere via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa69f78b080ef: [lldb] Add syntax color highlighting for 
disassembly (authored by JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D159164?vs=555489=13#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159164

Files:
  lldb/include/lldb/Core/Disassembler.h
  lldb/source/Core/Disassembler.cpp
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
  lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
  lldb/test/Shell/Commands/command-disassemble-aarch64-color.s

Index: lldb/test/Shell/Commands/command-disassemble-aarch64-color.s
===
--- /dev/null
+++ lldb/test/Shell/Commands/command-disassemble-aarch64-color.s
@@ -0,0 +1,33 @@
+# UNSUPPORTED: system-windows
+# REQUIRES: aarch64
+
+# This checks that lldb's disassembler colors AArch64 disassembly.
+
+# RUN: llvm-mc -filetype=obj -triple aarch64-linux-gnueabihf %s -o %t --mattr=+all
+# RUN: %lldb %t -o "settings set use-color true" -o "disassemble -n fn" -o exit 2>&1 | FileCheck %s
+
+.globl  fn
+.type   fn, @function
+fn:
+  // These are in alphabetical order by extension name
+  aesd v0.16b, v0.16b   // AEK_AES
+  bfadd z23.h, p3/m, z23.h, z13.h   // AEK_B16B16
+  bfdot v2.2s, v3.4h, v4.4h // AEK_BF16
+  brb iall  // AEK_BRBE
+  crc32b w0, w0, w0 // AEK_CRC
+  // AEK_CRYPTO enables a combination of other features
+  smin x0, x0, #0   // AEK_CSSC
+  sysp	#0, c2, c0, #0, x0, x1  // AEK_D128
+  sdot v0.2s, v1.8b, v2.8b  // AEK_DOTPROD
+  fmmla z0.s, z1.s, z2.s// AEK_F32MM
+
+# CHECK: `fn:
+# CHECK-NEXT: [0x0] <+0>:aesd   [0;36mv0[0m.16b, [0;36mv0[0m.16b
+# CHECK-NEXT: [0x4] <+4>:bfadd  [0;36mz23[0m.h, [0;36mp3[0m/m, [0;36mz23[0m.h, [0;36mz13[0m.h
+# CHECK-NEXT: [0x8] <+8>:bfdot  [0;36mv2[0m.2s, [0;36mv3[0m.4h, [0;36mv4[0m.4h
+# CHECK-NEXT: [0xc] <+12>:   brbiall
+# CHECK-NEXT: [0x10] <+16>:  crc32b [0;36mw0[0m, [0;36mw0[0m, [0;36mw0[0m
+# CHECK-NEXT: [0x14] <+20>:  smin   [0;36mx0[0m, [0;36mx0[0m, [0;31m#0x0[0m
+# CHECK-NEXT: [0x18] <+24>:  sysp   [0;31m#0x0[0m, c2, c0, [0;31m#0x0[0m, [0;36mx0[0m, [0;36mx1[0m
+# CHECK-NEXT: [0x1c] <+28>:  sdot   [0;36mv0[0m.2s, [0;36mv1[0m.8b, [0;36mv2[0m.8b
+# CHECK-NEXT: [0x20] <+32>:  fmmla  [0;36mz0[0m.s, [0;36mz1[0m.s, [0;36mz2[0m.s
Index: lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
===
--- lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
+++ lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
@@ -59,9 +59,19 @@
 elif arch in ("aarch64", "arm64"):
 self.assertEqual(inst.GetMnemonic(target), "mov")
 self.assertEqual(inst.GetOperands(target), "w0, #0x63")
+self.assertEqual(inst.GetComment(target), "=99 ")
 self.assertEqual(
 inst.GetControlFlowKind(target), lldb.eInstructionControlFlowKindUnknown
 )
+# Make sure that using colors doesn't affect the output here.
+res = lldb.SBCommandReturnObject()
+ci = self.dbg.GetCommandInterpreter()
+ci.HandleCommand("settings set use-color true", res)
+self.assertEqual(inst.GetOperands(target), "w0, #0x63")
+self.assertEqual(inst.GetMnemonic(target), "mov")
+self.assertEqual(inst.GetComment(target), "=99 ")
+ci.HandleCommand("settings set use-color false", res)
+
 elif arch == "arm":
 self.assertEqual(inst.GetMnemonic(target), "mov")
 self.assertEqual(inst.GetOperands(target), "r3, #99")
Index: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
===
--- lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
+++ lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
@@ -63,6 +63,8 @@
   void PrintMCInst(llvm::MCInst _inst, lldb::addr_t pc,
std::string _string, std::string _string);
   void SetStyle(bool use_hex_immed, HexImmediateStyle hex_style);
+  void SetUseColor(bool use_color);
+  bool GetUseColor() const;
   bool CanBranch(llvm::MCInst _inst) const;
   bool HasDelaySlot(llvm::MCInst _inst) const;
   bool IsCall(llvm::MCInst _inst) const;
@@ -565,7 +567,9 @@
 
 if (m_opcode.GetData(data)) {
   std::string out_string;
+  std::string markup_out_string;
   std::string comment_string;
+  std::string markup_comment_string;
 
   DisassemblerScope disasm(*this, exe_ctx);
   if (disasm) {
@@ -607,7

[Lldb-commits] [PATCH] D159164: [lldb] Add assembly syntax highlighting

2023-09-01 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

Thanks Greg!


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

https://reviews.llvm.org/D159164

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

[Lldb-commits] [PATCH] D159164: [lldb] Add assembly syntax highlighting

2023-09-01 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere updated this revision to Diff 555489.
JDevlieghere marked 9 inline comments as done.
JDevlieghere added a comment.

Use the execution context to enable colors.


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

https://reviews.llvm.org/D159164

Files:
  lldb/include/lldb/Core/Disassembler.h
  lldb/source/Core/Disassembler.cpp
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
  lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
  lldb/test/Shell/Commands/command-disassemble-aarch64-color.s

Index: lldb/test/Shell/Commands/command-disassemble-aarch64-color.s
===
--- /dev/null
+++ lldb/test/Shell/Commands/command-disassemble-aarch64-color.s
@@ -0,0 +1,32 @@
+# REQUIRES: aarch64
+
+# This checks that lldb's disassembler colors AArch64 disassembly.
+
+# RUN: llvm-mc -filetype=obj -triple aarch64-linux-gnueabihf %s -o %t --mattr=+all
+# RUN: %lldb %t -o "settings set use-color true" -o "disassemble -n fn" -o exit 2>&1 | FileCheck %s
+
+.globl  fn
+.type   fn, @function
+fn:
+  // These are in alphabetical order by extension name
+  aesd v0.16b, v0.16b   // AEK_AES
+  bfadd z23.h, p3/m, z23.h, z13.h   // AEK_B16B16
+  bfdot v2.2s, v3.4h, v4.4h // AEK_BF16
+  brb iall  // AEK_BRBE
+  crc32b w0, w0, w0 // AEK_CRC
+  // AEK_CRYPTO enables a combination of other features
+  smin x0, x0, #0   // AEK_CSSC
+  sysp	#0, c2, c0, #0, x0, x1  // AEK_D128
+  sdot v0.2s, v1.8b, v2.8b  // AEK_DOTPROD
+  fmmla z0.s, z1.s, z2.s// AEK_F32MM
+
+# CHECK: `fn:
+# CHECK-NEXT: [0x0] <+0>:aesd   [0;36mv0[0m.16b, [0;36mv0[0m.16b
+# CHECK-NEXT: [0x4] <+4>:bfadd  [0;36mz23[0m.h, [0;36mp3[0m/m, [0;36mz23[0m.h, [0;36mz13[0m.h
+# CHECK-NEXT: [0x8] <+8>:bfdot  [0;36mv2[0m.2s, [0;36mv3[0m.4h, [0;36mv4[0m.4h
+# CHECK-NEXT: [0xc] <+12>:   brbiall
+# CHECK-NEXT: [0x10] <+16>:  crc32b [0;36mw0[0m, [0;36mw0[0m, [0;36mw0[0m
+# CHECK-NEXT: [0x14] <+20>:  smin   [0;36mx0[0m, [0;36mx0[0m, [0;31m#0x0[0m
+# CHECK-NEXT: [0x18] <+24>:  sysp   [0;31m#0x0[0m, c2, c0, [0;31m#0x0[0m, [0;36mx0[0m, [0;36mx1[0m
+# CHECK-NEXT: [0x1c] <+28>:  sdot   [0;36mv0[0m.2s, [0;36mv1[0m.8b, [0;36mv2[0m.8b
+# CHECK-NEXT: [0x20] <+32>:  fmmla  [0;36mz0[0m.s, [0;36mz1[0m.s, [0;36mz2[0m.s
Index: lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
===
--- lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
+++ lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
@@ -59,9 +59,19 @@
 elif arch in ("aarch64", "arm64"):
 self.assertEqual(inst.GetMnemonic(target), "mov")
 self.assertEqual(inst.GetOperands(target), "w0, #0x63")
+self.assertEqual(inst.GetComment(target), "=99 ")
 self.assertEqual(
 inst.GetControlFlowKind(target), lldb.eInstructionControlFlowKindUnknown
 )
+# Make sure that using colors doesn't affect the output here.
+res = lldb.SBCommandReturnObject()
+ci = self.dbg.GetCommandInterpreter()
+ci.HandleCommand("settings set use-color true", res)
+self.assertEqual(inst.GetOperands(target), "w0, #0x63")
+self.assertEqual(inst.GetMnemonic(target), "mov")
+self.assertEqual(inst.GetComment(target), "=99 ")
+ci.HandleCommand("settings set use-color false", res)
+
 elif arch == "arm":
 self.assertEqual(inst.GetMnemonic(target), "mov")
 self.assertEqual(inst.GetOperands(target), "r3, #99")
Index: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
===
--- lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
+++ lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
@@ -63,6 +63,8 @@
   void PrintMCInst(llvm::MCInst _inst, lldb::addr_t pc,
std::string _string, std::string _string);
   void SetStyle(bool use_hex_immed, HexImmediateStyle hex_style);
+  void SetUseColor(bool use_color);
+  bool GetUseColor() const;
   bool CanBranch(llvm::MCInst _inst) const;
   bool HasDelaySlot(llvm::MCInst _inst) const;
   bool IsCall(llvm::MCInst _inst) const;
@@ -565,7 +567,9 @@
 
 if (m_opcode.GetData(data)) {
   std::string out_string;
+  std::string markup_out_string;
   std::string comment_string;
+  std::string markup_comment_string;
 
   DisassemblerScope disasm(*this, exe_ctx);
   if (disasm) {
@@ -607,7 +611,14 @@
 
 if (inst_size > 0) {
   mc_disasm_ptr->SetStyle(use_hex_immediates, hex_style);
+
+  const bool saved_use_color = mc_disasm_ptr->GetUseColor();
+

[Lldb-commits] [PATCH] D159164: [lldb] Add assembly syntax highlighting

2023-09-01 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

@clayborg let me know if you're happy with this. To answer your previous 
question, I'm not planning anymore changes.


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

https://reviews.llvm.org/D159164

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

[Lldb-commits] [PATCH] D159164: [lldb] Add assembly syntax highlighting

2023-09-01 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere updated this revision to Diff 555442.
JDevlieghere added a comment.

As I was adding another test I realized an issue with the previous approach due 
to the fact that we cache the result. The solution is to cache both the plain 
and the marked up result, and have the different accessors decide whether they 
want colored output or not. As a result, I've removed the global Disassembler 
option to enable color, as it's not really meaningful anymore. I've also added 
a test to make sure this doesn't break.


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

https://reviews.llvm.org/D159164

Files:
  lldb/include/lldb/Core/Disassembler.h
  lldb/source/API/SBInstruction.cpp
  lldb/source/API/SBInstructionList.cpp
  lldb/source/Core/Disassembler.cpp
  lldb/source/Core/DumpDataExtractor.cpp
  lldb/source/Expression/IRExecutionUnit.cpp
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
  
lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
  lldb/source/Symbol/Function.cpp
  lldb/source/Symbol/Symbol.cpp
  lldb/source/Target/ThreadPlanTracer.cpp
  lldb/source/Target/TraceDumper.cpp
  lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
  lldb/test/Shell/Commands/command-disassemble-aarch64-color.s

Index: lldb/test/Shell/Commands/command-disassemble-aarch64-color.s
===
--- /dev/null
+++ lldb/test/Shell/Commands/command-disassemble-aarch64-color.s
@@ -0,0 +1,32 @@
+# REQUIRES: aarch64
+
+# This checks that lldb's disassembler colors AArch64 disassembly.
+
+# RUN: llvm-mc -filetype=obj -triple aarch64-linux-gnueabihf %s -o %t --mattr=+all
+# RUN: %lldb %t -o "settings set use-color true" -o "disassemble -n fn" -o exit 2>&1 | FileCheck %s
+
+.globl  fn
+.type   fn, @function
+fn:
+  // These are in alphabetical order by extension name
+  aesd v0.16b, v0.16b   // AEK_AES
+  bfadd z23.h, p3/m, z23.h, z13.h   // AEK_B16B16
+  bfdot v2.2s, v3.4h, v4.4h // AEK_BF16
+  brb iall  // AEK_BRBE
+  crc32b w0, w0, w0 // AEK_CRC
+  // AEK_CRYPTO enables a combination of other features
+  smin x0, x0, #0   // AEK_CSSC
+  sysp	#0, c2, c0, #0, x0, x1  // AEK_D128
+  sdot v0.2s, v1.8b, v2.8b  // AEK_DOTPROD
+  fmmla z0.s, z1.s, z2.s// AEK_F32MM
+
+# CHECK: `fn:
+# CHECK-NEXT: [0x0] <+0>:aesd   [0;36mv0[0m.16b, [0;36mv0[0m.16b
+# CHECK-NEXT: [0x4] <+4>:bfadd  [0;36mz23[0m.h, [0;36mp3[0m/m, [0;36mz23[0m.h, [0;36mz13[0m.h
+# CHECK-NEXT: [0x8] <+8>:bfdot  [0;36mv2[0m.2s, [0;36mv3[0m.4h, [0;36mv4[0m.4h
+# CHECK-NEXT: [0xc] <+12>:   brbiall
+# CHECK-NEXT: [0x10] <+16>:  crc32b [0;36mw0[0m, [0;36mw0[0m, [0;36mw0[0m
+# CHECK-NEXT: [0x14] <+20>:  smin   [0;36mx0[0m, [0;36mx0[0m, [0;31m#0x0[0m
+# CHECK-NEXT: [0x18] <+24>:  sysp   [0;31m#0x0[0m, c2, c0, [0;31m#0x0[0m, [0;36mx0[0m, [0;36mx1[0m
+# CHECK-NEXT: [0x1c] <+28>:  sdot   [0;36mv0[0m.2s, [0;36mv1[0m.8b, [0;36mv2[0m.8b
+# CHECK-NEXT: [0x20] <+32>:  fmmla  [0;36mz0[0m.s, [0;36mz1[0m.s, [0;36mz2[0m.s
Index: lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
===
--- lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
+++ lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
@@ -59,9 +59,19 @@
 elif arch in ("aarch64", "arm64"):
 self.assertEqual(inst.GetMnemonic(target), "mov")
 self.assertEqual(inst.GetOperands(target), "w0, #0x63")
+self.assertEqual(inst.GetComment(target), "=99 ")
 self.assertEqual(
 inst.GetControlFlowKind(target), lldb.eInstructionControlFlowKindUnknown
 )
+# Make sure that using colors doesn't affect the output here.
+res = lldb.SBCommandReturnObject()
+ci = self.dbg.GetCommandInterpreter()
+ci.HandleCommand("settings set use-color true", res)
+self.assertEqual(inst.GetOperands(target), "w0, #0x63")
+self.assertEqual(inst.GetMnemonic(target), "mov")
+self.assertEqual(inst.GetComment(target), "=99 ")
+ci.HandleCommand("settings set use-color false", res)
+
 elif arch == "arm":
 self.assertEqual(inst.GetMnemonic(target), "mov")
 self.assertEqual(inst.GetOperands(target), "r3, #99")
Index: lldb/source/Target/TraceDumper.cpp
===
--- lldb/source/Target/TraceDumper.cpp
+++ lldb/source/Target/TraceDumper.cpp
@@ -189,7 +189,8 @@
 _s, /*max_opcode_byte_size=*/0,
 /*show_address=*/false,
 /*show_bytes=*/false, m_options.show_control_flow_kind,
-_info->exe_ctx, _info->sc,
+

[Lldb-commits] [PATCH] D159164: [lldb] Add assembly syntax highlighting

2023-09-01 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere updated this revision to Diff 555417.
JDevlieghere marked an inline comment as done.
JDevlieghere added a comment.

Test `GetMnemonic` and `GetComment`.


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

https://reviews.llvm.org/D159164

Files:
  lldb/include/lldb/Core/Disassembler.h
  lldb/source/Core/Disassembler.cpp
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h
  lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py

Index: lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
===
--- lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
+++ lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
@@ -59,9 +59,19 @@
 elif arch in ("aarch64", "arm64"):
 self.assertEqual(inst.GetMnemonic(target), "mov")
 self.assertEqual(inst.GetOperands(target), "w0, #0x63")
+self.assertEqual(inst.GetComment(target), "=99 ")
 self.assertEqual(
 inst.GetControlFlowKind(target), lldb.eInstructionControlFlowKindUnknown
 )
+# Make sure that using colors doesn't affect the output here.
+res = lldb.SBCommandReturnObject()
+ci = self.dbg.GetCommandInterpreter()
+ci.HandleCommand("settings set use-color true", res)
+self.assertEqual(inst.GetOperands(target), "w0, #0x63")
+self.assertEqual(inst.GetMnemonic(target), "mov")
+self.assertEqual(inst.GetComment(target), "=99 ")
+ci.HandleCommand("settings set use-color false", res)
+
 elif arch == "arm":
 self.assertEqual(inst.GetMnemonic(target), "mov")
 self.assertEqual(inst.GetOperands(target), "r3, #99")
Index: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h
===
--- lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h
+++ lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h
@@ -42,6 +42,8 @@
 lldb::offset_t data_offset, size_t num_instructions,
 bool append, bool data_from_file) override;
 
+  void SetUseColor(bool use_color) override;
+
   // PluginInterface protocol
   llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); }
 
Index: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
===
--- lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
+++ lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
@@ -9,6 +9,7 @@
 #include "DisassemblerLLVMC.h"
 
 #include "llvm-c/Disassembler.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/MC/MCAsmInfo.h"
@@ -63,6 +64,8 @@
   void PrintMCInst(llvm::MCInst _inst, lldb::addr_t pc,
std::string _string, std::string _string);
   void SetStyle(bool use_hex_immed, HexImmediateStyle hex_style);
+  void SetUseColor(bool use_color);
+  bool GetUseColor() const;
   bool CanBranch(llvm::MCInst _inst) const;
   bool HasDelaySlot(llvm::MCInst _inst) const;
   bool IsCall(llvm::MCInst _inst) const;
@@ -576,6 +579,12 @@
 else
   mc_disasm_ptr = disasm->m_disasm_up.get();
 
+// Disable coloring for the duration of this function.
+const bool saved_use_color = mc_disasm_ptr->GetUseColor();
+mc_disasm_ptr->SetUseColor(false);
+auto on_exit = llvm::make_scope_exit(
+[=]() { mc_disasm_ptr->SetUseColor(saved_use_color); });
+
 lldb::addr_t pc = m_address.GetFileAddress();
 m_using_file_addr = true;
 
@@ -1344,10 +1353,12 @@
   llvm::raw_string_ostream inst_stream(inst_string);
   llvm::raw_string_ostream comments_stream(comments_string);
 
+  inst_stream.enable_colors(m_instr_printer_up->getUseColor());
   m_instr_printer_up->setCommentStream(comments_stream);
   m_instr_printer_up->printInst(_inst, pc, llvm::StringRef(),
 *m_subtarget_info_up, inst_stream);
   m_instr_printer_up->setCommentStream(llvm::nulls());
+
   comments_stream.flush();
 
   static std::string g_newlines("\r\n");
@@ -1374,6 +1385,14 @@
   }
 }
 
+void DisassemblerLLVMC::MCDisasmInstance::SetUseColor(bool use_color) {
+  m_instr_printer_up->setUseColor(use_color);
+}
+
+bool DisassemblerLLVMC::MCDisasmInstance::GetUseColor() const {
+  return m_instr_printer_up->getUseColor();
+}
+
 bool DisassemblerLLVMC::MCDisasmInstance::CanBranch(
 llvm::MCInst _inst) const {
   if (m_instr_analysis_up)
@@ -1597,6 +1616,13 @@
   return lldb::DisassemblerSP();
 }
 
+void DisassemblerLLVMC::SetUseColor(bool use_color) {
+  if (m_disasm_up)
+m_disasm_up->SetUseColor(use_color);
+  if (m_alternate_disasm_up)
+

[Lldb-commits] [PATCH] D159315: [lldb] Add OperatingSystem base class to the lldb python module

2023-09-01 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

Looks great. LGTM!


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

https://reviews.llvm.org/D159315

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

[Lldb-commits] [PATCH] D159164: [lldb] Add assembly syntax highlighting

2023-09-01 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere updated this revision to Diff 555269.
JDevlieghere added a comment.

Discard whitespace changes


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

https://reviews.llvm.org/D159164

Files:
  lldb/include/lldb/Core/Disassembler.h
  lldb/source/Core/Disassembler.cpp
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h
  lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py

Index: lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
===
--- lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
+++ lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
@@ -62,6 +62,13 @@
 self.assertEqual(
 inst.GetControlFlowKind(target), lldb.eInstructionControlFlowKindUnknown
 )
+# Make sure that using colors doesn't affect the output here.
+res = lldb.SBCommandReturnObject()
+ci = self.dbg.GetCommandInterpreter()
+ci.HandleCommand("settings set use-color true", res)
+self.assertEqual(inst.GetOperands(target), "w0, #0x63")
+ci.HandleCommand("settings set use-color false", res)
+
 elif arch == "arm":
 self.assertEqual(inst.GetMnemonic(target), "mov")
 self.assertEqual(inst.GetOperands(target), "r3, #99")
Index: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h
===
--- lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h
+++ lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h
@@ -42,6 +42,8 @@
 lldb::offset_t data_offset, size_t num_instructions,
 bool append, bool data_from_file) override;
 
+  void SetUseColor(bool use_color) override;
+
   // PluginInterface protocol
   llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); }
 
Index: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
===
--- lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
+++ lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
@@ -9,6 +9,7 @@
 #include "DisassemblerLLVMC.h"
 
 #include "llvm-c/Disassembler.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/MC/MCAsmInfo.h"
@@ -63,6 +64,8 @@
   void PrintMCInst(llvm::MCInst _inst, lldb::addr_t pc,
std::string _string, std::string _string);
   void SetStyle(bool use_hex_immed, HexImmediateStyle hex_style);
+  void SetUseColor(bool use_color);
+  bool GetUseColor() const;
   bool CanBranch(llvm::MCInst _inst) const;
   bool HasDelaySlot(llvm::MCInst _inst) const;
   bool IsCall(llvm::MCInst _inst) const;
@@ -576,6 +579,12 @@
 else
   mc_disasm_ptr = disasm->m_disasm_up.get();
 
+// Disable coloring for the duration of this function.
+const bool saved_use_color = mc_disasm_ptr->GetUseColor();
+mc_disasm_ptr->SetUseColor(false);
+auto on_exit = llvm::make_scope_exit(
+[=]() { mc_disasm_ptr->SetUseColor(saved_use_color); });
+
 lldb::addr_t pc = m_address.GetFileAddress();
 m_using_file_addr = true;
 
@@ -1344,10 +1353,12 @@
   llvm::raw_string_ostream inst_stream(inst_string);
   llvm::raw_string_ostream comments_stream(comments_string);
 
+  inst_stream.enable_colors(m_instr_printer_up->getUseColor());
   m_instr_printer_up->setCommentStream(comments_stream);
   m_instr_printer_up->printInst(_inst, pc, llvm::StringRef(),
 *m_subtarget_info_up, inst_stream);
   m_instr_printer_up->setCommentStream(llvm::nulls());
+
   comments_stream.flush();
 
   static std::string g_newlines("\r\n");
@@ -1374,6 +1385,14 @@
   }
 }
 
+void DisassemblerLLVMC::MCDisasmInstance::SetUseColor(bool use_color) {
+  m_instr_printer_up->setUseColor(use_color);
+}
+
+bool DisassemblerLLVMC::MCDisasmInstance::GetUseColor() const {
+  return m_instr_printer_up->getUseColor();
+}
+
 bool DisassemblerLLVMC::MCDisasmInstance::CanBranch(
 llvm::MCInst _inst) const {
   if (m_instr_analysis_up)
@@ -1597,6 +1616,13 @@
   return lldb::DisassemblerSP();
 }
 
+void DisassemblerLLVMC::SetUseColor(bool use_color) {
+  if (m_disasm_up)
+m_disasm_up->SetUseColor(use_color);
+  if (m_alternate_disasm_up)
+m_alternate_disasm_up->SetUseColor(use_color);
+}
+
 size_t DisassemblerLLVMC::DecodeInstructions(const Address _addr,
  const DataExtractor ,
  lldb::offset_t data_offset,
Index: lldb/source/Core/Disassembler.cpp
===
--- lldb/source/Core/Disassembler.cpp
+++

[Lldb-commits] [PATCH] D159164: [lldb] Add assembly syntax highlighting

2023-09-01 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere updated this revision to Diff 555268.
JDevlieghere added a comment.

Address @clayborg's code review feedback


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

https://reviews.llvm.org/D159164

Files:
  lldb/include/lldb/Core/Disassembler.h
  lldb/source/Core/Disassembler.cpp
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h
  lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py

Index: lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
===
--- lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
+++ lldb/test/API/python_api/disassemble-raw-data/TestDisassembleRawData.py
@@ -62,6 +62,13 @@
 self.assertEqual(
 inst.GetControlFlowKind(target), lldb.eInstructionControlFlowKindUnknown
 )
+# Make sure that using colors doesn't affect the output here.
+res = lldb.SBCommandReturnObject()
+ci = self.dbg.GetCommandInterpreter()
+ci.HandleCommand("settings set use-color true", res)
+self.assertEqual(inst.GetOperands(target), "w0, #0x63")
+ci.HandleCommand("settings set use-color false", res)
+
 elif arch == "arm":
 self.assertEqual(inst.GetMnemonic(target), "mov")
 self.assertEqual(inst.GetOperands(target), "r3, #99")
Index: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h
===
--- lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h
+++ lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h
@@ -42,6 +42,8 @@
 lldb::offset_t data_offset, size_t num_instructions,
 bool append, bool data_from_file) override;
 
+  void SetUseColor(bool use_color) override;
+
   // PluginInterface protocol
   llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); }
 
Index: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
===
--- lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
+++ lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
@@ -9,6 +9,7 @@
 #include "DisassemblerLLVMC.h"
 
 #include "llvm-c/Disassembler.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/MC/MCAsmInfo.h"
@@ -63,6 +64,8 @@
   void PrintMCInst(llvm::MCInst _inst, lldb::addr_t pc,
std::string _string, std::string _string);
   void SetStyle(bool use_hex_immed, HexImmediateStyle hex_style);
+  void SetUseColor(bool use_color);
+  bool GetUseColor() const;
   bool CanBranch(llvm::MCInst _inst) const;
   bool HasDelaySlot(llvm::MCInst _inst) const;
   bool IsCall(llvm::MCInst _inst) const;
@@ -576,6 +579,12 @@
 else
   mc_disasm_ptr = disasm->m_disasm_up.get();
 
+// Disable coloring for the duration of this function.
+const bool saved_use_color = mc_disasm_ptr->GetUseColor();
+mc_disasm_ptr->SetUseColor(false);
+auto on_exit = llvm::make_scope_exit(
+[=]() { mc_disasm_ptr->SetUseColor(saved_use_color); });
+
 lldb::addr_t pc = m_address.GetFileAddress();
 m_using_file_addr = true;
 
@@ -1344,10 +1353,12 @@
   llvm::raw_string_ostream inst_stream(inst_string);
   llvm::raw_string_ostream comments_stream(comments_string);
 
+  inst_stream.enable_colors(m_instr_printer_up->getUseColor());
   m_instr_printer_up->setCommentStream(comments_stream);
   m_instr_printer_up->printInst(_inst, pc, llvm::StringRef(),
 *m_subtarget_info_up, inst_stream);
   m_instr_printer_up->setCommentStream(llvm::nulls());
+
   comments_stream.flush();
 
   static std::string g_newlines("\r\n");
@@ -1374,6 +1385,14 @@
   }
 }
 
+void DisassemblerLLVMC::MCDisasmInstance::SetUseColor(bool use_color) {
+  m_instr_printer_up->setUseColor(use_color);
+}
+
+bool DisassemblerLLVMC::MCDisasmInstance::GetUseColor() const {
+  return m_instr_printer_up->getUseColor();
+}
+
 bool DisassemblerLLVMC::MCDisasmInstance::CanBranch(
 llvm::MCInst _inst) const {
   if (m_instr_analysis_up)
@@ -1597,6 +1616,13 @@
   return lldb::DisassemblerSP();
 }
 
+void DisassemblerLLVMC::SetUseColor(bool use_color) {
+  if (m_disasm_up)
+m_disasm_up->SetUseColor(use_color);
+  if (m_alternate_disasm_up)
+m_alternate_disasm_up->SetUseColor(use_color);
+}
+
 size_t DisassemblerLLVMC::DecodeInstructions(const Address _addr,
  const DataExtractor ,
  lldb::offset_t data_offset,
Index: lldb/source/Core/Disassembler.cpp
===
---

[Lldb-commits] [PATCH] D159164: [lldb] Add assembly syntax highlighting

2023-09-01 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere marked 6 inline comments as done.
JDevlieghere added a comment.

In D159164#4632017 , @clayborg wrote:

> We shouldn't need to pass in "bool use_color" to the Disassembler creation 
> functions as the only reason to pass something to the creation function for 
> any plug-in is if that data would exclude a disassembler if it didn't support 
> color. But we always want to see some disassembly rather than none if a 
> plug-in is the only one that handles a certain architecture and doesn't 
> support color. We should either add accessors to enable colorization to the 
> Disassembler virtual plug-in interface _or_ add "bool use_color" to the 
> needed functions. Looks DisassembleBytes added a "bool use_color" argument to 
> the call already, so we shouldn't need a "bool use_color" for the plug-in 
> selection. Are there other places that need to know about the "use_color" 
> setting that do not results from a direct function call that can have an 
> argument added to it?

Thanks for the suggestion, that's definitely much nicer. I made it part of the 
create function because I thought we needed to know the value during 
initialization time, but you're correct that we can set this after the fact 
with the help of an accessor. This simplifies the patch by a lot.

> Even if colorization is enabled, It would still expect any APIs that return 
> information in individual instructions, like:
>
>   const char *SBInstruction::GetMnemonic(lldb::SBTarget target);
>   const char *SBInstruction::GetOperands(lldb::SBTarget target);
>   const char *SBInstruction::GetComment(lldb::SBTarget target);
>
> To not have colorization attached to the returned string.

Yeah, the updated patch accounts for that and adds a test.


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

https://reviews.llvm.org/D159164

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

[Lldb-commits] [PATCH] D159234: [llvm-objdump] Enable assembly highlighting in llvm-objdump

2023-08-31 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere abandoned this revision.
JDevlieghere added a comment.

Merged with D159224 .


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

https://reviews.llvm.org/D159234

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

[Lldb-commits] [PATCH] D159150: [lldb][NFCI] Replace bespoke iterator check with std::next

2023-08-30 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

LGTM 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159150

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

[Lldb-commits] [PATCH] D159164: [lldb] Add assembly syntax highlighting

2023-08-30 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere updated this revision to Diff 554873.
JDevlieghere added a comment.

- Fix inconsistencies and use `use_color` everywhere


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

https://reviews.llvm.org/D159164

Files:
  lldb/include/lldb/Core/Disassembler.h
  lldb/include/lldb/lldb-private-interfaces.h
  lldb/source/API/SBTarget.cpp
  lldb/source/Commands/CommandObjectDisassemble.cpp
  lldb/source/Core/Disassembler.cpp
  lldb/source/Core/DumpDataExtractor.cpp
  lldb/source/Expression/IRExecutionUnit.cpp
  lldb/source/Plugins/Architecture/Mips/ArchitectureMips.cpp
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h
  
lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
  lldb/source/Target/ThreadPlanTracer.cpp

Index: lldb/source/Target/ThreadPlanTracer.cpp
===
--- lldb/source/Target/ThreadPlanTracer.cpp
+++ lldb/source/Target/ThreadPlanTracer.cpp
@@ -96,7 +96,8 @@
 Disassembler *ThreadPlanAssemblyTracer::GetDisassembler() {
   if (!m_disassembler_sp)
 m_disassembler_sp = Disassembler::FindPlugin(
-m_process.GetTarget().GetArchitecture(), nullptr, nullptr);
+m_process.GetTarget().GetArchitecture(), nullptr,
+m_process.GetTarget().GetDebugger().GetUseColor(), nullptr);
   return m_disassembler_sp.get();
 }
 
Index: lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
===
--- lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
+++ lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
@@ -70,7 +70,7 @@
 
 const bool prefer_file_cache = true;
 DisassemblerSP disasm_sp(Disassembler::DisassembleBytes(
-m_arch, nullptr, nullptr, range.GetBaseAddress(), opcode_data,
+m_arch, nullptr, nullptr, false, range.GetBaseAddress(), opcode_data,
 opcode_size, 9, prefer_file_cache));
 
 Log *log = GetLog(LLDBLog::Unwind);
Index: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h
===
--- lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h
+++ lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h
@@ -22,8 +22,8 @@
 
 class DisassemblerLLVMC : public lldb_private::Disassembler {
 public:
-  DisassemblerLLVMC(const lldb_private::ArchSpec ,
-const char *flavor /* = NULL */);
+  DisassemblerLLVMC(const lldb_private::ArchSpec , const char *flavor,
+bool use_color = false);
 
   ~DisassemblerLLVMC() override;
 
@@ -35,7 +35,8 @@
   static llvm::StringRef GetPluginNameStatic() { return "llvm-mc"; }
 
   static lldb::DisassemblerSP CreateInstance(const lldb_private::ArchSpec ,
- const char *flavor);
+ const char *flavor,
+ bool use_color);
 
   size_t DecodeInstructions(const lldb_private::Address _addr,
 const lldb_private::DataExtractor ,
@@ -72,6 +73,7 @@
   InstructionLLVMC *m_inst;
   std::mutex m_mutex;
   bool m_data_from_file;
+  bool m_use_color;
   // Save the AArch64 ADRP instruction word and address it was at,
   // in case the next instruction is an ADD to the same register;
   // this is a pc-relative address calculation and we need both
Index: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
===
--- lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
+++ lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
@@ -54,7 +54,7 @@
 public:
   static std::unique_ptr
   Create(const char *triple, const char *cpu, const char *features_str,
- unsigned flavor, DisassemblerLLVMC );
+ unsigned flavor, bool use_color, DisassemblerLLVMC );
 
   ~MCDisasmInstance() = default;
 
@@ -1227,7 +1227,7 @@
 std::unique_ptr
 DisassemblerLLVMC::MCDisasmInstance::Create(const char *triple, const char *cpu,
 const char *features_str,
-unsigned flavor,
+unsigned flavor, bool use_color,
 DisassemblerLLVMC ) {
   using Instance = std::unique_ptr;
 
@@ -1291,6 +1291,7 @@
 return Instance();
 
   instr_printer_up->setPrintBranchImmAsAddress(true);
+  instr_printer_up->setUseColor(use_color);
 
   // Not all targets may have registered createMCInstrAnalysis().
   std::unique_ptr instr_analysis_up(
@@ -1344,6 +1345,8 @@
   llvm::raw_string_ostream inst_stream(inst_string);
   llvm::raw_string_ostream comments_stream(comments_string);
 
+

[Lldb-commits] [PATCH] D159237: [lldb][NFCI] Remove unneeded ConstString conversions

2023-08-30 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159237

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

[Lldb-commits] [PATCH] D159234: [llvm-objdump] Enable assembly highlighting in llvm-objdump

2023-08-30 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere created this revision.
JDevlieghere added reviewers: MaskRay, jhenderson.
Herald added a project: All.
JDevlieghere requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Enable assembly highlighting in llvm-objdump.


https://reviews.llvm.org/D159234

Files:
  llvm/include/llvm/Support/FormattedStream.h
  llvm/test/tools/llvm-objdump/MachO/arm64-disassembly-color.s
  llvm/tools/llvm-objdump/llvm-objdump.cpp


Index: llvm/tools/llvm-objdump/llvm-objdump.cpp
===
--- llvm/tools/llvm-objdump/llvm-objdump.cpp
+++ llvm/tools/llvm-objdump/llvm-objdump.cpp
@@ -901,6 +901,7 @@
   if (!InstPrinter)
 reportError(Obj.getFileName(),
 "no instruction printer for target " + TripleName);
+  InstPrinter->setUseColor(!NoColor);
   InstPrinter->setPrintImmHex(PrintImmHex);
   InstPrinter->setPrintBranchImmAsAddress(true);
   InstPrinter->setSymbolizeOperands(SymbolizeOperands);
Index: llvm/test/tools/llvm-objdump/MachO/arm64-disassembly-color.s
===
--- /dev/null
+++ llvm/test/tools/llvm-objdump/MachO/arm64-disassembly-color.s
@@ -0,0 +1,25 @@
+// RUN: llvm-mc -triple arm64-apple-macosx %s -filetype=obj -o %t
+// RUN: llvm-objdump --color --disassemble %t | FileCheck %s 
--check-prefix=COLOR
+// RUN: llvm-objdump --no-color --disassemble %t | FileCheck %s 
--check-prefix=NOCOLOR
+
+subsp, sp, #16
+strw0, [sp, #12]
+ldrw8, [sp, #12]
+ldrw9, [sp, #12]
+mulw0, w8, w9
+addsp, sp, #16
+
+
+// COLOR: sub  [0;36msp, [0;36msp, [0;31m#0x10
+// COLOR: str  [0;36mw0, [[0;36msp, [0;31m#0xc]
+// COLOR: ldr  [0;36mw8, [[0;36msp, [0;31m#0xc]
+// COLOR: ldr  [0;36mw9, [[0;36msp, [0;31m#0xc]
+// COLOR: mul  [0;36mw0, [0;36mw8, [0;36mw9
+// COLOR: add  [0;36msp, [0;36msp, [0;31m#0x10
+
+// NOCOLOR: subsp, sp, #0x10
+// NOCOLOR: strw0, [sp, #0xc]
+// NOCOLOR: ldrw8, [sp, #0xc]
+// NOCOLOR: ldrw9, [sp, #0xc]
+// NOCOLOR: mulw0, w8, w9
+// NOCOLOR: addsp, sp, #0x10
Index: llvm/include/llvm/Support/FormattedStream.h
===
--- llvm/include/llvm/Support/FormattedStream.h
+++ llvm/include/llvm/Support/FormattedStream.h
@@ -145,11 +145,6 @@
 return *this;
   }
 
-  raw_ostream (enum Colors Color, bool Bold, bool BG) override {
-TheStream->changeColor(Color, Bold, BG);
-return *this;
-  }
-
   bool is_displayed() const override {
 return TheStream->is_displayed();
   }


Index: llvm/tools/llvm-objdump/llvm-objdump.cpp
===
--- llvm/tools/llvm-objdump/llvm-objdump.cpp
+++ llvm/tools/llvm-objdump/llvm-objdump.cpp
@@ -901,6 +901,7 @@
   if (!InstPrinter)
 reportError(Obj.getFileName(),
 "no instruction printer for target " + TripleName);
+  InstPrinter->setUseColor(!NoColor);
   InstPrinter->setPrintImmHex(PrintImmHex);
   InstPrinter->setPrintBranchImmAsAddress(true);
   InstPrinter->setSymbolizeOperands(SymbolizeOperands);
Index: llvm/test/tools/llvm-objdump/MachO/arm64-disassembly-color.s
===
--- /dev/null
+++ llvm/test/tools/llvm-objdump/MachO/arm64-disassembly-color.s
@@ -0,0 +1,25 @@
+// RUN: llvm-mc -triple arm64-apple-macosx %s -filetype=obj -o %t
+// RUN: llvm-objdump --color --disassemble %t | FileCheck %s --check-prefix=COLOR
+// RUN: llvm-objdump --no-color --disassemble %t | FileCheck %s --check-prefix=NOCOLOR
+
+sub	sp, sp, #16
+str	w0, [sp, #12]
+ldr	w8, [sp, #12]
+ldr	w9, [sp, #12]
+mul	w0, w8, w9
+add	sp, sp, #16
+
+
+// COLOR: sub	[0;36msp, [0;36msp, [0;31m#0x10
+// COLOR: str	[0;36mw0, [[0;36msp, [0;31m#0xc]
+// COLOR: ldr	[0;36mw8, [[0;36msp, [0;31m#0xc]
+// COLOR: ldr	[0;36mw9, [[0;36msp, [0;31m#0xc]
+// COLOR: mul	[0;36mw0, [0;36mw8, [0;36mw9
+// COLOR: add	[0;36msp, [0;36msp, [0;31m#0x10
+
+// NOCOLOR: sub	sp, sp, #0x10
+// NOCOLOR: str	w0, [sp, #0xc]
+// NOCOLOR: ldr	w8, [sp, #0xc]
+// NOCOLOR: ldr	w9, [sp, #0xc]
+// NOCOLOR: mul	w0, w8, w9
+// NOCOLOR: add	sp, sp, #0x10
Index: llvm/include/llvm/Support/FormattedStream.h
===
--- llvm/include/llvm/Support/FormattedStream.h
+++ llvm/include/llvm/Support/FormattedStream.h
@@ -145,11 +145,6 @@
 return *this;
   }
 
-  raw_ostream (enum Colors Color, bool Bold, bool BG) override {
-TheStream->changeColor(Color, Bold, BG);
-return *this;
-  }
-
   bool is_displayed() const override {
 return TheStream->is_displayed();
   }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D159164: [lldb] Add assembly syntax highlighting

2023-08-29 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

Similar to the LLVM patch this depends on, this patch is a WIP to get some 
early feedback. This definitely needs a test. Most of the patch deals with 
plumbing the value of `Debugger::GetUseColor` to the `Disassembler`.


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

https://reviews.llvm.org/D159164

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

[Lldb-commits] [PATCH] D159164: [lldb] Add assembly syntax highlighting

2023-08-29 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere created this revision.
JDevlieghere added reviewers: jasonmolenda, DavidSpickett, clayborg.
Herald added subscribers: jrtc27, sdardis.
Herald added a project: All.
JDevlieghere requested review of this revision.

Add support for syntax highlighting assembly. The current patch uses a 
different color to highlight registers, immediate and addresses.


https://reviews.llvm.org/D159164

Files:
  lldb/include/lldb/Core/Disassembler.h
  lldb/include/lldb/lldb-private-interfaces.h
  lldb/source/API/SBTarget.cpp
  lldb/source/Commands/CommandObjectDisassemble.cpp
  lldb/source/Core/Disassembler.cpp
  lldb/source/Core/DumpDataExtractor.cpp
  lldb/source/Expression/IRExecutionUnit.cpp
  lldb/source/Plugins/Architecture/Mips/ArchitectureMips.cpp
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h
  
lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
  lldb/source/Target/ThreadPlanTracer.cpp

Index: lldb/source/Target/ThreadPlanTracer.cpp
===
--- lldb/source/Target/ThreadPlanTracer.cpp
+++ lldb/source/Target/ThreadPlanTracer.cpp
@@ -96,7 +96,8 @@
 Disassembler *ThreadPlanAssemblyTracer::GetDisassembler() {
   if (!m_disassembler_sp)
 m_disassembler_sp = Disassembler::FindPlugin(
-m_process.GetTarget().GetArchitecture(), nullptr, nullptr);
+m_process.GetTarget().GetArchitecture(), nullptr,
+m_process.GetTarget().GetDebugger().GetUseColor(), nullptr);
   return m_disassembler_sp.get();
 }
 
Index: lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
===
--- lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
+++ lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
@@ -70,7 +70,7 @@
 
 const bool prefer_file_cache = true;
 DisassemblerSP disasm_sp(Disassembler::DisassembleBytes(
-m_arch, nullptr, nullptr, range.GetBaseAddress(), opcode_data,
+m_arch, nullptr, nullptr, false, range.GetBaseAddress(), opcode_data,
 opcode_size, 9, prefer_file_cache));
 
 Log *log = GetLog(LLDBLog::Unwind);
Index: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h
===
--- lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h
+++ lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h
@@ -22,8 +22,8 @@
 
 class DisassemblerLLVMC : public lldb_private::Disassembler {
 public:
-  DisassemblerLLVMC(const lldb_private::ArchSpec ,
-const char *flavor /* = NULL */);
+  DisassemblerLLVMC(const lldb_private::ArchSpec , const char *flavor,
+bool use_colors = false);
 
   ~DisassemblerLLVMC() override;
 
@@ -35,7 +35,8 @@
   static llvm::StringRef GetPluginNameStatic() { return "llvm-mc"; }
 
   static lldb::DisassemblerSP CreateInstance(const lldb_private::ArchSpec ,
- const char *flavor);
+ const char *flavor,
+ bool use_colors);
 
   size_t DecodeInstructions(const lldb_private::Address _addr,
 const lldb_private::DataExtractor ,
@@ -72,6 +73,7 @@
   InstructionLLVMC *m_inst;
   std::mutex m_mutex;
   bool m_data_from_file;
+  bool m_use_colors;
   // Save the AArch64 ADRP instruction word and address it was at,
   // in case the next instruction is an ADD to the same register;
   // this is a pc-relative address calculation and we need both
Index: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
===
--- lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
+++ lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
@@ -54,7 +54,7 @@
 public:
   static std::unique_ptr
   Create(const char *triple, const char *cpu, const char *features_str,
- unsigned flavor, DisassemblerLLVMC );
+ unsigned flavor, bool use_color, DisassemblerLLVMC );
 
   ~MCDisasmInstance() = default;
 
@@ -1227,7 +1227,7 @@
 std::unique_ptr
 DisassemblerLLVMC::MCDisasmInstance::Create(const char *triple, const char *cpu,
 const char *features_str,
-unsigned flavor,
+unsigned flavor, bool use_color,
 DisassemblerLLVMC ) {
   using Instance = std::unique_ptr;
 
@@ -1291,6 +1291,7 @@
 return Instance();
 
   instr_printer_up->setPrintBranchImmAsAddress(true);
+  instr_printer_up->setUseColor(use_color);
 
   // Not all targets may have registered createMCInstrAnalysis().
   std::unique_ptr instr_analysis_up(
@@ -1344,6 +1345,8 @@

[Lldb-commits] [PATCH] D159017: [lldb/docs] Silence warnings when generating website

2023-08-28 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

Overall this LGTM. It's really unfortunate we have to define things like 
`__hex__` for every class. Is there a way to either (1) silence this warning on 
(2) have SWIG generate this for us?


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

https://reviews.llvm.org/D159017

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

[Lldb-commits] [PATCH] D158958: [LLDB][REPL] Change the default tab size

2023-08-28 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

+ @aprantl for impact on the Swift REPL


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158958

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

[Lldb-commits] [PATCH] D158785: [lldb] Add a "thread extrainfo" LC_NOTE for Mach-O corefiles, to store the thread IDs of the threads

2023-08-25 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added inline comments.



Comment at: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:5682-5693
+  for (uint32_t i = 0; i < m_header.ncmds; ++i) {
+const uint32_t cmd_offset = offset;
+llvm::MachO::load_command lc = {};
+if (m_data.GetU32(, , 2) == nullptr)
+  break;
+if (lc.cmd == LC_NOTE) {
+  char data_owner[17];

There's a *lot* of duplication between reading `LC_NOTE`s. At some point we 
should extract the common code instead of copy-pasting it every time. 



Comment at: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:5723-5724
+}
+const uint32_t num_threads = threads->GetSize();
+for (uint32_t i = 0; i < num_threads; i++) {
+  StructuredData::Dictionary *thread;

`StructuredData::Array::GetSize` returns a `size_t`. No point in turning this 
into a `uint32_t`. 



Comment at: lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp:603
+
+// populate used_tids
+for (uint32_t i = 0; i < num_threads; i++) {

Capitalize + period. 



Comment at: lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp:608-619
+// If any threads have an unspecified thread id,
+// find an unused number, use it instead.
+tid_t current_unused_tid = 0;
+for (uint32_t i = 0; i < num_threads; i++) {
+  if (tids[i] == LLDB_INVALID_THREAD_ID) {
+while (used_tids.find(current_unused_tid) != used_tids.end()) {
+  current_unused_tid++;

This code is (presumably on purpose) trying to find gaps in the available tids: 
for example if I had `[1, 2, ? ,4, ?, 6]` this is going to turn that into `[1, 
2, (3), 4, (5), 6]`. If that property matters, it probably should be part of 
the comment. If it doesn't, you could exploit the fact that the `set` is sorted 
and just take the last value (6 for the previous example) and return `[1, 2, 4, 
6, 7, 8]`, but again, I assume that's what you're trying to avoid here. 



Comment at: lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp:628-629
+  for (uint32_t i = 0; i < num_threads; i++) {
+ThreadSP thread_sp(new ThreadMachCore(*this, tids[i], i));
 new_thread_list.AddThread(thread_sp);
   }

`make_shared` (avoid "naked new")


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158785

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

[Lldb-commits] [PATCH] D158893: [lldb] Fix TestVSCode_completions on Darwin

2023-08-25 Thread Jonas Devlieghere via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa7ca1175d0d4: [lldb] Fix  re-enable 
TestVSCode_completions on Darwin (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158893

Files:
  lldb/test/API/tools/lldb-vscode/completions/TestVSCode_completions.py
  lldb/test/API/tools/lldb-vscode/completions/main.cpp


Index: lldb/test/API/tools/lldb-vscode/completions/main.cpp
===
--- lldb/test/API/tools/lldb-vscode/completions/main.cpp
+++ lldb/test/API/tools/lldb-vscode/completions/main.cpp
@@ -12,7 +12,11 @@
   foo* next_foo;
 };
 
-int fun(std::vector var) {
+struct baz {
+  char c;
+};
+
+int fun(std::vector var) {
   return var.size(); // breakpoint 1
 }
 
@@ -21,10 +25,10 @@
   int var2 = 1;
   std::string str1 = "a";
   std::string str2 = "b";
-  std::vector vec;
+  std::vector vec;
   fun(vec);
   bar bar1 = {2};
-  bar* bar2 =  
+  bar* bar2 = 
   foo foo1 = {3,, bar1, NULL};
   return 0; // breakpoint 2
 }
Index: lldb/test/API/tools/lldb-vscode/completions/TestVSCode_completions.py
===
--- lldb/test/API/tools/lldb-vscode/completions/TestVSCode_completions.py
+++ lldb/test/API/tools/lldb-vscode/completions/TestVSCode_completions.py
@@ -19,19 +19,18 @@
 self.assertNotIn(not_expected_item, actual_list)
 
 @skipIfWindows
-@skipIfDarwin  # Skip this test for now until we can figure out why tings 
aren't working on build bots
 def test_completions(self):
 """
 Tests the completion request at different breakpoints
 """
 program = self.getBuildArtifact("a.out")
 self.build_and_launch(program)
+
 source = "main.cpp"
 breakpoint1_line = line_number(source, "// breakpoint 1")
 breakpoint2_line = line_number(source, "// breakpoint 2")
-breakpoint_ids = self.set_source_breakpoints(
-source, [breakpoint1_line, breakpoint2_line]
-)
+
+self.set_source_breakpoints(source, [breakpoint1_line, 
breakpoint2_line])
 self.continue_to_next_stop()
 
 # shouldn't see variables inside main
@@ -40,7 +39,7 @@
 [
 {
 "text": "var",
-"label": "var -- vector> &",
+"label": "var -- vector &",
 }
 ],
 [
@@ -71,7 +70,7 @@
 [
 {
 "text": "var",
-"label": "var -- vector> &",
+"label": "var -- vector &",
 }
 ],
 )


Index: lldb/test/API/tools/lldb-vscode/completions/main.cpp
===
--- lldb/test/API/tools/lldb-vscode/completions/main.cpp
+++ lldb/test/API/tools/lldb-vscode/completions/main.cpp
@@ -12,7 +12,11 @@
   foo* next_foo;
 };
 
-int fun(std::vector var) {
+struct baz {
+  char c;
+};
+
+int fun(std::vector var) {
   return var.size(); // breakpoint 1
 }
 
@@ -21,10 +25,10 @@
   int var2 = 1;
   std::string str1 = "a";
   std::string str2 = "b";
-  std::vector vec;
+  std::vector vec;
   fun(vec);
   bar bar1 = {2};
-  bar* bar2 =  
+  bar* bar2 = 
   foo foo1 = {3,, bar1, NULL};
   return 0; // breakpoint 2
 }
Index: lldb/test/API/tools/lldb-vscode/completions/TestVSCode_completions.py
===
--- lldb/test/API/tools/lldb-vscode/completions/TestVSCode_completions.py
+++ lldb/test/API/tools/lldb-vscode/completions/TestVSCode_completions.py
@@ -19,19 +19,18 @@
 self.assertNotIn(not_expected_item, actual_list)
 
 @skipIfWindows
-@skipIfDarwin  # Skip this test for now until we can figure out why tings aren't working on build bots
 def test_completions(self):
 """
 Tests the completion request at different breakpoints
 """
 program = self.getBuildArtifact("a.out")
 self.build_and_launch(program)
+
 source = "main.cpp"
 breakpoint1_line = line_number(source, "// breakpoint 1")
 breakpoint2_line = line_number(source, "// breakpoint 2")
-breakpoint_ids = self.set_source_breakpoints(
-source, [breakpoint1_line, breakpoint2_line]
-)
+
+self.set_source_breakpoints(source, [breakpoint1_line, breakpoint2_line])
 self.continue_to_next_stop()
 
 # shouldn't see variables inside main
@@ -40,7 +39,7 @@
 [
 {
 "text": "var",
-"label": "var -- vector> &",
+"label": "var -- vector &",
 }
 ],
 [
@@ -71,7 +70,7 @@
 [
 {
 "text":

[Lldb-commits] [PATCH] D158893: [lldb] Fix TestVSCode_completions on Darwin

2023-08-25 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

In D158893#4618503 , @wallace wrote:

> Awesome!!

Thank you for the prompt reviews!


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

https://reviews.llvm.org/D158893

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

[Lldb-commits] [PATCH] D158893: [lldb] Fix TestVSCode_completions on Darwin

2023-08-25 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere created this revision.
JDevlieghere added reviewers: wallace, clayborg.
Herald added a project: All.
JDevlieghere requested review of this revision.

The test was expecting `vector> &` while the test returned 
`vector &`. Since `verify_completions` doesn't support regex matching, 
sidestep the issue by using a custom type (`baz`).


https://reviews.llvm.org/D158893

Files:
  lldb/test/API/tools/lldb-vscode/completions/TestVSCode_completions.py
  lldb/test/API/tools/lldb-vscode/completions/main.cpp


Index: lldb/test/API/tools/lldb-vscode/completions/main.cpp
===
--- lldb/test/API/tools/lldb-vscode/completions/main.cpp
+++ lldb/test/API/tools/lldb-vscode/completions/main.cpp
@@ -12,7 +12,11 @@
   foo* next_foo;
 };
 
-int fun(std::vector var) {
+struct baz {
+  char c;
+};
+
+int fun(std::vector var) {
   return var.size(); // breakpoint 1
 }
 
@@ -21,10 +25,10 @@
   int var2 = 1;
   std::string str1 = "a";
   std::string str2 = "b";
-  std::vector vec;
+  std::vector vec;
   fun(vec);
   bar bar1 = {2};
-  bar* bar2 =  
+  bar* bar2 = 
   foo foo1 = {3,, bar1, NULL};
   return 0; // breakpoint 2
 }
Index: lldb/test/API/tools/lldb-vscode/completions/TestVSCode_completions.py
===
--- lldb/test/API/tools/lldb-vscode/completions/TestVSCode_completions.py
+++ lldb/test/API/tools/lldb-vscode/completions/TestVSCode_completions.py
@@ -19,19 +19,18 @@
 self.assertNotIn(not_expected_item, actual_list)
 
 @skipIfWindows
-@skipIfDarwin  # Skip this test for now until we can figure out why tings 
aren't working on build bots
 def test_completions(self):
 """
 Tests the completion request at different breakpoints
 """
 program = self.getBuildArtifact("a.out")
 self.build_and_launch(program)
+
 source = "main.cpp"
 breakpoint1_line = line_number(source, "// breakpoint 1")
 breakpoint2_line = line_number(source, "// breakpoint 2")
-breakpoint_ids = self.set_source_breakpoints(
-source, [breakpoint1_line, breakpoint2_line]
-)
+
+self.set_source_breakpoints(source, [breakpoint1_line, 
breakpoint2_line])
 self.continue_to_next_stop()
 
 # shouldn't see variables inside main
@@ -40,7 +39,7 @@
 [
 {
 "text": "var",
-"label": "var -- vector> &",
+"label": "var -- vector &",
 }
 ],
 [
@@ -71,7 +70,7 @@
 [
 {
 "text": "var",
-"label": "var -- vector> &",
+"label": "var -- vector &",
 }
 ],
 )


Index: lldb/test/API/tools/lldb-vscode/completions/main.cpp
===
--- lldb/test/API/tools/lldb-vscode/completions/main.cpp
+++ lldb/test/API/tools/lldb-vscode/completions/main.cpp
@@ -12,7 +12,11 @@
   foo* next_foo;
 };
 
-int fun(std::vector var) {
+struct baz {
+  char c;
+};
+
+int fun(std::vector var) {
   return var.size(); // breakpoint 1
 }
 
@@ -21,10 +25,10 @@
   int var2 = 1;
   std::string str1 = "a";
   std::string str2 = "b";
-  std::vector vec;
+  std::vector vec;
   fun(vec);
   bar bar1 = {2};
-  bar* bar2 =  
+  bar* bar2 = 
   foo foo1 = {3,, bar1, NULL};
   return 0; // breakpoint 2
 }
Index: lldb/test/API/tools/lldb-vscode/completions/TestVSCode_completions.py
===
--- lldb/test/API/tools/lldb-vscode/completions/TestVSCode_completions.py
+++ lldb/test/API/tools/lldb-vscode/completions/TestVSCode_completions.py
@@ -19,19 +19,18 @@
 self.assertNotIn(not_expected_item, actual_list)
 
 @skipIfWindows
-@skipIfDarwin  # Skip this test for now until we can figure out why tings aren't working on build bots
 def test_completions(self):
 """
 Tests the completion request at different breakpoints
 """
 program = self.getBuildArtifact("a.out")
 self.build_and_launch(program)
+
 source = "main.cpp"
 breakpoint1_line = line_number(source, "// breakpoint 1")
 breakpoint2_line = line_number(source, "// breakpoint 2")
-breakpoint_ids = self.set_source_breakpoints(
-source, [breakpoint1_line, breakpoint2_line]
-)
+
+self.set_source_breakpoints(source, [breakpoint1_line, breakpoint2_line])
 self.continue_to_next_stop()
 
 # shouldn't see variables inside main
@@ -40,7 +39,7 @@
 [
 {
 "text": "var",
-"label": "var -- vector> &",
+"label": "var -- vector &",
 }
 ],
 [
@@ -71,7 +70,7 @@
 [
 {

[Lldb-commits] [PATCH] D158801: [lldb-vscode] Update package.json

2023-08-24 Thread Jonas Devlieghere via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7bd632895816: [lldb-vscode] Update package.json (authored by 
JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D158801?vs=553320=553363#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158801

Files:
  lldb/tools/lldb-vscode/package.json

Index: lldb/tools/lldb-vscode/package.json
===
--- lldb/tools/lldb-vscode/package.json
+++ lldb/tools/lldb-vscode/package.json
@@ -1,26 +1,30 @@
 {
 	"name": "lldb-vscode",
-	"displayName": "LLDB native Debug stub",
+	"displayName": "LLDB VSCode",
 	"version": "0.1.0",
 	"publisher": "llvm",
-	"repository": "llvm.org",
-	"description": "Debug adapter for LLDB which uses a C++ tool to interface directly with LLDB.",
-	"author": {
-		"name": "Greg Clayton",
-		"email": "clayb...@gmail.com"
+	"homepage": "https://lldb.llvm.org;,
+	"description": "LLDB debugging from VSCode",
+	"license": "Apache 2.0 License with LLVM exceptions",
+	"repository": {
+		"type": "git",
+		"url": "https://github.com/llvm/llvm-project.git;
+	},
+	"bugs": {
+		"url": "https://github.com/llvm/llvm-project/issues;
 	},
-	"license": "LLVM",
 	"keywords": [
-		"multi-root ready"
+		"C",
+		"C++",
+		"LLVM",
+		"LLDB"
 	],
 	"engines": {
-		"vscode": "^1.18.0",
-		"node": "^7.9.0"
+		"vscode": "^1.18.0"
 	},
 	"categories": [
 		"Debuggers"
 	],
-	"private": true,
 	"devDependencies": {
 		"@types/node": "7.0.43",
 		"@types/mocha": "2.2.45",
@@ -34,69 +38,69 @@
 	"contributes": {
 		"languages": [
 			{
-"id": "lldb.disassembly",
-"aliases": [
-"Disassembly"
-],
-"extensions": [
-".disasm"
-]
+"id": "lldb.disassembly",
+"aliases": [
+	"Disassembly"
+],
+"extensions": [
+	".disasm"
+]
 			}
 		],
 		"grammars": [
 			{
-"language": "lldb.disassembly",
-"scopeName": "source.disassembly",
-"path": "./syntaxes/disassembly.json"
-}
-],
-"breakpoints": [
-  {
-"language": "ada"
-  },
-  {
-"language": "arm"
-  },
-  {
-"language": "asm"
-  },
-  {
-"language": "c"
-  },
-  {
-"language": "cpp"
-  },
-  {
-"language": "crystal"
-  },
-  {
-"language": "d"
-  },
-  {
-"language": "fortan"
-  },
-  {
-"language": "fortran-modern"
-  },
-  {
-"language": "nim"
-  },
-  {
-"language": "objective-c"
-  },
-  {
-"language": "objectpascal"
-  },
-  {
-"language": "pascal"
-  },
-  {
-"language": "rust"
-  },
-  {
-"language": "swift"
-  }
-],
+"language": "lldb.disassembly",
+"scopeName": "source.disassembly",
+"path": "./syntaxes/disassembly.json"
+			}
+		],
+		"breakpoints": [
+			{
+"language": "ada"
+			},
+			{
+"language": "arm"
+			},
+			{
+"language": "asm"
+			},
+			{
+"language": "c"
+			},
+			{
+"language": "cpp"
+			},
+			{
+"language": "crystal"
+			},
+			{
+"language": "d"
+			},
+			{
+"language": "fortan"
+			},
+			{
+"language": "fortran-modern"
+			},
+			{
+"language": "nim"
+			},
+			{
+"language": "objective-c"
+			},
+			{
+"language": "objectpascal"
+			},
+			{
+"language": "pascal"
+			},
+			{
+"language": "rust"
+			},
+			{
+"language": "swift"
+			}
+		],
 		"debuggers": [
 			{
 "type": "lldb-vscode",
@@ -149,7 +153,7 @@
 			},
 			"env": {
 "type": "array",
-"description": "Additional environment variables to set when launching the program. This is an array of strings that contains the variable name followed by an optional '=' character and the environment variable's value. Example:  [\"FOO=BAR\", \"BAZ\"]",
+"description": "Additional environment variables to set when launching the program. This is an array of strings that contains the variable name followed by an optional '=' character and the environment variable's value.",
 "default": []
 			},
 			"stopOnEntry": {
@@ -183,12 +187,12 @@
 			},
 			"sourceMap": {
 "type": "array",
-"description": "Specify an array of path remappings; each element must itself be a two element array containing a source and destination pathname. Overrides sourcePath.",
+"description": "Specify an array of path remappings; each element must itself be a two element array

[Lldb-commits] [PATCH] D158801: [lldb-vscode] Update package.json

2023-08-24 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere created this revision.
JDevlieghere added reviewers: clayborg, wallace.
Herald added a project: All.
JDevlieghere requested review of this revision.

Update package.json:

- Update license, display name, repository and keywords
- Add homepage, issue page
- Fix formatting and typos


https://reviews.llvm.org/D158801

Files:
  lldb/tools/lldb-vscode/package.json

Index: lldb/tools/lldb-vscode/package.json
===
--- lldb/tools/lldb-vscode/package.json
+++ lldb/tools/lldb-vscode/package.json
@@ -1,26 +1,30 @@
 {
 	"name": "lldb-vscode",
-	"displayName": "LLDB native Debug stub",
+	"displayName": "LLDB VSCode",
 	"version": "0.1.0",
 	"publisher": "llvm",
-	"repository": "llvm.org",
-	"description": "Debug adapter for LLDB which uses a C++ tool to interface directly with LLDB.",
-	"author": {
-		"name": "Greg Clayton",
-		"email": "clayb...@gmail.com"
+	"homepage": "https://lldb.llvm.org;,
+	"description": "LLDB debugger for VSCode",
+	"license": "Apache 2.0 License with LLVM exceptions",
+	"repository": {
+		"type": "git",
+		"url": "https://github.com/llvm/llvm-project.git;
 	},
-	"license": "LLVM",
+	"bugs": {
+"url": "https://github.com/llvm/llvm-project/issues;
+  },
 	"keywords": [
-		"multi-root ready"
+		"C",
+		"C++",
+		"LLVM",
+		"LLDB"
 	],
 	"engines": {
-		"vscode": "^1.18.0",
-		"node": "^7.9.0"
+		"vscode": "^1.18.0"
 	},
 	"categories": [
 		"Debuggers"
 	],
-	"private": true,
 	"devDependencies": {
 		"@types/node": "7.0.43",
 		"@types/mocha": "2.2.45",
@@ -34,69 +38,69 @@
 	"contributes": {
 		"languages": [
 			{
-"id": "lldb.disassembly",
-"aliases": [
-"Disassembly"
-],
-"extensions": [
-".disasm"
-]
+"id": "lldb.disassembly",
+"aliases": [
+	"Disassembly"
+],
+"extensions": [
+	".disasm"
+]
 			}
 		],
 		"grammars": [
 			{
-"language": "lldb.disassembly",
-"scopeName": "source.disassembly",
-"path": "./syntaxes/disassembly.json"
-}
-],
-"breakpoints": [
-  {
-"language": "ada"
-  },
-  {
-"language": "arm"
-  },
-  {
-"language": "asm"
-  },
-  {
-"language": "c"
-  },
-  {
-"language": "cpp"
-  },
-  {
-"language": "crystal"
-  },
-  {
-"language": "d"
-  },
-  {
-"language": "fortan"
-  },
-  {
-"language": "fortran-modern"
-  },
-  {
-"language": "nim"
-  },
-  {
-"language": "objective-c"
-  },
-  {
-"language": "objectpascal"
-  },
-  {
-"language": "pascal"
-  },
-  {
-"language": "rust"
-  },
-  {
-"language": "swift"
-  }
-],
+"language": "lldb.disassembly",
+"scopeName": "source.disassembly",
+"path": "./syntaxes/disassembly.json"
+			}
+		],
+		"breakpoints": [
+			{
+"language": "ada"
+			},
+			{
+"language": "arm"
+			},
+			{
+"language": "asm"
+			},
+			{
+"language": "c"
+			},
+			{
+"language": "cpp"
+			},
+			{
+"language": "crystal"
+			},
+			{
+"language": "d"
+			},
+			{
+"language": "fortan"
+			},
+			{
+"language": "fortran-modern"
+			},
+			{
+"language": "nim"
+			},
+			{
+"language": "objective-c"
+			},
+			{
+"language": "objectpascal"
+			},
+			{
+"language": "pascal"
+			},
+			{
+"language": "rust"
+			},
+			{
+"language": "swift"
+			}
+		],
 		"debuggers": [
 			{
 "type": "lldb-vscode",
@@ -149,7 +153,7 @@
 			},
 			"env": {
 "type": "array",
-"description": "Additional environment variables to set when launching the program. This is an array of strings that contains the variable name followed by an optional '=' character and the environment variable's value. Example:  [\"FOO=BAR\", \"BAZ\"]",
+"description": "Additional environment variables to set when launching the program. This is an array of strings that contains the variable name followed by an optional '=' character and the environment variable's value.",
 "default": []
 			},
 			"stopOnEntry": {
@@ -183,12 +187,12 @@
 			},
 			"sourceMap": {
 "type": "array",
-"description": "Specify an array of path remappings; each element must itself be a two element array containing a source and destination pathname. Overrides sourcePath.",
+"description": "Specify an array of path remappings; each element must itself be a two element array containing a source and destination path name. Overrides sourcePath.",
 "default":

[Lldb-commits] [PATCH] D158788: [lldb-vscode] Use a switch to avoid else-after-return (NFC)

2023-08-24 Thread Jonas Devlieghere 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 rG719506769a89: [lldb-vscode] Use a switch to avoid 
else-after-return (NFC) (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158788

Files:
  lldb/tools/lldb-vscode/ProgressEvent.cpp


Index: lldb/tools/lldb-vscode/ProgressEvent.cpp
===
--- lldb/tools/lldb-vscode/ProgressEvent.cpp
+++ lldb/tools/lldb-vscode/ProgressEvent.cpp
@@ -9,6 +9,7 @@
 #include "ProgressEvent.h"
 
 #include "JSONUtils.h"
+#include "llvm/Support/ErrorHandling.h"
 #include 
 
 using namespace lldb_vscode;
@@ -91,12 +92,15 @@
 ProgressEventType ProgressEvent::GetEventType() const { return m_event_type; }
 
 StringRef ProgressEvent::GetEventName() const {
-  if (m_event_type == progressStart)
+  switch (m_event_type) {
+  case progressStart:
 return "progressStart";
-  else if (m_event_type == progressEnd)
-return "progressEnd";
-  else
+  case progressUpdate:
 return "progressUpdate";
+  case progressEnd:
+return "progressEnd";
+  }
+  llvm_unreachable("All cases handled above!");
 }
 
 json::Value ProgressEvent::ToJSON() const {


Index: lldb/tools/lldb-vscode/ProgressEvent.cpp
===
--- lldb/tools/lldb-vscode/ProgressEvent.cpp
+++ lldb/tools/lldb-vscode/ProgressEvent.cpp
@@ -9,6 +9,7 @@
 #include "ProgressEvent.h"
 
 #include "JSONUtils.h"
+#include "llvm/Support/ErrorHandling.h"
 #include 
 
 using namespace lldb_vscode;
@@ -91,12 +92,15 @@
 ProgressEventType ProgressEvent::GetEventType() const { return m_event_type; }
 
 StringRef ProgressEvent::GetEventName() const {
-  if (m_event_type == progressStart)
+  switch (m_event_type) {
+  case progressStart:
 return "progressStart";
-  else if (m_event_type == progressEnd)
-return "progressEnd";
-  else
+  case progressUpdate:
 return "progressUpdate";
+  case progressEnd:
+return "progressEnd";
+  }
+  llvm_unreachable("All cases handled above!");
 }
 
 json::Value ProgressEvent::ToJSON() const {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D158788: [lldb-vscode] Use a switch to avoid else-after-return

2023-08-24 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere created this revision.
JDevlieghere added a reviewer: wallace.
Herald added a project: All.
JDevlieghere requested review of this revision.

Use a switch to avoid else-after-return.


https://reviews.llvm.org/D158788

Files:
  lldb/tools/lldb-vscode/ProgressEvent.cpp


Index: lldb/tools/lldb-vscode/ProgressEvent.cpp
===
--- lldb/tools/lldb-vscode/ProgressEvent.cpp
+++ lldb/tools/lldb-vscode/ProgressEvent.cpp
@@ -9,6 +9,7 @@
 #include "ProgressEvent.h"
 
 #include "JSONUtils.h"
+#include "llvm/Support/ErrorHandling.h"
 #include 
 
 using namespace lldb_vscode;
@@ -91,12 +92,15 @@
 ProgressEventType ProgressEvent::GetEventType() const { return m_event_type; }
 
 StringRef ProgressEvent::GetEventName() const {
-  if (m_event_type == progressStart)
+  switch (m_event_type) {
+  case progressStart:
 return "progressStart";
-  else if (m_event_type == progressEnd)
-return "progressEnd";
-  else
+  case progressUpdate:
 return "progressUpdate";
+  case progressEnd:
+return "progressEnd";
+  }
+  llvm_unreachable("All cases handled above!");
 }
 
 json::Value ProgressEvent::ToJSON() const {


Index: lldb/tools/lldb-vscode/ProgressEvent.cpp
===
--- lldb/tools/lldb-vscode/ProgressEvent.cpp
+++ lldb/tools/lldb-vscode/ProgressEvent.cpp
@@ -9,6 +9,7 @@
 #include "ProgressEvent.h"
 
 #include "JSONUtils.h"
+#include "llvm/Support/ErrorHandling.h"
 #include 
 
 using namespace lldb_vscode;
@@ -91,12 +92,15 @@
 ProgressEventType ProgressEvent::GetEventType() const { return m_event_type; }
 
 StringRef ProgressEvent::GetEventName() const {
-  if (m_event_type == progressStart)
+  switch (m_event_type) {
+  case progressStart:
 return "progressStart";
-  else if (m_event_type == progressEnd)
-return "progressEnd";
-  else
+  case progressUpdate:
 return "progressUpdate";
+  case progressEnd:
+return "progressEnd";
+  }
+  llvm_unreachable("All cases handled above!");
 }
 
 json::Value ProgressEvent::ToJSON() const {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D158323: [lldb/infrastructure] Revamp lldb.llvm.org

2023-08-23 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

This patch changes the theme and removes the inline table-of-contents that are 
always shown on the right. If there are any other changes needed to better 
support the theme we can apply them as we discover them. LGTM.


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

https://reviews.llvm.org/D158323

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

[Lldb-commits] [PATCH] D158323: [lldb/infrastructure] Revamp lldb.llvm.org

2023-08-23 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added inline comments.



Comment at: lldb/docs/conf.py:52-57
 try:
 import sphinx_automodapi.automodapi
 except ModuleNotFoundError:
 print(
 f"install sphinx_automodapi with {sys.executable} -m pip install 
sphinx_automodapi"
 )

We should do something similar for the `furo` theme. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158323

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

[Lldb-commits] [PATCH] D157764: [LLDB] Allow expression evaluators to set arbitrary timeouts

2023-08-22 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

In D157764#4608265 , @wallace wrote:

> @jasonmolenda , yep. I have already reverted this patch. I'll figure out how 
> to make that work nicely with these changes.

My recommendation would be to change the timeout in the test from 0 to 500ms to 
maintain the previous behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157764

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

[Lldb-commits] [PATCH] D158467: [lldb] Link back to the LLVM Project and Developer Policy from the docs

2023-08-22 Thread Jonas Devlieghere via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG34f84697c37e: [lldb] Link back to the LLVM Project and 
Developer Policy from the docs (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158467

Files:
  lldb/docs/index.rst


Index: lldb/docs/index.rst
===
--- lldb/docs/index.rst
+++ lldb/docs/index.rst
@@ -6,8 +6,9 @@
 Welcome to the LLDB documentation!
 
 LLDB is a next generation, high-performance debugger. It is built as a set of
-reusable components which highly leverage existing libraries in the larger LLVM
-Project, such as the Clang expression parser and LLVM disassembler.
+reusable components which highly leverage existing libraries in the larger
+`LLVM Project `_, such as the Clang expression parser and
+LLVM disassembler.
 
 LLDB is the default debugger in Xcode on macOS and supports debugging C,
 Objective-C and C++ on the desktop and iOS devices and simulator.
@@ -163,5 +164,6 @@
Source Code 
Releases 
Discussion Forums 
+   Developer Policy 
Bug Reports 
Code Reviews 


Index: lldb/docs/index.rst
===
--- lldb/docs/index.rst
+++ lldb/docs/index.rst
@@ -6,8 +6,9 @@
 Welcome to the LLDB documentation!
 
 LLDB is a next generation, high-performance debugger. It is built as a set of
-reusable components which highly leverage existing libraries in the larger LLVM
-Project, such as the Clang expression parser and LLVM disassembler.
+reusable components which highly leverage existing libraries in the larger
+`LLVM Project `_, such as the Clang expression parser and
+LLVM disassembler.
 
 LLDB is the default debugger in Xcode on macOS and supports debugging C,
 Objective-C and C++ on the desktop and iOS devices and simulator.
@@ -163,5 +164,6 @@
Source Code 
Releases 
Discussion Forums 
+   Developer Policy 
Bug Reports 
Code Reviews 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D158470: [lldb] Add support for recognizing swift mangled names

2023-08-22 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added inline comments.



Comment at: lldb/source/Core/Mangled.cpp:61
 
+  // Swift's older style of mangling used "_T" as a mangling prefix. This can
+  // lead to false positives with other symbols that just so happen to start

aprantl wrote:
> fdeazeve wrote:
> > JDevlieghere wrote:
> > > aprantl wrote:
> > > > Feel free to completely ignore this, it's a pointless micro 
> > > > optimization.
> > > > 
> > > > I'm curious if it would be more efficient to write this as 
> > > > 
> > > > ```
> > > > switch (name[0]) {
> > > >   case '?': return Mangled::eManglingSchemeMSVC;
> > > >   case '_': 
> > > >  switch(name[1]) {
> > > > ...
> > > >  }
> > > >   case '$': 
> > > >  switch(name[1]) {
> > > > ...
> > > >  }
> > > > ```
> > > > or if it would actually be slower than the chain of "vector" 
> > > > comparisons we have right now?
> > > I actually started writing a similar comment before discarding it. Even 
> > > if this code is as hot as I expect it to be, I don't think it would 
> > > outweigh the complexity and the potential for bugs. I really like how you 
> > > can glance at the code and see the different mangling schemes and that's 
> > > the first thing we'd lose. Anyway happy to be proven wrong too. 
> > Honestly the optimizer is pretty good at doing those, look at the IR: 
> > https://godbolt.org/z/PY3TeadbM
> Sounds good, let's leave it!
> 
> Just trolling now: should we use some crazy #ifdef magic to flip the order 
> based on the host platform, such the `?` comes first on windows and `$` comes 
> first on Darwin?
Did someone say tablegen? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158470

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

[Lldb-commits] [PATCH] D158470: [lldb] Add support for recognizing swift mangled names

2023-08-22 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added inline comments.



Comment at: lldb/source/Core/Mangled.cpp:61
 
+  // Swift's older style of mangling used "_T" as a mangling prefix. This can
+  // lead to false positives with other symbols that just so happen to start

aprantl wrote:
> Feel free to completely ignore this, it's a pointless micro optimization.
> 
> I'm curious if it would be more efficient to write this as 
> 
> ```
> switch (name[0]) {
>   case '?': return Mangled::eManglingSchemeMSVC;
>   case '_': 
>  switch(name[1]) {
> ...
>  }
>   case '$': 
>  switch(name[1]) {
> ...
>  }
> ```
> or if it would actually be slower than the chain of "vector" comparisons we 
> have right now?
I actually started writing a similar comment before discarding it. Even if this 
code is as hot as I expect it to be, I don't think it would outweigh the 
complexity and the potential for bugs. I really like how you can glance at the 
code and see the different mangling schemes and that's the first thing we'd 
lose. Anyway happy to be proven wrong too. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158470

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

[Lldb-commits] [PATCH] D158470: [lldb] Add support for recognizing swift mangled names

2023-08-21 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

Given support for other languages like Rust and D, I think this is reasonable. 
LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158470

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

[Lldb-commits] [PATCH] D158457: [lldb][NFCI] Change return type of UnixSignals::GetSignalInfo

2023-08-21 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158457

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

[Lldb-commits] [PATCH] D158467: [lldb] Link back to the LLVM Project and Developer Policy from the docs

2023-08-21 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere created this revision.
JDevlieghere added reviewers: LLDB, tonic.
Herald added a subscriber: arphaman.
Herald added a project: All.
JDevlieghere requested review of this revision.

Add backlinks to the LLVM Project and the LLVM Developer Policy from the docs. 
As suggested by Tanya in 
https://discourse.llvm.org/t/rfc-revamping-lldbs-website/72899.


https://reviews.llvm.org/D158467

Files:
  lldb/docs/index.rst


Index: lldb/docs/index.rst
===
--- lldb/docs/index.rst
+++ lldb/docs/index.rst
@@ -6,8 +6,9 @@
 Welcome to the LLDB documentation!
 
 LLDB is a next generation, high-performance debugger. It is built as a set of
-reusable components which highly leverage existing libraries in the larger LLVM
-Project, such as the Clang expression parser and LLVM disassembler.
+reusable components which highly leverage existing libraries in the larger
+`LLVM Project `_, such as the Clang expression parser and
+LLVM disassembler.
 
 LLDB is the default debugger in Xcode on macOS and supports debugging C,
 Objective-C and C++ on the desktop and iOS devices and simulator.
@@ -163,5 +164,6 @@
Source Code 
Releases 
Discussion Forums 
+   Developer Policy 
Bug Reports 
Code Reviews 


Index: lldb/docs/index.rst
===
--- lldb/docs/index.rst
+++ lldb/docs/index.rst
@@ -6,8 +6,9 @@
 Welcome to the LLDB documentation!
 
 LLDB is a next generation, high-performance debugger. It is built as a set of
-reusable components which highly leverage existing libraries in the larger LLVM
-Project, such as the Clang expression parser and LLVM disassembler.
+reusable components which highly leverage existing libraries in the larger
+`LLVM Project `_, such as the Clang expression parser and
+LLVM disassembler.
 
 LLDB is the default debugger in Xcode on macOS and supports debugging C,
 Objective-C and C++ on the desktop and iOS devices and simulator.
@@ -163,5 +164,6 @@
Source Code 
Releases 
Discussion Forums 
+   Developer Policy 
Bug Reports 
Code Reviews 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D158023: [lldb] Simplify the LLDB website structure

2023-08-18 Thread Jonas Devlieghere via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3e3880e370c8: [lldb] Simplify the LLDB website structure 
(authored by JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D158023?vs=550791=551578#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158023

Files:
  lldb/docs/.htaccess
  lldb/docs/design/overview.rst
  lldb/docs/design/sbapi.rst
  lldb/docs/index.rst
  lldb/docs/resources/extensions.rst
  lldb/docs/resources/fuzzing.rst
  lldb/docs/resources/overview.rst
  lldb/docs/resources/projects.rst
  lldb/docs/resources/sbapi.rst
  lldb/docs/status/features.rst
  lldb/docs/status/goals.rst
  lldb/docs/status/projects.rst
  lldb/docs/status/releases.rst
  lldb/docs/status/status.rst
  lldb/docs/use/extensions.rst

Index: lldb/docs/status/status.rst
===
--- lldb/docs/status/status.rst
+++ /dev/null
@@ -1,68 +0,0 @@
-Status
-==
-
-FreeBSD

-
-LLDB on FreeBSD lags behind the Linux implementation but is improving rapidly.
-For more details, see the Features by OS section below.
-
-Linux
--
-
-LLDB is improving on Linux. Linux is nearing feature completeness with Darwin
-to debug x86_64, i386, ARM, AArch64, IBM POWER (ppc64), and IBM Z (s390x)
-programs. For more details, see the Features by OS section below.
-
-macOS
--
-
-LLDB is the system debugger on macOS, iOS, tvOS, and watchOS and
-can be used for C, C++, Objective-C and Swift development for x86_64,
-i386, ARM, and AArch64 debugging. The entire public API is exposed
-through a macOS framework which is used by Xcode and the `lldb`
-command line tool. It can also be imported from Python. The entire public API is
-exposed through script bridging which allows LLDB to use an embedded Python
-script interpreter, as well as having a Python module named "lldb" which can be
-used from Python on the command line. This allows debug sessions to be
-scripted. It also allows powerful debugging actions to be created and attached
-to a variety of debugging workflows.
-
-NetBSD
---
-
-LLDB is improving on NetBSD and reaching feature completeness with Linux.
-
-Windows

-
-LLDB on Windows is still under development, but already useful for i386
-programs (x86_64 untested) built with DWARF debug information, including
-postmortem analysis of minidumps. For more details, see the Features by OS
-section below.
-
-Features Matrix

-+---++-+---++--+
-| Feature   | FreeBSD| Linux   | macOS | NetBSD | Windows  |
-+===++=+===++==+
-| Backtracing   | YES| YES | YES   | YES| YES  |
-+---++-+---++--+
-| Breakpoints   | YES| YES | YES   | YES| YES  |
-+---++-+---++--+
-| C++11:| YES| YES | YES   | YES| Unknown  |
-+---++-+---++--+
-| Commandline tool  | YES| YES | YES   | YES| YES  |
-+---++-+---++--+
-| Core file debugging   | YES (ELF)  | YES (ELF)   | YES (MachO)   | YES (ELF)  | YES (Minidump)   |
-+---++-+---++--+
-| Remote debugging  | YES (lldb-server)  | YES (lldb-server)   | YES (debugserver) | YES (lldb-server)  | NO   |
-+---++-+---++--+
-| Disassembly   | YES| YES | YES   | YES| YES  |
-+---++-+---++--+
-| Expression evaluation | YES (known issues) | YES (known

[Lldb-commits] [PATCH] D158041: [lldb] [gdb-remote] Also handle high/low memory addressable bits setting in the stop info packet

2023-08-16 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

Thanks, this LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158041

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

[Lldb-commits] [PATCH] D158041: [lldb] [gdb-remote] Also handle high/low memory addressable bits setting in the stop info packet

2023-08-16 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

In D158041#4593205 , @jasonmolenda 
wrote:

> Update patch to change the `AddressableBits::IsValid` to 
> `AddressableBits::HasValue` which is a clearer description.  The `IsValid` 
> methods are used across the lldb_private classes everywhere.  I tried 
> changing this to `operator bool` but I thought that was less clear in use.  I 
> compromised by changing it to `HasValue` - this is a POD class so "IsValid" 
> seemed a little questionable, but I didn't like operator bool.  Let's try 
> this out.

Yeah that's fine. I don't feel strongly about `operator bool()`. My main 
concern is to not create another class that can be in an "invalid state" and 
requires you to check whether or not it's valid before doing an operation. As 
you point out, that's not really the case here anyway, and using different 
terminology helps convey that.

Comment at: 
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:2314-2316
+if (addressable_bits.HasValue()) {
+  addressable_bits.SetProcessMasks(*this);
+}

I would inline the check for `HasValue` in `SetProcessMasks`. You already do 
some kind of sanity checking there, might as well make that sound and do it 
unconditionally. 

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158041

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

[Lldb-commits] [PATCH] D158041: [lldb] [gdb-remote] Also handle high/low memory addressable bits setting in the stop info packet

2023-08-16 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added inline comments.



Comment at: lldb/source/Utility/AddressableBits.cpp:62-64
+void AddressableBits::Clear() {
+  m_low_memory_addr_bits = m_high_memory_addr_bits = 0;
+}

jasonmolenda wrote:
> jasonmolenda wrote:
> > JDevlieghere wrote:
> > > Where is this called?
> > Hm.  When I thought of this patch last night, I was sure I needed a Clear 
> > method for some reason, but you're right I never ended up using it.
> AHA I did use it, but it's in the previous patch.  It's in the base class 
> method of ObjectFile.
> 
> ```
>   virtual bool GetAddressableBits(lldb_private::AddressableBits 
> _bits) {
> address_bits.Clear();
> return false;
>   } 
> ```
> 
Why is this returning a bool and not `AddressableBits`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158041

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

[Lldb-commits] [PATCH] D158041: [lldb] [gdb-remote] Also handle high/low memory addressable bits setting in the stop info packet

2023-08-16 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added inline comments.



Comment at: lldb/source/Utility/AddressableBits.cpp:58-60
+bool AddressableBits::IsValid() {
+  return m_low_memory_addr_bits != 0 || m_high_memory_addr_bits != 0;
+}

Please implement `operator bool()` instead. 



Comment at: lldb/source/Utility/AddressableBits.cpp:62-64
+void AddressableBits::Clear() {
+  m_low_memory_addr_bits = m_high_memory_addr_bits = 0;
+}

Where is this called?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158041

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

[Lldb-commits] [PATCH] D157851: [lldb/crashlog] Add support for Last Exception Backtrace

2023-08-16 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

LGTM with Alex' comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157851

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

[Lldb-commits] [PATCH] D158023: [lldb] Simplify the LLDB website structure

2023-08-16 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere updated this revision to Diff 550791.
JDevlieghere added a comment.

- Rename "Extending LLDB" to "Scripting LLDB".
- Move "DWARF Extensions" to the developer section.


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

https://reviews.llvm.org/D158023

Files:
  lldb/docs/.htaccess
  lldb/docs/index.rst
  lldb/docs/resources/extensions.rst
  lldb/docs/resources/fuzzing.rst
  lldb/docs/status/features.rst
  lldb/docs/status/goals.rst
  lldb/docs/status/releases.rst
  lldb/docs/status/status.rst
  lldb/docs/use/extensions.rst

Index: lldb/docs/status/status.rst
===
--- lldb/docs/status/status.rst
+++ lldb/docs/status/status.rst
@@ -1,68 +0,0 @@
-Status
-==
-
-FreeBSD

-
-LLDB on FreeBSD lags behind the Linux implementation but is improving rapidly.
-For more details, see the Features by OS section below.
-
-Linux
--
-
-LLDB is improving on Linux. Linux is nearing feature completeness with Darwin
-to debug x86_64, i386, ARM, AArch64, IBM POWER (ppc64), and IBM Z (s390x)
-programs. For more details, see the Features by OS section below.
-
-macOS
--
-
-LLDB is the system debugger on macOS, iOS, tvOS, and watchOS and
-can be used for C, C++, Objective-C and Swift development for x86_64,
-i386, ARM, and AArch64 debugging. The entire public API is exposed
-through a macOS framework which is used by Xcode and the `lldb`
-command line tool. It can also be imported from Python. The entire public API is
-exposed through script bridging which allows LLDB to use an embedded Python
-script interpreter, as well as having a Python module named "lldb" which can be
-used from Python on the command line. This allows debug sessions to be
-scripted. It also allows powerful debugging actions to be created and attached
-to a variety of debugging workflows.
-
-NetBSD
---
-
-LLDB is improving on NetBSD and reaching feature completeness with Linux.
-
-Windows

-
-LLDB on Windows is still under development, but already useful for i386
-programs (x86_64 untested) built with DWARF debug information, including
-postmortem analysis of minidumps. For more details, see the Features by OS
-section below.
-
-Features Matrix

-+---++-+---++--+
-| Feature   | FreeBSD| Linux   | macOS | NetBSD | Windows  |
-+===++=+===++==+
-| Backtracing   | YES| YES | YES   | YES| YES  |
-+---++-+---++--+
-| Breakpoints   | YES| YES | YES   | YES| YES  |
-+---++-+---++--+
-| C++11:| YES| YES | YES   | YES| Unknown  |
-+---++-+---++--+
-| Commandline tool  | YES| YES | YES   | YES| YES  |
-+---++-+---++--+
-| Core file debugging   | YES (ELF)  | YES (ELF)   | YES (MachO)   | YES (ELF)  | YES (Minidump)   |
-+---++-+---++--+
-| Remote debugging  | YES (lldb-server)  | YES (lldb-server)   | YES (debugserver) | YES (lldb-server)  | NO   |
-+---++-+---++--+
-| Disassembly   | YES| YES | YES   | YES| YES  |
-+---++-+---++--+
-| Expression evaluation | YES (known issues) | YES (known issues)  | YES   | YES (known issues) | YES (known issues)   |
-+---++-+---++--+
-| JIT debugging | Unknown| Symbolic debugging only | Untested  | Work In

[Lldb-commits] [PATCH] D158022: [lldb] Print an actionable error message when sphinx_automodapi is not installed

2023-08-16 Thread Jonas Devlieghere via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5afa519c1ae9: [lldb] Print better error message when 
sphinx_automodapi is not installed (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158022

Files:
  lldb/docs/conf.py


Index: lldb/docs/conf.py
===
--- lldb/docs/conf.py
+++ lldb/docs/conf.py
@@ -49,6 +49,12 @@
 # Unless we only generate the basic manpage we need the plugin for generating
 # the Python API documentation.
 if not building_man_page:
+try:
+import sphinx_automodapi.automodapi
+except ModuleNotFoundError:
+print(
+f"install sphinx_automodapi with {sys.executable} -m pip install 
sphinx_automodapi"
+)
 extensions.append("sphinx_automodapi.automodapi")
 
 # Add any paths that contain templates here, relative to this directory.


Index: lldb/docs/conf.py
===
--- lldb/docs/conf.py
+++ lldb/docs/conf.py
@@ -49,6 +49,12 @@
 # Unless we only generate the basic manpage we need the plugin for generating
 # the Python API documentation.
 if not building_man_page:
+try:
+import sphinx_automodapi.automodapi
+except ModuleNotFoundError:
+print(
+f"install sphinx_automodapi with {sys.executable} -m pip install sphinx_automodapi"
+)
 extensions.append("sphinx_automodapi.automodapi")
 
 # Add any paths that contain templates here, relative to this directory.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D158085: [LLDB][Docs] Update cross compilation docs and examples

2023-08-16 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158085

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

[Lldb-commits] [PATCH] D158022: [lldb] Print an actionable error message when sphinx_automodapi is not installed

2023-08-15 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

In D158022#4590291 , @mib wrote:

> We should really add a `requirements.txt` file with all the dependencies.

I've long wanted to make this all automated, and have CMake install all the 
necessary dependencies. But that seems a bit intrusive without using something 
like a virtual environment, which would have to be project-wide and more work 
than I'm willing to put in. Regardless, that wouldn't actually help here, at 
least not on macOS if you installed sphinx with homebrew, because it packages 
its own version of Python 
(`/opt/homebrew/Cellar/sphinx-doc/7.1.2/libexec/bin/python`) and you have to 
install the `sphinx_automodapi` module with that. That's why I print 
`sys.executable` and really the motivation behind this patch.


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

https://reviews.llvm.org/D158022

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

[Lldb-commits] [PATCH] D158035: [lldb] Protect RNBRemote from a data race

2023-08-15 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added inline comments.



Comment at: lldb/tools/debugserver/source/RNBRemote.cpp:780
 
   PThreadMutex::Locker locker(m_mutex);
   if (m_rx_packets.empty()) {

This is an RAII object, right? Can we just block scope it? Right now it looks 
like we might not unlock if we return early on line 787. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158035

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

[Lldb-commits] [PATCH] D158034: [lldb] Fix data race in ThreadList

2023-08-15 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158034

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

[Lldb-commits] [PATCH] D158017: [lldb][NFCI] Rewrite error-handling code in ProcessGDBRemote::MonitorDebugserverProcess

2023-08-15 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

LGTM




Comment at: 
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:3379-3385
+  const char *signal_name =
   process_sp->GetUnixSignals()->GetSignalAsCString(signo);
-  if (signal_cstr)
-::snprintf(error_str, sizeof(error_str),
-   DEBUGSERVER_BASENAME " died with signal %s", signal_cstr);
+  const char *format_str = DEBUGSERVER_BASENAME " died with signal {0}";
+  if (signal_name)
+stream.Format(format_str, signal_name);
   else
+stream.Format(format_str, signo);




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158017

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

[Lldb-commits] [PATCH] D158023: [lldb] Simplify the LLDB website structure

2023-08-15 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere created this revision.
JDevlieghere added a reviewer: LLDB.
Herald added a subscriber: arphaman.
Herald added a project: All.
JDevlieghere requested review of this revision.

Feedback I hear regularly is that the LLDB website is hard to navigate. This 
patch is an attempt to simplify things by breaking the website up in 3 major 
areas: using LLDB, extending LLDB and developing LLDB.

Concretely:

- The majority of the "project" pages were eliminated, with the exception of 
the projects page, which was moved under "Developing LLDB". The goals, features 
and status page were pretty outdated and while they probably made sense in the 
past, they don't feel all that relevant anymore now that LLDB is an established 
debugger. The releases page was replaced with a link under "External links".
- "USE & EXTENSION" was renamed to "Using LLDB". Besides that this section 
remained mostly unchanged, with the exception of the Python pages which were 
moved under "Extending LLDB".
- "Development" was renamed to "Developing LLDB" and now houses all the 
resources for LLDB developers. The old "Design" section (which only contained 
two pages) was moved back under here too.


https://reviews.llvm.org/D158023

Files:
  lldb/docs/.htaccess
  lldb/docs/design/overview.rst
  lldb/docs/design/sbapi.rst
  lldb/docs/index.rst
  lldb/docs/resources/fuzzing.rst
  lldb/docs/resources/overview.rst
  lldb/docs/resources/projects.rst
  lldb/docs/resources/sbapi.rst
  lldb/docs/status/features.rst
  lldb/docs/status/goals.rst
  lldb/docs/status/projects.rst
  lldb/docs/status/releases.rst
  lldb/docs/status/status.rst

Index: lldb/docs/status/status.rst
===
--- lldb/docs/status/status.rst
+++ /dev/null
@@ -1,68 +0,0 @@
-Status
-==
-
-FreeBSD

-
-LLDB on FreeBSD lags behind the Linux implementation but is improving rapidly.
-For more details, see the Features by OS section below.
-
-Linux
--
-
-LLDB is improving on Linux. Linux is nearing feature completeness with Darwin
-to debug x86_64, i386, ARM, AArch64, IBM POWER (ppc64), and IBM Z (s390x)
-programs. For more details, see the Features by OS section below.
-
-macOS
--
-
-LLDB is the system debugger on macOS, iOS, tvOS, and watchOS and
-can be used for C, C++, Objective-C and Swift development for x86_64,
-i386, ARM, and AArch64 debugging. The entire public API is exposed
-through a macOS framework which is used by Xcode and the `lldb`
-command line tool. It can also be imported from Python. The entire public API is
-exposed through script bridging which allows LLDB to use an embedded Python
-script interpreter, as well as having a Python module named "lldb" which can be
-used from Python on the command line. This allows debug sessions to be
-scripted. It also allows powerful debugging actions to be created and attached
-to a variety of debugging workflows.
-
-NetBSD
---
-
-LLDB is improving on NetBSD and reaching feature completeness with Linux.
-
-Windows

-
-LLDB on Windows is still under development, but already useful for i386
-programs (x86_64 untested) built with DWARF debug information, including
-postmortem analysis of minidumps. For more details, see the Features by OS
-section below.
-
-Features Matrix

-+---++-+---++--+
-| Feature   | FreeBSD| Linux   | macOS | NetBSD | Windows  |
-+===++=+===++==+
-| Backtracing   | YES| YES | YES   | YES| YES  |
-+---++-+---++--+
-| Breakpoints   | YES| YES | YES   | YES| YES  |
-+---++-+---++--+
-| C++11:| YES| YES | YES   | YES| Unknown  |
-+---++-+---++--+
-| Commandline tool  | YES| YES | YES   | YES| YES  |
-+---++-+---++--+
-| Core file debugging   | YES (ELF)  | YES (ELF)   | YES (MachO)   | YES (ELF)  | YES (Minidump)   |

[Lldb-commits] [PATCH] D158022: [lldb] Print an actionable error message when sphinx_automodapi is not installed

2023-08-15 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere created this revision.
JDevlieghere added a reviewer: LLDB.
Herald added a project: All.
JDevlieghere requested review of this revision.

Print an error message with instructions on how to install sphinx_automodapi.


https://reviews.llvm.org/D158022

Files:
  lldb/docs/conf.py


Index: lldb/docs/conf.py
===
--- lldb/docs/conf.py
+++ lldb/docs/conf.py
@@ -49,6 +49,12 @@
 # Unless we only generate the basic manpage we need the plugin for generating
 # the Python API documentation.
 if not building_man_page:
+try:
+import sphinx_automodapi.automodapi
+except ModuleNotFoundError:
+print(
+f"install sphinx_automodapi with {sys.executable} -m pip install 
sphinx_automodapi"
+)
 extensions.append("sphinx_automodapi.automodapi")
 
 # Add any paths that contain templates here, relative to this directory.


Index: lldb/docs/conf.py
===
--- lldb/docs/conf.py
+++ lldb/docs/conf.py
@@ -49,6 +49,12 @@
 # Unless we only generate the basic manpage we need the plugin for generating
 # the Python API documentation.
 if not building_man_page:
+try:
+import sphinx_automodapi.automodapi
+except ModuleNotFoundError:
+print(
+f"install sphinx_automodapi with {sys.executable} -m pip install sphinx_automodapi"
+)
 extensions.append("sphinx_automodapi.automodapi")
 
 # Add any paths that contain templates here, relative to this directory.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D157852: [lldb/crashlog] Skip non-crashed threads in batch mode

2023-08-15 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157852

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

[Lldb-commits] [PATCH] D157850: [lldb/crashlog] Fix module loading for crashed thread behaviour

2023-08-15 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157850

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

[Lldb-commits] [PATCH] D158000: [lldb] Remove {Get, Set}CloseInputOnEOF and deprecate SB equivalent (NFC)

2023-08-15 Thread Jonas Devlieghere via Phabricator via lldb-commits

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



Comment at: lldb/source/API/SBDebugger.cpp:1538-1545
+LLDB_DEPRECATED("SBDebugger::GetCloseInputOnEOF() is deprecated.")
 bool SBDebugger::GetCloseInputOnEOF() const {
   LLDB_INSTRUMENT_VA(this);
 
-  return (m_opaque_sp ? m_opaque_sp->GetCloseInputOnEOF() : false);
+  return false;
 }
 

bulbazord wrote:
> You probably want these annotations on the header file so that projects that 
> compile against LLDB get the deprecation warnings.
Good point. Fixed in f2ec73c74650. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158000

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

[Lldb-commits] [PATCH] D158000: [lldb] Remove {Get, Set}CloseInputOnEOF and deprecate SB equivalent (NFC)

2023-08-15 Thread Jonas Devlieghere via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG23312cde45b9: [lldb] Remove {Get,Set}CloseInputOnEOF and 
deprecate SB equivalent (NFC) (authored by JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D158000?vs=550383=550388#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158000

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/source/API/SBDebugger.cpp
  lldb/source/Core/Debugger.cpp


Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -929,15 +929,6 @@
   });
 }
 
-bool Debugger::GetCloseInputOnEOF() const {
-  //return m_input_comm.GetCloseOnEOF();
-  return false;
-}
-
-void Debugger::SetCloseInputOnEOF(bool b) {
-  //m_input_comm.SetCloseOnEOF(b);
-}
-
 bool Debugger::GetAsyncExecution() {
   return !m_command_interpreter_up->GetSynchronous();
 }
Index: lldb/source/API/SBDebugger.cpp
===
--- lldb/source/API/SBDebugger.cpp
+++ lldb/source/API/SBDebugger.cpp
@@ -1535,17 +1535,16 @@
   return false;
 }
 
+LLDB_DEPRECATED("SBDebugger::GetCloseInputOnEOF() is deprecated.")
 bool SBDebugger::GetCloseInputOnEOF() const {
   LLDB_INSTRUMENT_VA(this);
 
-  return (m_opaque_sp ? m_opaque_sp->GetCloseInputOnEOF() : false);
+  return false;
 }
 
+LLDB_DEPRECATED("SBDebugger::SetCloseInputOnEOF() is deprecated.")
 void SBDebugger::SetCloseInputOnEOF(bool b) {
   LLDB_INSTRUMENT_VA(this, b);
-
-  if (m_opaque_sp)
-m_opaque_sp->SetCloseInputOnEOF(b);
 }
 
 SBTypeCategory SBDebugger::GetCategory(const char *category_name) {
Index: lldb/include/lldb/Core/Debugger.h
===
--- lldb/include/lldb/Core/Debugger.h
+++ lldb/include/lldb/Core/Debugger.h
@@ -241,10 +241,6 @@
 
   void ClearIOHandlers();
 
-  bool GetCloseInputOnEOF() const;
-
-  void SetCloseInputOnEOF(bool b);
-
   bool EnableLog(llvm::StringRef channel,
  llvm::ArrayRef categories,
  llvm::StringRef log_file, uint32_t log_options,


Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -929,15 +929,6 @@
   });
 }
 
-bool Debugger::GetCloseInputOnEOF() const {
-  //return m_input_comm.GetCloseOnEOF();
-  return false;
-}
-
-void Debugger::SetCloseInputOnEOF(bool b) {
-  //m_input_comm.SetCloseOnEOF(b);
-}
-
 bool Debugger::GetAsyncExecution() {
   return !m_command_interpreter_up->GetSynchronous();
 }
Index: lldb/source/API/SBDebugger.cpp
===
--- lldb/source/API/SBDebugger.cpp
+++ lldb/source/API/SBDebugger.cpp
@@ -1535,17 +1535,16 @@
   return false;
 }
 
+LLDB_DEPRECATED("SBDebugger::GetCloseInputOnEOF() is deprecated.")
 bool SBDebugger::GetCloseInputOnEOF() const {
   LLDB_INSTRUMENT_VA(this);
 
-  return (m_opaque_sp ? m_opaque_sp->GetCloseInputOnEOF() : false);
+  return false;
 }
 
+LLDB_DEPRECATED("SBDebugger::SetCloseInputOnEOF() is deprecated.")
 void SBDebugger::SetCloseInputOnEOF(bool b) {
   LLDB_INSTRUMENT_VA(this, b);
-
-  if (m_opaque_sp)
-m_opaque_sp->SetCloseInputOnEOF(b);
 }
 
 SBTypeCategory SBDebugger::GetCategory(const char *category_name) {
Index: lldb/include/lldb/Core/Debugger.h
===
--- lldb/include/lldb/Core/Debugger.h
+++ lldb/include/lldb/Core/Debugger.h
@@ -241,10 +241,6 @@
 
   void ClearIOHandlers();
 
-  bool GetCloseInputOnEOF() const;
-
-  void SetCloseInputOnEOF(bool b);
-
   bool EnableLog(llvm::StringRef channel,
  llvm::ArrayRef categories,
  llvm::StringRef log_file, uint32_t log_options,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D158000: [lldb] Remove {Get, Set}CloseInputOnEOF and deprecate SB equivalent (NFC)

2023-08-15 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere created this revision.
JDevlieghere added reviewers: bulbazord, clayborg.
Herald added a project: All.
JDevlieghere requested review of this revision.

These functions have been NO-OPs since 2014 (44d937820b451). Remove them
and deprecate the corresponding functions in SBDebugger.


https://reviews.llvm.org/D158000

Files:
  lldb/include/lldb/Core/Debugger.h
  lldb/source/API/SBDebugger.cpp
  lldb/source/Core/Debugger.cpp


Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -929,15 +929,6 @@
   });
 }
 
-bool Debugger::GetCloseInputOnEOF() const {
-  //return m_input_comm.GetCloseOnEOF();
-  return false;
-}
-
-void Debugger::SetCloseInputOnEOF(bool b) {
-  //m_input_comm.SetCloseOnEOF(b);
-}
-
 bool Debugger::GetAsyncExecution() {
   return !m_command_interpreter_up->GetSynchronous();
 }
Index: lldb/source/API/SBDebugger.cpp
===
--- lldb/source/API/SBDebugger.cpp
+++ lldb/source/API/SBDebugger.cpp
@@ -1535,17 +1535,15 @@
   return false;
 }
 
+LLDB_DEPRECATED("SBDebugger::GetCloseInputOnEOF() is deprecated.")
 bool SBDebugger::GetCloseInputOnEOF() const {
   LLDB_INSTRUMENT_VA(this);
-
-  return (m_opaque_sp ? m_opaque_sp->GetCloseInputOnEOF() : false);
+  return false;
 }
 
+LLDB_DEPRECATED("SBDebugger::SetCloseInputOnEOF() is deprecated.")
 void SBDebugger::SetCloseInputOnEOF(bool b) {
   LLDB_INSTRUMENT_VA(this, b);
-
-  if (m_opaque_sp)
-m_opaque_sp->SetCloseInputOnEOF(b);
 }
 
 SBTypeCategory SBDebugger::GetCategory(const char *category_name) {
Index: lldb/include/lldb/Core/Debugger.h
===
--- lldb/include/lldb/Core/Debugger.h
+++ lldb/include/lldb/Core/Debugger.h
@@ -241,10 +241,6 @@
 
   void ClearIOHandlers();
 
-  bool GetCloseInputOnEOF() const;
-
-  void SetCloseInputOnEOF(bool b);
-
   bool EnableLog(llvm::StringRef channel,
  llvm::ArrayRef categories,
  llvm::StringRef log_file, uint32_t log_options,


Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -929,15 +929,6 @@
   });
 }
 
-bool Debugger::GetCloseInputOnEOF() const {
-  //return m_input_comm.GetCloseOnEOF();
-  return false;
-}
-
-void Debugger::SetCloseInputOnEOF(bool b) {
-  //m_input_comm.SetCloseOnEOF(b);
-}
-
 bool Debugger::GetAsyncExecution() {
   return !m_command_interpreter_up->GetSynchronous();
 }
Index: lldb/source/API/SBDebugger.cpp
===
--- lldb/source/API/SBDebugger.cpp
+++ lldb/source/API/SBDebugger.cpp
@@ -1535,17 +1535,15 @@
   return false;
 }
 
+LLDB_DEPRECATED("SBDebugger::GetCloseInputOnEOF() is deprecated.")
 bool SBDebugger::GetCloseInputOnEOF() const {
   LLDB_INSTRUMENT_VA(this);
-
-  return (m_opaque_sp ? m_opaque_sp->GetCloseInputOnEOF() : false);
+  return false;
 }
 
+LLDB_DEPRECATED("SBDebugger::SetCloseInputOnEOF() is deprecated.")
 void SBDebugger::SetCloseInputOnEOF(bool b) {
   LLDB_INSTRUMENT_VA(this, b);
-
-  if (m_opaque_sp)
-m_opaque_sp->SetCloseInputOnEOF(b);
 }
 
 SBTypeCategory SBDebugger::GetCategory(const char *category_name) {
Index: lldb/include/lldb/Core/Debugger.h
===
--- lldb/include/lldb/Core/Debugger.h
+++ lldb/include/lldb/Core/Debugger.h
@@ -241,10 +241,6 @@
 
   void ClearIOHandlers();
 
-  bool GetCloseInputOnEOF() const;
-
-  void SetCloseInputOnEOF(bool b);
-
   bool EnableLog(llvm::StringRef channel,
  llvm::ArrayRef categories,
  llvm::StringRef log_file, uint32_t log_options,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D157759: [lldb] Remove use of future in python

2023-08-14 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere accepted this revision.
JDevlieghere added a comment.

In D157759#4581673 , @mib wrote:

> LGTM! Although we should make sure no one is still using Python 2 downstream

Python 2 has been deprecated for a while and we've removed other stuff that 
makes it no longer possible to use Python 2.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157759

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

[Lldb-commits] [PATCH] D157760: [lldb] Properly protect the Communication class with reader/writer lock

2023-08-14 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added inline comments.



Comment at: lldb/include/lldb/Core/Communication.h:172-174
   std::mutex
   m_write_mutex; ///< Don't let multiple threads write at the same time...
+  mutable std::shared_mutex m_connection_mutex;

Why do we need both? Can't we use `m_connection_mutex` in write mode instead of 
`m_write_mutex`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157760

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

[Lldb-commits] [PATCH] D157667: Define qHostInfo and Mach-O LC_NOTE "addrable bits" methods to describe high and low memory addressing bits

2023-08-14 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added inline comments.



Comment at: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h:125-126
 
-  lldb::addr_t GetAddressMask() override;
+  bool GetAddressMask(lldb::addr_t _mask,
+  lldb::addr_t _mask) override;
 

I would prefer this to return a struct with the two masks, possibly living in 
Utility. The struct would centralize default values, which values are 
meaningful, etc and could be stored as a member in 
`GDBRemoteCommunicationClient`. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157667

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

[Lldb-commits] [PATCH] D157043: [lldb/crashlog] Make TextCrashLogParser more resilient to new lines

2023-08-12 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

Test?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157043

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

[Lldb-commits] [PATCH] D157137: [lldb/crashlog] Skip images with empty path and 0 UUID from loading

2023-08-12 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

Test?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157137

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

[Lldb-commits] [PATCH] D157764: [LLDB] Allow expression evaluators to set arbitrary timeouts

2023-08-12 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere accepted this revision.
JDevlieghere added a comment.

The motivation behind this change was that the IRInterpreter wasn't 
interruptible, which means that if you interpret an infinite loop, you'd be 
stuck forever. I fixed that in 61af957aeaa3 
 so I 
think that's less of a concern today. I'm fine with going back to allowing an 
infinite timeout as long as there's no code path in LLDB that sets it to zero 
by default.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157764

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

[Lldb-commits] [PATCH] D157654: [lldb] Fix data race in PipePosix

2023-08-12 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157654

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

[Lldb-commits] [PATCH] D157654: [lldb] Fix data race in PipePosix

2023-08-11 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

In D157654#4581578 , @augusto2112 
wrote:

> @JDevlieghere I updated this to two shared mutexes because I'm assuming you 
> can have more than one concurrent read and more than one concurrent write. If 
> this is wrong I can go back to two regular mutexes.

According to POSIX [1] that would be undefined:

> The behavior of multiple concurrent reads on the same pipe, FIFO, or terminal 
> device is unspecified.

A regular mutex should be fine.

[1] https://pubs.opengroup.org/onlinepubs/9699919799/functions/read.html


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157654

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

[Lldb-commits] [PATCH] D157640: [lldb] Improve error message when trying to debug a non-debuggable process

2023-08-11 Thread Jonas Devlieghere via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbe237b76da7e: [lldb] Improve error message when trying to 
debug a non-debuggable process (authored by JDevlieghere).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D157640?vs=549119=549459#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157640

Files:
  lldb/tools/debugserver/source/DNB.cpp


Index: lldb/tools/debugserver/source/DNB.cpp
===
--- lldb/tools/debugserver/source/DNB.cpp
+++ lldb/tools/debugserver/source/DNB.cpp
@@ -382,11 +382,22 @@
 if (err_str && err_len > 0) {
   if (launch_err.AsString()) {
 ::snprintf(err_str, err_len,
-   "failed to get the task for process %i (%s)", pid,
+   "failed to get the task for process %i: %s", pid,
launch_err.AsString());
   } else {
+
+const char *ent_name =
+#if TARGET_OS_OSX
+  "com.apple.security.get-task-allow";
+#else
+  "get-task-allow";
+#endif
 ::snprintf(err_str, err_len,
-   "failed to get the task for process %i", pid);
+   "failed to get the task for process %i: this likely "
+   "means the process cannot be debugged, either because "
+   "it's a system process or because the process is "
+   "missing the %s entitlement.",
+   pid, ent_name);
   }
 }
   } else {


Index: lldb/tools/debugserver/source/DNB.cpp
===
--- lldb/tools/debugserver/source/DNB.cpp
+++ lldb/tools/debugserver/source/DNB.cpp
@@ -382,11 +382,22 @@
 if (err_str && err_len > 0) {
   if (launch_err.AsString()) {
 ::snprintf(err_str, err_len,
-   "failed to get the task for process %i (%s)", pid,
+   "failed to get the task for process %i: %s", pid,
launch_err.AsString());
   } else {
+
+const char *ent_name =
+#if TARGET_OS_OSX
+  "com.apple.security.get-task-allow";
+#else
+  "get-task-allow";
+#endif
 ::snprintf(err_str, err_len,
-   "failed to get the task for process %i", pid);
+   "failed to get the task for process %i: this likely "
+   "means the process cannot be debugged, either because "
+   "it's a system process or because the process is "
+   "missing the %s entitlement.",
+   pid, ent_name);
   }
 }
   } else {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D157654: [lldb] Fix data race in PipePosix

2023-08-11 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

Shouldn't we have two murexes, one for the read FD and one for the write FD? 
The current approach is overly strict: it preventing concurrent reads and 
writes because `ReadWithTimeout` and `Write` are locking the same mutex.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157654

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

[Lldb-commits] [PATCH] D157640: [lldb] Improve error message when trying to debug a non-debuggable process

2023-08-11 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

In D157640#4578198 , @jingham wrote:

> In this function we have the path to the binary.  We could spawn `codesign -d 
> -entitlements -` and then we would know whether it had that entitlement.
>
> Maybe that's more work than you wanted to do here, however.

Yeah I'm not sure that being able to be more precise is a huge improvement over 
the current error message. If we still see a lot of people struggling with this 
we can easily reconsider.

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

https://reviews.llvm.org/D157640

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

[Lldb-commits] [PATCH] D157648: [lldb] Fix data race in Process

2023-08-10 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

Is the reported race specifically about the shared pointer being accessed 
concurrently or operations on the `IOHandler`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157648

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

[Lldb-commits] [PATCH] D157636: [lldb][test] Remove tests relying on deprecated std::char_traits specializations

2023-08-10 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

If this was important we could put this behind a define, break up the test and 
conditionally skip it based on the libc++ version. But I personally don't think 
that's worth it for this. LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157636

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

[Lldb-commits] [PATCH] D157640: [lldb] Improve error message when trying to debug a non-debuggable process

2023-08-10 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere created this revision.
JDevlieghere added reviewers: jasonmolenda, aprantl.
Herald added a project: All.
JDevlieghere requested review of this revision.

On the Swift forums [1][2], people are disabling SIP in order to debug process 
that are missing the get-task-allow entitlement. Improve the error to give 
developers a hint at the potential issues.

[1] 
https://forums.swift.org/t/yo-apple-xcode-debugging-swift-is-still-horribly-broken/62702/26
[2] 
https://forums.swift.org/t/finding-cycles-with-the-memory-graph-debugger-with-swift-pm-projects/62769/7

rdar://113704200


https://reviews.llvm.org/D157640

Files:
  lldb/tools/debugserver/source/DNB.cpp


Index: lldb/tools/debugserver/source/DNB.cpp
===
--- lldb/tools/debugserver/source/DNB.cpp
+++ lldb/tools/debugserver/source/DNB.cpp
@@ -382,11 +382,15 @@
 if (err_str && err_len > 0) {
   if (launch_err.AsString()) {
 ::snprintf(err_str, err_len,
-   "failed to get the task for process %i (%s)", pid,
+   "failed to get the task for process %i: %s", pid,
launch_err.AsString());
   } else {
 ::snprintf(err_str, err_len,
-   "failed to get the task for process %i", pid);
+   "failed to get the task for process %i: this likely "
+   "means the process cannot be debugged, either because "
+   "it's a system process or because the process is "
+   "missing the get-task-allow entitlement.",
+   pid);
   }
 }
   } else {


Index: lldb/tools/debugserver/source/DNB.cpp
===
--- lldb/tools/debugserver/source/DNB.cpp
+++ lldb/tools/debugserver/source/DNB.cpp
@@ -382,11 +382,15 @@
 if (err_str && err_len > 0) {
   if (launch_err.AsString()) {
 ::snprintf(err_str, err_len,
-   "failed to get the task for process %i (%s)", pid,
+   "failed to get the task for process %i: %s", pid,
launch_err.AsString());
   } else {
 ::snprintf(err_str, err_len,
-   "failed to get the task for process %i", pid);
+   "failed to get the task for process %i: this likely "
+   "means the process cannot be debugged, either because "
+   "it's a system process or because the process is "
+   "missing the get-task-allow entitlement.",
+   pid);
   }
 }
   } else {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D157347: [lldb] Fix data race in NativeFile

2023-08-10 Thread Jonas Devlieghere via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9c135f1478cd: [lldb] Fix data race in NativeFile (authored 
by JDevlieghere).

Changed prior to commit:
  https://reviews.llvm.org/D157347?vs=548849=549116#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157347

Files:
  lldb/include/lldb/Host/File.h
  lldb/source/Host/common/File.cpp

Index: lldb/source/Host/common/File.cpp
===
--- lldb/source/Host/common/File.cpp
+++ lldb/source/Host/common/File.cpp
@@ -219,7 +219,7 @@
 size_t File::PrintfVarArg(const char *format, va_list args) {
   llvm::SmallString<0> s;
   if (VASprintf(s, format, args)) {
-size_t written = s.size();;
+size_t written = s.size();
 Write(s.data(), written);
 return written;
   }
@@ -247,15 +247,21 @@
   return file_stats.st_mode & (S_IRWXU | S_IRWXG | S_IRWXO);
 }
 
+bool NativeFile::IsValid() const {
+  std::scoped_lock lock(m_descriptor_mutex, m_stream_mutex);
+  return DescriptorIsValidUnlocked() || StreamIsValidUnlocked();
+}
+
 Expected NativeFile::GetOptions() const { return m_options; }
 
 int NativeFile::GetDescriptor() const {
-  if (DescriptorIsValid())
+  if (ValueGuard descriptor_guard = DescriptorIsValid()) {
 return m_descriptor;
+  }
 
   // Don't open the file descriptor if we don't need to, just get it from the
   // stream if we have one.
-  if (StreamIsValid()) {
+  if (ValueGuard stream_guard = StreamIsValid()) {
 #if defined(_WIN32)
 return _fileno(m_stream);
 #else
@@ -272,8 +278,9 @@
 }
 
 FILE *NativeFile::GetStream() {
-  if (!StreamIsValid()) {
-if (DescriptorIsValid()) {
+  ValueGuard stream_guard = StreamIsValid();
+  if (!stream_guard) {
+if (ValueGuard descriptor_guard = DescriptorIsValid()) {
   auto mode = GetStreamOpenModeFromOptions(m_options);
   if (!mode)
 llvm::consumeError(mode.takeError());
@@ -282,9 +289,9 @@
 // We must duplicate the file descriptor if we don't own it because when you
 // call fdopen, the stream will own the fd
 #ifdef _WIN32
-  m_descriptor = ::_dup(GetDescriptor());
+  m_descriptor = ::_dup(m_descriptor);
 #else
-  m_descriptor = dup(GetDescriptor());
+  m_descriptor = dup(m_descriptor);
 #endif
   m_own_descriptor = true;
 }
@@ -306,8 +313,11 @@
 }
 
 Status NativeFile::Close() {
+  std::scoped_lock lock(m_descriptor_mutex, m_stream_mutex);
+
   Status error;
-  if (StreamIsValid()) {
+
+  if (StreamIsValidUnlocked()) {
 if (m_own_stream) {
   if (::fclose(m_stream) == EOF)
 error.SetErrorToErrno();
@@ -322,15 +332,17 @@
   }
 }
   }
-  if (DescriptorIsValid() && m_own_descriptor) {
+
+  if (DescriptorIsValidUnlocked() && m_own_descriptor) {
 if (::close(m_descriptor) != 0)
   error.SetErrorToErrno();
   }
-  m_descriptor = kInvalidDescriptor;
+
   m_stream = kInvalidStream;
-  m_options = OpenOptions(0);
   m_own_stream = false;
+  m_descriptor = kInvalidDescriptor;
   m_own_descriptor = false;
+  m_options = OpenOptions(0);
   m_is_interactive = eLazyBoolCalculate;
   m_is_real_terminal = eLazyBoolCalculate;
   return error;
@@ -374,7 +386,7 @@
 
 off_t NativeFile::SeekFromStart(off_t offset, Status *error_ptr) {
   off_t result = 0;
-  if (DescriptorIsValid()) {
+  if (ValueGuard descriptor_guard = DescriptorIsValid()) {
 result = ::lseek(m_descriptor, offset, SEEK_SET);
 
 if (error_ptr) {
@@ -383,7 +395,10 @@
   else
 error_ptr->Clear();
 }
-  } else if (StreamIsValid()) {
+return result;
+  }
+
+  if (ValueGuard stream_guard = StreamIsValid()) {
 result = ::fseek(m_stream, offset, SEEK_SET);
 
 if (error_ptr) {
@@ -392,15 +407,17 @@
   else
 error_ptr->Clear();
 }
-  } else if (error_ptr) {
-error_ptr->SetErrorString("invalid file handle");
+return result;
   }
+
+  if (error_ptr)
+error_ptr->SetErrorString("invalid file handle");
   return result;
 }
 
 off_t NativeFile::SeekFromCurrent(off_t offset, Status *error_ptr) {
   off_t result = -1;
-  if (DescriptorIsValid()) {
+  if (ValueGuard descriptor_guard = DescriptorIsValid()) {
 result = ::lseek(m_descriptor, offset, SEEK_CUR);
 
 if (error_ptr) {
@@ -409,7 +426,10 @@
   else
 error_ptr->Clear();
 }
-  } else if (StreamIsValid()) {
+return result;
+  }
+
+  if (ValueGuard stream_guard = StreamIsValid()) {
 result = ::fseek(m_stream, offset, SEEK_CUR);
 
 if (error_ptr) {
@@ -418,15 +438,17 @@
   else
 error_ptr->Clear();
 }
-  } else if (error_ptr) {
-error_ptr->SetErrorString("invalid file handle");
+return result;
   }
+
+  if (error_ptr)
+error_ptr->SetErrorString("invalid file handle");
   return result;
 }
 
 off_t NativeFile::SeekFromEnd(off_t offset, Status *error_ptr) {
   off_t result = -1;
-  if

[Lldb-commits] [PATCH] D157556: Replace the singleton "ShadowListener" with a primary and N secondary listeners

2023-08-10 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added inline comments.



Comment at: lldb/include/lldb/Utility/Event.h:228
   void Clear() { m_data_sp.reset(); }
+  
+  /// This is used by Broadcasters with Primary Listeners to store the other

Nit: trailing whitespace



Comment at: 
lldb/test/API/functionalities/interactive_scripted_process/TestInteractiveScriptedProcess.py:26
 @skipUnlessDarwin
-@skipIfDarwin
+#@skipIfDarwin
 def test_passthrough_launch(self):

bulbazord wrote:
> Remove this comment?
Remove?



Comment at: 
lldb/test/API/functionalities/interactive_scripted_process/TestInteractiveScriptedProcess.py:56
 @skipUnlessDarwin
-@skipIfDarwin
+#@skipIfDarwin
 def test_multiplexed_launch(self):

bulbazord wrote:
> delete?
Remove?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157556

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

[Lldb-commits] [PATCH] D157347: [lldb] Fix data race in NativeFile

2023-08-09 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added inline comments.



Comment at: lldb/source/Host/common/File.cpp:609
   num_bytes = bytes_written;
-  } else if (StreamIsValid()) {
+  } else if (ValueGuard stream_guard = StreamIsValid()) {
 bytes_written = ::fwrite(buf, 1, num_bytes, m_stream);

augusto2112 wrote:
> bulbazord wrote:
> > augusto2112 wrote:
> > > One thing that's not super obvious for people reading this is that 
> > > `descriptor_guard` is still in scope in the else branch (due to C++'s 
> > > weird scoping rules when it comes to declaring variables inside `if` 
> > > statements), and will keep the descriptor mutex locked which is probably 
> > > not what you intended. I don't think this affects the correctness of this 
> > > implementation at the moment, but could be dangerous with further changes 
> > > on top of this one.
> > If that's true, then this change needs further adjustment. `GetStream` 
> > acquires the `ValueGuard`s in the opposite order, meaning Thread 1 can call 
> > this function and acquire `descriptor_guard` while Thread 2 can call 
> > `GetStream` and acquire `stream_guard` at the same time. Then they're both 
> > waiting on the other lock (deadlock).
> You're right, nice catch.
Thanks, I didn't know about that and that was certainly not the intention. 


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

https://reviews.llvm.org/D157347

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

[Lldb-commits] [PATCH] D157347: [lldb] Fix data race in NativeFile

2023-08-09 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere updated this revision to Diff 548849.
JDevlieghere marked 4 inline comments as done.
JDevlieghere added a comment.

Limit scope of `ValueGuard` by avoiding `else if`.


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

https://reviews.llvm.org/D157347

Files:
  lldb/include/lldb/Host/File.h
  lldb/source/Host/common/File.cpp

Index: lldb/source/Host/common/File.cpp
===
--- lldb/source/Host/common/File.cpp
+++ lldb/source/Host/common/File.cpp
@@ -219,7 +219,7 @@
 size_t File::PrintfVarArg(const char *format, va_list args) {
   llvm::SmallString<0> s;
   if (VASprintf(s, format, args)) {
-size_t written = s.size();;
+size_t written = s.size();
 Write(s.data(), written);
 return written;
   }
@@ -247,15 +247,20 @@
   return file_stats.st_mode & (S_IRWXU | S_IRWXG | S_IRWXO);
 }
 
+bool NativeFile::IsValid() const {
+  return DescriptorIsValid() || StreamIsValid();
+}
+
 Expected NativeFile::GetOptions() const { return m_options; }
 
 int NativeFile::GetDescriptor() const {
-  if (DescriptorIsValid())
+  if (ValueGuard descriptor_guard = DescriptorIsValid()) {
 return m_descriptor;
+  }
 
   // Don't open the file descriptor if we don't need to, just get it from the
   // stream if we have one.
-  if (StreamIsValid()) {
+  if (ValueGuard stream_guard = StreamIsValid()) {
 #if defined(_WIN32)
 return _fileno(m_stream);
 #else
@@ -272,8 +277,9 @@
 }
 
 FILE *NativeFile::GetStream() {
-  if (!StreamIsValid()) {
-if (DescriptorIsValid()) {
+  ValueGuard stream_guard = StreamIsValid();
+  if (!stream_guard) {
+if (ValueGuard descriptor_guard = DescriptorIsValid()) {
   auto mode = GetStreamOpenModeFromOptions(m_options);
   if (!mode)
 llvm::consumeError(mode.takeError());
@@ -282,9 +288,9 @@
 // We must duplicate the file descriptor if we don't own it because when you
 // call fdopen, the stream will own the fd
 #ifdef _WIN32
-  m_descriptor = ::_dup(GetDescriptor());
+  m_descriptor = ::_dup(m_descriptor);
 #else
-  m_descriptor = dup(GetDescriptor());
+  m_descriptor = dup(m_descriptor);
 #endif
   m_own_descriptor = true;
 }
@@ -306,8 +312,11 @@
 }
 
 Status NativeFile::Close() {
+  std::scoped_lock lock(m_descriptor_mutex, m_stream_mutex);
+
   Status error;
-  if (StreamIsValid()) {
+
+  if (StreamIsValidUnlocked()) {
 if (m_own_stream) {
   if (::fclose(m_stream) == EOF)
 error.SetErrorToErrno();
@@ -322,15 +331,17 @@
   }
 }
   }
-  if (DescriptorIsValid() && m_own_descriptor) {
+
+  if (DescriptorIsValidUnlocked() && m_own_descriptor) {
 if (::close(m_descriptor) != 0)
   error.SetErrorToErrno();
   }
-  m_descriptor = kInvalidDescriptor;
+
   m_stream = kInvalidStream;
-  m_options = OpenOptions(0);
   m_own_stream = false;
+  m_descriptor = kInvalidDescriptor;
   m_own_descriptor = false;
+  m_options = OpenOptions(0);
   m_is_interactive = eLazyBoolCalculate;
   m_is_real_terminal = eLazyBoolCalculate;
   return error;
@@ -374,7 +385,7 @@
 
 off_t NativeFile::SeekFromStart(off_t offset, Status *error_ptr) {
   off_t result = 0;
-  if (DescriptorIsValid()) {
+  if (ValueGuard descriptor_guard = DescriptorIsValid()) {
 result = ::lseek(m_descriptor, offset, SEEK_SET);
 
 if (error_ptr) {
@@ -383,7 +394,10 @@
   else
 error_ptr->Clear();
 }
-  } else if (StreamIsValid()) {
+return result;
+  }
+
+  if (ValueGuard stream_guard = StreamIsValid()) {
 result = ::fseek(m_stream, offset, SEEK_SET);
 
 if (error_ptr) {
@@ -392,15 +406,17 @@
   else
 error_ptr->Clear();
 }
-  } else if (error_ptr) {
-error_ptr->SetErrorString("invalid file handle");
+return result;
   }
+
+  if (error_ptr)
+error_ptr->SetErrorString("invalid file handle");
   return result;
 }
 
 off_t NativeFile::SeekFromCurrent(off_t offset, Status *error_ptr) {
   off_t result = -1;
-  if (DescriptorIsValid()) {
+  if (ValueGuard descriptor_guard = DescriptorIsValid()) {
 result = ::lseek(m_descriptor, offset, SEEK_CUR);
 
 if (error_ptr) {
@@ -409,7 +425,10 @@
   else
 error_ptr->Clear();
 }
-  } else if (StreamIsValid()) {
+return result;
+  }
+
+  if (ValueGuard stream_guard = StreamIsValid()) {
 result = ::fseek(m_stream, offset, SEEK_CUR);
 
 if (error_ptr) {
@@ -418,15 +437,17 @@
   else
 error_ptr->Clear();
 }
-  } else if (error_ptr) {
-error_ptr->SetErrorString("invalid file handle");
+return result;
   }
+
+  if (error_ptr)
+error_ptr->SetErrorString("invalid file handle");
   return result;
 }
 
 off_t NativeFile::SeekFromEnd(off_t offset, Status *error_ptr) {
   off_t result = -1;
-  if (DescriptorIsValid()) {
+  if (ValueGuard descriptor_guard = DescriptorIsValid()) {
 result = ::lseek(m_descriptor, offset, SEEK_END);
 
 if (error_ptr) {
@@ -435,7 +456,10 @@
   else

[Lldb-commits] [PATCH] D157361: [lldb] FIx data race in ThreadedCommunication

2023-08-09 Thread Jonas Devlieghere via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1a8d9a7657bb: [lldb] Fix data race in ThreadedCommunication 
(authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157361

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


Index: lldb/source/Core/ThreadedCommunication.cpp
===
--- lldb/source/Core/ThreadedCommunication.cpp
+++ lldb/source/Core/ThreadedCommunication.cpp
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -155,6 +156,8 @@
 }
 
 bool ThreadedCommunication::StartReadThread(Status *error_ptr) {
+  std::lock_guard lock(m_read_thread_mutex);
+
   if (error_ptr)
 error_ptr->Clear();
 
@@ -189,6 +192,8 @@
 }
 
 bool ThreadedCommunication::StopReadThread(Status *error_ptr) {
+  std::lock_guard lock(m_read_thread_mutex);
+
   if (!m_read_thread.IsJoinable())
 return true;
 
@@ -199,13 +204,13 @@
 
   BroadcastEvent(eBroadcastBitReadThreadShouldExit, nullptr);
 
-  // error = m_read_thread.Cancel();
-
   Status error = m_read_thread.Join(nullptr);
   return error.Success();
 }
 
 bool ThreadedCommunication::JoinReadThread(Status *error_ptr) {
+  std::lock_guard lock(m_read_thread_mutex);
+
   if (!m_read_thread.IsJoinable())
 return true;
 
Index: lldb/include/lldb/Core/ThreadedCommunication.h
===
--- lldb/include/lldb/Core/ThreadedCommunication.h
+++ lldb/include/lldb/Core/ThreadedCommunication.h
@@ -223,10 +223,22 @@
   }
 
 protected:
-  HostThread m_read_thread; ///< The read thread handle in case we need to
-/// cancel the thread.
+  /// The read thread handle in case we need to cancel the thread.
+  /// @{
+  HostThread m_read_thread;
+  std::mutex m_read_thread_mutex;
+  /// @}
+
+  /// Whether the read thread is enabled. This cannot be guarded by the read
+  /// thread mutex becuase it is used as the control variable to exit the read
+  /// thread.
   std::atomic m_read_thread_enabled;
+
+  /// Whether the read thread is enabled. Technically this could be guarded by
+  /// the read thread mutex but that needlessly complicates things to
+  /// check this variables momentary value.
   std::atomic m_read_thread_did_exit;
+
   std::string
   m_bytes; ///< A buffer to cache bytes read in the ReadThread function.
   std::recursive_mutex m_bytes_mutex;   ///< A mutex to protect multi-threaded


Index: lldb/source/Core/ThreadedCommunication.cpp
===
--- lldb/source/Core/ThreadedCommunication.cpp
+++ lldb/source/Core/ThreadedCommunication.cpp
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -155,6 +156,8 @@
 }
 
 bool ThreadedCommunication::StartReadThread(Status *error_ptr) {
+  std::lock_guard lock(m_read_thread_mutex);
+
   if (error_ptr)
 error_ptr->Clear();
 
@@ -189,6 +192,8 @@
 }
 
 bool ThreadedCommunication::StopReadThread(Status *error_ptr) {
+  std::lock_guard lock(m_read_thread_mutex);
+
   if (!m_read_thread.IsJoinable())
 return true;
 
@@ -199,13 +204,13 @@
 
   BroadcastEvent(eBroadcastBitReadThreadShouldExit, nullptr);
 
-  // error = m_read_thread.Cancel();
-
   Status error = m_read_thread.Join(nullptr);
   return error.Success();
 }
 
 bool ThreadedCommunication::JoinReadThread(Status *error_ptr) {
+  std::lock_guard lock(m_read_thread_mutex);
+
   if (!m_read_thread.IsJoinable())
 return true;
 
Index: lldb/include/lldb/Core/ThreadedCommunication.h
===
--- lldb/include/lldb/Core/ThreadedCommunication.h
+++ lldb/include/lldb/Core/ThreadedCommunication.h
@@ -223,10 +223,22 @@
   }
 
 protected:
-  HostThread m_read_thread; ///< The read thread handle in case we need to
-/// cancel the thread.
+  /// The read thread handle in case we need to cancel the thread.
+  /// @{
+  HostThread m_read_thread;
+  std::mutex m_read_thread_mutex;
+  /// @}
+
+  /// Whether the read thread is enabled. This cannot be guarded by the read
+  /// thread mutex becuase it is used as the control variable to exit the read
+  /// thread.
   std::atomic m_read_thread_enabled;
+
+  /// Whether the read thread is enabled. Technically this could be guarded by
+  /// the read thread mutex but that needlessly complicates things to
+  /// check this variables momentary value.
   std::atomic m_read_thread_did_exit;
+
   std::string
   m_bytes; ///< A buffer to cache bytes read in the ReadThread function.
   std::recursive_mutex m_bytes_mutex;   ///< A mutex to protect multi-threaded
___
lldb-commits mailing list

[Lldb-commits] [PATCH] D157455: [lldb][NFCI] Remove MappedHash.h

2023-08-09 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157455

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

[Lldb-commits] [PATCH] D157538: [lldb][NFCI] Remove use of ifdef __cpluplus where unneeded

2023-08-09 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157538

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

[Lldb-commits] [PATCH] D157347: [lldb] Fix data race in NativeFile

2023-08-09 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere updated this revision to Diff 548729.
JDevlieghere marked an inline comment as done.
JDevlieghere added a comment.

Address @bulbazord's code review feedback


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

https://reviews.llvm.org/D157347

Files:
  lldb/include/lldb/Host/File.h
  lldb/source/Host/common/File.cpp

Index: lldb/source/Host/common/File.cpp
===
--- lldb/source/Host/common/File.cpp
+++ lldb/source/Host/common/File.cpp
@@ -219,7 +219,7 @@
 size_t File::PrintfVarArg(const char *format, va_list args) {
   llvm::SmallString<0> s;
   if (VASprintf(s, format, args)) {
-size_t written = s.size();;
+size_t written = s.size();
 Write(s.data(), written);
 return written;
   }
@@ -247,15 +247,20 @@
   return file_stats.st_mode & (S_IRWXU | S_IRWXG | S_IRWXO);
 }
 
+bool NativeFile::IsValid() const {
+  return DescriptorIsValid() || StreamIsValid();
+}
+
 Expected NativeFile::GetOptions() const { return m_options; }
 
 int NativeFile::GetDescriptor() const {
-  if (DescriptorIsValid())
+  if (ValueGuard descriptor_guard = DescriptorIsValid()) {
 return m_descriptor;
+  }
 
   // Don't open the file descriptor if we don't need to, just get it from the
   // stream if we have one.
-  if (StreamIsValid()) {
+  if (ValueGuard stream_guard = StreamIsValid()) {
 #if defined(_WIN32)
 return _fileno(m_stream);
 #else
@@ -272,8 +277,9 @@
 }
 
 FILE *NativeFile::GetStream() {
-  if (!StreamIsValid()) {
-if (DescriptorIsValid()) {
+  ValueGuard stream_guard = StreamIsValid();
+  if (!stream_guard) {
+if (ValueGuard descriptor_guard = DescriptorIsValid()) {
   auto mode = GetStreamOpenModeFromOptions(m_options);
   if (!mode)
 llvm::consumeError(mode.takeError());
@@ -306,8 +312,11 @@
 }
 
 Status NativeFile::Close() {
+  std::scoped_lock lock(m_descriptor_mutex, m_stream_mutex);
+
   Status error;
-  if (StreamIsValid()) {
+
+  if (StreamIsValidUnlocked()) {
 if (m_own_stream) {
   if (::fclose(m_stream) == EOF)
 error.SetErrorToErrno();
@@ -322,15 +331,17 @@
   }
 }
   }
-  if (DescriptorIsValid() && m_own_descriptor) {
+
+  if (DescriptorIsValidUnlocked() && m_own_descriptor) {
 if (::close(m_descriptor) != 0)
   error.SetErrorToErrno();
   }
-  m_descriptor = kInvalidDescriptor;
+
   m_stream = kInvalidStream;
-  m_options = OpenOptions(0);
   m_own_stream = false;
+  m_descriptor = kInvalidDescriptor;
   m_own_descriptor = false;
+  m_options = OpenOptions(0);
   m_is_interactive = eLazyBoolCalculate;
   m_is_real_terminal = eLazyBoolCalculate;
   return error;
@@ -374,7 +385,7 @@
 
 off_t NativeFile::SeekFromStart(off_t offset, Status *error_ptr) {
   off_t result = 0;
-  if (DescriptorIsValid()) {
+  if (ValueGuard descriptor_guard = DescriptorIsValid()) {
 result = ::lseek(m_descriptor, offset, SEEK_SET);
 
 if (error_ptr) {
@@ -383,7 +394,7 @@
   else
 error_ptr->Clear();
 }
-  } else if (StreamIsValid()) {
+  } else if (ValueGuard stream_guard = StreamIsValid()) {
 result = ::fseek(m_stream, offset, SEEK_SET);
 
 if (error_ptr) {
@@ -400,7 +411,7 @@
 
 off_t NativeFile::SeekFromCurrent(off_t offset, Status *error_ptr) {
   off_t result = -1;
-  if (DescriptorIsValid()) {
+  if (ValueGuard descriptor_guard = DescriptorIsValid()) {
 result = ::lseek(m_descriptor, offset, SEEK_CUR);
 
 if (error_ptr) {
@@ -409,7 +420,7 @@
   else
 error_ptr->Clear();
 }
-  } else if (StreamIsValid()) {
+  } else if (ValueGuard stream_guard = StreamIsValid()) {
 result = ::fseek(m_stream, offset, SEEK_CUR);
 
 if (error_ptr) {
@@ -426,7 +437,7 @@
 
 off_t NativeFile::SeekFromEnd(off_t offset, Status *error_ptr) {
   off_t result = -1;
-  if (DescriptorIsValid()) {
+  if (ValueGuard descriptor_guard = DescriptorIsValid()) {
 result = ::lseek(m_descriptor, offset, SEEK_END);
 
 if (error_ptr) {
@@ -435,7 +446,7 @@
   else
 error_ptr->Clear();
 }
-  } else if (StreamIsValid()) {
+  } else if (ValueGuard stream_guard = StreamIsValid()) {
 result = ::fseek(m_stream, offset, SEEK_END);
 
 if (error_ptr) {
@@ -452,18 +463,23 @@
 
 Status NativeFile::Flush() {
   Status error;
-  if (StreamIsValid()) {
+  if (ValueGuard stream_guard = StreamIsValid()) {
 if (llvm::sys::RetryAfterSignal(EOF, ::fflush, m_stream) == EOF)
   error.SetErrorToErrno();
-  } else if (!DescriptorIsValid()) {
-error.SetErrorString("invalid file handle");
+return error;
+  }
+
+  {
+ValueGuard descriptor_guard = DescriptorIsValid();
+if (!descriptor_guard)
+  error.SetErrorString("invalid file handle");
   }
   return error;
 }
 
 Status NativeFile::Sync() {
   Status error;
-  if (DescriptorIsValid()) {
+  if (ValueGuard descriptor_guard = DescriptorIsValid()) {
 #ifdef _WIN32
 int err = FlushFileBuffers((HANDLE)_get_osfhandle(m_descriptor));
 if (err == 0)

[Lldb-commits] [PATCH] D157450: [lldb][NFCI] Remove unused IOStreamMacros

2023-08-08 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

Cool


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157450

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

[Lldb-commits] [PATCH] D157160: Surface error messages from the DebugSymbols DBGShellCommands external agent; add logging for LC_NOTEs in Mach-O corefiles

2023-08-08 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

LGTM




Comment at: lldb/source/Core/DynamicLoader.cpp:241-242
+ error.AsCString("")[0] != '\0') {
+Stream  = target.GetDebugger().GetErrorStream();
+s << error.AsCString();
   }

If you don't need the Stream anymore you could do 

```
target.GetDebugger().GetErrorStream() << error.AsCString();
```



Comment at: lldb/source/Core/DynamicLoader.cpp:273-277
   LLDB_LOGF(log,
 "DynamicLoader::LoadBinaryWithUUIDAndAddress Loading "
-"binary UUID %s at %s 0x%" PRIx64,
-uuid.GetAsString().c_str(),
+"binary %s UUID %s at %s 0x%" PRIx64,
+name.str().c_str(), uuid.GetAsString().c_str(),
 value_is_offset ? "offset" : "address", value);

LLDB_LOG could simplify this a lot ;-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157160

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

[Lldb-commits] [PATCH] D156949: [lldb] Update LLDB Code Ownership

2023-08-08 Thread Jonas Devlieghere via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG013a8fc26ef8: [lldb] Update LLDB Code Ownership (authored by 
JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156949

Files:
  lldb/CODE_OWNERS.txt
  lldb/CodeOwners.rst

Index: lldb/CodeOwners.rst
===
--- /dev/null
+++ lldb/CodeOwners.rst
@@ -0,0 +1,252 @@
+
+LLDB Code Owners
+
+
+This file is a list of the `code owners `_ for LLDB.
+
+.. contents::
+   :depth: 2
+   :local:
+
+Current Code Owners
+===
+The following people are the active code owners for the project. Please reach
+out to them for code reviews, questions about their area of expertise, or other
+assistance.
+
+All parts of LLDB not covered by someone else
+--
+| Jonas Devlieghere
+| jonas\@devlieghere.com (email), jdevlieghere (Phabricator), jdevlieghere (GitHub), jdevlieghere (Discourse), jdevlieghere (Discord)
+
+Components
+--
+These code owners are responsible for particular high-level components within
+LLDB.
+
+ABI
+~~~
+| Jason Molenda
+| jmolenda\@apple.com (email), jasonmolenda (Phabricator), jasonmolenda (GitHub), jasonmolenda (Discourse), jasonmolenda (Discord)
+
+| David Spickett
+| david.spickett\@linaro.org (email), DavidSpickett (Phabricator), DavidSpickett (GitHub), DavidSpickett (Discourse), davidspickett (Discord)
+
+
+Breakpoint
+~~
+| Jim Ingham
+| jingham\@apple.com (email), jingham (Phabricator), jimingham (GitHub), jingham (Discourse)
+
+CMake & Build System
+
+| Jonas Devlieghere
+| jonas\@devlieghere.com (email), jdevlieghere (Phabricator), jdevlieghere (GitHub), jdevlieghere (Discourse), jdevlieghere (Discord)
+
+| Alex Langford
+| alangford\@apple.com (email), bulbazord (Phabricator), bulbazord (GitHub), bulbazord (Discourse), bulba_zord (Discord)
+
+Commands
+
+| Jim Ingham
+| jingham\@apple.com (email), jingham (Phabricator), jimingham (GitHub), jingham (Discourse)
+
+Expression Parser
+~
+| Michael Buch
+| michaelbuch12\@gmail.com (email), Michael137 (Phabricator), Michael137 (GitHub), Michael137 (Discourse)
+
+| Jim Ingham
+| jingham\@apple.com (email), jingham (Phabricator), jimingham (GitHub), jingham (Discourse)
+
+Interpreter
+~~~
+| Jim Ingham
+| jingham\@apple.com (email), jingham (Phabricator), jimingham (GitHub), jingham (Discourse)
+
+| Greg Clayton
+| gclayton\@fb.com (email), clayborg (Phabricator), clayborg (GitHub), clayborg (Discourse)
+
+
+Lua
+~~~
+| Jonas Delvieghere
+| jonas\@devlieghere.com (email), jdevlieghere (Phabricator), jdevlieghere (GitHub), jdevlieghere (Discourse), jdevlieghere (Discord)
+
+Python
+~~
+| Med Ismail Bennani
+| ismail\@bennani.ma (email), mib (Phabricator), medismailben (GitHub), mib (Discourse), mib#8727 (Discord)
+
+Target/Process Control
+~~
+| Med Ismail Bennani
+| ismail\@bennani.ma (email), mib (Phabricator), medismailben (GitHub), mib (Discourse), mib#8727 (Discord)
+
+| Jim Ingham
+| jingham\@apple.com (email), jingham (Phabricator), jimingham (GitHub), jingham (Discourse)
+
+Test Suite
+~~
+| Jonas Devlieghere
+| jonas\@devlieghere.com (email), jdevlieghere (Phabricator), jdevlieghere (GitHub), jdevlieghere (Discourse), jdevlieghere (Discord)
+
+| Pavel Labath
+| pavel\@labath.sk (email), labath (Phabricator), labath (GitHub), labath (Discourse)
+
+Trace
+~
+| Walter Erquinigo
+| a20012251\@gmail.com (email), wallace (Phabricator), walter-erquinigo (GitHub), wallace (Discourse), werquinigo (Discord)
+
+Unwinding
+~
+| Jason Molenda
+| jmolenda\@apple.com (email), jasonmolenda (Phabricator), jasonmolenda (GitHub), jasonmolenda (Discourse), jasonmolenda (Discord)
+
+Utility
+~~~
+| Jonas Devlieghere
+| jonas\@devlieghere.com (email), jdevlieghere (Phabricator), jdevlieghere (GitHub), jdevlieghere (Discourse), jdevlieghere (Discord)
+
+| Pavel Labath
+| pavel\@labath.sk (email), labath (Phabricator), labath (GitHub), labath (Discourse)
+
+ValueObject
+~~~
+| Jim Ingham
+| jingham\@apple.com (email), jingham (Phabricator), jimingham (GitHub), jingham (Discourse)
+
+Watchpoints
+~~~
+| Jason Molenda
+| jmolenda\@apple.com (email), jasonmolenda (Phabricator), jasonmolenda (GitHub), jasonmolenda (Discourse), jasonmolenda (Discord)
+
+File Formats
+
+The following people are responsible for decisions involving file and debug
+info formats.
+
+(PE)COFF
+
+| Saleem Abdulrasool
+| compnerd\@compnerd.org (email), compnerd (Phabricator), compnerd (GitHub), compnerd (Discourse), compnerd (Discord)
+
+Breakpad
+
+| Zequan Wu
+| zequanwu\@google.com (email), zequanwu (Phabricator),

[Lldb-commits] [PATCH] D156949: [lldb] Update LLDB Code Ownership

2023-08-08 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

Alright, seems like we have consensus. The only person that hasn't chimed in 
yet is Greg, but based on a comment in another thread he might be OOO. We can 
alway address concerns post-commit.


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

https://reviews.llvm.org/D156949

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

[Lldb-commits] [PATCH] D157361: [lldb] FIx data race in ThreadedCommunication

2023-08-07 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere created this revision.
JDevlieghere added reviewers: bulbazord, augusto2112.
Herald added a project: All.
JDevlieghere requested review of this revision.

TSan reports the following race:

  Write of size 8 at 0x000107707ee8 by main thread:
#0 lldb_private::ThreadedCommunication::StartReadThread(...) 
ThreadedCommunication.cpp:175
#1 lldb_private::Process::SetSTDIOFileDescriptor(...) Process.cpp:4533
#2 lldb_private::Platform::DebugProcess(...) Platform.cpp:1121
#3 lldb_private::PlatformDarwin::DebugProcess(...) PlatformDarwin.cpp:711
#4 lldb_private::Target::Launch(...) Target.cpp:3235
#5 CommandObjectProcessLaunch::DoExecute(...) CommandObjectProcess.cpp:256
#6 lldb_private::CommandObjectParsed::Execute(...) CommandObject.cpp:751
#7 lldb_private::CommandInterpreter::HandleCommand(...) 
CommandInterpreter.cpp:2054
  
  Previous read of size 8 at 0x000107707ee8 by thread T5:
#0 lldb_private::HostThread::IsJoinable(...) const HostThread.cpp:30
#1 lldb_private::ThreadedCommunication::StopReadThread(...) 
ThreadedCommunication.cpp:192
#2 lldb_private::Process::ShouldBroadcastEvent(...) Process.cpp:3420
#3 lldb_private::Process::HandlePrivateEvent(...) Process.cpp:3728
#4 lldb_private::Process::RunPrivateStateThread(...) Process.cpp:3914
#5 
std::__1::__function::__funchttps://reviews.llvm.org/D157361

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


Index: lldb/source/Core/ThreadedCommunication.cpp
===
--- lldb/source/Core/ThreadedCommunication.cpp
+++ lldb/source/Core/ThreadedCommunication.cpp
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -155,6 +156,8 @@
 }
 
 bool ThreadedCommunication::StartReadThread(Status *error_ptr) {
+  std::lock_guard lock(m_read_thread_mutex);
+
   if (error_ptr)
 error_ptr->Clear();
 
@@ -189,6 +192,8 @@
 }
 
 bool ThreadedCommunication::StopReadThread(Status *error_ptr) {
+  std::lock_guard lock(m_read_thread_mutex);
+
   if (!m_read_thread.IsJoinable())
 return true;
 
@@ -199,13 +204,13 @@
 
   BroadcastEvent(eBroadcastBitReadThreadShouldExit, nullptr);
 
-  // error = m_read_thread.Cancel();
-
   Status error = m_read_thread.Join(nullptr);
   return error.Success();
 }
 
 bool ThreadedCommunication::JoinReadThread(Status *error_ptr) {
+  std::lock_guard lock(m_read_thread_mutex);
+
   if (!m_read_thread.IsJoinable())
 return true;
 
Index: lldb/include/lldb/Core/ThreadedCommunication.h
===
--- lldb/include/lldb/Core/ThreadedCommunication.h
+++ lldb/include/lldb/Core/ThreadedCommunication.h
@@ -223,10 +223,22 @@
   }
 
 protected:
-  HostThread m_read_thread; ///< The read thread handle in case we need to
-/// cancel the thread.
+  /// The read thread handle in case we need to cancel the thread.
+  /// @{
+  HostThread m_read_thread;
+  std::mutex m_read_thread_mutex;
+  /// @}
+
+  /// Whether the read thread is enabled. This cannot be guarded by the read
+  /// thread mutex becuase it is used as the control variable to exit the read
+  /// thread.
   std::atomic m_read_thread_enabled;
+
+  /// Whether the read thread is enabled. Technically this could be guarded by
+  /// the read thread mutex but that needlessly complicates things to
+  /// check this variables momentary value.
   std::atomic m_read_thread_did_exit;
+
   std::string
   m_bytes; ///< A buffer to cache bytes read in the ReadThread function.
   std::recursive_mutex m_bytes_mutex;   ///< A mutex to protect multi-threaded


Index: lldb/source/Core/ThreadedCommunication.cpp
===
--- lldb/source/Core/ThreadedCommunication.cpp
+++ lldb/source/Core/ThreadedCommunication.cpp
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -155,6 +156,8 @@
 }
 
 bool ThreadedCommunication::StartReadThread(Status *error_ptr) {
+  std::lock_guard lock(m_read_thread_mutex);
+
   if (error_ptr)
 error_ptr->Clear();
 
@@ -189,6 +192,8 @@
 }
 
 bool ThreadedCommunication::StopReadThread(Status *error_ptr) {
+  std::lock_guard lock(m_read_thread_mutex);
+
   if (!m_read_thread.IsJoinable())
 return true;
 
@@ -199,13 +204,13 @@
 
   BroadcastEvent(eBroadcastBitReadThreadShouldExit, nullptr);
 
-  // error = m_read_thread.Cancel();
-
   Status error = m_read_thread.Join(nullptr);
   return error.Success();
 }
 
 bool ThreadedCommunication::JoinReadThread(Status *error_ptr) {
+  std::lock_guard lock(m_read_thread_mutex);
+
   if (!m_read_thread.IsJoinable())
 return true;
 
Index: lldb/include/lldb/Core/ThreadedCommunication.h
===
--- lldb/include/lldb/Core/ThreadedCommunication.h
+++ lldb/include/lldb/Core/ThreadedCommunication.h
@@

[Lldb-commits] [PATCH] D157159: [lldb] Properly protect the Communication class with reader/writer lock

2023-08-07 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere abandoned this revision.
JDevlieghere added a comment.

Abandoning this in favor of D157347 


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

https://reviews.llvm.org/D157159

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

[Lldb-commits] [PATCH] D157347: [lldb] Fix data race in NativeFile

2023-08-07 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere updated this revision to Diff 548056.

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

https://reviews.llvm.org/D157347

Files:
  lldb/include/lldb/Host/File.h
  lldb/source/Host/common/File.cpp

Index: lldb/source/Host/common/File.cpp
===
--- lldb/source/Host/common/File.cpp
+++ lldb/source/Host/common/File.cpp
@@ -78,21 +78,20 @@
 }
 
 Expected File::GetOptionsFromMode(llvm::StringRef mode) {
-  OpenOptions opts =
-  llvm::StringSwitch(mode)
-  .Cases("r", "rb", eOpenOptionReadOnly)
-  .Cases("w", "wb", eOpenOptionWriteOnly)
-  .Cases("a", "ab",
- eOpenOptionWriteOnly | eOpenOptionAppend |
- eOpenOptionCanCreate)
-  .Cases("r+", "rb+", "r+b", eOpenOptionReadWrite)
-  .Cases("w+", "wb+", "w+b",
- eOpenOptionReadWrite | eOpenOptionCanCreate |
- eOpenOptionTruncate)
-  .Cases("a+", "ab+", "a+b",
- eOpenOptionReadWrite | eOpenOptionAppend |
- eOpenOptionCanCreate)
-  .Default(eOpenOptionInvalid);
+  OpenOptions opts = llvm::StringSwitch(mode)
+ .Cases("r", "rb", eOpenOptionReadOnly)
+ .Cases("w", "wb", eOpenOptionWriteOnly)
+ .Cases("a", "ab",
+eOpenOptionWriteOnly | eOpenOptionAppend |
+eOpenOptionCanCreate)
+ .Cases("r+", "rb+", "r+b", eOpenOptionReadWrite)
+ .Cases("w+", "wb+", "w+b",
+eOpenOptionReadWrite | eOpenOptionCanCreate |
+eOpenOptionTruncate)
+ .Cases("a+", "ab+", "a+b",
+eOpenOptionReadWrite | eOpenOptionAppend |
+eOpenOptionCanCreate)
+ .Default(eOpenOptionInvalid);
   if (opts != eOpenOptionInvalid)
 return opts;
   return llvm::createStringError(
@@ -219,7 +218,8 @@
 size_t File::PrintfVarArg(const char *format, va_list args) {
   llvm::SmallString<0> s;
   if (VASprintf(s, format, args)) {
-size_t written = s.size();;
+size_t written = s.size();
+;
 Write(s.data(), written);
 return written;
   }
@@ -247,15 +247,20 @@
   return file_stats.st_mode & (S_IRWXU | S_IRWXG | S_IRWXO);
 }
 
+bool NativeFile::IsValid() const {
+  return DescriptorIsValid() || StreamIsValid();
+}
+
 Expected NativeFile::GetOptions() const { return m_options; }
 
 int NativeFile::GetDescriptor() const {
-  if (DescriptorIsValid())
+  if (ValueGuard descriptor_guard = DescriptorIsValid()) {
 return m_descriptor;
+  }
 
   // Don't open the file descriptor if we don't need to, just get it from the
   // stream if we have one.
-  if (StreamIsValid()) {
+  if (ValueGuard stream_guard = StreamIsValid()) {
 #if defined(_WIN32)
 return _fileno(m_stream);
 #else
@@ -272,8 +277,9 @@
 }
 
 FILE *NativeFile::GetStream() {
-  if (!StreamIsValid()) {
-if (DescriptorIsValid()) {
+  ValueGuard stream_guard = StreamIsValid();
+  if (!stream_guard) {
+if (ValueGuard descriptor_guard = DescriptorIsValid()) {
   auto mode = GetStreamOpenModeFromOptions(m_options);
   if (!mode)
 llvm::consumeError(mode.takeError());
@@ -307,30 +313,39 @@
 
 Status NativeFile::Close() {
   Status error;
-  if (StreamIsValid()) {
-if (m_own_stream) {
-  if (::fclose(m_stream) == EOF)
-error.SetErrorToErrno();
-} else {
-  File::OpenOptions rw =
-  m_options & (File::eOpenOptionReadOnly | File::eOpenOptionWriteOnly |
-   File::eOpenOptionReadWrite);
 
-  if (rw == eOpenOptionWriteOnly || rw == eOpenOptionReadWrite) {
-if (::fflush(m_stream) == EOF)
+  {
+ValueGuard stream_guard = StreamIsValid();
+if (stream_guard) {
+  if (m_own_stream) {
+if (::fclose(m_stream) == EOF)
   error.SetErrorToErrno();
+  } else {
+File::OpenOptions rw = m_options & (File::eOpenOptionReadOnly |
+File::eOpenOptionWriteOnly |
+File::eOpenOptionReadWrite);
+
+if (rw == eOpenOptionWriteOnly || rw == eOpenOptionReadWrite) {
+  if (::fflush(m_stream) == EOF)
+error.SetErrorToErrno();
+}
   }
 }
+m_stream = kInvalidStream;
+m_own_stream = false;
   }
-  if (DescriptorIsValid() && m_own_descriptor) {
-if (::close(m_descriptor) != 0)
-  error.SetErrorToErrno();
+
+  {
+ValueGuard descriptor_guard = DescriptorIsValid();
+if (descriptor_guard && m_own_descriptor) {
+  if (::close(m_descriptor) != 0)
+error.SetErrorToErrno();
+}
+m_descriptor = kInvalidDescriptor;
+m_own_descriptor = false;
   }
-  m_descriptor = kInvalidDescriptor;
-

[Lldb-commits] [PATCH] D157347: [lldb] Fix data race in ConnectionFileDescriptor

2023-08-07 Thread Jonas Devlieghere via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0bdbe7bd7f15: [lldb] Fix data race in 
ConnectionFileDescriptor (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157347

Files:
  lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
  lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp


Index: lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
===
--- lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
+++ lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -118,6 +118,7 @@
 }
 
 bool ConnectionFileDescriptor::IsConnected() const {
+  std::lock_guard guard(m_mutex);
   return m_io_sp && m_io_sp->IsValid();
 }
 
Index: lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
===
--- lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
+++ lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
@@ -131,7 +131,7 @@
   lldb::IOObjectSP m_io_sp;
 
   Pipe m_pipe;
-  std::recursive_mutex m_mutex;
+  mutable std::recursive_mutex m_mutex;
   std::atomic m_shutting_down; // This marks that we are shutting down so
  // if we get woken up from
   // BytesAvailable to disconnect, we won't try to read again.


Index: lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
===
--- lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
+++ lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -118,6 +118,7 @@
 }
 
 bool ConnectionFileDescriptor::IsConnected() const {
+  std::lock_guard guard(m_mutex);
   return m_io_sp && m_io_sp->IsValid();
 }
 
Index: lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
===
--- lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
+++ lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
@@ -131,7 +131,7 @@
   lldb::IOObjectSP m_io_sp;
 
   Pipe m_pipe;
-  std::recursive_mutex m_mutex;
+  mutable std::recursive_mutex m_mutex;
   std::atomic m_shutting_down; // This marks that we are shutting down so
  // if we get woken up from
   // BytesAvailable to disconnect, we won't try to read again.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D157160: Surface error messages from the DebugSymbols DBGShellCommands external agent; add logging for LC_NOTEs in Mach-O corefiles

2023-08-07 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added inline comments.



Comment at: lldb/source/Core/DynamicLoader.cpp:241-243
+Stream  = target.GetDebugger().GetOutputStream();
+s.Printf("Tried DBGShellCommands cmd, got error: %s\n",
+ error.AsCString());

Shouldn't this be the error stream?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157160

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

[Lldb-commits] [PATCH] D157347: [lldb] Fix data race in ConnectionFileDescriptor

2023-08-07 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere created this revision.
JDevlieghere added reviewers: bulbazord, augusto2112.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
JDevlieghere requested review of this revision.

TSan reports the following data race:

  Write of size 4 at 0x000109e0b160 by thread T2 (mutexes: write M0, write M1):
#0 lldb_private::NativeFile::Close() File.cpp:329 
(liblldb.18.0.0git.dylib:arm64+0x58dac0)
#1 
lldb_private::ConnectionFileDescriptor::Disconnect(lldb_private::Status*) 
ConnectionFileDescriptorPosix.cpp:232 (liblldb.18.0.0git.dylib:arm64+0x5bad6c)
#2 lldb_private::Communication::Disconnect(lldb_private::Status*) 
Communication.cpp:61 (liblldb.18.0.0git.dylib:arm64+0x451f04)
#3 lldb_private::process_gdb_remote::ProcessGDBRemote::DidExit() 
ProcessGDBRemote.cpp:1164 (liblldb.18.0.0git.dylib:arm64+0xadfd80)
#4 lldb_private::Process::SetExitStatus(int, char const*) Process.cpp:1097 
(liblldb.18.0.0git.dylib:arm64+0x6ed338)
#5 
lldb_private::process_gdb_remote::ProcessGDBRemote::MonitorDebugserverProcess(std::__1::weak_ptr,
 unsigned long long, int, int) ProcessGDBRemote.cpp:3387 
(liblldb.18.0.0git.dylib:arm64+0xae9464)
  
  Previous read of size 4 at 0x000109e0b160 by main thread (mutexes: write M2):
#0 lldb_private::NativeFile::IsValid() const File.h:393 
(liblldb.18.0.0git.dylib:arm64+0x590830)
#1 lldb_private::ConnectionFileDescriptor::IsConnected() const 
ConnectionFileDescriptorPosix.cpp:121 (liblldb.18.0.0git.dylib:arm64+0x5b8ab4)
#2 lldb_private::Communication::IsConnected() const Communication.cpp:79 
(liblldb.18.0.0git.dylib:arm64+0x451fcc)
#3 
lldb_private::process_gdb_remote::GDBRemoteCommunication::WaitForPacketNoLock(StringExtractorGDBRemote&,
 lldb_private::Timeout>, bool) 
GDBRemoteCommunication.cpp:256 (liblldb.18.0.0git.dylib:arm64+0xaac060)
#4 
lldb_private::process_gdb_remote::GDBRemoteCommunication::WaitForPacketNoLock(StringExtractorGDBRemote&,
 lldb_private::Timeout>, bool) 
GDBRemoteCommunication.cpp:244 (liblldb.18.0.0git.dylib:arm64+0xaabe78)
#5 
lldb_private::process_gdb_remote::GDBRemoteClientBase::SendPacketAndWaitForResponseNoLock(llvm::StringRef,
 StringExtractorGDBRemote&) GDBRemoteClientBase.cpp:246 
(liblldb.18.0.0git.dylib:arm64+0xaaa184)

The problem is that in `WaitForPacketNoLock`'s run loop, it checks that the 
connection is still connected. This races with the `ConnectionFileDescriptor` 
disconnecting. Most (but not all) access to the `IOObject` in 
`ConnectionFileDescriptorPosix` is already gated by the mutex. This patch just 
protects `IsConnected` in the same way.


https://reviews.llvm.org/D157347

Files:
  lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
  lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp


Index: lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
===
--- lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
+++ lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -118,6 +118,7 @@
 }
 
 bool ConnectionFileDescriptor::IsConnected() const {
+  std::lock_guard guard(m_mutex);
   return m_io_sp && m_io_sp->IsValid();
 }
 
Index: lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
===
--- lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
+++ lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
@@ -131,7 +131,7 @@
   lldb::IOObjectSP m_io_sp;
 
   Pipe m_pipe;
-  std::recursive_mutex m_mutex;
+  mutable std::recursive_mutex m_mutex;
   std::atomic m_shutting_down; // This marks that we are shutting down so
  // if we get woken up from
   // BytesAvailable to disconnect, we won't try to read again.


Index: lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
===
--- lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
+++ lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
@@ -118,6 +118,7 @@
 }
 
 bool ConnectionFileDescriptor::IsConnected() const {
+  std::lock_guard guard(m_mutex);
   return m_io_sp && m_io_sp->IsValid();
 }
 
Index: lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
===
--- lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
+++ lldb/include/lldb/Host/posix/ConnectionFileDescriptorPosix.h
@@ -131,7 +131,7 @@
   lldb::IOObjectSP m_io_sp;
 
   Pipe m_pipe;
-  std::recursive_mutex m_mutex;
+  mutable std::recursive_mutex m_mutex;
   std::atomic m_shutting_down; // This marks that we are shutting down so
  // if we get woken up from
   // BytesAvailable to disconnect, we won't try to read again.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D157167: [lldb] Add flag to DynamicLoader::LoadBinaryWithUUIDAndAddress to control whether we fall back to reading the binary out of memory

2023-08-07 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157167

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

[Lldb-commits] [PATCH] D157168: [lldb] [mach-o corefiles] If we have LC_NOTE metadata and can't find a binary, don't fall back to an exhaustive scan

2023-08-07 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added inline comments.



Comment at: lldb/source/Plugins/Process/mach-core/ProcessMachCore.h:89
   void CreateMemoryRegions();
-  void LoadBinariesViaMetadata();
+  bool LoadBinariesViaMetadata();
   void LoadBinariesViaExhaustiveSearch();

bulbazord wrote:
> Since `LoadBinariesViaMetadata` can fail and we now care about the return 
> value, what do you think about the name `TryLoadBinariesViaMetadata`? I think 
> a name that indicates that it may fail to actually load would make it easier 
> to read at a glance.
The same is true for the other methods though, so I don't know if that is 
really all that meaningful. IIUC, before this patch, all these methods conveyed 
whether they were successful by setting `m_dyld_plugin_name`. Now, 
`LoadBinariesViaMetadata` does something extra, which is conveyed by the 
boolean. I dislike the inconsistency, but I can't really think of a better way 
of doing this, I considered suggesting an out-parameter but that doesn't really 
improve anything. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157168

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

[Lldb-commits] [PATCH] D157152: [lldb] Make TSan errors fatal when running the test suite

2023-08-07 Thread Jonas Devlieghere via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG17226c976e04: [lldb] Make TSan errors fatal when running the 
test suite (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157152

Files:
  lldb/test/API/lit.cfg.py
  lldb/test/Shell/lit.cfg.py
  lldb/test/Unit/lit.cfg.py


Index: lldb/test/Unit/lit.cfg.py
===
--- lldb/test/Unit/lit.cfg.py
+++ lldb/test/Unit/lit.cfg.py
@@ -34,5 +34,9 @@
 )
 llvm_config.with_environment("PATH", os.path.dirname(sys.executable), 
append_path=True)
 
+# Enable sanitizer runtime flags.
+config.environment["ASAN_OPTIONS"] = "detect_stack_use_after_return=1"
+config.environment["TSAN_OPTIONS"] = "halt_on_error=1"
+
 # testFormat: The test format to use to interpret tests.
 config.test_format = lit.formats.GoogleTest(config.llvm_build_mode, "Tests")
Index: lldb/test/Shell/lit.cfg.py
===
--- lldb/test/Shell/lit.cfg.py
+++ lldb/test/Shell/lit.cfg.py
@@ -48,6 +48,10 @@
 ]
 )
 
+# Enable sanitizer runtime flags.
+config.environment["ASAN_OPTIONS"] = "detect_stack_use_after_return=1"
+config.environment["TSAN_OPTIONS"] = "halt_on_error=1"
+
 # Support running the test suite under the lldb-repro wrapper. This makes it
 # possible to capture a test suite run and then rerun all the test from the
 # just captured reproducer.
Index: lldb/test/API/lit.cfg.py
===
--- lldb/test/API/lit.cfg.py
+++ lldb/test/API/lit.cfg.py
@@ -123,6 +123,7 @@
 )
 
 if "Thread" in config.llvm_use_sanitizer:
+config.environment["TSAN_OPTIONS"] = "halt_on_error=1"
 if "Darwin" in config.host_os:
 config.environment["DYLD_INSERT_LIBRARIES"] = 
find_sanitizer_runtime(
 "libclang_rt.tsan_osx_dynamic.dylib"


Index: lldb/test/Unit/lit.cfg.py
===
--- lldb/test/Unit/lit.cfg.py
+++ lldb/test/Unit/lit.cfg.py
@@ -34,5 +34,9 @@
 )
 llvm_config.with_environment("PATH", os.path.dirname(sys.executable), append_path=True)
 
+# Enable sanitizer runtime flags.
+config.environment["ASAN_OPTIONS"] = "detect_stack_use_after_return=1"
+config.environment["TSAN_OPTIONS"] = "halt_on_error=1"
+
 # testFormat: The test format to use to interpret tests.
 config.test_format = lit.formats.GoogleTest(config.llvm_build_mode, "Tests")
Index: lldb/test/Shell/lit.cfg.py
===
--- lldb/test/Shell/lit.cfg.py
+++ lldb/test/Shell/lit.cfg.py
@@ -48,6 +48,10 @@
 ]
 )
 
+# Enable sanitizer runtime flags.
+config.environment["ASAN_OPTIONS"] = "detect_stack_use_after_return=1"
+config.environment["TSAN_OPTIONS"] = "halt_on_error=1"
+
 # Support running the test suite under the lldb-repro wrapper. This makes it
 # possible to capture a test suite run and then rerun all the test from the
 # just captured reproducer.
Index: lldb/test/API/lit.cfg.py
===
--- lldb/test/API/lit.cfg.py
+++ lldb/test/API/lit.cfg.py
@@ -123,6 +123,7 @@
 )
 
 if "Thread" in config.llvm_use_sanitizer:
+config.environment["TSAN_OPTIONS"] = "halt_on_error=1"
 if "Darwin" in config.host_os:
 config.environment["DYLD_INSERT_LIBRARIES"] = find_sanitizer_runtime(
 "libclang_rt.tsan_osx_dynamic.dylib"
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D157152: [lldb] Make TSan errors fatal when running the test suite

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

JDevlieghere updated this revision to Diff 547641.
JDevlieghere added a comment.

Include the unit tests


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

https://reviews.llvm.org/D157152

Files:
  lldb/test/API/lit.cfg.py
  lldb/test/Shell/lit.cfg.py
  lldb/test/Unit/lit.cfg.py


Index: lldb/test/Unit/lit.cfg.py
===
--- lldb/test/Unit/lit.cfg.py
+++ lldb/test/Unit/lit.cfg.py
@@ -34,5 +34,9 @@
 )
 llvm_config.with_environment("PATH", os.path.dirname(sys.executable), 
append_path=True)
 
+# Enable sanitizer runtime flags.
+config.environment["ASAN_OPTIONS"] = "detect_stack_use_after_return=1"
+config.environment["TSAN_OPTIONS"] = "halt_on_error=1"
+
 # testFormat: The test format to use to interpret tests.
 config.test_format = lit.formats.GoogleTest(config.llvm_build_mode, "Tests")
Index: lldb/test/Shell/lit.cfg.py
===
--- lldb/test/Shell/lit.cfg.py
+++ lldb/test/Shell/lit.cfg.py
@@ -48,6 +48,10 @@
 ]
 )
 
+# Enable sanitizer runtime flags.
+config.environment["ASAN_OPTIONS"] = "detect_stack_use_after_return=1"
+config.environment["TSAN_OPTIONS"] = "halt_on_error=1"
+
 # Support running the test suite under the lldb-repro wrapper. This makes it
 # possible to capture a test suite run and then rerun all the test from the
 # just captured reproducer.
Index: lldb/test/API/lit.cfg.py
===
--- lldb/test/API/lit.cfg.py
+++ lldb/test/API/lit.cfg.py
@@ -123,6 +123,7 @@
 )
 
 if "Thread" in config.llvm_use_sanitizer:
+config.environment["TSAN_OPTIONS"] = "halt_on_error=1"
 if "Darwin" in config.host_os:
 config.environment["DYLD_INSERT_LIBRARIES"] = 
find_sanitizer_runtime(
 "libclang_rt.tsan_osx_dynamic.dylib"


Index: lldb/test/Unit/lit.cfg.py
===
--- lldb/test/Unit/lit.cfg.py
+++ lldb/test/Unit/lit.cfg.py
@@ -34,5 +34,9 @@
 )
 llvm_config.with_environment("PATH", os.path.dirname(sys.executable), append_path=True)
 
+# Enable sanitizer runtime flags.
+config.environment["ASAN_OPTIONS"] = "detect_stack_use_after_return=1"
+config.environment["TSAN_OPTIONS"] = "halt_on_error=1"
+
 # testFormat: The test format to use to interpret tests.
 config.test_format = lit.formats.GoogleTest(config.llvm_build_mode, "Tests")
Index: lldb/test/Shell/lit.cfg.py
===
--- lldb/test/Shell/lit.cfg.py
+++ lldb/test/Shell/lit.cfg.py
@@ -48,6 +48,10 @@
 ]
 )
 
+# Enable sanitizer runtime flags.
+config.environment["ASAN_OPTIONS"] = "detect_stack_use_after_return=1"
+config.environment["TSAN_OPTIONS"] = "halt_on_error=1"
+
 # Support running the test suite under the lldb-repro wrapper. This makes it
 # possible to capture a test suite run and then rerun all the test from the
 # just captured reproducer.
Index: lldb/test/API/lit.cfg.py
===
--- lldb/test/API/lit.cfg.py
+++ lldb/test/API/lit.cfg.py
@@ -123,6 +123,7 @@
 )
 
 if "Thread" in config.llvm_use_sanitizer:
+config.environment["TSAN_OPTIONS"] = "halt_on_error=1"
 if "Darwin" in config.host_os:
 config.environment["DYLD_INSERT_LIBRARIES"] = find_sanitizer_runtime(
 "libclang_rt.tsan_osx_dynamic.dylib"
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D157159: [lldb] Properly protect the Communication class with reader/writer lock

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

JDevlieghere added inline comments.



Comment at: lldb/source/Core/ThreadedCommunication.cpp:63-67
-void ThreadedCommunication::Clear() {
-  SetReadThreadBytesReceivedCallback(nullptr, nullptr);
-  StopReadThread(nullptr);
-  Communication::Clear();
-}

This wasn't used: nobody called `Clear` on an instance of 
`ThreadedCommunication`. 



Comment at: lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp:560
   StopAsyncThread();
-  m_comm.Clear();
 

According to the comment on line 541 this is wrong:

``` // We are running and we can't interrupt a running kernel, so we need to
// just close the connection to the kernel and hope for the best```

`m_comm` is an instance of `CommunicationKDP` which didn't override 
`Communication::Clear` and hence just called `Disconnected`. 


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

https://reviews.llvm.org/D157159

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

[Lldb-commits] [PATCH] D157159: [lldb] Properly protect the Communication class with reader/writer lock

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

JDevlieghere updated this revision to Diff 547639.
JDevlieghere added a comment.

- Remove `Communication::Clear` method


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

https://reviews.llvm.org/D157159

Files:
  lldb/include/lldb/Core/Communication.h
  lldb/include/lldb/Core/ThreadedCommunication.h
  lldb/source/Core/Communication.cpp
  lldb/source/Core/ThreadedCommunication.cpp
  lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp

Index: lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
===
--- lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
+++ lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
@@ -557,7 +557,6 @@
 }
   }
   StopAsyncThread();
-  m_comm.Clear();
 
   SetPrivateState(eStateDetached);
   ResumePrivateStateThread();
Index: lldb/source/Core/ThreadedCommunication.cpp
===
--- lldb/source/Core/ThreadedCommunication.cpp
+++ lldb/source/Core/ThreadedCommunication.cpp
@@ -60,12 +60,6 @@
this, GetBroadcasterName());
 }
 
-void ThreadedCommunication::Clear() {
-  SetReadThreadBytesReceivedCallback(nullptr, nullptr);
-  StopReadThread(nullptr);
-  Communication::Clear();
-}
-
 ConnectionStatus ThreadedCommunication::Disconnect(Status *error_ptr) {
   assert((!m_read_thread_enabled || m_read_thread_did_exit) &&
  "Disconnecting while the read thread is running is racy!");
Index: lldb/source/Core/Communication.cpp
===
--- lldb/source/Core/Communication.cpp
+++ lldb/source/Core/Communication.cpp
@@ -27,68 +27,57 @@
 using namespace lldb_private;
 
 Communication::Communication()
-: m_connection_sp(), m_write_mutex(), m_close_on_eof(true) {
-}
-
-Communication::~Communication() {
-  Clear();
-}
+: m_connection_sp(), m_shared_mutex(), m_close_on_eof(true) {}
 
-void Communication::Clear() {
-  Disconnect(nullptr);
-}
+Communication::~Communication() { Disconnect(nullptr); }
 
 ConnectionStatus Communication::Connect(const char *url, Status *error_ptr) {
-  Clear();
-
   LLDB_LOG(GetLog(LLDBLog::Communication),
"{0} Communication::Connect (url = {1})", this, url);
 
-  lldb::ConnectionSP connection_sp(m_connection_sp);
-  if (connection_sp)
-return connection_sp->Connect(url, error_ptr);
+  std::unique_lock guard(m_shared_mutex);
+
+  DisconnectUnlocked();
+
+  if (m_connection_sp)
+return m_connection_sp->Connect(url, error_ptr);
   if (error_ptr)
 error_ptr->SetErrorString("Invalid connection.");
   return eConnectionStatusNoConnection;
 }
 
 ConnectionStatus Communication::Disconnect(Status *error_ptr) {
+  std::unique_lock guard(m_shared_mutex);
+  return DisconnectUnlocked();
+}
+
+ConnectionStatus Communication::DisconnectUnlocked(Status *error_ptr) {
   LLDB_LOG(GetLog(LLDBLog::Communication), "{0} Communication::Disconnect ()",
this);
 
-  lldb::ConnectionSP connection_sp(m_connection_sp);
-  if (connection_sp) {
-ConnectionStatus status = connection_sp->Disconnect(error_ptr);
-// We currently don't protect connection_sp with any mutex for multi-
-// threaded environments. So lets not nuke our connection class without
-// putting some multi-threaded protections in. We also probably don't want
-// to pay for the overhead it might cause if every time we access the
-// connection we have to take a lock.
-//
-// This unique pointer will cleanup after itself when this object goes
-// away, so there is no need to currently have it destroy itself
-// immediately upon disconnect.
-// connection_sp.reset();
+  if (m_connection_sp) {
+ConnectionStatus status = m_connection_sp->Disconnect(error_ptr);
 return status;
   }
   return eConnectionStatusNoConnection;
 }
 
 bool Communication::IsConnected() const {
-  lldb::ConnectionSP connection_sp(m_connection_sp);
-  return (connection_sp ? connection_sp->IsConnected() : false);
+  std::shared_lock guard(m_shared_mutex);
+  return (m_connection_sp ? m_connection_sp->IsConnected() : false);
 }
 
 bool Communication::HasConnection() const {
+  std::shared_lock guard(m_shared_mutex);
   return m_connection_sp.get() != nullptr;
 }
 
 size_t Communication::Read(void *dst, size_t dst_len,
const Timeout ,
ConnectionStatus , Status *error_ptr) {
-  Log *log = GetLog(LLDBLog::Communication);
+  std::shared_lock guard(m_shared_mutex);
   LLDB_LOG(
-  log,
+  GetLog(LLDBLog::Communication),
   "this = {0}, dst = {1}, dst_len = {2}, timeout = {3}, connection = {4}",
   this, dst, dst_len, timeout, m_connection_sp.get());
 
@@ -97,16 +86,20 @@
 
 size_t Communication::Write(const void *src, size_t src_len,
 ConnectionStatus , Status *error_ptr) {
-  lldb::ConnectionSP connection_sp(m_connection_sp);
+  std::unique_lock

[Lldb-commits] [PATCH] D157167: [lldb] Add flag to DynamicLoader::LoadBinaryWithUUIDAndAddress to control whether we fall back to reading the binary out of memory

2023-08-05 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added inline comments.



Comment at: lldb/include/lldb/Target/DynamicLoader.h:267
   ///
+  /// \param[in] allow_use_memory_image_last_resort
+  /// If no better binary image can be found, allow reading the binary

Nit: seems like either `allow_memory_image_last_resort` or 
`use_memory_image_last_resort` would convey the same thing. 



Comment at: 
lldb/test/API/macosx/lc-note/multiple-binary-corefile/TestMultipleBinaryCorefile.py:38-39
 )
+if self.TraceOn():
+self.runCmd("script print('Creating corefile with command %s')" % 
cmd)
 call(cmd, shell=True)

Out of curiosity what's the point of doing `self.runCmd("script print ...` vs 
printing the same thing directly? Does this run in another context or 
something? Do we save the command output somewhere? 



Comment at: 
lldb/test/API/macosx/lc-note/multiple-binary-corefile/TestMultipleBinaryCorefile.py:122-123
 )
+dwarfdump_cmd_output = subprocess.check_output(
+('/usr/bin/dwarfdump --uuid "%s"' % self.aout_exe), shell=True
+).decode("utf-8")

I know this was moved and this goes beyond the scope of this patch, but we 
should be using the just-built `llvm-dwarfdump` instead of whatever happens to 
be on the system. Obviously no big deal for extracting the `--uuid` but it's 
relatively easy to hook up and what we do for other llvm tools such as 
dsymutil. objdump, etc. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157167

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

1 2 3 4 5 6 7 8 9 10 >

1 - 100 of 4268 matches

Mail list logo