[Lldb-commits] [PATCH] D128268: [lldb] Fix reading i686-windows executables with GNU environment

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D128268#3611053 , @mstorsjo wrote:

> The odd thing about the second one is the added hardcoded 
> `AddArch(ArchSpec("i686-pc-windows"));` and 
> `AddArch(ArchSpec("i386-pc-windows"));` at the top of `PlatformWindows.cpp`.
>
> Anyway, it does seem to work fine in my quick test to get rid of this 
> duality, so I'll post a patch that does that.

I think those hardcoded lines are there precisely because of the duality (i.e. 
wanting to support both i386 and i686 regardless of what the host layer 
reports). If that is removed, maybe all of that can be changed to  the pattern 
used in other platform plugins 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128268

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

[Lldb-commits] [PATCH] D128617: [lldb] Stop passing both i386 and i686 in parallel as architectures on Windows

[Pavel Labath via Phabricator via lldb-commits]

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

Thanks for cleaning this up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128617

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

[Lldb-commits] [PATCH] D128710: [lldb] [llgs] Fix multi-resume bugs with nonstop mode

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

mgorny created this revision.
mgorny added reviewers: labath, krytarowski, jingham, emaste.
Herald added a subscriber: arichardson.
Herald added a project: All.
mgorny requested review of this revision.

Improve handling of multiple successive continue packets in non-stop
mode.  More specifically:

1. Explicitly send error response (instead of crashing on assertion) if the 
user attempts to resume the same process twice.  Since we do not support 
thread-level non-stop mode, one needs to always stop the process explicitly 
before resuming another thread set.

2. Actually stop the process if "vCont;t" is delivered to a running process.  
Similarly, we only support stopping all the running threads simultaneously and 
return an error if the action would result in a subset of threads still running.

With this patch, running multiple processes simultaneously is still
unsupported.  The patch also employs a hack to avoid enabling stdio
forwarding on "vCont;t" packet.  Both of these issues will be addressed
by followup patches.

Sponsored by: The FreeBSD Foundation


https://reviews.llvm.org/D128710

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/test/API/tools/lldb-server/TestNonStop.py

Index: lldb/test/API/tools/lldb-server/TestNonStop.py
===
--- lldb/test/API/tools/lldb-server/TestNonStop.py
+++ lldb/test/API/tools/lldb-server/TestNonStop.py
@@ -170,3 +170,128 @@
  "send packet: $OK#00",
  ], True)
 self.expect_gdbremote_sequence()
