[Lldb-commits] [lldb] f659bf0 - [lldb] [test] Add synchronization fix Subprocess test flakiness

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-09-10T09:11:08+02:00
New Revision: f659bf00b4c0f33947bbce19113ac7cd28e5da86

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

LOG: [lldb] [test] Add synchronization fix Subprocess test flakiness

Add synchronization routines to ensure that Subprocess tests output
in a predictable order, and all test strings are output before the tests
terminate.

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

Added: 


Modified: 
lldb/test/Shell/Subprocess/Inputs/fork.cpp
lldb/test/Shell/Subprocess/clone-follow-child-softbp.test
lldb/test/Shell/Subprocess/clone-follow-child-wp.test
lldb/test/Shell/Subprocess/clone-follow-child.test
lldb/test/Shell/Subprocess/clone-follow-parent-softbp.test
lldb/test/Shell/Subprocess/clone-follow-parent-wp.test
lldb/test/Shell/Subprocess/clone-follow-parent.test
lldb/test/Shell/Subprocess/fork-follow-child-softbp.test
lldb/test/Shell/Subprocess/fork-follow-child-wp.test
lldb/test/Shell/Subprocess/fork-follow-child.test
lldb/test/Shell/Subprocess/fork-follow-parent-softbp.test
lldb/test/Shell/Subprocess/fork-follow-parent-wp.test
lldb/test/Shell/Subprocess/fork-follow-parent.test
lldb/test/Shell/Subprocess/vfork-follow-child-softbp.test
lldb/test/Shell/Subprocess/vfork-follow-child-wp.test
lldb/test/Shell/Subprocess/vfork-follow-child.test
lldb/test/Shell/Subprocess/vfork-follow-parent-softbp.test
lldb/test/Shell/Subprocess/vfork-follow-parent-wp.test
lldb/test/Shell/Subprocess/vfork-follow-parent.test

Removed: 




diff  --git a/lldb/test/Shell/Subprocess/Inputs/fork.cpp 
b/lldb/test/Shell/Subprocess/Inputs/fork.cpp
index 44242934bd3e..f4cf1c2c42dc 100644
--- a/lldb/test/Shell/Subprocess/Inputs/fork.cpp
+++ b/lldb/test/Shell/Subprocess/Inputs/fork.cpp
@@ -5,6 +5,7 @@
 #include 
 #endif
 #include 
+#include 
 #include 
 #include 
 
@@ -15,28 +16,87 @@ void parent_func() {
   printf("function run in parent\n");
 }
 
-int child_func(void *unused) {
+int parent_done[2];
+char parent_done_str[16];
+
+void wait_for_parent() {
+  char buf[2];
+  // wait for the parent to finish its part
+  int ret = read(parent_done[0], buf, sizeof(buf));
+  assert(ret == 2);
+  ret = close(parent_done[0]);
+  assert(ret == 0);
+}
+
+int child_func(void *argv0_ptr) {
+  const char *argv0 = static_cast(argv0_ptr);
+
+  // NB: when using vfork(), the parent may be suspended while running
+  // this function, so do not rely on any synchronization until we exec
+#if defined(TEST_FORK)
+  if (TEST_FORK != vfork)
+#endif
+wait_for_parent();
+
+  int ret = close(parent_done[1]);
+  assert(ret == 0);
+
   // we need to avoid memory modifications for vfork(), yet we want
   // to be able to test watchpoints, so do the next best thing
   // and restore the original value
   g_val = 2;
   g_val = 0;
-  return 0;
+  execl(argv0, argv0, parent_done_str, NULL);
+  assert(0 && "this should not be reached");
+  return 1;
 }
 
-int main() {
+int main(int argc, char* argv[]) {
   alignas(uintmax_t) char stack[4096];
+  int ret;
+
+  if (argv[1]) {
+parent_done[0] = atoi(argv[1]);
+assert(parent_done[0] != 0);
+
+#if defined(TEST_FORK)
+// for vfork(), we need to synchronize after exec
+if (TEST_FORK == vfork)
+  wait_for_parent();
+#endif
+
+fprintf(stderr, "function run in exec'd child\n");
+return 0;
+  }
+
+  ret = pipe(parent_done);
+  assert(ret == 0);
+
+  ret = snprintf(parent_done_str, sizeof(parent_done_str),
+ "%d", parent_done[0]);
+  assert(ret != -1);
 
 #if defined(TEST_CLONE)
-  pid_t pid = clone(child_func, &stack[sizeof(stack)], 0, NULL);
+  pid_t pid = clone(child_func, &stack[sizeof(stack)], 0, argv[0]);
 #elif defined(TEST_FORK)
   pid_t pid = TEST_FORK();
+  // NB: this must be equivalent to the clone() call above
   if (pid == 0)
-_exit(child_func(NULL));
+_exit(child_func(argv[0]));
 #endif
   assert(pid != -1);
 
+  ret = close(parent_done[0]);
+  assert(ret == 0);
+
   parent_func();
+
+  // resume the child
+  ret = write(parent_done[1], "go", 2);
+  assert(ret == 2);
+  ret = close(parent_done[1]);
+  assert(ret == 0);
+
   int status, wait_flags = 0;
 #if defined(TEST_CLONE)
   wait_flags = __WALL;
@@ -44,7 +104,7 @@ int main() {
   pid_t waited = waitpid(pid, &status, wait_flags);
   assert(waited == pid);
   assert(WIFEXITED(status));
-  printf("child exited: %d\n", WEXITSTATUS(status));
+  assert(WEXITSTATUS(status) == 0);
 
   return 0;
 }

diff  --git a/lldb/test/Shell/Subprocess/clone-follow-child-softbp.test 
b/lldb/test/Shell/Subprocess/clone-follow-child-softbp.test
index f4fe8588ca32..f67b5a95e9cf 100644
--- a/lldb/test/Shell/Subprocess/clone-follow-child-softbp.test
+++ b/lldb/test/Shell/Subproces

[Lldb-commits] [PATCH] D109495: [lldb] [test] Add synchronization fix Subprocess test flakiness

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf659bf00b4c0: [lldb] [test] Add synchronization fix 
Subprocess test flakiness (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109495

Files:
  lldb/test/Shell/Subprocess/Inputs/fork.cpp
  lldb/test/Shell/Subprocess/clone-follow-child-softbp.test
  lldb/test/Shell/Subprocess/clone-follow-child-wp.test
  lldb/test/Shell/Subprocess/clone-follow-child.test
  lldb/test/Shell/Subprocess/clone-follow-parent-softbp.test
  lldb/test/Shell/Subprocess/clone-follow-parent-wp.test
  lldb/test/Shell/Subprocess/clone-follow-parent.test
  lldb/test/Shell/Subprocess/fork-follow-child-softbp.test
  lldb/test/Shell/Subprocess/fork-follow-child-wp.test
  lldb/test/Shell/Subprocess/fork-follow-child.test
  lldb/test/Shell/Subprocess/fork-follow-parent-softbp.test
  lldb/test/Shell/Subprocess/fork-follow-parent-wp.test
  lldb/test/Shell/Subprocess/fork-follow-parent.test
  lldb/test/Shell/Subprocess/vfork-follow-child-softbp.test
  lldb/test/Shell/Subprocess/vfork-follow-child-wp.test
  lldb/test/Shell/Subprocess/vfork-follow-child.test
  lldb/test/Shell/Subprocess/vfork-follow-parent-softbp.test
  lldb/test/Shell/Subprocess/vfork-follow-parent-wp.test
  lldb/test/Shell/Subprocess/vfork-follow-parent.test

Index: lldb/test/Shell/Subprocess/vfork-follow-parent.test
===
--- lldb/test/Shell/Subprocess/vfork-follow-parent.test
+++ lldb/test/Shell/Subprocess/vfork-follow-parent.test
@@ -8,4 +8,4 @@
 # CHECK: stop reason = breakpoint
 continue
 # CHECK: function run in parent
-# CHECK: child exited: 0
+# CHECK: function run in exec'd child
Index: lldb/test/Shell/Subprocess/vfork-follow-parent-wp.test
===
--- lldb/test/Shell/Subprocess/vfork-follow-parent-wp.test
+++ lldb/test/Shell/Subprocess/vfork-follow-parent-wp.test
@@ -11,4 +11,4 @@
 # CHECK: stop reason = watchpoint
 continue
 # CHECK: function run in parent
-# CHECK: child exited: 0
+# CHECK: function run in exec'd child
Index: lldb/test/Shell/Subprocess/vfork-follow-parent-softbp.test
===
--- lldb/test/Shell/Subprocess/vfork-follow-parent-softbp.test
+++ lldb/test/Shell/Subprocess/vfork-follow-parent-softbp.test
@@ -10,4 +10,4 @@
 # CHECK: stop reason = breakpoint
 continue
 # CHECK: function run in parent
-# CHECK: child exited: 0
+# CHECK: function run in exec'd child
Index: lldb/test/Shell/Subprocess/vfork-follow-child.test
===
--- lldb/test/Shell/Subprocess/vfork-follow-child.test
+++ lldb/test/Shell/Subprocess/vfork-follow-child.test
@@ -1,12 +1,11 @@
 # REQUIRES: native
 # UNSUPPORTED: system-darwin
 # UNSUPPORTED: system-windows
-# This test is very flaky on aarch64.
-# UNSUPPORTED: system-linux && target-aarch64
 # RUN: %clangxx_host %p/Inputs/fork.cpp -DTEST_FORK=vfork -o %t
 # RUN: %lldb -b -s %s %t | FileCheck %s
 settings set target.process.follow-fork-mode child
+settings set target.process.stop-on-exec false
 b parent_func
 process launch
 # CHECK: function run in parent
-# CHECK: child exited: 0
+# CHECK: function run in exec'd child
Index: lldb/test/Shell/Subprocess/vfork-follow-child-wp.test
===
--- lldb/test/Shell/Subprocess/vfork-follow-child-wp.test
+++ lldb/test/Shell/Subprocess/vfork-follow-child-wp.test
@@ -1,13 +1,13 @@
 # REQUIRES: native && dbregs-set
 # UNSUPPORTED: system-darwin
 # UNSUPPORTED: system-windows
-# This test is very flaky on aarch64.
-# UNSUPPORTED: system-linux && target-aarch64
 # RUN: %clangxx_host -g %p/Inputs/fork.cpp -DTEST_FORK=vfork -o %t
 # RUN: %lldb -b -s %s %t | FileCheck %s
 settings set target.process.follow-fork-mode child
+settings set target.process.stop-on-exec false
 process launch -s
 watchpoint set variable -w write g_val
 # CHECK: Watchpoint created:
 continue
-# CHECK: child exited: 0
+# CHECK: function run in parent
+# CHECK: function run in exec'd child
Index: lldb/test/Shell/Subprocess/vfork-follow-child-softbp.test
===
--- lldb/test/Shell/Subprocess/vfork-follow-child-softbp.test
+++ lldb/test/Shell/Subprocess/vfork-follow-child-softbp.test
@@ -1,13 +1,12 @@
 # REQUIRES: native
 # UNSUPPORTED: system-darwin
 # UNSUPPORTED: system-windows
-# This test is very flaky on aarch64.
-# UNSUPPORTED: system-linux && target-aarch64
 # RUN: %clangxx_host %p/Inputs/fork.cpp -DTEST_FORK=vfork -o %t
 # RUN: %lldb -b -s %s %t | FileCheck %s
 settings set target.process.follow-fork-mode child
+settings set target.process.stop-on-exec false
 b child_func
 b parent_func
 process launch
 # CHECK: function run in parent
-# C

[Lldb-commits] [lldb] 24332f0 - [lldb] [Process/FreeBSD] Introduce mips64 FPU reg support

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-09-10T09:13:15+02:00
New Revision: 24332f0e27e17bbe3edfe3d66636c48c17d6ad5f

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

LOG: [lldb] [Process/FreeBSD] Introduce mips64 FPU reg support

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

Added: 


Modified: 
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_mips64.cpp
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_mips64.h
lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_mips64.cpp
lldb/source/Plugins/Process/Utility/RegisterInfos_mips64.h
lldb/source/Plugins/Process/Utility/lldb-mips-freebsd-register-enums.h
lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_mips64.cpp 
b/lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_mips64.cpp
index 8e722c09314c..d93b7fd33815 100644
--- 
a/lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_mips64.cpp
+++ 
b/lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_mips64.cpp
@@ -15,6 +15,7 @@
 #include "lldb/Utility/Status.h"
 
 #include "Plugins/Process/FreeBSD/NativeProcessFreeBSD.h"
+#include "Plugins/Process/Utility/lldb-mips-freebsd-register-enums.h"
 
 // clang-format off
 #include 
@@ -59,11 +60,32 @@ uint32_t 
NativeRegisterContextFreeBSD_mips64::GetUserRegisterCount() const {
   return count;
 }
 
+llvm::Optional
+NativeRegisterContextFreeBSD_mips64::GetSetForNativeRegNum(
+uint32_t reg_num) const {
+  switch (GetRegisterInfoInterface().GetTargetArchitecture().GetMachine()) {
+  case llvm::Triple::mips64:
+if (reg_num >= k_first_gpr_mips64 && reg_num <= k_last_gpr_mips64)
+  return GPRegSet;
+if (reg_num >= k_first_fpr_mips64 && reg_num <= k_last_fpr_mips64)
+  return FPRegSet;
+break;
+  default:
+llvm_unreachable("Unhandled target architecture.");
+  }
+
+  llvm_unreachable("Register does not belong to any register set");
+}
+
 Status NativeRegisterContextFreeBSD_mips64::ReadRegisterSet(RegSetKind set) {
   switch (set) {
   case GPRegSet:
 return NativeProcessFreeBSD::PtraceWrapper(PT_GETREGS, m_thread.GetID(),
m_reg_data.data());
+  case FPRegSet:
+return NativeProcessFreeBSD::PtraceWrapper(
+PT_GETFPREGS, m_thread.GetID(),
+m_reg_data.data() + GetRegisterInfo().GetGPRSize());
   }
   llvm_unreachable("NativeRegisterContextFreeBSD_mips64::ReadRegisterSet");
 }
@@ -73,6 +95,10 @@ Status 
NativeRegisterContextFreeBSD_mips64::WriteRegisterSet(RegSetKind set) {
   case GPRegSet:
 return NativeProcessFreeBSD::PtraceWrapper(PT_SETREGS, m_thread.GetID(),
m_reg_data.data());
+  case FPRegSet:
+return NativeProcessFreeBSD::PtraceWrapper(
+PT_SETFPREGS, m_thread.GetID(),
+m_reg_data.data() + GetRegisterInfo().GetGPRSize());
   }
   llvm_unreachable("NativeRegisterContextFreeBSD_mips64::WriteRegisterSet");
 }
@@ -94,7 +120,16 @@ NativeRegisterContextFreeBSD_mips64::ReadRegister(const 
RegisterInfo *reg_info,
? reg_info->name
: "");
 
-  RegSetKind set = GPRegSet;
+  llvm::Optional opt_set = GetSetForNativeRegNum(reg);
+  if (!opt_set) {
+// This is likely an internal register for lldb use only and should not be
+// directly queried.
+error.SetErrorStringWithFormat("register \"%s\" is in unrecognized set",
+   reg_info->name);
+return error;
+  }
+
+  RegSetKind set = opt_set.getValue();
   error = ReadRegisterSet(set);
   if (error.Fail())
 return error;
@@ -119,7 +154,16 @@ Status NativeRegisterContextFreeBSD_mips64::WriteRegister(
? reg_info->name
: "");
 
-  RegSetKind set = GPRegSet;
+  llvm::Optional opt_set = GetSetForNativeRegNum(reg);
+  if (!opt_set) {
+// This is likely an internal register for lldb use only and should not be
+// directly queried.
+error.SetErrorStringWithFormat("register \"%s\" is in unrecognized set",
+   reg_info->name);
+return error;
+  }
+
+  RegSetKind set = opt_set.getValue();
   error = ReadRegisterSet(set);
   if (error.Fail())
 return error;
@@ -139,6 +183,10 @@ Status 
NativeRegisterContextFreeBSD_mips64::ReadAllRegisterValues(
   if (error.Fail())
 return error;
 
+  error = ReadRegisterSet(FPRegSet);
+  if (error.Fail())
+return error;
+
   data_sp.reset(new DataBufferHeap(m_reg_data.size(), 0));
   uint8_t *dst = data_sp->GetBytes();
   :

[Lldb-commits] [PATCH] D96766: [lldb] [Process/FreeBSD] Introduce mips64 FPU reg support

[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 rG24332f0e27e1: [lldb] [Process/FreeBSD] Introduce mips64 FPU 
reg support (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96766

Files:
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_mips64.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_mips64.h
  lldb/source/Plugins/Process/Utility/RegisterContextFreeBSD_mips64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfos_mips64.h
  lldb/source/Plugins/Process/Utility/lldb-mips-freebsd-register-enums.h
  lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp

Index: lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
===
--- lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
+++ lldb/unittests/Process/Utility/RegisterContextFreeBSDTest.cpp
@@ -409,6 +409,11 @@
   EXPECT_THAT(GetRegParams(reg_ctx, gpr_##lldb_reg##_mips64),  \
   ::testing::Pair(offsetof(reg, r_regs[fbsd_regno]),   \
   sizeof(reg::r_regs[fbsd_regno])))
+#define EXPECT_FPU_MIPS64(lldb_reg, fbsd_regno)\
+  EXPECT_THAT( \
+  GetRegParams(reg_ctx, fpr_##lldb_reg##_mips64),  \
+  ::testing::Pair(offsetof(fpreg, r_regs[fbsd_regno]) + base_offset,   \
+  sizeof(fpreg::r_regs[fbsd_regno])))
 
 TEST(RegisterContextFreeBSDTest, mips64) {
   ArchSpec arch{"mips64-unknown-freebsd"};
@@ -457,6 +462,10 @@
   EXPECT_GPR_MIPS64(pc, 37);
   EXPECT_GPR_MIPS64(ic, 38);
   EXPECT_GPR_MIPS64(dummy, 39);
+
+  size_t base_offset = reg_ctx.GetRegisterInfo()[fpr_f0_mips64].byte_offset;
+
+  EXPECT_FPU_MIPS64(f0, 0);
 }
 
 #endif // defined(__mips64__)
Index: lldb/source/Plugins/Process/Utility/lldb-mips-freebsd-register-enums.h
===
--- lldb/source/Plugins/Process/Utility/lldb-mips-freebsd-register-enums.h
+++ lldb/source/Plugins/Process/Utility/lldb-mips-freebsd-register-enums.h
@@ -57,9 +57,47 @@
   gpr_dummy_mips64,
   k_last_gpr_mips64 = gpr_dummy_mips64,
 
+  k_first_fpr_mips64,
+  fpr_f0_mips64 = k_first_fpr_mips64,
+  fpr_f1_mips64,
+  fpr_f2_mips64,
+  fpr_f3_mips64,
+  fpr_f4_mips64,
+  fpr_f5_mips64,
+  fpr_f6_mips64,
+  fpr_f7_mips64,
+  fpr_f8_mips64,
+  fpr_f9_mips64,
+  fpr_f10_mips64,
+  fpr_f11_mips64,
+  fpr_f12_mips64,
+  fpr_f13_mips64,
+  fpr_f14_mips64,
+  fpr_f15_mips64,
+  fpr_f16_mips64,
+  fpr_f17_mips64,
+  fpr_f18_mips64,
+  fpr_f19_mips64,
+  fpr_f20_mips64,
+  fpr_f21_mips64,
+  fpr_f22_mips64,
+  fpr_f23_mips64,
+  fpr_f24_mips64,
+  fpr_f25_mips64,
+  fpr_f26_mips64,
+  fpr_f27_mips64,
+  fpr_f28_mips64,
+  fpr_f29_mips64,
+  fpr_f30_mips64,
+  fpr_f31_mips64,
+  fpr_fcsr_mips64,
+  fpr_fir_mips64,
+  k_last_fpr_mips64 = fpr_fir_mips64,
+
   k_num_registers_mips64,
 
-  k_num_gpr_registers_mips64 = k_last_gpr_mips64 - k_first_gpr_mips64 + 1
+  k_num_gpr_registers_mips64 = k_last_gpr_mips64 - k_first_gpr_mips64 + 1,
+  k_num_fpr_registers_mips64 = k_last_fpr_mips64 - k_first_fpr_mips64 + 1,
 };
-}
+} // namespace lldb_private
 #endif // LLDB_SOURCE_PLUGINS_PROCESS_UTILITY_LLDB_MIPS_FREEBSD_REGISTER_ENUMS_H
Index: lldb/source/Plugins/Process/Utility/RegisterInfos_mips64.h
===
--- lldb/source/Plugins/Process/Utility/RegisterInfos_mips64.h
+++ lldb/source/Plugins/Process/Utility/RegisterInfos_mips64.h
@@ -11,12 +11,16 @@
 #include "lldb/Core/dwarf.h"
 #include "llvm/Support/Compiler.h"
 
-
 #ifdef DECLARE_REGISTER_INFOS_MIPS64_STRUCT
 
 // Computes the offset of the given GPR in the user data area.
 #define GPR_OFFSET(regname) (LLVM_EXTENSION offsetof(GPR_freebsd_mips, regname))
 
+// Computes the offset of the given FPR in the extended data area.
+#define FPR_OFFSET(regname)\
+  (sizeof(GPR_freebsd_mips) +  \
+   LLVM_EXTENSION offsetof(FPR_freebsd_mips, regname))
+
 // RegisterKind: EHFrame, DWARF, Generic, Process Plugin, LLDB
 
 // Note that the size and offset will be updated by platform-specific classes.
@@ -29,6 +33,31 @@
   NULL, NULL, NULL, 0  \
   }
 
+const uint8_t dwarf_opcode_mips64[] = {
+llvm::dwarf::DW_OP_regx,  dwarf_sr_mips64,llvm::dwarf::DW_OP_lit1,
+llvm::dwarf::DW_OP_lit26, llvm::dwarf::DW_OP_shl, llvm::dwarf::DW_OP_and,
+llvm::dwarf::DW_OP_lit26, llvm::dwarf::DW_OP_shr};
+
+#define DEFINE_FPR(reg, alt, kind1, kind2, kind3) 

[Lldb-commits] [PATCH] D107840: [lldb] [gdb-server] Implement the vFile:fstat packet

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp:763
+template 
+void fill_clamp(T &dest, U src, typename T::value_type fallback) {
+  dest = dest <= std::numeric_limits::max() ? src

static void



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp:764
+void fill_clamp(T &dest, U src, typename T::value_type fallback) {
+  dest = dest <= std::numeric_limits::max() ? src
+: fallback;

`src <=`



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp:776
+
+  std::array data;
+  DataEncoder encoder{data.data(), data.size(), lldb::eByteOrderBig,

mgorny wrote:
> labath wrote:
> > consider:
> > ```
> > struct GdbStat {
> >   llvm::support::ubig32_t st_dev;
> >   llvm::support::ubig32_t st_ino;
> >   ...
> > };
> > 
> > ...
> > 
> > translate(gdb_stats.st_dev, file_stats.st_dev, 0); // I'm not sure that 
> > this clamping is really necessary.
> > ...
> > ```
> > 
> > Seems like it could be nicer, particularly as the vFile_Stat function will 
> > need to do the same thing...
> What's this `translate()` thing? I don't see anything matching in LLVM or 
> LLDB. Or are you saying I should define a helper function?
> 
> As for clamping, I think it's better if we send 0 (a "clearly invalid value") 
> than e.g. truncated `st_dev` that would be simply wrong.
Yeah, that was a new helper function.


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

https://reviews.llvm.org/D107840

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

[Lldb-commits] [PATCH] D107811: [lldb] [gdb-remote] Add fallbacks for vFile:mode and vFile:exists

[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/GDBRemoteCommunicationClient.cpp:3184-3185
+  // Fallback to fstat.
+  llvm::Optional st = Stat(file_spec);
+  if (st) {
+file_permissions = st->gdb_st_mode & (S_IRWXU | S_IRWXG | S_IRWXO);

This is exactly the use case that if-declarations (`if (Optional<...> st = 
...)`were designed for.


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

https://reviews.llvm.org/D107811

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

[Lldb-commits] [PATCH] D107780: [lldb] [gdb-remote] Implement fallback to vFile:stat for GetFileSize()

[Pavel Labath via Phabricator via lldb-commits]

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

cool


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

https://reviews.llvm.org/D107780

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

[Lldb-commits] [PATCH] D103127: lldb: Don't check for CMAKE_SYSTEM_NAME==Android.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D103127#2990249 , @pcc wrote:

> In D103127#2985203 , @labath wrote:
>
>> How exactly are you building this? CMAKE_SYSTEM_NAME is set in the official 
>> android cmake toolchain file 
>> (https://android.googlesource.com/platform/ndk/+/refs/heads/ndk-release-r23/build/cmake/android.toolchain.cmake#213)
>
> I configured cmake using a script that looks like this:
>
>   #!/bin/sh
>   
>   target_flags='--target=aarch64-linux-android21 
> --sysroot=/path/to/android-ndk-r21-beta1/toolchains/llvm/prebuilt/linux-x86_64/sysroot
>  
> --gcc-toolchain=/path/to/android-ndk-r21-beta1/toolchains/llvm/prebuilt/linux-x86_64
>  -mno-outline-atomics'
>   
>   cmake \
> -GNinja \
> -DCMAKE_BUILD_TYPE=Release \
> -DLLVM_ENABLE_ASSERTIONS=ON \
> -DCMAKE_C_COMPILER=`pwd`/../ra/bin/clang \
> -DCMAKE_CXX_COMPILER=`pwd`/../ra/bin/clang++ \
> -DLLVM_ENABLE_THREADS=OFF \
> -DLLVM_CONFIG_PATH=`pwd`/../ra/bin/llvm-config \
> "-DCMAKE_C_FLAGS=$target_flags" \
> "-DCMAKE_CXX_FLAGS=$target_flags" \
> "-DCMAKE_ASM_FLAGS=$target_flags" \
> -DSANITIZER_CXX_ABI=none \
> -DSANITIZER_CXX_ABI_LIBRARY=c++abi \
> -DCMAKE_EXE_LINKER_FLAGS="-fuse-ld=lld -Wl,-lstdc++ -static-libstdc++ 
> --rtlib=libgcc" \
> -DCMAKE_SHARED_LINKER_FLAGS=-Wl,-u__cxa_demangle \
> -DANDROID=1 \
> -DCOMPILER_RT_INCLUDE_TESTS=ON \
> "-DCOMPILER_RT_TEST_COMPILER_CFLAGS=$target_flags" \
> -DLLVM_TABLEGEN=`pwd`/../ra/bin/llvm-tblgen \
> -DLLDB_TABLEGEN=`pwd`/../ra/bin/lldb-tblgen \
> -DCLANG_TABLEGEN=`pwd`/../ra/bin/clang-tblgen \
> -DLLVM_ENABLE_PROJECTS='clang;lldb' \
> -DLLDB_ENABLE_LIBEDIT=0 \
> -DLLDB_ENABLE_LIBXML2=0 \
> -DLLVM_HOST_TRIPLE=aarch64-linux-android21 \
> -DCMAKE_SYSTEM_NAME=Android \
> -DLLVM_TARGETS_TO_BUILD='X86;AArch64' \
> ../../llvm
>
> But now I'm switching to gn to build LLDB, so I don't need to worry about 
> this any more.

Yeah, that's not really the right way to use cmake. You're supposed to create a 
toolchain file (similar to the android one), and use appropriate cmake 
variables (like CMAKE_SYSROOT) instead of putting everything inside CXXFLAGS. 
From within the toolchain file, you could also set the right value of 
CMAKE_SYSTEM_NAME, and then everything would work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103127

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

[Lldb-commits] [PATCH] D108148: [lldb] [gdb-remote] Use standardized GDB errno values

[Pavel Labath via Phabricator via lldb-commits]

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

LG, modulo the comment. Keep an eye out on the bots though. I fear some systems 
(windows, in particular), may not have all errno constants defined, and we'll 
need to do something smarter.




Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:3075
+#include "Plugins/Process/gdb-remote/GDBRemoteErrno.def"
+#undef HANDLE_ERRNO
+  default:

Normally, it's the job of the .def file to undef this macro.


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

https://reviews.llvm.org/D108148

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

[Lldb-commits] [PATCH] D108554: [lldb] Support querying registers via generic names without alt_names

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I'm not sure we should be preserving this behavior for the native register 
context. User-provided strings should not make it there, and code should really 
be retrieving the registers in some other way (like via the 
LLDB_REGNUM_GENERIC_*** constants).

It would also be nice to follow this up by actually removing the generic 
altnames from register definitions, for cases where they don't match 
architecturally defined names.


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

https://reviews.llvm.org/D108554

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

[Lldb-commits] [PATCH] D107840: [lldb] [gdb-server] Implement the vFile:fstat packet

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

mgorny marked 3 inline comments as done.
mgorny added a comment.

Thanks!


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

https://reviews.llvm.org/D107840

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

[Lldb-commits] [PATCH] D108148: [lldb] [gdb-remote] Use standardized GDB errno values

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

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

In D108148#2993746 , @labath wrote:

> LG, modulo the comment. Keep an eye out on the bots though. I fear some 
> systems (windows, in particular), may not have all errno constants defined, 
> and we'll need to do something smarter.

Yeah, I was thinking how to handle this… after all, we can't `#ifdef` here.


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

https://reviews.llvm.org/D108148

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

[Lldb-commits] [PATCH] D108554: [lldb] Support querying registers via generic names without alt_names

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

mgorny added a comment.

In D108554#2993766 , @labath wrote:

> I'm not sure we should be preserving this behavior for the native register 
> context. User-provided strings should not make it there, and code should 
> really be retrieving the registers in some other way (like via the 
> LLDB_REGNUM_GENERIC_*** constants).

Could you rephrase this comment to indicate the requested change, if any? ;-)

> It would also be nice to follow this up by actually removing the generic 
> altnames from register definitions, for cases where they don't match 
> architecturally defined names.

That was roughly my plan, presuming you like the idea. Should I include this in 
the same commit or separately for each platform?


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

https://reviews.llvm.org/D108554

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

[Lldb-commits] [lldb] 21e2d7c - [lldb] [gdb-remote] Implement fallback to vFile:stat for GetFileSize()

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-09-10T11:09:35+02:00
New Revision: 21e2d7ce43c42df5d60a2805c801b8f1eda7919c

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

LOG: [lldb] [gdb-remote] Implement fallback to vFile:stat for GetFileSize()

Implement a fallback to getting the file size via vFile:stat packet
when the remote server does not implement vFile:size.  This makes it
possible to query file sizes from remote gdbserver.

Note that unlike vFile:size, the fallback will not work if the server is
unable to open the file.

While at it, add a few tests for the 'platform get-size' command.

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

Added: 


Modified: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py

Removed: 




diff  --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
index b1e2075a64fef..967dc82f0270a 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
@@ -51,6 +51,26 @@ enum class CompressionType {
   LZMA, // Lempel–Ziv–Markov chain algorithm
 };
 
+// Data included in the vFile:fstat packet.
+// https://sourceware.org/gdb/onlinedocs/gdb/struct-stat.html#struct-stat
+struct GDBRemoteFStatData {
+  llvm::support::ubig32_t gdb_st_dev;
+  llvm::support::ubig32_t gdb_st_ino;
+  llvm::support::ubig32_t gdb_st_mode;
+  llvm::support::ubig32_t gdb_st_nlink;
+  llvm::support::ubig32_t gdb_st_uid;
+  llvm::support::ubig32_t gdb_st_gid;
+  llvm::support::ubig32_t gdb_st_rdev;
+  llvm::support::ubig64_t gdb_st_size;
+  llvm::support::ubig64_t gdb_st_blksize;
+  llvm::support::ubig64_t gdb_st_blocks;
+  llvm::support::ubig32_t gdb_st_atime;
+  llvm::support::ubig32_t gdb_st_mtime;
+  llvm::support::ubig32_t gdb_st_ctime;
+};
+static_assert(sizeof(GDBRemoteFStatData) == 64,
+  "size of GDBRemoteFStatData is not 64");
+
 class ProcessGDBRemote;
 
 class GDBRemoteCommunication : public Communication {

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 16a27af700c6e..4bcef1f05cdbc 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -65,6 +65,7 @@ GDBRemoteCommunicationClient::GDBRemoteCommunicationClient()
   m_supports_QEnvironmentHexEncoded(true), m_supports_qSymbol(true),
   m_qSymbol_requests_done(false), m_supports_qModuleInfo(true),
   m_supports_jThreadsInfo(true), m_supports_jModulesInfo(true),
+  m_supports_vFileSize(true),
 
   m_host_arch(), m_process_arch(), m_os_build(), m_os_kernel(),
   m_hostname(), m_gdb_server_name(), m_default_packet_timeout(0),
@@ -3068,22 +3069,66 @@ bool 
GDBRemoteCommunicationClient::CloseFile(lldb::user_id_t fd,
   return false;
 }
 
-// Extension of host I/O packets to get the file size.
-lldb::user_id_t GDBRemoteCommunicationClient::GetFileSize(
-const lldb_private::FileSpec &file_spec) {
-  std::string path(file_spec.GetPath(false));
+llvm::Optional
+GDBRemoteCommunicationClient::FStat(lldb::user_id_t fd) {
   lldb_private::StreamString stream;
-  stream.PutCString("vFile:size:");
-  stream.PutStringAsRawHex8(path);
+  stream.Printf("vFile:fstat:%" PRIx64, fd);
   StringExtractorGDBRemote response;
   if (SendPacketAndWaitForResponse(stream.GetString(), response) ==
   PacketResult::Success) {
 if (response.GetChar() != 'F')
+  return llvm::None;
+int64_t size = response.GetS64(-1, 16);
+if (size > 0 && response.GetChar() == ';') {
+  std::string buffer;
+  if (response.GetEscapedBinaryData(buffer)) {
+GDBRemoteFStatData out;
+if (buffer.size() != sizeof(out))
+  return llvm::None;
+memcpy(&out, buffer.data(), sizeof(out));
+return out;
+  }
+}
+  }
+  return llvm::None;
+}
+
+llvm::Optional
+GDBRemoteCommunicationClient::Stat(const lldb_private::FileSpec &file_spec) {
+  Status error;
+  lldb::user_id_t fd = OpenFile(file_spec, File::eOpenOptionReadOnly, 0, 
error);
+  if (fd == UINT64_MAX)
+return llvm::None;
+  llvm::Optional st = FStat(fd);
+  CloseFile(fd, error);
+  return st;
+}
+
+// Extension of host I/O packets to get the file size.
+lldb::user_id_t GDBRemoteCommunicationClient::GetFileSize(
+const lldb_private::FileSpec &file_spec) {
+  if (m_supports_vFil

[Lldb-commits] [lldb] 9e886fb - [lldb] [gdb-server] Implement the vFile:fstat packet

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-09-10T11:09:35+02:00
New Revision: 9e886fbb18b525c080c04f4a12bd481c9aa849c0

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

LOG: [lldb] [gdb-server] Implement the vFile:fstat packet

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

Added: 


Modified: 
lldb/include/lldb/Utility/StringExtractorGDBRemote.h

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
lldb/source/Utility/StringExtractorGDBRemote.cpp
lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py

Removed: 




diff  --git a/lldb/include/lldb/Utility/StringExtractorGDBRemote.h 
b/lldb/include/lldb/Utility/StringExtractorGDBRemote.h
index 90b2837944cd1..0c4ae32691b71 100644
--- a/lldb/include/lldb/Utility/StringExtractorGDBRemote.h
+++ b/lldb/include/lldb/Utility/StringExtractorGDBRemote.h
@@ -88,6 +88,7 @@ class StringExtractorGDBRemote : public StringExtractor {
 eServerPacketType_vFile_mode,
 eServerPacketType_vFile_exists,
 eServerPacketType_vFile_md5,
+eServerPacketType_vFile_fstat,
 eServerPacketType_vFile_stat,
 eServerPacketType_vFile_symlink,
 eServerPacketType_vFile_unlink,

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
index 9627fb1720049..21700f57c77de 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -156,6 +156,9 @@ 
GDBRemoteCommunicationServerCommon::GDBRemoteCommunicationServerCommon(
   RegisterMemberFunctionHandler(
   StringExtractorGDBRemote::eServerPacketType_vFile_size,
   &GDBRemoteCommunicationServerCommon::Handle_vFile_Size);
+  RegisterMemberFunctionHandler(
+  StringExtractorGDBRemote::eServerPacketType_vFile_fstat,
+  &GDBRemoteCommunicationServerCommon::Handle_vFile_FStat);
   RegisterMemberFunctionHandler(
   StringExtractorGDBRemote::eServerPacketType_vFile_stat,
   &GDBRemoteCommunicationServerCommon::Handle_vFile_Stat);
@@ -754,6 +757,46 @@ GDBRemoteCommunicationServerCommon::Handle_qPlatform_shell(
   return SendErrorResponse(24);
 }
 
+template 
+static void fill_clamp(T &dest, U src, typename T::value_type fallback) {
+  dest = src <= std::numeric_limits::max() ? src
+   : fallback;
+}
+
+GDBRemoteCommunication::PacketResult
+GDBRemoteCommunicationServerCommon::Handle_vFile_FStat(
+StringExtractorGDBRemote &packet) {
+  StreamGDBRemote response;
+  packet.SetFilePos(::strlen("vFile:fstat:"));
+  int fd = packet.GetS32(-1, 16);
+
+  struct stat file_stats;
+  if (::fstat(fd, &file_stats) == -1) {
+const int save_errno = errno;
+response.Printf("F-1,%x", save_errno);
+return SendPacketNoLock(response.GetString());
+  }
+
+  GDBRemoteFStatData data;
+  fill_clamp(data.gdb_st_dev, file_stats.st_dev, 0);
+  fill_clamp(data.gdb_st_ino, file_stats.st_ino, 0);
+  data.gdb_st_mode = file_stats.st_mode;
+  fill_clamp(data.gdb_st_nlink, file_stats.st_nlink, UINT32_MAX);
+  fill_clamp(data.gdb_st_uid, file_stats.st_uid, 0);
+  fill_clamp(data.gdb_st_gid, file_stats.st_gid, 0);
+  fill_clamp(data.gdb_st_rdev, file_stats.st_rdev, 0);
+  data.gdb_st_size = file_stats.st_size;
+  data.gdb_st_blksize = file_stats.st_blksize;
+  data.gdb_st_blocks = file_stats.st_blocks;
+  fill_clamp(data.gdb_st_atime, file_stats.st_atime, 0);
+  fill_clamp(data.gdb_st_mtime, file_stats.st_mtime, 0);
+  fill_clamp(data.gdb_st_ctime, file_stats.st_ctime, 0);
+
+  response.Printf("F%zx;", sizeof(data));
+  response.PutEscapedBytes(&data, sizeof(data));
+  return SendPacketNoLock(response.GetString());
+}
+
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerCommon::Handle_vFile_Stat(
 StringExtractorGDBRemote &packet) {

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
index ecd80923fcf0f..029972348ef01 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
@@ -71,6 +71,8 @@ class GDBRemoteCommunicationServerCommon : public 
GDBRemoteCommunicationServer {
 
   PacketResult Handle_vFile_unlink(StringExtractorGDBRemote &packet);
 
+  PacketResult Handle_vFile_FStat(StringExtractorGDBRemote &packet);
+
   PacketResult Handle_vFile_Stat(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_vFile_MD5(StringExtractorG

[Lldb-commits] [PATCH] D107780: [lldb] [gdb-remote] Implement fallback to vFile:stat for GetFileSize()

[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 rG21e2d7ce43c4: [lldb] [gdb-remote] Implement fallback to 
vFile:stat for GetFileSize() (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107780

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py

Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
@@ -77,3 +77,58 @@
 ])
 finally:
 self.dbg.GetSelectedPlatform().DisconnectRemote()
+
+def test_file_size(self):
+"""Test 'platform get-size'"""
+
+class Responder(MockGDBServerResponder):
+def vFile(self, packet):
+return "F1000"
+
+self.server.responder = Responder()
+
+try:
+self.runCmd("platform select remote-gdb-server")
+self.runCmd("platform connect connect://" +
+self.server.get_connect_address())
+self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
+
+self.match("platform get-size /some/file.txt",
+   [r"File size of /some/file\.txt \(remote\): 4096"])
+self.assertPacketLogContains([
+"vFile:size:2f736f6d652f66696c652e747874",
+])
+finally:
+self.dbg.GetSelectedPlatform().DisconnectRemote()
+
+def test_file_size_fallback(self):
+"""Test 'platform get-size fallback to vFile:fstat'"""
+
+class Responder(MockGDBServerResponder):
+def vFile(self, packet):
+if packet.startswith("vFile:open:"):
+return "F5"
+elif packet.startswith("vFile:fstat:"):
+return "F40;" + 28 * "\0" + "\0\0\0\0\0\1\2\3" + 28 * "\0"
+if packet.startswith("vFile:close:"):
+return "F0"
+return ""
+
+self.server.responder = Responder()
+
+try:
+self.runCmd("platform select remote-gdb-server")
+self.runCmd("platform connect connect://" +
+self.server.get_connect_address())
+self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
+
+self.match("platform get-size /some/file.txt",
+   [r"File size of /some/file\.txt \(remote\): 66051"])
+self.assertPacketLogContains([
+"vFile:size:2f736f6d652f66696c652e747874",
+"vFile:open:2f736f6d652f66696c652e747874,,",
+"vFile:fstat:5",
+"vFile:close:5",
+])
+finally:
+self.dbg.GetSelectedPlatform().DisconnectRemote()
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -376,6 +376,12 @@
 
   bool CloseFile(lldb::user_id_t fd, Status &error);
 
+  llvm::Optional FStat(lldb::user_id_t fd);
+
+  // NB: this is just a convenience wrapper over open() + fstat().  It does not
+  // work if the file cannot be opened.
+  llvm::Optional Stat(const FileSpec &file_spec);
+
   lldb::user_id_t GetFileSize(const FileSpec &file_spec);
 
   void AutoCompleteDiskFileOrDirectory(CompletionRequest &request,
@@ -581,7 +587,7 @@
   m_supports_QEnvironment : 1, m_supports_QEnvironmentHexEncoded : 1,
   m_supports_qSymbol : 1, m_qSymbol_requests_done : 1,
   m_supports_qModuleInfo : 1, m_supports_jThreadsInfo : 1,
-  m_supports_jModulesInfo : 1;
+  m_supports_jModulesInfo : 1, m_supports_vFileSize : 1;
 
   /// Current gdb remote protocol process identifier for all other operations
   lldb::pid_t m_curr_pid = LLDB_INVALID_PROCESS_ID;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -65,6 +65,7 @@
   m_supports_QEnvironmentHexEncoded(true), m_supports_qSymbol(true),
   m_qSymbol_requests_done(false), m_supports_qModuleInfo(true),
   

[Lldb-commits] [PATCH] D107840: [lldb] [gdb-server] Implement the vFile:fstat packet

[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 rG9e886fbb18b5: [lldb] [gdb-server] Implement the vFile:fstat 
packet (authored by mgorny).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D107840?vs=371674&id=371834#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107840

Files:
  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py

Index: lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
@@ -9,7 +9,38 @@
 from gdbremote_testcase import GdbRemoteTestCaseBase
 
 import binascii
+import os
 import stat
+import struct
+import typing
+
+
+class GDBStat(typing.NamedTuple):
+st_dev: int
+st_ino: int
+st_mode: int
+st_nlink: int
+st_uid: int
+st_gid: int
+st_rdev: int
+st_size: int
+st_blksize: int
+st_blocks: int
+st_atime: int
+st_mtime: int
+st_ctime: int
+
+
+def uint32_or_zero(x):
+return x if x < 2**32 else 0
+
+
+def uint32_or_max(x):
+return x if x < 2**32 else 2**32 - 1
+
+
+def uint32_trunc(x):
+return x & (2**32 - 1)
 
 
 class TestGdbRemotePlatformFile(GdbRemoteTestCaseBase):
@@ -178,6 +209,70 @@
 True)
 self.expect_gdbremote_sequence()
 
+@skipIfWindows
+@add_test_categories(["llgs"])
+def test_platform_file_fstat(self):
+server = self.connect_to_debug_monitor()
+self.assertIsNotNone(server)
+
+with tempfile.NamedTemporaryFile() as temp_file:
+temp_file.write(b"some test data for stat")
+temp_file.flush()
+
+self.do_handshake()
+self.test_sequence.add_log_lines(
+["read packet: $vFile:open:%s,0,0#00" % (
+binascii.b2a_hex(temp_file.name.encode()).decode(),),
+ {"direction": "send",
+ "regex": r"^\$F([0-9a-fA-F]+)#[0-9a-fA-F]{2}$",
+ "capture": {1: "fd"}}],
+True)
+
+context = self.expect_gdbremote_sequence()
+self.assertIsNotNone(context)
+fd = int(context["fd"], 16)
+
+self.reset_test_sequence()
+self.test_sequence.add_log_lines(
+["read packet: $vFile:fstat:%x#00" % (fd,),
+ {"direction": "send",
+ "regex": r"^\$F([0-9a-fA-F]+);(.*)#[0-9a-fA-F]{2}$",
+ "capture": {1: "size", 2: "data"}}],
+True)
+context = self.expect_gdbremote_sequence()
+self.assertEqual(int(context["size"], 16), 64)
+# NB: we're using .encode() as a hack because the test suite
+# is wrongly using (unicode) str instead of bytes
+gdb_stat = GDBStat(
+*struct.unpack(">IIIQQQIII",
+   self.decode_gdbremote_binary(context["data"])
+.encode("iso-8859-1")))
+sys_stat = os.fstat(temp_file.fileno())
+
+self.assertEqual(gdb_stat.st_dev, uint32_or_zero(sys_stat.st_dev))
+self.assertEqual(gdb_stat.st_ino, uint32_or_zero(sys_stat.st_ino))
+self.assertEqual(gdb_stat.st_mode, uint32_trunc(sys_stat.st_mode))
+self.assertEqual(gdb_stat.st_nlink, uint32_or_max(sys_stat.st_nlink))
+self.assertEqual(gdb_stat.st_uid, uint32_or_zero(sys_stat.st_uid))
+self.assertEqual(gdb_stat.st_gid, uint32_or_zero(sys_stat.st_gid))
+self.assertEqual(gdb_stat.st_rdev, uint32_or_zero(sys_stat.st_rdev))
+self.assertEqual(gdb_stat.st_size, sys_stat.st_size)
+self.assertEqual(gdb_stat.st_blksize, sys_stat.st_blksize)
+self.assertEqual(gdb_stat.st_blocks, sys_stat.st_blocks)
+self.assertEqual(gdb_stat.st_atime,
+ uint32_or_zero(int(sys_stat.st_atime)))
+self.assertEqual(gdb_stat.st_mtime,
+ uint32_or_zero(int(sys_stat.st_mtime)))
+self.assertEqual(gdb_stat.st_ctime,
+ uint32_or_zero(int(sys_stat.st_ctime)))
+
+self.reset_test_sequence()
+self.test_sequence.add_log_lines(
+["read packet: $vFile:close:%x#00" % (fd,),
+ "send packet: $F0#00"],
+True)
+self.expect_gdbremote_sequence()
+
 def expect_error(self):
  

[Lldb-commits] [PATCH] D108148: [lldb] [gdb-remote] Use standardized GDB errno values

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Judging by 
https://docs.microsoft.com/en-us/cpp/c-runtime-library/errno-constants?view=msvc-160,
 this may just work, so let's cross that bridge when we get to it.


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

https://reviews.llvm.org/D108148

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

[Lldb-commits] [PATCH] D109585: [lldb] [test] Move "platform connect" logic into a common class

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

mgorny created this revision.
mgorny added reviewers: labath, emaste, krytarowski.
mgorny requested review of this revision.

Create a common GDBPlatformClientTestBase class and move the platform
select/connect logic there to reduce duplication.


https://reviews.llvm.org/D109585

Files:
  
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteDiskFileCompletion.py
  lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
  lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py

Index: lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
===
--- lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
+++ lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
@@ -566,3 +566,19 @@
 if i < len(packets):
 self.fail(u"Did not receive: %s\nLast 10 packets:\n\t%s" %
 (packets[i], u'\n\t'.join(log)))
+
+
+class GDBPlatformClientTestBase(GDBRemoteTestBase):
+"""
+Base class for platform server clients.
+
+This class extends GDBRemoteTestBase by automatically connecting
+via "platform connect" in the setUp() method.
+"""
+
+def setUp(self):
+super().setUp()
+self.runCmd("platform select remote-gdb-server")
+self.runCmd("platform connect connect://" +
+self.server.get_connect_address())
+self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
@@ -1,6 +1,6 @@
 from gdbclientutils import *
 
-class TestGDBRemotePlatformFile(GDBRemoteTestBase):
+class TestGDBRemotePlatformFile(GDBPlatformClientTestBase):
 
 def test_file(self):
 """Test mock operations on a remote file"""
@@ -20,11 +20,6 @@
 self.server.responder = Responder()
 
 try:
-self.runCmd("platform select remote-gdb-server")
-self.runCmd("platform connect connect://" +
-self.server.get_connect_address())
-self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
-
 self.match("platform file open /some/file.txt -v 0755",
[r"File Descriptor = 16"])
 self.match("platform file read 16 -o 11 -c 13",
@@ -52,11 +47,6 @@
 self.server.responder = Responder()
 
 try:
-self.runCmd("platform select remote-gdb-server")
-self.runCmd("platform connect connect://" +
-self.server.get_connect_address())
-self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
-
 self.match("platform file open /some/file.txt -v 0755",
[r"error: Invalid argument"],
error=True)
@@ -88,11 +78,6 @@
 self.server.responder = Responder()
 
 try:
-self.runCmd("platform select remote-gdb-server")
-self.runCmd("platform connect connect://" +
-self.server.get_connect_address())
-self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
-
 self.match("platform get-size /some/file.txt",
[r"File size of /some/file\.txt \(remote\): 4096"])
 self.assertPacketLogContains([
@@ -117,11 +102,6 @@
 self.server.responder = Responder()
 
 try:
-self.runCmd("platform select remote-gdb-server")
-self.runCmd("platform connect connect://" +
-self.server.get_connect_address())
-self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
-
 self.match("platform get-size /some/file.txt",
[r"File size of /some/file\.txt \(remote\): 66051"])
 self.assertPacketLogContains([
Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteDiskFileCompletion.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteDiskFileCompletion.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteDiskFileCompletion.py
@@ -1,6 +1,6 @@
 from gdbclientutils import *
 
-class TestGDBRemoteDiskFileCompletion(GDBRemoteTestBase):
+class TestGDBRemoteDiskFileCompletion(GDBPlatformClientTestBase):
 
 def test_autocomplete_request(self):
 """Test remote disk completion on remote-gdb-server plugin"""
@@ -15,11 +15,6 @@
 self.server.responder = Responder()
 
 try:
-self.runCmd("platform select remote-gdb-server")
-self.runCmd("platform connect connect://" +
-self.server.get_connect_address())
-   

[Lldb-commits] [PATCH] D107809: [lldb] Add new commands and tests for getting file perms & exists

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

mgorny updated this revision to Diff 371841.
mgorny marked an inline comment as done.
mgorny added a comment.

Remove now-redundant `platform connect` calls.

TODO: server tests.


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

https://reviews.llvm.org/D107809

Files:
  lldb/source/Commands/CommandObjectPlatform.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py

Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
@@ -112,3 +112,57 @@
 ])
 finally:
 self.dbg.GetSelectedPlatform().DisconnectRemote()
+
+def test_file_permissions(self):
+"""Test 'platform get-permissions'"""
+
+class Responder(MockGDBServerResponder):
+def vFile(self, packet):
+return "F1a4"
+
+self.server.responder = Responder()
+
+try:
+self.match("platform get-permissions /some/file.txt",
+   [r"File permissions of /some/file\.txt \(remote\): 0o0644"])
+self.assertPacketLogContains([
+"vFile:mode:2f736f6d652f66696c652e747874",
+])
+finally:
+self.dbg.GetSelectedPlatform().DisconnectRemote()
+
+def test_file_exists(self):
+"""Test 'platform file-exists'"""
+
+class Responder(MockGDBServerResponder):
+def vFile(self, packet):
+return "F,1"
+
+self.server.responder = Responder()
+
+try:
+self.match("platform file-exists /some/file.txt",
+   [r"File /some/file\.txt \(remote\) exists"])
+self.assertPacketLogContains([
+"vFile:exists:2f736f6d652f66696c652e747874",
+])
+finally:
+self.dbg.GetSelectedPlatform().DisconnectRemote()
+
+def test_file_exists_not(self):
+"""Test 'platform file-exists' with non-existing file"""
+
+class Responder(MockGDBServerResponder):
+def vFile(self, packet):
+return "F,0"
+
+self.server.responder = Responder()
+
+try:
+self.match("platform file-exists /some/file.txt",
+   [r"File /some/file\.txt \(remote\) does not exist"])
+self.assertPacketLogContains([
+"vFile:exists:2f736f6d652f66696c652e747874",
+])
+finally:
+self.dbg.GetSelectedPlatform().DisconnectRemote()
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -661,9 +661,10 @@
 std::error_code ec;
 const uint32_t mode = FileSystem::Instance().GetPermissions(file_spec, ec);
 StreamString response;
-response.Printf("F%x", mode);
-if (mode == 0 || ec)
-  response.Printf(",%x", (int)Status(ec).GetError());
+if (mode != llvm::sys::fs::perms_not_known)
+  response.Printf("F%x", mode);
+else
+  response.Printf("F-1,%x", (int)Status(ec).GetError());
 return SendPacketNoLock(response.GetString());
   }
   return SendErrorResponse(23);
Index: lldb/source/Commands/CommandObjectPlatform.cpp
===
--- lldb/source/Commands/CommandObjectPlatform.cpp
+++ lldb/source/Commands/CommandObjectPlatform.cpp
@@ -926,6 +926,141 @@
   }
 };
 
+// "platform get-permissions remote-file-path"
+class CommandObjectPlatformGetPermissions : public CommandObjectParsed {
+public:
+  CommandObjectPlatformGetPermissions(CommandInterpreter &interpreter)
+  : CommandObjectParsed(interpreter, "platform get-permissions",
+"Get the file permission bits from the remote end.",
+"platform get-permissions ", 0) {
+SetHelpLong(
+R"(Examples:
+
+(lldb) platform get-permissions /the/remote/file/path
+
+Get the file permissions from the remote end with path /the/remote/file/path.)");
+
+CommandArgumentEntry arg1;
+CommandArgumentData file_arg_remote;
+
+// Define the first (and only) variant of this arg.
+file_arg_remote.arg_type = eArgTypeFilename;
+file_arg_remote.arg_repetition = eArgRepeatPlain;
+// There is only one variant this argument could be; put it into the
+// argument entry.
+arg1.push_back(file_arg_remote);
+
+// Push the data for the first argument into the m_arguments vector.
+m_arguments.push_back(arg1);
+  }
+
+  ~Comm

[Lldb-commits] [PATCH] D107811: [lldb] [gdb-remote] Add fallbacks for vFile:mode and vFile:exists

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

mgorny updated this revision to Diff 371842.
mgorny added a comment.

Remove now-redundant `platform connect` from tests. Use if-declaration as 
requested.


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

https://reviews.llvm.org/D107811

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py

Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
@@ -131,6 +131,33 @@
 finally:
 self.dbg.GetSelectedPlatform().DisconnectRemote()
 
+def test_file_permissions_fallback(self):
+"""Test 'platform get-permissions' fallback to fstat"""
+
+class Responder(MockGDBServerResponder):
+def vFile(self, packet):
+if packet.startswith("vFile:open:"):
+return "F5"
+elif packet.startswith("vFile:fstat:"):
+return "F40;" + 8 * "\0" + "\0\0\1\xA4" + 52 * "\0"
+if packet.startswith("vFile:close:"):
+return "F0"
+return ""
+
+self.server.responder = Responder()
+
+try:
+self.match("platform get-permissions /some/file.txt",
+   [r"File permissions of /some/file\.txt \(remote\): 0o0644"])
+self.assertPacketLogContains([
+"vFile:mode:2f736f6d652f66696c652e747874",
+"vFile:open:2f736f6d652f66696c652e747874,,",
+"vFile:fstat:5",
+"vFile:close:5",
+])
+finally:
+self.dbg.GetSelectedPlatform().DisconnectRemote()
+
 def test_file_exists(self):
 """Test 'platform file-exists'"""
 
@@ -166,3 +193,48 @@
 ])
 finally:
 self.dbg.GetSelectedPlatform().DisconnectRemote()
+
+def test_file_exists_fallback(self):
+"""Test 'platform file-exists' fallback to open"""
+
+class Responder(MockGDBServerResponder):
+def vFile(self, packet):
+if packet.startswith("vFile:open:"):
+return "F5"
+if packet.startswith("vFile:close:"):
+return "F0"
+return ""
+
+self.server.responder = Responder()
+
+try:
+self.match("platform file-exists /some/file.txt",
+   [r"File /some/file\.txt \(remote\) exists"])
+self.assertPacketLogContains([
+"vFile:exists:2f736f6d652f66696c652e747874",
+"vFile:open:2f736f6d652f66696c652e747874,,",
+"vFile:close:5",
+])
+finally:
+self.dbg.GetSelectedPlatform().DisconnectRemote()
+
+def test_file_exists_not_fallback(self):
+"""Test 'platform file-exists' fallback to open with non-existing file"""
+
+class Responder(MockGDBServerResponder):
+def vFile(self, packet):
+if packet.startswith("vFile:open:"):
+return "F-1,2"
+return ""
+
+self.server.responder = Responder()
+
+try:
+self.match("platform file-exists /some/file.txt",
+   [r"File /some/file\.txt \(remote\) does not exist"])
+self.assertPacketLogContains([
+"vFile:exists:2f736f6d652f66696c652e747874",
+"vFile:open:2f736f6d652f66696c652e747874,,",
+])
+finally:
+self.dbg.GetSelectedPlatform().DisconnectRemote()
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -587,7 +587,8 @@
   m_supports_QEnvironment : 1, m_supports_QEnvironmentHexEncoded : 1,
   m_supports_qSymbol : 1, m_qSymbol_requests_done : 1,
   m_supports_qModuleInfo : 1, m_supports_jThreadsInfo : 1,
-  m_supports_jModulesInfo : 1, m_supports_vFileSize : 1;
+  m_supports_jModulesInfo : 1, m_supports_vFileSize : 1,
+  m_supports_vFileMode : 1, m_supports_vFileExists : 1;
 
   /// Current gdb remote protocol process identifier for all other operations
   lldb::pid_t m_curr_pid = LLDB_INVALID_PROCESS_ID;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.c

[Lldb-commits] [PATCH] D108554: [lldb] Support querying registers via generic names without alt_names

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D108554#2993830 , @mgorny wrote:

> In D108554#2993766 , @labath wrote:
>
>> I'm not sure we should be preserving this behavior for the native register 
>> context. User-provided strings should not make it there, and code should 
>> really be retrieving the registers in some other way (like via the 
>> LLDB_REGNUM_GENERIC_*** constants).
>
> Could you rephrase this comment to indicate the requested change, if any? ;-)

Well.. the comment was open ended, because my thoughts on this are quite open 
ended as well.. :) But if I wanted to be more precise, I suppose it boils down 
to this:

- Is the NativeRC change there for a particular reason (like, does anything 
break if you leave it out)?
- Do _you_ agree with my assessment of its necessity?

If there's no reason for its presence, and you don't see it as necessary, then 
I'd like to remove it. If not, let's talk.

>> It would also be nice to follow this up by actually removing the generic 
>> altnames from register definitions, for cases where they don't match 
>> architecturally defined names.
>
> That was roughly my plan, presuming you like the idea. Should I include this 
> in the same commit or separately for each platform?

Sounds good. I think we should do it as a separate patch. Whether it's one or 
more of them depends on how large they get...


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

https://reviews.llvm.org/D108554

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

[Lldb-commits] [PATCH] D109585: [lldb] [test] Move "platform connect" logic into a common class

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

looks better. Can we also move the DisconnectRemote() thingy into the tearDown 
method ?


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

https://reviews.llvm.org/D109585

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

[Lldb-commits] [PATCH] D108554: [lldb] Support querying registers via generic names without alt_names

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

mgorny added a comment.

In D108554#2993993 , @labath wrote:

> In D108554#2993830 , @mgorny wrote:
>
>> In D108554#2993766 , @labath wrote:
>>
>>> I'm not sure we should be preserving this behavior for the native register 
>>> context. User-provided strings should not make it there, and code should 
>>> really be retrieving the registers in some other way (like via the 
>>> LLDB_REGNUM_GENERIC_*** constants).
>>
>> Could you rephrase this comment to indicate the requested change, if any? ;-)
>
> Well.. the comment was open ended, because my thoughts on this are quite open 
> ended as well.. :) But if I wanted to be more precise, I suppose it boils 
> down to this:
>
> - Is the NativeRC change there for a particular reason (like, does anything 
> break if you leave it out)?
> - Do _you_ agree with my assessment of its necessity?
>
> If there's no reason for its presence, and you don't see it as necessary, 
> then I'd like to remove it. If not, let's talk.

Ah, now I get what you mean. To be honest, I've done that because I wasn't sure 
whether there isn't any code relying on that. I guess I'll try removing it and 
see if any tests fail.


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

https://reviews.llvm.org/D108554

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

[Lldb-commits] [PATCH] D109585: [lldb] [test] Move "platform connect" logic into a common class

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

mgorny updated this revision to Diff 371844.
mgorny added a comment.

Sorry about missing that. Done now.


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

https://reviews.llvm.org/D109585

Files:
  
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteDiskFileCompletion.py
  lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
  lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py

Index: lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
===
--- lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
+++ lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
@@ -566,3 +566,23 @@
 if i < len(packets):
 self.fail(u"Did not receive: %s\nLast 10 packets:\n\t%s" %
 (packets[i], u'\n\t'.join(log)))
+
+
+class GDBPlatformClientTestBase(GDBRemoteTestBase):
+"""
+Base class for platform server clients.
+
+This class extends GDBRemoteTestBase by automatically connecting
+via "platform connect" in the setUp() method.
+"""
+
+def setUp(self):
+super().setUp()
+self.runCmd("platform select remote-gdb-server")
+self.runCmd("platform connect connect://" +
+self.server.get_connect_address())
+self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
+
+def tearDown(self):
+self.dbg.GetSelectedPlatform().DisconnectRemote()
+super().tearDown()
Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
@@ -1,6 +1,6 @@
 from gdbclientutils import *
 
-class TestGDBRemotePlatformFile(GDBRemoteTestBase):
+class TestGDBRemotePlatformFile(GDBPlatformClientTestBase):
 
 def test_file(self):
 """Test mock operations on a remote file"""
@@ -19,28 +19,20 @@
 
 self.server.responder = Responder()
 
-try:
-self.runCmd("platform select remote-gdb-server")
-self.runCmd("platform connect connect://" +
-self.server.get_connect_address())
-self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
-
-self.match("platform file open /some/file.txt -v 0755",
-   [r"File Descriptor = 16"])
-self.match("platform file read 16 -o 11 -c 13",
-   [r"Return = 11\nData = \"frobnicator\""])
-self.match("platform file write 16 -o 11 -d teststring",
-   [r"Return = 10"])
-self.match("platform file close 16",
-   [r"file 16 closed."])
-self.assertPacketLogContains([
-"vFile:open:2f736f6d652f66696c652e747874,0202,01ed",
-"vFile:pread:10,d,b",
-"vFile:pwrite:10,b,teststring",
-"vFile:close:10",
-])
-finally:
-self.dbg.GetSelectedPlatform().DisconnectRemote()
+self.match("platform file open /some/file.txt -v 0755",
+   [r"File Descriptor = 16"])
+self.match("platform file read 16 -o 11 -c 13",
+   [r"Return = 11\nData = \"frobnicator\""])
+self.match("platform file write 16 -o 11 -d teststring",
+   [r"Return = 10"])
+self.match("platform file close 16",
+   [r"file 16 closed."])
+self.assertPacketLogContains([
+"vFile:open:2f736f6d652f66696c652e747874,0202,01ed",
+"vFile:pread:10,d,b",
+"vFile:pwrite:10,b,teststring",
+"vFile:close:10",
+])
 
 def test_file_fail(self):
 """Test mocked failures of remote operations"""
@@ -51,32 +43,24 @@
 
 self.server.responder = Responder()
 
-try:
-self.runCmd("platform select remote-gdb-server")
-self.runCmd("platform connect connect://" +
-self.server.get_connect_address())
-self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
-
-self.match("platform file open /some/file.txt -v 0755",
-   [r"error: Invalid argument"],
-   error=True)
-self.match("platform file read 16 -o 11 -c 13",
-   [r"error: Invalid argument"],
-   error=True)
-self.match("platform file write 16 -o 11 -d teststring",
-   [r"error: Invalid argument"],
-   error=True)
-self.match("platform file close 16",
-   [r"error: Invalid argument"],
-   error=True)
-self.assertPacketLogContains([
-  

[Lldb-commits] [lldb] 70558d3 - Revert "[lldb] [gdb-server] Implement the vFile:fstat packet"

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-09-10T11:43:24+02:00
New Revision: 70558d39f01beb87ab561bfaefeecb4d9534beed

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

LOG: Revert "[lldb] [gdb-server] Implement the vFile:fstat packet"

This reverts commit 9e886fbb18b525c080c04f4a12bd481c9aa849c0.  It breaks
on Windows.

Added: 


Modified: 
lldb/include/lldb/Utility/StringExtractorGDBRemote.h

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
lldb/source/Utility/StringExtractorGDBRemote.cpp
lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py

Removed: 




diff  --git a/lldb/include/lldb/Utility/StringExtractorGDBRemote.h 
b/lldb/include/lldb/Utility/StringExtractorGDBRemote.h
index 0c4ae32691b71..90b2837944cd1 100644
--- a/lldb/include/lldb/Utility/StringExtractorGDBRemote.h
+++ b/lldb/include/lldb/Utility/StringExtractorGDBRemote.h
@@ -88,7 +88,6 @@ class StringExtractorGDBRemote : public StringExtractor {
 eServerPacketType_vFile_mode,
 eServerPacketType_vFile_exists,
 eServerPacketType_vFile_md5,
-eServerPacketType_vFile_fstat,
 eServerPacketType_vFile_stat,
 eServerPacketType_vFile_symlink,
 eServerPacketType_vFile_unlink,

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
index 21700f57c77de..9627fb1720049 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -156,9 +156,6 @@ 
GDBRemoteCommunicationServerCommon::GDBRemoteCommunicationServerCommon(
   RegisterMemberFunctionHandler(
   StringExtractorGDBRemote::eServerPacketType_vFile_size,
   &GDBRemoteCommunicationServerCommon::Handle_vFile_Size);
-  RegisterMemberFunctionHandler(
-  StringExtractorGDBRemote::eServerPacketType_vFile_fstat,
-  &GDBRemoteCommunicationServerCommon::Handle_vFile_FStat);
   RegisterMemberFunctionHandler(
   StringExtractorGDBRemote::eServerPacketType_vFile_stat,
   &GDBRemoteCommunicationServerCommon::Handle_vFile_Stat);
@@ -757,46 +754,6 @@ GDBRemoteCommunicationServerCommon::Handle_qPlatform_shell(
   return SendErrorResponse(24);
 }
 
-template 
-static void fill_clamp(T &dest, U src, typename T::value_type fallback) {
-  dest = src <= std::numeric_limits::max() ? src
-   : fallback;
-}
-
-GDBRemoteCommunication::PacketResult
-GDBRemoteCommunicationServerCommon::Handle_vFile_FStat(
-StringExtractorGDBRemote &packet) {
-  StreamGDBRemote response;
-  packet.SetFilePos(::strlen("vFile:fstat:"));
-  int fd = packet.GetS32(-1, 16);
-
-  struct stat file_stats;
-  if (::fstat(fd, &file_stats) == -1) {
-const int save_errno = errno;
-response.Printf("F-1,%x", save_errno);
-return SendPacketNoLock(response.GetString());
-  }
-
-  GDBRemoteFStatData data;
-  fill_clamp(data.gdb_st_dev, file_stats.st_dev, 0);
-  fill_clamp(data.gdb_st_ino, file_stats.st_ino, 0);
-  data.gdb_st_mode = file_stats.st_mode;
-  fill_clamp(data.gdb_st_nlink, file_stats.st_nlink, UINT32_MAX);
-  fill_clamp(data.gdb_st_uid, file_stats.st_uid, 0);
-  fill_clamp(data.gdb_st_gid, file_stats.st_gid, 0);
-  fill_clamp(data.gdb_st_rdev, file_stats.st_rdev, 0);
-  data.gdb_st_size = file_stats.st_size;
-  data.gdb_st_blksize = file_stats.st_blksize;
-  data.gdb_st_blocks = file_stats.st_blocks;
-  fill_clamp(data.gdb_st_atime, file_stats.st_atime, 0);
-  fill_clamp(data.gdb_st_mtime, file_stats.st_mtime, 0);
-  fill_clamp(data.gdb_st_ctime, file_stats.st_ctime, 0);
-
-  response.Printf("F%zx;", sizeof(data));
-  response.PutEscapedBytes(&data, sizeof(data));
-  return SendPacketNoLock(response.GetString());
-}
-
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerCommon::Handle_vFile_Stat(
 StringExtractorGDBRemote &packet) {

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
index 029972348ef01..ecd80923fcf0f 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
@@ -71,8 +71,6 @@ class GDBRemoteCommunicationServerCommon : public 
GDBRemoteCommunicationServer {
 
   PacketResult Handle_vFile_unlink(StringExtractorGDBRemote &packet);
 
-  PacketResult Handle_vFile_FStat(StringExtractorGDBRemote &packet);
-
   PacketResult Handle_vFile_Stat(StringExtractorGDBRemote &packet);
 
   PacketRe

[Lldb-commits] [PATCH] D107840: [lldb] [gdb-server] Implement the vFile:fstat packet

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

mgorny updated this revision to Diff 371847.
mgorny added a comment.

Skip `st_blocks` and `st_blksize` on Windows.


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

https://reviews.llvm.org/D107840

Files:
  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py

Index: lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
@@ -9,7 +9,38 @@
 from gdbremote_testcase import GdbRemoteTestCaseBase
 
 import binascii
+import os
 import stat
+import struct
+import typing
+
+
+class GDBStat(typing.NamedTuple):
+st_dev: int
+st_ino: int
+st_mode: int
+st_nlink: int
+st_uid: int
+st_gid: int
+st_rdev: int
+st_size: int
+st_blksize: int
+st_blocks: int
+st_atime: int
+st_mtime: int
+st_ctime: int
+
+
+def uint32_or_zero(x):
+return x if x < 2**32 else 0
+
+
+def uint32_or_max(x):
+return x if x < 2**32 else 2**32 - 1
+
+
+def uint32_trunc(x):
+return x & (2**32 - 1)
 
 
 class TestGdbRemotePlatformFile(GdbRemoteTestCaseBase):
@@ -178,6 +209,70 @@
 True)
 self.expect_gdbremote_sequence()
 
+@skipIfWindows
+@add_test_categories(["llgs"])
+def test_platform_file_fstat(self):
+server = self.connect_to_debug_monitor()
+self.assertIsNotNone(server)
+
+with tempfile.NamedTemporaryFile() as temp_file:
+temp_file.write(b"some test data for stat")
+temp_file.flush()
+
+self.do_handshake()
+self.test_sequence.add_log_lines(
+["read packet: $vFile:open:%s,0,0#00" % (
+binascii.b2a_hex(temp_file.name.encode()).decode(),),
+ {"direction": "send",
+ "regex": r"^\$F([0-9a-fA-F]+)#[0-9a-fA-F]{2}$",
+ "capture": {1: "fd"}}],
+True)
+
+context = self.expect_gdbremote_sequence()
+self.assertIsNotNone(context)
+fd = int(context["fd"], 16)
+
+self.reset_test_sequence()
+self.test_sequence.add_log_lines(
+["read packet: $vFile:fstat:%x#00" % (fd,),
+ {"direction": "send",
+ "regex": r"^\$F([0-9a-fA-F]+);(.*)#[0-9a-fA-F]{2}$",
+ "capture": {1: "size", 2: "data"}}],
+True)
+context = self.expect_gdbremote_sequence()
+self.assertEqual(int(context["size"], 16), 64)
+# NB: we're using .encode() as a hack because the test suite
+# is wrongly using (unicode) str instead of bytes
+gdb_stat = GDBStat(
+*struct.unpack(">IIIQQQIII",
+   self.decode_gdbremote_binary(context["data"])
+.encode("iso-8859-1")))
+sys_stat = os.fstat(temp_file.fileno())
+
+self.assertEqual(gdb_stat.st_dev, uint32_or_zero(sys_stat.st_dev))
+self.assertEqual(gdb_stat.st_ino, uint32_or_zero(sys_stat.st_ino))
+self.assertEqual(gdb_stat.st_mode, uint32_trunc(sys_stat.st_mode))
+self.assertEqual(gdb_stat.st_nlink, uint32_or_max(sys_stat.st_nlink))
+self.assertEqual(gdb_stat.st_uid, uint32_or_zero(sys_stat.st_uid))
+self.assertEqual(gdb_stat.st_gid, uint32_or_zero(sys_stat.st_gid))
+self.assertEqual(gdb_stat.st_rdev, uint32_or_zero(sys_stat.st_rdev))
+self.assertEqual(gdb_stat.st_size, sys_stat.st_size)
+self.assertEqual(gdb_stat.st_blksize, sys_stat.st_blksize)
+self.assertEqual(gdb_stat.st_blocks, sys_stat.st_blocks)
+self.assertEqual(gdb_stat.st_atime,
+ uint32_or_zero(int(sys_stat.st_atime)))
+self.assertEqual(gdb_stat.st_mtime,
+ uint32_or_zero(int(sys_stat.st_mtime)))
+self.assertEqual(gdb_stat.st_ctime,
+ uint32_or_zero(int(sys_stat.st_ctime)))
+
+self.reset_test_sequence()
+self.test_sequence.add_log_lines(
+["read packet: $vFile:close:%x#00" % (fd,),
+ "send packet: $F0#00"],
+True)
+self.expect_gdbremote_sequence()
+
 def expect_error(self):
 self.test_sequence.add_log_lines(
 [{"direction": "send",
Index: lldb/source/Utility/StringExtractorGDBRemote.cpp
===
--- lldb/source/Utility/StringExtractorGDBRemote.cpp
+++ lldb/source/Utilit

[Lldb-commits] [PATCH] D107840: [lldb] [gdb-server] Implement the vFile:fstat packet

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp:789
+  data.gdb_st_size = file_stats.st_size;
+#if !defined(_WIN32)
+  data.gdb_st_blksize = file_stats.st_blksize;

Just make sure the fields stay initialized (to zero, I guess)


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

https://reviews.llvm.org/D107840

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

[Lldb-commits] [PATCH] D109585: [lldb] [test] Move "platform connect" logic into a common class

[Pavel Labath via Phabricator via lldb-commits]

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

cool


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

https://reviews.llvm.org/D109585

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

[Lldb-commits] [lldb] a1097d3 - Reland "[lldb] [gdb-server] Implement the vFile:fstat packet"

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-09-10T11:57:59+02:00
New Revision: a1097d315c80a1c079dc7dda1661d0c2baa2d1e6

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

LOG: Reland "[lldb] [gdb-server] Implement the vFile:fstat packet"

Now with an #ifdef for WIN32.

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

Added: 


Modified: 
lldb/include/lldb/Utility/StringExtractorGDBRemote.h

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
lldb/source/Utility/StringExtractorGDBRemote.cpp
lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py

Removed: 




diff  --git a/lldb/include/lldb/Utility/StringExtractorGDBRemote.h 
b/lldb/include/lldb/Utility/StringExtractorGDBRemote.h
index 90b2837944cd1..0c4ae32691b71 100644
--- a/lldb/include/lldb/Utility/StringExtractorGDBRemote.h
+++ b/lldb/include/lldb/Utility/StringExtractorGDBRemote.h
@@ -88,6 +88,7 @@ class StringExtractorGDBRemote : public StringExtractor {
 eServerPacketType_vFile_mode,
 eServerPacketType_vFile_exists,
 eServerPacketType_vFile_md5,
+eServerPacketType_vFile_fstat,
 eServerPacketType_vFile_stat,
 eServerPacketType_vFile_symlink,
 eServerPacketType_vFile_unlink,

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
index 9627fb1720049..16aa0d687c2ee 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -156,6 +156,9 @@ 
GDBRemoteCommunicationServerCommon::GDBRemoteCommunicationServerCommon(
   RegisterMemberFunctionHandler(
   StringExtractorGDBRemote::eServerPacketType_vFile_size,
   &GDBRemoteCommunicationServerCommon::Handle_vFile_Size);
+  RegisterMemberFunctionHandler(
+  StringExtractorGDBRemote::eServerPacketType_vFile_fstat,
+  &GDBRemoteCommunicationServerCommon::Handle_vFile_FStat);
   RegisterMemberFunctionHandler(
   StringExtractorGDBRemote::eServerPacketType_vFile_stat,
   &GDBRemoteCommunicationServerCommon::Handle_vFile_Stat);
@@ -754,6 +757,48 @@ GDBRemoteCommunicationServerCommon::Handle_qPlatform_shell(
   return SendErrorResponse(24);
 }
 
+template 
+static void fill_clamp(T &dest, U src, typename T::value_type fallback) {
+  dest = src <= std::numeric_limits::max() ? src
+   : fallback;
+}
+
+GDBRemoteCommunication::PacketResult
+GDBRemoteCommunicationServerCommon::Handle_vFile_FStat(
+StringExtractorGDBRemote &packet) {
+  StreamGDBRemote response;
+  packet.SetFilePos(::strlen("vFile:fstat:"));
+  int fd = packet.GetS32(-1, 16);
+
+  struct stat file_stats;
+  if (::fstat(fd, &file_stats) == -1) {
+const int save_errno = errno;
+response.Printf("F-1,%x", save_errno);
+return SendPacketNoLock(response.GetString());
+  }
+
+  GDBRemoteFStatData data;
+  fill_clamp(data.gdb_st_dev, file_stats.st_dev, 0);
+  fill_clamp(data.gdb_st_ino, file_stats.st_ino, 0);
+  data.gdb_st_mode = file_stats.st_mode;
+  fill_clamp(data.gdb_st_nlink, file_stats.st_nlink, UINT32_MAX);
+  fill_clamp(data.gdb_st_uid, file_stats.st_uid, 0);
+  fill_clamp(data.gdb_st_gid, file_stats.st_gid, 0);
+  fill_clamp(data.gdb_st_rdev, file_stats.st_rdev, 0);
+  data.gdb_st_size = file_stats.st_size;
+#if !defined(_WIN32)
+  data.gdb_st_blksize = file_stats.st_blksize;
+  data.gdb_st_blocks = file_stats.st_blocks;
+#endif
+  fill_clamp(data.gdb_st_atime, file_stats.st_atime, 0);
+  fill_clamp(data.gdb_st_mtime, file_stats.st_mtime, 0);
+  fill_clamp(data.gdb_st_ctime, file_stats.st_ctime, 0);
+
+  response.Printf("F%zx;", sizeof(data));
+  response.PutEscapedBytes(&data, sizeof(data));
+  return SendPacketNoLock(response.GetString());
+}
+
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerCommon::Handle_vFile_Stat(
 StringExtractorGDBRemote &packet) {

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
index ecd80923fcf0f..029972348ef01 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
@@ -71,6 +71,8 @@ class GDBRemoteCommunicationServerCommon : public 
GDBRemoteCommunicationServer {
 
   PacketResult Handle_vFile_unlink(StringExtractorGDBRemote &packet);
 
+  PacketResult Handle_vFile_FStat(StringExtractorGDBRemote &packet);
+
   PacketResult Handle_vFile_Stat(StringExtractorG

[Lldb-commits] [PATCH] D107840: [lldb] [gdb-server] Implement the vFile:fstat packet

[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 rGa1097d315c80: Reland "[lldb] [gdb-server] Implement the 
vFile:fstat packet" (authored by mgorny).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107840

Files:
  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py

Index: lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
@@ -9,7 +9,38 @@
 from gdbremote_testcase import GdbRemoteTestCaseBase
 
 import binascii
+import os
 import stat
+import struct
+import typing
+
+
+class GDBStat(typing.NamedTuple):
+st_dev: int
+st_ino: int
+st_mode: int
+st_nlink: int
+st_uid: int
+st_gid: int
+st_rdev: int
+st_size: int
+st_blksize: int
+st_blocks: int
+st_atime: int
+st_mtime: int
+st_ctime: int
+
+
+def uint32_or_zero(x):
+return x if x < 2**32 else 0
+
+
+def uint32_or_max(x):
+return x if x < 2**32 else 2**32 - 1
+
+
+def uint32_trunc(x):
+return x & (2**32 - 1)
 
 
 class TestGdbRemotePlatformFile(GdbRemoteTestCaseBase):
@@ -178,6 +209,70 @@
 True)
 self.expect_gdbremote_sequence()
 
+@skipIfWindows
+@add_test_categories(["llgs"])
+def test_platform_file_fstat(self):
+server = self.connect_to_debug_monitor()
+self.assertIsNotNone(server)
+
+with tempfile.NamedTemporaryFile() as temp_file:
+temp_file.write(b"some test data for stat")
+temp_file.flush()
+
+self.do_handshake()
+self.test_sequence.add_log_lines(
+["read packet: $vFile:open:%s,0,0#00" % (
+binascii.b2a_hex(temp_file.name.encode()).decode(),),
+ {"direction": "send",
+ "regex": r"^\$F([0-9a-fA-F]+)#[0-9a-fA-F]{2}$",
+ "capture": {1: "fd"}}],
+True)
+
+context = self.expect_gdbremote_sequence()
+self.assertIsNotNone(context)
+fd = int(context["fd"], 16)
+
+self.reset_test_sequence()
+self.test_sequence.add_log_lines(
+["read packet: $vFile:fstat:%x#00" % (fd,),
+ {"direction": "send",
+ "regex": r"^\$F([0-9a-fA-F]+);(.*)#[0-9a-fA-F]{2}$",
+ "capture": {1: "size", 2: "data"}}],
+True)
+context = self.expect_gdbremote_sequence()
+self.assertEqual(int(context["size"], 16), 64)
+# NB: we're using .encode() as a hack because the test suite
+# is wrongly using (unicode) str instead of bytes
+gdb_stat = GDBStat(
+*struct.unpack(">IIIQQQIII",
+   self.decode_gdbremote_binary(context["data"])
+.encode("iso-8859-1")))
+sys_stat = os.fstat(temp_file.fileno())
+
+self.assertEqual(gdb_stat.st_dev, uint32_or_zero(sys_stat.st_dev))
+self.assertEqual(gdb_stat.st_ino, uint32_or_zero(sys_stat.st_ino))
+self.assertEqual(gdb_stat.st_mode, uint32_trunc(sys_stat.st_mode))
+self.assertEqual(gdb_stat.st_nlink, uint32_or_max(sys_stat.st_nlink))
+self.assertEqual(gdb_stat.st_uid, uint32_or_zero(sys_stat.st_uid))
+self.assertEqual(gdb_stat.st_gid, uint32_or_zero(sys_stat.st_gid))
+self.assertEqual(gdb_stat.st_rdev, uint32_or_zero(sys_stat.st_rdev))
+self.assertEqual(gdb_stat.st_size, sys_stat.st_size)
+self.assertEqual(gdb_stat.st_blksize, sys_stat.st_blksize)
+self.assertEqual(gdb_stat.st_blocks, sys_stat.st_blocks)
+self.assertEqual(gdb_stat.st_atime,
+ uint32_or_zero(int(sys_stat.st_atime)))
+self.assertEqual(gdb_stat.st_mtime,
+ uint32_or_zero(int(sys_stat.st_mtime)))
+self.assertEqual(gdb_stat.st_ctime,
+ uint32_or_zero(int(sys_stat.st_ctime)))
+
+self.reset_test_sequence()
+self.test_sequence.add_log_lines(
+["read packet: $vFile:close:%x#00" % (fd,),
+ "send packet: $F0#00"],
+True)
+self.expect_gdbremote_sequence()
+
 def expect_error(self):
 self.test_sequence.add_log_lines(
 [{"direction": "send",
Index: lldb/source/Utility/St

[Lldb-commits] [lldb] e066c00 - [lldb] [gdb-server] Zero-initialize fields on WIN32

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-09-10T11:59:06+02:00
New Revision: e066c00be09a9257a28eaf12059e4d28f095ae65

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

LOG: [lldb] [gdb-server] Zero-initialize fields on WIN32

Added: 


Modified: 

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
index 16aa0d687c2e..410937483dcd 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -789,6 +789,9 @@ GDBRemoteCommunicationServerCommon::Handle_vFile_FStat(
 #if !defined(_WIN32)
   data.gdb_st_blksize = file_stats.st_blksize;
   data.gdb_st_blocks = file_stats.st_blocks;
+#else
+  data.gdb_st_blksize = 0;
+  data.gdb_st_blocks = 0;
 #endif
   fill_clamp(data.gdb_st_atime, file_stats.st_atime, 0);
   fill_clamp(data.gdb_st_mtime, file_stats.st_mtime, 0);



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

[Lldb-commits] [PATCH] D107809: [lldb] Add new commands and tests for getting file perms & exists

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

mgorny updated this revision to Diff 371858.
mgorny marked 2 inline comments as done.
mgorny added a comment.

Add a server test for error condition. Rebase client tests.


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

https://reviews.llvm.org/D107809

Files:
  lldb/source/Commands/CommandObjectPlatform.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
  lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py

Index: lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
@@ -176,6 +176,23 @@
 context = self.expect_gdbremote_sequence()
 self.assertEqual(int(context["mode"], 16), test_mode)
 
+@skipIfWindows
+@add_test_categories(["llgs"])
+def test_platform_file_mode_fail(self):
+server = self.connect_to_debug_monitor()
+self.assertIsNotNone(server)
+
+temp_path = self.getBuildArtifact("nonexist")
+
+self.do_handshake()
+self.test_sequence.add_log_lines(
+["read packet: $vFile:mode:%s#00" % (
+binascii.b2a_hex(temp_path.encode()).decode(),),
+ {"direction": "send",
+ "regex": r"^\$F-1,0*2+#[0-9a-fA-F]{2}$"}],
+True)
+self.expect_gdbremote_sequence()
+
 @skipIfWindows
 @add_test_categories(["llgs"])
 def test_platform_file_exists(self):
Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
@@ -100,3 +100,48 @@
 "vFile:fstat:5",
 "vFile:close:5",
 ])
+
+def test_file_permissions(self):
+"""Test 'platform get-permissions'"""
+
+class Responder(MockGDBServerResponder):
+def vFile(self, packet):
+return "F1a4"
+
+self.server.responder = Responder()
+
+self.match("platform get-permissions /some/file.txt",
+   [r"File permissions of /some/file\.txt \(remote\): 0o0644"])
+self.assertPacketLogContains([
+"vFile:mode:2f736f6d652f66696c652e747874",
+])
+
+def test_file_exists(self):
+"""Test 'platform file-exists'"""
+
+class Responder(MockGDBServerResponder):
+def vFile(self, packet):
+return "F,1"
+
+self.server.responder = Responder()
+
+self.match("platform file-exists /some/file.txt",
+   [r"File /some/file\.txt \(remote\) exists"])
+self.assertPacketLogContains([
+"vFile:exists:2f736f6d652f66696c652e747874",
+])
+
+def test_file_exists_not(self):
+"""Test 'platform file-exists' with non-existing file"""
+
+class Responder(MockGDBServerResponder):
+def vFile(self, packet):
+return "F,0"
+
+self.server.responder = Responder()
+
+self.match("platform file-exists /some/file.txt",
+   [r"File /some/file\.txt \(remote\) does not exist"])
+self.assertPacketLogContains([
+"vFile:exists:2f736f6d652f66696c652e747874",
+])
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -661,9 +661,10 @@
 std::error_code ec;
 const uint32_t mode = FileSystem::Instance().GetPermissions(file_spec, ec);
 StreamString response;
-response.Printf("F%x", mode);
-if (mode == 0 || ec)
-  response.Printf(",%x", (int)Status(ec).GetError());
+if (mode != llvm::sys::fs::perms_not_known)
+  response.Printf("F%x", mode);
+else
+  response.Printf("F-1,%x", (int)Status(ec).GetError());
 return SendPacketNoLock(response.GetString());
   }
   return SendErrorResponse(23);
Index: lldb/source/Commands/CommandObjectPlatform.cpp
===
--- lldb/source/Commands/CommandObjectPlatform.cpp
+++ lldb/source/Commands/CommandObjectPlatform.cpp
@@ -926,6 +926,141 @@
   }
 };
 
+// "platform get-permissions remote-file-path"
+class CommandObjectPlatformGetPermissions : public CommandObjectParsed {
+public:
+  CommandObjectPlatformGetPermissions(CommandInterpreter &interpreter)
+  : CommandObjectParsed(interpreter, "platform get-permissions",
+"Get the file permissio

[Lldb-commits] [PATCH] D109591: [lldb] [test] Synchronize before the breakpoint in fork tests

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

mgorny created this revision.
mgorny added reviewers: labath, krytarowski, emaste.
mgorny requested review of this revision.

We set breakpoint on child_func, so synchronization inside it is too
late to guarantee ordering between the parent output and child
breakpoint.  Split the function in two, and perform synchronization
before the breakpoint.


https://reviews.llvm.org/D109591

Files:
  lldb/test/Shell/Subprocess/Inputs/fork.cpp


Index: lldb/test/Shell/Subprocess/Inputs/fork.cpp
===
--- lldb/test/Shell/Subprocess/Inputs/fork.cpp
+++ lldb/test/Shell/Subprocess/Inputs/fork.cpp
@@ -28,9 +28,24 @@
   assert(ret == 0);
 }
 
-int child_func(void *argv0_ptr) {
+// This is the function we set breakpoint on.
+int child_func(const char *argv0) {
+  // we need to avoid memory modifications for vfork(), yet we want
+  // to be able to test watchpoints, so do the next best thing
+  // and restore the original value
+  g_val = 2;
+  g_val = 0;
+  execl(argv0, argv0, parent_done_str, NULL);
+  assert(0 && "this should not be reached");
+  return 1;
+}
+
+int child_top_func(void *argv0_ptr) {
   const char *argv0 = static_cast(argv0_ptr);
 
+  int ret = close(parent_done[1]);
+  assert(ret == 0);
+
   // NB: when using vfork(), the parent may be suspended while running
   // this function, so do not rely on any synchronization until we exec
 #if defined(TEST_FORK)
@@ -38,17 +53,7 @@
 #endif
 wait_for_parent();
 
-  int ret = close(parent_done[1]);
-  assert(ret == 0);
-
-  // we need to avoid memory modifications for vfork(), yet we want
-  // to be able to test watchpoints, so do the next best thing
-  // and restore the original value
-  g_val = 2;
-  g_val = 0;
-  execl(argv0, argv0, parent_done_str, NULL);
-  assert(0 && "this should not be reached");
-  return 1;
+  return child_func(argv0);
 }
 
 int main(int argc, char* argv[]) {
@@ -77,12 +82,12 @@
   assert(ret != -1);
 
 #if defined(TEST_CLONE)
-  pid_t pid = clone(child_func, &stack[sizeof(stack)], 0, argv[0]);
+  pid_t pid = clone(child_top_func, &stack[sizeof(stack)], 0, argv[0]);
 #elif defined(TEST_FORK)
   pid_t pid = TEST_FORK();
   // NB: this must be equivalent to the clone() call above
   if (pid == 0)
-_exit(child_func(argv[0]));
+_exit(child_top_func(argv[0]));
 #endif
   assert(pid != -1);
 


Index: lldb/test/Shell/Subprocess/Inputs/fork.cpp
===
--- lldb/test/Shell/Subprocess/Inputs/fork.cpp
+++ lldb/test/Shell/Subprocess/Inputs/fork.cpp
@@ -28,9 +28,24 @@
   assert(ret == 0);
 }
 
-int child_func(void *argv0_ptr) {
+// This is the function we set breakpoint on.
+int child_func(const char *argv0) {
+  // we need to avoid memory modifications for vfork(), yet we want
+  // to be able to test watchpoints, so do the next best thing
+  // and restore the original value
+  g_val = 2;
+  g_val = 0;
+  execl(argv0, argv0, parent_done_str, NULL);
+  assert(0 && "this should not be reached");
+  return 1;
+}
+
+int child_top_func(void *argv0_ptr) {
   const char *argv0 = static_cast(argv0_ptr);
 
+  int ret = close(parent_done[1]);
+  assert(ret == 0);
+
   // NB: when using vfork(), the parent may be suspended while running
   // this function, so do not rely on any synchronization until we exec
 #if defined(TEST_FORK)
@@ -38,17 +53,7 @@
 #endif
 wait_for_parent();
 
-  int ret = close(parent_done[1]);
-  assert(ret == 0);
-
-  // we need to avoid memory modifications for vfork(), yet we want
-  // to be able to test watchpoints, so do the next best thing
-  // and restore the original value
-  g_val = 2;
-  g_val = 0;
-  execl(argv0, argv0, parent_done_str, NULL);
-  assert(0 && "this should not be reached");
-  return 1;
+  return child_func(argv0);
 }
 
 int main(int argc, char* argv[]) {
@@ -77,12 +82,12 @@
   assert(ret != -1);
 
 #if defined(TEST_CLONE)
-  pid_t pid = clone(child_func, &stack[sizeof(stack)], 0, argv[0]);
+  pid_t pid = clone(child_top_func, &stack[sizeof(stack)], 0, argv[0]);
 #elif defined(TEST_FORK)
   pid_t pid = TEST_FORK();
   // NB: this must be equivalent to the clone() call above
   if (pid == 0)
-_exit(child_func(argv[0]));
+_exit(child_top_func(argv[0]));
 #endif
   assert(pid != -1);
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] caf508d - [lldb] [test] Synchronize before the breakpoint in fork tests

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-09-10T13:06:01+02:00
New Revision: caf508d7124353522e7604dbfea36b429469bd39

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

LOG: [lldb] [test] Synchronize before the breakpoint in fork tests

We set breakpoint on child_func, so synchronization inside it is too
late to guarantee ordering between the parent output and child
breakpoint.  Split the function in two, and perform synchronization
before the breakpoint.

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

Added: 


Modified: 
lldb/test/Shell/Subprocess/Inputs/fork.cpp

Removed: 




diff  --git a/lldb/test/Shell/Subprocess/Inputs/fork.cpp 
b/lldb/test/Shell/Subprocess/Inputs/fork.cpp
index f4cf1c2c42dc1..81b1d116c9436 100644
--- a/lldb/test/Shell/Subprocess/Inputs/fork.cpp
+++ b/lldb/test/Shell/Subprocess/Inputs/fork.cpp
@@ -28,9 +28,24 @@ void wait_for_parent() {
   assert(ret == 0);
 }
 
-int child_func(void *argv0_ptr) {
+// This is the function we set breakpoint on.
+int child_func(const char *argv0) {
+  // we need to avoid memory modifications for vfork(), yet we want
+  // to be able to test watchpoints, so do the next best thing
+  // and restore the original value
+  g_val = 2;
+  g_val = 0;
+  execl(argv0, argv0, parent_done_str, NULL);
+  assert(0 && "this should not be reached");
+  return 1;
+}
+
+int child_top_func(void *argv0_ptr) {
   const char *argv0 = static_cast(argv0_ptr);
 
+  int ret = close(parent_done[1]);
+  assert(ret == 0);
+
   // NB: when using vfork(), the parent may be suspended while running
   // this function, so do not rely on any synchronization until we exec
 #if defined(TEST_FORK)
@@ -38,17 +53,7 @@ int child_func(void *argv0_ptr) {
 #endif
 wait_for_parent();
 
-  int ret = close(parent_done[1]);
-  assert(ret == 0);
-
-  // we need to avoid memory modifications for vfork(), yet we want
-  // to be able to test watchpoints, so do the next best thing
-  // and restore the original value
-  g_val = 2;
-  g_val = 0;
-  execl(argv0, argv0, parent_done_str, NULL);
-  assert(0 && "this should not be reached");
-  return 1;
+  return child_func(argv0);
 }
 
 int main(int argc, char* argv[]) {
@@ -77,12 +82,12 @@ int main(int argc, char* argv[]) {
   assert(ret != -1);
 
 #if defined(TEST_CLONE)
-  pid_t pid = clone(child_func, &stack[sizeof(stack)], 0, argv[0]);
+  pid_t pid = clone(child_top_func, &stack[sizeof(stack)], 0, argv[0]);
 #elif defined(TEST_FORK)
   pid_t pid = TEST_FORK();
   // NB: this must be equivalent to the clone() call above
   if (pid == 0)
-_exit(child_func(argv[0]));
+_exit(child_top_func(argv[0]));
 #endif
   assert(pid != -1);
 



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

[Lldb-commits] [PATCH] D109591: [lldb] [test] Synchronize before the breakpoint in fork tests

[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 rGcaf508d71243: [lldb] [test] Synchronize before the 
breakpoint in fork tests (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109591

Files:
  lldb/test/Shell/Subprocess/Inputs/fork.cpp


Index: lldb/test/Shell/Subprocess/Inputs/fork.cpp
===
--- lldb/test/Shell/Subprocess/Inputs/fork.cpp
+++ lldb/test/Shell/Subprocess/Inputs/fork.cpp
@@ -28,9 +28,24 @@
   assert(ret == 0);
 }
 
-int child_func(void *argv0_ptr) {
+// This is the function we set breakpoint on.
+int child_func(const char *argv0) {
+  // we need to avoid memory modifications for vfork(), yet we want
+  // to be able to test watchpoints, so do the next best thing
+  // and restore the original value
+  g_val = 2;
+  g_val = 0;
+  execl(argv0, argv0, parent_done_str, NULL);
+  assert(0 && "this should not be reached");
+  return 1;
+}
+
+int child_top_func(void *argv0_ptr) {
   const char *argv0 = static_cast(argv0_ptr);
 
+  int ret = close(parent_done[1]);
+  assert(ret == 0);
+
   // NB: when using vfork(), the parent may be suspended while running
   // this function, so do not rely on any synchronization until we exec
 #if defined(TEST_FORK)
@@ -38,17 +53,7 @@
 #endif
 wait_for_parent();
 
-  int ret = close(parent_done[1]);
-  assert(ret == 0);
-
-  // we need to avoid memory modifications for vfork(), yet we want
-  // to be able to test watchpoints, so do the next best thing
-  // and restore the original value
-  g_val = 2;
-  g_val = 0;
-  execl(argv0, argv0, parent_done_str, NULL);
-  assert(0 && "this should not be reached");
-  return 1;
+  return child_func(argv0);
 }
 
 int main(int argc, char* argv[]) {
@@ -77,12 +82,12 @@
   assert(ret != -1);
 
 #if defined(TEST_CLONE)
-  pid_t pid = clone(child_func, &stack[sizeof(stack)], 0, argv[0]);
+  pid_t pid = clone(child_top_func, &stack[sizeof(stack)], 0, argv[0]);
 #elif defined(TEST_FORK)
   pid_t pid = TEST_FORK();
   // NB: this must be equivalent to the clone() call above
   if (pid == 0)
-_exit(child_func(argv[0]));
+_exit(child_top_func(argv[0]));
 #endif
   assert(pid != -1);
 


Index: lldb/test/Shell/Subprocess/Inputs/fork.cpp
===
--- lldb/test/Shell/Subprocess/Inputs/fork.cpp
+++ lldb/test/Shell/Subprocess/Inputs/fork.cpp
@@ -28,9 +28,24 @@
   assert(ret == 0);
 }
 
-int child_func(void *argv0_ptr) {
+// This is the function we set breakpoint on.
+int child_func(const char *argv0) {
+  // we need to avoid memory modifications for vfork(), yet we want
+  // to be able to test watchpoints, so do the next best thing
+  // and restore the original value
+  g_val = 2;
+  g_val = 0;
+  execl(argv0, argv0, parent_done_str, NULL);
+  assert(0 && "this should not be reached");
+  return 1;
+}
+
+int child_top_func(void *argv0_ptr) {
   const char *argv0 = static_cast(argv0_ptr);
 
+  int ret = close(parent_done[1]);
+  assert(ret == 0);
+
   // NB: when using vfork(), the parent may be suspended while running
   // this function, so do not rely on any synchronization until we exec
 #if defined(TEST_FORK)
@@ -38,17 +53,7 @@
 #endif
 wait_for_parent();
 
-  int ret = close(parent_done[1]);
-  assert(ret == 0);
-
-  // we need to avoid memory modifications for vfork(), yet we want
-  // to be able to test watchpoints, so do the next best thing
-  // and restore the original value
-  g_val = 2;
-  g_val = 0;
-  execl(argv0, argv0, parent_done_str, NULL);
-  assert(0 && "this should not be reached");
-  return 1;
+  return child_func(argv0);
 }
 
 int main(int argc, char* argv[]) {
@@ -77,12 +82,12 @@
   assert(ret != -1);
 
 #if defined(TEST_CLONE)
-  pid_t pid = clone(child_func, &stack[sizeof(stack)], 0, argv[0]);
+  pid_t pid = clone(child_top_func, &stack[sizeof(stack)], 0, argv[0]);
 #elif defined(TEST_FORK)
   pid_t pid = TEST_FORK();
   // NB: this must be equivalent to the clone() call above
   if (pid == 0)
-_exit(child_func(argv[0]));
+_exit(child_top_func(argv[0]));
 #endif
   assert(pid != -1);
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 0c8444b - [lldb] Fix Clang modules build after D101329

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2021-09-10T13:10:19+02:00
New Revision: 0c8444bd3462a3d05c8ac637a554e1a368dee0ac

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

LOG: [lldb] Fix Clang modules build after D101329

D101329 introduces the Process:SaveCore function returning a
`llvm::Expected`. That function causes that Clang with -fmodules crashes
while compiling LLDB's PythonDataObjects.cpp. With enabled asserts Clang fails
because of:

Assertion failed: (CachedFieldIndex && "failed to find field in parent")

Crash can be reproduced by building via -DLLVM_ENABLE_MODULES=On with Clang
12.0.1 and then building PythonDataObjects.cpp.o .

Clang bug is tracked at rdar://82901462

Added: 


Modified: 
lldb/include/lldb/Target/Process.h
lldb/source/Target/Process.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index eb7ad99d5adf3..741c9e1021cc6 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -695,9 +695,7 @@ class Process : public 
std::enable_shared_from_this,
   /// \return
   /// true if saved successfully, false if saving the core dump
   /// is not supported by the plugin, error otherwise.
-  virtual llvm::Expected SaveCore(llvm::StringRef outfile) {
-return false;
-  }
+  virtual llvm::Expected SaveCore(llvm::StringRef outfile);
 
 protected:
   virtual JITLoaderList &GetJITLoaders();

diff  --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 5ae37d062581d..540f58d2ebc9e 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -2650,6 +2650,10 @@ DynamicLoader *Process::GetDynamicLoader() {
 
 DataExtractor Process::GetAuxvData() { return DataExtractor(); }
 
+llvm::Expected Process::SaveCore(llvm::StringRef outfile) {
+  return false;
+}
+
 JITLoaderList &Process::GetJITLoaders() {
   if (!m_jit_loaders_up) {
 m_jit_loaders_up = std::make_unique();



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

[Lldb-commits] [PATCH] D108061: [lldb] Add support for shared library load when executable called through ld.

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.

Ok, let's give this one more shot.

Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108061

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

[Lldb-commits] [PATCH] D107809: [lldb] Add new commands and tests for getting file perms & exists

[Pavel Labath via Phabricator via lldb-commits]

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

cool


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

https://reviews.llvm.org/D107809

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

[Lldb-commits] [PATCH] D108554: [lldb] Support querying registers via generic names without alt_names

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

mgorny added a comment.

Ok, so I've done some testing and indeed Native* part doesn't seem necessary.

However, after removing the aliases from x86_64 target def, there are two test 
regressions. The two following snippets no longer seem to work:

  eip = frame.FindRegister("pc")
  sp_value = frame0.FindValue("sp", lldb.eValueTypeRegister)

This doesn't seem to work anymore. Should I attempt to fix the API to make 
grabbing these registers by name work again, or should I try to modify the 
tests to grab these registers via generic reg nums?


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

https://reviews.llvm.org/D108554

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

[Lldb-commits] [lldb] c240d2b - [lldb] [test] Move "platform connect" logic into a common class

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-09-10T14:08:35+02:00
New Revision: c240d2bb06dabe8fb3a1c2da978fbdc9f642de73

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

LOG: [lldb] [test] Move "platform connect" logic into a common class

Create a common GDBPlatformClientTestBase class and move the platform
select/connect logic there to reduce duplication.

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

Added: 


Modified: 

lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteDiskFileCompletion.py
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteDiskFileCompletion.py
 
b/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteDiskFileCompletion.py
index c0afce00a4cf1..8109caf3d7310 100644
--- 
a/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteDiskFileCompletion.py
+++ 
b/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteDiskFileCompletion.py
@@ -1,6 +1,6 @@
 from gdbclientutils import *
 
-class TestGDBRemoteDiskFileCompletion(GDBRemoteTestBase):
+class TestGDBRemoteDiskFileCompletion(GDBPlatformClientTestBase):
 
 def test_autocomplete_request(self):
 """Test remote disk completion on remote-gdb-server plugin"""
@@ -14,16 +14,8 @@ def qPathComplete(self):
 
 self.server.responder = Responder()
 
-try:
-self.runCmd("platform select remote-gdb-server")
-self.runCmd("platform connect connect://" +
-self.server.get_connect_address())
-self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
-
-self.complete_from_to('platform get-size ', ['test', '123'])
-self.complete_from_to('platform get-file ', ['test', '123'])
-self.complete_from_to('platform put-file foo ', ['test', '123'])
-self.complete_from_to('platform file open ', ['test', '123'])
-self.complete_from_to('platform settings -w ', ['test', '123'])
-finally:
-self.dbg.GetSelectedPlatform().DisconnectRemote()
+self.complete_from_to('platform get-size ', ['test', '123'])
+self.complete_from_to('platform get-file ', ['test', '123'])
+self.complete_from_to('platform put-file foo ', ['test', '123'])
+self.complete_from_to('platform file open ', ['test', '123'])
+self.complete_from_to('platform settings -w ', ['test', '123'])

diff  --git 
a/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py 
b/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
index fcc2579266806..d5475986bb57d 100644
--- 
a/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
+++ 
b/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
@@ -1,6 +1,6 @@
 from gdbclientutils import *
 
-class TestGDBRemotePlatformFile(GDBRemoteTestBase):
+class TestGDBRemotePlatformFile(GDBPlatformClientTestBase):
 
 def test_file(self):
 """Test mock operations on a remote file"""
@@ -19,28 +19,20 @@ def vFile(self, packet):
 
 self.server.responder = Responder()
 
-try:
-self.runCmd("platform select remote-gdb-server")
-self.runCmd("platform connect connect://" +
-self.server.get_connect_address())
-self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
-
-self.match("platform file open /some/file.txt -v 0755",
-   [r"File Descriptor = 16"])
-self.match("platform file read 16 -o 11 -c 13",
-   [r"Return = 11\nData = \"frobnicator\""])
-self.match("platform file write 16 -o 11 -d teststring",
-   [r"Return = 10"])
-self.match("platform file close 16",
-   [r"file 16 closed."])
-self.assertPacketLogContains([
-"vFile:open:2f736f6d652f66696c652e747874,0202,01ed",
-"vFile:pread:10,d,b",
-"vFile:pwrite:10,b,teststring",
-"vFile:close:10",
-])
-finally:
-self.dbg.GetSelectedPlatform().DisconnectRemote()
+self.match("platform file open /some/file.txt -v 0755",
+   [r"File Descriptor = 16"])
+self.match("platform file read 16 -o 11 -c 13",
+   [r"Return = 11\nData = \"frobnicator\""])
+self.match("platform file write 16 -o 11 -d teststring",
+   [r"Return = 10"])
+self.match("platform file close 16",
+   [r"file 16 closed."])

[Lldb-commits] [lldb] dbb0c14 - [lldb] Add new commands and tests for getting file perms & exists

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-09-10T14:08:36+02:00
New Revision: dbb0c14d2729d135d9d6bb2d0e858e128129da08

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

LOG: [lldb] Add new commands and tests for getting file perms & exists

Add two new commands 'platform get-file-permissions' and 'platform
file-exists' for the respective bits of LLDB protocol.  Add tests for
them.  Fix error handling in GetFilePermissions().

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

Added: 


Modified: 
lldb/source/Commands/CommandObjectPlatform.cpp

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectPlatform.cpp 
b/lldb/source/Commands/CommandObjectPlatform.cpp
index 0274605f2e329..7760eba290d4b 100644
--- a/lldb/source/Commands/CommandObjectPlatform.cpp
+++ b/lldb/source/Commands/CommandObjectPlatform.cpp
@@ -926,6 +926,141 @@ class CommandObjectPlatformGetSize : public 
CommandObjectParsed {
   }
 };
 
+// "platform get-permissions remote-file-path"
+class CommandObjectPlatformGetPermissions : public CommandObjectParsed {
+public:
+  CommandObjectPlatformGetPermissions(CommandInterpreter &interpreter)
+  : CommandObjectParsed(interpreter, "platform get-permissions",
+"Get the file permission bits from the remote 
end.",
+"platform get-permissions ", 0) {
+SetHelpLong(
+R"(Examples:
+
+(lldb) platform get-permissions /the/remote/file/path
+
+Get the file permissions from the remote end with path 
/the/remote/file/path.)");
+
+CommandArgumentEntry arg1;
+CommandArgumentData file_arg_remote;
+
+// Define the first (and only) variant of this arg.
+file_arg_remote.arg_type = eArgTypeFilename;
+file_arg_remote.arg_repetition = eArgRepeatPlain;
+// There is only one variant this argument could be; put it into the
+// argument entry.
+arg1.push_back(file_arg_remote);
+
+// Push the data for the first argument into the m_arguments vector.
+m_arguments.push_back(arg1);
+  }
+
+  ~CommandObjectPlatformGetPermissions() override = default;
+
+  void
+  HandleArgumentCompletion(CompletionRequest &request,
+   OptionElementVector &opt_element_vector) override {
+if (request.GetCursorIndex() != 0)
+  return;
+
+CommandCompletions::InvokeCommonCompletionCallbacks(
+GetCommandInterpreter(), CommandCompletions::eRemoteDiskFileCompletion,
+request, nullptr);
+  }
+
+  bool DoExecute(Args &args, CommandReturnObject &result) override {
+// If the number of arguments is incorrect, issue an error message.
+if (args.GetArgumentCount() != 1) {
+  result.AppendError("required argument missing; specify the source file "
+ "path as the only argument");
+  return false;
+}
+
+PlatformSP platform_sp(
+GetDebugger().GetPlatformList().GetSelectedPlatform());
+if (platform_sp) {
+  std::string remote_file_path(args.GetArgumentAtIndex(0));
+  uint32_t permissions;
+  Status error = 
platform_sp->GetFilePermissions(FileSpec(remote_file_path),
+ permissions);
+  if (error.Success()) {
+result.AppendMessageWithFormat(
+"File permissions of %s (remote): 0o%04" PRIo32 "\n",
+remote_file_path.c_str(), permissions);
+result.SetStatus(eReturnStatusSuccessFinishResult);
+  } else
+result.AppendError(error.AsCString());
+} else {
+  result.AppendError("no platform currently selected\n");
+}
+return result.Succeeded();
+  }
+};
+
+// "platform file-exists remote-file-path"
+class CommandObjectPlatformFileExists : public CommandObjectParsed {
+public:
+  CommandObjectPlatformFileExists(CommandInterpreter &interpreter)
+  : CommandObjectParsed(interpreter, "platform file-exists",
+"Check if the file exists on the remote end.",
+"platform file-exists ", 0) {
+SetHelpLong(
+R"(Examples:
+
+(lldb) platform file-exists /the/remote/file/path
+
+Check if /the/remote/file/path exists on the remote end.)");
+
+CommandArgumentEntry arg1;
+CommandArgumentData file_arg_remote;
+
+// Define the first (and only) variant of this arg.
+file_arg_remote.arg_type = eArgTypeFilename;
+file_arg_remote.arg_repetition = eArgRepeatPlain;
+// There is only one variant this argument could be; put it into the
+// argument entry.
+arg1.push_back(file_arg_remote)

[Lldb-commits] [PATCH] D109585: [lldb] [test] Move "platform connect" logic into a common class

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc240d2bb06da: [lldb] [test] Move "platform 
connect" logic into a common class (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109585

Files:
  
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteDiskFileCompletion.py
  lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
  lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py

Index: lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
===
--- lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
+++ lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
@@ -566,3 +566,23 @@
 if i < len(packets):
 self.fail(u"Did not receive: %s\nLast 10 packets:\n\t%s" %
 (packets[i], u'\n\t'.join(log)))
+
+
+class GDBPlatformClientTestBase(GDBRemoteTestBase):
+"""
+Base class for platform server clients.
+
+This class extends GDBRemoteTestBase by automatically connecting
+via "platform connect" in the setUp() method.
+"""
+
+def setUp(self):
+super().setUp()
+self.runCmd("platform select remote-gdb-server")
+self.runCmd("platform connect connect://" +
+self.server.get_connect_address())
+self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
+
+def tearDown(self):
+self.dbg.GetSelectedPlatform().DisconnectRemote()
+super().tearDown()
Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
@@ -1,6 +1,6 @@
 from gdbclientutils import *
 
-class TestGDBRemotePlatformFile(GDBRemoteTestBase):
+class TestGDBRemotePlatformFile(GDBPlatformClientTestBase):
 
 def test_file(self):
 """Test mock operations on a remote file"""
@@ -19,28 +19,20 @@
 
 self.server.responder = Responder()
 
-try:
-self.runCmd("platform select remote-gdb-server")
-self.runCmd("platform connect connect://" +
-self.server.get_connect_address())
-self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
-
-self.match("platform file open /some/file.txt -v 0755",
-   [r"File Descriptor = 16"])
-self.match("platform file read 16 -o 11 -c 13",
-   [r"Return = 11\nData = \"frobnicator\""])
-self.match("platform file write 16 -o 11 -d teststring",
-   [r"Return = 10"])
-self.match("platform file close 16",
-   [r"file 16 closed."])
-self.assertPacketLogContains([
-"vFile:open:2f736f6d652f66696c652e747874,0202,01ed",
-"vFile:pread:10,d,b",
-"vFile:pwrite:10,b,teststring",
-"vFile:close:10",
-])
-finally:
-self.dbg.GetSelectedPlatform().DisconnectRemote()
+self.match("platform file open /some/file.txt -v 0755",
+   [r"File Descriptor = 16"])
+self.match("platform file read 16 -o 11 -c 13",
+   [r"Return = 11\nData = \"frobnicator\""])
+self.match("platform file write 16 -o 11 -d teststring",
+   [r"Return = 10"])
+self.match("platform file close 16",
+   [r"file 16 closed."])
+self.assertPacketLogContains([
+"vFile:open:2f736f6d652f66696c652e747874,0202,01ed",
+"vFile:pread:10,d,b",
+"vFile:pwrite:10,b,teststring",
+"vFile:close:10",
+])
 
 def test_file_fail(self):
 """Test mocked failures of remote operations"""
@@ -51,32 +43,24 @@
 
 self.server.responder = Responder()
 
-try:
-self.runCmd("platform select remote-gdb-server")
-self.runCmd("platform connect connect://" +
-self.server.get_connect_address())
-self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
-
-self.match("platform file open /some/file.txt -v 0755",
-   [r"error: Invalid argument"],
-   error=True)
-self.match("platform file read 16 -o 11 -c 13",
-   [r"error: Invalid argument"],
-   error=True)
-self.match("platform file write 16 -o 11 -d teststring",
-   [r"error: Invalid argument"],
-   error=True)
-self.match("platfo

[Lldb-commits] [lldb] 501eaf8 - [lldb] [gdb-remote] Add fallbacks for vFile:mode and vFile:exists

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-09-10T14:08:36+02:00
New Revision: 501eaf88770d15de92fa0eb7435f0470a3b93b0a

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

LOG: [lldb] [gdb-remote] Add fallbacks for vFile:mode and vFile:exists

Add a GDB-compatible fallback to vFile:fstat for vFile:mode, and to
vFile:open for vFile:exists.  Note that this is only partial fallback,
as it fails if the file cannot be opened.

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

Added: 


Modified: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 4bcef1f05cdb..588e3faf0187 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -65,7 +65,8 @@ GDBRemoteCommunicationClient::GDBRemoteCommunicationClient()
   m_supports_QEnvironmentHexEncoded(true), m_supports_qSymbol(true),
   m_qSymbol_requests_done(false), m_supports_qModuleInfo(true),
   m_supports_jThreadsInfo(true), m_supports_jModulesInfo(true),
-  m_supports_vFileSize(true),
+  m_supports_vFileSize(true), m_supports_vFileMode(true),
+  m_supports_vFileExists(true),
 
   m_host_arch(), m_process_arch(), m_os_build(), m_os_kernel(),
   m_hostname(), m_gdb_server_name(), m_default_packet_timeout(0),
@@ -3159,37 +3160,50 @@ void 
GDBRemoteCommunicationClient::AutoCompleteDiskFileOrDirectory(
 Status
 GDBRemoteCommunicationClient::GetFilePermissions(const FileSpec &file_spec,
  uint32_t &file_permissions) {
-  std::string path{file_spec.GetPath(false)};
-  Status error;
-  lldb_private::StreamString stream;
-  stream.PutCString("vFile:mode:");
-  stream.PutStringAsRawHex8(path);
-  StringExtractorGDBRemote response;
-  if (SendPacketAndWaitForResponse(stream.GetString(), response) ==
-  PacketResult::Success) {
-if (response.GetChar() != 'F') {
-  error.SetErrorStringWithFormat("invalid response to '%s' packet",
+  if (m_supports_vFileMode) {
+std::string path{file_spec.GetPath(false)};
+Status error;
+lldb_private::StreamString stream;
+stream.PutCString("vFile:mode:");
+stream.PutStringAsRawHex8(path);
+StringExtractorGDBRemote response;
+if (SendPacketAndWaitForResponse(stream.GetString(), response) !=
+PacketResult::Success) {
+  error.SetErrorStringWithFormat("failed to send '%s' packet",
  stream.GetData());
-} else {
-  const uint32_t mode = response.GetS32(-1, 16);
-  if (static_cast(mode) == -1) {
-if (response.GetChar() == ',') {
-  int response_errno = response.GetS32(-1, 16);
-  if (response_errno > 0)
-error.SetError(response_errno, lldb::eErrorTypePOSIX);
-  else
-error.SetErrorToGenericError();
-} else
-  error.SetErrorToGenericError();
+  return error;
+}
+if (!response.IsUnsupportedResponse()) {
+  if (response.GetChar() != 'F') {
+error.SetErrorStringWithFormat("invalid response to '%s' packet",
+   stream.GetData());
   } else {
-file_permissions = mode & (S_IRWXU | S_IRWXG | S_IRWXO);
+const uint32_t mode = response.GetS32(-1, 16);
+if (static_cast(mode) == -1) {
+  if (response.GetChar() == ',') {
+int response_errno = response.GetS32(-1, 16);
+if (response_errno > 0)
+  error.SetError(response_errno, lldb::eErrorTypePOSIX);
+else
+  error.SetErrorToGenericError();
+  } else
+error.SetErrorToGenericError();
+} else {
+  file_permissions = mode & (S_IRWXU | S_IRWXG | S_IRWXO);
+}
   }
+  return error;
+} else { // response.IsUnsupportedResponse()
+  m_supports_vFileMode = false;
 }
-  } else {
-error.SetErrorStringWithFormat("failed to send '%s' packet",
-   stream.GetData());
   }
-  return error;
+
+  // Fallback to fstat.
+  if (llvm::Optional st = Stat(file_spec)) {
+file_permissions = st->gdb_st_mode & (S_IRWXU | S_IRWXG | S_IRWXO);
+return Status();
+  }
+  return Status("fstat failed");
 }
 
 uint64_t GDBRemoteCommunicationClient::ReadFile(lldb::user_id_t fd,
@@ -3332,21 +3346,33 @@ Status GDBRemoteCommunicationClient::Unlink(const 
FileSpec &file_

[Lldb-commits] [lldb] 6ba3f72 - [lldb] [gdb-remote] Implement the vRun packet

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-09-10T14:08:36+02:00
New Revision: 6ba3f7237dc750aad2ce1d6a7a15e3b78370221a

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

LOG: [lldb] [gdb-remote] Implement the vRun packet

Implement the simpler vRun packet and prefer it over the A packet.
Unlike the latter, it tranmits command-line arguments without redundant
indices and lengths.  This also improves GDB compatibility since modern
versions of gdbserver do not implement the A packet at all.

Make qLaunchSuccess not obligatory when using vRun.  It is not
implemented by gdbserver, and since vRun returns the stop reason,
we can assume it to be successful.

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

Added: 


Modified: 
lldb/include/lldb/Utility/StringExtractorGDBRemote.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
lldb/source/Utility/StringExtractorGDBRemote.cpp
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
lldb/test/API/tools/lldb-server/TestLldbGdbServer.py

Removed: 




diff  --git a/lldb/include/lldb/Utility/StringExtractorGDBRemote.h 
b/lldb/include/lldb/Utility/StringExtractorGDBRemote.h
index 0c4ae32691b71..1712c113d396b 100644
--- a/lldb/include/lldb/Utility/StringExtractorGDBRemote.h
+++ b/lldb/include/lldb/Utility/StringExtractorGDBRemote.h
@@ -136,6 +136,7 @@ class StringExtractorGDBRemote : public StringExtractor {
 eServerPacketType_vAttachName,
 eServerPacketType_vCont,
 eServerPacketType_vCont_actions, // vCont?
+eServerPacketType_vRun,
 
 eServerPacketType_stop_reason, // '?'
 

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 588e3faf01874..3696c7096e30e 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -66,7 +66,7 @@ GDBRemoteCommunicationClient::GDBRemoteCommunicationClient()
   m_qSymbol_requests_done(false), m_supports_qModuleInfo(true),
   m_supports_jThreadsInfo(true), m_supports_jModulesInfo(true),
   m_supports_vFileSize(true), m_supports_vFileMode(true),
-  m_supports_vFileExists(true),
+  m_supports_vFileExists(true), m_supports_vRun(true),
 
   m_host_arch(), m_process_arch(), m_os_build(), m_os_kernel(),
   m_hostname(), m_gdb_server_name(), m_default_packet_timeout(0),
@@ -794,6 +794,11 @@ bool 
GDBRemoteCommunicationClient::GetLaunchSuccess(std::string &error_str) {
   PacketResult::Success) {
 if (response.IsOKResponse())
   return true;
+// GDB does not implement qLaunchSuccess -- but if we used vRun,
+// then we already received a successful launch indication via stop
+// reason.
+if (response.IsUnsupportedResponse() && m_supports_vRun)
+  return true;
 if (response.GetChar() == 'E') {
   // A string the describes what failed when launching...
   error_str = std::string(response.GetStringRef().substr(1));
@@ -832,6 +837,36 @@ int GDBRemoteCommunicationClient::SendArgumentsPacket(
 }
   }
   if (!argv.empty()) {
+// try vRun first
+if (m_supports_vRun) {
+  StreamString packet;
+  packet.PutCString("vRun");
+  for (const char *arg : argv) {
+packet.PutChar(';');
+packet.PutBytesAsRawHex8(arg, strlen(arg));
+  }
+
+  StringExtractorGDBRemote response;
+  if (SendPacketAndWaitForResponse(packet.GetString(), response) !=
+  PacketResult::Success)
+return -1;
+
+  if (response.IsErrorResponse()) {
+uint8_t error = response.GetError();
+if (error)
+  return error;
+return -1;
+  }
+  // vRun replies with a stop reason packet
+  // FIXME: right now we just discard the packet and LLDB queries
+  // for stop reason again
+  if (!response.IsUnsupportedResponse())
+return 0;
+
+  m_supports_vRun = false;
+}
+
+// fallback to A
 StreamString packet;
 packet.PutChar('A');
 for (size_t i = 0, n = argv.size(); i < n; ++i) {

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
index fde5897f84136..376adfeb95d0f 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ b/lldb/source/Plugins/Pro

[Lldb-commits] [lldb] 3fade95 - [lldb] [gdb-remote] Support QEnvironment fallback to hex-encoded

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-09-10T14:08:36+02:00
New Revision: 3fade9542200c96021522f91ba5afdbff02729e1

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

LOG: [lldb] [gdb-remote] Support QEnvironment fallback to hex-encoded

Fall back to QEnvironmentHexEncoded if QEnvironment is not supported.
The latter packet is an LLDB extension, while the former is universally
supported.

Add tests for both QEnvironment and QEnvironmentHexEncoded packets,
including both use due to characters that need escaping and fallback
when QEnvironment is not supported.

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

Added: 


Modified: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
lldb/test/API/tools/lldb-server/main.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 3696c7096e30..ae5ef51c5b38 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -903,7 +903,6 @@ int GDBRemoteCommunicationClient::SendEnvironment(const 
Environment &env) {
 int GDBRemoteCommunicationClient::SendEnvironmentPacket(
 char const *name_equal_value) {
   if (name_equal_value && name_equal_value[0]) {
-StreamString packet;
 bool send_hex_encoding = false;
 for (const char *p = name_equal_value; *p != '\0' && !send_hex_encoding;
  ++p) {
@@ -925,33 +924,43 @@ int GDBRemoteCommunicationClient::SendEnvironmentPacket(
 }
 
 StringExtractorGDBRemote response;
-if (send_hex_encoding) {
-  if (m_supports_QEnvironmentHexEncoded) {
-packet.PutCString("QEnvironmentHexEncoded:");
-packet.PutBytesAsRawHex8(name_equal_value, strlen(name_equal_value));
-if (SendPacketAndWaitForResponse(packet.GetString(), response) ==
-PacketResult::Success) {
-  if (response.IsOKResponse())
-return 0;
-  uint8_t error = response.GetError();
-  if (error)
-return error;
-  if (response.IsUnsupportedResponse())
-m_supports_QEnvironmentHexEncoded = false;
-}
+// Prefer sending unencoded, if possible and the server supports it.
+if (!send_hex_encoding && m_supports_QEnvironment) {
+  StreamString packet;
+  packet.Printf("QEnvironment:%s", name_equal_value);
+  if (SendPacketAndWaitForResponse(packet.GetString(), response) !=
+  PacketResult::Success)
+return -1;
+
+  if (response.IsOKResponse())
+return 0;
+  if (response.IsUnsupportedResponse())
+m_supports_QEnvironment = false;
+  else {
+uint8_t error = response.GetError();
+if (error)
+  return error;
+return -1;
   }
+}
 
-} else if (m_supports_QEnvironment) {
-  packet.Printf("QEnvironment:%s", name_equal_value);
-  if (SendPacketAndWaitForResponse(packet.GetString(), response) ==
-  PacketResult::Success) {
-if (response.IsOKResponse())
-  return 0;
+if (m_supports_QEnvironmentHexEncoded) {
+  StreamString packet;
+  packet.PutCString("QEnvironmentHexEncoded:");
+  packet.PutBytesAsRawHex8(name_equal_value, strlen(name_equal_value));
+  if (SendPacketAndWaitForResponse(packet.GetString(), response) !=
+  PacketResult::Success)
+return -1;
+
+  if (response.IsOKResponse())
+return 0;
+  if (response.IsUnsupportedResponse())
+m_supports_QEnvironmentHexEncoded = false;
+  else {
 uint8_t error = response.GetError();
 if (error)
   return error;
-if (response.IsUnsupportedResponse())
-  m_supports_QEnvironment = false;
+return -1;
   }
 }
   }

diff  --git 
a/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py 
b/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
index f5b59c689020..049ea6f42057 100644
--- a/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
@@ -258,3 +258,96 @@ def A(self, packet):
 self.assertPacketLogContains([
   "vRun;%s;61726731;61726732;61726733" % (exe_hex,)
 ])
+
+def test_launch_QEnvironment(self):
+class MyResponder(MockGDBServerResponder):
+def qC(self):
+return "E42"
+
+def qfTh

[Lldb-commits] [PATCH] D107809: [lldb] Add new commands and tests for getting file perms & exists

[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 rGdbb0c14d2729: [lldb] Add new commands and tests for getting 
file perms & exists (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107809

Files:
  lldb/source/Commands/CommandObjectPlatform.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
  lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py

Index: lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
@@ -176,6 +176,23 @@
 context = self.expect_gdbremote_sequence()
 self.assertEqual(int(context["mode"], 16), test_mode)
 
+@skipIfWindows
+@add_test_categories(["llgs"])
+def test_platform_file_mode_fail(self):
+server = self.connect_to_debug_monitor()
+self.assertIsNotNone(server)
+
+temp_path = self.getBuildArtifact("nonexist")
+
+self.do_handshake()
+self.test_sequence.add_log_lines(
+["read packet: $vFile:mode:%s#00" % (
+binascii.b2a_hex(temp_path.encode()).decode(),),
+ {"direction": "send",
+ "regex": r"^\$F-1,0*2+#[0-9a-fA-F]{2}$"}],
+True)
+self.expect_gdbremote_sequence()
+
 @skipIfWindows
 @add_test_categories(["llgs"])
 def test_platform_file_exists(self):
Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
@@ -100,3 +100,48 @@
 "vFile:fstat:5",
 "vFile:close:5",
 ])
+
+def test_file_permissions(self):
+"""Test 'platform get-permissions'"""
+
+class Responder(MockGDBServerResponder):
+def vFile(self, packet):
+return "F1a4"
+
+self.server.responder = Responder()
+
+self.match("platform get-permissions /some/file.txt",
+   [r"File permissions of /some/file\.txt \(remote\): 0o0644"])
+self.assertPacketLogContains([
+"vFile:mode:2f736f6d652f66696c652e747874",
+])
+
+def test_file_exists(self):
+"""Test 'platform file-exists'"""
+
+class Responder(MockGDBServerResponder):
+def vFile(self, packet):
+return "F,1"
+
+self.server.responder = Responder()
+
+self.match("platform file-exists /some/file.txt",
+   [r"File /some/file\.txt \(remote\) exists"])
+self.assertPacketLogContains([
+"vFile:exists:2f736f6d652f66696c652e747874",
+])
+
+def test_file_exists_not(self):
+"""Test 'platform file-exists' with non-existing file"""
+
+class Responder(MockGDBServerResponder):
+def vFile(self, packet):
+return "F,0"
+
+self.server.responder = Responder()
+
+self.match("platform file-exists /some/file.txt",
+   [r"File /some/file\.txt \(remote\) does not exist"])
+self.assertPacketLogContains([
+"vFile:exists:2f736f6d652f66696c652e747874",
+])
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -661,9 +661,10 @@
 std::error_code ec;
 const uint32_t mode = FileSystem::Instance().GetPermissions(file_spec, ec);
 StreamString response;
-response.Printf("F%x", mode);
-if (mode == 0 || ec)
-  response.Printf(",%x", (int)Status(ec).GetError());
+if (mode != llvm::sys::fs::perms_not_known)
+  response.Printf("F%x", mode);
+else
+  response.Printf("F-1,%x", (int)Status(ec).GetError());
 return SendPacketNoLock(response.GetString());
   }
   return SendErrorResponse(23);
Index: lldb/source/Commands/CommandObjectPlatform.cpp
===
--- lldb/source/Commands/CommandObjectPlatform.cpp
+++ lldb/source/Commands/CommandObjectPlatform.cpp
@@ -926,6 +926,141 @@
   }
 };
 
+// "platform get-permissions remote-file-path"
+class CommandObjectPlatformGetPermissions : public CommandObjectParsed {
+public:
+  CommandObjectPlatformGetPermissions(Comman

[Lldb-commits] [lldb] 3d3017d - [lldb] [gdb-remote] Use standardized GDB errno values

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-09-10T14:08:36+02:00
New Revision: 3d3017d344f6514bbb2e5ff49a335d36e31faf55

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

LOG: [lldb] [gdb-remote] Use standardized GDB errno values

GDB uses normalized errno values for vFile errors.  Implement
the translation between them and system errno values in the gdb-remote
plugin.

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

Added: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteErrno.def

Modified: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py

Removed: 




diff  --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
index 967dc82f0270..b9a9fc1d5894 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
@@ -71,6 +71,12 @@ struct GDBRemoteFStatData {
 static_assert(sizeof(GDBRemoteFStatData) == 64,
   "size of GDBRemoteFStatData is not 64");
 
+enum GDBErrno {
+#define HANDLE_ERRNO(name, value) GDB_##name = value,
+#include "Plugins/Process/gdb-remote/GDBRemoteErrno.def"
+  GDB_EUNKNOWN = 
+};
+
 class ProcessGDBRemote;
 
 class GDBRemoteCommunication : public Communication {

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index ae5ef51c5b38..829fb328a130 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -3062,6 +3062,17 @@ GDBRemoteCommunicationClient::SetFilePermissions(const 
FileSpec &file_spec,
   return Status(response.GetHexMaxU32(false, UINT32_MAX), eErrorTypePOSIX);
 }
 
+static int gdb_errno_to_system(int err) {
+  switch (err) {
+#define HANDLE_ERRNO(name, value)  
\
+  case GDB_##name: 
\
+return name;
+#include "Plugins/Process/gdb-remote/GDBRemoteErrno.def"
+  default:
+return -1;
+  }
+}
+
 static uint64_t ParseHostIOPacketResponse(StringExtractorGDBRemote &response,
   uint64_t fail_result, Status &error) 
{
   response.SetFilePos(0);
@@ -3071,8 +3082,8 @@ static uint64_t 
ParseHostIOPacketResponse(StringExtractorGDBRemote &response,
   if (result == -2)
 return fail_result;
   if (response.GetChar() == ',') {
-int result_errno = response.GetS32(-2, 16);
-if (result_errno != -2)
+int result_errno = gdb_errno_to_system(response.GetS32(-1, 16));
+if (result_errno != -1)
   error.SetError(result_errno, eErrorTypePOSIX);
 else
   error.SetError(-1, eErrorTypeGeneric);
@@ -3225,7 +3236,7 @@ GDBRemoteCommunicationClient::GetFilePermissions(const 
FileSpec &file_spec,
 const uint32_t mode = response.GetS32(-1, 16);
 if (static_cast(mode) == -1) {
   if (response.GetChar() == ',') {
-int response_errno = response.GetS32(-1, 16);
+int response_errno = gdb_errno_to_system(response.GetS32(-1, 16));
 if (response_errno > 0)
   error.SetError(response_errno, lldb::eErrorTypePOSIX);
 else
@@ -3266,7 +3277,7 @@ uint64_t 
GDBRemoteCommunicationClient::ReadFile(lldb::user_id_t fd,
 if (retcode == -1) {
   error.SetErrorToGenericError();
   if (response.GetChar() == ',') {
-int response_errno = response.GetS32(-1, 16);
+int response_errno = gdb_errno_to_system(response.GetS32(-1, 16));
 if (response_errno > 0)
   error.SetError(response_errno, lldb::eErrorTypePOSIX);
   }
@@ -3309,7 +3320,7 @@ uint64_t 
GDBRemoteCommunicationClient::WriteFile(lldb::user_id_t fd,
 if (bytes_written == -1) {
   error.SetErrorToGenericError();
   if (response.GetChar() == ',') {
-int response_errno = response.GetS32(-1, 16);
+int response_errno = gdb_errno_to_system(response.GetS32(-1, 16));
 if (response_errno > 0)
   error.SetError(response_errno, lldb::eErrorTypePOSIX);
   }
@@ -3341,7 +3352,7 @@ Status GDBRemoteCommunicationClient::CreateSymlink(const 
FileSpec &src,
   if (result != 0) {
 error.SetErrorToGenericError();
 if (response.GetChar() == ',') {
-  int response_errno = response.GetS32(-1, 16);
+  int response_errno = gdb_errno_to_syste

[Lldb-commits] [PATCH] D107811: [lldb] [gdb-remote] Add fallbacks for vFile:mode and vFile:exists

[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 rG501eaf88770d: [lldb] [gdb-remote] Add fallbacks for 
vFile:mode and vFile:exists (authored by mgorny).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D107811?vs=371842&id=371889#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107811

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py

Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
@@ -116,6 +116,33 @@
 "vFile:mode:2f736f6d652f66696c652e747874",
 ])
 
+def test_file_permissions_fallback(self):
+"""Test 'platform get-permissions' fallback to fstat"""
+
+class Responder(MockGDBServerResponder):
+def vFile(self, packet):
+if packet.startswith("vFile:open:"):
+return "F5"
+elif packet.startswith("vFile:fstat:"):
+return "F40;" + 8 * "\0" + "\0\0\1\xA4" + 52 * "\0"
+if packet.startswith("vFile:close:"):
+return "F0"
+return ""
+
+self.server.responder = Responder()
+
+try:
+self.match("platform get-permissions /some/file.txt",
+   [r"File permissions of /some/file\.txt \(remote\): 0o0644"])
+self.assertPacketLogContains([
+"vFile:mode:2f736f6d652f66696c652e747874",
+"vFile:open:2f736f6d652f66696c652e747874,,",
+"vFile:fstat:5",
+"vFile:close:5",
+])
+finally:
+self.dbg.GetSelectedPlatform().DisconnectRemote()
+
 def test_file_exists(self):
 """Test 'platform file-exists'"""
 
@@ -145,3 +172,42 @@
 self.assertPacketLogContains([
 "vFile:exists:2f736f6d652f66696c652e747874",
 ])
+
+def test_file_exists_fallback(self):
+"""Test 'platform file-exists' fallback to open"""
+
+class Responder(MockGDBServerResponder):
+def vFile(self, packet):
+if packet.startswith("vFile:open:"):
+return "F5"
+if packet.startswith("vFile:close:"):
+return "F0"
+return ""
+
+self.server.responder = Responder()
+
+self.match("platform file-exists /some/file.txt",
+   [r"File /some/file\.txt \(remote\) exists"])
+self.assertPacketLogContains([
+"vFile:exists:2f736f6d652f66696c652e747874",
+"vFile:open:2f736f6d652f66696c652e747874,,",
+"vFile:close:5",
+])
+
+def test_file_exists_not_fallback(self):
+"""Test 'platform file-exists' fallback to open with non-existing file"""
+
+class Responder(MockGDBServerResponder):
+def vFile(self, packet):
+if packet.startswith("vFile:open:"):
+return "F-1,2"
+return ""
+
+self.server.responder = Responder()
+
+self.match("platform file-exists /some/file.txt",
+   [r"File /some/file\.txt \(remote\) does not exist"])
+self.assertPacketLogContains([
+"vFile:exists:2f736f6d652f66696c652e747874",
+"vFile:open:2f736f6d652f66696c652e747874,,",
+])
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -587,7 +587,8 @@
   m_supports_QEnvironment : 1, m_supports_QEnvironmentHexEncoded : 1,
   m_supports_qSymbol : 1, m_qSymbol_requests_done : 1,
   m_supports_qModuleInfo : 1, m_supports_jThreadsInfo : 1,
-  m_supports_jModulesInfo : 1, m_supports_vFileSize : 1;
+  m_supports_jModulesInfo : 1, m_supports_vFileSize : 1,
+  m_supports_vFileMode : 1, m_supports_vFileExists : 1;
 
   /// Current gdb remote protocol process identifier for all other operations
   lldb::pid_t m_curr_pid = LLDB_INVALID_PROCESS_ID;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicat

[Lldb-commits] [PATCH] D107931: [lldb] [gdb-remote] Implement vRun packet

[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 rG6ba3f7237dc7: [lldb] [gdb-remote] Implement the vRun packet 
(authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107931

Files:
  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
  lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
  lldb/test/API/tools/lldb-server/TestLldbGdbServer.py

Index: lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
===
--- lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
+++ lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
@@ -10,6 +10,9 @@
 the initial set of tests implemented.
 """
 
+import binascii
+import itertools
+
 import unittest2
 import gdbremote_testcase
 import lldbgdbserverutils
@@ -1246,3 +1249,61 @@
 # Make sure we read back what we wrote.
 self.assertEqual(read_value, expected_reg_values[thread_index])
 thread_index += 1
+
+def test_launch_via_A(self):
+self.build()
+exe_path = self.getBuildArtifact("a.out")
+args = [exe_path, "stderr:arg1", "stderr:arg2", "stderr:arg3"]
+hex_args = [binascii.b2a_hex(x.encode()).decode() for x in args]
+
+server = self.connect_to_debug_monitor()
+self.assertIsNotNone(server)
+self.do_handshake()
+# NB: strictly speaking we should use %x here but this packet
+# is deprecated, so no point in changing lldb-server's expectations
+self.test_sequence.add_log_lines(
+["read packet: $A %d,0,%s,%d,1,%s,%d,2,%s,%d,3,%s#00" %
+ tuple(itertools.chain.from_iterable(
+ [(len(x), x) for x in hex_args])),
+ "send packet: $OK#00",
+ "read packet: $c#00",
+ "send packet: $W00#00"],
+True)
+context = self.expect_gdbremote_sequence()
+self.assertEqual(context["O_content"],
+ b'arg1\r\narg2\r\narg3\r\n')
+
+def test_launch_via_vRun(self):
+self.build()
+exe_path = self.getBuildArtifact("a.out")
+args = [exe_path, "stderr:arg1", "stderr:arg2", "stderr:arg3"]
+hex_args = [binascii.b2a_hex(x.encode()).decode() for x in args]
+
+server = self.connect_to_debug_monitor()
+self.assertIsNotNone(server)
+self.do_handshake()
+self.test_sequence.add_log_lines(
+["read packet: $vRun;%s;%s;%s;%s#00" % tuple(hex_args),
+ {"direction": "send",
+  "regex": r"^\$T([0-9a-fA-F]+)"},
+ "read packet: $c#00",
+ "send packet: $W00#00"],
+True)
+context = self.expect_gdbremote_sequence()
+
+def test_launch_via_vRun_no_args(self):
+self.build()
+exe_path = self.getBuildArtifact("a.out")
+hex_path = binascii.b2a_hex(exe_path.encode()).decode()
+
+server = self.connect_to_debug_monitor()
+self.assertIsNotNone(server)
+self.do_handshake()
+self.test_sequence.add_log_lines(
+["read packet: $vRun;%s#00" % (hex_path,),
+ {"direction": "send",
+  "regex": r"^\$T([0-9a-fA-F]+)"},
+ "read packet: $c#00",
+ "send packet: $W00#00"],
+True)
+self.expect_gdbremote_sequence()
Index: lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
===
--- lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
+++ lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
@@ -190,6 +190,10 @@
 return self.qPathComplete()
 if packet.startswith("vFile:"):
 return self.vFile(packet)
+if packet.startswith("vRun;"):
+return self.vRun(packet)
+if packet.startswith("qLaunchSuccess"):
+return self.qLaunchSuccess()
 
 return self.other(packet)
 
@@ -303,6 +307,12 @@
 def vFile(self, packet):
 return ""
 
+def vRun(self, packet):
+return ""
+
+def qLaunchSuccess(self):
+return ""
+
 """
 Raised when we receive a packet for which there is no default action.
 Override the responder class to implement behavior suitable for the test at
Index: lldb/test/API/functionalities/gdb_remote_cli

[Lldb-commits] [PATCH] D108148: [lldb] [gdb-remote] Use standardized GDB errno values

[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 rG3d3017d344f6: [lldb] [gdb-remote] Use standardized GDB errno 
values (authored by mgorny).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D108148?vs=371351&id=371892#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108148

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteErrno.def
  lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py

Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
@@ -15,7 +15,7 @@
 return "Fa"
 elif packet.startswith("vFile:close:"):
 return "F0"
-return "F-1,16"
+return "F-1,58"
 
 self.server.responder = Responder()
 
@@ -39,21 +39,23 @@
 
 class Responder(MockGDBServerResponder):
 def vFile(self, packet):
-return "F-1,16"
+# use ENOSYS as this constant differs between GDB Remote
+# Protocol and Linux, so we can test the translation
+return "F-1,58"
 
 self.server.responder = Responder()
 
 self.match("platform file open /some/file.txt -v 0755",
-   [r"error: Invalid argument"],
+   [r"error: Function not implemented"],
error=True)
 self.match("platform file read 16 -o 11 -c 13",
-   [r"error: Invalid argument"],
+   [r"error: Function not implemented"],
error=True)
 self.match("platform file write 16 -o 11 -d teststring",
-   [r"error: Invalid argument"],
+   [r"error: Function not implemented"],
error=True)
 self.match("platform file close 16",
-   [r"error: Invalid argument"],
+   [r"error: Function not implemented"],
error=True)
 self.assertPacketLogContains([
 "vFile:open:2f736f6d652f66696c652e747874,0202,01ed",
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteErrno.def
===
--- /dev/null
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteErrno.def
@@ -0,0 +1,39 @@
+//===-- GDBRemoteErrno.def --*- 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
+//
+//===--===//
+
+// NOTE: NO INCLUDE GUARD DESIRED!
+
+// HANDLE_ERRNO(name, value)
+#ifndef HANDLE_ERRNO
+#error "HANDLE_ERRNO must be defined"
+#endif
+
+// from gdb's include/gdb/fileio.h
+HANDLE_ERRNO(EPERM, 1)
+HANDLE_ERRNO(ENOENT,2)
+HANDLE_ERRNO(EINTR, 4)
+HANDLE_ERRNO(EIO,   5)
+HANDLE_ERRNO(EBADF, 9)
+HANDLE_ERRNO(EACCES,   13)
+HANDLE_ERRNO(EFAULT,   14)
+HANDLE_ERRNO(EBUSY,16)
+HANDLE_ERRNO(EEXIST,   17)
+HANDLE_ERRNO(ENODEV,   19)
+HANDLE_ERRNO(ENOTDIR,  20)
+HANDLE_ERRNO(EISDIR,   21)
+HANDLE_ERRNO(EINVAL,   22)
+HANDLE_ERRNO(ENFILE,   23)
+HANDLE_ERRNO(EMFILE,   24)
+HANDLE_ERRNO(EFBIG,27)
+HANDLE_ERRNO(ENOSPC,   28)
+HANDLE_ERRNO(ESPIPE,   29)
+HANDLE_ERRNO(EROFS,30)
+HANDLE_ERRNO(ENOSYS,   88)
+HANDLE_ERRNO(ENAMETOOLONG, 91)
+
+#undef HANDLE_ERRNO
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -534,6 +534,17 @@
   return SendErrorResponse(18);
 }
 
+static GDBErrno system_errno_to_gdb(int err) {
+  switch (err) {
+#define HANDLE_ERRNO(name, value)  \
+  case name:   \
+return GDB_##name;
+#include "Plugins/Process/gdb-remote/GDBRemoteErrno.def"
+  default:
+return GDB_EUNKNOWN;
+  }

[Lldb-commits] [PATCH] D108018: [lldb] [gdb-remote] Support QEnvironment fallback to hex-encoded

[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 rG3fade9542200: [lldb] [gdb-remote] Support QEnvironment 
fallback to hex-encoded (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108018

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
  lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
  lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
  lldb/test/API/tools/lldb-server/main.cpp

Index: lldb/test/API/tools/lldb-server/main.cpp
===
--- lldb/test/API/tools/lldb-server/main.cpp
+++ lldb/test/API/tools/lldb-server/main.cpp
@@ -326,6 +326,10 @@
   g_threads_do_segfault = true;
 } else if (consume_front(arg, "print-pid")) {
   print_pid();
+} else if (consume_front(arg, "print-env:")) {
+  // Print the value of specified envvar to stdout.
+  const char *value = getenv(arg.c_str());
+  printf("%s\n", value ? value : "__unset__");
 } else {
   // Treat the argument as text for stdout.
   printf("%s\n", argv[i]);
Index: lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
===
--- lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
+++ lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
@@ -1307,3 +1307,56 @@
  "send packet: $W00#00"],
 True)
 self.expect_gdbremote_sequence()
+
+def test_QEnvironment(self):
+self.build()
+exe_path = self.getBuildArtifact("a.out")
+env = {"FOO": "test", "BAR": "a=z"}
+args = [exe_path, "print-env:FOO", "print-env:BAR"]
+hex_args = [binascii.b2a_hex(x.encode()).decode() for x in args]
+
+server = self.connect_to_debug_monitor()
+self.assertIsNotNone(server)
+self.do_handshake()
+
+for key, value in env.items():
+self.test_sequence.add_log_lines(
+["read packet: $QEnvironment:%s=%s#00" % (key, value),
+ "send packet: $OK#00"],
+True)
+self.test_sequence.add_log_lines(
+["read packet: $vRun;%s#00" % (";".join(hex_args),),
+ {"direction": "send",
+  "regex": r"^\$T([0-9a-fA-F]+)"},
+ "read packet: $c#00",
+ "send packet: $W00#00"],
+True)
+context = self.expect_gdbremote_sequence()
+self.assertEqual(context["O_content"], b"test\r\na=z\r\n")
+
+def test_QEnvironmentHexEncoded(self):
+self.build()
+exe_path = self.getBuildArtifact("a.out")
+env = {"FOO": "test", "BAR": "a=z", "BAZ": "a*}#z"}
+args = [exe_path, "print-env:FOO", "print-env:BAR", "print-env:BAZ"]
+hex_args = [binascii.b2a_hex(x.encode()).decode() for x in args]
+
+server = self.connect_to_debug_monitor()
+self.assertIsNotNone(server)
+self.do_handshake()
+
+for key, value in env.items():
+hex_enc = binascii.b2a_hex(("%s=%s" % (key, value)).encode()).decode()
+self.test_sequence.add_log_lines(
+["read packet: $QEnvironmentHexEncoded:%s#00" % (hex_enc,),
+ "send packet: $OK#00"],
+True)
+self.test_sequence.add_log_lines(
+["read packet: $vRun;%s#00" % (";".join(hex_args),),
+ {"direction": "send",
+  "regex": r"^\$T([0-9a-fA-F]+)"},
+ "read packet: $c#00",
+ "send packet: $W00#00"],
+True)
+context = self.expect_gdbremote_sequence()
+self.assertEqual(context["O_content"], b"test\r\na=z\r\na*}#z\r\n")
Index: lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
===
--- lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
+++ lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py
@@ -194,6 +194,10 @@
 return self.vRun(packet)
 if packet.startswith("qLaunchSuccess"):
 return self.qLaunchSuccess()
+if packet.startswith("QEnvironment:"):
+return self.QEnvironment(packet)
+if packet.startswith("QEnvironmentHexEncoded:"):
+return self.QEnvironmentHexEncoded(packet)
 
 return self.other(packet)
 
@@ -313,6 +317,12 @@
 def qLaunchSuccess(self):
 return ""
 
+def QEnvironment(self, packet):
+return "OK"
+
+def QEnvironmentHexEncoded(self, packet):
+return "OK"
+
 """
 Raised when we receive a packet for which there is no default action.
 Override the responder class to implement behavior suitable

[Lldb-commits] [PATCH] D108554: [lldb] Support querying registers via generic names without alt_names

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

These are pretty much the SB equivalents of register read, and they should keep 
working


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

https://reviews.llvm.org/D108554

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

[Lldb-commits] [lldb] beb768f - [lldb] Clean up Platform/CMakeLists.txt

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2021-09-10T14:18:41+02:00
New Revision: beb768f40b47e23e05766738edc0e7723e2f98d4

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

LOG: [lldb] Clean up Platform/CMakeLists.txt

Remove comments looking like preprocessor directives (thankfully cmake
does not have those yet), and sort the file.

Added: 


Modified: 
lldb/source/Plugins/Platform/CMakeLists.txt

Removed: 




diff  --git a/lldb/source/Plugins/Platform/CMakeLists.txt 
b/lldb/source/Plugins/Platform/CMakeLists.txt
index 5f284e517dcac..7d1e095311cbd 100644
--- a/lldb/source/Plugins/Platform/CMakeLists.txt
+++ b/lldb/source/Plugins/Platform/CMakeLists.txt
@@ -1,17 +1,9 @@
-#if (CMAKE_SYSTEM_NAME MATCHES "Linux")
-  add_subdirectory(Linux)
-#elseif (CMAKE_SYSTEM_NAME MATCHES "FreeBSD")
-  add_subdirectory(FreeBSD)
-#elseif (CMAKE_SYSTEM_NAME MATCHES "NetBSD")
-  add_subdirectory(NetBSD)
-#elseif (CMAKE_SYSTEM_NAME MATCHES "OpenBSD")
-  add_subdirectory(OpenBSD)
-#elseif (CMAKE_SYSTEM_NAME MATCHES "Darwin")
-  add_subdirectory(MacOSX)
-#elseif (CMAKE_SYSTEM_NAME MATCHES "Windows")
-  add_subdirectory(Windows)
-#endif()
-
-add_subdirectory(POSIX)
-add_subdirectory(gdb-server)
 add_subdirectory(Android)
+add_subdirectory(FreeBSD)
+add_subdirectory(gdb-server)
+add_subdirectory(Linux)
+add_subdirectory(MacOSX)
+add_subdirectory(NetBSD)
+add_subdirectory(OpenBSD)
+add_subdirectory(POSIX)
+add_subdirectory(Windows)



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

[Lldb-commits] [PATCH] D109600: [lldb] Remove PluginInterface::GetPluginVersion

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: teemperor, JDevlieghere.
Herald added subscribers: atanasyan, jrtc27, kbarton, sbc100, nemanjai, 
sdardis, emaste.
labath requested review of this revision.
Herald added subscribers: MaskRay, aheejin.
Herald added a project: LLDB.

In all these years, we haven't found a use for this function (it has
zero callers). Lets just remove the boilerplate.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D109600

Files:
  lldb/include/lldb/Core/PluginInterface.h
  lldb/include/lldb/Symbol/SymbolVendor.h
  lldb/include/lldb/Target/ProcessTrace.h
  lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
  lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.h
  lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
  lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.h
  lldb/source/Plugins/ABI/ARC/ABISysV_arc.cpp
  lldb/source/Plugins/ABI/ARC/ABISysV_arc.h
  lldb/source/Plugins/ABI/ARM/ABIMacOSX_arm.cpp
  lldb/source/Plugins/ABI/ARM/ABIMacOSX_arm.h
  lldb/source/Plugins/ABI/ARM/ABISysV_arm.cpp
  lldb/source/Plugins/ABI/ARM/ABISysV_arm.h
  lldb/source/Plugins/ABI/Hexagon/ABISysV_hexagon.cpp
  lldb/source/Plugins/ABI/Hexagon/ABISysV_hexagon.h
  lldb/source/Plugins/ABI/Mips/ABISysV_mips.cpp
  lldb/source/Plugins/ABI/Mips/ABISysV_mips.h
  lldb/source/Plugins/ABI/Mips/ABISysV_mips64.cpp
  lldb/source/Plugins/ABI/Mips/ABISysV_mips64.h
  lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc.cpp
  lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc.h
  lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc64.cpp
  lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc64.h
  lldb/source/Plugins/ABI/SystemZ/ABISysV_s390x.cpp
  lldb/source/Plugins/ABI/SystemZ/ABISysV_s390x.h
  lldb/source/Plugins/ABI/X86/ABIMacOSX_i386.cpp
  lldb/source/Plugins/ABI/X86/ABIMacOSX_i386.h
  lldb/source/Plugins/ABI/X86/ABISysV_i386.h
  lldb/source/Plugins/ABI/X86/ABISysV_x86_64.cpp
  lldb/source/Plugins/ABI/X86/ABISysV_x86_64.h
  lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.cpp
  lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.h
  lldb/source/Plugins/Architecture/AArch64/ArchitectureAArch64.cpp
  lldb/source/Plugins/Architecture/AArch64/ArchitectureAArch64.h
  lldb/source/Plugins/Architecture/Arm/ArchitectureArm.cpp
  lldb/source/Plugins/Architecture/Arm/ArchitectureArm.h
  lldb/source/Plugins/Architecture/Mips/ArchitectureMips.cpp
  lldb/source/Plugins/Architecture/Mips/ArchitectureMips.h
  lldb/source/Plugins/Architecture/PPC64/ArchitecturePPC64.cpp
  lldb/source/Plugins/Architecture/PPC64/ArchitecturePPC64.h
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.h
  lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
  lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.h
  lldb/source/Plugins/DynamicLoader/Hexagon-DYLD/DynamicLoaderHexagonDYLD.cpp
  lldb/source/Plugins/DynamicLoader/Hexagon-DYLD/DynamicLoaderHexagonDYLD.h
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.h
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
  lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.h
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
  lldb/source/Plugins/DynamicLoader/Static/DynamicLoaderStatic.cpp
  lldb/source/Plugins/DynamicLoader/Static/DynamicLoaderStatic.h
  lldb/source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
  lldb/source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.h
  lldb/source/Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.h
  lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.h
  lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.h
  lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.h
  lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.h
  lldb/source/Plugins/Instruction/PPC64/EmulateInstructionPPC64.h
  lldb/source/Plugins/InstrumentationRuntime/ASan/InstrumentationRuntimeASan.h
  
lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.h
  lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.h
  lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.h
  lldb/source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp
  lldb/source/Plugins/JITLoader/GDB/JITLoaderGDB.h
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
  lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
  lldb/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h
  
lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp
  
lldb/source/P

[Lldb-commits] [lldb] 9a4379c - [lldb] [test] Skip file permission tests on Windows

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-09-10T16:23:42+02:00
New Revision: 9a4379c3dcab8f7d99975eb6741fe17d1788e461

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

LOG: [lldb] [test] Skip file permission tests on Windows

Added: 


Modified: 
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py 
b/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
index 0e242fc2d1c0..cc38e1775bbe 100644
--- 
a/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
+++ 
b/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
@@ -1,5 +1,7 @@
 from gdbclientutils import *
 
+from lldbsuite.test.decorators import *
+
 class TestGDBRemotePlatformFile(GDBPlatformClientTestBase):
 
 def test_file(self):
@@ -103,6 +105,7 @@ def vFile(self, packet):
 "vFile:close:5",
 ])
 
+@skipIfWindows
 def test_file_permissions(self):
 """Test 'platform get-permissions'"""
 
@@ -118,6 +121,7 @@ def vFile(self, packet):
 "vFile:mode:2f736f6d652f66696c652e747874",
 ])
 
+@skipIfWindows
 def test_file_permissions_fallback(self):
 """Test 'platform get-permissions' fallback to fstat"""
 



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

[Lldb-commits] [lldb] c362f61 - [lldb] [test] Mark new launch/QEnvironment tests as llgs category

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-09-10T16:23:43+02:00
New Revision: c362f610f8c08fc53bdf6aea6e990b26fe63b097

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

LOG: [lldb] [test] Mark new launch/QEnvironment tests as llgs category

Added: 


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

Removed: 




diff  --git a/lldb/test/API/tools/lldb-server/TestLldbGdbServer.py 
b/lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
index aac73dd53998..9b901756c093 100644
--- a/lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
+++ b/lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
@@ -1250,6 +1250,7 @@ def test_P_and_p_thread_suffix_work(self):
 self.assertEqual(read_value, expected_reg_values[thread_index])
 thread_index += 1
 
+@add_test_categories(["llgs"])
 def test_launch_via_A(self):
 self.build()
 exe_path = self.getBuildArtifact("a.out")
@@ -1273,6 +1274,7 @@ def test_launch_via_A(self):
 self.assertEqual(context["O_content"],
  b'arg1\r\narg2\r\narg3\r\n')
 
+@add_test_categories(["llgs"])
 def test_launch_via_vRun(self):
 self.build()
 exe_path = self.getBuildArtifact("a.out")
@@ -1291,6 +1293,7 @@ def test_launch_via_vRun(self):
 True)
 context = self.expect_gdbremote_sequence()
 
+@add_test_categories(["llgs"])
 def test_launch_via_vRun_no_args(self):
 self.build()
 exe_path = self.getBuildArtifact("a.out")
@@ -1308,6 +1311,7 @@ def test_launch_via_vRun_no_args(self):
 True)
 self.expect_gdbremote_sequence()
 
+@add_test_categories(["llgs"])
 def test_QEnvironment(self):
 self.build()
 exe_path = self.getBuildArtifact("a.out")
@@ -1334,6 +1338,7 @@ def test_QEnvironment(self):
 context = self.expect_gdbremote_sequence()
 self.assertEqual(context["O_content"], b"test\r\na=z\r\n")
 
+@add_test_categories(["llgs"])
 def test_QEnvironmentHexEncoded(self):
 self.build()
 exe_path = self.getBuildArtifact("a.out")



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

[Lldb-commits] [lldb] 784281d - [lldb] [test] Attempt to fix gdb_remote_client A/vRun tests on Windows

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-09-10T16:27:29+02:00
New Revision: 784281d3164862cb5c7f8f20de80f3d52cc13307

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

LOG: [lldb] [test] Attempt to fix gdb_remote_client A/vRun tests on Windows

Added: 


Modified: 
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py 
b/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
index 049ea6f42057..a58de7f19ec6 100644
--- a/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
@@ -1,5 +1,6 @@
 import lldb
 import binascii
+import os.path
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test.decorators import *
 from gdbclientutils import *
@@ -181,7 +182,8 @@ def qLaunchSuccess(self):
 self.server.responder = MyResponder()
 
 target = self.createTarget("a.yaml")
-exe_path = self.getBuildArtifact("a")
+# NB: apparently GDB packets are using "/" on Windows too
+exe_path = self.getBuildArtifact("a").replace(os.path.sep, '/')
 exe_hex = binascii.b2a_hex(exe_path.encode()).decode()
 process = self.connect(target)
 lldbutil.expect_state_changes(self, self.dbg.GetListener(), process,
@@ -236,7 +238,8 @@ def A(self, packet):
 self.server.responder = MyResponder()
 
 target = self.createTarget("a.yaml")
-exe_path = self.getBuildArtifact("a")
+# NB: apparently GDB packets are using "/" on Windows too
+exe_path = self.getBuildArtifact("a").replace(os.path.sep, '/')
 exe_hex = binascii.b2a_hex(exe_path.encode()).decode()
 process = self.connect(target)
 lldbutil.expect_state_changes(self, self.dbg.GetListener(), process,



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

[Lldb-commits] [lldb] d727bd6 - [lldb] [test] Skip A/vRun/QEnvironment* tests on Windows, and fix them

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-09-10T16:34:20+02:00
New Revision: d727bd696293a5e8ba63c01b6ec80b2b17eb373a

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

LOG: [lldb] [test] Skip A/vRun/QEnvironment* tests on Windows, and fix them

Skip A/vRun/QEnvironment* tests on Windows as testing for output is
known not to work there.  Add a missing output check to the vRun test.

Added: 


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

Removed: 




diff  --git a/lldb/test/API/tools/lldb-server/TestLldbGdbServer.py 
b/lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
index 9b901756c093..2f76654c47b1 100644
--- a/lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
+++ b/lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
@@ -1250,6 +1250,7 @@ def test_P_and_p_thread_suffix_work(self):
 self.assertEqual(read_value, expected_reg_values[thread_index])
 thread_index += 1
 
+@skipIfWindows # No pty support to test any inferior output
 @add_test_categories(["llgs"])
 def test_launch_via_A(self):
 self.build()
@@ -1274,6 +1275,7 @@ def test_launch_via_A(self):
 self.assertEqual(context["O_content"],
  b'arg1\r\narg2\r\narg3\r\n')
 
+@skipIfWindows # No pty support to test any inferior output
 @add_test_categories(["llgs"])
 def test_launch_via_vRun(self):
 self.build()
@@ -1292,6 +1294,8 @@ def test_launch_via_vRun(self):
  "send packet: $W00#00"],
 True)
 context = self.expect_gdbremote_sequence()
+self.assertEqual(context["O_content"],
+ b'arg1\r\narg2\r\narg3\r\n')
 
 @add_test_categories(["llgs"])
 def test_launch_via_vRun_no_args(self):
@@ -1311,6 +1315,7 @@ def test_launch_via_vRun_no_args(self):
 True)
 self.expect_gdbremote_sequence()
 
+@skipIfWindows # No pty support to test any inferior output
 @add_test_categories(["llgs"])
 def test_QEnvironment(self):
 self.build()
@@ -1338,6 +1343,7 @@ def test_QEnvironment(self):
 context = self.expect_gdbremote_sequence()
 self.assertEqual(context["O_content"], b"test\r\na=z\r\n")
 
+@skipIfWindows # No pty support to test any inferior output
 @add_test_categories(["llgs"])
 def test_QEnvironmentHexEncoded(self):
 self.build()



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

[Lldb-commits] [PATCH] D109483: [APInt] Normalize naming on keep constructors / predicate methods.

[Chris Lattner via Phabricator via lldb-commits]

lattner added inline comments.



Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:3243
"Don't know how to expand this subtraction!");
-Tmp1 = DAG.getNode(ISD::XOR, dl, VT, Node->getOperand(1),
-   DAG.getConstant(APInt::getAllOnesValue(VT.getSizeInBits()), dl,
-   VT));
+Tmp1 = DAG.getNode(
+ISD::XOR, dl, VT, Node->getOperand(1),

craig.topper wrote:
> This could use DAG.getNOT if you're willing to make that change.
I'd prefer to keep this one separate, it isn't related to APInt.



Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:12185
 else
-  OtherOp = DAG.getConstant(APInt::getAllOnesValue(VT.getSizeInBits()), dl,
-VT);
+  OtherOp = DAG.getConstant(APInt::getAllOnes(VT.getSizeInBits()), dl, VT);
 return true;

craig.topper wrote:
> I think we have DAG.getAllOnesConstant if you want to use it
Will do this in a followup



Comment at: llvm/lib/Target/Lanai/LanaiISelLowering.cpp:1403
 else
-  OtherOp =
-  DAG.getConstant(APInt::getAllOnesValue(VT.getSizeInBits()), dl, VT);
+  OtherOp = DAG.getConstant(APInt::getAllOnes(VT.getSizeInBits()), dl, VT);
 return true;

craig.topper wrote:
> I think we have DAG.getAllOnesConstant if you want to use it
Likewise



Comment at: llvm/unittests/ADT/APIntTest.cpp:29
 TEST(APIntTest, ShiftLeftByZero) {
-  APInt One = APInt::getNullValue(65) + 1;
+  APInt One = APInt::getZero(65) + 1;
   APInt Shl = One.shl(0);

craig.topper wrote:
> That's a strange way to write APInt(64, 1) unless there was some specific bug.
I agree, assume that this was testing some former bug.  Unit tests are designed 
to be weird ;-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109483

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

[Lldb-commits] [PATCH] D109483: [APInt] Normalize naming on keep constructors / predicate methods.

[Craig Topper via Phabricator via lldb-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109483

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

[Lldb-commits] [PATCH] D109483: [APInt] Normalize naming on keep constructors / predicate methods.

[Chris Lattner via Phabricator via lldb-commits]

lattner added a comment.

Thank you for the detailed review Craig!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109483

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

[Lldb-commits] [PATCH] D109483: [APInt] Normalize naming on keep constructors / predicate methods.

[Chris Lattner via Phabricator via lldb-commits]

lattner added a comment.

I'll take care of the DAG.getAllOnesConstant change, but i'd appreciate it if 
you could look at the NOT cases.  Running tests on the DAG.getAllOnesConstant 
patch now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109483

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

[Lldb-commits] [PATCH] D109345: MemoryBuffer: Migrate to Expected/llvm::Error from ErrorOr/std::error_code

[Duncan P. N. Exon Smith via Phabricator via lldb-commits]

dexonsmith added a comment.

In D109345#2990612 , @dblaikie wrote:

> Given the amount of churn this means, though - reckon it's worth it? Reckon 
> it needs more llvm-dev thread/buy-in/etc?

I think the churn is worth since my intuition is that it has high value for 
downstreams/clients (in terms of mitigating risk/etc.). (For example, the Swift 
compiler also uses MemoryBuffer and uses `auto` quite heavily.)

Not sure if this needs more buy-in, but probably worth communicating the plan 
on llvm-dev. If nothing else, makes it easy for relevant commits to point to it 
on lists.llvm.org. Could even add a working `sed -e` or `perl` command for the 
renames?

> Any other bike-shedding for MemoryBufferCompat? (MemoryBufferDeprecated? but 
> I don't really mind)



- MemoryBufferDeprecated SGTM (more descriptive than "compat"), 
MemoryBufferLegacy would work too
- MemoryBufferErrorCodeAPI is even more descriptive -- see related idea below
- could also rename all the (relevant) functions instead of the class... but 
since these are all `static` anyway renaming the class feels easier?

Thinking the names through gave me an idea for a refined staging:

1. Add MemoryBufferErrorAPI (wrapping APIs with errorOrToExpected) and 
MemoryBufferErrorCodeAPI (alias for MemoryBuffer) in the same commit.
2. Migrate in-tree callers to MemoryBufferErrorCodeAPI (via mass rename). 
(Could even move some to MemoryBufferErrorAPI?)
3. Update MemoryBuffer to use Error/Expected APIs, change MemoryBufferErrorAPI 
to an alias of it, and leave behind MemoryBufferErrorCodeAPI (wrapping APIs 
with expectedToErrorOr).
4. One or more commits:
  1. Migrate in-tree callers to MemoryBuffer.
  2. Delete MemoryBufferErrorAPI alias.
5. Delete MemoryBufferErrorCodeAPI wrappers.

The refinement is that the new (1) is better designed for cherry-picking to a 
stable branch:

- Isolated to MemoryBuffer (no changes to callers), making conflict resolution 
trivial.
- Downstreams / clients can migrate to MemoryBufferError without integrating 
the change to the default behaviour of MemoryBuffer.
- Particularly useful for clients that want to cherry-pick/merge changes 
between a branch that builds against ToT LLVM and one that builds against a 
"stable" version that predates the transition.

The new (3) and (5) are the same as the old (2) and (4) -- isolated changes 
that are easy to revert.

(But if you're not convinced, I think my previous idea for staging would still 
be a huge help.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109345

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

[Lldb-commits] [PATCH] D109345: MemoryBuffer: Migrate to Expected/llvm::Error from ErrorOr/std::error_code

[Duncan P. N. Exon Smith via Phabricator via lldb-commits]

dexonsmith added a comment.

In D109345#2992274 , @dexonsmith 
wrote:

> 4. One or more commits:
>   1. Migrate in-tree callers to MemoryBuffer.
>   2. Delete MemoryBufferErrorAPI alias.
> 5. Delete MemoryBufferErrorCodeAPI wrappers.

(Potentially MemoryBufferErrorAPI and MemoryBufferErrorAPI could be kept across 
an LLVM release branch boundary to help LLVM clients that use those... not sure 
how useful that'd be?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109345

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

[Lldb-commits] [lldb] 4e7ac6f - [lldb] [test] Remove parent check in Subprocess/clone-follow-child-softbp.test

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-09-10T18:03:05+02:00
New Revision: 4e7ac6facad6d6c895c750a232d3786defedcc7b

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

LOG: [lldb] [test] Remove parent check in 
Subprocess/clone-follow-child-softbp.test

Hopefully this will resolve the remaining flakiness.

Added: 


Modified: 
lldb/test/Shell/Subprocess/clone-follow-child-softbp.test

Removed: 




diff  --git a/lldb/test/Shell/Subprocess/clone-follow-child-softbp.test 
b/lldb/test/Shell/Subprocess/clone-follow-child-softbp.test
index f67b5a95e9cf..4bf13c7710c9 100644
--- a/lldb/test/Shell/Subprocess/clone-follow-child-softbp.test
+++ b/lldb/test/Shell/Subprocess/clone-follow-child-softbp.test
@@ -8,7 +8,6 @@ settings set target.process.stop-on-exec false
 b child_func
 b parent_func
 process launch
-# CHECK: function run in parent
 # CHECK: stop reason = breakpoint
 # CHECK-NEXT: child_func
 continue



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

[Lldb-commits] [PATCH] D108061: [lldb] Add support for shared library load when executable called through ld.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Thanks Pavel for figuring out the XML issue. I will need to add that to my box 
of tricks when buildbots fail! I look forward to seeing this patch go in


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108061

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

[Lldb-commits] [PATCH] D109600: [lldb] Remove PluginInterface::GetPluginVersion

[Jonas Devlieghere via Phabricator via lldb-commits]

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109600

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

[Lldb-commits] [PATCH] D108554: [lldb] Support querying registers via generic names without alt_names

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

mgorny updated this revision to Diff 371941.
mgorny added a comment.

Fixed `SBFrame` stuffs. While at it, changed `ValueObjectRegister` to take 
`const RegisterInfo *` instead of the index, since it's almost always what it 
really wants.


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

https://reviews.llvm.org/D108554

Files:
  lldb/include/lldb/Core/ValueObjectRegister.h
  lldb/source/API/SBFrame.cpp
  lldb/source/Core/ValueObjectRegister.cpp
  lldb/source/Target/RegisterContext.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py

Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
@@ -9,6 +9,7 @@
 
 @skipIfXmlSupportMissing
 @skipIfRemote
+@skipIfLLVMTargetMissing("X86")
 def test_x86_64_vec_regs(self):
 """Test rendering of x86_64 vector registers from gdbserver."""
 class MyResponder(MockGDBServerResponder):
@@ -20,8 +21,16 @@
   i386:x86-64
   GNU/Linux
   
+
+
+
+
+
 
+
+
 
+
 
 
 
@@ -59,8 +68,16 @@
 
 def readRegisters(self):
 return (
-"0102030405060708"  # rsp
-"1112131415161718"  # rip
+"0102030405060708"  # rcx
+"1112131415161718"  # rdx
+"2122232425262728"  # rsi
+"3132333435363738"  # rdi
+"4142434445464748"  # rbp
+"5152535455565758"  # rsp
+"6162636465666768"  # r8
+"7172737475767778"  # r9
+"8182838485868788"  # rip
+"91929394"  # eflags
 "0102030405060708090a"  # st0
 "1112131415161718191a" +  # st1
 "2122232425262728292a" * 6 +  # st2..st7
@@ -82,9 +99,31 @@
 
 # rsp and rip should be displayed as uints
 self.match("register read rsp",
-   ["rsp = 0x0807060504030201"])
+   ["rsp = 0x5857565554535251"])
 self.match("register read rip",
-   ["rip = 0x1817161514131211"])
+   ["rip = 0x8887868584838281"])
+
+# test generic aliases
+self.match("register read arg4",
+   ["rcx = 0x0807060504030201"])
+self.match("register read arg3",
+   ["rdx = 0x1817161514131211"])
+self.match("register read arg2",
+   ["rsi = 0x2827262524232221"])
+self.match("register read arg1",
+   ["rdi = 0x3837363534333231"])
+self.match("register read fp",
+   ["rbp = 0x4847464544434241"])
+self.match("register read sp",
+   ["rsp = 0x5857565554535251"])
+self.match("register read arg5",
+   ["r8 = 0x6867666564636261"])
+self.match("register read arg6",
+   ["r9 = 0x7877767574737271"])
+self.match("register read pc",
+   ["rip = 0x8887868584838281"])
+self.match("register read flags",
+   ["eflags = 0x94939291"])
 
 # both stX and xmmX should be displayed as vectors
 self.match("register read st0",
Index: lldb/source/Target/RegisterContext.cpp
===
--- lldb/source/Target/RegisterContext.cpp
+++ lldb/source/Target/RegisterContext.cpp
@@ -54,6 +54,17 @@
   if (reg_name.empty())
 return nullptr;
 
+  // Try matching generic register names first.  This is consistent with
+  // how aliases work, esp. wrt "sp" generic alias taking precedence over
+  // "sp" pseudo-register on x86_64.
+  uint32_t generic_reg = Args::StringToGenericRegister(reg_name);
+  if (generic_reg != LLDB_INVALID_REGNUM) {
+const RegisterInfo *reg_info =
+GetRegisterInfo(eRegisterKindGeneric, generic_reg);
+if (reg_info)
+  return reg_info;
+  }
+
   const uint32_t num_registers = GetRegisterCount();
   for (uint32_t reg = start_idx; reg < num_registers; ++reg) {
 const RegisterInfo *reg_info = GetRegisterInfoAtIndex(reg);
@@ -62,6 +73,7 @@
 reg_name.equals_insensitive(reg_info->alt_name))
   return reg_info;
   }
+
   return nullptr;
 }
 
Index: lldb/source/Core/ValueObjectRegister.cpp
=

[Lldb-commits] [PATCH] D109600: [lldb] Remove PluginInterface::GetPluginVersion

[Alex Langford via Phabricator via lldb-commits]

bulbazord accepted this revision.
bulbazord added a comment.

Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109600

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

[Lldb-commits] [PATCH] D108061: [lldb] Add support for shared library load when executable called through ld.

[Rumeet Dhindsa via Phabricator via lldb-commits]

rdhindsa added a comment.

@teemperor: I believe I have addressed your comments. If there is any other 
concern, I can address it post-commit.

Thank you everyone for the review and help with the patch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108061

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

[Lldb-commits] [lldb] 03df971 - [lldb] Add support for debugging via the dynamic linker.

[Rumeet Dhindsa via lldb-commits]

Author: Rumeet Dhindsa
Date: 2021-09-10T10:59:31-07:00
New Revision: 03df97101287e8cb647c6c0982c4efdb82585c21

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

LOG: [lldb] Add support for debugging via the dynamic linker.

This patch adds support for shared library load when the executable is
called through ld.so.

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

Added: 
lldb/test/API/functionalities/dyld-launch-linux/Makefile
lldb/test/API/functionalities/dyld-launch-linux/TestDyldLaunchLinux.py
lldb/test/API/functionalities/dyld-launch-linux/main.cpp
lldb/test/API/functionalities/dyld-launch-linux/signal_file.cpp
lldb/test/API/functionalities/dyld-launch-linux/signal_file.h

Modified: 
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp

Removed: 




diff  --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp 
b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
index 866acbddbdc8a..7e80dc28e56b2 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
@@ -24,28 +24,35 @@
 using namespace lldb;
 using namespace lldb_private;
 
-/// Locates the address of the rendezvous structure.  Returns the address on
-/// success and LLDB_INVALID_ADDRESS on failure.
-static addr_t ResolveRendezvousAddress(Process *process) {
+DYLDRendezvous::DYLDRendezvous(Process *process)
+: m_process(process), m_rendezvous_addr(LLDB_INVALID_ADDRESS),
+  m_executable_interpreter(false), m_current(), m_previous(),
+  m_loaded_modules(), m_soentries(), m_added_soentries(),
+  m_removed_soentries() {
+  m_thread_info.valid = false;
+  UpdateExecutablePath();
+}
+
+addr_t DYLDRendezvous::ResolveRendezvousAddress() {
   Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
   addr_t info_location;
   addr_t info_addr;
   Status error;
 
-  if (!process) {
+  if (!m_process) {
 LLDB_LOGF(log, "%s null process provided", __FUNCTION__);
 return LLDB_INVALID_ADDRESS;
   }
 
   // Try to get it from our process.  This might be a remote process and might
   // grab it via some remote-specific mechanism.
-  info_location = process->GetImageInfoAddress();
+  info_location = m_process->GetImageInfoAddress();
   LLDB_LOGF(log, "%s info_location = 0x%" PRIx64, __FUNCTION__, info_location);
 
   // If the process fails to return an address, fall back to seeing if the
   // local object file can help us find it.
   if (info_location == LLDB_INVALID_ADDRESS) {
-Target *target = &process->GetTarget();
+Target *target = &m_process->GetTarget();
 if (target) {
   ObjectFile *obj_file = target->GetExecutableModule()->GetObjectFile();
   Address addr = obj_file->GetImageInfoAddress(target);
@@ -56,6 +63,20 @@ static addr_t ResolveRendezvousAddress(Process *process) {
   "%s resolved via direct object file approach to 0x%" PRIx64,
   __FUNCTION__, info_location);
   } else {
+const Symbol *_r_debug =
+target->GetExecutableModule()->FindFirstSymbolWithNameAndType(
+ConstString("_r_debug"));
+if (_r_debug) {
+  info_addr = _r_debug->GetAddress().GetLoadAddress(target);
+  if (info_addr != LLDB_INVALID_ADDRESS) {
+LLDB_LOGF(log,
+  "%s resolved by finding symbol '_r_debug' whose value is 
"
+  "0x%" PRIx64,
+  __FUNCTION__, info_addr);
+m_executable_interpreter = true;
+return info_addr;
+  }
+}
 LLDB_LOGF(log,
   "%s FAILED - direct object file approach did not yield a "
   "valid address",
@@ -70,9 +91,9 @@ static addr_t ResolveRendezvousAddress(Process *process) {
   }
 
   LLDB_LOGF(log, "%s reading pointer (%" PRIu32 " bytes) from 0x%" PRIx64,
-__FUNCTION__, process->GetAddressByteSize(), info_location);
+__FUNCTION__, m_process->GetAddressByteSize(), info_location);
 
-  info_addr = process->ReadPointerFromMemory(info_location, error);
+  info_addr = m_process->ReadPointerFromMemory(info_location, error);
   if (error.Fail()) {
 LLDB_LOGF(log, "%s FAILED - could not read from the info location: %s",
   __FUNCTION__, error.AsCString());
@@ -90,14 +111,6 @@ static addr_t ResolveRendezvousAddress(Process *process) {
   return info_addr;
 }
 
-DYLDRendezvous::DYLDRendezvous(Process *process)
-: m_process(process), m_rendezvous_addr(LLDB_INVALID_ADDRESS), m_current(),
-  m_previous(), m_loaded_

[Lldb-commits] [PATCH] D108061: [lldb] Add support for shared library load when executable called through ld.

[Rumeet Dhindsa via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG03df97101287: [lldb] Add support for debugging via the 
dynamic linker. (authored by rdhindsa).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108061

Files:
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.h
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  lldb/test/API/functionalities/dyld-launch-linux/Makefile
  lldb/test/API/functionalities/dyld-launch-linux/TestDyldLaunchLinux.py
  lldb/test/API/functionalities/dyld-launch-linux/main.cpp
  lldb/test/API/functionalities/dyld-launch-linux/signal_file.cpp
  lldb/test/API/functionalities/dyld-launch-linux/signal_file.h

Index: lldb/test/API/functionalities/dyld-launch-linux/signal_file.h
===
--- /dev/null
+++ lldb/test/API/functionalities/dyld-launch-linux/signal_file.h
@@ -0,0 +1 @@
+int get_signal_crash();
Index: lldb/test/API/functionalities/dyld-launch-linux/signal_file.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/dyld-launch-linux/signal_file.cpp
@@ -0,0 +1,7 @@
+#include "signal_file.h"
+#include 
+
+int get_signal_crash(void) {
+  raise(SIGSEGV);
+  return 0;
+}
Index: lldb/test/API/functionalities/dyld-launch-linux/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/dyld-launch-linux/main.cpp
@@ -0,0 +1,6 @@
+#include "signal_file.h"
+
+int main() {
+  // Break here
+  return get_signal_crash();
+}
Index: lldb/test/API/functionalities/dyld-launch-linux/TestDyldLaunchLinux.py
===
--- /dev/null
+++ lldb/test/API/functionalities/dyld-launch-linux/TestDyldLaunchLinux.py
@@ -0,0 +1,58 @@
+"""
+Test that LLDB can launch a linux executable through the dynamic loader and still hit a breakpoint.
+"""
+
+import lldb
+import os
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+class TestLinux64LaunchingViaDynamicLoader(TestBase):
+mydir = TestBase.compute_mydir(__file__)
+
+@skipIf(oslist=no_match(['linux']))
+@no_debug_info_test
+def test(self):
+self.build()
+
+# Extracts path of the interpreter.
+spec = lldb.SBModuleSpec()
+spec.SetFileSpec(lldb.SBFileSpec(self.getBuildArtifact("a.out")))
+interp_section = lldb.SBModule(spec).FindSection(".interp")
+if not interp_section:
+  return
+section_data = interp_section.GetSectionData()
+error = lldb.SBError()
+exe = section_data.GetString(error,0)
+if error.Fail():
+  return
+
+target = self.dbg.CreateTarget(exe)
+self.assertTrue(target, VALID_TARGET)
+
+# Set breakpoints both on shared library function as well as on
+# main. Both of them will be pending breakpoints.
+breakpoint_main = target.BreakpointCreateBySourceRegex("// Break here", lldb.SBFileSpec("main.cpp"))
+breakpoint_shared_library = target.BreakpointCreateBySourceRegex("get_signal_crash", lldb.SBFileSpec("signal_file.cpp"))
+launch_info = lldb.SBLaunchInfo([ "--library-path", self.get_process_working_directory(), self.getBuildArtifact("a.out")])
+launch_info.SetWorkingDirectory(self.get_process_working_directory())
+error = lldb.SBError()
+process = target.Launch(launch_info, error)
+self.assertTrue(error.Success())
+
+# Stopped on main here.
+self.assertEqual(process.GetState(), lldb.eStateStopped)
+thread = process.GetSelectedThread()
+self.assertIn("main", thread.GetFrameAtIndex(0).GetDisplayFunctionName())
+process.Continue()
+
+# Stopped on get_signal_crash function here.
+self.assertEqual(process.GetState(), lldb.eStateStopped)
+self.assertIn("get_signal_crash", thread.GetFrameAtIndex(0).GetDisplayFunctionName())
+process.Continue()
+
+# Stopped because of generated signal.
+self.assertEqual(process.GetState(), lldb.eStateStopped)
+self.assertIn("raise", thread.GetFrameAtIndex(0).GetDisplayFunctionName())
+self.assertIn("get_signal_crash", thread.GetFrameAtIndex(1).GetDisplayFunctionName())
Index: lldb/test/API/functionalities/dyld-launch-linux/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/dyld-launch-linux/Makefile
@@ -0,0 +1,4 @@
+CXX_SOURCES := main.cpp
+DYLIB_NAME := signal_file
+DYLIB_CXX_SOURCES := signal_file.cpp
+include Makefile.rules
Ind

[Lldb-commits] [PATCH] D108061: [lldb] Add support for shared library load when executable called through ld.

[Rumeet Dhindsa via Phabricator via lldb-commits]

rdhindsa added a comment.

With this patch, it looks like the test added in this patch fails for 
lldb-arm-ubuntu:
https://lab.llvm.org/buildbot/#/builders/17/builds/10498

Everything passes on these buildbots:  lldb-aarch64-ubuntu, 
lldb-x64-windows-ninja, lldb-x86_64-debian.

The test logic checks for breakpoint for main as well as for shared library 
load. It is able to extract that information using thread.GetFrameAtIndex(0). 
However, when it gets the signal, it is not able to extract the back trace and 
hence fails with the following error:

  ERROR: test (TestDyldLaunchLinux.TestLinux64LaunchingViaDynamicLoader)
  Traceback (most recent call last):
File 
"/home/tcwg-buildslave/worker/lldb-arm-ubuntu/llvm-project/lldb/test/API/functionalities/dyld-launch-linux/TestDyldLaunchLinux.py",
 line 57, in test
  self.assertIn("raise", thread.GetFrameAtIndex(0).GetDisplayFunctionName())
File 
"/home/tcwg-buildslave/worker/lldb-arm-ubuntu/llvm-project/lldb/third_party/Python/module/unittest2/unittest2/case.py",
 line 884, in assertIn
  if member not in container:
  TypeError: argument of type 'NoneType' is not iterable
  Config=arm-/home/tcwg-buildslave/worker/lldb-arm-ubuntu/build/bin/clang

@DavidSpickett, @zatrazz: If possible, could you please take a look. Is this 
expected behavior and test should be marked as unsupported for arm, or am I 
missing something here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108061

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

[Lldb-commits] [PATCH] D109626: [lldb] Remove redundant register alt_names

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

mgorny created this revision.
mgorny added reviewers: labath, krytarowski, emaste.
Herald added subscribers: atanasyan, jrtc27, kbarton, kristof.beyls, nemanjai, 
sdardis.
mgorny requested review of this revision.

Remove redundant register alt_names that correspond to their respective
generic names.  D108554  makes it possible to 
query registers through
their generic names directly, therefore making repeating them via
alt_name unnecessary.

While at it, also remove alt_names that are equal to register names
on PPC.

This patch does not alter register definitions where the generic names
are listed as primary names, and other names are provided as alt_name
(e.g. ARM).


https://reviews.llvm.org/D109626

Files:
  lldb/source/Plugins/ABI/ARM/ABIMacOSX_arm.cpp
  lldb/source/Plugins/ABI/ARM/ABISysV_arm.cpp
  lldb/source/Plugins/ABI/Mips/ABISysV_mips.cpp
  lldb/source/Plugins/ABI/Mips/ABISysV_mips64.cpp
  lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc.cpp
  lldb/source/Plugins/ABI/SystemZ/ABISysV_s390x.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextWindows_x86_64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfos_powerpc.h
  lldb/source/Plugins/Process/Utility/RegisterInfos_ppc64.h
  lldb/source/Plugins/Process/Utility/RegisterInfos_ppc64le.h
  lldb/source/Plugins/Process/Utility/RegisterInfos_s390x.h
  lldb/source/Plugins/Process/Utility/RegisterInfos_x86_64.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp

Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
@@ -888,14 +888,14 @@
 static RegisterInfo g_register_infos[] = {
 //   NAME ALT SZ   OFF  ENCODING  FORMAT  EH_FRAME DWARFGENERIC PROCESS PLUGIN  LLDBVALUE REGSINVALIDATE REGS SIZE EXPR SIZE LEN
 //   ==   ==  ===  ===  = ==  ===  ===  ==  =   ===== = 
-{ "r0",   "arg1",   4,   0, eEncodingUint,eFormatHex,   { ehframe_r0,  dwarf_r0,LLDB_REGNUM_GENERIC_ARG1,0,   0 }, nullptr,   nullptr,  nullptr,   0 },
-{ "r1",   "arg2",   4,   0, eEncodingUint,eFormatHex,   { ehframe_r1,  dwarf_r1,LLDB_REGNUM_GENERIC_ARG2,1,   1 }, nullptr,   nullptr,  nullptr,   0 },
-{ "r2",   "arg3",   4,   0, eEncodingUint,eFormatHex,   { ehframe_r2,  dwarf_r2,LLDB_REGNUM_GENERIC_ARG3,2,   2 }, nullptr,   nullptr,  nullptr,   0 },
-{ "r3",   "arg4",   4,   0, eEncodingUint,eFormatHex,   { ehframe_r3,  dwarf_r3,LLDB_REGNUM_GENERIC_ARG4,3,   3 }, nullptr,   nullptr,  nullptr,   0 },
+{ "r0",  nullptr,   4,   0, eEncodingUint,eFormatHex,   { ehframe_r0,  dwarf_r0,LLDB_REGNUM_GENERIC_ARG1,0,   0 }, nullptr,   nullptr,  nullptr,   0 },
+{ "r1",  nullptr,   4,   0, eEncodingUint,eFormatHex,   { ehframe_r1,  dwarf_r1,LLDB_REGNUM_GENERIC_ARG2,1,   1 }, nullptr,   nullptr,  nullptr,   0 },
+{ "r2",  nullptr,   4,   0, eEncodingUint,eFormatHex,   { ehframe_r2,  dwarf_r2,LLDB_REGNUM_GENERIC_ARG3,2,   2 }, nullptr,   nullptr,  nullptr,   0 },
+{ "r3",  nullptr,   4,   0, eEncodingUint,eFormatHex,   { ehframe_r3,  dwarf_r3,LLDB_REGNUM_GENERIC_ARG4,3,   3 }, nullptr,   nullptr,  nullptr,   0 },
 { "r4",  nullptr,   4,   0, eEncodingUint,eFormatHex,   { ehframe_r4,  dwarf_r4,LLDB_INVALID_REGNUM, 4,   4 }, nullptr,   nullptr,  nullptr,   0 },
 { "r5",  nullptr,   4,   0, eEncodingUint,eFormatHex,   { ehframe_r5,  dwarf_r5,LLDB_INVALID_REGNUM, 5,   5 }, nullptr,   nullptr,  nullptr,   0 },
 { "r6",  nullptr,   4,   0, eEncodingUint,eFormatHex,   { ehframe_r6,  dwarf_r6,LLDB_INVALID_REGNUM, 6,   6 }, nullptr,   nullptr,  nullptr,   0 },
-{ "r7", "fp",   4,   0, eEncodingUint,eFormatHex,   { ehframe_r7,  dwarf_r7,LLDB_REGNUM_GENERIC_FP,  7,   7 }, nullptr,   nullptr,  nullptr,   0 },
+{ "r7",  nullptr,   4,   0, eEncodingUint,eFormatHex,   { ehframe_r7,  dwarf_r7,LLDB_REGNUM_GENERIC_FP,  7,   7 }, nullptr,   nullptr,  nullptr,   0 },
 { "r8",  nul

[Lldb-commits] [lldb] 8dae355 - [lldb] Remove unused typedefs from lldb-forward.h

[Alex Langford via lldb-commits]

Author: Alex Langford
Date: 2021-09-10T14:16:47-07:00
New Revision: 8dae35527fb77a4698d05d39ed66f8c545885098

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

LOG: [lldb] Remove unused typedefs from lldb-forward.h

Added: 


Modified: 
lldb/include/lldb/lldb-forward.h

Removed: 




diff  --git a/lldb/include/lldb/lldb-forward.h 
b/lldb/include/lldb/lldb-forward.h
index ad5298151e4a4..bb6ab1fa0a843 100644
--- a/lldb/include/lldb/lldb-forward.h
+++ b/lldb/include/lldb/lldb-forward.h
@@ -290,7 +290,6 @@ typedef std::shared_ptr BlockSP;
 typedef std::shared_ptr BreakpointSP;
 typedef std::weak_ptr BreakpointWP;
 typedef std::shared_ptr BreakpointSiteSP;
-typedef std::weak_ptr BreakpointSiteWP;
 typedef std::shared_ptr BreakpointLocationSP;
 typedef std::weak_ptr BreakpointLocationWP;
 typedef std::shared_ptr
@@ -301,7 +300,6 @@ typedef std::shared_ptr 
BroadcasterManagerSP;
 typedef std::weak_ptr BroadcasterManagerWP;
 typedef std::shared_ptr UserExpressionSP;
 typedef std::shared_ptr CommandObjectSP;
-typedef std::shared_ptr CommunicationSP;
 typedef std::shared_ptr ConnectionSP;
 typedef std::shared_ptr CompUnitSP;
 typedef std::shared_ptr DataBufferSP;
@@ -311,7 +309,6 @@ typedef std::weak_ptr DebuggerWP;
 typedef std::shared_ptr DisassemblerSP;
 typedef std::unique_ptr
 DynamicCheckerFunctionsUP;
-typedef std::shared_ptr DynamicLoaderSP;
 typedef std::unique_ptr DynamicLoaderUP;
 typedef std::shared_ptr EventSP;
 typedef std::shared_ptr EventDataSP;
@@ -323,7 +320,6 @@ typedef std::shared_ptr 
ExpressionVariableSP;
 typedef std::unique_ptr FileUP;
 typedef std::shared_ptr FileSP;
 typedef std::shared_ptr FunctionSP;
-typedef std::shared_ptr FunctionCallerSP;
 typedef std::shared_ptr FuncUnwindersSP;
 typedef std::shared_ptr InlineFunctionInfoSP;
 typedef std::shared_ptr InstructionSP;
@@ -335,9 +331,7 @@ typedef std::shared_ptr 
IRExecutionUnitSP;
 typedef std::shared_ptr JITLoaderSP;
 typedef std::unique_ptr JITLoaderListUP;
 typedef std::shared_ptr LanguageRuntimeSP;
-typedef std::shared_ptr SystemRuntimeSP;
 typedef std::unique_ptr SystemRuntimeUP;
-typedef std::shared_ptr LineTableSP;
 typedef std::shared_ptr ListenerSP;
 typedef std::weak_ptr ListenerWP;
 typedef std::shared_ptr MemoryHistorySP;
@@ -346,7 +340,6 @@ typedef std::shared_ptr 
MemoryRegionInfoSP;
 typedef std::shared_ptr ModuleSP;
 typedef std::weak_ptr ModuleWP;
 typedef std::shared_ptr ObjectFileSP;
-typedef std::weak_ptr ObjectFileWP;
 typedef std::shared_ptr
 ObjectFileJITDelegateSP;
 typedef std::weak_ptr
@@ -354,32 +347,12 @@ typedef std::weak_ptr
 typedef std::unique_ptr OperatingSystemUP;
 typedef std::shared_ptr OptionValueSP;
 typedef std::weak_ptr OptionValueWP;
-typedef std::shared_ptr OptionValueArchSP;
-typedef std::shared_ptr OptionValueArgsSP;
-typedef std::shared_ptr OptionValueArraySP;
-typedef std::shared_ptr OptionValueBooleanSP;
-typedef std::shared_ptr
-OptionValueDictionarySP;
-typedef std::shared_ptr
-OptionValueFileSpecSP;
-typedef std::shared_ptr
-OptionValueFileSpecListSP;
-typedef std::shared_ptr OptionValueFormatSP;
-typedef std::shared_ptr
-OptionValuePathMappingsSP;
 typedef std::shared_ptr
 OptionValuePropertiesSP;
-typedef std::shared_ptr OptionValueRegexSP;
-typedef std::shared_ptr OptionValueSInt64SP;
-typedef std::shared_ptr OptionValueStringSP;
-typedef std::shared_ptr OptionValueUInt64SP;
-typedef std::shared_ptr OptionValueUUIDSP;
 typedef std::shared_ptr PlatformSP;
 typedef std::shared_ptr ProcessSP;
 typedef std::shared_ptr ProcessAttachInfoSP;
-typedef std::shared_ptr ProcessLaunchInfoSP;
 typedef std::weak_ptr ProcessWP;
-typedef std::shared_ptr PropertySP;
 typedef std::shared_ptr RegisterCheckpointSP;
 typedef std::shared_ptr RegisterContextSP;
 typedef std::shared_ptr RegularExpressionSP;
@@ -392,7 +365,6 @@ typedef std::shared_ptr
 typedef std::shared_ptr
 ScriptSummaryFormatSP;
 typedef std::shared_ptr ScriptInterpreterSP;
-typedef std::unique_ptr ScriptInterpreterUP;
 typedef std::unique_ptr
 ScriptedProcessInterfaceUP;
 typedef std::shared_ptr SectionSP;
@@ -400,10 +372,8 @@ typedef std::unique_ptr 
SectionListUP;
 typedef std::weak_ptr SectionWP;
 typedef std::shared_ptr SectionLoadListSP;
 typedef std::shared_ptr SearchFilterSP;
-typedef std::shared_ptr SettingsSP;
 typedef std::unique_ptr SourceManagerUP;
 typedef std::shared_ptr StackFrameSP;
-typedef std::unique_ptr StackFrameUP;
 typedef std::weak_ptr StackFrameWP;
 typedef std::shared_ptr StackFrameListSP;
 typedef std::shared_ptr
@@ -412,7 +382,6 @@ typedef 
std::unique_ptr
 StackFrameRecognizerManagerUP;
 typedef std::shared_ptr StopInfoSP;
 typedef std::shared_ptr StreamSP;
-typedef std::weak_ptr StreamWP;
 typedef std::shared_ptr StreamFileSP;
 typedef std::s

[Lldb-commits] [PATCH] D109633: [lldb-vscode] Fix focus thread when previous thread exits

[Ted Woodward via Phabricator via lldb-commits]

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

The thread that Visual Studio Code displays on a stop is called the focus 
thread. When the previous focus thread exits and we stop in a new thread, 
lldb-vscode does not tell vscode to set the new thread as the focus thread, so 
it selects the first thread in the thread list.

This patch changes lldb-vscode to tell vscode that the new thread is the focus 
thread. It also includes a test that verifies the DAP stop message for this 
case contains the correct values.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D109633

Files:
  lldb/test/API/tools/lldb-vscode/correct-thread/Makefile
  lldb/test/API/tools/lldb-vscode/correct-thread/TestVSCode_correct_thread.py
  lldb/test/API/tools/lldb-vscode/correct-thread/main.c
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -232,13 +232,17 @@
   // set it as the focus thread if below if needed.
   lldb::tid_t first_tid_with_reason = LLDB_INVALID_THREAD_ID;
   uint32_t num_threads_with_reason = 0;
+  bool focus_thread_exists = false;
   for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
 lldb::SBThread thread = process.GetThreadAtIndex(thread_idx);
 const lldb::tid_t tid = thread.GetThreadID();
 const bool has_reason = ThreadHasStopReason(thread);
 // If the focus thread doesn't have a stop reason, clear the thread ID
-if (tid == g_vsc.focus_tid && !has_reason)
-  g_vsc.focus_tid = LLDB_INVALID_THREAD_ID;
+if (tid == g_vsc.focus_tid) {
+  focus_thread_exists = true;
+  if (!has_reason)
+g_vsc.focus_tid = LLDB_INVALID_THREAD_ID;
+}
 if (has_reason) {
   ++num_threads_with_reason;
   if (first_tid_with_reason == LLDB_INVALID_THREAD_ID)
@@ -246,10 +250,10 @@
 }
   }
 
-  // We will have cleared g_vsc.focus_tid if he focus thread doesn't
-  // have a stop reason, so if it was cleared, or wasn't set, then set the
-  // focus thread to the first thread with a stop reason.
-  if (g_vsc.focus_tid == LLDB_INVALID_THREAD_ID)
+  // We will have cleared g_vsc.focus_tid if he focus thread doesn't have
+  // a stop reason, so if it was cleared, or wasn't set, or doesn't exist,
+  // then set the focus thread to the first thread with a stop reason.
+  if (!focus_thread_exists || g_vsc.focus_tid == LLDB_INVALID_THREAD_ID)
 g_vsc.focus_tid = first_tid_with_reason;
 
   // If no threads stopped with a reason, then report the first one so
Index: lldb/test/API/tools/lldb-vscode/correct-thread/main.c
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/correct-thread/main.c
@@ -0,0 +1,23 @@
+#include 
+#include 
+
+int state_var;
+
+void *thread (void *in)
+{
+  state_var++; // break here
+  return NULL;
+}
+
+int main(int argc, char **argv)
+{
+  pthread_t t1, t2;
+
+  pthread_create(&t1, NULL, *thread, NULL);
+  pthread_join(t1, NULL);
+  pthread_create(&t2, NULL, *thread, NULL);
+  pthread_join(t2, NULL);
+
+  printf("state_var is %d\n", state_var);
+  return 0;
+}
Index: lldb/test/API/tools/lldb-vscode/correct-thread/TestVSCode_correct_thread.py
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/correct-thread/TestVSCode_correct_thread.py
@@ -0,0 +1,47 @@
+"""
+Test lldb-vscode setBreakpoints request
+"""
+
+from __future__ import print_function
+
+import unittest2
+import vscode
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+import lldbvscode_testcase
+
+
+class TestVSCode_correct_thread(lldbvscode_testcase.VSCodeTestCaseBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipIfWindows
+@skipIfRemote
+def test_correct_thread(self):
+'''
+Tests that the correct thread is selected if we continue from
+a thread that goes away and hit a breakpoint in another thread.
+In this case, the selected thread should be the thread that
+just hit the breakpoint, and not the first thread in the list.
+'''
+program = self.getBuildArtifact("a.out")
+self.build_and_launch(program)
+source = 'main.c'
+breakpoint_line = line_number(source, '// break here')
+lines = [breakpoint_line]
+# Set breakpoint in the thread function
+breakpoint_ids = self.set_source_breakpoints(source, lines)
+self.assertEqual(len(breakpoint_ids), len(lines),
+"expect correct numb

[Lldb-commits] [lldb] 89ed21a - Recognize namespaced all_image_infos symbol name from dyld

[Jason Molenda via lldb-commits]

Author: Jason Molenda
Date: 2021-09-10T16:56:48-07:00
New Revision: 89ed21a8f864b1c33a95ed127d073842adc2fcd0

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

LOG: Recognize namespaced all_image_infos symbol name from dyld

In macOS 12, the symbol name for the dyld_all_image_infos struct
in dyld has a namespace qualifier.  Search for it without qualification,
then with qualification when doing a by-name search.  (lldb will
only search for it by name when loading a user process Mach-O corefile)

rdar://76270013

Added: 


Modified: 
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp 
b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
index 91b1682ad001..a14e45c16c5c 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
@@ -251,6 +251,7 @@ bool 
DynamicLoaderMacOSXDYLD::ReadDYLDInfoFromMemoryAndSetNotificationCallback(
   std::lock_guard baseclass_guard(GetMutex());
   DataExtractor data; // Load command data
   static ConstString g_dyld_all_image_infos("dyld_all_image_infos");
+  static ConstString g_new_dyld_all_image_infos("dyld4::dyld_all_image_infos");
   if (ReadMachHeader(addr, &m_dyld.header, &data)) {
 if (m_dyld.header.filetype == llvm::MachO::MH_DYLINKER) {
   m_dyld.address = addr;
@@ -268,6 +269,10 @@ bool 
DynamicLoaderMacOSXDYLD::ReadDYLDInfoFromMemoryAndSetNotificationCallback(
   dyld_module_sp.get()) {
 const Symbol *symbol = dyld_module_sp->FindFirstSymbolWithNameAndType(
 g_dyld_all_image_infos, eSymbolTypeData);
+if (!symbol) {
+  symbol = dyld_module_sp->FindFirstSymbolWithNameAndType(
+  g_new_dyld_all_image_infos, eSymbolTypeData);
+}
 if (symbol)
   m_dyld_all_image_infos_addr = symbol->GetLoadAddress(&target);
   }



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