+
+def multiple_resume_test(self, second_command):
+self.build()
+self.set_inferior_startup_launch()
+procs = self.prep_debug_monitor_and_inferior(
+inferior_args=["sleep:15"])
+self.test_sequence.add_log_lines(
+["read packet: $QNonStop:1#00",
+ "send packet: $OK#00",
+ "read packet: $c#63",
+ "send packet: $OK#00",
+ "read packet: ${}#00".format(second_command),
+ "send packet: $E37#00",
+ ], True)
+self.expect_gdbremote_sequence()
+
+@add_test_categories(["llgs"])
+def test_multiple_C(self):
+self.multiple_resume_test("C05")
+
+@add_test_categories(["llgs"])
+def test_multiple_c(self):
+self.multiple_resume_test("c")
+
+@add_test_categories(["llgs"])
+def test_multiple_s(self):
+self.multiple_resume_test("s")
+
+@add_test_categories(["llgs"])
+def test_multiple_vCont(self):
+self.build()
+self.set_inferior_startup_launch()
+procs = self.prep_debug_monitor_and_inferior(
+inferior_args=["thread:new", "trap", "sleep:15"])
+self.test_sequence.add_log_lines(
+["read packet: $QNonStop:1#00",
+ "send packet: $OK#00",
+ "read packet: $c#63",
+ "send packet: $OK#00",
+ {"direction": "send",
+  "regex": r"^%Stop:T[0-9a-fA-F]{2}thread:([0-9a-fA-F]+);",
+  "capture": {1: "tid1"},
+  },
+ "read packet: $vStopped#63",
+ {"direction": "send",
+  "regex": r"^[$]T[0-9a-fA-F]{2}thread:([0-9a-fA-F]+);",
+  "capture": {1: "tid2"},
+  },
+ "read packet: $vStopped#63",
+ "send packet: $OK#00",
+ ], True)
+ret = self.expect_gdbremote_sequence()
+
+self.reset_test_sequence()
+self.test_sequence.add_log_lines(
+["read packet: $vCont;c:{}#00".format(ret["tid1"]),
+ "send packet: $OK#00",
+ "read packet: $vCont;c:{}#00".format(ret["tid2"]),
+ "send packet: $E37#00",
+ ], True)
+self.expect_gdbremote_sequence()
+
+@add_test_categories(["llgs"])
+def test_vCont_then_stop(self):
+self.build()
+self.set_inferior_startup_launch()
+procs = self.prep_debug_monitor_and_inferior(
+inferior_args=["sleep:15"])
+self.test_sequence.add_log_lines(
+["read packet: $QNonStop:1#00",
+ "send packet: $OK#00",
+ "read packet: $c#63",
+ "send packet: $OK#00",
+ "read packet: $vCont;t#00",
+ "send packet: $OK#00",
+ ], True)
+self.expect_gdbremote_sequence()
+
+def vCont_then_partial_stop_test(self, run_both):
+self.build()
+self.set_inferior_startup_launch()
+procs = self.prep_debug_monitor_and_inferior(
+inferior_args=["thread:new", "trap", "sleep:15"])
+self.test_sequence.add_log_lines(
+["read packet: $QNonStop:1#00",
+ "send packet: $OK#00",
+ "read packet: $c#63",
+ "send packet: $OK#00",
+ {"direction": "send",
+  "regex": r"^%Stop:T[0-9a-fA-F]{2}thread:([0-9a-fA-F]+);",
+  "capture"

[Lldb-commits] [PATCH] D127755: [lldb] [llgs] Add test for resuming via c in multiprocess scenarios

[Muhammad Omair Javaid via Phabricator via lldb-commits]

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

This breaks on LLDB Arm,AArch64/Linux buildbots
https://lab.llvm.org/buildbot/#/builders/17
https://lab.llvm.org/buildbot/#/builders/96


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127755

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

[Lldb-commits] [PATCH] D128543: [trace] Improve the TraceCursor iteration API

[Jakob Johnson via Phabricator via lldb-commits]

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

lgtm, thanks for making the cursor traversal much cleaner




Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:228-231
-  // We insert a fake error signaling an empty trace if needed becasue the
-  // TraceCursor requires non-empty traces.
-  if (m_instruction_ips.empty())
-AppendError(createStringError(inconvertibleErrorCode(), "empty trace"));

nice, the new approach is much cleaner



Comment at: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp:38
+void TraceCursorIntelPT::Next() {
+  m_pos += IsForwards() ? 1 : -1;
 

should only do this increment or decrement if `HasValue()` is true? otherwise 
(in theory) this value could wrap around if it's incremented/decremented too 
many times?



Comment at: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp:40
 
+  {
+// We try to go to a neighbor tsc range that might contain the current pos

why is this new scope introduced here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128543

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

[Lldb-commits] [PATCH] D128678: [LLDB] Add PDB/calling-conventions.test for Arm/Windows

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid added inline comments.



Comment at: lldb/test/Shell/SymbolFile/PDB/calling-conventions-arm.test:1
+REQUIRES: system-windows, lld, (target-arm || target-aarch64) 
+RUN: %build --compiler=clang-cl --arch=32 --nodefaultlib --output=%t.exe 
%S/Inputs/CallingConventionsTest.cpp

mstorsjo wrote:
> Hmm, I wasn't aware of the fact that you can do such `||` expressions in the 
> `REQUIRES` line - I presume you've verified that this test actually does get 
> picked up and executed?
This is tested on both Arm/Windows and x86/Windows.



Comment at: lldb/test/Shell/SymbolFile/PDB/calling-conventions-arm.test:2
+REQUIRES: system-windows, lld, (target-arm || target-aarch64) 
+RUN: %build --compiler=clang-cl --arch=32 --nodefaultlib --output=%t.exe 
%S/Inputs/CallingConventionsTest.cpp
+RUN: lldb-test symbols -dump-ast %t.exe | FileCheck %s

mstorsjo wrote:
> Here, there's probably no need to force it to 32 bit mode - unless we expect 
> to have a similar test for the undecorated mangling in aarch64 and x86_64 
> mode too?
Mangling is specific to x86 32 bit only. All other configs shouldnt do it. I 
guess we can test it for both 64bit and 32bit arm. Let me update my change.


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

https://reviews.llvm.org/D128678

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

[Lldb-commits] [lldb] b83b82f - [lldb] Fix build on older Linux kernel versions

[Yi Kong via lldb-commits]

Author: Yi Kong
Date: 2022-06-28T20:23:33+08:00
New Revision: b83b82f9f431f20ae35cb3b11443049e31a71481

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

LOG: [lldb] Fix build on older Linux kernel versions

PERF_COUNT_SW_DUMMY is introduced in Linux 3.12.

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

Added: 


Modified: 
lldb/source/Plugins/Process/Linux/Perf.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Process/Linux/Perf.cpp 
b/lldb/source/Plugins/Process/Linux/Perf.cpp
index bc2038c371712..fa4e8fb42e6cd 100644
--- a/lldb/source/Plugins/Process/Linux/Perf.cpp
+++ b/lldb/source/Plugins/Process/Linux/Perf.cpp
@@ -15,6 +15,7 @@
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/MemoryBuffer.h"
 
+#include 
 #include 
 #include 
 #include 
@@ -26,6 +27,7 @@ using namespace llvm;
 
 Expected
 lldb_private::process_linux::LoadPerfTscConversionParameters() {
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(3,12,0)
   lldb::pid_t pid = getpid();
   perf_event_attr attr;
   memset(&attr, 0, sizeof(attr));
@@ -55,6 +57,10 @@ 
lldb_private::process_linux::LoadPerfTscConversionParameters() {
   err_cap);
 return llvm::createStringError(llvm::inconvertibleErrorCode(), err_msg);
   }
+#else
+  std::string err_msg = "PERF_COUNT_SW_DUMMY requires Linux 3.12";
+  return llvm::createStringError(llvm::inconvertibleErrorCode(), err_msg);
+#endif
 }
 
 void resource_handle::MmapDeleter::operator()(void *ptr) {



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

[Lldb-commits] [PATCH] D128707: [lldb] Fix build on older Linux kernel versions

[Yi Kong via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb83b82f9f431: [lldb] Fix build on older Linux kernel 
versions (authored by kongyi).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128707

Files:
  lldb/source/Plugins/Process/Linux/Perf.cpp


Index: lldb/source/Plugins/Process/Linux/Perf.cpp
===
--- lldb/source/Plugins/Process/Linux/Perf.cpp
+++ lldb/source/Plugins/Process/Linux/Perf.cpp
@@ -15,6 +15,7 @@
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/MemoryBuffer.h"
 
+#include 
 #include 
 #include 
 #include 
@@ -26,6 +27,7 @@
 
 Expected
 lldb_private::process_linux::LoadPerfTscConversionParameters() {
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(3,12,0)
   lldb::pid_t pid = getpid();
   perf_event_attr attr;
   memset(&attr, 0, sizeof(attr));
@@ -55,6 +57,10 @@
   err_cap);
 return llvm::createStringError(llvm::inconvertibleErrorCode(), err_msg);
   }
+#else
+  std::string err_msg = "PERF_COUNT_SW_DUMMY requires Linux 3.12";
+  return llvm::createStringError(llvm::inconvertibleErrorCode(), err_msg);
+#endif
 }
 
 void resource_handle::MmapDeleter::operator()(void *ptr) {


Index: lldb/source/Plugins/Process/Linux/Perf.cpp
===
--- lldb/source/Plugins/Process/Linux/Perf.cpp
+++ lldb/source/Plugins/Process/Linux/Perf.cpp
@@ -15,6 +15,7 @@
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/MemoryBuffer.h"
 
+#include 
 #include 
 #include 
 #include 
@@ -26,6 +27,7 @@
 
 Expected
 lldb_private::process_linux::LoadPerfTscConversionParameters() {
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(3,12,0)
   lldb::pid_t pid = getpid();
   perf_event_attr attr;
   memset(&attr, 0, sizeof(attr));
@@ -55,6 +57,10 @@
   err_cap);
 return llvm::createStringError(llvm::inconvertibleErrorCode(), err_msg);
   }
+#else
+  std::string err_msg = "PERF_COUNT_SW_DUMMY requires Linux 3.12";
+  return llvm::createStringError(llvm::inconvertibleErrorCode(), err_msg);
+#endif
 }
 
 void resource_handle::MmapDeleter::operator()(void *ptr) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D128678: [LLDB] Add PDB/calling-conventions.test for Arm/Windows

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid updated this revision to Diff 440581.
omjavaid added a comment.

Added test for AArch64 and x64 calling conventions.


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

https://reviews.llvm.org/D128678

Files:
  lldb/test/Shell/SymbolFile/PDB/calling-conventions-arm.test
  lldb/test/Shell/SymbolFile/PDB/calling-conventions-x86.test
  lldb/test/Shell/SymbolFile/PDB/calling-conventions.test


Index: lldb/test/Shell/SymbolFile/PDB/calling-conventions.test
===
--- lldb/test/Shell/SymbolFile/PDB/calling-conventions.test
+++ /dev/null
@@ -1,10 +0,0 @@
-REQUIRES: system-windows, lld
-RUN: %build --compiler=clang-cl --arch=32 --nodefaultlib --output=%t.exe 
%S/Inputs/CallingConventionsTest.cpp
-RUN: lldb-test symbols -dump-ast %t.exe | FileCheck %s
-
-CHECK: Module: {{.*}}
-CHECK-DAG: int (*FuncCCallPtr)();
-CHECK-DAG: int (*FuncStdCallPtr)() __attribute__((stdcall));
-CHECK-DAG: int (*FuncFastCallPtr)() __attribute__((fastcall));
-CHECK-DAG: int (*FuncVectorCallPtr)() __attribute__((vectorcall));
-CHECK-DAG: int (S::*FuncThisCallPtr)() __attribute__((thiscall));
Index: lldb/test/Shell/SymbolFile/PDB/calling-conventions-x86.test
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/PDB/calling-conventions-x86.test
@@ -0,0 +1,19 @@
+REQUIRES: system-windows, lld, (target-x86 || target-x86_64) 
+RUN: %build --compiler=clang-cl --arch=32 --nodefaultlib --output=%t.exe 
%S/Inputs/CallingConventionsTest.cpp \
+RUN:  && lldb-test symbols -dump-ast %t.exe | FileCheck --check-prefix 
32BIT-CHECK %s
+RUN: %build --compiler=clang-cl --arch=64 --nodefaultlib --output=%t.exe 
%S/Inputs/CallingConventionsTest.cpp \
+RUN:  && lldb-test symbols -dump-ast %t.exe | FileCheck --check-prefix 
64BIT-CHECK %s
+
+64BIT-CHECK: Module: {{.*}}
+64BIT-CHECK-DAG: int (*FuncCCallPtr)();
+64BIT-CHECK-DAG: int (*FuncStdCallPtr)();
+64BIT-CHECK-DAG: int (*FuncFastCallPtr)();
+64BIT-CHECK-DAG: int (*FuncVectorCallPtr)() __attribute__((vectorcall));
+64BIT-CHECK-DAG: int (S::*FuncThisCallPtr)();
+
+32BIT-CHECK: Module: {{.*}}
+32BIT-CHECK-DAG: int (*FuncCCallPtr)();
+32BIT-CHECK-DAG: int (*FuncStdCallPtr)() __attribute__((stdcall));
+32BIT-CHECK-DAG: int (*FuncFastCallPtr)() __attribute__((fastcall));
+32BIT-CHECK-DAG: int (*FuncVectorCallPtr)() __attribute__((vectorcall));
+32BIT-CHECK-DAG: int (S::*FuncThisCallPtr)() __attribute__((thiscall));
Index: lldb/test/Shell/SymbolFile/PDB/calling-conventions-arm.test
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/PDB/calling-conventions-arm.test
@@ -0,0 +1,11 @@
+REQUIRES: system-windows, lld, (target-arm || target-aarch64) 
+RUN: %build --compiler=clang-cl --arch=32 --nodefaultlib --output=%t.exe 
%S/Inputs/CallingConventionsTest.cpp
+RUN: %build --compiler=clang-cl --arch=64 --nodefaultlib --output=%t.exe 
%S/Inputs/CallingConventionsTest.cpp
+RUN: lldb-test symbols -dump-ast %t.exe | FileCheck %s
+
+CHECK: Module: {{.*}}
+CHECK-DAG: int (*FuncCCallPtr)();
+CHECK-DAG: int (*FuncStdCallPtr)();
+CHECK-DAG: int (*FuncFastCallPtr)();
+CHECK-DAG: int (*FuncVectorCallPtr)();
+CHECK-DAG: int (S::*FuncThisCallPtr)();


Index: lldb/test/Shell/SymbolFile/PDB/calling-conventions.test
===
--- lldb/test/Shell/SymbolFile/PDB/calling-conventions.test
+++ /dev/null
@@ -1,10 +0,0 @@
-REQUIRES: system-windows, lld
-RUN: %build --compiler=clang-cl --arch=32 --nodefaultlib --output=%t.exe %S/Inputs/CallingConventionsTest.cpp
-RUN: lldb-test symbols -dump-ast %t.exe | FileCheck %s
-
-CHECK: Module: {{.*}}
-CHECK-DAG: int (*FuncCCallPtr)();
-CHECK-DAG: int (*FuncStdCallPtr)() __attribute__((stdcall));
-CHECK-DAG: int (*FuncFastCallPtr)() __attribute__((fastcall));
-CHECK-DAG: int (*FuncVectorCallPtr)() __attribute__((vectorcall));
-CHECK-DAG: int (S::*FuncThisCallPtr)() __attribute__((thiscall));
Index: lldb/test/Shell/SymbolFile/PDB/calling-conventions-x86.test
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/PDB/calling-conventions-x86.test
@@ -0,0 +1,19 @@
+REQUIRES: system-windows, lld, (target-x86 || target-x86_64) 
+RUN: %build --compiler=clang-cl --arch=32 --nodefaultlib --output=%t.exe %S/Inputs/CallingConventionsTest.cpp \
+RUN:  && lldb-test symbols -dump-ast %t.exe | FileCheck --check-prefix 32BIT-CHECK %s
+RUN: %build --compiler=clang-cl --arch=64 --nodefaultlib --output=%t.exe %S/Inputs/CallingConventionsTest.cpp \
+RUN:  && lldb-test symbols -dump-ast %t.exe | FileCheck --check-prefix 64BIT-CHECK %s
+
+64BIT-CHECK: Module: {{.*}}
+64BIT-CHECK-DAG: int (*FuncCCallPtr)();
+64BIT-CHECK-DAG: int (*FuncStdCallPtr)();
+64BIT-CHECK-DAG: int (*FuncFastCallPtr)();
+64BIT-CHECK-DAG: int (*FuncVectorCallPtr)() __attribute__((vectorcall));
+64BIT-CHECK-DAG: int (S::*FuncThisCallPtr)(

[Lldb-commits] [PATCH] D128221: [LLDB] Add Arm64 CodeView to LLDB regnum mapping

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid added inline comments.



Comment at: 
lldb/source/Plugins/SymbolFile/NativePDB/CodeViewRegisterMapping.cpp:51
+gpr_w27_arm64, // ARM64_W27, 37)
+gpr_w28_arm64, // ARM64_W28, 38)
+LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM,

DavidSpickett wrote:
> Is there a reason W29/W30/WZR are missing here?
So this is mapping between LLDB and CodeView register numbering. LLDB register 
numbering ignores W29, W30 and WZR in favor of FP, LR and SP. Ref 
./lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h



Comment at: 
lldb/source/Plugins/SymbolFile/NativePDB/CodeViewRegisterMapping.cpp:88
+gpr_sp_arm64,  // ARM64_SP, 81)
+LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM,
+LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM,

DavidSpickett wrote:
> ARM64_ZR here? Though they're the same encoding in the instructions so it 
> probably doesn't change much.
Here again LLDB register numbering considers GPR 31 as SP although AARM suggest 
it can be used as ZR. hence the choice of LLDB specific encoding.


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

https://reviews.llvm.org/D128221

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

[Lldb-commits] [PATCH] D128639: [lldb] [llgs] Fix premature server exit if multiprocess+nonstop

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added inline comments.
This revision is now accepted and ready to land.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:3876
-  // the server.
-  if (m_inferior_prev_state == eStateExited) {
 m_exit_now = true;

I have a feeling that, in the multi-process world, any reference to this 
variable is going to be a bug. It'd be best if it didn't exist.


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

https://reviews.llvm.org/D128639

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

[Lldb-commits] [PATCH] D127755: [lldb] [llgs] Add test for resuming via c in multiprocess scenarios

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

mgorny added a comment.

Thanks for letting me know. I'll set up an AArch64 development environment in a 
minute and try to figure it out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127755

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

[Lldb-commits] [PATCH] D50304: [lldb] Fix thread step until to not set breakpoint(s) on incorrect line numbers

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

The problem is most likely due to the arm64 version having slightly different 
line number sequences due to different instruction scheduling. You might be 
able to gain some insight just by comparing the debug line content, without 
actually running it.

It might be helpful to make the test more hermetic by removing the calls to 
printf (a library function coming from the environment).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D50304

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

[Lldb-commits] [PATCH] D128638: [lldb] [llgs] Add base nonstop fork/vfork tests

[Pavel Labath via Phabricator via lldb-commits]

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

In D128638#3611932 , @mgorny wrote:

> BTW this test is getting a bit long as well but I don't have a good idea how 
> to split it, except for just moving some functions into a second file and 
> making some reusable base class.

I don't know if that's necessary right now, but the general approach seems fine 
to me.


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

https://reviews.llvm.org/D128638

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

[Lldb-commits] [PATCH] D127755: [lldb] [llgs] Add test for resuming via c in multiprocess scenarios

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

mgorny added a comment.

Ok, it seems that LLDB is hitting some deadloop on `brk #0xf000` in code. I've 
filed https://github.com/llvm/llvm-project/issues/56268 for it and I'm going to 
skip the tests relying on traps.

I'm going to try if I manage to get 32-bit ARM container to work as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127755

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

[Lldb-commits] [PATCH] D128678: [LLDB] Add PDB/calling-conventions.test for Arm/Windows

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

It seems like this is not actually running the code. Can we make it such that 
these tests are conditional on the appropriate llvm targets being enabled 
(instead of the host being of a specific type)?


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

https://reviews.llvm.org/D128678

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

[Lldb-commits] [PATCH] D128410: [lldb] Add a testcase for nested exceptions on Windows

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D128410#3609185 , @mstorsjo wrote:

> In D128410#3608190 , @labath wrote:
>
>> In D128410#3604927 , @alvinhochun 
>> wrote:
>>
>>> It may be possible to stuff a pointer to an `EXCEPTION_RECORD` into another 
>>> `EXCEPTION_RECORD` and use `RtlRaiseException` to generate the exception, 
>>> but you'll have to test how it actually works.
>>
>> That would be pretty cool.
>
> Yeah - I guess it's two separate kinds of testcases; this one would be more 
> of a macro-testcase, "does this real-world case work - whichever way lldb 
> happens to handle it" (nested exception or not?) while that would be more of 
> a clinical unit test for specifically testing nested exceptions.

That's true. However, if I had to choose between the two, I would always go for 
the one with the fewest moving parts. Lldb has a lot of problems with 
reproducibility of tests, so I am always looking for ways to make tests more 
specific.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128410

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

[Lldb-commits] [PATCH] D128264: [lldb/dyld-posix] Avoid reading the module list in inconsistent states

[Pavel Labath via Phabricator via lldb-commits]

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

@mgorny, ping. Do you maybe want to try this out on freebsd?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128264

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

[Lldb-commits] [PATCH] D127999: [lldb] fix stepping through POSIX trampolines

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.

Cool, thanks. So do you have commit access, or you need someone to commit this 
for you?


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

https://reviews.llvm.org/D127999

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

[Lldb-commits] [PATCH] D127999: [lldb] fix stepping through POSIX trampolines

[Michael Daniels via Phabricator via lldb-commits]

mdaniels added a comment.

I do not have commit access, so I would need someone to commit for me.


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

https://reviews.llvm.org/D127999

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

[Lldb-commits] [lldb] a1df636 - [lldb] [test] Skip llgs tests broken due to #56268 on aarch64

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-06-28T16:24:38+02:00
New Revision: a1df636a8b51a660d58708d82a5bc060ee5f57d3

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

LOG: [lldb] [test] Skip llgs tests broken due to #56268 on aarch64

Sponsored by: The FreeBSD Foundation

Added: 


Modified: 
lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py

Removed: 




diff  --git a/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py 
b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
index d8c93b84d0ce5..aa97ccd514583 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -317,44 +317,64 @@ def resume_one_test(self, run_order, use_vCont=False):
 ], True)
 self.expect_gdbremote_sequence()
 
+@expectedFailureAll(archs=["aarch64"],
+
bugnumber="https://github.com/llvm/llvm-project/issues/56268";)
 @add_test_categories(["fork"])
 def test_c_parent(self):
 self.resume_one_test(run_order=["parent", "parent"])
 
+@expectedFailureAll(archs=["aarch64"],
+
bugnumber="https://github.com/llvm/llvm-project/issues/56268";)
 @add_test_categories(["fork"])
 def test_c_child(self):
 self.resume_one_test(run_order=["child", "child"])
 
+@expectedFailureAll(archs=["aarch64"],
+
bugnumber="https://github.com/llvm/llvm-project/issues/56268";)
 @add_test_categories(["fork"])
 def test_c_parent_then_child(self):
 self.resume_one_test(run_order=["parent", "parent", "child", "child"])
 
+@expectedFailureAll(archs=["aarch64"],
+
bugnumber="https://github.com/llvm/llvm-project/issues/56268";)
 @add_test_categories(["fork"])
 def test_c_child_then_parent(self):
 self.resume_one_test(run_order=["child", "child", "parent", "parent"])
 
+@expectedFailureAll(archs=["aarch64"],
+
bugnumber="https://github.com/llvm/llvm-project/issues/56268";)
 @add_test_categories(["fork"])
 def test_c_interspersed(self):
 self.resume_one_test(run_order=["parent", "child", "parent", "child"])
 
+@expectedFailureAll(archs=["aarch64"],
+
bugnumber="https://github.com/llvm/llvm-project/issues/56268";)
 @add_test_categories(["fork"])
 def test_vCont_parent(self):
 self.resume_one_test(run_order=["parent", "parent"], use_vCont=True)
 
+@expectedFailureAll(archs=["aarch64"],
+
bugnumber="https://github.com/llvm/llvm-project/issues/56268";)
 @add_test_categories(["fork"])
 def test_vCont_child(self):
 self.resume_one_test(run_order=["child", "child"], use_vCont=True)
 
+@expectedFailureAll(archs=["aarch64"],
+
bugnumber="https://github.com/llvm/llvm-project/issues/56268";)
 @add_test_categories(["fork"])
 def test_vCont_parent_then_child(self):
 self.resume_one_test(run_order=["parent", "parent", "child", "child"],
  use_vCont=True)
 
+@expectedFailureAll(archs=["aarch64"],
+
bugnumber="https://github.com/llvm/llvm-project/issues/56268";)
 @add_test_categories(["fork"])
 def test_vCont_child_then_parent(self):
 self.resume_one_test(run_order=["child", "child", "parent", "parent"],
  use_vCont=True)
 
+@expectedFailureAll(archs=["aarch64"],
+
bugnumber="https://github.com/llvm/llvm-project/issues/56268";)
 @add_test_categories(["fork"])
 def test_vCont_interspersed(self):
 self.resume_one_test(run_order=["parent", "child", "parent", "child"],



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

Re: [Lldb-commits] [PATCH] D128069: [lldb] add SBSection.alignment to python bindings

[David M. Lary via lldb-commits]

On Fri, Jun 17, 2022 at 14:32 Jonas Devlieghere via Phabricator
 wrote:
>
> JDevlieghere added a comment.
>
> In D128069#3592733 , @dmlary wrote:
>
> > I went through the existing tests for SBSection, and there is only one test 
> > case for all the getters & props, and that is for `target_byte_size`.  
> > Based on that lack, and the simplicity of the getter, I didn't think a test 
> > case was warranted here.
>
> I'm less concerned about the getter and the property and more about the 
> underlying functionality. It doesn't look like it's being tested, and adding 
> it to the SB API allows to change that.

Just to clarify, are you asking me to add a test for the underlying
functionality, `Section.GetLog2Align()`?  Or is the goal to add the
test at the SBSection level to ensure we detect if someone ever
changes the underlying api?
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D122988: [BOLT][DWARF] Add version 5 split dwarf support

[Alexander Yermolovich via Phabricator via lldb-commits]

ayermolo added inline comments.



Comment at: bolt/include/bolt/Core/DebugData.h:375-377
+if (Optional DWOId = Unit.getDWOId())
+  return *DWOId;
+return Unit.getOffset();

ayermolo wrote:
> dblaikie wrote:
> > That seems like a somewhat problematic API - returning two very different 
> > kinds of data (the DWO_ID or the unit offset) seems quite easy to misuse 
> > this?
> The idea I had behind this APIS is for it to return unique ID representing 
> the CU. As it applies to .debug_addr. For monolithic case is its offset with 
> .debug_info for Split Dwarf case is its DWO ID. At least in my head viewed in 
> that context the return data is the same. It's just something that uniquely 
> identifies this CU, and logic is encapsulated in it.
> 
@dblaikie What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122988

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

[Lldb-commits] [PATCH] D122988: [BOLT][DWARF] Add version 5 split dwarf support

[Alexander Yermolovich via Phabricator via lldb-commits]

ayermolo added inline comments.



Comment at: bolt/include/bolt/Core/DebugData.h:375-377
+if (Optional DWOId = Unit.getDWOId())
+  return *DWOId;
+return Unit.getOffset();

dblaikie wrote:
> That seems like a somewhat problematic API - returning two very different 
> kinds of data (the DWO_ID or the unit offset) seems quite easy to misuse this?
The idea I had behind this APIS is for it to return unique ID representing the 
CU. As it applies to .debug_addr. For monolithic case is its offset with 
.debug_info for Split Dwarf case is its DWO ID. At least in my head viewed in 
that context the return data is the same. It's just something that uniquely 
identifies this CU, and logic is encapsulated in it.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122988

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

[Lldb-commits] [PATCH] D123580: [libc++] Use bit field for checking if string is in long or short mode

[Mark de Wever via Phabricator via lldb-commits]

Mordante added inline comments.



Comment at: libcxx/utils/gdb/libcxx/printers.py:192
 
 class StdStringPrinter(object):
 """Print a std::string."""

ldionne wrote:
> philnik wrote:
> > labath wrote:
> > > philnik wrote:
> > > > dblaikie wrote:
> > > > > philnik wrote:
> > > > > > jgorbe wrote:
> > > > > > > Mordante wrote:
> > > > > > > > philnik wrote:
> > > > > > > > > Mordante wrote:
> > > > > > > > > > philnik wrote:
> > > > > > > > > > > Mordante wrote:
> > > > > > > > > > > > Does this also break the LLDB pretty printer?
> > > > > > > > > > > Probably. Would be nice to have a test runner for that.
> > > > > > > > > > I already planned to look into that, D97044#3440904 ;-)
> > > > > > > > > Do you know where I would have to look to know what the LLDB 
> > > > > > > > > pretty printers do?
> > > > > > > > Unfortunately no. @jingham seems to be the Data formatter code 
> > > > > > > > owner.
> > > > > > > There was a recent lldb change fixing prettyprinters after a 
> > > > > > > similar change to string: 
> > > > > > > https://github.com/llvm/llvm-project/commit/45428412fd7c9900d3d6ac9803aa7dcf6adfa6fe
> > > > > > > 
> > > > > > > If the gdb prettyprinter needed fixing for this change, chances 
> > > > > > > are that lldb will need a similar update too.
> > > > > > Could someone from #lldb help me figure out what to change in the 
> > > > > > pretty printers? I looked at the file, but I don't really 
> > > > > > understand how it works and TBH I don't really feel like spending a 
> > > > > > lot of time figuring it out. If nobody says anything I'll land this 
> > > > > > in a week.
> > > > > > 
> > > > > > As a side note: it would be really nice if there were a few more 
> > > > > > comments inside `LibCxx.cpp` to explain what happens there. That 
> > > > > > would make fixing the pretty printer a lot easier. The code is 
> > > > > > probably not very hard (at least it doesn't look like it), but I am 
> > > > > > looking for a few things that I can't find and I have no idea what 
> > > > > > some of the things mean.
> > > > > Looks like something around 
> > > > > https://github.com/llvm/llvm-project/blob/2e6ac54cf48aa04f7b05c382c33135b16d3f01ea/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp#L597
> > > > >  (& the similar masking in the `else` block a few lines down) - I 
> > > > > guess a specific lookup for the new field would be needed, rather 
> > > > > than the bitmasking.
> > > > Yes, but what do the numbers in `size_mode_locations` mean? Why is 
> > > > there no checking if it's big or little endian? Is it unsupported 
> > > > maybe? Does it work because of something else? Is there a reason that 
> > > > `g_data_name` exists instead of comparing directly? Should I add 
> > > > another layout style or should I just update the code for the new 
> > > > layout?
> > > > I don't know anything about the LLDB codebase, so I don't understand 
> > > > the code and I don't know how I should change it.
> > > I don't think there's been any official policy decision either way, but 
> > > historically we haven't been asking libc++ authors to update lldb pretty 
> > > printers -- we would just fix them up on the lldb side when we noticed 
> > > the change. The thing that has changed recently is that google started 
> > > relying (and testing) more on lldb, which considerably shortened the time 
> > > it takes to notice this change, and also makes it difficult for some 
> > > people to make progress while we are in this state. But I don't think 
> > > that means that updating the pretty printer is suddenly your 
> > > responsibility.
> > > 
> > > As for your questions, I'll try to answer them as best as I can:
> > > > what do the numbers in size_mode_locations mean?
> > > These are the indexes of fields in the string object. For some reason 
> > > (unknown to me), the pretty printer uses indexes rather than field names 
> > > for its work. Prompted by the previous patch, I've been trying to change 
> > > that, but I haven't done it yet, as I was trying to improve the testing 
> > > story (more on that later).
> > > > Why is there no checking if it's big or little endian? Is it 
> > > > unsupported maybe?
> > > Most likely yes. Although most parts of lldb support big endian, I am not 
> > > aware of anyone testing it on a regular basis, so it's quite likely that 
> > > a lot of things are in fact broken.
> > > > Is there a reason that g_data_name exists instead of comparing directly?
> > > LLDB uses a global string pool, so this is an attempt to reduce the 
> > > number of string pool queries. The pattern is not consistently used 
> > > everywhere, and overall, I wouldn't be too worried about it.
> > > > Should I add another layout style or should I just update the code for 
> > > > the new layout?
> > > As the pretty printers ship with lldb, they are expected to support not 
> > > just the current format, but also the past ones (within reason). This is 
> > > what makes 

[Lldb-commits] [PATCH] D123580: [libc++] Use bit field for checking if string is in long or short mode

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: libcxx/utils/gdb/libcxx/printers.py:192
 
 class StdStringPrinter(object):
 """Print a std::string."""

Mordante wrote:
> ldionne wrote:
> > philnik wrote:
> > > labath wrote:
> > > > philnik wrote:
> > > > > dblaikie wrote:
> > > > > > philnik wrote:
> > > > > > > jgorbe wrote:
> > > > > > > > Mordante wrote:
> > > > > > > > > philnik wrote:
> > > > > > > > > > Mordante wrote:
> > > > > > > > > > > philnik wrote:
> > > > > > > > > > > > Mordante wrote:
> > > > > > > > > > > > > Does this also break the LLDB pretty printer?
> > > > > > > > > > > > Probably. Would be nice to have a test runner for that.
> > > > > > > > > > > I already planned to look into that, D97044#3440904 ;-)
> > > > > > > > > > Do you know where I would have to look to know what the 
> > > > > > > > > > LLDB pretty printers do?
> > > > > > > > > Unfortunately no. @jingham seems to be the Data formatter 
> > > > > > > > > code owner.
> > > > > > > > There was a recent lldb change fixing prettyprinters after a 
> > > > > > > > similar change to string: 
> > > > > > > > https://github.com/llvm/llvm-project/commit/45428412fd7c9900d3d6ac9803aa7dcf6adfa6fe
> > > > > > > > 
> > > > > > > > If the gdb prettyprinter needed fixing for this change, chances 
> > > > > > > > are that lldb will need a similar update too.
> > > > > > > Could someone from #lldb help me figure out what to change in the 
> > > > > > > pretty printers? I looked at the file, but I don't really 
> > > > > > > understand how it works and TBH I don't really feel like spending 
> > > > > > > a lot of time figuring it out. If nobody says anything I'll land 
> > > > > > > this in a week.
> > > > > > > 
> > > > > > > As a side note: it would be really nice if there were a few more 
> > > > > > > comments inside `LibCxx.cpp` to explain what happens there. That 
> > > > > > > would make fixing the pretty printer a lot easier. The code is 
> > > > > > > probably not very hard (at least it doesn't look like it), but I 
> > > > > > > am looking for a few things that I can't find and I have no idea 
> > > > > > > what some of the things mean.
> > > > > > Looks like something around 
> > > > > > https://github.com/llvm/llvm-project/blob/2e6ac54cf48aa04f7b05c382c33135b16d3f01ea/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp#L597
> > > > > >  (& the similar masking in the `else` block a few lines down) - I 
> > > > > > guess a specific lookup for the new field would be needed, rather 
> > > > > > than the bitmasking.
> > > > > Yes, but what do the numbers in `size_mode_locations` mean? Why is 
> > > > > there no checking if it's big or little endian? Is it unsupported 
> > > > > maybe? Does it work because of something else? Is there a reason that 
> > > > > `g_data_name` exists instead of comparing directly? Should I add 
> > > > > another layout style or should I just update the code for the new 
> > > > > layout?
> > > > > I don't know anything about the LLDB codebase, so I don't understand 
> > > > > the code and I don't know how I should change it.
> > > > I don't think there's been any official policy decision either way, but 
> > > > historically we haven't been asking libc++ authors to update lldb 
> > > > pretty printers -- we would just fix them up on the lldb side when we 
> > > > noticed the change. The thing that has changed recently is that google 
> > > > started relying (and testing) more on lldb, which considerably 
> > > > shortened the time it takes to notice this change, and also makes it 
> > > > difficult for some people to make progress while we are in this state. 
> > > > But I don't think that means that updating the pretty printer is 
> > > > suddenly your responsibility.
> > > > 
> > > > As for your questions, I'll try to answer them as best as I can:
> > > > > what do the numbers in size_mode_locations mean?
> > > > These are the indexes of fields in the string object. For some reason 
> > > > (unknown to me), the pretty printer uses indexes rather than field 
> > > > names for its work. Prompted by the previous patch, I've been trying to 
> > > > change that, but I haven't done it yet, as I was trying to improve the 
> > > > testing story (more on that later).
> > > > > Why is there no checking if it's big or little endian? Is it 
> > > > > unsupported maybe?
> > > > Most likely yes. Although most parts of lldb support big endian, I am 
> > > > not aware of anyone testing it on a regular basis, so it's quite likely 
> > > > that a lot of things are in fact broken.
> > > > > Is there a reason that g_data_name exists instead of comparing 
> > > > > directly?
> > > > LLDB uses a global string pool, so this is an attempt to reduce the 
> > > > number of string pool queries. The pattern is not consistently used 
> > > > everywhere, and overall, I wouldn't be too worried about it.
> > > > > Should I add another layout style or should I just update the code 
> > > > > for the new lay

[Lldb-commits] [PATCH] D122988: [BOLT][DWARF] Add version 5 split dwarf support

[Alexander Yermolovich via Phabricator via lldb-commits]

ayermolo added inline comments.



Comment at: bolt/include/bolt/Core/DebugData.h:375-377
+if (Optional DWOId = Unit.getDWOId())
+  return *DWOId;
+return Unit.getOffset();

dblaikie wrote:
> ayermolo wrote:
> > ayermolo wrote:
> > > dblaikie wrote:
> > > > That seems like a somewhat problematic API - returning two very 
> > > > different kinds of data (the DWO_ID or the unit offset) seems quite 
> > > > easy to misuse this?
> > > The idea I had behind this APIS is for it to return unique ID 
> > > representing the CU. As it applies to .debug_addr. For monolithic case is 
> > > its offset with .debug_info for Split Dwarf case is its DWO ID. At least 
> > > in my head viewed in that context the return data is the same. It's just 
> > > something that uniquely identifies this CU, and logic is encapsulated in 
> > > it.
> > > 
> > @dblaikie What do you think?
> Could you use the offset consistently? The debug_addr section is always in 
> the executable - the offset of the skeleton CU would be unique?
In theory yes, if I always use offset of the CU/Skeleton CU in the main binary. 
For that I still need to do a check of CU that is passed in is DWO CU, get it 
skeleton CU, then get it's offset. At the end what we get is offset all the 
time.  Is it really better? Sure we always getting an offset, but intent of 
this API is not really get an offset but the unique ID of the CU. Are you 
concern that offset and DWO ID will collide?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122988

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

[Lldb-commits] [PATCH] D122988: [BOLT][DWARF] Add version 5 split dwarf support

[Maksim Panchenko via Phabricator via lldb-commits]

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

Awesome!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122988

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

[Lldb-commits] [PATCH] D125860: [clang] Only use major version in resource dir

[Timm Bäder via Phabricator via lldb-commits]

tbaeder created this revision.
tbaeder added a reviewer: tstellar.
Herald added a subscriber: mgorny.
Herald added a reviewer: bollu.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added subscribers: llvm-commits, openmp-commits, lldb-commits, 
Sanitizers, cfe-commits, MaskRay.
Herald added projects: clang, Sanitizers, LLDB, OpenMP, LLVM.

As discussed in 
https://discourse.llvm.org/t/should-we-continue-embed-the-full-llvm-version-in-lib-clang/62094/8

Release note update still missing of course.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125860

Files:
  clang/include/clang/Basic/Version.inc.in
  clang/lib/Driver/Driver.cpp
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Tooling/CMakeLists.txt
  clang/runtime/CMakeLists.txt
  compiler-rt/cmake/base-config-ix.cmake
  lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
  lldb/unittests/Expression/ClangParserTest.cpp
  llvm/cmake/modules/LLVMExternalProjectUtils.cmake
  openmp/runtime/src/CMakeLists.txt
  polly/lib/External/isl/interface/extract_interface.cc

Index: polly/lib/External/isl/interface/extract_interface.cc
===
--- polly/lib/External/isl/interface/extract_interface.cc
+++ polly/lib/External/isl/interface/extract_interface.cc
@@ -109,7 +109,7 @@
 	llvm::cl::value_desc("name"));
 
 static const char *ResourceDir =
-	CLANG_PREFIX "/lib/clang/" CLANG_VERSION_STRING;
+CLANG_PREFIX "/lib/clang/" CLANG_VERSION_MAJOR_STRING;
 
 /* Does decl have an attribute of the following form?
  *
Index: openmp/runtime/src/CMakeLists.txt
===
--- openmp/runtime/src/CMakeLists.txt
+++ openmp/runtime/src/CMakeLists.txt
@@ -353,7 +353,7 @@
   set(LIBOMP_HEADERS_INSTALL_PATH "${CMAKE_INSTALL_INCLUDEDIR}")
 else()
   string(REGEX MATCH "[0-9]+\\.[0-9]+(\\.[0-9]+)?" CLANG_VERSION ${PACKAGE_VERSION})
-  set(LIBOMP_HEADERS_INSTALL_PATH "${OPENMP_INSTALL_LIBDIR}/clang/${CLANG_VERSION}/include")
+  set(LIBOMP_HEADERS_INSTALL_PATH "${OPENMP_INSTALL_LIBDIR}/clang/${CLANG_VERSION_MAJOR}/include")
 endif()
 if(WIN32)
   install(TARGETS omp RUNTIME DESTINATION "${CMAKE_INSTALL_BINDIR}")
Index: llvm/cmake/modules/LLVMExternalProjectUtils.cmake
===
--- llvm/cmake/modules/LLVMExternalProjectUtils.cmake
+++ llvm/cmake/modules/LLVMExternalProjectUtils.cmake
@@ -259,7 +259,7 @@
 if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
   string(REGEX MATCH "[0-9]+\\.[0-9]+(\\.[0-9]+)?" CLANG_VERSION
  ${PACKAGE_VERSION})
-  set(resource_dir "${LLVM_LIBRARY_DIR}/clang/${CLANG_VERSION}")
+  set(resource_dir "${LLVM_LIBRARY_DIR}/clang/${CLANG_VERSION_MAJOR}")
   set(flag_types ASM C CXX MODULE_LINKER SHARED_LINKER EXE_LINKER)
   foreach(type ${flag_types})
 set(${type}_flag -DCMAKE_${type}_FLAGS=-resource-dir=${resource_dir})
Index: lldb/unittests/Expression/ClangParserTest.cpp
===
--- lldb/unittests/Expression/ClangParserTest.cpp
+++ lldb/unittests/Expression/ClangParserTest.cpp
@@ -37,10 +37,12 @@
 TEST_F(ClangHostTest, ComputeClangResourceDirectory) {
 #if !defined(_WIN32)
   std::string path_to_liblldb = "/foo/bar/lib/";
-  std::string path_to_clang_dir = "/foo/bar/lib" LLDB_LIBDIR_SUFFIX "/clang/" CLANG_VERSION_STRING;
+  std::string path_to_clang_dir =
+  "/foo/bar/lib" LLDB_LIBDIR_SUFFIX "/clang/" CLANG_VERSION_MAJOR_STRING;
 #else
   std::string path_to_liblldb = "C:\\foo\\bar\\lib";
-  std::string path_to_clang_dir = "C:\\foo\\bar\\lib\\clang\\" CLANG_VERSION_STRING;
+  std::string path_to_clang_dir =
+  "C:\\foo\\bar\\lib\\clang\\" CLANG_VERSION_MAJOR_STRING;
 #endif
   EXPECT_EQ(ComputeClangResourceDir(path_to_liblldb), path_to_clang_dir);
 
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
@@ -54,8 +54,8 @@
 
   static const llvm::StringRef kResourceDirSuffixes[] = {
   // LLVM.org's build of LLDB uses the clang resource directory placed
-  // in $install_dir/lib{,64}/clang/$clang_version.
-  "lib" CLANG_LIBDIR_SUFFIX "/clang/" CLANG_VERSION_STRING,
+  // in $install_dir/lib{,64}/clang/$clang_major_version.
+  "lib" CLANG_LIBDIR_SUFFIX "/clang/" CLANG_VERSION_MAJOR_STRING,
   // swift-lldb uses the clang resource directory copied from swift, which
   // by default is placed in $install_dir/lib{,64}/lldb/clang. LLDB places
   // it there, so we use LLDB_LIBDIR_SUFFIX.
Index: compiler-rt/cmake/base-config-ix.cmake
===
--- compiler-rt/cmake/base-config-ix.cmake
+++ compiler-rt/cmake/base-config-ix.cmake
@@ -43,9 +43,9 @@
   string(REGEX M

[Lldb-commits] [PATCH] D123580: [libc++] Use bit field for checking if string is in long or short mode

[Mark de Wever via Phabricator via lldb-commits]

Mordante added inline comments.



Comment at: libcxx/utils/gdb/libcxx/printers.py:192
 
 class StdStringPrinter(object):
 """Print a std::string."""

labath wrote:
> Mordante wrote:
> > ldionne wrote:
> > > philnik wrote:
> > > > labath wrote:
> > > > > philnik wrote:
> > > > > > dblaikie wrote:
> > > > > > > philnik wrote:
> > > > > > > > jgorbe wrote:
> > > > > > > > > Mordante wrote:
> > > > > > > > > > philnik wrote:
> > > > > > > > > > > Mordante wrote:
> > > > > > > > > > > > philnik wrote:
> > > > > > > > > > > > > Mordante wrote:
> > > > > > > > > > > > > > Does this also break the LLDB pretty printer?
> > > > > > > > > > > > > Probably. Would be nice to have a test runner for 
> > > > > > > > > > > > > that.
> > > > > > > > > > > > I already planned to look into that, D97044#3440904 ;-)
> > > > > > > > > > > Do you know where I would have to look to know what the 
> > > > > > > > > > > LLDB pretty printers do?
> > > > > > > > > > Unfortunately no. @jingham seems to be the Data formatter 
> > > > > > > > > > code owner.
> > > > > > > > > There was a recent lldb change fixing prettyprinters after a 
> > > > > > > > > similar change to string: 
> > > > > > > > > https://github.com/llvm/llvm-project/commit/45428412fd7c9900d3d6ac9803aa7dcf6adfa6fe
> > > > > > > > > 
> > > > > > > > > If the gdb prettyprinter needed fixing for this change, 
> > > > > > > > > chances are that lldb will need a similar update too.
> > > > > > > > Could someone from #lldb help me figure out what to change in 
> > > > > > > > the pretty printers? I looked at the file, but I don't really 
> > > > > > > > understand how it works and TBH I don't really feel like 
> > > > > > > > spending a lot of time figuring it out. If nobody says anything 
> > > > > > > > I'll land this in a week.
> > > > > > > > 
> > > > > > > > As a side note: it would be really nice if there were a few 
> > > > > > > > more comments inside `LibCxx.cpp` to explain what happens 
> > > > > > > > there. That would make fixing the pretty printer a lot easier. 
> > > > > > > > The code is probably not very hard (at least it doesn't look 
> > > > > > > > like it), but I am looking for a few things that I can't find 
> > > > > > > > and I have no idea what some of the things mean.
> > > > > > > Looks like something around 
> > > > > > > https://github.com/llvm/llvm-project/blob/2e6ac54cf48aa04f7b05c382c33135b16d3f01ea/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp#L597
> > > > > > >  (& the similar masking in the `else` block a few lines down) - I 
> > > > > > > guess a specific lookup for the new field would be needed, rather 
> > > > > > > than the bitmasking.
> > > > > > Yes, but what do the numbers in `size_mode_locations` mean? Why is 
> > > > > > there no checking if it's big or little endian? Is it unsupported 
> > > > > > maybe? Does it work because of something else? Is there a reason 
> > > > > > that `g_data_name` exists instead of comparing directly? Should I 
> > > > > > add another layout style or should I just update the code for the 
> > > > > > new layout?
> > > > > > I don't know anything about the LLDB codebase, so I don't 
> > > > > > understand the code and I don't know how I should change it.
> > > > > I don't think there's been any official policy decision either way, 
> > > > > but historically we haven't been asking libc++ authors to update lldb 
> > > > > pretty printers -- we would just fix them up on the lldb side when we 
> > > > > noticed the change. The thing that has changed recently is that 
> > > > > google started relying (and testing) more on lldb, which considerably 
> > > > > shortened the time it takes to notice this change, and also makes it 
> > > > > difficult for some people to make progress while we are in this 
> > > > > state. But I don't think that means that updating the pretty printer 
> > > > > is suddenly your responsibility.
> > > > > 
> > > > > As for your questions, I'll try to answer them as best as I can:
> > > > > > what do the numbers in size_mode_locations mean?
> > > > > These are the indexes of fields in the string object. For some reason 
> > > > > (unknown to me), the pretty printer uses indexes rather than field 
> > > > > names for its work. Prompted by the previous patch, I've been trying 
> > > > > to change that, but I haven't done it yet, as I was trying to improve 
> > > > > the testing story (more on that later).
> > > > > > Why is there no checking if it's big or little endian? Is it 
> > > > > > unsupported maybe?
> > > > > Most likely yes. Although most parts of lldb support big endian, I am 
> > > > > not aware of anyone testing it on a regular basis, so it's quite 
> > > > > likely that a lot of things are in fact broken.
> > > > > > Is there a reason that g_data_name exists instead of comparing 
> > > > > > directly?
> > > > > LLDB uses a global string pool, so this is an attempt to reduce the 
> > > > > number of string pool queries. The pattern i

[Lldb-commits] [PATCH] D62732: [RISCV] Add SystemV ABI

[Luís Marques via Phabricator via lldb-commits]

luismarques added a comment.

In D62732#3157524 , @kasper81 wrote:

> Hi Luis, are you planning on adding plugin architecture support (in 
> `lldb/source/Plugins/Architecture`) as part of this work?

I commandeered this patch from Simon Cook, the original author, but I haven't 
been a good steward. For a while I seemed to always have something higher 
priority in my queue and eventually I kinda stopped thinking about it. So don't 
rely on my plans for this patch. Feel free to adopt it and continue this line 
of work. My apologies to the community.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62732

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

[Lldb-commits] [PATCH] D121876: [BOLT][DWARF] Implement monolithic DWARF5

[Amir Ayupov via Phabricator via lldb-commits]

Amir added a comment.

Hi,
The test started to fail on Ubuntu buildbot: 
https://lab.llvm.org/buildbot/#/builders/215/builds/6376

  # PRECHECK: include_directories[ 0] = .debug_line_str[0x]
  ^
  :8:12: note: scanning from here
   version: 5
 ^
  :30:1: note: possible intended match here
  include_directories[ 0] = .debug_line_str[0x0009] = "."

Can you please take a look?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121876

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

[Lldb-commits] [PATCH] D121876: [BOLT][DWARF] Implement monolithic DWARF5

[Alexander Yermolovich via Phabricator via lldb-commits]

ayermolo added a comment.

In D121876#3542766 , @Amir wrote:

> Hi,
> The test started to fail on Ubuntu buildbot: 
> https://lab.llvm.org/buildbot/#/builders/215/builds/6376
>
>   # PRECHECK: include_directories[ 0] = .debug_line_str[0x]
>   ^
>   :8:12: note: scanning from here
>version: 5
>  ^
>   :30:1: note: possible intended match here
>   include_directories[ 0] = .debug_line_str[0x0009] = "."
>
> Can you please take a look?

Fixed in: D126733 . Was due to LLD change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121876

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

[Lldb-commits] [PATCH] D125860: [clang] Only use major version in resource dir

[Timm Bäder via Phabricator via lldb-commits]

tbaeder updated this revision to Diff 435850.
Herald added a subscriber: Enna1.

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

https://reviews.llvm.org/D125860

Files:
  clang/include/clang/Basic/Version.inc.in
  clang/lib/Driver/Driver.cpp
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Tooling/CMakeLists.txt
  clang/runtime/CMakeLists.txt
  compiler-rt/cmake/base-config-ix.cmake
  lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
  lldb/unittests/Expression/ClangParserTest.cpp
  llvm/cmake/modules/LLVMExternalProjectUtils.cmake
  openmp/runtime/src/CMakeLists.txt
  polly/lib/External/isl/interface/extract_interface.cc

Index: polly/lib/External/isl/interface/extract_interface.cc
===
--- polly/lib/External/isl/interface/extract_interface.cc
+++ polly/lib/External/isl/interface/extract_interface.cc
@@ -109,7 +109,7 @@
 	llvm::cl::value_desc("name"));
 
 static const char *ResourceDir =
-	CLANG_PREFIX "/lib/clang/" CLANG_VERSION_STRING;
+CLANG_PREFIX "/lib/clang/" CLANG_VERSION_MAJOR_STRING;
 
 /* Does decl have an attribute of the following form?
  *
Index: openmp/runtime/src/CMakeLists.txt
===
--- openmp/runtime/src/CMakeLists.txt
+++ openmp/runtime/src/CMakeLists.txt
@@ -352,8 +352,8 @@
 if(${OPENMP_STANDALONE_BUILD})
   set(LIBOMP_HEADERS_INSTALL_PATH "${CMAKE_INSTALL_INCLUDEDIR}")
 else()
-  string(REGEX MATCH "[0-9]+\\.[0-9]+(\\.[0-9]+)?" CLANG_VERSION ${PACKAGE_VERSION})
-  set(LIBOMP_HEADERS_INSTALL_PATH "${OPENMP_INSTALL_LIBDIR}/clang/${CLANG_VERSION}/include")
+  string(REGEX MATCH "^[0-9]+" CLANG_VERSION_MAJOR ${PACKAGE_VERSION})
+  set(LIBOMP_HEADERS_INSTALL_PATH "${OPENMP_INSTALL_LIBDIR}/clang/${CLANG_VERSION_MAJOR}/include")
 endif()
 if(WIN32)
   install(TARGETS omp RUNTIME DESTINATION "${CMAKE_INSTALL_BINDIR}")
Index: llvm/cmake/modules/LLVMExternalProjectUtils.cmake
===
--- llvm/cmake/modules/LLVMExternalProjectUtils.cmake
+++ llvm/cmake/modules/LLVMExternalProjectUtils.cmake
@@ -255,9 +255,9 @@
 set(llvm_config_path ${LLVM_CONFIG_PATH})
 
 if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
-  string(REGEX MATCH "[0-9]+\\.[0-9]+(\\.[0-9]+)?" CLANG_VERSION
+  string(REGEX MATCH "^[0-9]+" CLANG_VERSION_MAJOR
  ${PACKAGE_VERSION})
-  set(resource_dir "${LLVM_LIBRARY_DIR}/clang/${CLANG_VERSION}")
+  set(resource_dir "${LLVM_LIBRARY_DIR}/clang/${CLANG_VERSION_MAJOR}")
   set(flag_types ASM C CXX MODULE_LINKER SHARED_LINKER EXE_LINKER)
   foreach(type ${flag_types})
 set(${type}_flag -DCMAKE_${type}_FLAGS=-resource-dir=${resource_dir})
Index: lldb/unittests/Expression/ClangParserTest.cpp
===
--- lldb/unittests/Expression/ClangParserTest.cpp
+++ lldb/unittests/Expression/ClangParserTest.cpp
@@ -37,10 +37,12 @@
 TEST_F(ClangHostTest, ComputeClangResourceDirectory) {
 #if !defined(_WIN32)
   std::string path_to_liblldb = "/foo/bar/lib/";
-  std::string path_to_clang_dir = "/foo/bar/lib" LLDB_LIBDIR_SUFFIX "/clang/" CLANG_VERSION_STRING;
+  std::string path_to_clang_dir =
+  "/foo/bar/lib" LLDB_LIBDIR_SUFFIX "/clang/" CLANG_VERSION_MAJOR_STRING;
 #else
   std::string path_to_liblldb = "C:\\foo\\bar\\lib";
-  std::string path_to_clang_dir = "C:\\foo\\bar\\lib\\clang\\" CLANG_VERSION_STRING;
+  std::string path_to_clang_dir =
+  "C:\\foo\\bar\\lib\\clang\\" CLANG_VERSION_MAJOR_STRING;
 #endif
   EXPECT_EQ(ComputeClangResourceDir(path_to_liblldb), path_to_clang_dir);
 
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
@@ -55,7 +55,7 @@
   static const llvm::StringRef kResourceDirSuffixes[] = {
   // LLVM.org's build of LLDB uses the clang resource directory placed
   // in $install_dir/lib{,64}/clang/$clang_version.
-  "lib" CLANG_LIBDIR_SUFFIX "/clang/" CLANG_VERSION_STRING,
+  "lib" CLANG_LIBDIR_SUFFIX "/clang/" CLANG_VERSION_MAJOR_STRING,
   // swift-lldb uses the clang resource directory copied from swift, which
   // by default is placed in $install_dir/lib{,64}/lldb/clang. LLDB places
   // it there, so we use LLDB_LIBDIR_SUFFIX.
Index: compiler-rt/cmake/base-config-ix.cmake
===
--- compiler-rt/cmake/base-config-ix.cmake
+++ compiler-rt/cmake/base-config-ix.cmake
@@ -38,14 +38,16 @@
 
 if (LLVM_TREE_AVAILABLE)
   # Compute the Clang version from the LLVM version.
-  # FIXME: We should be able to reuse CLANG_VERSION variable calculated
+  # FIXME: We should be able to reuse CLANG_VERSION_MAJOR variable calculated
   #in Clang cmake files, instead of copying the rules h

[Lldb-commits] [PATCH] D127224: [flang][OpenMP] Lowering support for atomic capture

[Nimish Mishra via Phabricator via lldb-commits]

NimishMishra updated this revision to Diff 435060.
NimishMishra added a comment.
Herald added subscribers: cfe-commits, llvm-commits, libc-commits, 
libcxx-commits, openmp-commits, lldb-commits, Sanitizers, jsji, Enna1, kosarev, 
mravishankar, jsilvanus, mattd, gchakrabarti, hsmhsm, pmatos, asb, 
pcwang-thead, yota9, ayermolo, awarzynski, arjunp, luke957, asavonic, 
carlosgalvezp, abrachet, Groverkss, armkevincheng, ormris, foad, jsmolens, 
eric-k256, mstorsjo, frasercrmck, ecnelises, ThomasRaoux, vkmr, martong, 
phosek, kerbowa, csigg, luismarques, apazos, sameer.abuasal, pengfei, 
s.egerton, dmgreen, Jim, jocewei, PkmX, arphaman, the_o, brucehoult, 
MartinMosbeck, rogfer01, steven_wu, atanasyan, mgrang, edward-jones, zzheng, 
MaskRay, jrtc27, gbedwell, niosHD, cryptoad, sabuasal, simoncook, johnrusso, 
rbar, fedor.sergeev, kbarton, aheejin, hiraditya, jgravelle-google, 
arichardson, sbc100, mgorny, nhaehnle, jvesely, nemanjai, sdardis, dylanmckay, 
jyknight, dschuff, arsenm, jholewinski.
Herald added a reviewer: bollu.
Herald added a reviewer: andreadb.
Herald added a reviewer: alexander-shaposhnikov.
Herald added a reviewer: rupprecht.
Herald added a reviewer: jhenderson.
Herald added a reviewer: antiagainst.
Herald added a reviewer: nicolasvasilache.
Herald added a reviewer: aartbik.
Herald added a reviewer: ftynse.
Herald added a reviewer: aartbik.
Herald added a reviewer: aartbik.
Herald added a reviewer: sjarus.
Herald added a reviewer: zuban32.
Herald added a reviewer: rafauler.
Herald added a reviewer: Amir.
Herald added a reviewer: maksfb.
Herald added projects: clang, Sanitizers, LLDB, libc++, libc-project, 
libc++abi, libunwind, LLVM, lld-macho, clang-tools-extra.
Herald added a reviewer: libc++.
Herald added a reviewer: libc++abi.
Herald added a reviewer: libunwind.
Herald added a reviewer: lld-macho.

[flang][OpenMP] Added lowering support for atomic capture


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127224

Files:
  bolt/lib/Rewrite/DWARFRewriter.cpp
  bolt/lib/Rewrite/MachORewriteInstance.cpp
  bolt/lib/Rewrite/RewriteInstance.cpp
  clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/pseudo/lib/cxx.bnf
  clang-tools-extra/pseudo/test/cxx/empty-member-spec.cpp
  clang-tools-extra/pseudo/test/cxx/keyword.cpp
  clang-tools-extra/pseudo/test/cxx/parameter-decl-clause.cpp
  clang-tools-extra/pseudo/test/cxx/predefined-identifier.cpp
  clang-tools-extra/pseudo/test/cxx/template-empty-type-parameter.cpp
  clang-tools-extra/pseudo/test/cxx/unsized-array.cpp
  clang-tools-extra/pseudo/test/glr.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  clang/lib/AST/Decl.cpp
  clang/lib/AST/Mangle.cpp
  clang/lib/Basic/Targets/OSTargets.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
  clang/lib/CodeGen/SanitizerMetadata.cpp
  clang/lib/CodeGen/SanitizerMetadata.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Headers/unwind.h
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
  clang/lib/StaticAnalyzer/Core/ConstraintManager.cpp
  clang/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
  clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  clang/test/Analysis/cast-value-notes.cpp
  clang/test/Analysis/cast-value-state-dump.cpp
  clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
  clang/test/CodeGen/builtins-memcpy-inline.c
  clang/test/CodeGenCXX/externc-used-not-replaced.cpp
  clang/test/Driver/cl-zc.cpp
  clang/test/Driver/sanitizer-ld.c
  clang/test/Driver/windows-exceptions.cpp
  clang/test/Modules/Inputs/gmodules-deduction-guide.h
  clang/test/Modules/gmodules-deduction-guide.cpp
  clang/test/Sema/aarch64-sve-vector-arith-ops.c
  clang/test/Sema/aarch64-sve-vector-bitwise-ops.c
  clang/test/Sema/aarch64-sve-vector-compare-ops.c
  clang/test/Sema/builtins-memcpy-inline.cpp
  clang/test/SemaCXX/aarch64-sve-vector-conditional-op.cpp
  clang/test/SemaCXX/cxx2a-consteval.cpp
  clang/tools/clang-offload-packager/ClangOffloadPackager.cpp
  clang/unittests/Analysis/FlowSensitive/MultiVarConstantPropagationTest.cpp
  clang/unittests/Analysis/FlowSensitive/SingleVarConstantPropagationTest.cpp
  clang/www/c_status.html
  compiler-rt/lib/asan/asan_allocator.h
  compiler-rt/lib/asan/asan_errors.cpp

[Lldb-commits] [PATCH] D125860: [clang] Only use major version in resource dir

[Timm Bäder via Phabricator via lldb-commits]

tbaeder updated this revision to Diff 435876.

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

https://reviews.llvm.org/D125860

Files:
  clang/include/clang/Basic/Version.inc.in
  clang/lib/Driver/Driver.cpp
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Tooling/CMakeLists.txt
  clang/runtime/CMakeLists.txt
  compiler-rt/cmake/base-config-ix.cmake
  lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
  lldb/unittests/Expression/ClangParserTest.cpp
  llvm/cmake/modules/LLVMExternalProjectUtils.cmake
  openmp/runtime/src/CMakeLists.txt
  polly/lib/External/isl/interface/extract_interface.cc

Index: polly/lib/External/isl/interface/extract_interface.cc
===
--- polly/lib/External/isl/interface/extract_interface.cc
+++ polly/lib/External/isl/interface/extract_interface.cc
@@ -109,7 +109,7 @@
 	llvm::cl::value_desc("name"));
 
 static const char *ResourceDir =
-	CLANG_PREFIX "/lib/clang/" CLANG_VERSION_STRING;
+CLANG_PREFIX "/lib/clang/" CLANG_VERSION_MAJOR_STRING;
 
 /* Does decl have an attribute of the following form?
  *
Index: openmp/runtime/src/CMakeLists.txt
===
--- openmp/runtime/src/CMakeLists.txt
+++ openmp/runtime/src/CMakeLists.txt
@@ -352,8 +352,8 @@
 if(${OPENMP_STANDALONE_BUILD})
   set(LIBOMP_HEADERS_INSTALL_PATH "${CMAKE_INSTALL_INCLUDEDIR}")
 else()
-  string(REGEX MATCH "[0-9]+\\.[0-9]+(\\.[0-9]+)?" CLANG_VERSION ${PACKAGE_VERSION})
-  set(LIBOMP_HEADERS_INSTALL_PATH "${OPENMP_INSTALL_LIBDIR}/clang/${CLANG_VERSION}/include")
+  string(REGEX MATCH "^[0-9]+" CLANG_VERSION_MAJOR ${PACKAGE_VERSION})
+  set(LIBOMP_HEADERS_INSTALL_PATH "${OPENMP_INSTALL_LIBDIR}/clang/${CLANG_VERSION_MAJOR}/include")
 endif()
 if(WIN32)
   install(TARGETS omp RUNTIME DESTINATION "${CMAKE_INSTALL_BINDIR}")
Index: llvm/cmake/modules/LLVMExternalProjectUtils.cmake
===
--- llvm/cmake/modules/LLVMExternalProjectUtils.cmake
+++ llvm/cmake/modules/LLVMExternalProjectUtils.cmake
@@ -255,9 +255,9 @@
 set(llvm_config_path ${LLVM_CONFIG_PATH})
 
 if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
-  string(REGEX MATCH "[0-9]+\\.[0-9]+(\\.[0-9]+)?" CLANG_VERSION
+  string(REGEX MATCH "^[0-9]+" CLANG_VERSION_MAJOR
  ${PACKAGE_VERSION})
-  set(resource_dir "${LLVM_LIBRARY_DIR}/clang/${CLANG_VERSION}")
+  set(resource_dir "${LLVM_LIBRARY_DIR}/clang/${CLANG_VERSION_MAJOR}")
   set(flag_types ASM C CXX MODULE_LINKER SHARED_LINKER EXE_LINKER)
   foreach(type ${flag_types})
 set(${type}_flag -DCMAKE_${type}_FLAGS=-resource-dir=${resource_dir})
Index: lldb/unittests/Expression/ClangParserTest.cpp
===
--- lldb/unittests/Expression/ClangParserTest.cpp
+++ lldb/unittests/Expression/ClangParserTest.cpp
@@ -37,10 +37,12 @@
 TEST_F(ClangHostTest, ComputeClangResourceDirectory) {
 #if !defined(_WIN32)
   std::string path_to_liblldb = "/foo/bar/lib/";
-  std::string path_to_clang_dir = "/foo/bar/lib" LLDB_LIBDIR_SUFFIX "/clang/" CLANG_VERSION_STRING;
+  std::string path_to_clang_dir =
+  "/foo/bar/lib" LLDB_LIBDIR_SUFFIX "/clang/" CLANG_VERSION_MAJOR_STRING;
 #else
   std::string path_to_liblldb = "C:\\foo\\bar\\lib";
-  std::string path_to_clang_dir = "C:\\foo\\bar\\lib\\clang\\" CLANG_VERSION_STRING;
+  std::string path_to_clang_dir =
+  "C:\\foo\\bar\\lib\\clang\\" CLANG_VERSION_MAJOR_STRING;
 #endif
   EXPECT_EQ(ComputeClangResourceDir(path_to_liblldb), path_to_clang_dir);
 
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangHost.cpp
@@ -55,7 +55,7 @@
   static const llvm::StringRef kResourceDirSuffixes[] = {
   // LLVM.org's build of LLDB uses the clang resource directory placed
   // in $install_dir/lib{,64}/clang/$clang_version.
-  "lib" CLANG_LIBDIR_SUFFIX "/clang/" CLANG_VERSION_STRING,
+  "lib" CLANG_LIBDIR_SUFFIX "/clang/" CLANG_VERSION_MAJOR_STRING,
   // swift-lldb uses the clang resource directory copied from swift, which
   // by default is placed in $install_dir/lib{,64}/lldb/clang. LLDB places
   // it there, so we use LLDB_LIBDIR_SUFFIX.
Index: compiler-rt/cmake/base-config-ix.cmake
===
--- compiler-rt/cmake/base-config-ix.cmake
+++ compiler-rt/cmake/base-config-ix.cmake
@@ -38,14 +38,14 @@
 
 if (LLVM_TREE_AVAILABLE)
   # Compute the Clang version from the LLVM version.
-  # FIXME: We should be able to reuse CLANG_VERSION variable calculated
+  # FIXME: We should be able to reuse CLANG_VERSION_MAJOR variable calculated
   #in Clang cmake files, instead of copying the rules here.
-  string(REGEX MATCH "[0-9]+

[Lldb-commits] [PATCH] D127684: [NFC] Use `https` instead of `http` in the urls

[Nikolas Klauser via Phabricator via lldb-commits]

philnik added a comment.

Could you do this on a per-project basis? More than 1000 files is way to much 
to sift through.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127684

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

[Lldb-commits] [PATCH] D127684: [NFC] Use `https` instead of `http` in the urls

[Shao-Ce SUN via Phabricator via lldb-commits]

sunshaoce created this revision.
Herald added subscribers: libc-commits, libcxx-commits, jsji, Enna1, 
bzcheeseman, kosarev, jsilvanus, mattd, gchakrabarti, hsmhsm, ThomasRaoux, 
pmatos, asb, ayermolo, awarzynski, sdasgup3, asavonic, carlosgalvezp, 
jeroen.dobbelaere, abrachet, wenzhicui, wrengr, Chia-hungDuan, lebedev.ri, 
ormris, foad, smeenai, dcaballe, cota, mravishankar, teijeong, frasercrmck, 
rdzhabarov, tatianashp, wenlei, okura, jdoerfert, msifontes, jurahul, kuter, 
Kayjukh, grosul1, jvesely, martong, Joonsoo, phosek, kerbowa, liufengdb, 
aartbik, mgester, arpith-jacob, csigg, antiagainst, shauheen, rriddle, 
mehdi_amini, luismarques, apazos, sameer.abuasal, usaxena95, pengfei, 
s.egerton, dmgreen, Jim, asbirlea, mstorsjo, kadircet, jocewei, rupprecht, 
PkmX, arphaman, the_o, brucehoult, MartinMosbeck, rogfer01, steven_wu, 
atanasyan, edward-jones, george.burgess.iv, zzheng, jrtc27, delcypher, niosHD, 
sabuasal, simoncook, johnrusso, rbar, fedor.sergeev, kbarton, hiraditya, 
jgravelle-google, whisperity, arichardson, sbc100, mgorny, nhaehnle, nemanjai, 
sdardis, emaste, dylanmckay, jyknight, dschuff, arsenm.
Herald added a reviewer: deadalnix.
Herald added a reviewer: bollu.
Herald added a reviewer: JDevlieghere.
Herald added a reviewer: lebedev.ri.
Herald added a reviewer: lebedev.ri.
Herald added a reviewer: ldionne.
Herald added a reviewer: jhenderson.
Herald added a reviewer: lebedev.ri.
Herald added a reviewer: aartbik.
Herald added a reviewer: MaskRay.
Herald added a reviewer: sscalpone.
Herald added a reviewer: ftynse.
Herald added a reviewer: aaron.ballman.
Herald added a reviewer: aartbik.
Herald added a reviewer: lebedev.ri.
Herald added a reviewer: lebedev.ri.
Herald added a reviewer: rafauler.
Herald added a reviewer: Amir.
Herald added a reviewer: maksfb.
Herald added projects: libunwind, libc-project, Flang, All.
Herald added a reviewer: libunwind.
sunshaoce requested review of this revision.
Herald added subscribers: cfe-commits, llvm-commits, openmp-commits, 
lldb-commits, Sanitizers, pcwang-thead, yota9, StephenFan, sstefan1, 
stephenneuendorffer, nicolasvasilache, aheejin, jholewinski.
Herald added a reviewer: jdoerfert.
Herald added a reviewer: jdoerfert.
Herald added a reviewer: sstefan1.
Herald added a reviewer: nicolasvasilache.
Herald added projects: clang, Sanitizers, LLDB, libc++, OpenMP, libc++abi, 
MLIR, LLVM, clang-tools-extra.
Herald added a reviewer: libc++.
Herald added a reviewer: libc++abi.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127684

Files:
  .github/PULL_REQUEST_TEMPLATE.md
  .github/workflows/repo-lockdown.yml
  .gitignore
  README.md
  bolt/docs/CMakeLists.txt
  bolt/docs/OptimizingClang.md
  bolt/docs/doxygen.cfg.in
  clang-tools-extra/.gitignore
  clang-tools-extra/README.txt
  clang-tools-extra/clang-doc/Representation.cpp
  clang-tools-extra/clang-doc/assets/clang-doc-default-stylesheet.css
  
clang-tools-extra/clang-include-fixer/find-all-symbols/tool/run-find-all-symbols.py
  clang-tools-extra/clang-include-fixer/tool/clang-include-fixer.el
  clang-tools-extra/clang-include-fixer/tool/clang-include-fixer.py
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/abseil/CleanupCtadCheck.h
  clang-tools-extra/clang-tidy/abseil/DurationAdditionCheck.h
  clang-tools-extra/clang-tidy/abseil/DurationComparisonCheck.h
  clang-tools-extra/clang-tidy/abseil/DurationConversionCastCheck.h
  clang-tools-extra/clang-tidy/abseil/DurationDivisionCheck.h
  clang-tools-extra/clang-tidy/abseil/DurationFactoryFloatCheck.h
  clang-tools-extra/clang-tidy/abseil/DurationFactoryScaleCheck.h
  clang-tools-extra/clang-tidy/abseil/DurationSubtractionCheck.h
  clang-tools-extra/clang-tidy/abseil/DurationUnnecessaryConversionCheck.h
  clang-tools-extra/clang-tidy/abseil/FasterStrsplitDelimiterCheck.h
  clang-tools-extra/clang-tidy/abseil/NoInternalDependenciesCheck.h
  clang-tools-extra/clang-tidy/abseil/NoNamespaceCheck.h
  clang-tools-extra/clang-tidy/abseil/RedundantStrcatCallsCheck.h
  clang-tools-extra/clang-tidy/abseil/StrCatAppendCheck.h
  clang-tools-extra/clang-tidy/abseil/StringFindStrContainsCheck.h
  clang-tools-extra/clang-tidy/abseil/TimeComparisonCheck.h
  clang-tools-extra/clang-tidy/abseil/TimeSubtractionCheck.h
  clang-tools-extra/clang-tidy/abseil/UpgradeDurationConversionsCheck.h
  clang-tools-extra/clang-tidy/add_new_check.py
  clang-tools-extra/clang-tidy/altera/IdDependentBackwardBranchCheck.h
  clang-tools-extra/clang-tidy/altera/KernelNameRestrictionCheck.h
  clang-tools-extra/clang-tidy/altera/SingleWorkItemBarrierCheck.h
  clang-tools-extra/clang-tidy/altera/StructPackAlignCheck.h
  clang-tools-extra/clang-tidy/altera/UnrollLoopsCheck.h
  clang-tools-extra/clang-tidy/android/CloexecAccept4Check.h
  clang-tools-extra/clang-tidy/android/CloexecAcceptCheck.h
  clang-tools-extra/clang-tidy/android/CloexecCreatCheck.h
  clang-tools-extra/clang-tidy/a

[Lldb-commits] [PATCH] D127684: [NFC] Use `https` instead of `http` in the urls

[Eli Friedman via Phabricator via lldb-commits]

efriedma added a comment.

A couple notes skimming the changes:

1. If you're going to update URLs, please verify the new URLs are actually 
valid.
2. Some of the "URLs" you're updating aren't actually URLs. For example, the 
`xmlns` attribute in SVG files must be the exact string written in the 
standard; changing it could make the file unreadable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127684

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

[Lldb-commits] [PATCH] D127741: [Disassembler] Add +all CPU feature to subtargetInfo and use it for disassembling AArch64

[Denis via Phabricator via lldb-commits]

treapster created this revision.
treapster added a reviewer: rafaelauler.
Herald added subscribers: jsji, ayermolo, pengfei, rupprecht, hiraditya, 
kristof.beyls.
Herald added a reviewer: jhenderson.
Herald added a reviewer: MaskRay.
Herald added a reviewer: rafauler.
Herald added a reviewer: Amir.
Herald added a reviewer: maksfb.
Herald added a project: All.
treapster requested review of this revision.
Herald added subscribers: llvm-commits, lldb-commits, yota9, StephenFan.
Herald added projects: LLDB, LLVM.

It think it's reasonable to enable all supported extensions when we are 
disassembling something because we don't want to 
[hardcode](hhtps://reviews.llvm.org/D121999) it in gdb, objdump, bolt and 
whatever other places use disassembler in llvm. The only reason not to do it I 
can think of is if there are conflicting instructions with the same encoding in 
different extensions, but it doesn't seem possible within one archeticture. We 
probably should use +all on x86 and other platforms? I'm not sure how test for 
this should look like, because disassembler is used by at least three tools I 
know of(gdb, objdump, BOLT), so it's not clear where should we put it, and how 
to make sure that all new instructions are added to it. Waiting for your 
feedback.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127741

Files:
  bolt/lib/Core/BinaryContext.cpp
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
  llvm/lib/MC/MCSubtargetInfo.cpp
  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
@@ -1693,6 +1693,10 @@
   if (!MAttrs.empty())
 for (unsigned I = 0; I != MAttrs.size(); ++I)
   Features.AddFeature(MAttrs[I]);
+  else {
+if (Obj->getArch() == llvm::Triple::aarch64)
+  Features.AddFeature("+all");
+  }
 
   std::unique_ptr MRI(
   TheTarget->createMCRegInfo(TripleName));
Index: llvm/lib/MC/MCSubtargetInfo.cpp
===
--- llvm/lib/MC/MCSubtargetInfo.cpp
+++ llvm/lib/MC/MCSubtargetInfo.cpp
@@ -198,6 +198,8 @@
   Help(ProcDesc, ProcFeatures);
 else if (Feature == "+cpuhelp")
   cpuHelp(ProcDesc);
+else if (Feature == "+all")
+  Bits.set();
 else
   ApplyFeatureFlag(Bits, Feature, ProcFeatures);
   }
Index: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
===
--- lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
+++ lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
@@ -1179,16 +1179,9 @@
   features_str += "+dspr2,";
   }
 
-  // If any AArch64 variant, enable latest ISA with all extensions.
+  // If any AArch64 variant, enable all features.
   if (triple.isAArch64()) {
-features_str += "+v9.3a,";
-std::vector features;
-// Get all possible features
-llvm::AArch64::getExtensionFeatures(-1, features);
-features_str += llvm::join(features, ",");
-
-if (triple.getVendor() == llvm::Triple::Apple)
-  cpu = "apple-latest";
+features_str += "+all";
   }
 
   if (triple.isRISCV()) {
Index: bolt/lib/Core/BinaryContext.cpp
===
--- bolt/lib/Core/BinaryContext.cpp
+++ bolt/lib/Core/BinaryContext.cpp
@@ -124,8 +124,7 @@
 break;
   case llvm::Triple::aarch64:
 ArchName = "aarch64";
-FeaturesStr = "+fp-armv8,+neon,+crypto,+dotprod,+crc,+lse,+ras,+rdm,"
-  "+fullfp16,+spe,+fuse-aes,+rcpc";
+FeaturesStr = "+all";
 break;
   default:
 return createStringError(std::errc::not_supported,


Index: llvm/tools/llvm-objdump/llvm-objdump.cpp
===
--- llvm/tools/llvm-objdump/llvm-objdump.cpp
+++ llvm/tools/llvm-objdump/llvm-objdump.cpp
@@ -1693,6 +1693,10 @@
   if (!MAttrs.empty())
 for (unsigned I = 0; I != MAttrs.size(); ++I)
   Features.AddFeature(MAttrs[I]);
+  else {
+if (Obj->getArch() == llvm::Triple::aarch64)
+  Features.AddFeature("+all");
+  }
 
   std::unique_ptr MRI(
   TheTarget->createMCRegInfo(TripleName));
Index: llvm/lib/MC/MCSubtargetInfo.cpp
===
--- llvm/lib/MC/MCSubtargetInfo.cpp
+++ llvm/lib/MC/MCSubtargetInfo.cpp
@@ -198,6 +198,8 @@
   Help(ProcDesc, ProcFeatures);
 else if (Feature == "+cpuhelp")
   cpuHelp(ProcDesc);
+else if (Feature == "+all")
+  Bits.set();
 else
   ApplyFeatureFlag(Bits, Feature, ProcFeatures);
   }
Index: lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
===
--- lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
+++ lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLV

[Lldb-commits] [PATCH] D127741: [Disassembler] Add +all CPU feature to subtargetInfo and use it for disassembling AArch64

[Denis via Phabricator via lldb-commits]

treapster added a comment.

Well, it broke a lot of tests that very explicitly checking that instructions 
are not disassembled without explicitly specified extensions, but i don't quite 
get the point of such tests. If new instructions were added by default in lldb, 
why can't we do the same with objdump?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127741

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

[Lldb-commits] [PATCH] D127684: [NFC] Use `https` instead of `http` in the `LLVM.org` URLs

[Shao-Ce SUN via Phabricator via lldb-commits]

sunshaoce abandoned this revision.
sunshaoce added a comment.

Thanks for comments!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127684

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

[Lldb-commits] [PATCH] D127684: [NFC] Use `https` instead of `http` in the `LLVM.org` URLs

[Jonas Devlieghere via Phabricator via lldb-commits]

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

+1 on breaking this down per sub-project and creating separate patches for 
review.

I sampled the patch and about a quarter or so of the links I tried didn't work, 
even before the change. There's no point in generating churn by changing those, 
but there's also little value in keeping them around, and fixing them is beyond 
the scope of this patch. I'm inclined to say let's leave them be, but I'm 
curious to hear what others think about that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127684

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

[Lldb-commits] [PATCH] D127741: [Disassembler] Add +all CPU feature to subtargetInfo and use it for disassembling AArch64

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added subscribers: labrinea, DavidSpickett.
DavidSpickett added a reviewer: DavidSpickett.
DavidSpickett added a comment.
Herald added a subscriber: Michael137.

> conflicting instructions with the same encoding in different extensions, but 
> it doesn't seem possible within one archeticture

I seem to remember some system register names changing between architecture 
*profiles* not version. @labrinea may know more about that.

> Well, it broke a lot of tests that were checking that instructions are not 
> disassembled without explicitly specified extensions, but i don't quite get 
> the point of such tests.

Without looking at them in detail I assume they are checking that the proper 
extension name is printed when it says "requires bla" and that each instruction 
is properly tied to the correct feature name. This is for when you are 
assembling as opposed to disassembling.

I agree that objdump should have some kind of "max" setting, even a default. 
However there will still need to be a way to test what I referred to.

What might be possible is to write the testing such that it doesn't use 
llvm-objdump. I haven't seen the structure of the tests perhaps this would mean 
duplicating tests all over the place.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127741

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

[Lldb-commits] [PATCH] D127741: [Disassembler] Add +all CPU feature to subtargetInfo and use it for disassembling AArch64

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

Now I think about it, tests could do something like "-mattr=-all" to remove it, 
right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127741

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

[Lldb-commits] [PATCH] D127741: [Disassembler] Add +all CPU feature to subtargetInfo and use it for disassembling AArch64

[Stephen Hines via Phabricator via lldb-commits]

srhines added a comment.

We had some internal Google folks hit this exact issue recently. I don't think 
that the same "default" CPU should be used for debugging tools like 
`llvm-objdump`, and that is the crux of the matter here. Perhaps updating the 
test to specify "generic" as the CPU when passed to `llvm-objdump` for those 
tests is the right pattern? Then the default for disassembly should be to have 
all attributes/features enabled unless the developer specifies on the command 
line that they want a different target.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127741

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

[Lldb-commits] [PATCH] D127741: [Disassembler] Add +all CPU feature to subtargetInfo and use it for disassembling AArch64

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> Why not use objdump? If we only check that instructions get disassembled, it 
> will work fine.

I just remember some tests using llvm-mc to disassemble and some using objdump. 
Istr that using llvm-mc for assemble and disassemble usually meant having two 
test files instead of one. Anyway, not important.

objdump with a generic cpu or with the all feature removed sgtm.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127741

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

[Lldb-commits] [PATCH] D127741: [Disassembler] Add +all CPU feature to subtargetInfo and use it for disassembling AArch64

[Denis via Phabricator via lldb-commits]

treapster added a comment.

For now i'd like to hear more about possible clashing instruction encodings 
from @labrinea, and if it's not a concern, i think we can fix the tests if we 
remove `CHECK-UNKNOWN:` and only call objdump with `CHECK-INST`. What do you 
think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127741

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

[Lldb-commits] [PATCH] D127741: [Disassembler] Add +all CPU feature to subtargetInfo and use it for disassembling AArch64

[Denis via Phabricator via lldb-commits]

treapster added a comment.

In D127741#3583559 , @DavidSpickett 
wrote:

> 



> I agree that objdump should have some kind of "max" setting, even a default. 
> However there will still need to be a way to test what I referred to.

As you said, ` instruction requires bla` is part of assembler, not 
disassembler, so it is not affected by the patch. The problem here is that 
tests do `CHECK-UNKNOWN: 83 a9 91 c0 ` which basically checks that 
objdump cannot disassemble instruction that was assembled in the very same 
test. If we change that, we can make `+all` default attribute for objdump and 
it will disassemble everything the same way GNU objdump and lldb do.

> What might be possible is to write the testing such that it doesn't use 
> llvm-objdump. I haven't seen the structure of the tests perhaps this would 
> mean duplicating tests all over the place.

Why not use objdump? If we only check that instructions get disassembled, it 
will work fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127741

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

[Lldb-commits] [PATCH] D127741: [Disassembler] Add +all CPU feature to subtargetInfo and use it for disassembling AArch64

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added a comment.

I like the idea of `all` for llvm-objdump, but it should be a separate change, 
with `-mattr=-all` tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127741

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

[Lldb-commits] [PATCH] D128166: ManagedStatic: Destroy from destructor

[Nicolai Hähnle via Phabricator via lldb-commits]

nhaehnle created this revision.
Herald added a reviewer: deadalnix.
Herald added subscribers: bzcheeseman, ayermolo, sdasgup3, wenzhicui, wrengr, 
dcaballe, cota, teijeong, rdzhabarov, tatianashp, msifontes, jurahul, Kayjukh, 
grosul1, jvesely, Joonsoo, liufengdb, aartbik, mgester, arpith-jacob, 
antiagainst, shauheen, rriddle, mehdi_amini, mstorsjo, hiraditya, mgorny.
Herald added a reviewer: bollu.
Herald added a reviewer: MaskRay.
Herald added a reviewer: rafauler.
Herald added a reviewer: Amir.
Herald added a reviewer: maksfb.
Herald added a project: All.
nhaehnle requested review of this revision.
Herald added subscribers: lldb-commits, cfe-commits, yota9, StephenFan, 
stephenneuendorffer, nicolasvasilache.
Herald added projects: clang, LLDB, MLIR, LLVM.

This allows LLVM to be safely loaded and unloaded as a shared library
without leaking memory:

Consider the scenario where shared library Foo links against LLVM as
a shared library, and Foo may be loaded and unloaded multiple times
in the same process. Should Foo call llvm_shutdown or not? If it does
not, and LLVM is also loaded and unloaded multiple times, then
memory is leaked. If it does call llvm_shutdown and LLVM *isn't*
also unloaded, the state of LLVM's global statics is corrupted because
while the ManagedStatics may be re-initialized, *other* global
constructors aren't re-run.

Keep in mind that Foo itself may use ManagedStatic. If Foo is unloaded
but LLVM isn't, those statics should be destroyed.

The safe solution is to skip llvm_shutdown() altogether and destroy
managed statics from their destructor.

LLD relies on being able to call _exit() and still get some side-effects
of managed static destructors. We make this expectation much more
explicit.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128166

Files:
  bolt/tools/driver/llvm-bolt.cpp
  bolt/tools/merge-fdata/merge-fdata.cpp
  clang/include/clang/Frontend/CompilerInstance.h
  clang/tools/clang-repl/ClangRepl.cpp
  clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp
  clang/unittests/Interpreter/InterpreterTest.cpp
  clang/utils/TableGen/TableGen.cpp
  libclc/utils/prepare-builtins.cpp
  lld/Common/ErrorHandler.cpp
  lldb/tools/driver/Driver.cpp
  lldb/tools/lldb-test/lldb-test.cpp
  lldb/unittests/Utility/LogTest.cpp
  lldb/utils/TableGen/LLDBTableGen.cpp
  llvm/docs/ProgrammersManual.rst
  llvm/examples/BrainF/BrainFDriver.cpp
  llvm/examples/HowToUseJIT/HowToUseJIT.cpp
  llvm/include/llvm-c/Core.h
  llvm/include/llvm/ADT/Statistic.h
  llvm/include/llvm/PassRegistry.h
  llvm/include/llvm/Support/DynamicLibrary.h
  llvm/include/llvm/Support/FastShutdown.h
  llvm/include/llvm/Support/InitLLVM.h
  llvm/include/llvm/Support/ManagedStatic.h
  llvm/lib/IR/Core.cpp
  llvm/lib/IR/Pass.cpp
  llvm/lib/IR/PassRegistry.cpp
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/FastShutdown.cpp
  llvm/lib/Support/InitLLVM.cpp
  llvm/lib/Support/ManagedStatic.cpp
  llvm/lib/Support/Parallel.cpp
  llvm/lib/Support/Statistic.cpp
  llvm/lib/Support/Unix/DynamicLibrary.inc
  llvm/lib/Support/Unix/Signals.inc
  llvm/lib/Support/Windows/DynamicLibrary.inc
  llvm/tools/gold/gold-plugin.cpp
  llvm/tools/llvm-gsymutil/llvm-gsymutil.cpp
  llvm/unittests/ExecutionEngine/ExecutionEngineTest.cpp
  llvm/unittests/Support/DynamicLibrary/DynamicLibraryTest.cpp
  llvm/utils/KillTheDoctor/KillTheDoctor.cpp
  mlir/tools/mlir-vulkan-runner/mlir-vulkan-runner.cpp
  polly/lib/External/isl/interface/extract_interface.cc

Index: polly/lib/External/isl/interface/extract_interface.cc
===
--- polly/lib/External/isl/interface/extract_interface.cc
+++ polly/lib/External/isl/interface/extract_interface.cc
@@ -587,7 +587,6 @@
 
 	delete sema;
 	delete Clang;
-	llvm::llvm_shutdown();
 
 	if (Diags.hasErrorOccurred())
 		return EXIT_FAILURE;
Index: mlir/tools/mlir-vulkan-runner/mlir-vulkan-runner.cpp
===
--- mlir/tools/mlir-vulkan-runner/mlir-vulkan-runner.cpp
+++ mlir/tools/mlir-vulkan-runner/mlir-vulkan-runner.cpp
@@ -61,7 +61,6 @@
 }
 
 int main(int argc, char **argv) {
-  llvm::llvm_shutdown_obj x;
   registerPassManagerCLOptions();
 
   llvm::InitLLVM y(argc, argv);
Index: llvm/utils/KillTheDoctor/KillTheDoctor.cpp
===
--- llvm/utils/KillTheDoctor/KillTheDoctor.cpp
+++ llvm/utils/KillTheDoctor/KillTheDoctor.cpp
@@ -297,7 +297,6 @@
   // Print a stack trace if we signal out.
   sys::PrintStackTraceOnErrorSignal(argv[0]);
   PrettyStackTraceProgram X(argc, argv);
-  llvm_shutdown_obj Y;  // Call llvm_shutdown() on exit.
 
   ToolName = argv[0];
 
Index: llvm/unittests/Support/DynamicLibrary/DynamicLibraryTest.cpp
===
--- llvm/unittests/Support/DynamicLibrary/DynamicLibraryTest.cpp
+++ llvm/unittests/Support/DynamicLibrary/DynamicLibrary

[Lldb-commits] [PATCH] D128465: [WIP] Zstandard as a second compression method to LLVM

[Cole Kissane via Phabricator via lldb-commits]

ckissane created this revision.
ckissane added reviewers: phosek, leonardchan.
ckissane added a project: LLVM.
Herald added subscribers: Enna1, abrachet, wenlei, usaxena95, kadircet, 
arphaman, hiraditya, arichardson, mgorny, emaste.
Herald added a reviewer: alexander-shaposhnikov.
Herald added a reviewer: rupprecht.
Herald added a reviewer: jhenderson.
Herald added a reviewer: MaskRay.
Herald added projects: Flang, All.
ckissane requested review of this revision.
Herald added subscribers: cfe-commits, llvm-commits, lldb-commits, Sanitizers, 
StephenFan, jdoerfert.
Herald added projects: clang, Sanitizers, LLDB, clang-tools-extra.

All work in progress POC implementation of the [RFC] Zstandard as a second 
compression method to LLVM.

Note:

- An interesting unresolved caveat crc32 implementation in the compression 
namespace requires ZLIB even if only enabling ZSTD




Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128465

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/index/Serialization.cpp
  clang-tools-extra/clangd/test/lit.site.cfg.py.in
  clang-tools-extra/clangd/unittests/SerializationTests.cpp
  clang/cmake/caches/Apple-stage1.cmake
  clang/cmake/caches/Apple-stage2.cmake
  clang/cmake/caches/Fuchsia-stage2.cmake
  clang/cmake/caches/Fuchsia.cmake
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  compiler-rt/lib/sanitizer_common/symbolizer/scripts/build_symbolizer.sh
  compiler-rt/test/lit.common.configured.in
  flang/CMakeLists.txt
  lld/ELF/CMakeLists.txt
  lld/ELF/Driver.cpp
  lld/ELF/InputSection.cpp
  lldb/source/Plugins/Process/gdb-remote/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/cmake/config-ix.cmake
  llvm/cmake/modules/LLVMConfig.cmake.in
  llvm/include/llvm/Config/llvm-config.h.cmake
  llvm/include/llvm/Support/Compression.h
  llvm/lib/MC/ELFObjectWriter.cpp
  llvm/lib/ObjCopy/ELF/ELFObject.cpp
  llvm/lib/Object/Decompressor.cpp
  llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
  llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp
  llvm/lib/ProfileData/InstrProf.cpp
  llvm/lib/ProfileData/SampleProfReader.cpp
  llvm/lib/ProfileData/SampleProfWriter.cpp
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/Compression.cpp
  llvm/tools/llvm-mc/llvm-mc.cpp
  llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
  llvm/unittests/ProfileData/InstrProfTest.cpp
  llvm/unittests/Support/CompressionTest.cpp
  utils/bazel/llvm_configs/llvm-config.h.cmake

Index: utils/bazel/llvm_configs/llvm-config.h.cmake
===
--- utils/bazel/llvm_configs/llvm-config.h.cmake
+++ utils/bazel/llvm_configs/llvm-config.h.cmake
@@ -92,6 +92,9 @@
 /* Define if zlib compression is available */
 #cmakedefine01 LLVM_ENABLE_ZLIB
 
+/* Define if zstd compression is available */
+#cmakedefine01 LLVM_ENABLE_ZSTD
+
 /* Define if LLVM was built with a dependency to the libtensorflow dynamic library */
 #cmakedefine LLVM_HAVE_TF_API
 
Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -18,6 +18,7 @@
 #include "gtest/gtest.h"
 
 using namespace llvm;
+using namespace compression;
 
 namespace {
 
@@ -70,4 +71,53 @@
 
 #endif
 
+#if LLVM_ENABLE_ZSTD
+
+void TestZstdCompression(StringRef Input, int Level) {
+  SmallString<32> Compressed;
+  SmallString<32> Uncompressed;
+
+  zstd::compress(Input, Compressed, Level);
+
+  // Check that uncompressed buffer is the same as original.
+  Error E = zstd::uncompress(Compressed, Uncompressed, Input.size());
+  consumeError(std::move(E));
+
+  EXPECT_EQ(Input, Uncompressed);
+  if (Input.size() > 0) {
+// Uncompression fails if expected length is too short.
+E = zstd::uncompress(Compressed, Uncompressed, Input.size() - 1);
+EXPECT_EQ("zstd error: Z_BUF_ERROR", llvm::toString(std::move(E)));
+  }
+}
+
+TEST(CompressionTest, Zstd) {
+  TestZstdCompression("", zstd::DefaultCompression);
+
+  TestZstdCompression("hello, world!", zstd::NoCompression);
+  TestZstdCompression("hello, world!", zstd::BestSizeCompression);
+  TestZstdCompression("hello, world!", zstd::BestSpeedCompression);
+  TestZstdCompression("hello, world!", zstd::DefaultCompression);
+
+  const size_t kSize = 1024;
+  char BinaryData[kSize];
+  for (size_t i = 0; i < kSize; ++i) {
+BinaryData[i] = i & 255;
+  }
+  StringRef BinaryDataStr(BinaryData, kSize);
+
+  TestZstdCompression(BinaryDataStr, zstd::NoCompression);
+  TestZstdCompression(BinaryDataStr, zstd::BestSizeCompression);
+  TestZstdCompression(BinaryDataStr, zstd::BestSpeedCompression);
+  TestZstdCompression(BinaryDataStr, zstd::DefaultCompression);
+}
+
+TEST(CompressionTest, ZstdCRC32) {
+  EXPECT_EQ(
+  0x414FA339U,
+  zstd::crc32(StringR

[Lldb-commits] [PATCH] D128250: [LLDB][RISCV]Add initial support for lldb-server.

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

I'll give this a more thorough read later just one comment for now.

In the meantime, what's your plan for testing this config going forward? This 
patch would be fine as the first of a series but I'd like to see the follow ups 
that make it usable.

Will there be a build bot? You could at least enable building lldb on an 
existing riscv bot, while you iron out all the inevitable test suite issues.




Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.h:9
+
+#if defined(__riscv) || __riscv_xlen == 64
+

Should this be `&&`? I'm assuming that `__riscv` is also defined for riscv32.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128250

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

[Lldb-commits] [PATCH] D128250: [LLDB][RISCV]Add initial support for lldb-server.

[Emmmer S via Phabricator via lldb-commits]

Emmmer added a comment.

In D128250#3598223 , @DavidSpickett 
wrote:

> I'll give this a more thorough read later just one comment for now.
>
> In the meantime, what's your plan for testing this config going forward? This 
> patch would be fine as the first of a series but I'd like to see the follow 
> ups that make it usable.
>
> Will there be a build bot? You could at least enable building lldb on an 
> existing riscv bot, while you iron out all the inevitable test suite issues.

To use gdbserver by lldb requires the participation of lldb-server, so my 
current plan is to allow lldb to co-op with gdbserver, then let lldb be able to 
use lldb-server, pass the unit tests, and finally add riscv32 support. 
We use `Open Build Server` as the riscv build bot 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128250

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

[Lldb-commits] [PATCH] D128250: [LLDB][RISCV]Add initial support for lldb-server.

[Emmmer S via Phabricator via lldb-commits]

Emmmer created this revision.
Emmmer added reviewers: DavidSpickett, craig.topper.
Emmmer added a project: LLDB.
Herald added subscribers: sunshaoce, VincentWu, luke957, StephenFan, vkmr, 
frasercrmck, evandro, luismarques, apazos, sameer.abuasal, JDevlieghere, 
s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, 
rogfer01, edward-jones, zzheng, jrtc27, shiva0217, kito-cheng, niosHD, 
sabuasal, simoncook, johnrusso, rbar, asb, arichardson, mgorny.
Herald added a project: All.
Emmmer requested review of this revision.
Herald added subscribers: lldb-commits, pcwang-thead, eopXD, MaskRay.

I added some initial support for lldb-server on **riscv64** platform, now it 
can be built on riscv64.
But when I use lldb for debugging, it reports:

  error: 'A' packet returned an error: -1

I tried to fix this error with gdb, but the debugging process was too slow, so 
I'm sorry for not being able to provide a test patch for verification of my 
commit.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128250

Files:
  lldb/source/Plugins/Process/Linux/CMakeLists.txt
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.h
  lldb/source/Plugins/Process/Utility/CMakeLists.txt
  lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_riscv64.cpp
  lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_riscv64.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_riscv64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_riscv64.h
  lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h
  lldb/source/Utility/RISCV64_DWARF_Registers.h

Index: lldb/source/Utility/RISCV64_DWARF_Registers.h
===
--- /dev/null
+++ lldb/source/Utility/RISCV64_DWARF_Registers.h
@@ -0,0 +1,56 @@
+//===-- RISCV64_DWARF_Registers.h ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_SOURCE_UTILITY_RISCV64_DWARF_REGISTERS_H
+#define LLDB_SOURCE_UTILITY_RISCV64_DWARF_REGISTERS_H
+
+#include "lldb/lldb-private.h"
+
+namespace riscv64_dwarf {
+
+enum {
+  x0 = 0,
+  x1,
+  x2,
+  x3,
+  x4,
+  x5,
+  x6,
+  x7,
+  x8,
+  x9,
+  x10,
+  x11,
+  x12,
+  x13,
+  x14,
+  x15,
+  x16,
+  x17,
+  x18,
+  x19,
+  x20,
+  x21,
+  x22,
+  x23,
+  x24,
+  x25,
+  x26,
+  x27,
+  x28,
+  x29,
+  x30,
+  x31,
+  ra = x1,
+  sp = x2,
+  fp = x8,
+};
+
+} // namespace riscv64_dwarf
+
+#endif // LLDB_SOURCE_UTILITY_RISCV64_DWARF_REGISTERS_H
Index: lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h
===
--- /dev/null
+++ lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h
@@ -0,0 +1,156 @@
+//===-- RegisterInfos_riscv64.h -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifdef DECLARE_REGISTER_INFOS_RISCV64_STRUCT
+
+#include 
+
+#include "lldb/lldb-defines.h"
+#include "lldb/lldb-enumerations.h"
+#include "lldb/lldb-private.h"
+
+#include "Utility/RISCV64_DWARF_Registers.h"
+
+#ifndef GPR_OFFSET
+#error GPR_OFFSET must be defined before including this header file
+#endif
+
+#ifndef GPR_OFFSET_NAME
+#error GPR_OFFSET_NAME must be defined before including this header file
+#endif
+
+#ifndef FPU_OFFSET
+#error FPU_OFFSET must be defined before including this header file
+#endif
+
+#ifndef FPU_OFFSET_NAME
+#error FPU_OFFSET_NAME must be defined before including this header file
+#endif
+
+#ifndef DBG_OFFSET_NAME
+#error DBG_OFFSET_NAME must be defined before including this header file
+#endif
+
+#ifndef DEFINE_DBG
+#error DEFINE_DBG must be defined before including this header file
+#endif
+
+// Offsets for a little-endian layout of the register context
+#define GPR_W_PSEUDO_REG_ENDIAN_OFFSET 0
+#define FPU_S_PSEUDO_REG_ENDIAN_OFFSET 0
+#define FPU_D_PSEUDO_REG_ENDIAN_OFFSET 0
+
+enum {
+  gpr_x0 = 0,
+  gpr_x1,
+  gpr_x2,
+  gpr_x3,
+  gpr_x4,
+  gpr_x5,
+  gpr_x6,
+  gpr_x7,
+  gpr_x8,
+  gpr_x9,
+  gpr_x10,
+  gpr_x11,
+  gpr_x12,
+  gpr_x13,
+  gpr_x14,
+  gpr_x15,
+  gpr_x16,
+  gpr_x17,
+  gpr_x18,
+  gpr_x19,
+  gpr_x20,
+  gpr_x21,
+  gpr_x22,
+  gpr_x23,
+  gpr_x24,
+  gpr_x25,
+  gpr_x26,
+  gpr_x27,
+  gpr_x28,
+  gpr_x29,
+  gpr_x30,
+  gpr_x31,
+  gpr_ra = gpr_x1,
+  gpr_sp = gpr_x2,
+  gpr_fp = gpr_x8,
+
+  k_num_registers
+};
+
+// Generates register kinds array

[Lldb-commits] [PATCH] D128250: [LLDB][RISCV]Add initial support for lldb-server.

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> I tried to fix this error with gdb, but the debugging process was too slow, 
> so I'm sorry for not being able to provide a test patch for verification of 
> my commit.

Oh and if you want help with this please ask on discord/discourse. This stuff 
is a bit tedious with so many exchanges on first connection.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128250

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

[Lldb-commits] [PATCH] D128250: [LLDB][RISCV]Add initial support for lldb-server.

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

Overall this looks fine. Without the ability to test it I don't think we would 
accept it yet, so I didn't look into the details too much.

> To use gdbserver by lldb requires the participation of lldb-server, so my 
> current plan is to allow lldb to co-op with gdbserver, then let lldb be able 
> to use lldb-server, pass the unit tests, and finally add riscv32 support.

Sounds good. If those lldb changes can be tested by existing bots then they can 
go in first. For anything else it's probably better to build a tree of your 
changes and when you've got something that ends in a testable lldb-server then 
put them into review.

I appreciate it's difficult to test a lot of the enabling code e.g. the 
breakpoint code in this change. That'll be exercised by the test suite in 
general once more things are working. So it would be fine to say here is a 
series that ends in an lldb/lldb-server that can run a large amount of existing 
tests.

If you find anything non-riscv specific e.g. some gdb protocol difference that 
can go into review whenever. Anything to improve lldb-gdb compatibility is 
welcome.

> We use Open Build Server as the riscv build bot.

I'm not familiar with this infrastructure. In a future where your changes are 
in llvm `main` will developers be notified when they break the riscv lldb build?




Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.cpp:249
+bool NativeRegisterContextLinux_riscv64::IsGPR(unsigned reg) const {
+  if (GetRegisterInfo().GetRegisterSetFromRegisterIndex(reg) ==
+  RegisterInfoPOSIX_riscv64::GPRegSet)

```
  return GetRegisterInfo().GetRegisterSetFromRegisterIndex(reg) == 
RegisterInfoPOSIX_riscv64::GPRegSet;
```



Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.cpp:256
+bool NativeRegisterContextLinux_riscv64::IsFPR(unsigned reg) const {
+  return false;
+}

Seems like this should do a lookup?



Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.cpp:323
+
+void NativeRegisterContextLinux_riscv64::InvalidateAllRegisters() {
+  m_gpr_is_valid = false;

Minor thing but if you call this in the constructor it's one less place to `= 
false` all the things.



Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.cpp:341
+llvm::Error NativeRegisterContextLinux_riscv64::ReadHardwareDebugInfo() {
+  return llvm::Error::success();
+}

This also a placeholder I assume (I'm not sure what this does for other targets 
tbh).



Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.cpp:345
+NativeRegisterContextLinux_riscv64::DREGType hwbType) {
+  return llvm::Error::success();
+}

This is a placeholder?



Comment at: 
lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_riscv64.cpp:19
+constexpr uint32_t g_enable_bit = 1;
+// PAC (bits 2:1): 0b10
+constexpr uint32_t g_pac_bits = (2 << 1);

and PAC means? (please add it to the comment)



Comment at: 
lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_riscv64.cpp:34
+  Log *log = GetLog(LLDBLog::Breakpoints);
+  llvm::Error error = ReadHardwareDebugInfo();
+  if (error) {

I don't see an implementation of this yet, not one that actually reads 
anything. Intentional? It'll just see 0 breakpoints I guess.



Comment at: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_riscv64.cpp:51
+  switch (target_arch.GetMachine()) {
+  case llvm::Triple::riscv32:
+  case llvm::Triple::riscv64:

So the way lldb-server works, at least at the moment, is that one binary does 
one architecture. So for example the AArch64 linux build doesn't handle AArch32 
mode, you'd need to use an AArch32 built binary for that.

Maybe you can make this work with riscv but I'd start with just looking for 
riscv64 here.



Comment at: lldb/source/Utility/RISCV64_DWARF_Registers.h:54
+
+} // namespace riscv64_dwarf
+

No DWARF numbers for the FPU regs?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128250

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

[Lldb-commits] [PATCH] D128250: [LLDB][RISCV]Add initial support for lldb-server.

[Emmmer S via Phabricator via lldb-commits]

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



Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.h:9
+
+#if defined(__riscv) || __riscv_xlen == 64
+

DavidSpickett wrote:
> Should this be `&&`? I'm assuming that `__riscv` is also defined for riscv32.
WIP


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128250

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

[Lldb-commits] [PATCH] D128250: [LLDB][RISCV]Add initial support for lldb-server.

[Emmmer S via Phabricator via lldb-commits]

Emmmer updated this revision to Diff 439955.
Emmmer marked an inline comment as done.
Emmmer added a comment.

This patch change:

- add `lldb/source/Plugins/Architecture/RISCV64`
- add `lldb/source/Plugins/Architecture/RISCV32`
- update `lldb/source/Utility/ArchSpec.cpp`
- remove 
`lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_riscv64`

> No DWARF numbers for the FPU regs?

I would like to add support for GPR first, after making sure they work well, 
FPU regs are a simple copy-paste.

> AArch64 linux build doesn't handle AArch32 mode, you'd need to use an AArch32 
> built binary for that.

Thanks for your suggestion, I split them out.

> and PAC means? (please add it to the comment)
> I don't see an implementation of this yet, not one that actually reads 
> anything. Intentional? It'll just see 0 breakpoints I guess.

At first, I referenced the AArch64 code to add riscv64 support because their 
arch is similar, but I overlooked that riscv64 doesn't have a Debug reg yet, so 
it was a mistake on my part.


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

https://reviews.llvm.org/D128250

Files:
  lldb/source/Plugins/Architecture/CMakeLists.txt
  lldb/source/Plugins/Architecture/RISCV32/ArchitectureRISCV32.cpp
  lldb/source/Plugins/Architecture/RISCV32/ArchitectureRISCV32.h
  lldb/source/Plugins/Architecture/RISCV32/CMakeLists.txt
  lldb/source/Plugins/Architecture/RISCV64/ArchitectureRISCV64.cpp
  lldb/source/Plugins/Architecture/RISCV64/ArchitectureRISCV64.h
  lldb/source/Plugins/Architecture/RISCV64/CMakeLists.txt
  lldb/source/Plugins/Process/Linux/CMakeLists.txt
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.h
  lldb/source/Plugins/Process/Utility/CMakeLists.txt
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_riscv64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_riscv64.h
  lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h
  lldb/source/Utility/ArchSpec.cpp
  lldb/source/Utility/RISCV64_DWARF_Registers.h

Index: lldb/source/Utility/RISCV64_DWARF_Registers.h
===
--- /dev/null
+++ lldb/source/Utility/RISCV64_DWARF_Registers.h
@@ -0,0 +1,56 @@
+//===-- RISCV64_DWARF_Registers.h ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_SOURCE_UTILITY_RISCV64_DWARF_REGISTERS_H
+#define LLDB_SOURCE_UTILITY_RISCV64_DWARF_REGISTERS_H
+
+#include "lldb/lldb-private.h"
+
+namespace riscv64_dwarf {
+
+enum {
+  x0 = 0,
+  x1,
+  x2,
+  x3,
+  x4,
+  x5,
+  x6,
+  x7,
+  x8,
+  x9,
+  x10,
+  x11,
+  x12,
+  x13,
+  x14,
+  x15,
+  x16,
+  x17,
+  x18,
+  x19,
+  x20,
+  x21,
+  x22,
+  x23,
+  x24,
+  x25,
+  x26,
+  x27,
+  x28,
+  x29,
+  x30,
+  x31,
+  ra = x1,
+  sp = x2,
+  fp = x8,
+};
+
+} // namespace riscv64_dwarf
+
+#endif // LLDB_SOURCE_UTILITY_RISCV64_DWARF_REGISTERS_H
Index: lldb/source/Utility/ArchSpec.cpp
===
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -724,6 +724,8 @@
   case llvm::Triple::systemz:
   case llvm::Triple::xcore:
   case llvm::Triple::arc:
+  case llvm::Triple::riscv32:
+  case llvm::Triple::riscv64:
 return false;
   }
 }
@@ -1376,6 +1378,20 @@
 }
 break;
 
+  case ArchSpec::eCore_riscv32:
+if (!enforce_exact_match) {
+  if (core2 == ArchSpec::eCore_riscv32)
+return true;
+}
+break;
+
+  case ArchSpec::eCore_riscv64:
+if (!enforce_exact_match) {
+  if (core2 == ArchSpec::eCore_riscv64)
+return true;
+}
+break;
+
   default:
 break;
   }
Index: lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h
===
--- /dev/null
+++ lldb/source/Plugins/Process/Utility/RegisterInfos_riscv64.h
@@ -0,0 +1,153 @@
+//===-- RegisterInfos_riscv64.h -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifdef DECLARE_REGISTER_INFOS_RISCV64_STRUCT
+
+#include 
+
+#include "lldb/lldb-defines.h"
+#include "lldb/lldb-enumerations.h"
+#include "lldb/lldb-private.h"
+
+#include "Utility/RISCV64_DWARF_Registers.h"
+
+#ifndef GPR_OFFSET
+#error GPR_OFFSET must be defined before including this header file
+#endif
+
+#ifndef GPR_OFFSET_NAME
+#error GPR_OFFSET_NAME must be defined bef

[Lldb-commits] [PATCH] D128612: RISC-V big-endian support implementation

[Guy Benyei via Phabricator via lldb-commits]

gbenyei created this revision.
gbenyei added a reviewer: asb.
gbenyei added projects: clang, LLVM, lld.
Herald added subscribers: Enna1, sunshaoce, VincentWu, luke957, StephenFan, 
vkmr, frasercrmck, luismarques, apazos, sameer.abuasal, s.egerton, Jim, ormris, 
jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, 
zzheng, jrtc27, shiva0217, kito-cheng, niosHD, sabuasal, simoncook, johnrusso, 
rbar, hiraditya, arichardson, mgorny, emaste.
Herald added a reviewer: alexander-shaposhnikov.
Herald added a reviewer: rupprecht.
Herald added a reviewer: jhenderson.
Herald added a reviewer: MaskRay.
Herald added a reviewer: aaron.ballman.
Herald added a project: All.
gbenyei requested review of this revision.
Herald added subscribers: lldb-commits, cfe-commits, pcwang-thead.
Herald added a project: LLDB.

Implement riscv32be and riscv64be targets.
The RISC-V big- and bi-endian targets are discussed in the RISC-V spec  Version 
20191213, but some aspects, like ABI are still unclear.
The instruction encoding is little endian in both big- and little-endian modes. 
ISA spec Volume 1 1.15: "Instructions are stored in memory as a sequence
of 16-bit little-endian parcels, regardless of memory system endianness".

RISC-V Big-endian cores are already supported by GCC. Where spec is unclear, we 
aim to be compatible with GCC.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128612

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/Basic/Targets.cpp
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/lib/Driver/ToolChains/RISCVToolchain.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  lld/ELF/Arch/RISCV.cpp
  lld/ELF/InputFiles.cpp
  lldb/source/Utility/ArchSpec.cpp
  llvm/cmake/config-ix.cmake
  llvm/cmake/config.guess
  llvm/include/llvm/ADT/Triple.h
  llvm/include/llvm/Object/ELFObjectFile.h
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/lib/ExecutionEngine/JITLink/ELF.cpp
  llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp
  llvm/lib/ExecutionEngine/Orc/EPCIndirectionUtils.cpp
  llvm/lib/ExecutionEngine/Orc/IndirectionUtils.cpp
  llvm/lib/ExecutionEngine/Orc/LazyReexports.cpp
  llvm/lib/Object/RelocationResolver.cpp
  llvm/lib/Support/Triple.cpp
  llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
  llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCAsmInfo.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCTargetDesc.cpp
  llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
  llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
  llvm/lib/Target/RISCV/TargetInfo/RISCVTargetInfo.cpp
  llvm/lib/Target/RISCV/TargetInfo/RISCVTargetInfo.h
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
  llvm/unittests/Object/ELFObjectFileTest.cpp

Index: llvm/unittests/Object/ELFObjectFileTest.cpp
===
--- llvm/unittests/Object/ELFObjectFileTest.cpp
+++ llvm/unittests/Object/ELFObjectFileTest.cpp
@@ -186,10 +186,10 @@
 }
 
 TEST(ELFObjectFileTest, MachineTestForRISCV) {
-  std::array Formats = {"elf32-littleriscv", "elf32-littleriscv",
-  "elf64-littleriscv", "elf64-littleriscv"};
-  std::array Archs = {Triple::riscv32, Triple::riscv32,
-   Triple::riscv64, Triple::riscv64};
+  std::array Formats = {"elf32-littleriscv", "elf32-bigriscv",
+  "elf64-littleriscv", "elf64-bigriscv"};
+  std::array Archs = {Triple::riscv32, Triple::riscv32be,
+   Triple::riscv64, Triple::riscv64be};
   size_t I = 0;
   for (const DataForTest &D : generateData(ELF::EM_RISCV)) {
 checkFormatAndArch(D, Formats[I], Archs[I]);
Index: llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
===
--- llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
+++ llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
@@ -301,6 +301,8 @@
 // RISC-V
 {"elf32-littleriscv", {ELF::EM_RISCV, false, true}},
 {"elf64-littleriscv", {ELF::EM_RISCV, true, true}},
+{"elf32-bigriscv", {ELF::EM_RISCV, false, false}},
+{"elf64-bigriscv", {ELF::EM_RISCV, true, false}},
 // PowerPC
 {"elf32-powerpc", {ELF::EM_PPC, false, false}},
 {"elf32-powerpcle", {ELF::EM_PPC, fa

[Lldb-commits] [PATCH] D128612: RISC-V big-endian support implementation

[Craig Topper via Phabricator via lldb-commits]

craig.topper added inline comments.



Comment at: clang/lib/Basic/Targets/RISCV.h:111
 SizeType = UnsignedInt;
-resetDataLayout("e-m:e-p:32:32-i64:64-n32-S128");
   }

Instead of creating new classes, could we have a branch on the Arch or 
isLittleEndian portion of the triple to decide what to pass to resetDataLayout? 
That's what PPC32TargetInfo does for example.



Comment at: lld/ELF/Arch/RISCV.cpp:160
+  if (config->is64) {
+if (config->isLE)
+  write64le(buf, mainPart->dynamic->getVA());

Is it possible to use write64 instead of write64le/be? It looks like it checks 
the endianness internally.



Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp:628
+  // For big endian cores, data fixup should be swapped.
+  bool swapValue = (Endian == support::big) && isDataFixup(Kind);
   for (unsigned i = 0; i != NumBytes; ++i) {

Capitalize `swapValue`



Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h:34
   const MCTargetOptions &Options)
-  : MCAsmBackend(support::little), STI(STI), OSABI(OSABI), 
Is64Bit(Is64Bit),
+  : MCAsmBackend(IsLittleEndian ? support::little : support::big), 
STI(STI), OSABI(OSABI), Is64Bit(Is64Bit),
 TargetOptions(Options) {

Is this longer than 80 columns?



Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCAsmInfo.cpp:24
+  if (TT.getArch() == Triple::riscv32be || TT.getArch() == Triple::riscv64be)
+IsLittleEndian = false;
+

Could we do IsLittleEndian = TT.isLittleEndian()?



Comment at: llvm/lib/Target/RISCV/RISCVTargetMachine.cpp:64
+  return "e-m:e-p:64:64-i64:64-i128:128-n64-S128";
+else
+  return "E-m:e-p:64:64-i64:64-i128:128-n64-S128";

No else after return


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128612

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

[Lldb-commits] [PATCH] D128612: RISC-V big-endian support implementation

[Guy Benyei via Phabricator via lldb-commits]

gbenyei updated this revision to Diff 440196.
gbenyei added a comment.

Thanks, Craig. Updated the patch with your remarks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128612

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/Basic/Targets.cpp
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/lib/Driver/ToolChains/RISCVToolchain.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  lld/ELF/Arch/RISCV.cpp
  lld/ELF/InputFiles.cpp
  lldb/source/Utility/ArchSpec.cpp
  llvm/cmake/config-ix.cmake
  llvm/cmake/config.guess
  llvm/include/llvm/ADT/Triple.h
  llvm/include/llvm/Object/ELFObjectFile.h
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/lib/ExecutionEngine/JITLink/ELF.cpp
  llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp
  llvm/lib/ExecutionEngine/Orc/EPCIndirectionUtils.cpp
  llvm/lib/ExecutionEngine/Orc/IndirectionUtils.cpp
  llvm/lib/ExecutionEngine/Orc/LazyReexports.cpp
  llvm/lib/Object/RelocationResolver.cpp
  llvm/lib/Support/Triple.cpp
  llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
  llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCAsmInfo.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCTargetDesc.cpp
  llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
  llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
  llvm/lib/Target/RISCV/TargetInfo/RISCVTargetInfo.cpp
  llvm/lib/Target/RISCV/TargetInfo/RISCVTargetInfo.h
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
  llvm/unittests/Object/ELFObjectFileTest.cpp

Index: llvm/unittests/Object/ELFObjectFileTest.cpp
===
--- llvm/unittests/Object/ELFObjectFileTest.cpp
+++ llvm/unittests/Object/ELFObjectFileTest.cpp
@@ -186,10 +186,10 @@
 }
 
 TEST(ELFObjectFileTest, MachineTestForRISCV) {
-  std::array Formats = {"elf32-littleriscv", "elf32-littleriscv",
-  "elf64-littleriscv", "elf64-littleriscv"};
-  std::array Archs = {Triple::riscv32, Triple::riscv32,
-   Triple::riscv64, Triple::riscv64};
+  std::array Formats = {"elf32-littleriscv", "elf32-bigriscv",
+  "elf64-littleriscv", "elf64-bigriscv"};
+  std::array Archs = {Triple::riscv32, Triple::riscv32be,
+   Triple::riscv64, Triple::riscv64be};
   size_t I = 0;
   for (const DataForTest &D : generateData(ELF::EM_RISCV)) {
 checkFormatAndArch(D, Formats[I], Archs[I]);
Index: llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
===
--- llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
+++ llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
@@ -301,6 +301,8 @@
 // RISC-V
 {"elf32-littleriscv", {ELF::EM_RISCV, false, true}},
 {"elf64-littleriscv", {ELF::EM_RISCV, true, true}},
+{"elf32-bigriscv", {ELF::EM_RISCV, false, false}},
+{"elf64-bigriscv", {ELF::EM_RISCV, true, false}},
 // PowerPC
 {"elf32-powerpc", {ELF::EM_PPC, false, false}},
 {"elf32-powerpcle", {ELF::EM_PPC, false, true}},
Index: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -479,7 +479,8 @@
   bool IsMIPS64 = TargetTriple.isMIPS64();
   bool IsArmOrThumb = TargetTriple.isARM() || TargetTriple.isThumb();
   bool IsAArch64 = TargetTriple.getArch() == Triple::aarch64;
-  bool IsRISCV64 = TargetTriple.getArch() == Triple::riscv64;
+  bool IsRISCV64 = TargetTriple.getArch() == Triple::riscv64 ||
+   TargetTriple.getArch() == Triple::riscv64be;
   bool IsWindows = TargetTriple.isOSWindows();
   bool IsFuchsia = TargetTriple.isOSFuchsia();
   bool IsEmscripten = TargetTriple.isOSEmscripten();
Index: llvm/lib/Target/RISCV/TargetInfo/RISCVTargetInfo.h
===
--- llvm/lib/Target/RISCV/TargetInfo/RISCVTargetInfo.h
+++ llvm/lib/Target/RISCV/TargetInfo/RISCVTargetInfo.h
@@ -15,6 +15,8 @@
 
 Target &getTheRISCV32Target();
 Target &getTheRISCV64Target();
+Target &getTheRISCV32beTarget

[Lldb-commits] [PATCH] D128612: RISC-V big-endian support implementation

[James Henderson via Phabricator via lldb-commits]

jhenderson added inline comments.



Comment at: llvm/include/llvm/ADT/Triple.h:864
 
   /// Tests whether the target is RISC-V (32- and 64-bit).
   bool isRISCV() const {

Perhaps worth updating to mention big and little endian here, like `isPPC64` 
above?



Comment at: llvm/tools/llvm-objcopy/ObjcopyOptions.cpp:304-305
 {"elf64-littleriscv", {ELF::EM_RISCV, true, true}},
+{"elf32-bigriscv", {ELF::EM_RISCV, false, false}},
+{"elf64-bigriscv", {ELF::EM_RISCV, true, false}},
 // PowerPC

We need llvm-objcopy testing for these new targets.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128612

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

[Lldb-commits] [PATCH] D128612: RISC-V big-endian support implementation

[Jessica Clarke via Phabricator via lldb-commits]

jrtc27 added inline comments.



Comment at: clang/lib/Basic/Targets/RISCV.h:113
+if (Triple.isLittleEndian())
+  resetDataLayout("e-m:e-p:32:32-i64:64-n32-S128");
+else

And please avoid repeating the whole data layout, just make the e/E a variable



Comment at: clang/lib/Driver/ToolChains/FreeBSD.cpp:235
+CmdArgs.push_back("-m");
+CmdArgs.push_back("elf32briscv");
+break;

-X to match LE. Or ditch these (and I can ditch riscv32...) since FreeBSD only 
supports riscv64.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1700
 {M.gccSuffix(),
  "/../../../../riscv64-unknown-elf/lib" + M.gccSuffix(),
  "/../../../../riscv32-unknown-elf/lib" + M.gccSuffix()});

Just shove a `+ Be +` in the middle of these two rather than introducing a 
whole new set and picking between them?



Comment at: llvm/cmake/config-ix.cmake:463
   set(LLVM_NATIVE_ARCH RISCV)
+elseif (LLVM_NATIVE_ARCH MATCHES "riscv32be")
+  set(LLVM_NATIVE_ARCH RISCV)

I believe these need to come before the unsuffixed versions to do anything, but 
also the unsuffixed versions already handle the suffixed versions correctly so 
this isn't needed?



Comment at: llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp:554
 .buildGraph();
-  } else {
-assert((*ELFObj)->getArch() == Triple::riscv32 &&
-   "Invalid triple for RISCV ELF object file");
+  } else if ((*ELFObj)->getArch() == Triple::riscv64be) {
+auto &ELFObjFile = cast>(**ELFObj);

Why switch to this order when before you've used 32, 64, 32be, 64be as the order



Comment at: llvm/lib/Target/RISCV/RISCVTargetMachine.cpp:65
+
+return "E-m:e-p:64:64-i64:64-i128:128-n64-S128";
+  }

As with Clang


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128612

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

[Lldb-commits] [PATCH] D128465: Zstandard as a second compression method to LLVM

[Cole Kissane via Phabricator via lldb-commits]

ckissane updated this revision to Diff 440288.
ckissane added a comment.

Fix missing cmake findZstd, remove unused crc32 from compression namespaces and 
simplify some code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128465

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/index/Serialization.cpp
  clang-tools-extra/clangd/test/lit.site.cfg.py.in
  clang-tools-extra/clangd/unittests/SerializationTests.cpp
  clang/cmake/caches/Apple-stage1.cmake
  clang/cmake/caches/Apple-stage2.cmake
  clang/cmake/caches/Fuchsia-stage2.cmake
  clang/cmake/caches/Fuchsia.cmake
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  compiler-rt/lib/sanitizer_common/symbolizer/scripts/build_symbolizer.sh
  compiler-rt/test/lit.common.configured.in
  flang/CMakeLists.txt
  lld/ELF/CMakeLists.txt
  lld/ELF/Driver.cpp
  lld/ELF/InputSection.cpp
  lldb/source/Plugins/Process/gdb-remote/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/cmake/config-ix.cmake
  llvm/cmake/modules/FindZSTD.cmake
  llvm/cmake/modules/LLVMConfig.cmake.in
  llvm/include/llvm/Config/llvm-config.h.cmake
  llvm/include/llvm/Support/Compression.h
  llvm/lib/MC/ELFObjectWriter.cpp
  llvm/lib/ObjCopy/ELF/ELFObject.cpp
  llvm/lib/Object/Decompressor.cpp
  llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
  llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp
  llvm/lib/ProfileData/InstrProf.cpp
  llvm/lib/ProfileData/SampleProfReader.cpp
  llvm/lib/ProfileData/SampleProfWriter.cpp
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/Compression.cpp
  llvm/tools/llvm-mc/llvm-mc.cpp
  llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
  llvm/unittests/ProfileData/InstrProfTest.cpp
  llvm/unittests/Support/CompressionTest.cpp
  utils/bazel/llvm_configs/llvm-config.h.cmake

Index: utils/bazel/llvm_configs/llvm-config.h.cmake
===
--- utils/bazel/llvm_configs/llvm-config.h.cmake
+++ utils/bazel/llvm_configs/llvm-config.h.cmake
@@ -92,6 +92,9 @@
 /* Define if zlib compression is available */
 #cmakedefine01 LLVM_ENABLE_ZLIB
 
+/* Define if zstd compression is available */
+#cmakedefine01 LLVM_ENABLE_ZSTD
+
 /* Define if LLVM was built with a dependency to the libtensorflow dynamic library */
 #cmakedefine LLVM_HAVE_TF_API
 
Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -11,13 +11,16 @@
 //===--===//
 
 #include "llvm/Support/Compression.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Config/config.h"
+#include "llvm/Support/CRC.h"
 #include "llvm/Support/Error.h"
 #include "gtest/gtest.h"
 
 using namespace llvm;
+using namespace compression;
 
 namespace {
 
@@ -63,9 +66,54 @@
 }
 
 TEST(CompressionTest, ZlibCRC32) {
+  const char* StringString = strdup("The quick brown fox jumps over the lazy dog");
+  const ArrayRef StringByteArray = ArrayRef(*StringString);
   EXPECT_EQ(
   0x414FA339U,
-  zlib::crc32(StringRef("The quick brown fox jumps over the lazy dog")));
+  llvm::crc32(StringByteArray));
+}
+
+#endif
+
+#if LLVM_ENABLE_ZSTD
+
+void TestZstdCompression(StringRef Input, int Level) {
+  SmallString<32> Compressed;
+  SmallString<32> Uncompressed;
+
+  zstd::compress(Input, Compressed, Level);
+
+  // Check that uncompressed buffer is the same as original.
+  Error E = zstd::uncompress(Compressed, Uncompressed, Input.size());
+  consumeError(std::move(E));
+
+  EXPECT_EQ(Input, Uncompressed);
+  if (Input.size() > 0) {
+// Uncompression fails if expected length is too short.
+E = zstd::uncompress(Compressed, Uncompressed, Input.size() - 1);
+EXPECT_EQ("zstd error: Z_BUF_ERROR", llvm::toString(std::move(E)));
+  }
+}
+
+TEST(CompressionTest, Zstd) {
+  TestZstdCompression("", zstd::DefaultCompression);
+
+  TestZstdCompression("hello, world!", zstd::NoCompression);
+  TestZstdCompression("hello, world!", zstd::BestSizeCompression);
+  TestZstdCompression("hello, world!", zstd::BestSpeedCompression);
+  TestZstdCompression("hello, world!", zstd::DefaultCompression);
+
+  const size_t kSize = 1024;
+  char BinaryData[kSize];
+  for (size_t i = 0; i < kSize; ++i) {
+BinaryData[i] = i & 255;
+  }
+  StringRef BinaryDataStr(BinaryData, kSize);
+
+  TestZstdCompression(BinaryDataStr, zstd::NoCompression);
+  TestZstdCompression(BinaryDataStr, zstd::BestSizeCompression);
+  TestZstdCompression(BinaryDataStr, zstd::BestSpeedCompression);
+  TestZstdCompression(BinaryDataStr, zstd::DefaultCompression);
 }
 
 #endif
Index: llvm/unittests/Profile

[Lldb-commits] [PATCH] D128465: Zstandard as a second compression method to LLVM

[Cole Kissane via Phabricator via lldb-commits]

ckissane updated this revision to Diff 440298.
ckissane added a comment.

fix type for FindZstd cmake usage


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128465

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/index/Serialization.cpp
  clang-tools-extra/clangd/test/lit.site.cfg.py.in
  clang-tools-extra/clangd/unittests/SerializationTests.cpp
  clang/cmake/caches/Apple-stage1.cmake
  clang/cmake/caches/Apple-stage2.cmake
  clang/cmake/caches/Fuchsia-stage2.cmake
  clang/cmake/caches/Fuchsia.cmake
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  compiler-rt/lib/sanitizer_common/symbolizer/scripts/build_symbolizer.sh
  compiler-rt/test/lit.common.configured.in
  flang/CMakeLists.txt
  lld/ELF/CMakeLists.txt
  lld/ELF/Driver.cpp
  lld/ELF/InputSection.cpp
  lldb/source/Plugins/Process/gdb-remote/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/cmake/config-ix.cmake
  llvm/cmake/modules/FindZSTD.cmake
  llvm/cmake/modules/LLVMConfig.cmake.in
  llvm/include/llvm/Config/llvm-config.h.cmake
  llvm/include/llvm/Support/Compression.h
  llvm/lib/MC/ELFObjectWriter.cpp
  llvm/lib/ObjCopy/ELF/ELFObject.cpp
  llvm/lib/Object/Decompressor.cpp
  llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
  llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp
  llvm/lib/ProfileData/InstrProf.cpp
  llvm/lib/ProfileData/SampleProfReader.cpp
  llvm/lib/ProfileData/SampleProfWriter.cpp
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/Compression.cpp
  llvm/tools/llvm-mc/llvm-mc.cpp
  llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
  llvm/unittests/ProfileData/InstrProfTest.cpp
  llvm/unittests/Support/CompressionTest.cpp
  utils/bazel/llvm_configs/llvm-config.h.cmake

Index: utils/bazel/llvm_configs/llvm-config.h.cmake
===
--- utils/bazel/llvm_configs/llvm-config.h.cmake
+++ utils/bazel/llvm_configs/llvm-config.h.cmake
@@ -92,6 +92,9 @@
 /* Define if zlib compression is available */
 #cmakedefine01 LLVM_ENABLE_ZLIB
 
+/* Define if zstd compression is available */
+#cmakedefine01 LLVM_ENABLE_ZSTD
+
 /* Define if LLVM was built with a dependency to the libtensorflow dynamic library */
 #cmakedefine LLVM_HAVE_TF_API
 
Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -11,13 +11,16 @@
 //===--===//
 
 #include "llvm/Support/Compression.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Config/config.h"
+#include "llvm/Support/CRC.h"
 #include "llvm/Support/Error.h"
 #include "gtest/gtest.h"
 
 using namespace llvm;
+using namespace compression;
 
 namespace {
 
@@ -63,9 +66,54 @@
 }
 
 TEST(CompressionTest, ZlibCRC32) {
+  const char* StringString = strdup("The quick brown fox jumps over the lazy dog");
+  const ArrayRef StringByteArray = ArrayRef(*StringString);
   EXPECT_EQ(
   0x414FA339U,
-  zlib::crc32(StringRef("The quick brown fox jumps over the lazy dog")));
+  llvm::crc32(StringByteArray));
+}
+
+#endif
+
+#if LLVM_ENABLE_ZSTD
+
+void TestZstdCompression(StringRef Input, int Level) {
+  SmallString<32> Compressed;
+  SmallString<32> Uncompressed;
+
+  zstd::compress(Input, Compressed, Level);
+
+  // Check that uncompressed buffer is the same as original.
+  Error E = zstd::uncompress(Compressed, Uncompressed, Input.size());
+  consumeError(std::move(E));
+
+  EXPECT_EQ(Input, Uncompressed);
+  if (Input.size() > 0) {
+// Uncompression fails if expected length is too short.
+E = zstd::uncompress(Compressed, Uncompressed, Input.size() - 1);
+EXPECT_EQ("zstd error: Z_BUF_ERROR", llvm::toString(std::move(E)));
+  }
+}
+
+TEST(CompressionTest, Zstd) {
+  TestZstdCompression("", zstd::DefaultCompression);
+
+  TestZstdCompression("hello, world!", zstd::NoCompression);
+  TestZstdCompression("hello, world!", zstd::BestSizeCompression);
+  TestZstdCompression("hello, world!", zstd::BestSpeedCompression);
+  TestZstdCompression("hello, world!", zstd::DefaultCompression);
+
+  const size_t kSize = 1024;
+  char BinaryData[kSize];
+  for (size_t i = 0; i < kSize; ++i) {
+BinaryData[i] = i & 255;
+  }
+  StringRef BinaryDataStr(BinaryData, kSize);
+
+  TestZstdCompression(BinaryDataStr, zstd::NoCompression);
+  TestZstdCompression(BinaryDataStr, zstd::BestSizeCompression);
+  TestZstdCompression(BinaryDataStr, zstd::BestSpeedCompression);
+  TestZstdCompression(BinaryDataStr, zstd::DefaultCompression);
 }
 
 #endif
Index: llvm/unittests/ProfileData/InstrProfTest.cpp
===

[Lldb-commits] [PATCH] D128465: Zstandard as a second compression method to LLVM

[Petr Hosek via Phabricator via lldb-commits]

phosek added a comment.

I think this patch should be broken into at least two:

1. Refactor `llvm/include/llvm/Support/Compression.h` and 
`llvm/lib/Support/Compression.cpp` to introduce a generic interface and use it 
throughout the codebase.
2. zstd support in `llvm/include/llvm/Support/Compression.h` including the 
CMake bits.

When uploading future changes, please also make sure to include full context.




Comment at: llvm/include/llvm/Support/Compression.h:67-87
+#if LLVM_ENABLE_ZSTD
+namespace internal = llvm::compression::zstd;
+#else
+namespace internal = llvm::compression::zlib;
+#endif
+
+namespace elf = llvm::compression::zlib;

I think think that this should be a runtime rather than compile time selection. 
Many (if not most) toolchain vendors would likely want to enable both zstd and 
zlib and choose the algorithm based an option or file content.

For example, in the case of profiles, we want to be able to read existing 
profiles that use zlib, but for newly created profiles we may want to use zstd 
if available.

To make it possible, I suspect we may want to wrap the existing functions into 
a class that implements a common interface. I expect we would also need a 
function to detect the compression type based on the buffer content.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128465

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

[Lldb-commits] [PATCH] D128465: Zstandard as a second compression method to LLVM

[Cole Kissane via Phabricator via lldb-commits]

ckissane updated this revision to Diff 440325.
ckissane added a comment.

more context


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

https://reviews.llvm.org/D128465

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/index/Serialization.cpp
  clang-tools-extra/clangd/test/lit.site.cfg.py.in
  clang-tools-extra/clangd/unittests/SerializationTests.cpp
  clang/cmake/caches/Apple-stage1.cmake
  clang/cmake/caches/Apple-stage2.cmake
  clang/cmake/caches/Fuchsia-stage2.cmake
  clang/cmake/caches/Fuchsia.cmake
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  compiler-rt/lib/sanitizer_common/symbolizer/scripts/build_symbolizer.sh
  compiler-rt/test/lit.common.configured.in
  flang/CMakeLists.txt
  lld/ELF/CMakeLists.txt
  lld/ELF/Driver.cpp
  lld/ELF/InputSection.cpp
  lldb/source/Plugins/Process/gdb-remote/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/cmake/config-ix.cmake
  llvm/cmake/modules/FindZSTD.cmake
  llvm/cmake/modules/LLVMConfig.cmake.in
  llvm/include/llvm/Config/llvm-config.h.cmake
  llvm/include/llvm/Support/Compression.h
  llvm/lib/MC/ELFObjectWriter.cpp
  llvm/lib/ObjCopy/ELF/ELFObject.cpp
  llvm/lib/Object/Decompressor.cpp
  llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
  llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp
  llvm/lib/ProfileData/InstrProf.cpp
  llvm/lib/ProfileData/SampleProfReader.cpp
  llvm/lib/ProfileData/SampleProfWriter.cpp
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/Compression.cpp
  llvm/tools/llvm-mc/llvm-mc.cpp
  llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
  llvm/unittests/ProfileData/InstrProfTest.cpp
  llvm/unittests/Support/CompressionTest.cpp
  utils/bazel/llvm_configs/llvm-config.h.cmake

Index: utils/bazel/llvm_configs/llvm-config.h.cmake
===
--- utils/bazel/llvm_configs/llvm-config.h.cmake
+++ utils/bazel/llvm_configs/llvm-config.h.cmake
@@ -92,6 +92,9 @@
 /* Define if zlib compression is available */
 #cmakedefine01 LLVM_ENABLE_ZLIB
 
+/* Define if zstd compression is available */
+#cmakedefine01 LLVM_ENABLE_ZSTD
+
 /* Define if LLVM was built with a dependency to the libtensorflow dynamic library */
 #cmakedefine LLVM_HAVE_TF_API
 
Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -11,13 +11,16 @@
 //===--===//
 
 #include "llvm/Support/Compression.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Config/config.h"
+#include "llvm/Support/CRC.h"
 #include "llvm/Support/Error.h"
 #include "gtest/gtest.h"
 
 using namespace llvm;
+using namespace compression;
 
 namespace {
 
@@ -63,9 +66,54 @@
 }
 
 TEST(CompressionTest, ZlibCRC32) {
+  const char* StringString = strdup("The quick brown fox jumps over the lazy dog");
+  const ArrayRef StringByteArray = ArrayRef(*StringString);
   EXPECT_EQ(
   0x414FA339U,
-  zlib::crc32(StringRef("The quick brown fox jumps over the lazy dog")));
+  llvm::crc32(StringByteArray));
+}
+
+#endif
+
+#if LLVM_ENABLE_ZSTD
+
+void TestZstdCompression(StringRef Input, int Level) {
+  SmallString<32> Compressed;
+  SmallString<32> Uncompressed;
+
+  zstd::compress(Input, Compressed, Level);
+
+  // Check that uncompressed buffer is the same as original.
+  Error E = zstd::uncompress(Compressed, Uncompressed, Input.size());
+  consumeError(std::move(E));
+
+  EXPECT_EQ(Input, Uncompressed);
+  if (Input.size() > 0) {
+// Uncompression fails if expected length is too short.
+E = zstd::uncompress(Compressed, Uncompressed, Input.size() - 1);
+EXPECT_EQ("zstd error: Z_BUF_ERROR", llvm::toString(std::move(E)));
+  }
+}
+
+TEST(CompressionTest, Zstd) {
+  TestZstdCompression("", zstd::DefaultCompression);
+
+  TestZstdCompression("hello, world!", zstd::NoCompression);
+  TestZstdCompression("hello, world!", zstd::BestSizeCompression);
+  TestZstdCompression("hello, world!", zstd::BestSpeedCompression);
+  TestZstdCompression("hello, world!", zstd::DefaultCompression);
+
+  const size_t kSize = 1024;
+  char BinaryData[kSize];
+  for (size_t i = 0; i < kSize; ++i) {
+BinaryData[i] = i & 255;
+  }
+  StringRef BinaryDataStr(BinaryData, kSize);
+
+  TestZstdCompression(BinaryDataStr, zstd::NoCompression);
+  TestZstdCompression(BinaryDataStr, zstd::BestSizeCompression);
+  TestZstdCompression(BinaryDataStr, zstd::BestSpeedCompression);
+  TestZstdCompression(BinaryDataStr, zstd::DefaultCompression);
 }
 
 #endif
Index: llvm/unittests/ProfileData/InstrProfTest.cpp
===
--- llvm/unittests/ProfileData/Inst

[Lldb-commits] [PATCH] D128465: Zstandard as a second compression method to LLVM

[Cole Kissane via Phabricator via lldb-commits]

ckissane updated this revision to Diff 440327.
ckissane added a comment.

format


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

https://reviews.llvm.org/D128465

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/index/Serialization.cpp
  clang-tools-extra/clangd/test/lit.site.cfg.py.in
  clang-tools-extra/clangd/unittests/SerializationTests.cpp
  clang/cmake/caches/Apple-stage1.cmake
  clang/cmake/caches/Apple-stage2.cmake
  clang/cmake/caches/Fuchsia-stage2.cmake
  clang/cmake/caches/Fuchsia.cmake
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  compiler-rt/lib/sanitizer_common/symbolizer/scripts/build_symbolizer.sh
  compiler-rt/test/lit.common.configured.in
  flang/CMakeLists.txt
  lld/ELF/CMakeLists.txt
  lld/ELF/Driver.cpp
  lld/ELF/InputSection.cpp
  lldb/source/Plugins/Process/gdb-remote/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/cmake/config-ix.cmake
  llvm/cmake/modules/FindZSTD.cmake
  llvm/cmake/modules/LLVMConfig.cmake.in
  llvm/include/llvm/Config/llvm-config.h.cmake
  llvm/include/llvm/Support/Compression.h
  llvm/lib/MC/ELFObjectWriter.cpp
  llvm/lib/ObjCopy/ELF/ELFObject.cpp
  llvm/lib/Object/Decompressor.cpp
  llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
  llvm/lib/ProfileData/Coverage/CoverageMappingWriter.cpp
  llvm/lib/ProfileData/InstrProf.cpp
  llvm/lib/ProfileData/SampleProfReader.cpp
  llvm/lib/ProfileData/SampleProfWriter.cpp
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/Compression.cpp
  llvm/tools/llvm-mc/llvm-mc.cpp
  llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
  llvm/unittests/ProfileData/InstrProfTest.cpp
  llvm/unittests/Support/CompressionTest.cpp
  utils/bazel/llvm_configs/llvm-config.h.cmake

Index: utils/bazel/llvm_configs/llvm-config.h.cmake
===
--- utils/bazel/llvm_configs/llvm-config.h.cmake
+++ utils/bazel/llvm_configs/llvm-config.h.cmake
@@ -92,6 +92,9 @@
 /* Define if zlib compression is available */
 #cmakedefine01 LLVM_ENABLE_ZLIB
 
+/* Define if zstd compression is available */
+#cmakedefine01 LLVM_ENABLE_ZSTD
+
 /* Define if LLVM was built with a dependency to the libtensorflow dynamic library */
 #cmakedefine LLVM_HAVE_TF_API
 
Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -11,13 +11,16 @@
 //===--===//
 
 #include "llvm/Support/Compression.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Config/config.h"
+#include "llvm/Support/CRC.h"
 #include "llvm/Support/Error.h"
 #include "gtest/gtest.h"
 
 using namespace llvm;
+using namespace compression;
 
 namespace {
 
@@ -63,9 +66,53 @@
 }
 
 TEST(CompressionTest, ZlibCRC32) {
-  EXPECT_EQ(
-  0x414FA339U,
-  zlib::crc32(StringRef("The quick brown fox jumps over the lazy dog")));
+  const char *StringString =
+  strdup("The quick brown fox jumps over the lazy dog");
+  const ArrayRef StringByteArray = ArrayRef(*StringString);
+  EXPECT_EQ(0x414FA339U, llvm::crc32(StringByteArray));
+}
+
+#endif
+
+#if LLVM_ENABLE_ZSTD
+
+void TestZstdCompression(StringRef Input, int Level) {
+  SmallString<32> Compressed;
+  SmallString<32> Uncompressed;
+
+  zstd::compress(Input, Compressed, Level);
+
+  // Check that uncompressed buffer is the same as original.
+  Error E = zstd::uncompress(Compressed, Uncompressed, Input.size());
+  consumeError(std::move(E));
+
+  EXPECT_EQ(Input, Uncompressed);
+  if (Input.size() > 0) {
+// Uncompression fails if expected length is too short.
+E = zstd::uncompress(Compressed, Uncompressed, Input.size() - 1);
+EXPECT_EQ("zstd error: Z_BUF_ERROR", llvm::toString(std::move(E)));
+  }
+}
+
+TEST(CompressionTest, Zstd) {
+  TestZstdCompression("", zstd::DefaultCompression);
+
+  TestZstdCompression("hello, world!", zstd::NoCompression);
+  TestZstdCompression("hello, world!", zstd::BestSizeCompression);
+  TestZstdCompression("hello, world!", zstd::BestSpeedCompression);
+  TestZstdCompression("hello, world!", zstd::DefaultCompression);
+
+  const size_t kSize = 1024;
+  char BinaryData[kSize];
+  for (size_t i = 0; i < kSize; ++i) {
+BinaryData[i] = i & 255;
+  }
+  StringRef BinaryDataStr(BinaryData, kSize);
+
+  TestZstdCompression(BinaryDataStr, zstd::NoCompression);
+  TestZstdCompression(BinaryDataStr, zstd::BestSizeCompression);
+  TestZstdCompression(BinaryDataStr, zstd::BestSpeedCompression);
+  TestZstdCompression(BinaryDataStr, zstd::DefaultCompression);
 }
 
 #endif
Index: llvm/unittests/ProfileData/InstrProfTest.cpp
===
--- llvm/unitte

[Lldb-commits] [PATCH] D128465: Zstandard as a second compression method to LLVM

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added a comment.

In D128465#3612948 , @phosek wrote:

> I think this patch should be broken into at least two:
>
> 1. Refactor `llvm/include/llvm/Support/Compression.h` and 
> `llvm/lib/Support/Compression.cpp` to introduce a generic interface and use 
> it throughout the codebase.
> 2. zstd support in `llvm/include/llvm/Support/Compression.h` including the 
> CMake bits.
>
> When uploading future changes, please also make sure to include full context.

Agree. As soon as the namespace refactoring is in a good enough shape, I think 
you may land the refactoring part before the rest of zstd patches.

Note: if you have a deep stacked patches, it may be useful to have a branch 
somewhere (e.g. your llvm-project fork on Github) so that interested folks can 
get the whole picture more easily.
(`arc patch Dx` can technically apply a deep stack, but it often fails to 
apply changes cleanly.)


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

https://reviews.llvm.org/D128465

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

[Lldb-commits] [PATCH] D128612: RISC-V big-endian support implementation

[Guy Benyei via Phabricator via lldb-commits]

gbenyei updated this revision to Diff 440579.
gbenyei marked 7 inline comments as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128612

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/Basic/Targets.cpp
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/lib/Driver/ToolChains/RISCVToolchain.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  lld/ELF/Arch/RISCV.cpp
  lld/ELF/InputFiles.cpp
  lldb/source/Utility/ArchSpec.cpp
  llvm/cmake/config.guess
  llvm/include/llvm/ADT/Triple.h
  llvm/include/llvm/Object/ELFObjectFile.h
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/lib/ExecutionEngine/JITLink/ELF.cpp
  llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp
  llvm/lib/ExecutionEngine/Orc/EPCIndirectionUtils.cpp
  llvm/lib/ExecutionEngine/Orc/IndirectionUtils.cpp
  llvm/lib/ExecutionEngine/Orc/LazyReexports.cpp
  llvm/lib/Object/RelocationResolver.cpp
  llvm/lib/Support/Triple.cpp
  llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
  llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCAsmInfo.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCTargetDesc.cpp
  llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
  llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
  llvm/lib/Target/RISCV/TargetInfo/RISCVTargetInfo.cpp
  llvm/lib/Target/RISCV/TargetInfo/RISCVTargetInfo.h
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  llvm/test/tools/llvm-objcopy/ELF/binary-output-target.test
  llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
  llvm/unittests/Object/ELFObjectFileTest.cpp

Index: llvm/unittests/Object/ELFObjectFileTest.cpp
===
--- llvm/unittests/Object/ELFObjectFileTest.cpp
+++ llvm/unittests/Object/ELFObjectFileTest.cpp
@@ -186,10 +186,10 @@
 }
 
 TEST(ELFObjectFileTest, MachineTestForRISCV) {
-  std::array Formats = {"elf32-littleriscv", "elf32-littleriscv",
-  "elf64-littleriscv", "elf64-littleriscv"};
-  std::array Archs = {Triple::riscv32, Triple::riscv32,
-   Triple::riscv64, Triple::riscv64};
+  std::array Formats = {"elf32-littleriscv", "elf32-bigriscv",
+  "elf64-littleriscv", "elf64-bigriscv"};
+  std::array Archs = {Triple::riscv32, Triple::riscv32be,
+   Triple::riscv64, Triple::riscv64be};
   size_t I = 0;
   for (const DataForTest &D : generateData(ELF::EM_RISCV)) {
 checkFormatAndArch(D, Formats[I], Archs[I]);
Index: llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
===
--- llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
+++ llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
@@ -301,6 +301,8 @@
 // RISC-V
 {"elf32-littleriscv", {ELF::EM_RISCV, false, true}},
 {"elf64-littleriscv", {ELF::EM_RISCV, true, true}},
+{"elf32-bigriscv", {ELF::EM_RISCV, false, false}},
+{"elf64-bigriscv", {ELF::EM_RISCV, true, false}},
 // PowerPC
 {"elf32-powerpc", {ELF::EM_PPC, false, false}},
 {"elf32-powerpcle", {ELF::EM_PPC, false, true}},
Index: llvm/test/tools/llvm-objcopy/ELF/binary-output-target.test
===
--- llvm/test/tools/llvm-objcopy/ELF/binary-output-target.test
+++ llvm/test/tools/llvm-objcopy/ELF/binary-output-target.test
@@ -33,6 +33,12 @@
 # RUN: llvm-objcopy -I binary -O elf64-littleriscv %t.txt %t.rv64.o
 # RUN: llvm-readobj --file-headers %t.rv64.o | FileCheck %s --check-prefixes=CHECK,LE,RISCV64,64
 
+# RUN: llvm-objcopy -I binary -O elf32-bigriscv %t.txt %t.rv32.o
+# RUN: llvm-readobj --file-headers %t.rv32.o | FileCheck %s --check-prefixes=CHECK,BE,RISCV32,32
+
+# RUN: llvm-objcopy -I binary -O elf64-bigriscv %t.txt %t.rv64.o
+# RUN: llvm-readobj --file-headers %t.rv64.o | FileCheck %s --check-prefixes=CHECK,BE,RISCV64,64
+
 # RUN: llvm-objcopy -I binary -O elf32-sparc %t.txt %t.sparc.o
 # RUN: llvm-readobj --file-headers %t.sparc.o | FileCheck %s --check-prefixes=CHECK,BE,SPARC,32
 
Index: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ llvm/lib/T

[Lldb-commits] [PATCH] D128611: [libc++] Implement `std::ranges::merge`

[Hui via Phabricator via lldb-commits]

huixie90 updated this revision to Diff 440493.
huixie90 added a comment.
Herald added subscribers: cfe-commits, llvm-commits, libc-commits, 
openmp-commits, lldb-commits, Sanitizers, steakhal, nlopes, mtrofin, jsji, 
Enna1, bzcheeseman, kosarev, jsilvanus, mattd, gchakrabarti, ThomasRaoux, 
pmatos, asb, pcwang-thead, yota9, ayermolo, awarzynski, arjunp, sdasgup3, 
luke957, asavonic, carlosgalvezp, jeroen.dobbelaere, Groverkss, wenzhicui, 
wrengr, armkevincheng, ormris, foad, jsmolens, eric-k256, dcaballe, ChuanqiXu, 
cota, mravishankar, teijeong, frasercrmck, rdzhabarov, ecnelises, tatianashp, 
wenlei, mehdi_amini, okura, jdoerfert, bmahjour, msifontes, sstefan1, jurahul, 
kuter, Kayjukh, grosul1, martong, Joonsoo, stephenneuendorffer, kerbowa, 
liufengdb, aartbik, mgester, arpith-jacob, csigg, nicolasvasilache, 
antiagainst, shauheen, rriddle, luismarques, apazos, sameer.abuasal, usaxena95, 
pengfei, s.egerton, Jim, asbirlea, mstorsjo, miyuki, kadircet, jocewei, 
rupprecht, PkmX, arphaman, george.burgess.iv, the_o, brucehoult, MartinMosbeck, 
rogfer01, steven_wu, mgrang, edward-jones, zzheng, MaskRay, jrtc27, gbedwell, 
delcypher, niosHD, sabuasal, simoncook, haicheng, johnrusso, rbar, javed.absar, 
fedor.sergeev, kbarton, aheejin, hiraditya, jgravelle-google, arichardson, 
sbc100, nhaehnle, jvesely, nemanjai, emaste, dylanmckay, jyknight, dschuff, 
arsenm, qcolombet, MatzeB, jholewinski.
Herald added a reviewer: deadalnix.
Herald added a reviewer: bollu.
Herald added a reviewer: andreadb.
Herald added a reviewer: jdoerfert.
Herald added a reviewer: jhenderson.
Herald added a reviewer: jdoerfert.
Herald added a reviewer: sstefan1.
Herald added a reviewer: nicolasvasilache.
Herald added a reviewer: rriddle.
Herald added a reviewer: antiagainst.
Herald added a reviewer: aartbik.
Herald added a reviewer: MaskRay.
Herald added a reviewer: sscalpone.
Herald added a reviewer: jpienaar.
Herald added a reviewer: ftynse.
Herald added a reviewer: aaron.ballman.
Herald added a reviewer: aartbik.
Herald added a reviewer: aartbik.
Herald added a reviewer: awarzynski.
Herald added a reviewer: sjarus.
Herald added a reviewer: clementval.
Herald added a reviewer: rafauler.
Herald added a reviewer: Amir.
Herald added a reviewer: maksfb.
Herald added a reviewer: paulkirth.
Herald added projects: clang, Sanitizers, LLDB, OpenMP, libc-project, 
libunwind, MLIR, LLVM, lld-macho, clang-tools-extra, Flang.
Herald added a reviewer: libunwind.
Herald added a reviewer: lld-macho.

reorder includes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128611

Files:
  bolt/README.md
  bolt/docs/OptimizingClang.md
  bolt/include/bolt/Passes/SplitFunctions.h
  bolt/lib/Core/BinaryFunctionProfile.cpp
  bolt/lib/Core/DebugData.cpp
  bolt/lib/Passes/IndirectCallPromotion.cpp
  bolt/lib/Passes/LongJmp.cpp
  bolt/lib/Passes/SplitFunctions.cpp
  bolt/lib/Rewrite/DWARFRewriter.cpp
  bolt/test/X86/Inputs/dwarf5-call-pc-helper.s
  bolt/test/X86/Inputs/dwarf5-call-pc-main.s
  bolt/test/X86/Inputs/dwarf5-return-pc-helper.s
  bolt/test/X86/Inputs/dwarf5-return-pc-main.s
  bolt/test/X86/bug-reorder-bb-jrcxz.s
  bolt/test/X86/dwarf5-call-pc.test
  bolt/test/X86/dwarf5-return-pc.test
  bolt/test/X86/jump-table-icp.test
  bolt/test/X86/shared_object_entry.s
  bolt/test/X86/unreachable.test
  bolt/test/runtime/X86/exceptions-instrumentation.test
  bolt/test/runtime/X86/pie-exceptions-split.test
  bolt/test/runtime/meta-merge-fdata.test
  clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/CompileCommands.cpp
  clang-tools-extra/clangd/FeatureModule.cpp
  clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  clang-tools-extra/clangd/unittests/DumpASTTests.cpp
  clang-tools-extra/clangd/unittests/FileIndexTests.cpp
  clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
  clang-tools-extra/clangd/unittests/HeadersTests.cpp
  clang-tools-extra/clangd/unittests/LSPClient.cpp
  clang-tools-extra/clangd/unittests/Matchers.h
  clang-tools-extra/clangd/unittests/SerializationTests.cpp
  clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
  clang-tools-extra/clangd/unittests/SyncAPI.cpp
  clang-tools-extra/clangd/unittests/TestTU.cpp
  clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/pseudo/include/clang-pseudo/grammar/LRTable.h
  clang-tools-extra/pseudo/lib/GLR.cpp
  clang-tools-extra/pseudo/lib/grammar/LRTable.cpp
  clang-tools-extra/pseudo/lib/grammar/LRTableBuild.cpp
  clang-tools-extra/pseudo/test/lr-build-basic.test
  clang-tools-extra/pseudo/

[Lldb-commits] [PATCH] D128694: [lldb/Dataformatters] Adapt C++ std::string dataformatter for D128285

[Louis Dionne via Phabricator via lldb-commits]

ldionne added inline comments.



Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:637
 if (location_sp->GetName() == g_size_name)
-  location_sp = short_sp->GetChildAtIndex(3, true);
+  location_sp = short_sp->GetChildAtIndex(2, true);
 if (using_bitmasks)

mib wrote:
> mib wrote:
> > aprantl wrote:
> > > Let me know if I',m misunderstanding what the code is doing, but this 
> > > looks like it is replacing the previous implementation? Ideally we would 
> > > detect the layout and then parse it correctly depending on which version 
> > > we're dealing with. Otherwise we risk breaking the matrix bot that checks 
> > > that LLDB can debug C++ produced by older versions of LLVM (and by 
> > > extension libcxx).
> > I've look at D12828 and D123580, and I don't see any way of versioning 
> > these changes ... may be @ldionne have an idea on how we could do this 
> > properly ?
> > 
> > Also, in D124113, @labath mentions that this data formatter uses mostly 
> > indices to parse and access the various fields of the type data structure 
> > (because it uses some anonymous structs). This makes it very fragile on our 
> > end because our data formatter break every time they make a change in the 
> > layout ...
> > 
> > @aprantl, I'll update the line your pointed at to the the field identifier 
> > instead of using changing the index while waiting for a better way to 
> > version this.
> @aprantl, I'll update the line you pointed at to *use* the field identifier 
> instead of using changing the index, while waiting for a better way to 
> version this.
I don't see a way to version this. You don't have access to the value of macros 
that were defined when the executable was compiled, right? If you did, you 
could check `_LIBCPP_VERSION` (1400 = old implementation, 1500 = current 
implementation). I'm very naive when it comes to debuggers but I assume that's 
not possible.


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

https://reviews.llvm.org/D128694

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

[Lldb-commits] [lldb] 25f4608 - [lldb] [test] XFAIL llgs tests failing on arm

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-06-28T17:02:59+02:00
New Revision: 25f46084d8e11894edc0b71b737f45e0f9115b45

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

LOG: [lldb] [test] XFAIL llgs tests failing on arm

Sponsored by: The FreeBSD Foundation

Added: 


Modified: 
lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py

Removed: 




diff  --git a/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py 
b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
index aa97ccd51458..70cea2f85f6e 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -317,48 +317,56 @@ def resume_one_test(self, run_order, use_vCont=False):
 ], True)
 self.expect_gdbremote_sequence()
 
+@expectedFailureAll(archs=["arm"])  # TODO
 @expectedFailureAll(archs=["aarch64"],
 
bugnumber="https://github.com/llvm/llvm-project/issues/56268";)
 @add_test_categories(["fork"])
 def test_c_parent(self):
 self.resume_one_test(run_order=["parent", "parent"])
 
+@expectedFailureAll(archs=["arm"])  # TODO
 @expectedFailureAll(archs=["aarch64"],
 
bugnumber="https://github.com/llvm/llvm-project/issues/56268";)
 @add_test_categories(["fork"])
 def test_c_child(self):
 self.resume_one_test(run_order=["child", "child"])
 
+@expectedFailureAll(archs=["arm"])  # TODO
 @expectedFailureAll(archs=["aarch64"],
 
bugnumber="https://github.com/llvm/llvm-project/issues/56268";)
 @add_test_categories(["fork"])
 def test_c_parent_then_child(self):
 self.resume_one_test(run_order=["parent", "parent", "child", "child"])
 
+@expectedFailureAll(archs=["arm"])  # TODO
 @expectedFailureAll(archs=["aarch64"],
 
bugnumber="https://github.com/llvm/llvm-project/issues/56268";)
 @add_test_categories(["fork"])
 def test_c_child_then_parent(self):
 self.resume_one_test(run_order=["child", "child", "parent", "parent"])
 
+@expectedFailureAll(archs=["arm"])  # TODO
 @expectedFailureAll(archs=["aarch64"],
 
bugnumber="https://github.com/llvm/llvm-project/issues/56268";)
 @add_test_categories(["fork"])
 def test_c_interspersed(self):
 self.resume_one_test(run_order=["parent", "child", "parent", "child"])
 
+@expectedFailureAll(archs=["arm"])  # TODO
 @expectedFailureAll(archs=["aarch64"],
 
bugnumber="https://github.com/llvm/llvm-project/issues/56268";)
 @add_test_categories(["fork"])
 def test_vCont_parent(self):
 self.resume_one_test(run_order=["parent", "parent"], use_vCont=True)
 
+@expectedFailureAll(archs=["arm"])  # TODO
 @expectedFailureAll(archs=["aarch64"],
 
bugnumber="https://github.com/llvm/llvm-project/issues/56268";)
 @add_test_categories(["fork"])
 def test_vCont_child(self):
 self.resume_one_test(run_order=["child", "child"], use_vCont=True)
 
+@expectedFailureAll(archs=["arm"])  # TODO
 @expectedFailureAll(archs=["aarch64"],
 
bugnumber="https://github.com/llvm/llvm-project/issues/56268";)
 @add_test_categories(["fork"])
@@ -366,6 +374,7 @@ def test_vCont_parent_then_child(self):
 self.resume_one_test(run_order=["parent", "parent", "child", "child"],
  use_vCont=True)
 
+@expectedFailureAll(archs=["arm"])  # TODO
 @expectedFailureAll(archs=["aarch64"],
 
bugnumber="https://github.com/llvm/llvm-project/issues/56268";)
 @add_test_categories(["fork"])
@@ -373,6 +382,7 @@ def test_vCont_child_then_parent(self):
 self.resume_one_test(run_order=["child", "child", "parent", "parent"],
  use_vCont=True)
 
+@expectedFailureAll(archs=["arm"])  # TODO
 @expectedFailureAll(archs=["aarch64"],
 
bugnumber="https://github.com/llvm/llvm-project/issues/56268";)
 @add_test_categories(["fork"])
@@ -415,6 +425,7 @@ def test_vCont_all_processes_implicit(self):
 ], True)
 self.expect_gdbremote_sequence()
 
+@expectedFailureAll(archs=["arm"])  # TODO
 @add_test_categories(["fork"])
 def test_threadinfo(self):
 parent_pid, parent_tid, child_pid, child_tid = (
@@ -460,6 +471,7 @@ def test_threadinfo(self):
  ], True)
 self.expect_gdbremote_sequence()
 
+@expectedFailureAll(archs=["arm"])  # TODO
 @add_test_categories(["fork"])
 def test_memory_read_write(self):
 self.build()
@@ -547,6 +559,7 @@ def test_memory_read_write(self):
 self.assertEqual(data, name + "\0")

[Lldb-commits] [PATCH] D127755: [lldb] [llgs] Add test for resuming via c in multiprocess scenarios

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

mgorny added a comment.

Unfortunately, I wasn't able to get ARM to work (some triple mismatch) but I've 
masked the tests xfail for now, and both buildbots seem green.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127755

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

[Lldb-commits] [PATCH] D128576: [trace] Make events first class items in the trace cursor and rework errors

[Jakob Johnson via Phabricator via lldb-commits]

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

review- part 1




Comment at: lldb/include/lldb/Target/Trace.h:171
+  /// information failed to load, an \a llvm::Error is returned.
+  virtual llvm::Expected
+  CreateNewCursor(Thread &thread) = 0;

Do we want to keep the cursor as a UP or take this refactor before creating the 
public bindings to make it a SP? IMO a UP might make sense because I can't 
really think of many cases where you would want multiple users of the same 
cursor.
wdyt?



Comment at: lldb/include/lldb/Target/TraceCursor.h:44-46
+/// The cursor can also point at events in the trace, which aren't errors
+/// nor instructions. An example of an event could be a context switch in
+/// between two instructions.

do you want to include pointers to the methods you can use to check/get events 
similar to what you did for errors above?



Comment at: lldb/include/lldb/Target/TraceCursor.h:232-246
+  virtual const char *GetError() const = 0;
 
-  /// Get the corresponding error message if the cursor points to an error in
-  /// the trace.
-  ///
   /// \return
-  /// \b nullptr if the cursor is not pointing to an error in
-  /// the trace. Otherwise return the actual error message.
-  virtual const char *GetError() = 0;
+  /// Whether the cursor points to an event or not.
+  virtual bool IsEvent() const = 0;
 
   /// \return

For the getters, what are your thoughts on returning an optional instead of 
using designated value/variants (ie nullptr, LLDB_INVALID_INSTRUCTION, 
eTraceEventNone`) to indicate that the current item does not match what `Get` 
method is being used?

Along the same lines, but a slightly bigger change:
With the introduction of `Events` as first class citizens of the trace cursor 
API, it may make sense to introduce the notion of of a trace "item" or 
something similar that encapsulates (instructions, events, errors, etc). I'm 
thinking of a tagged union type structure (ie a Rust enum)  that enforces the 
correctness of the access to the underlying item.
wdyt?



Comment at: lldb/include/lldb/Target/TraceCursor.h:258
+  virtual llvm::Optional
+  GetCounter(lldb::TraceCounter counter_type) const = 0;
 

Now that we are introducing the notion of an `Event`? wdyt about combining 
Events and Counters since a counter value in a trace really is just a type of 
event? In this case, `Counter` could just become a variant of `TraceEvent`. 
This may make more sense because even in the case of TSCs in an IntelPT trace 
because, strictly speaking, these aren't associated with instructions, correct? 
Instead the TSC values are emitted with PSBs and then we "attribute" these 
values to the nearest instruction, right?



Comment at: lldb/include/lldb/Target/TraceDumper.h:59
+  /// Helper struct that holds all the information we know about a trace item
+  struct TraceItem {
 lldb::user_id_t id;

oh looks like you already introduced a `TraceItem` structure!
Similar to my comment above related to introducing a trace item type structure 
for the trace cursor, would it make sense to have a separate type for each of 
the different "items". With the way the TraceItem struct is currently, a lot of 
this data is mutually exclusive, hence the many optional fields, correct?



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:145-147
   /// \return
-  /// The control flow categories, or \b 0 if the instruction is an error.
+  /// The control flow categories, or an undefined vlaue if the item is not
+  /// an instruction.

Why not use an optional here instead?



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:236-239
+  enum class ItemKind : uint64_t {
+Instruction = 0,
+Error = LLDB_INVALID_ADDRESS - 1,
+Event = LLDB_INVALID_ADDRESS - 2,

Continuing the theme of my comments related to the notion of a `TraceItem` - 
would it be possible to unite the notion of a trace item? currently there is an 
Item enum here, a item struct in the Dumper and I feel there is a need for a 
Item at the trace cursor level as well.
wdyt?



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:241
+  };
+  ItemKind LowestNonInstructionItemKind = ItemKind::Event;
+

why is this needed?



Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:253-258
+  /// The low level storage of all instruction addresses, error and events. 
Each
+  /// trace item has an index in this vector and this index is used throughout
+  /// the code. As an storage optimization, values of \b ItemKind::Error
+  /// indicate that this item is an error, \b ItemKind::Event if the item is an
+  /// event, and otherwise 

[Lldb-commits] [PATCH] D128638: [lldb] [llgs] Add base nonstop fork/vfork tests

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

mgorny added a comment.

Ok, I'll commit as-is then and will revisit if we start seeing timeouts.


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

https://reviews.llvm.org/D128638

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

[Lldb-commits] [PATCH] D128707: [lldb] Fix build on older Linux kernel versions

[walter erquinigo via Phabricator via lldb-commits]

wallace added a comment.
Herald added a subscriber: JDevlieghere.

thanks a good point, Jakob. Let's keep it in mind. Maybe we can bootcamp that


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128707

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

[Lldb-commits] [PATCH] D128707: [lldb] Fix build on older Linux kernel versions

[walter erquinigo via Phabricator via lldb-commits]

wallace added a comment.

@kongyi thanks for fixing this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128707

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

[Lldb-commits] [lldb] f7bf9d1 - TestIgnoredExceptions.py needs support from debugserver, so it

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2022-06-28T11:49:55-07:00
New Revision: f7bf9d13d50d785889952deb18cc93de0176cb96

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

LOG: TestIgnoredExceptions.py needs support from debugserver, so it
needs to be marked skip if out of tree debugserver.

Added: 


Modified: 
lldb/test/API/macosx/ignore_exceptions/TestIgnoredExceptions.py

Removed: 




diff  --git a/lldb/test/API/macosx/ignore_exceptions/TestIgnoredExceptions.py 
b/lldb/test/API/macosx/ignore_exceptions/TestIgnoredExceptions.py
index e90b2ef8dbb3..f8112b6803e1 100644
--- a/lldb/test/API/macosx/ignore_exceptions/TestIgnoredExceptions.py
+++ b/lldb/test/API/macosx/ignore_exceptions/TestIgnoredExceptions.py
@@ -12,6 +12,7 @@ class TestDarwinSignalHandlers(TestBase):
 
 NO_DEBUG_INFO_TESTCASE = True
 
+@skipIfOutOfTreeDebugserver
 @skipUnlessDarwin
 def test_ignored_thread(self):
 """It isn't possible to convert an EXC_BAD_ACCESS to a signal when



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

[Lldb-commits] [lldb] e095cdd - [lldb] Add a NativeProcessProtocol::Threads() iterable

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-06-28T21:49:16+02:00
New Revision: e095cddb763fde72b577f3fab5e039cea4c3

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

LOG: [lldb] Add a NativeProcessProtocol::Threads() iterable

Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D128698

Added: 


Modified: 
lldb/include/lldb/Host/common/NativeProcessProtocol.h
lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

Removed: 




diff  --git a/lldb/include/lldb/Host/common/NativeProcessProtocol.h 
b/lldb/include/lldb/Host/common/NativeProcessProtocol.h
index 63dbb3a62ea5f..37dacca6ff813 100644
--- a/lldb/include/lldb/Host/common/NativeProcessProtocol.h
+++ b/lldb/include/lldb/Host/common/NativeProcessProtocol.h
@@ -15,6 +15,7 @@
 #include "lldb/Host/Host.h"
 #include "lldb/Host/MainLoop.h"
 #include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/Iterable.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/TraceGDBRemotePackets.h"
 #include "lldb/Utility/UnimplementedError.h"
@@ -48,6 +49,16 @@ class NativeProcessProtocol {
 public:
   virtual ~NativeProcessProtocol() = default;
 
+  typedef std::vector> thread_collection;
+  template 
+  static NativeThreadProtocol &thread_list_adapter(I &iter) {
+assert(*iter);
+return **iter;
+  }
+  typedef LockingAdaptedIterable
+  ThreadIterable;
+
   virtual Status Resume(const ResumeActionList &resume_actions) = 0;
 
   virtual Status Halt() = 0;
@@ -210,6 +221,10 @@ class NativeProcessProtocol {
 return GetThreadByID(m_current_thread_id);
   }
 
+  ThreadIterable Threads() const {
+return ThreadIterable(m_threads, m_threads_mutex);
+  }
+
   // Access to inferior stdio
   virtual int GetTerminalFileDescriptor() { return m_terminal_fd; }
 

diff  --git a/lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp 
b/lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
index f7990f2d394fc..d038c78a59941 100644
--- a/lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
+++ b/lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
@@ -98,8 +98,8 @@ Error IntelPTCollector::TraceStart(const 
TraceIntelPTStartRequest &request) {
   }
 } else {
   std::vector process_threads;
-  for (size_t i = 0; m_process.GetThreadAtIndex(i); i++)
-process_threads.push_back(m_process.GetThreadAtIndex(i)->GetID());
+  for (NativeThreadProtocol &thread : m_process.Threads())
+process_threads.push_back(thread.GetID());
 
   // per-thread process tracing
   if (Expected trace =

diff  --git a/lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp 
b/lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp
index ef4c5ae46ed53..f538ef7776491 100644
--- a/lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp
+++ b/lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp
@@ -107,9 +107,9 @@ void IntelPTMultiCoreTrace::ProcessWillResume() {
 TraceIntelPTGetStateResponse IntelPTMultiCoreTrace::GetState() {
   TraceIntelPTGetStateResponse state;
 
-  for (size_t i = 0; m_process.GetThreadAtIndex(i); i++)
+  for (NativeThreadProtocol &thread : m_process.Threads())
 state.traced_threads.push_back(
-TraceThreadState{m_process.GetThreadAtIndex(i)->GetID(), {}});
+TraceThreadState{thread.GetID(), {}});
 
   state.cpus.emplace();
   ForEachCore([&](lldb::cpu_id_t cpu_id,

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index ae8b8ef8ae156..49125f91d743a 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -724,17 +724,12 @@ GetJSONThreadsInfo(NativeProcessProtocol &process, bool 
abridged) {
   json::Array threads_array;
 
   // Ensure we can get info on the given thread.
-  uint32_t thread_idx = 0;
-  for (NativeThreadProtocol *thread;
-   (thread = process.GetThreadAtIndex(thread_idx)) != nullptr;
-   ++thread_idx) {
-
-lldb::tid_t tid = thread->GetID();
-
+  for (NativeThreadProtocol &thread : process.Threads()) {
+lldb::tid_t tid = thread.GetID();
 // Grab the reason this thread stopped.
 struct ThreadStopInfo tid_stop_info;
 std::string description;
-if (!thread->GetStopReason(tid_stop_info, description))
+if (!thread.GetStopReason(tid_stop_info, description))
   return llvm::make_error(
   "failed to get stop reason", llvm::inconvertibleErrorCode());
 
@@ -751,7 +746,7 @@ GetJSONThreadsInfo(NativeProcessP

[Lldb-commits] [lldb] b415f8e - [lldb] [llgs] Add base nonstop fork/vfork tests

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-06-28T21:49:16+02:00
New Revision: b415f8e3dfb839a886b5290bbebbdd42344c07a9

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

LOG: [lldb] [llgs] Add base nonstop fork/vfork tests

Extend the most of baseline fork tests to run in nonstop mode as well.
For more cases, we're just testing one example scenario to save time.
This patch does not cover tests that rely on correct exit handling,
as fixing that is addressed in a followup patch.

Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D128638

Added: 


Modified: 
lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py

Removed: 




diff  --git a/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py 
b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
index 70cea2f85f6ec..8b8e30c9b7379 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -9,10 +9,12 @@ class 
TestGdbRemoteFork(gdbremote_testcase.GdbRemoteTestCaseBase):
 
 fork_regex = ("[$]T05thread:p([0-9a-f]+)[.]([0-9a-f]+);.*"
   "{}:p([0-9a-f]+)[.]([0-9a-f]+).*")
+fork_regex_nonstop = ("%Stop:T05thread:p([0-9a-f]+)[.]([0-9a-f]+);.*"
+  "{}:p([0-9a-f]+)[.]([0-9a-f]+).*")
 fork_capture = {1: "parent_pid", 2: "parent_tid",
 3: "child_pid", 4: "child_tid"}
 
-def start_fork_test(self, args, variant="fork"):
+def start_fork_test(self, args, variant="fork", nonstop=False):
 self.build()
 self.prep_debug_monitor_and_inferior(inferior_args=args)
 self.add_qSupported_packets(["multiprocess+",
@@ -22,11 +24,24 @@ def start_fork_test(self, args, variant="fork"):
 self.reset_test_sequence()
 
 # continue and expect fork
-self.test_sequence.add_log_lines([
-"read packet: $c#00",
-{"direction": "send", "regex": self.fork_regex.format(variant),
- "capture": self.fork_capture},
-], True)
+if nonstop:
+self.test_sequence.add_log_lines([
+"read packet: $QNonStop:1#00",
+"send packet: $OK#00",
+"read packet: $c#00",
+"send packet: $OK#00",
+{"direction": "send",
+ "regex": self.fork_regex_nonstop.format(variant),
+ "capture": self.fork_capture},
+"read packet: $vStopped#00",
+"send packet: $OK#00",
+], True)
+else:
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+{"direction": "send", "regex": self.fork_regex.format(variant),
+ "capture": self.fork_capture},
+], True)
 ret = self.expect_gdbremote_sequence()
 self.reset_test_sequence()
 
@@ -45,9 +60,9 @@ def test_fork_multithreaded(self):
 ], True)
 self.expect_gdbremote_sequence()
 
-def fork_and_detach_test(self, variant):
+def fork_and_detach_test(self, variant, nonstop=False):
 parent_pid, parent_tid, child_pid, child_tid = (
-self.start_fork_test([variant], variant))
+self.start_fork_test([variant], variant, nonstop=nonstop))
 
 # detach the forked child
 self.test_sequence.add_log_lines([
@@ -77,6 +92,20 @@ def test_fork(self):
 ], True)
 self.expect_gdbremote_sequence()
 
+@add_test_categories(["fork"])
+def test_fork_nonstop(self):
+parent_pid, _ = self.fork_and_detach_test("fork", nonstop=True)
+
+# resume the parent
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+"send packet: $OK#00",
+"send packet: %Stop:W00;process:{}#00".format(parent_pid),
+"read packet: $vStopped#00",
+"send packet: $OK#00",
+], True)
+self.expect_gdbremote_sequence()
+
 @add_test_categories(["fork"])
 def test_vfork(self):
 parent_pid, parent_tid = self.fork_and_detach_test("vfork")
@@ -93,9 +122,32 @@ def test_vfork(self):
 ], True)
 self.expect_gdbremote_sequence()
 
-def fork_and_follow_test(self, variant):
+@add_test_categories(["fork"])
+def test_vfork_nonstop(self):
+parent_pid, parent_tid = self.fork_and_detach_test("vfork",
+   nonstop=True)
+
+# resume the parent
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+"send packet: $OK#00",
+{"direction": "send",
+ "regex": r"%Stop:T05thread:p{}[.]{}.*vforkdone.*".format(
+ parent_pid, parent_tid),
+ },
+   

[Lldb-commits] [PATCH] D128698: [lldb] Add a NativeProcessProtocol::Threads() iterable

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe095cddb7622: [lldb] Add a NativeProcessProtocol::Threads() 
iterable (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128698

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp
  lldb/source/Plugins/Process/Linux/IntelPTMultiCoreTrace.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -724,17 +724,12 @@
   json::Array threads_array;
 
   // Ensure we can get info on the given thread.
-  uint32_t thread_idx = 0;
-  for (NativeThreadProtocol *thread;
-   (thread = process.GetThreadAtIndex(thread_idx)) != nullptr;
-   ++thread_idx) {
-
-lldb::tid_t tid = thread->GetID();
-
+  for (NativeThreadProtocol &thread : process.Threads()) {
+lldb::tid_t tid = thread.GetID();
 // Grab the reason this thread stopped.
 struct ThreadStopInfo tid_stop_info;
 std::string description;
-if (!thread->GetStopReason(tid_stop_info, description))
+if (!thread.GetStopReason(tid_stop_info, description))
   return llvm::make_error(
   "failed to get stop reason", llvm::inconvertibleErrorCode());
 
@@ -751,7 +746,7 @@
 json::Object thread_obj;
 
 if (!abridged) {
-  if (llvm::Optional registers = GetRegistersAsJSON(*thread))
+  if (llvm::Optional registers = GetRegistersAsJSON(thread))
 thread_obj.try_emplace("registers", std::move(*registers));
 }
 
@@ -760,7 +755,7 @@
 if (signum != 0)
   thread_obj.try_emplace("signal", signum);
 
-const std::string thread_name = thread->GetName();
+const std::string thread_name = thread.GetName();
 if (!thread_name.empty())
   thread_obj.try_emplace("name", thread_name);
 
@@ -856,14 +851,12 @@
   if (m_list_threads_in_stop_reply) {
 response.PutCString("threads:");
 
-uint32_t thread_index = 0;
-NativeThreadProtocol *listed_thread;
-for (listed_thread = process.GetThreadAtIndex(thread_index); listed_thread;
- ++thread_index,
-listed_thread = process.GetThreadAtIndex(thread_index)) {
-  if (thread_index > 0)
+uint32_t thread_num = 0;
+for (NativeThreadProtocol &listed_thread : process.Threads()) {
+  if (thread_num > 0)
 response.PutChar(',');
-  response.Printf("%" PRIx64, listed_thread->GetID());
+  response.Printf("%" PRIx64, listed_thread.GetID());
+  ++thread_num;
 }
 response.PutChar(';');
 
@@ -872,7 +865,7 @@
 // is hex ascii JSON that contains the thread IDs thread stop info only for
 // threads that have stop reasons. Only send this if we have more than one
 // thread otherwise this packet has all the info it needs.
-if (thread_index > 1) {
+if (thread_num > 1) {
   const bool threads_with_valid_stop_info_only = true;
   llvm::Expected threads_info = GetJSONThreadsInfo(
   *m_current_process, threads_with_valid_stop_info_only);
@@ -889,12 +882,10 @@
   }
 }
 
-uint32_t i = 0;
 response.PutCString("thread-pcs");
 char delimiter = ':';
-for (NativeThreadProtocol *thread;
- (thread = process.GetThreadAtIndex(i)) != nullptr; ++i) {
-  NativeRegisterContext ®_ctx = thread->GetRegisterContext();
+for (NativeThreadProtocol &thread : process.Threads()) {
+  NativeRegisterContext ®_ctx = thread.GetRegisterContext();
 
   uint32_t reg_to_read = reg_ctx.ConvertRegisterKindToRegisterNumber(
   eRegisterKindGeneric, LLDB_REGNUM_GENERIC_PC);
@@ -1024,12 +1015,10 @@
   if (!m_non_stop)
 return;
 
-  uint32_t thread_index = 0;
-  while (NativeThreadProtocol *listed_thread =
- m_current_process->GetThreadAtIndex(thread_index++)) {
-if (listed_thread->GetID() != thread_to_skip)
+  for (NativeThreadProtocol &listed_thread : m_current_process->Threads()) {
+if (listed_thread.GetID() != thread_to_skip)
   m_stop_notification_queue.push_back(
-  PrepareStopReplyPacketForThread(*listed_thread).GetString().str());
+  PrepareStopReplyPacketForThread(listed_thread).GetString().str());
   }
 }
 
@@ -1990,15 +1979,10 @@
 return;
 
   LLDB_LOG(log, "iterating over threads of process {0}", process.GetID());
-  NativeThreadProtocol *thread;
-  uint32_t thread_index;
-  for (thread_index = 0, thread = process.GetThreadAtIndex(thread_index);
-   thread;
-   ++thread_index, thread = process.GetThreadAtIndex(thread_index)) {
-LLDB_LOG(log, "iterated thread {0} (tid={1

[Lldb-commits] [lldb] 261d003 - [lldb] [llgs] Fix premature server exit if multiprocess+nonstop

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2022-06-28T21:49:17+02:00
New Revision: 261d0033500eaa281b74c9d54ae240257a0ea00c

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

LOG: [lldb] [llgs] Fix premature server exit if multiprocess+nonstop

Fix lldb-server in the non-stop + multiprocess mode to exit on vStopped
only if all processes have exited, rather than when the first one exits.

Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D128639

Added: 


Modified: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index 49125f91d743a..63174ef552196 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -3860,9 +3860,9 @@ GDBRemoteCommunicationServerLLGS::Handle_vStopped(
   m_stop_notification_queue.pop_front();
   if (!m_stop_notification_queue.empty())
 return SendPacketNoLock(m_stop_notification_queue.front());
-  // If this was the last notification and the process exited, terminate
-  // the server.
-  if (m_inferior_prev_state == eStateExited) {
+  // If this was the last notification and all the processes exited,
+  // terminate the server.
+  if (m_debugged_processes.empty()) {
 m_exit_now = true;
 m_mainloop.RequestTermination();
   }

diff  --git a/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py 
b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
index 8b8e30c9b7379..c58f0fc381ebb 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -392,17 +392,17 @@ def test_vkill_both(self):
 def test_vkill_both_nonstop(self):
 self.vkill_test(kill_parent=True, kill_child=True, nonstop=True)
 
-def resume_one_test(self, run_order, use_vCont=False):
+def resume_one_test(self, run_order, use_vCont=False, nonstop=False):
 parent_pid, parent_tid, child_pid, child_tid = (
-self.start_fork_test(["fork", "trap"]))
+self.start_fork_test(["fork", "trap"], nonstop=nonstop))
 
 parent_expect = [
-"[$]T05thread:p{}.{};.*".format(parent_pid, parent_tid),
-"[$]W00;process:{}#.*".format(parent_pid),
+"T05thread:p{}.{};.*".format(parent_pid, parent_tid),
+"W00;process:{}#.*".format(parent_pid),
 ]
 child_expect = [
-"[$]T05thread:p{}.{};.*".format(child_pid, child_tid),
-"[$]W00;process:{}#.*".format(child_pid),
+"T05thread:p{}.{};.*".format(child_pid, child_tid),
+"W00;process:{}#.*".format(child_pid),
 ]
 
 for x in run_order:
@@ -427,9 +427,17 @@ def resume_one_test(self, run_order, use_vCont=False):
 "send packet: $OK#00",
 "read packet: $c#00",
 ], True)
-self.test_sequence.add_log_lines([
-{"direction": "send", "regex": expect},
-], True)
+if nonstop:
+self.test_sequence.add_log_lines([
+"send packet: $OK#00",
+{"direction": "send", "regex": "%Stop:" + expect},
+"read packet: $vStopped#00",
+"send packet: $OK#00",
+], True)
+else:
+self.test_sequence.add_log_lines([
+{"direction": "send", "regex": "[$]" + expect},
+], True)
 # if at least one process remained, check both PIDs
 if parent_expect or child_expect:
 self.test_sequence.add_log_lines([
@@ -475,6 +483,14 @@ def test_c_child_then_parent(self):
 def test_c_interspersed(self):
 self.resume_one_test(run_order=["parent", "child", "parent", "child"])
 
+@expectedFailureAll(archs=["arm"])  # TODO
+@expectedFailureAll(archs=["aarch64"],
+
bugnumber="https://github.com/llvm/llvm-project/issues/56268";)
+@add_test_categories(["fork"])
+def test_c_interspersed_nonstop(self):
+self.resume_one_test(run_order=["parent", "child", "parent", "child"],
+ nonstop=True)
+
 @expectedFailureAll(archs=["arm"])  # TODO
 @expectedFailureAll(archs=["aarch64"],
 
bugnumber="https://github.com/llvm/llvm-project/issues/56268";)
@@ -513,6 +529,14 @@ def test_vCont_interspersed(self):
 self.resume_one_test(run_order=["parent", "child", "pare

[Lldb-commits] [PATCH] D128638: [lldb] [llgs] Add base nonstop fork/vfork tests

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb415f8e3dfb8: [lldb] [llgs] Add base nonstop fork/vfork 
tests (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128638

Files:
  lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py

Index: lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -9,10 +9,12 @@
 
 fork_regex = ("[$]T05thread:p([0-9a-f]+)[.]([0-9a-f]+);.*"
   "{}:p([0-9a-f]+)[.]([0-9a-f]+).*")
+fork_regex_nonstop = ("%Stop:T05thread:p([0-9a-f]+)[.]([0-9a-f]+);.*"
+  "{}:p([0-9a-f]+)[.]([0-9a-f]+).*")
 fork_capture = {1: "parent_pid", 2: "parent_tid",
 3: "child_pid", 4: "child_tid"}
 
-def start_fork_test(self, args, variant="fork"):
+def start_fork_test(self, args, variant="fork", nonstop=False):
 self.build()
 self.prep_debug_monitor_and_inferior(inferior_args=args)
 self.add_qSupported_packets(["multiprocess+",
@@ -22,11 +24,24 @@
 self.reset_test_sequence()
 
 # continue and expect fork
-self.test_sequence.add_log_lines([
-"read packet: $c#00",
-{"direction": "send", "regex": self.fork_regex.format(variant),
- "capture": self.fork_capture},
-], True)
+if nonstop:
+self.test_sequence.add_log_lines([
+"read packet: $QNonStop:1#00",
+"send packet: $OK#00",
+"read packet: $c#00",
+"send packet: $OK#00",
+{"direction": "send",
+ "regex": self.fork_regex_nonstop.format(variant),
+ "capture": self.fork_capture},
+"read packet: $vStopped#00",
+"send packet: $OK#00",
+], True)
+else:
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+{"direction": "send", "regex": self.fork_regex.format(variant),
+ "capture": self.fork_capture},
+], True)
 ret = self.expect_gdbremote_sequence()
 self.reset_test_sequence()
 
@@ -45,9 +60,9 @@
 ], True)
 self.expect_gdbremote_sequence()
 
-def fork_and_detach_test(self, variant):
+def fork_and_detach_test(self, variant, nonstop=False):
 parent_pid, parent_tid, child_pid, child_tid = (
-self.start_fork_test([variant], variant))
+self.start_fork_test([variant], variant, nonstop=nonstop))
 
 # detach the forked child
 self.test_sequence.add_log_lines([
@@ -77,6 +92,20 @@
 ], True)
 self.expect_gdbremote_sequence()
 
+@add_test_categories(["fork"])
+def test_fork_nonstop(self):
+parent_pid, _ = self.fork_and_detach_test("fork", nonstop=True)
+
+# resume the parent
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+"send packet: $OK#00",
+"send packet: %Stop:W00;process:{}#00".format(parent_pid),
+"read packet: $vStopped#00",
+"send packet: $OK#00",
+], True)
+self.expect_gdbremote_sequence()
+
 @add_test_categories(["fork"])
 def test_vfork(self):
 parent_pid, parent_tid = self.fork_and_detach_test("vfork")
@@ -93,9 +122,32 @@
 ], True)
 self.expect_gdbremote_sequence()
 
-def fork_and_follow_test(self, variant):
+@add_test_categories(["fork"])
+def test_vfork_nonstop(self):
+parent_pid, parent_tid = self.fork_and_detach_test("vfork",
+   nonstop=True)
+
+# resume the parent
+self.test_sequence.add_log_lines([
+"read packet: $c#00",
+"send packet: $OK#00",
+{"direction": "send",
+ "regex": r"%Stop:T05thread:p{}[.]{}.*vforkdone.*".format(
+ parent_pid, parent_tid),
+ },
+"read packet: $vStopped#00",
+"send packet: $OK#00",
+"read packet: $c#00",
+"send packet: $OK#00",
+"send packet: %Stop:W00;process:{}#00".format(parent_pid),
+"read packet: $vStopped#00",
+"send packet: $OK#00",
+], True)
+self.expect_gdbremote_sequence()
+
+def fork_and_follow_test(self, variant, nonstop=False):
 parent_pid, parent_tid, child_pid, child_tid = (
-self.start_fork_test([variant], variant))
+self.start_fork_test([variant], variant, nonstop=nonstop))
 
 # switch to the forked child
 self.test_sequence.add_log_lines([
@@ -113,18 +165,37 @@
  

[Lldb-commits] [PATCH] D128639: [lldb] [llgs] Fix premature server exit if multiprocess+nonstop

[Michał Górny 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 rG261d0033500e: [lldb] [llgs] Fix premature server exit if 
multiprocess+nonstop (authored by mgorny).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D128639?vs=440187&id=440733#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128639

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py


Index: lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -392,17 +392,17 @@
 def test_vkill_both_nonstop(self):
 self.vkill_test(kill_parent=True, kill_child=True, nonstop=True)
 
-def resume_one_test(self, run_order, use_vCont=False):
+def resume_one_test(self, run_order, use_vCont=False, nonstop=False):
 parent_pid, parent_tid, child_pid, child_tid = (
-self.start_fork_test(["fork", "trap"]))
+self.start_fork_test(["fork", "trap"], nonstop=nonstop))
 
 parent_expect = [
-"[$]T05thread:p{}.{};.*".format(parent_pid, parent_tid),
-"[$]W00;process:{}#.*".format(parent_pid),
+"T05thread:p{}.{};.*".format(parent_pid, parent_tid),
+"W00;process:{}#.*".format(parent_pid),
 ]
 child_expect = [
-"[$]T05thread:p{}.{};.*".format(child_pid, child_tid),
-"[$]W00;process:{}#.*".format(child_pid),
+"T05thread:p{}.{};.*".format(child_pid, child_tid),
+"W00;process:{}#.*".format(child_pid),
 ]
 
 for x in run_order:
@@ -427,9 +427,17 @@
 "send packet: $OK#00",
 "read packet: $c#00",
 ], True)
-self.test_sequence.add_log_lines([
-{"direction": "send", "regex": expect},
-], True)
+if nonstop:
+self.test_sequence.add_log_lines([
+"send packet: $OK#00",
+{"direction": "send", "regex": "%Stop:" + expect},
+"read packet: $vStopped#00",
+"send packet: $OK#00",
+], True)
+else:
+self.test_sequence.add_log_lines([
+{"direction": "send", "regex": "[$]" + expect},
+], True)
 # if at least one process remained, check both PIDs
 if parent_expect or child_expect:
 self.test_sequence.add_log_lines([
@@ -475,6 +483,14 @@
 def test_c_interspersed(self):
 self.resume_one_test(run_order=["parent", "child", "parent", "child"])
 
+@expectedFailureAll(archs=["arm"])  # TODO
+@expectedFailureAll(archs=["aarch64"],
+
bugnumber="https://github.com/llvm/llvm-project/issues/56268";)
+@add_test_categories(["fork"])
+def test_c_interspersed_nonstop(self):
+self.resume_one_test(run_order=["parent", "child", "parent", "child"],
+ nonstop=True)
+
 @expectedFailureAll(archs=["arm"])  # TODO
 @expectedFailureAll(archs=["aarch64"],
 
bugnumber="https://github.com/llvm/llvm-project/issues/56268";)
@@ -513,6 +529,14 @@
 self.resume_one_test(run_order=["parent", "child", "parent", "child"],
  use_vCont=True)
 
+@expectedFailureAll(archs=["arm"])  # TODO
+@expectedFailureAll(archs=["aarch64"],
+
bugnumber="https://github.com/llvm/llvm-project/issues/56268";)
+@add_test_categories(["fork"])
+def test_vCont_interspersed_nonstop(self):
+self.resume_one_test(run_order=["parent", "child", "parent", "child"],
+ use_vCont=True, nonstop=True)
+
 @add_test_categories(["fork"])
 def test_vCont_two_processes(self):
 parent_pid, parent_tid, child_pid, child_tid = (
Index: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -3860,9 +3860,9 @@
   m_stop_notification_queue.pop_front();
   if (!m_stop_notification_queue.empty())
 return SendPacketNoLock(m_stop_notification_queue.front());
-  // If this was the last notification and the process exited, terminate
-  // the server.
-  if (m_inferior_prev_state == eStateExited) {
+  // If this was the last notification and all the processes exited,
+  // terminate the server.
+  if (m_debugged_processes.empty(

[Lldb-commits] [PATCH] D128268: [lldb] Fix reading i686-windows executables with GNU environment

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D128268#3614661 , @labath wrote:

> In D128268#3611053 , @mstorsjo 
> wrote:
>
>> The odd thing about the second one is the added hardcoded 
>> `AddArch(ArchSpec("i686-pc-windows"));` and 
>> `AddArch(ArchSpec("i386-pc-windows"));` at the top of `PlatformWindows.cpp`.
>>
>> Anyway, it does seem to work fine in my quick test to get rid of this 
>> duality, so I'll post a patch that does that.
>
> I think those hardcoded lines are there precisely because of the duality 
> (i.e. wanting to support both i386 and i686 regardless of what the host layer 
> reports).

Both that, and because the i386+i686 fat binary arches from ObjectFile 
triggered the more elaborate arch validation, these had to be present to make 
it work. Anyway, after getting rid of the duality, those hardcoded ones don't 
seem to be needed either.

> If that is removed, maybe all of that can be changed to  the pattern used in 
> other platform plugins 
> .

That looks reasonable!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128268

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

[Lldb-commits] [PATCH] D128678: [LLDB] Add PDB/calling-conventions.test for Arm/Windows

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

Thanks, this looks more complete and consistent to me now!

In D128678#3615531 , @labath wrote:

> It seems like this is not actually running the code. Can we make it such that 
> these tests are conditional on the appropriate llvm targets being enabled 
> (instead of the host being of a specific type)?

Well it does need being able to link executables for these targets, which you 
generally can't rely on I think? (I don't think the `%build` macro in general 
works as a cross compiler?)


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

https://reviews.llvm.org/D128678

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

[Lldb-commits] [PATCH] D128678: [LLDB] Add PDB/calling-conventions.test for Arm/Windows

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid added a comment.

In D128678#3616685 , @mstorsjo wrote:

> Thanks, this looks more complete and consistent to me now!
>
> In D128678#3615531 , @labath wrote:
>
>> It seems like this is not actually running the code. Can we make it such 
>> that these tests are conditional on the appropriate llvm targets being 
>> enabled (instead of the host being of a specific type)?
>
> Well it does need being able to link executables for these targets, which you 
> generally can't rely on I think? (I don't think the `%build` macro in general 
> works as a cross compiler?)

We can compile the test on linux as it doesnt require any libraries i-e 
--nodefaultlib. However its MS DIA SDK based PDB test which is only available 
on system windows. NATIVE PDB reader tests are already being run independently.


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

https://reviews.llvm.org/D128678

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

[Lldb-commits] [PATCH] D128694: [lldb/Dataformatters] Adapt C++ std::string dataformatter for D128285

[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 agree with Adrian that we should try to support both the old and new layout. 
Is there a way we can fall back? I think it's fine to land the patch as is to 
unblock the bots and add the fallback later.


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

https://reviews.llvm.org/D128694

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

[Lldb-commits] [PATCH] D128768: [lldb/Core] Fix finite progress event reporting

[Med Ismail Bennani via Phabricator via lldb-commits]

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

This patch should fix event handling for finite progress reports.

Previously, the event handler would get stuck when receiving a finite
progress report, and stop displaying upcoming reports.
This was due to the fact that we were checking if the progress event was
completed by calling `GetCompleted` but returns the completion amount
instead of saying whether it's completed.

That caused the current event id to remain the same, preventing all the
following progress reports to be shown to the user.

This patch also adds some logging to facilitate debugging progress events.

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128768

Files:
  lldb/source/Core/Debugger.cpp


Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1827,6 +1827,12 @@
 }
 
 void Debugger::HandleProgressEvent(const lldb::EventSP &event_sp) {
+  Log *log = GetLog(LLDBLog::Events);
+
+  LLDB_LOGF(log, "%p Debugger('%llu')::HandleProgressEvent (event_sp = {%p})",
+static_cast(this), m_uid,
+static_cast(event_sp.get()));
+
   auto *data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
   if (!data)
 return;
@@ -1835,10 +1841,22 @@
   // going to show the progress.
   const uint64_t id = data->GetID();
   if (m_current_event_id) {
+if (data->IsFinite())
+  LLDB_LOGF(log,
+"%p Debugger('%llu')::HandleProgressEvent (m_current_event_id "
+"= {%llu}, data = {id = %llu, message = %s, completed = %llu, "
+"total = %llu})",
+static_cast(this), m_uid, *m_current_event_id, id,
+data->GetMessage().data(), data->GetCompleted(),
+data->GetTotal());
+else
+  LLDB_LOGF(log,
+"%p Debugger('%llu')::HandleProgressEvent (m_current_event_id "
+"= {%llu}, data = {id = %llu, message = %s, infinite = true})",
+static_cast(this), m_uid, *m_current_event_id, id,
+data->GetMessage().data());
 if (id != *m_current_event_id)
   return;
-if (data->GetCompleted())
-  m_current_event_id.reset();
   } else {
 m_current_event_id = id;
   }
@@ -1860,8 +1878,9 @@
   // Print over previous line, if any.
   output->Printf("\r");
 
-  if (data->GetCompleted()) {
+  if (data->GetCompleted() == data->GetTotal()) {
 // Clear the current line.
+m_current_event_id.reset();
 output->Printf("\x1B[2K");
 output->Flush();
 return;


Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1827,6 +1827,12 @@
 }
 
 void Debugger::HandleProgressEvent(const lldb::EventSP &event_sp) {
+  Log *log = GetLog(LLDBLog::Events);
+
+  LLDB_LOGF(log, "%p Debugger('%llu')::HandleProgressEvent (event_sp = {%p})",
+static_cast(this), m_uid,
+static_cast(event_sp.get()));
+
   auto *data = ProgressEventData::GetEventDataFromEvent(event_sp.get());
   if (!data)
 return;
@@ -1835,10 +1841,22 @@
   // going to show the progress.
   const uint64_t id = data->GetID();
   if (m_current_event_id) {
+if (data->IsFinite())
+  LLDB_LOGF(log,
+"%p Debugger('%llu')::HandleProgressEvent (m_current_event_id "
+"= {%llu}, data = {id = %llu, message = %s, completed = %llu, "
+"total = %llu})",
+static_cast(this), m_uid, *m_current_event_id, id,
+data->GetMessage().data(), data->GetCompleted(),
+data->GetTotal());
+else
+  LLDB_LOGF(log,
+"%p Debugger('%llu')::HandleProgressEvent (m_current_event_id "
+"= {%llu}, data = {id = %llu, message = %s, infinite = true})",
+static_cast(this), m_uid, *m_current_event_id, id,
+data->GetMessage().data());
 if (id != *m_current_event_id)
   return;
-if (data->GetCompleted())
-  m_current_event_id.reset();
   } else {
 m_current_event_id = id;
   }
@@ -1860,8 +1878,9 @@
   // Print over previous line, if any.
   output->Printf("\r");
 
-  if (data->GetCompleted()) {
+  if (data->GetCompleted() == data->GetTotal()) {
 // Clear the current line.
+m_current_event_id.reset();
 output->Printf("\x1B[2K");
 output->Flush();
 return;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D128694: [lldb/Dataformatters] Adapt C++ std::string dataformatter for D128285

[Med Ismail Bennani via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG44a114fec715: [lldb/Dataformatters] Adapt C++ std::string 
dataformatter for D128285 (authored by mib).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128694

Files:
  lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp


Index: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -583,8 +583,18 @@
   : eLibcxxStringLayoutModeCSD;
   uint64_t size_mode_value = 0;
 
-  if (ValueObjectSP is_long = dataval_sp->GetChildAtNamePath(
-  {ConstString("__s"), ConstString("__is_long_")})) {
+  ValueObjectSP short_sp(dataval_sp->GetChildAtIndex(1, true));
+  if (!short_sp)
+return {};
+
+  // After D128285, we need to access the `__is_long_` and `__size_` fields 
from
+  // a packed anonymous struct
+  ValueObjectSP packed_fields_sp = short_sp->GetChildAtIndex(0, true);
+  if (!packed_fields_sp)
+return {};
+
+  if (ValueObjectSP is_long = packed_fields_sp->GetChildMemberWithName(
+  ConstString("__is_long_"), true)) {
 using_bitmasks = false;
 short_mode = !is_long->GetValueAsUnsigned(/*fail_value=*/0);
 if (ValueObjectSP size_member =
@@ -621,14 +631,11 @@
   }
 
   if (short_mode) {
-ValueObjectSP short_sp(dataval_sp->GetChildAtIndex(1, true));
-if (!short_sp)
-  return {};
-ValueObjectSP location_sp = short_sp->GetChildAtIndex(
+ValueObjectSP location_sp = packed_fields_sp->GetChildAtIndex(
 (layout == eLibcxxStringLayoutModeDSC) ? 0 : 1, true);
 // After D125496, there is a flat layout.
 if (location_sp->GetName() == g_size_name)
-  location_sp = short_sp->GetChildAtIndex(3, true);
+  location_sp = short_sp->GetChildMemberWithName(g_data_name, true);
 if (using_bitmasks)
   size = (layout == eLibcxxStringLayoutModeDSC)
  ? size_mode_value
@@ -650,13 +657,19 @@
   ValueObjectSP l(dataval_sp->GetChildAtIndex(0, true));
   if (!l)
 return {};
+
+  // After D128285, we need to access the `__cap_` field from a packed 
anonymous
+  // struct
+  packed_fields_sp = l->GetChildAtIndex(0, true);
+  if (!packed_fields_sp)
+return {};
   // we can use the layout_decider object as the data pointer
   ValueObjectSP location_sp =
   l->GetChildMemberWithName(ConstString("__data_"), /*can_create=*/true);
   ValueObjectSP size_vo =
   l->GetChildMemberWithName(ConstString("__size_"), /*can_create=*/true);
-  ValueObjectSP capacity_vo =
-  l->GetChildMemberWithName(ConstString("__cap_"), /*can_create=*/true);
+  ValueObjectSP capacity_vo = packed_fields_sp->GetChildMemberWithName(
+  ConstString("__cap_"), /*can_create=*/true);
   if (!size_vo || !location_sp || !capacity_vo)
 return {};
   size = size_vo->GetValueAsUnsigned(LLDB_INVALID_OFFSET);


Index: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
===
--- lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -583,8 +583,18 @@
   : eLibcxxStringLayoutModeCSD;
   uint64_t size_mode_value = 0;
 
-  if (ValueObjectSP is_long = dataval_sp->GetChildAtNamePath(
-  {ConstString("__s"), ConstString("__is_long_")})) {
+  ValueObjectSP short_sp(dataval_sp->GetChildAtIndex(1, true));
+  if (!short_sp)
+return {};
+
+  // After D128285, we need to access the `__is_long_` and `__size_` fields from
+  // a packed anonymous struct
+  ValueObjectSP packed_fields_sp = short_sp->GetChildAtIndex(0, true);
+  if (!packed_fields_sp)
+return {};
+
+  if (ValueObjectSP is_long = packed_fields_sp->GetChildMemberWithName(
+  ConstString("__is_long_"), true)) {
 using_bitmasks = false;
 short_mode = !is_long->GetValueAsUnsigned(/*fail_value=*/0);
 if (ValueObjectSP size_member =
@@ -621,14 +631,11 @@
   }
 
   if (short_mode) {
-ValueObjectSP short_sp(dataval_sp->GetChildAtIndex(1, true));
-if (!short_sp)
-  return {};
-ValueObjectSP location_sp = short_sp->GetChildAtIndex(
+ValueObjectSP location_sp = packed_fields_sp->GetChildAtIndex(
 (layout == eLibcxxStringLayoutModeDSC) ? 0 : 1, true);
 // After D125496, there is a flat layout.
 if (location_sp->GetName() == g_size_name)
-  location_sp = short_sp->GetChildAtIndex(3, true);
+  location_sp = short_sp->GetChildMemberWithName(g_data_name, true);
 if (using_bitmasks)
   size = (layout == eLibcxxStringLayoutModeDSC)
  ? size_mode_value
@@ -650,13 +657,19 @@
   ValueObjectSP l(dataval_sp->GetChildAtIndex(0, true));
   if (!l)
 return {};
+
+  // After D128285, we need to access the `

[Lldb-commits] [lldb] 44a114f - [lldb/Dataformatters] Adapt C++ std::string dataformatter for D128285

[Med Ismail Bennani via lldb-commits]

Author: Med Ismail Bennani
Date: 2022-06-28T15:34:17-07:00
New Revision: 44a114fec715bd9c9b725c939a09de4b746108e3

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

LOG: [lldb/Dataformatters] Adapt C++ std::string dataformatter for D128285

This patch changes the C++ `std::string` dataformatter to reflect
internal layout changes following D128285.

Now, in short-mode strings, in order to access the `__size_` and
`__is_long_` attributes, we need to access a packed anonymous struct,
which introduces another indirection.

We need to do the same in order to access the `__cap_` field for
long-mode strings.

This should fix the various test failures that are happening on
GreenDragon:

https://green.lab.llvm.org/green/job/lldb-cmake/44918/

rdar://96010248

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

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 
lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp 
b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
index cd09c3e7d1e9b..18bca3b43406a 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -583,8 +583,18 @@ ExtractLibcxxStringInfo(ValueObject &valobj) {
   : eLibcxxStringLayoutModeCSD;
   uint64_t size_mode_value = 0;
 
-  if (ValueObjectSP is_long = dataval_sp->GetChildAtNamePath(
-  {ConstString("__s"), ConstString("__is_long_")})) {
+  ValueObjectSP short_sp(dataval_sp->GetChildAtIndex(1, true));
+  if (!short_sp)
+return {};
+
+  // After D128285, we need to access the `__is_long_` and `__size_` fields 
from
+  // a packed anonymous struct
+  ValueObjectSP packed_fields_sp = short_sp->GetChildAtIndex(0, true);
+  if (!packed_fields_sp)
+return {};
+
+  if (ValueObjectSP is_long = packed_fields_sp->GetChildMemberWithName(
+  ConstString("__is_long_"), true)) {
 using_bitmasks = false;
 short_mode = !is_long->GetValueAsUnsigned(/*fail_value=*/0);
 if (ValueObjectSP size_member =
@@ -621,14 +631,11 @@ ExtractLibcxxStringInfo(ValueObject &valobj) {
   }
 
   if (short_mode) {
-ValueObjectSP short_sp(dataval_sp->GetChildAtIndex(1, true));
-if (!short_sp)
-  return {};
-ValueObjectSP location_sp = short_sp->GetChildAtIndex(
+ValueObjectSP location_sp = packed_fields_sp->GetChildAtIndex(
 (layout == eLibcxxStringLayoutModeDSC) ? 0 : 1, true);
 // After D125496, there is a flat layout.
 if (location_sp->GetName() == g_size_name)
-  location_sp = short_sp->GetChildAtIndex(3, true);
+  location_sp = short_sp->GetChildMemberWithName(g_data_name, true);
 if (using_bitmasks)
   size = (layout == eLibcxxStringLayoutModeDSC)
  ? size_mode_value
@@ -650,13 +657,19 @@ ExtractLibcxxStringInfo(ValueObject &valobj) {
   ValueObjectSP l(dataval_sp->GetChildAtIndex(0, true));
   if (!l)
 return {};
+
+  // After D128285, we need to access the `__cap_` field from a packed 
anonymous
+  // struct
+  packed_fields_sp = l->GetChildAtIndex(0, true);
+  if (!packed_fields_sp)
+return {};
   // we can use the layout_decider object as the data pointer
   ValueObjectSP location_sp =
   l->GetChildMemberWithName(ConstString("__data_"), /*can_create=*/true);
   ValueObjectSP size_vo =
   l->GetChildMemberWithName(ConstString("__size_"), /*can_create=*/true);
-  ValueObjectSP capacity_vo =
-  l->GetChildMemberWithName(ConstString("__cap_"), /*can_create=*/true);
+  ValueObjectSP capacity_vo = packed_fields_sp->GetChildMemberWithName(
+  ConstString("__cap_"), /*can_create=*/true);
   if (!size_vo || !location_sp || !capacity_vo)
 return {};
   size = size_vo->GetValueAsUnsigned(LLDB_INVALID_OFFSET);



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

[Lldb-commits] [PATCH] D128638: [lldb] [llgs] Add base nonstop fork/vfork tests

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added a comment.
Herald added a subscriber: JDevlieghere.

Hi @mgorny,

There are some random test failures following this patch on 
`lldb-debian-x86_64`: https://lab.llvm.org/buildbot/#/builders/68/builds/34883

  
  Unresolved Tests (1):
lldb-api :: tools/lldb-server/TestGdbRemoteFork.py

Would you mind taking a look at this ? Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128638

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

[Lldb-commits] [PATCH] D128477: [trace] Add a flag to the decoder to output the instruction type

[Sujin Park via Phabricator via lldb-commits]

persona0220 updated this revision to Diff 440806.
persona0220 added a comment.
Herald added subscribers: Michael137, MaskRay.

This new diff attach additional metadata 'control flow kind' for instruction.
Add '-k / --kind' flag for both `disassemble` and `thread trace dump` command.

Thanks to @jingham for the suggestion!

Sample output:

  (lldb) disas -k
  a.out`main:
  0x4005b6 <+0>:  other pushq  %rbp
  0x4005b7 <+1>:  other movq   %rsp, %rbp
  0x4005ba <+4>:  other movl   $0x400668, %edi   ; imm = 
0x400668
  0x4005bf <+9>:  call  callq  0x4004c0  ; symbol stub 
for: puts
  ->  0x4005c4 <+14>: other movl   $0x0, %eax
  0x4005c9 <+19>: other popq   %rbp
  0x4005ca <+20>: returnretq
  
  (lldb) thread trace dump instruction --kind
  thread #1: tid = 1893259
libc.so.6`write + 30
  2027: 0x771fa86econd jump ja 0x1208c8  ; 
<+120>
  2026: 0x771fa868other cmpq   $-0x1000, %rax; 
imm = 0xF000
  2025: 0x771fa866far call  syscall
  2024: 0x771fa861other movl   $0x1, %eax
  2023: 0x771fa85fcond jump jne0x120878  ; 
<+40>
  2022: 0x771fa85dother testl  %eax, %eax
  2021: 0x771fa85bother movl   (%rax), %eax
  2020: 0x771fa854other leaq   0x2a4d35(%rip), %rax  ; 
__libc_multiple_threads
  2019: 0x771fa850other endbr64


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

https://reviews.llvm.org/D128477

Files:
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/test/Driver/aarch64-target-as-march.s
  clang/test/Driver/arm-cortex-cpus-2.c
  clang/test/Driver/arm-ias-Wa.s
  clang/test/Preprocessor/aarch64-target-features.c
  lldb/include/lldb/Core/Disassembler.h
  lldb/include/lldb/Target/TraceCursor.h
  lldb/include/lldb/Target/TraceInstructionDumper.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/API/SBInstruction.cpp
  lldb/source/API/SBInstructionList.cpp
  lldb/source/Commands/CommandObjectDisassemble.cpp
  lldb/source/Commands/CommandObjectDisassemble.h
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/Options.td
  lldb/source/Core/Disassembler.cpp
  lldb/source/Core/DumpDataExtractor.cpp
  lldb/source/Expression/IRExecutionUnit.cpp
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
  lldb/source/Plugins/TraceExporter/common/TraceHTR.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/TraceCursor.cpp
  lldb/source/Target/TraceInstructionDumper.cpp

Index: lldb/source/Target/TraceInstructionDumper.cpp
===
--- lldb/source/Target/TraceInstructionDumper.cpp
+++ lldb/source/Target/TraceInstructionDumper.cpp
@@ -151,6 +151,7 @@
 insn.symbol_info->instruction->Dump(&m_s, /*max_opcode_byte_size=*/0,
 /*show_address=*/false,
 /*show_bytes=*/false,
+/*show_kind=*/m_options.show_kind,
 &insn.symbol_info->exe_ctx,
 &insn.symbol_info->sc,
 /*prev_sym_ctx=*/nullptr,
Index: lldb/source/Target/TraceCursor.cpp
===
--- lldb/source/Target/TraceCursor.cpp
+++ lldb/source/Target/TraceCursor.cpp
@@ -21,15 +21,6 @@
   return m_exe_ctx_ref;
 }
 
-void TraceCursor::SetGranularity(
-lldb::TraceInstructionControlFlowType granularity) {
-  m_granularity = granularity;
-}
-
-void TraceCursor::SetIgnoreErrors(bool ignore_errors) {
-  m_ignore_errors = ignore_errors;
-}
-
 void TraceCursor::SetForwards(bool forwards) { m_forwards = forwards; }
 
 bool TraceCursor::IsForwards() const { return m_forwards; }
Index: lldb/source/Target/ThreadPlanTracer.cpp
===
--- lldb/source/Target/ThreadPlanTracer.cpp
+++ lldb/source/Target/ThreadPlanTracer.cpp
@@ -170,12 +170,13 @@
   if (instruction_list.GetSize()) {
 const bool show_bytes = true;
 const bool show_address = true;
+const bool show_kind = true;
 Instruction *instruction =
 instruction_list.GetInstructionAtIndex(0).get();
 const FormatEntity::Entry *disassemble_format =
 m_process.GetTarget().GetDebugger().GetDisassemblyFormat();
 instru

[Lldb-commits] [PATCH] D128477: [trace] Add a flag to the decoder to output the instruction type

[walter erquinigo via Phabricator via lldb-commits]

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

remove unwanted files


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

https://reviews.llvm.org/D128477

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

[Lldb-commits] [PATCH] D128477: [trace] Add a flag to the decoder to output the instruction type

[Sujin Park via Phabricator via lldb-commits]

persona0220 updated this revision to Diff 440808.

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

https://reviews.llvm.org/D128477

Files:
  lldb/include/lldb/Core/Disassembler.h
  lldb/include/lldb/Target/TraceCursor.h
  lldb/include/lldb/Target/TraceInstructionDumper.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/API/SBInstruction.cpp
  lldb/source/API/SBInstructionList.cpp
  lldb/source/Commands/CommandObjectDisassemble.cpp
  lldb/source/Commands/CommandObjectDisassemble.h
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/Options.td
  lldb/source/Core/Disassembler.cpp
  lldb/source/Core/DumpDataExtractor.cpp
  lldb/source/Expression/IRExecutionUnit.cpp
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
  lldb/source/Plugins/TraceExporter/common/TraceHTR.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/TraceCursor.cpp
  lldb/source/Target/TraceInstructionDumper.cpp

Index: lldb/source/Target/TraceInstructionDumper.cpp
===
--- lldb/source/Target/TraceInstructionDumper.cpp
+++ lldb/source/Target/TraceInstructionDumper.cpp
@@ -151,6 +151,7 @@
 insn.symbol_info->instruction->Dump(&m_s, /*max_opcode_byte_size=*/0,
 /*show_address=*/false,
 /*show_bytes=*/false,
+/*show_kind=*/m_options.show_kind,
 &insn.symbol_info->exe_ctx,
 &insn.symbol_info->sc,
 /*prev_sym_ctx=*/nullptr,
Index: lldb/source/Target/TraceCursor.cpp
===
--- lldb/source/Target/TraceCursor.cpp
+++ lldb/source/Target/TraceCursor.cpp
@@ -21,15 +21,6 @@
   return m_exe_ctx_ref;
 }
 
-void TraceCursor::SetGranularity(
-lldb::TraceInstructionControlFlowType granularity) {
-  m_granularity = granularity;
-}
-
-void TraceCursor::SetIgnoreErrors(bool ignore_errors) {
-  m_ignore_errors = ignore_errors;
-}
-
 void TraceCursor::SetForwards(bool forwards) { m_forwards = forwards; }
 
 bool TraceCursor::IsForwards() const { return m_forwards; }
Index: lldb/source/Target/ThreadPlanTracer.cpp
===
--- lldb/source/Target/ThreadPlanTracer.cpp
+++ lldb/source/Target/ThreadPlanTracer.cpp
@@ -170,12 +170,13 @@
   if (instruction_list.GetSize()) {
 const bool show_bytes = true;
 const bool show_address = true;
+const bool show_kind = true;
 Instruction *instruction =
 instruction_list.GetInstructionAtIndex(0).get();
 const FormatEntity::Entry *disassemble_format =
 m_process.GetTarget().GetDebugger().GetDisassemblyFormat();
 instruction->Dump(stream, max_opcode_byte_size, show_address,
-  show_bytes, nullptr, nullptr, nullptr,
+  show_bytes, show_kind, nullptr, nullptr, nullptr,
   disassemble_format, 0);
   }
 }
Index: lldb/source/Symbol/Symbol.cpp
===
--- lldb/source/Symbol/Symbol.cpp
+++ lldb/source/Symbol/Symbol.cpp
@@ -558,8 +558,9 @@
   if (disassembler_sp) {
 const bool show_address = true;
 const bool show_bytes = false;
+const bool show_kind = false;
 disassembler_sp->GetInstructionList().Dump(&strm, show_address, show_bytes,
-   &exe_ctx);
+   show_kind, &exe_ctx);
 return true;
   }
   return false;
Index: lldb/source/Symbol/Function.cpp
===
--- lldb/source/Symbol/Function.cpp
+++ lldb/source/Symbol/Function.cpp
@@ -440,8 +440,9 @@
   if (disassembler_sp) {
 const bool show_address = true;
 const bool show_bytes = false;
+const bool show_kind = false;
 disassembler_sp->GetInstructionList().Dump(&strm, show_address, show_bytes,
-   &exe_ctx);
+   show_kind, &exe_ctx);
 return true;
   }
   return false;
Index: lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
===
--- lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
+++ lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cp

[Lldb-commits] [PATCH] D128543: [trace] Improve the TraceCursor iteration API

[walter erquinigo via Phabricator via lldb-commits]

wallace added inline comments.



Comment at: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp:38
+void TraceCursorIntelPT::Next() {
+  m_pos += IsForwards() ? 1 : -1;
 

jj10306 wrote:
> should only do this increment or decrement if `HasValue()` is true? otherwise 
> (in theory) this value could wrap around if it's incremented/decremented too 
> many times?
i think that's a very extreme case =P



Comment at: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp:40
 
+  {
+// We try to go to a neighbor tsc range that might contain the current pos

jj10306 wrote:
> why is this new scope introduced here?
i'll remove it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128543

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

[Lldb-commits] [lldb] f91d828 - [trace] Improve the TraceCursor iteration API

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2022-06-28T16:50:12-07:00
New Revision: f91d82816ff5a88a59e86b54a4d63547776d4854

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

LOG: [trace] Improve the TraceCursor iteration API

The current way ot traversing the cursor is a bit uncommon and it can't handle 
empty traces, in fact, its invariant is that it shold always point to a valid 
item. This diff simplifies the cursor API and allows it to point to invalid 
items, thus being able to handle empty traces or to know it ran out of data.

- Removed all the granularity functionalities, because we are not actually 
making use of that. We can bring them back when they are actually needed.
- change the looping logic to the following:

```
  for (; cursor->HasValue(); cursor->Next()) {
 if (cursor->IsError()) {
   .. do something for error
   continue;
 }
 .. do something for instruction
  }

```

- added a HasValue method that can be used to identify if the cursor ran out of 
data, the trace is empty, or the user tried to move to an invalid position via 
SetId() or Seek()
- made several simplifications to severals parts of the code.

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

Added: 


Modified: 
lldb/include/lldb/Target/TraceCursor.h
lldb/include/lldb/Target/TraceInstructionDumper.h
lldb/source/Commands/CommandObjectThread.cpp
lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp
lldb/source/Target/TraceCursor.cpp
lldb/source/Target/TraceInstructionDumper.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/TraceCursor.h 
b/lldb/include/lldb/Target/TraceCursor.h
index 79c4cd371e4f4..7640fed93b99f 100644
--- a/lldb/include/lldb/Target/TraceCursor.h
+++ b/lldb/include/lldb/Target/TraceCursor.h
@@ -25,51 +25,66 @@ namespace lldb_private {
 /// Live processes:
 ///  In the case of a live process trace, an instance of a \a TraceCursor 
should
 ///  point to the trace at the moment it was collected. If the process is later
-///  resumed and new trace data is collected, that should leave that old cursor
-///  unaffected.
+///  resumed and new trace data is collected, then it's up to each trace 
plug-in
+///  to decide whether to leave the old cursor unaffected or not.
 ///
 /// Errors in the trace:
 ///  As there could be errors when reconstructing the instructions of a trace,
 ///  these errors are represented as failed instructions, and the cursor can
-///  point at them. The consumer should invoke \a TraceCursor::GetError() to
-///  check if the cursor is pointing to either a valid instruction or an error.
+///  point at them. The consumer should invoke \a TraceCursor::IsError() to
+///  check if the cursor is pointing to either a valid instruction or an error,
+///  and then \a TraceCursor::GetError() can return the actual error message.
 ///
 /// Instructions:
 ///  A \a TraceCursor always points to a specific instruction or error in the
 ///  trace.
 ///
 /// Defaults:
-///   By default, the cursor points at the end item of the trace, moves
-///   backwards, has a move granularity of \a
-///   eTraceInstructionControlFlowTypeInstruction (i.e. visit every 
instruction)
-///   and stops at every error (the "ignore errors" flag is \b false). See the
-///   \a TraceCursor::Next() method for more documentation.
+///   By default, the cursor points at the most recent item in the trace and is
+///   set up to iterate backwards. See the \a TraceCursor::Next() method for
+///   more documentation.
 ///
 /// Sample usage:
 ///
 ///  TraceCursorUP cursor = trace.GetTrace(thread);
 ///
-///  cursor->SetGranularity(eTraceInstructionControlFlowTypeCall |
-///eTraceInstructionControlFlowTypeReturn);
+///  for (; cursor->HasValue(); cursor->Next()) {
+/// if (cursor->IsError()) {
+///   cout << "error found at: " << cursor->GetError() << endl;
+///   continue;
+/// }
 ///
-///  do {
-/// if (llvm::Error error = cursor->GetError())
-///   cout << "error found at: " << llvm::toString(error) << endl;
-/// else if (cursor->GetInstructionControlFlowType() &
-/// eTraceInstructionControlFlowTypeCall)
-///   std::cout << "call found at " << cursor->GetLoadAddress() <<
-///   std::endl;
-/// else if (cursor->GetInstructionControlFlowType() &
-/// eTraceInstructionControlFlowTypeReturn)
-///   std::cout << "return found at " << cursor->GetLoadAddress() <<
-///   std::endl;
-///  } while(cursor->Next());
+/// switch (cursor->GetInstructionControlFlowType()) {
+///   eTraceInstructionControlFlowTypeCall:
+///

[Lldb-commits] [PATCH] D128543: [trace] Improve the TraceCursor iteration API

[Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf91d82816ff5: [trace] Improve the TraceCursor iteration API 
(authored by Walter Erquinigo ).

Changed prior to commit:
  https://reviews.llvm.org/D128543?vs=439827&id=440810#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128543

Files:
  lldb/include/lldb/Target/TraceCursor.h
  lldb/include/lldb/Target/TraceInstructionDumper.h
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
  lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp
  lldb/source/Target/TraceCursor.cpp
  lldb/source/Target/TraceInstructionDumper.cpp

Index: lldb/source/Target/TraceInstructionDumper.cpp
===
--- lldb/source/Target/TraceInstructionDumper.cpp
+++ lldb/source/Target/TraceInstructionDumper.cpp
@@ -261,33 +261,20 @@
 : m_cursor_up(std::move(cursor_up)), m_options(options),
   m_writer_up(CreateWriter(s, m_options)) {
 
-  if (m_options.id) {
-if (!m_cursor_up->GoToId(*m_options.id)) {
-  m_writer_up->InfoMessage("invalid instruction id");
-  SetNoMoreData();
-}
-  } else if (m_options.forwards) {
+  if (m_options.id)
+m_cursor_up->GoToId(*m_options.id);
+  else if (m_options.forwards)
 m_cursor_up->Seek(0, TraceCursor::SeekType::Beginning);
-  } else {
+  else
 m_cursor_up->Seek(0, TraceCursor::SeekType::End);
-  }
 
   m_cursor_up->SetForwards(m_options.forwards);
   if (m_options.skip) {
-uint64_t to_skip = *m_options.skip;
-if (m_cursor_up->Seek((m_options.forwards ? 1 : -1) * to_skip,
-  TraceCursor::SeekType::Current) < to_skip) {
-  // This happens when the skip value was more than the number of
-  // available instructions.
-  SetNoMoreData();
-}
+m_cursor_up->Seek((m_options.forwards ? 1 : -1) * *m_options.skip,
+  TraceCursor::SeekType::Current);
   }
 }
 
-void TraceInstructionDumper::SetNoMoreData() { m_no_more_data = true; }
-
-bool TraceInstructionDumper::HasMoreData() { return !m_no_more_data; }
-
 TraceInstructionDumper::InstructionEntry
 TraceInstructionDumper::CreatRawInstructionEntry() {
   InstructionEntry insn;
@@ -375,11 +362,8 @@
   ExecutionContext exe_ctx;
   thread_sp->GetProcess()->GetTarget().CalculateExecutionContext(exe_ctx);
 
-  for (size_t i = 0; i < count; i++) {
-if (!HasMoreData()) {
-  m_writer_up->InfoMessage("no more data");
-  break;
-}
+  for (size_t i = 0; i < count && m_cursor_up->HasValue();
+   m_cursor_up->Next(), i++) {
 last_id = m_cursor_up->GetId();
 
 if (m_options.forwards) {
@@ -418,9 +402,8 @@
   // makes sense.
   PrintEvents();
 }
-
-if (!m_cursor_up->Next())
-  SetNoMoreData();
   }
+  if (!m_cursor_up->HasValue())
+m_writer_up->InfoMessage("no more data");
   return last_id;
 }
Index: lldb/source/Target/TraceCursor.cpp
===
--- lldb/source/Target/TraceCursor.cpp
+++ lldb/source/Target/TraceCursor.cpp
@@ -21,15 +21,6 @@
   return m_exe_ctx_ref;
 }
 
-void TraceCursor::SetGranularity(
-lldb::TraceInstructionControlFlowType granularity) {
-  m_granularity = granularity;
-}
-
-void TraceCursor::SetIgnoreErrors(bool ignore_errors) {
-  m_ignore_errors = ignore_errors;
-}
-
 void TraceCursor::SetForwards(bool forwards) { m_forwards = forwards; }
 
 bool TraceCursor::IsForwards() const { return m_forwards; }
Index: lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp
===
--- lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp
+++ lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp
@@ -154,7 +154,8 @@
   // TODO: Make distinction between errors by storing the error messages.
   // Currently, all errors are treated the same.
   m_instruction_layer_up->AppendInstruction(0);
-  more_data_in_trace = cursor.Next();
+  cursor.Next();
+  more_data_in_trace = cursor.HasValue();
 } else {
   lldb::addr_t current_instruction_load_address = cursor.GetLoadAddress();
   lldb::TraceInstructionControlFlowType current_instruction_type =
@@ -162,7 +163,8 @@
 
   m_instruction_layer_up->AppendInstruction(
   current_instruction_load_address);
-  more_data_in_trace = cursor.Next();
+  cursor.Next();
+  more_data_in_trace = cursor.HasValue();
   if (current_instruction_type &
   lldb::eTraceInstructionControlFlowTypeCall) {
 if (more_data_in_trace && !cursor.IsError()) {
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
===
--- lldb/