[Lldb-commits] [PATCH] D82863: [LLDB] Add support to resize SVE registers at run-time

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid added a comment.

In D82863#2373557 , @labath wrote:

> [ I've replying to your message in a slightly different order that what it 
> was written. ]
>
> In D82863#2373356 , @omjavaid wrote:
>
>> In D82863#2373276 , @labath wrote:
>>
>>> I'd like to avoid sending bogus offsets, if possible. I'd rather that we 
>>> teach the server to not send them, and then have the client use this as a 
>>> cue that it needs to recompute something.
>>
>> I dont want to write -1 into offset field on server side because it is good 
>> helper for accessing data in register buffers but we can introduce a 
>> mechanism for all architectures supporting g/G packet we should not send the 
>> offset field in qRegisterInfo or target xml. The offset will be constructed 
>> by client itself and populated on client side register infos list for 
>> helping out with register data management.
>
> Ok, I see where you're going now. If you can pull that off, I think that'd be 
> great. Is that what the current patch does (I haven't looked at the today's 
> update yet)?
>
> Just in case that doesn't work for any reason, I'll elaborate on the idea I 
> had in mind.
>
>>> Then qRegisterInfo (and target.xml) could just check for the -1 value and 
>>> use that as a cue to skip sending the offset field? All current 
>>> architectures always hard-code an offset, so this would be a no-op for 
>>> them...
>>
>> byte_offset field in RegisterInfo struct is also used elsewhere in code 
>> specially to copy data to-from cached register values in data heap buffer. 
>> Also on the server side this byte_offset field is used to calculate position 
>> of register data in ptrace buffer etc.
>
> The register context could keep relying on the byte_offset for all non-SVE 
> registers. The SVE registers are pretty special anyway, so I was thinking 
> part of their specialness could be that they get their offsets from some 
> other place. Note that we already have the escape hatch called 
> `GetPtraceOffset`, which was added because we could not reconcile the ptrace 
> and `g` uses of the byte_offset fields on x86. So, in general, I think that 
> separating these two things is not a bad direction to go in. For example, the 
> bsd implementations already do a switch over the register number instead of 
> reusing the byte_offset for ptrace purposes. (I think that code could be 
> implemented in a more elegant way, but it is still consistent with that 
> direction.) Your approach also kind of makes that possible (not completely, 
> but it does move us slightly closer towards that), which is why I like it.
>
>>> I'm not sure I understand that. Can you elaborate on the relevance of 
>>> value_regs here?
>>
>> So value regs are pseudo register for example V register in Arm64 SVE is a 
>> subset of first 16-bytes of a Z register. We do not include value registers 
>> in g packet but then need a way to tell where in g/G packet data a specific 
>> Vx pseudo register will be located. In my updated patch I update offsets of 
>> all primary registers GPR, Z, P, FFR etc and also I copy that offset into 
>> their corresponding value registers for example for Z0, same offset is 
>> copied to V0, D0, S0. This helps with quick fetching of data on both client 
>> and server side.
>
> Ok, I see what you mean. So how will this be handled in your proposal? Will 
> the client "know" that the V/D/S registers are located at particular 
> positions within the Z registers ? Because I think the same solution could be 
> applied to my idea (your proposal is basically a stricter (less flexible) 
> version of what I had in mind -- it does not allow combining registers with 
> offsets and offset-less regs (and that may be more flexibility than we need)).
>
> I think that embedding this kind of knowledge into the client is fine. 
> AFAICT, this is what gdb is doing anyway (I don't see e.g. eax/ax/ah/al being 
> described in the x86_64 target.xml).

GDB does not have pseudo registers as part of xml register description. GDB is 
managing pseudo registers on client side and independent of rest of the 
register set. In case of SVE Z registers, GDB describes a composite type kind 
of a union of (Z, V, D and S) which helps in giving registers a view in terms 
of V, S and D which are not actually register.

As invalidate_regs and value_regs are totally LLDB specific so we can put 
information about LLDB speicific pseudo registers in those lists which we 
actually do by the way. Taking advantage of that I have S, V and D registers in 
invalidate_reg list of a Z register which share the same starting offset as the 
corresponding Z register but are size restricted to 4, 8 or 16 bytes.

For the sake of clarity here is the scheme:

We have two types of registers:

1. Primary registers

Includes 64 bit GPRs Xn regs, PC etc
Also includes V registers for all non-sve targets
Include

[Lldb-commits] [PATCH] D82863: [LLDB] Add support to resize SVE registers at run-time

[Muhammad Omair Javaid via Phabricator via lldb-commits]

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

This updates after discussion yesterday.


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

https://reviews.llvm.org/D82863

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1761,6 +1761,21 @@
 gdb_thread->PrivateSetRegisterValue(pair.first, buffer_sp->GetData());
   }
 
+  // Code below is specific to AArch64 target in SVE state
+  // If expedited register set contains vector granule (vg) register
+  // then thread's register context reconfiguration is triggered by
+  // calling UpdateARM64SVERegistersInfos.
+  const ArchSpec &arch = GetTarget().GetArchitecture();
+  if (arch.IsValid() && (arch.GetMachine() == llvm::Triple::aarch64 ||
+ arch.GetMachine() == llvm::Triple::aarch64_be)) {
+GDBRemoteRegisterContext *reg_ctx_sp =
+static_cast(
+gdb_thread->GetRegisterContext().get());
+
+if (reg_ctx_sp)
+  reg_ctx_sp->AArch64SVEReconfigure();
+  }
+
   thread_sp->SetName(thread_name.empty() ? nullptr : thread_name.c_str());
 
   gdb_thread->SetThreadDispatchQAddr(thread_dispatch_qaddr);
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
@@ -40,6 +40,9 @@
 
   void HardcodeARMRegisters(bool from_scratch);
 
+  bool UpdateARM64SVERegistersInfos(uint64_t vg, uint32_t &end_reg_offset,
+std::vector &invalidate_regs);
+
   void CloneFrom(GDBRemoteDynamicRegisterInfoSP process_reginfo);
 };
 
@@ -79,6 +82,8 @@
   uint32_t ConvertRegisterKindToRegisterNumber(lldb::RegisterKind kind,
uint32_t num) override;
 
+  bool AArch64SVEReconfigure();
+
 protected:
   friend class ThreadGDBRemote;
 
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
@@ -213,8 +213,8 @@
   for (int i = 0; i < regcount; i++) {
 struct RegisterInfo *reginfo =
 m_reg_info_sp->GetRegisterInfoAtIndex(i);
-if (reginfo->byte_offset + reginfo->byte_size 
-   <= buffer_sp->GetByteSize()) {
+if (reginfo->byte_offset + reginfo->byte_size <=
+buffer_sp->GetByteSize()) {
   m_reg_valid[i] = true;
 } else {
   m_reg_valid[i] = false;
@@ -343,6 +343,17 @@
   if (dst == nullptr)
 return false;
 
+  // Code below is specific to AArch64 target in SVE state
+  // If vector granule (vg) register is being written then thread's
+  // register context reconfiguration is triggered on success.
+  bool do_reconfigure_arm64_sve = false;
+  const ArchSpec &arch = process->GetTarget().GetArchitecture();
+  if (arch.IsValid() && (arch.GetMachine() == llvm::Triple::aarch64 ||
+ arch.GetMachine() == llvm::Triple::aarch64_be)) {
+if (strcmp(reg_info->name, "vg") == 0)
+  do_reconfigure_arm64_sve = true;
+  }
+
   if (data.CopyByteOrderedData(data_offset,// src offset
reg_info->byte_size,// src length
dst,// dst
@@ -362,6 +373,11 @@
 
 {
   SetAllRegisterValid(false);
+
+  if (do_reconfigure_arm64_sve &&
+  GetPrimordialRegister(reg_info, gdb_comm))
+AArch64SVEReconfigure();
+
   return true;
 }
   } else {
@@ -390,6 +406,10 @@
 } else {
   // This is an actual register, write it
   success = SetPrimordialRegister(reg_info, gdb_comm);
+
+  if (success && do_reconfigure_arm64_sve &&
+  GetPrimordialRegister(reg_info, gdb_comm))
+AArch64SVEReconfigure();
 }
 
 // Check if writing this register will invalidate any other register
@@ -655,9 +675,8 @@
   if (m_thread.GetProcess().get()) {
 const ArchSpe

[Lldb-commits] [PATCH] D90769: [lldb][ObjectFile] Relocate sections for in-memory objects (e.g. received via JITLoaderGDB)

[Pavel Labath via Phabricator via lldb-commits]

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

Seems reasonable to me.

I do wonder though if the jit could be changed to avoid relocation. I don't 
know what's the behavior of other JITs, but given that the jitted object is not 
going to get "linked" in the normal sense of the word, wasting memory on debug 
info relocations seems suboptimal...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90769

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

[Lldb-commits] [PATCH] D90757: [lldb] Enable FreeBSDRemote plugin by default and update test status

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

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



Comment at: 
lldb/test/API/tools/lldb-server/thread-name/TestGdbRemoteThreadName.py:32
 @skipIfWindows # the test is not updated for Windows.
+@expectedFailureAll(oslist=["freebsd"])
 @llgs_test

labath wrote:
> mgorny wrote:
> > labath wrote:
> > > What's up with this? I though you fixed thread names already..
> > The test code is buggy. I want to fix it separately not to mix things up 
> > too much.
> Fixing it here is not a good idea, yes. But I think it would have been good 
> to fix in the patch that was implementing thread name support.
I'm planning to run the test suite a lot more once this lands, and we have more 
predictable results.


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

https://reviews.llvm.org/D90757

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

[Lldb-commits] [PATCH] D90840: [lldb/DWARF] Fix sizes of DW_OP_const[1248][us] and DW_OP_litN results

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: JDevlieghere, aprantl, dblaikie.
Herald added a project: LLDB.
labath requested review of this revision.

Dwarf says (Section 2.5.1.1. of DWARF v5) that these operations should
push "generic" (pointer-sized) values. This was not the case for
DW_OP_const operations (which were pushing values based on the size of
arguments), nor DW_OP_litN (which were always pushing 64-bit values).

The practical effect of this that were were unable to display the values
of variables if the size of the DW_OP_const opcode was smaller than the
value of the variable it was describing. This would happen because we
would store this (small) result into a buffer and then would not be able
to read sufficient data out of it (in Value::GetValueAsData). Gcc emits
debug info like this.

Other (more subtle) effects are also possible.

The same fix should be applied to DW_OP_const[us] (leb128 versions), but
I'm not doing that right now, because that would cause us to display
wrong (truncated) values of variables on 32-bit targets (pr48087).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90840

Files:
  lldb/include/lldb/Utility/Scalar.h
  lldb/source/Expression/DWARFExpression.cpp
  lldb/unittests/Expression/DWARFExpressionTest.cpp

Index: lldb/unittests/Expression/DWARFExpressionTest.cpp
===
--- lldb/unittests/Expression/DWARFExpressionTest.cpp
+++ lldb/unittests/Expression/DWARFExpressionTest.cpp
@@ -75,6 +75,37 @@
llvm::Failed());
 }
 
+TEST(DWARFExpression, DW_OP_const) {
+  // Extend to address size.
+  EXPECT_THAT_EXPECTED(Evaluate({DW_OP_const1u, 0x88}), llvm::HasValue(0x88));
+  EXPECT_THAT_EXPECTED(Evaluate({DW_OP_const1s, 0x88}),
+   llvm::HasValue(0xff88));
+  EXPECT_THAT_EXPECTED(Evaluate({DW_OP_const2u, 0x47, 0x88}),
+   llvm::HasValue(0x8847));
+  EXPECT_THAT_EXPECTED(Evaluate({DW_OP_const2s, 0x47, 0x88}),
+   llvm::HasValue(0x8847));
+  EXPECT_THAT_EXPECTED(Evaluate({DW_OP_const4u, 0x44, 0x42, 0x47, 0x88}),
+   llvm::HasValue(0x88474244));
+  EXPECT_THAT_EXPECTED(Evaluate({DW_OP_const4s, 0x44, 0x42, 0x47, 0x88}),
+   llvm::HasValue(0x88474244));
+
+  // Truncate to address size.
+  EXPECT_THAT_EXPECTED(
+  Evaluate({DW_OP_const8u, 0x00, 0x11, 0x22, 0x33, 0x44, 0x42, 0x47, 0x88}),
+  llvm::HasValue(0x33221100));
+  EXPECT_THAT_EXPECTED(
+  Evaluate({DW_OP_const8s, 0x00, 0x11, 0x22, 0x33, 0x44, 0x42, 0x47, 0x88}),
+  llvm::HasValue(0x33221100));
+
+  // Don't truncate to address size for compatibility with clang (pr48087).
+  EXPECT_THAT_EXPECTED(
+  Evaluate({DW_OP_constu, 0x81, 0x82, 0x84, 0x88, 0x90, 0xa0, 0x40}),
+  llvm::HasValue(0x01010101010101));
+  EXPECT_THAT_EXPECTED(
+  Evaluate({DW_OP_consts, 0x81, 0x82, 0x84, 0x88, 0x90, 0xa0, 0x40}),
+  llvm::HasValue(0x010101010101));
+}
+
 TEST(DWARFExpression, DW_OP_convert) {
   /// Auxiliary debug info.
   const char *yamldata = R"(
@@ -157,22 +188,15 @@
   // Positive tests.
   //
 
-  // Truncate to default unspecified (pointer-sized) type.
-  EXPECT_THAT_EXPECTED(
-  t.Eval({DW_OP_const8u, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, //
-  DW_OP_convert, 0x00}),
-  llvm::HasValue(GetScalar(32, 0x44332211, not_signed)));
-  // Truncate to 32 bits.
-  EXPECT_THAT_EXPECTED(t.Eval({DW_OP_const8u, //
-   0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88,//
+  // Leave as is.
+  EXPECT_THAT_EXPECTED(t.Eval({DW_OP_const4u, 0x11, 0x22, 0x33, 0x44, //
DW_OP_convert, offs_uint32_t}),
-   llvm::HasValue(GetScalar(32, 0x44332211, not_signed)));
+   llvm::HasValue(GetScalar(64, 0x44332211, not_signed)));
 
-  // Leave as is.
-  EXPECT_THAT_EXPECTED(
-  t.Eval({DW_OP_const8u, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, //
-  DW_OP_convert, offs_uint64_t}),
-  llvm::HasValue(GetScalar(64, 0x8877665544332211, not_signed)));
+  // Zero-extend to 64 bits.
+  EXPECT_THAT_EXPECTED(t.Eval({DW_OP_const4u, 0x11, 0x22, 0x33, 0x44, //
+   DW_OP_convert, offs_uint64_t}),
+   llvm::HasValue(GetScalar(64, 0x44332211, not_signed)));
 
   // Sign-extend to 64 bits.
   EXPECT_THAT_EXPECTED(
@@ -180,6 +204,18 @@
   DW_OP_convert, offs_sint64_t}),
   llvm::HasValue(GetScalar(64, 0xffeeddcc, is_signed)));
 
+  // Sign-extend, then truncate.
+  EXPECT_THAT_EXPECTED(t.Eval({DW_OP_const4s, 0xcc, 0xdd, 0xee, 0xff, //
+   DW_OP_convert, offs_sint64_t,  //
+   DW_OP_convert, offs_uint32_t}),
+   llvm::HasValue(GetScalar(32, 0xffeeddcc, not_signed)));
+
+  // Truncate to default unspecified (pointer-sized) type.
+  

[Lldb-commits] [PATCH] D90840: [lldb/DWARF] Fix sizes of DW_OP_const[1248][us] and DW_OP_litN results

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/source/Expression/DWARFExpression.cpp:945
+  auto to_generic = [addr_size = opcodes.GetAddressByteSize()](auto v) {
+bool is_signed = std::is_signed::value;
+return Scalar(

This is probably not conforming either -- dwarf says the generic type is of 
"unspecified signedness", but I think it means a single global value -- not 
choosing signedness on a per-operand basis.

I've kept that for now, since that was the original behavior, though I didn't 
notice any issues with this bit removed.



Comment at: lldb/unittests/Expression/DWARFExpressionTest.cpp:191
 
-  // Truncate to default unspecified (pointer-sized) type.
-  EXPECT_THAT_EXPECTED(
-  t.Eval({DW_OP_const8u, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, //
-  DW_OP_convert, 0x00}),
-  llvm::HasValue(GetScalar(32, 0x44332211, not_signed)));
-  // Truncate to 32 bits.
-  EXPECT_THAT_EXPECTED(t.Eval({DW_OP_const8u, //
-   0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 
0x88,//
+  // Leave as is.
+  EXPECT_THAT_EXPECTED(t.Eval({DW_OP_const4u, 0x11, 0x22, 0x33, 0x44, //

Had to rewrite these, as they were relying on the fact that DW_OP_const 
operations were not implicitly converting to the generic type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90840

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

[Lldb-commits] [PATCH] D90787: [LLDB-lua] modify Lua's 'print' to respect 'io.stdout'

[Pedro Tammela via Phabricator via lldb-commits]

tammela updated this revision to Diff 303088.
tammela added a comment.

Add test case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90787

Files:
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
  lldb/test/Shell/ScriptInterpreter/Lua/print.test


Index: lldb/test/Shell/ScriptInterpreter/Lua/print.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/print.test
@@ -0,0 +1,23 @@
+# REQUIRES: lua
+# UNSUPPORTED: lldb-repro
+#
+# RUN: rm -rf %t.stderr %t.stdout
+# RUN: cat %s | %lldb --script-language lua 2> %t.stderr > %t.stdout
+# RUN: cat %t.stdout | FileCheck %s --check-prefix STDOUT
+# RUN: cat %t.stderr | FileCheck %s --check-prefix STDERR
+script
+file = lldb.SBFile(2, "w", false)
+lldb.debugger:SetOutputFile(file)
+print(95000 + 126, nil, 'a')
+quit
+script
+print({})
+quit
+
+# STDOUT: 95126nil a
+# STDOUT-NOT: table: {{0x[[:xdigit:]]+}}
+# STDERR: table: {{0x[[:xdigit:]]+}}
+
+# RUN: rm -rf %t.stderr %t.stdout
+# RUN: %lldb --script-language lua -o 'script print(95000 + 126, nil, "a")' 2> 
%t.stderr > %t.stdout
+# RUN: cat %t.stdout | FileCheck %s --check-prefix STDOUT
Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
@@ -25,16 +25,8 @@
 
 class Lua {
 public:
-  Lua() : m_lua_state(luaL_newstate()) {
-assert(m_lua_state);
-luaL_openlibs(m_lua_state);
-luaopen_lldb(m_lua_state);
-  }
-
-  ~Lua() {
-assert(m_lua_state);
-lua_close(m_lua_state);
-  }
+  Lua();
+  ~Lua();
 
   llvm::Error Run(llvm::StringRef buffer);
   llvm::Error LoadModule(llvm::StringRef filename);
Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
@@ -14,6 +14,35 @@
 using namespace lldb_private;
 using namespace lldb;
 
+static int lldb_print(lua_State *L) {
+  int n = lua_gettop(L);
+  lua_getglobal(L, "io");
+  lua_getfield(L, -1, "stdout");
+  lua_getfield(L, -1, "write");
+  for (int i = 1; i <= n; i++) {
+lua_pushvalue(L, -1); // write()
+lua_pushvalue(L, -3); // io.stdout
+luaL_tolstring(L, i, nullptr);
+lua_pushstring(L, i != n ? "\t" : "\n");
+lua_call(L, 3, 0);
+  }
+  return 0;
+}
+
+Lua::Lua() {
+  m_lua_state = luaL_newstate();
+  assert(m_lua_state);
+  luaL_openlibs(m_lua_state);
+  luaopen_lldb(m_lua_state);
+  lua_pushcfunction(m_lua_state, lldb_print);
+  lua_setglobal(m_lua_state, "print");
+}
+
+Lua::~Lua() {
+  assert(m_lua_state);
+  lua_close(m_lua_state);
+}
+
 llvm::Error Lua::Run(llvm::StringRef buffer) {
   int error =
   luaL_loadbuffer(m_lua_state, buffer.data(), buffer.size(), "buffer") ||


Index: lldb/test/Shell/ScriptInterpreter/Lua/print.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/print.test
@@ -0,0 +1,23 @@
+# REQUIRES: lua
+# UNSUPPORTED: lldb-repro
+#
+# RUN: rm -rf %t.stderr %t.stdout
+# RUN: cat %s | %lldb --script-language lua 2> %t.stderr > %t.stdout
+# RUN: cat %t.stdout | FileCheck %s --check-prefix STDOUT
+# RUN: cat %t.stderr | FileCheck %s --check-prefix STDERR
+script
+file = lldb.SBFile(2, "w", false)
+lldb.debugger:SetOutputFile(file)
+print(95000 + 126, nil, 'a')
+quit
+script
+print({})
+quit
+
+# STDOUT: 95126	nil	a
+# STDOUT-NOT: table: {{0x[[:xdigit:]]+}}
+# STDERR: table: {{0x[[:xdigit:]]+}}
+
+# RUN: rm -rf %t.stderr %t.stdout
+# RUN: %lldb --script-language lua -o 'script print(95000 + 126, nil, "a")' 2> %t.stderr > %t.stdout
+# RUN: cat %t.stdout | FileCheck %s --check-prefix STDOUT
Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
@@ -25,16 +25,8 @@
 
 class Lua {
 public:
-  Lua() : m_lua_state(luaL_newstate()) {
-assert(m_lua_state);
-luaL_openlibs(m_lua_state);
-luaopen_lldb(m_lua_state);
-  }
-
-  ~Lua() {
-assert(m_lua_state);
-lua_close(m_lua_state);
-  }
+  Lua();
+  ~Lua();
 
   llvm::Error Run(llvm::StringRef buffer);
   llvm::Error LoadModule(llvm::StringRef filename);
Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
@@ -14,6 +14,35 @@
 using namespace lldb_private;
 using namespace lldb;
 
+static int lldb_print(lua_State *L) {
+  int n = lua_gettop(L);
+  lua_getglobal(L, "io");
+  lua_

[Lldb-commits] [lldb] 2f84b59 - [lldb] Also Catch invalid calls to TestPExpectTest's expect()

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-11-05T14:08:46+01:00
New Revision: 2f84b59a4cf92a0ce1b985e7f44e17483efa33c0

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

LOG: [lldb] Also Catch invalid calls to TestPExpectTest's expect()

This is a follow up to D88792 which found an issue in a call to PExpectTest's
expect function that allows passing a string to the `substrs` parameter. However
this issue was found by just grepping and TestPExpect's expect function is still
accepting a single string as a value to `substrs`.

This patch adds the same sanity check that D88792 added to the PExpectTest's
implementation of `expect` and also adds a small test for it.

Reviewed By: kastiglione, JDevlieghere

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

Added: 
lldb/test/API/test_utils/TestPExpectTest.py

Modified: 
lldb/packages/Python/lldbsuite/test/lldbpexpect.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/lldbpexpect.py 
b/lldb/packages/Python/lldbsuite/test/lldbpexpect.py
index 12ac4d381e2c..1a6c32094ba8 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbpexpect.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbpexpect.py
@@ -54,6 +54,10 @@ def launch(self, executable=None, extra_args=None, 
timeout=30, dimensions=None):
 def expect(self, cmd, substrs=None):
 self.assertNotIn('\n', cmd)
 self.child.sendline(cmd)
+# If 'substrs' is a string then this code would just check that every
+# character of the string is in the output.
+assert not isinstance(substrs, six.string_types), \
+"substrs must be a collection of strings"
 if substrs is not None:
 for s in substrs:
 self.child.expect_exact(s)

diff  --git a/lldb/test/API/test_utils/TestPExpectTest.py 
b/lldb/test/API/test_utils/TestPExpectTest.py
new file mode 100644
index ..c754f1faa7ec
--- /dev/null
+++ b/lldb/test/API/test_utils/TestPExpectTest.py
@@ -0,0 +1,29 @@
+"""
+Test the PExpectTest test functions.
+"""
+
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+from textwrap import dedent
+
+
+class TestPExpectTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+def assert_expect_fails_with(self, cmd, expect_args, expected_msg):
+try:
+self.expect(cmd, **expect_args)
+except AssertionError as e:
+self.assertIn(expected_msg, str(e))
+else:
+self.fail("expect should have raised AssertionError!")
+
+def test_expect(self):
+# Test that passing a string to the 'substrs' argument is rejected.
+self.assert_expect_fails_with("settings list prompt",
+dict(substrs="some substring"),
+"substrs must be a collection of strings")



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

[Lldb-commits] [PATCH] D89302: [lldb] Also Catch invalid calls to TestPExpectTest's expect()

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2f84b59a4cf9: [lldb] Also Catch invalid calls to 
TestPExpectTest's expect() (authored by teemperor).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89302

Files:
  lldb/packages/Python/lldbsuite/test/lldbpexpect.py
  lldb/test/API/test_utils/TestPExpectTest.py


Index: lldb/test/API/test_utils/TestPExpectTest.py
===
--- /dev/null
+++ lldb/test/API/test_utils/TestPExpectTest.py
@@ -0,0 +1,29 @@
+"""
+Test the PExpectTest test functions.
+"""
+
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+from textwrap import dedent
+
+
+class TestPExpectTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+def assert_expect_fails_with(self, cmd, expect_args, expected_msg):
+try:
+self.expect(cmd, **expect_args)
+except AssertionError as e:
+self.assertIn(expected_msg, str(e))
+else:
+self.fail("expect should have raised AssertionError!")
+
+def test_expect(self):
+# Test that passing a string to the 'substrs' argument is rejected.
+self.assert_expect_fails_with("settings list prompt",
+dict(substrs="some substring"),
+"substrs must be a collection of strings")
Index: lldb/packages/Python/lldbsuite/test/lldbpexpect.py
===
--- lldb/packages/Python/lldbsuite/test/lldbpexpect.py
+++ lldb/packages/Python/lldbsuite/test/lldbpexpect.py
@@ -54,6 +54,10 @@
 def expect(self, cmd, substrs=None):
 self.assertNotIn('\n', cmd)
 self.child.sendline(cmd)
+# If 'substrs' is a string then this code would just check that every
+# character of the string is in the output.
+assert not isinstance(substrs, six.string_types), \
+"substrs must be a collection of strings"
 if substrs is not None:
 for s in substrs:
 self.child.expect_exact(s)


Index: lldb/test/API/test_utils/TestPExpectTest.py
===
--- /dev/null
+++ lldb/test/API/test_utils/TestPExpectTest.py
@@ -0,0 +1,29 @@
+"""
+Test the PExpectTest test functions.
+"""
+
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+from textwrap import dedent
+
+
+class TestPExpectTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+def assert_expect_fails_with(self, cmd, expect_args, expected_msg):
+try:
+self.expect(cmd, **expect_args)
+except AssertionError as e:
+self.assertIn(expected_msg, str(e))
+else:
+self.fail("expect should have raised AssertionError!")
+
+def test_expect(self):
+# Test that passing a string to the 'substrs' argument is rejected.
+self.assert_expect_fails_with("settings list prompt",
+dict(substrs="some substring"),
+"substrs must be a collection of strings")
Index: lldb/packages/Python/lldbsuite/test/lldbpexpect.py
===
--- lldb/packages/Python/lldbsuite/test/lldbpexpect.py
+++ lldb/packages/Python/lldbsuite/test/lldbpexpect.py
@@ -54,6 +54,10 @@
 def expect(self, cmd, substrs=None):
 self.assertNotIn('\n', cmd)
 self.child.sendline(cmd)
+# If 'substrs' is a string then this code would just check that every
+# character of the string is in the output.
+assert not isinstance(substrs, six.string_types), \
+"substrs must be a collection of strings"
 if substrs is not None:
 for s in substrs:
 self.child.expect_exact(s)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D89695: [lldb] Skip TestChangeProcessGroup on watchOS/tvOS

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG239f488fd692: [lldb] Skip TestChangeProcessGroup on 
watchOS/tvOS (authored by teemperor).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89695

Files:
  lldb/test/API/functionalities/process_group/TestChangeProcessGroup.py


Index: lldb/test/API/functionalities/process_group/TestChangeProcessGroup.py
===
--- lldb/test/API/functionalities/process_group/TestChangeProcessGroup.py
+++ lldb/test/API/functionalities/process_group/TestChangeProcessGroup.py
@@ -25,6 +25,8 @@
 @expectedFailureAndroid("http://llvm.org/pr23762";, api_levels=[16])
 @expectedFailureNetBSD
 @skipIfReproducer # File synchronization is not supported during replay.
+@skipIftvOS # fork not available on tvOS.
+@skipIfwatchOS # fork not available on watchOS.
 def test_setpgid(self):
 self.build()
 exe = self.getBuildArtifact("a.out")


Index: lldb/test/API/functionalities/process_group/TestChangeProcessGroup.py
===
--- lldb/test/API/functionalities/process_group/TestChangeProcessGroup.py
+++ lldb/test/API/functionalities/process_group/TestChangeProcessGroup.py
@@ -25,6 +25,8 @@
 @expectedFailureAndroid("http://llvm.org/pr23762";, api_levels=[16])
 @expectedFailureNetBSD
 @skipIfReproducer # File synchronization is not supported during replay.
+@skipIftvOS # fork not available on tvOS.
+@skipIfwatchOS # fork not available on watchOS.
 def test_setpgid(self):
 self.build()
 exe = 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] 239f488 - [lldb] Skip TestChangeProcessGroup on watchOS/tvOS

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-11-05T15:11:30+01:00
New Revision: 239f488fd692f2af506b2b45d335404d0d2ab30b

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

LOG: [lldb] Skip TestChangeProcessGroup on watchOS/tvOS

`fork` is marked as `__WATCHOS_PROHIBITED __TVOS_PROHIBITED` so the test source
which is calling fork will never compile on watchOS/tvOS. This just adds the
skip decorator for these platforms.

Reviewed By: mib

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

Added: 


Modified: 
lldb/test/API/functionalities/process_group/TestChangeProcessGroup.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/process_group/TestChangeProcessGroup.py 
b/lldb/test/API/functionalities/process_group/TestChangeProcessGroup.py
index 51c0ae75c1a3..75e0b9332fbe 100644
--- a/lldb/test/API/functionalities/process_group/TestChangeProcessGroup.py
+++ b/lldb/test/API/functionalities/process_group/TestChangeProcessGroup.py
@@ -25,6 +25,8 @@ def setUp(self):
 @expectedFailureAndroid("http://llvm.org/pr23762";, api_levels=[16])
 @expectedFailureNetBSD
 @skipIfReproducer # File synchronization is not supported during replay.
+@skipIftvOS # fork not available on tvOS.
+@skipIfwatchOS # fork not available on watchOS.
 def test_setpgid(self):
 self.build()
 exe = 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] d68ebea - Reland [lldb] Explicitly use the configuration architecture when building test executables

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-11-05T15:13:48+01:00
New Revision: d68ebea7670f6bbba136f1517a1cff3696b6a800

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

LOG: Reland [lldb] Explicitly use the configuration architecture when building 
test executables

This originally broke the TestQuoting which explicitly called buildDefault
instead of calling build() and marking the test as no_debug_info_test.
TestQuoting has been rewritten by now and is using `build`, so this should now
pass on all platforms.

Original summary:

The Darwin builder currently assumes in `getArchCFlags` that the passed `arch`
value is an actual string it can string.join with vendor/os/version/env strings:

```
   triple = '-'.join([arch, vendor, os, version, env])
```

However this is not true for most tests as we just pass down the `arch=None`
default value from `TestBase.build`. This causes that if we actually end up in
this function we just error out when concatenating `None` with the other actual
strings of vendor/os/version/env. What we should do instead is check that if
there is no test-specific architecture that we fall back to the configuration's
architecture value.

It seems we already worked around this in `builder.getArchSpec` by explicitly
falling back to the architecture specified in the configuration.

This patch just moves this fallback logic to the top `build` function so that it
affects all functions called from `TestBase.build`.

Reviewed By: JDevlieghere

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

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/builders/builder.py
lldb/packages/Python/lldbsuite/test/lldbtest.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/builders/builder.py 
b/lldb/packages/Python/lldbsuite/test/builders/builder.py
index fbfa86700e22..6c9584224f4a 100644
--- a/lldb/packages/Python/lldbsuite/test/builders/builder.py
+++ b/lldb/packages/Python/lldbsuite/test/builders/builder.py
@@ -93,11 +93,7 @@ def getArchSpec(self, architecture):
 Helper function to return the key-value string to specify the 
architecture
 used for the make system.
 """
-arch = architecture if architecture else None
-if not arch and configuration.arch:
-arch = configuration.arch
-
-return ("ARCH=" + arch) if arch else ""
+return ("ARCH=" + architecture) if architecture else ""
 
 def getCCSpec(self, compiler):
 """

diff  --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py 
b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index 81110f9f80c4..73007caf2108 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -2607,6 +2607,9 @@ def build(
 """Platform specific way to build the default binaries."""
 module = builder_module()
 
+if not architecture and configuration.arch:
+architecture = configuration.arch
+
 dictionary = lldbplatformutil.finalize_build_dictionary(dictionary)
 if self.getDebugInfo() is None:
 return self.buildDefault(architecture, compiler, dictionary)



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

[Lldb-commits] [lldb] 26a8e85 - [lldb] Add Apple simulator platforms to lldbplatform.py

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-11-05T15:34:42+01:00
New Revision: 26a8e8502b5943cc13177bea48841491dadfef9b

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

LOG: [lldb] Add Apple simulator platforms to lldbplatform.py

This just adds the simulator platforms to the lldbplatform enumerations
and the respective test decorator.

The platform names for the simulator are just the SDK names since D85537, so
that's why we are not using LLDB's usual platform names here (e.g., SDK =
"iphonesimulator" vs LLDB platform ="ios-simulator").

Also removes the duplicate platform enumaration in lldbplatformutil.py.

Reviewed By: JDevlieghere

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

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/decorators.py
lldb/packages/Python/lldbsuite/test/lldbplatform.py
lldb/packages/Python/lldbsuite/test/lldbplatformutil.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/decorators.py 
b/lldb/packages/Python/lldbsuite/test/decorators.py
index 1775c07c5b7a..358005dba70f 100644
--- a/lldb/packages/Python/lldbsuite/test/decorators.py
+++ b/lldb/packages/Python/lldbsuite/test/decorators.py
@@ -551,16 +551,16 @@ def is_ios_simulator():
 return skipTestIfFn(is_ios_simulator)(func)
 
 def skipIfiOS(func):
-return skipIfPlatform(["ios"])(func)
+return skipIfPlatform(lldbplatform.translate(lldbplatform.ios))(func)
 
 def skipIftvOS(func):
-return skipIfPlatform(["tvos"])(func)
+return skipIfPlatform(lldbplatform.translate(lldbplatform.tvos))(func)
 
 def skipIfwatchOS(func):
-return skipIfPlatform(["watchos"])(func)
+return skipIfPlatform(lldbplatform.translate(lldbplatform.watchos))(func)
 
 def skipIfbridgeOS(func):
-return skipIfPlatform(["bridgeos"])(func)
+return skipIfPlatform(lldbplatform.translate(lldbplatform.bridgeos))(func)
 
 def skipIfDarwinEmbedded(func):
 """Decorate the item to skip tests that should be skipped on Darwin 
armv7/arm64 targets."""
@@ -568,6 +568,12 @@ def skipIfDarwinEmbedded(func):
 lldbplatform.translate(
 lldbplatform.darwin_embedded))(func)
 
+def skipIfDarwinSimulator(func):
+"""Decorate the item to skip tests that should be skipped on Darwin 
simulator targets."""
+return skipIfPlatform(
+lldbplatform.translate(
+lldbplatform.darwin_simulator))(func)
+
 def skipIfFreeBSD(func):
 """Decorate the item to skip tests that should be skipped on FreeBSD."""
 return skipIfPlatform(["freebsd"])(func)

diff  --git a/lldb/packages/Python/lldbsuite/test/lldbplatform.py 
b/lldb/packages/Python/lldbsuite/test/lldbplatform.py
index 365c752758d8..18a4fe5754de 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbplatform.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbplatform.py
@@ -11,20 +11,25 @@
 # LLDB modules
 import lldb
 
-windows, linux, macosx, darwin, ios, tvos, watchos, bridgeos, darwin_all, 
darwin_embedded, freebsd, netbsd, bsd_all, android = range(
-14)
+windows, linux, macosx, darwin, ios, tvos, watchos, bridgeos, darwin_all, \
+darwin_embedded, darwin_simulator, freebsd, netbsd, bsd_all, android \
+= range(15)
+
+__darwin_embedded = ["ios", "tvos", "watchos", "bridgeos"]
+__darwin_simulators = ["iphonesimulator", "watchsimulator", "appletvsimulator"]
 
 __name_lookup = {
 windows: ["windows"],
 linux: ["linux"],
 macosx: ["macosx"],
 darwin: ["darwin"],
-ios: ["ios"],
-tvos: ["tvos"],
-watchos: ["watchos"],
+ios: ["ios", "iphonesimulator"],
+tvos: ["tvos", "appletvsimulator"],
+watchos: ["watchos", "watchsimulator"],
 bridgeos: ["bridgeos"],
-darwin_all: ["macosx", "darwin", "ios", "tvos", "watchos", "bridgeos"],
-darwin_embedded: ["ios", "tvos", "watchos", "bridgeos"],
+darwin_all: ["macosx", "darwin"] + __darwin_embedded + __darwin_simulators,
+darwin_embedded: __darwin_embedded + __darwin_simulators,
+darwin_simulator: __darwin_simulators,
 freebsd: ["freebsd"],
 netbsd: ["netbsd"],
 bsd_all: ["freebsd", "netbsd"],

diff  --git a/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py 
b/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
index cc21865e4751..3d6402c13b47 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
@@ -124,8 +124,7 @@ def getHostPlatform():
 
 
 def getDarwinOSTriples():
-return ['darwin', 'macosx', 'ios', 'watchos', 'tvos', 'bridgeos']
-
+return lldbplatform.translate(lldbplatform.darwin_all)
 
 def getPlatform():
 """Returns the target platform which the tests are running on."""



___
lldb-commits mailing list
lldb-commits@lists.llvm.o

[Lldb-commits] [PATCH] D89694: [lldb] Add Apple simulator platforms to lldbplatform.py

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG26a8e8502b59: [lldb] Add Apple simulator platforms to 
lldbplatform.py (authored by teemperor).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89694

Files:
  lldb/packages/Python/lldbsuite/test/decorators.py
  lldb/packages/Python/lldbsuite/test/lldbplatform.py
  lldb/packages/Python/lldbsuite/test/lldbplatformutil.py


Index: lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
===
--- lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
+++ lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
@@ -124,8 +124,7 @@
 
 
 def getDarwinOSTriples():
-return ['darwin', 'macosx', 'ios', 'watchos', 'tvos', 'bridgeos']
-
+return lldbplatform.translate(lldbplatform.darwin_all)
 
 def getPlatform():
 """Returns the target platform which the tests are running on."""
Index: lldb/packages/Python/lldbsuite/test/lldbplatform.py
===
--- lldb/packages/Python/lldbsuite/test/lldbplatform.py
+++ lldb/packages/Python/lldbsuite/test/lldbplatform.py
@@ -11,20 +11,25 @@
 # LLDB modules
 import lldb
 
-windows, linux, macosx, darwin, ios, tvos, watchos, bridgeos, darwin_all, 
darwin_embedded, freebsd, netbsd, bsd_all, android = range(
-14)
+windows, linux, macosx, darwin, ios, tvos, watchos, bridgeos, darwin_all, \
+darwin_embedded, darwin_simulator, freebsd, netbsd, bsd_all, android \
+= range(15)
+
+__darwin_embedded = ["ios", "tvos", "watchos", "bridgeos"]
+__darwin_simulators = ["iphonesimulator", "watchsimulator", "appletvsimulator"]
 
 __name_lookup = {
 windows: ["windows"],
 linux: ["linux"],
 macosx: ["macosx"],
 darwin: ["darwin"],
-ios: ["ios"],
-tvos: ["tvos"],
-watchos: ["watchos"],
+ios: ["ios", "iphonesimulator"],
+tvos: ["tvos", "appletvsimulator"],
+watchos: ["watchos", "watchsimulator"],
 bridgeos: ["bridgeos"],
-darwin_all: ["macosx", "darwin", "ios", "tvos", "watchos", "bridgeos"],
-darwin_embedded: ["ios", "tvos", "watchos", "bridgeos"],
+darwin_all: ["macosx", "darwin"] + __darwin_embedded + __darwin_simulators,
+darwin_embedded: __darwin_embedded + __darwin_simulators,
+darwin_simulator: __darwin_simulators,
 freebsd: ["freebsd"],
 netbsd: ["netbsd"],
 bsd_all: ["freebsd", "netbsd"],
Index: lldb/packages/Python/lldbsuite/test/decorators.py
===
--- lldb/packages/Python/lldbsuite/test/decorators.py
+++ lldb/packages/Python/lldbsuite/test/decorators.py
@@ -551,16 +551,16 @@
 return skipTestIfFn(is_ios_simulator)(func)
 
 def skipIfiOS(func):
-return skipIfPlatform(["ios"])(func)
+return skipIfPlatform(lldbplatform.translate(lldbplatform.ios))(func)
 
 def skipIftvOS(func):
-return skipIfPlatform(["tvos"])(func)
+return skipIfPlatform(lldbplatform.translate(lldbplatform.tvos))(func)
 
 def skipIfwatchOS(func):
-return skipIfPlatform(["watchos"])(func)
+return skipIfPlatform(lldbplatform.translate(lldbplatform.watchos))(func)
 
 def skipIfbridgeOS(func):
-return skipIfPlatform(["bridgeos"])(func)
+return skipIfPlatform(lldbplatform.translate(lldbplatform.bridgeos))(func)
 
 def skipIfDarwinEmbedded(func):
 """Decorate the item to skip tests that should be skipped on Darwin 
armv7/arm64 targets."""
@@ -568,6 +568,12 @@
 lldbplatform.translate(
 lldbplatform.darwin_embedded))(func)
 
+def skipIfDarwinSimulator(func):
+"""Decorate the item to skip tests that should be skipped on Darwin 
simulator targets."""
+return skipIfPlatform(
+lldbplatform.translate(
+lldbplatform.darwin_simulator))(func)
+
 def skipIfFreeBSD(func):
 """Decorate the item to skip tests that should be skipped on FreeBSD."""
 return skipIfPlatform(["freebsd"])(func)


Index: lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
===
--- lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
+++ lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
@@ -124,8 +124,7 @@
 
 
 def getDarwinOSTriples():
-return ['darwin', 'macosx', 'ios', 'watchos', 'tvos', 'bridgeos']
-
+return lldbplatform.translate(lldbplatform.darwin_all)
 
 def getPlatform():
 """Returns the target platform which the tests are running on."""
Index: lldb/packages/Python/lldbsuite/test/lldbplatform.py
===
--- lldb/packages/Python/lldbsuite/test/lldbplatform.py
+++ lldb/packages/Python/lldbsuite/test/lldbplatform.py
@@ -11,20 +11,25 @@
 # LLDB modules
 import lldb
 
-windows, linux, macosx, darwin, ios, tvos,

[Lldb-commits] [PATCH] D90757: [lldb] Enable FreeBSDRemote plugin by default and update test status

[Ed Maste via Phabricator via lldb-commits]

emaste accepted this revision.
emaste added a comment.

> What do you suggest? Is it fine to go with my results for as long as I work 
> on it? I can update it to match CI/buildbot results later.

Yes absolutely. I pasted my results mainly as an FYI/for comparison, since I 
recall there were many intermittent or flaky tests. I definitely want to see 
this go in and we can iterate on the individual annotations.


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

https://reviews.llvm.org/D90757

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

[Lldb-commits] [PATCH] D90757: [lldb] Enable FreeBSDRemote plugin by default and update test status

[Ed Maste via Phabricator via lldb-commits]

emaste added inline comments.



Comment at: lldb/packages/Python/lldbsuite/test/dotest.py:955
 
-# Don't do lldb-server (llgs) tests on anything except Linux and Windows.
+# Don't do lldb-server (llgs) tests on platforms not supporting it.
 configuration.dont_do_llgs_test = not (

mgorny wrote:
> emaste wrote:
> > It's a shame that these variables and the comments are inverted sense / 
> > double negatives, but not an issue in this patch.
> Yes, I had to look at it for a while to make sure I'm doing it right. I'll 
> change them in a followup commit.
Thanks.



Comment at: 
lldb/test/API/commands/register/register/register_command/TestRegisters.py:31
 @skipIf(archs=no_match(['amd64', 'arm', 'i386', 'x86_64']))
-@expectedFailureNetBSD
+@expectedFailureAll(oslist=["freebsd", "netbsd"])
 def test_register_commands(self):

mgorny wrote:
> emaste wrote:
> > there's no (existing) PR for the failure?
> No. I'm planning to file bugs during the next stage, after trying to fix the 
> trivial ones, if that's ok with you.
Yes absolutely. I was just curious because this was already failing for NetBSD



Comment at: 
lldb/test/API/functionalities/inferior-crashing/TestInferiorCrashingStep.py:54-55
 # intended IMHO.
 @skipIfLinux
-@skipIfFreeBSD
-@expectedFailureNetBSD
+@expectedFailureAll(oslist=["freebsd", "netbsd"])
 def test_inferior_crashing_expr_step_and_expr(self):

labath wrote:
> I'm pretty sure the root cause here is the same for net/free bsd as it is for 
> linux (it comes down to macos catching the "crashes" specially, before they 
> even get turned to a SEGV -- something that's not possible elsewhere). I 
> marked it skip because that's not something we should support, ever. I don't 
> care that much which decorator (skip vs. xfail) is used here, but I think 
> they should be consistent.
Sounds reasonable, we may want to change the comment above to make it clear 
this is an explicit decision.


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

https://reviews.llvm.org/D90757

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

[Lldb-commits] [lldb] b9b5f12 - [lldb] Set the default architecture also in buildDefault

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-11-05T16:32:05+01:00
New Revision: b9b5f12bd4cd0647ff630c1631e0cf20f430fa15

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

LOG: [lldb] Set the default architecture also in buildDefault

In D89056 the default value for architecture was moved to `build` so that
all called functions see the same architecture value. It seems there are a
few functions that call buildDefault directly (and not via build), so
on some test configurations that set a custom arch value the architecture
value is no longer available.

This just adds the architecture code from build to buildDefault to get
the bots green again while I'm looking for a better solution.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/lldbtest.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py 
b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index 73007caf2108..08f44c731383 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -1591,6 +1591,10 @@ def buildDefault(
 """Platform specific way to build the default binaries."""
 testdir = self.mydir
 testname = self.getBuildDirBasename()
+
+if not architecture and configuration.arch:
+architecture = configuration.arch
+
 if self.getDebugInfo():
 raise Exception("buildDefault tests must set 
NO_DEBUG_INFO_TESTCASE")
 module = builder_module()



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

[Lldb-commits] [lldb] 79d1676 - [lldb][NFC] Fix compiler warnings after removal of eValueTypeVector

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2020-11-05T17:17:33+01:00
New Revision: 79d16764dd29aeddb7e6400e6b2d89d31653886c

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

LOG: [lldb][NFC] Fix compiler warnings after removal of eValueTypeVector

5d64574301836c4c17127794121d49a62d24f803 removes this enum value and now
all the switch statements that previously relied on handling this in the
'default' branch are causes compiler warnings due to redundant default cases.

This just removes the now unreachable code in there.

Added: 


Modified: 
lldb/source/Core/Value.cpp
lldb/source/Core/ValueObjectChild.cpp
lldb/source/Core/ValueObjectMemory.cpp

Removed: 




diff  --git a/lldb/source/Core/Value.cpp b/lldb/source/Core/Value.cpp
index 8007df4f934e0..cc8f3f4e26153 100644
--- a/lldb/source/Core/Value.cpp
+++ b/lldb/source/Core/Value.cpp
@@ -113,7 +113,6 @@ Value::ValueType Value::GetValueType() const { return 
m_value_type; }
 
 AddressType Value::GetValueAddressType() const {
   switch (m_value_type) {
-  default:
   case eValueTypeScalar:
 break;
   case eValueTypeLoadAddress:
@@ -278,9 +277,6 @@ lldb::Format Value::GetValueDefaultFormat() {
 
 bool Value::GetData(DataExtractor &data) {
   switch (m_value_type) {
-  default:
-break;
-
   case eValueTypeScalar:
 if (m_value.GetData(data))
   return true;
@@ -571,7 +567,6 @@ Scalar &Value::ResolveValue(ExecutionContext *exe_ctx) {
 case eValueTypeScalar: // raw scalar value
   break;
 
-default:
 case eValueTypeFileAddress:
 case eValueTypeLoadAddress: // load address value
 case eValueTypeHostAddress: // host address value (for memory in the 
process

diff  --git a/lldb/source/Core/ValueObjectChild.cpp 
b/lldb/source/Core/ValueObjectChild.cpp
index 97974d7b98fb2..34baa19f0a248 100644
--- a/lldb/source/Core/ValueObjectChild.cpp
+++ b/lldb/source/Core/ValueObjectChild.cpp
@@ -190,9 +190,6 @@ bool ValueObjectChild::UpdateValue() {
   m_value.GetScalar() = scalar;
 }
 break;
-  default:
-m_error.SetErrorString("parent has invalid value.");
-break;
   }
 
   if (m_error.Success()) {

diff  --git a/lldb/source/Core/ValueObjectMemory.cpp 
b/lldb/source/Core/ValueObjectMemory.cpp
index 17fade9e5fdc3..abf7b38ed89ac 100644
--- a/lldb/source/Core/ValueObjectMemory.cpp
+++ b/lldb/source/Core/ValueObjectMemory.cpp
@@ -168,9 +168,6 @@ bool ValueObjectMemory::UpdateValue() {
 Value::ValueType value_type = m_value.GetValueType();
 
 switch (value_type) {
-default:
-  llvm_unreachable("Unhandled expression result value kind...");
-
 case Value::eValueTypeScalar:
   // The variable value is in the Scalar value inside the m_value. We can
   // point our m_data right to it.



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

[Lldb-commits] [PATCH] D90787: [LLDB-lua] modify Lua's 'print' to respect 'io.stdout'

[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/D90787/new/

https://reviews.llvm.org/D90787

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

[Lldb-commits] [lldb] 6ba2c2b - [lldb] [test/Shell] Simplify -pthread condition

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2020-11-05T17:49:20+01:00
New Revision: 6ba2c2bf90f23381c1d052acb010cee364bebe8e

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

LOG: [lldb] [test/Shell] Simplify -pthread condition

Pass -pthread on all systems except for Darwin and Windows.
Suggested by Pavel Labath.

Added: 


Modified: 
lldb/test/Shell/helper/toolchain.py

Removed: 




diff  --git a/lldb/test/Shell/helper/toolchain.py 
b/lldb/test/Shell/helper/toolchain.py
index cda66b652961..9b85da01f822 100644
--- a/lldb/test/Shell/helper/toolchain.py
+++ b/lldb/test/Shell/helper/toolchain.py
@@ -116,7 +116,7 @@ def use_support_substitutions(config):
 sdk_path = lit.util.to_string(out)
 llvm_config.lit_config.note('using SDKROOT: %r' % sdk_path)
 host_flags += ['-isysroot', sdk_path]
-elif platform.system() in ['FreeBSD', 'NetBSD', 'OpenBSD', 'Linux']:
+elif sys.platform != 'win32':
 host_flags += ['-pthread']
 
 if sys.platform.startswith('netbsd'):



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

[Lldb-commits] [lldb] 2c2eb5e - [lldb] Enable FreeBSDRemote plugin by default and update test status

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2020-11-05T17:49:46+01:00
New Revision: 2c2eb5e6702bf3bbb8fb8f09790b1ab7b139e879

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

LOG: [lldb] Enable FreeBSDRemote plugin by default and update test status

The new FreeBSDRemote plugin has reached feature parity on i386
and amd64 targets.  Use it by default on these architectures, while
allowing the use of the legacy plugin via FREEBSD_LEGACY_PLUGIN envvar.

Revisit the method of switching plugins.  Apparently, the return value
of PlatformFreeBSD::CanDebugProcess() is what really decides whether
the legacy or the new plugin is used.

Update the test status.  Reenable the tests that were previously
disabled on FreeBSD and do not cause hangs or are irrelevant to FreeBSD.
Mark all tests that fail reliably as expectedFailure.  For now, tests
that are flaky (i.e. produce unstable results) are left enabled
and cause unpredictable test failures.

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

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/dotest.py
lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp
lldb/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp
lldb/test/API/api/multiple-debuggers/TestMultipleDebuggers.py
lldb/test/API/commands/expression/call-restarts/TestCallThatRestarts.py
lldb/test/API/commands/expression/formatters/TestFormatters.py
lldb/test/API/commands/expression/no-deadlock/TestExprDoesntBlock.py
lldb/test/API/commands/register/register/register_command/TestRegisters.py

lldb/test/API/commands/watchpoints/multiple_threads/TestWatchpointMultipleThreads.py
lldb/test/API/functionalities/avoids-fd-leak/TestFdLeak.py

lldb/test/API/functionalities/data-formatter/data-formatter-python-synth/TestDataFormatterPythonSynth.py

lldb/test/API/functionalities/data-formatter/data-formatter-synthtype/TestDataFormatterSynthType.py

lldb/test/API/functionalities/data-formatter/data-formatter-synthval/TestDataFormatterSynthVal.py

lldb/test/API/functionalities/data-formatter/varscript_formatting/TestDataFormatterVarScriptFormatting.py
lldb/test/API/functionalities/deleted-executable/TestDeletedExecutable.py
lldb/test/API/functionalities/exec/TestExec.py
lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
lldb/test/API/functionalities/inferior-crashing/TestInferiorCrashingStep.py

lldb/test/API/functionalities/inferior-crashing/recursive-inferior/TestRecursiveInferiorStep.py
lldb/test/API/functionalities/load_unload/TestLoadUnload.py
lldb/test/API/functionalities/load_using_paths/TestLoadUsingPaths.py
lldb/test/API/functionalities/longjmp/TestLongjmp.py

lldb/test/API/functionalities/plugins/python_os_plugin/stepping_plugin_threads/TestOSPluginStepping.py
lldb/test/API/functionalities/signal/TestSendSignal.py
lldb/test/API/functionalities/signal/raise/TestRaise.py

lldb/test/API/functionalities/thread/create_after_attach/TestCreateAfterAttach.py
lldb/test/API/functionalities/thread/exit_during_step/TestExitDuringStep.py
lldb/test/API/functionalities/thread/state/TestThreadStates.py
lldb/test/API/lang/c/modules/TestCModules.py
lldb/test/API/lit.cfg.py
lldb/test/API/python_api/event/TestEvents.py
lldb/test/API/tools/lldb-server/TestGdbRemote_qThreadStopInfo.py
lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
lldb/test/API/tools/lldb-server/commandline/TestStubSetSID.py

lldb/test/API/tools/lldb-server/libraries-svr4/TestGdbRemoteLibrariesSvr4Support.py
lldb/test/API/tools/lldb-server/register-reading/TestGdbRemoteGPacket.py
lldb/test/API/tools/lldb-server/thread-name/TestGdbRemoteThreadName.py
lldb/test/Shell/ExecControl/StopHook/stop-hook-threads.test
lldb/test/Shell/Recognizer/assert.test
lldb/test/Shell/lit.cfg.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/dotest.py 
b/lldb/packages/Python/lldbsuite/test/dotest.py
index a98965e50c19..420c52a07427 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest.py
@@ -952,8 +952,9 @@ def run_suite():
 "netbsd" in target_platform or
 "windows" in target_platform)
 
-# Don't do lldb-server (llgs) tests on anything except Linux and Windows.
+# Don't do lldb-server (llgs) tests on platforms not supporting it.
 configuration.dont_do_llgs_test = not (
+"freebsd" in target_platform or
 "linux" in target_platform or
 "netbsd" in target_platform or
 "windows" in target_platform)

diff  --git a/lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp 
b/lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp
index a5ce513aa3

[Lldb-commits] [PATCH] D90757: [lldb] Enable FreeBSDRemote plugin by default and update test status

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

This revision was automatically updated to reflect the committed changes.
mgorny marked an inline comment as done.
Closed by commit rG2c2eb5e6702b: [lldb] Enable FreeBSDRemote plugin by default 
and update test status (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90757

Files:
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/source/Plugins/Platform/FreeBSD/PlatformFreeBSD.cpp
  lldb/source/Plugins/Process/FreeBSD/ProcessFreeBSD.cpp
  lldb/test/API/api/multiple-debuggers/TestMultipleDebuggers.py
  lldb/test/API/commands/expression/call-restarts/TestCallThatRestarts.py
  lldb/test/API/commands/expression/formatters/TestFormatters.py
  lldb/test/API/commands/expression/no-deadlock/TestExprDoesntBlock.py
  lldb/test/API/commands/register/register/register_command/TestRegisters.py
  
lldb/test/API/commands/watchpoints/multiple_threads/TestWatchpointMultipleThreads.py
  lldb/test/API/functionalities/avoids-fd-leak/TestFdLeak.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-python-synth/TestDataFormatterPythonSynth.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-synthtype/TestDataFormatterSynthType.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-synthval/TestDataFormatterSynthVal.py
  
lldb/test/API/functionalities/data-formatter/varscript_formatting/TestDataFormatterVarScriptFormatting.py
  lldb/test/API/functionalities/deleted-executable/TestDeletedExecutable.py
  lldb/test/API/functionalities/exec/TestExec.py
  lldb/test/API/functionalities/gdb_remote_client/TestProcessConnect.py
  lldb/test/API/functionalities/inferior-crashing/TestInferiorCrashingStep.py
  
lldb/test/API/functionalities/inferior-crashing/recursive-inferior/TestRecursiveInferiorStep.py
  lldb/test/API/functionalities/load_unload/TestLoadUnload.py
  lldb/test/API/functionalities/load_using_paths/TestLoadUsingPaths.py
  lldb/test/API/functionalities/longjmp/TestLongjmp.py
  
lldb/test/API/functionalities/plugins/python_os_plugin/stepping_plugin_threads/TestOSPluginStepping.py
  lldb/test/API/functionalities/signal/TestSendSignal.py
  lldb/test/API/functionalities/signal/raise/TestRaise.py
  
lldb/test/API/functionalities/thread/create_after_attach/TestCreateAfterAttach.py
  lldb/test/API/functionalities/thread/exit_during_step/TestExitDuringStep.py
  lldb/test/API/functionalities/thread/state/TestThreadStates.py
  lldb/test/API/lang/c/modules/TestCModules.py
  lldb/test/API/lit.cfg.py
  lldb/test/API/python_api/event/TestEvents.py
  lldb/test/API/tools/lldb-server/TestGdbRemote_qThreadStopInfo.py
  lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
  lldb/test/API/tools/lldb-server/commandline/TestStubSetSID.py
  
lldb/test/API/tools/lldb-server/libraries-svr4/TestGdbRemoteLibrariesSvr4Support.py
  lldb/test/API/tools/lldb-server/register-reading/TestGdbRemoteGPacket.py
  lldb/test/API/tools/lldb-server/thread-name/TestGdbRemoteThreadName.py
  lldb/test/Shell/ExecControl/StopHook/stop-hook-threads.test
  lldb/test/Shell/Recognizer/assert.test
  lldb/test/Shell/lit.cfg.py

Index: lldb/test/Shell/lit.cfg.py
===
--- lldb/test/Shell/lit.cfg.py
+++ lldb/test/Shell/lit.cfg.py
@@ -135,4 +135,4 @@
 config.available_features.add('dbregs-set')
 
 # pass control variable through
-llvm_config.with_system_environment('FREEBSD_REMOTE_PLUGIN')
+llvm_config.with_system_environment('FREEBSD_LEGACY_PLUGIN')
Index: lldb/test/Shell/Recognizer/assert.test
===
--- lldb/test/Shell/Recognizer/assert.test
+++ lldb/test/Shell/Recognizer/assert.test
@@ -1,4 +1,5 @@
 # XFAIL: target-arm && linux-gnu
+# XFAIL: system-freebsd
 # UNSUPPORTED: system-windows
 # RUN: %clang_host -g -O0 %S/Inputs/assert.c -o %t.out
 # RUN: %lldb -b -s %s %t.out | FileCheck %s
Index: lldb/test/Shell/ExecControl/StopHook/stop-hook-threads.test
===
--- lldb/test/Shell/ExecControl/StopHook/stop-hook-threads.test
+++ lldb/test/Shell/ExecControl/StopHook/stop-hook-threads.test
@@ -3,6 +3,7 @@
 # RUN: | FileCheck --check-prefix=CHECK --check-prefix=CHECK-NO-FILTER %s
 # RUN: %lldb -b -s %p/Inputs/stop-hook-threads-2.lldbinit -s %s -f %t \
 # RUN: | FileCheck --check-prefix=CHECK --check-prefix=CHECK-FILTER %s
+# XFAIL: system-freebsd
 # XFAIL: system-netbsd
 # UNSUPPORTED: system-windows
 # This test is flakey and hangs on windows periodically: llvm.org/pr38373
Index: lldb/test/API/tools/lldb-server/thread-name/TestGdbRemoteThreadName.py
===
--- lldb/test/API/tools/lldb-server/thread-name/TestGdbRemoteThreadName.py
+++ lldb/test/API/tools/lldb-server/thread-name/TestGdbRemoteThreadName.py
@@ -29,6 +29,7 @@
 self.assertEqual(expected_na

[Lldb-commits] [PATCH] D90862: [lldb] [test] Fix TestGdbRemoteThreadName code on FreeBSD

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

mgorny created this revision.
mgorny added reviewers: labath, krytarowski, emaste.
Herald added a subscriber: arichardson.
Herald added a reviewer: JDevlieghere.
mgorny requested review of this revision.

Fix TestGdbRemoteThreadName to call ::pthread_setname_np instead
of ::pthread_set_name_np on FreeBSD.  While technically both names
are correct, the former is preferable because of compatibility
with Linux.  Furthermore, the latter requires `#include `
that was missing causing the test to fail to compile.


https://reviews.llvm.org/D90862

Files:
  lldb/test/API/tools/lldb-server/thread-name/main.cpp


Index: lldb/test/API/tools/lldb-server/thread-name/main.cpp
===
--- lldb/test/API/tools/lldb-server/thread-name/main.cpp
+++ lldb/test/API/tools/lldb-server/thread-name/main.cpp
@@ -4,9 +4,7 @@
 void set_thread_name(const char *name) {
 #if defined(__APPLE__)
   ::pthread_setname_np(name);
-#elif defined(__FreeBSD__)
-  ::pthread_set_name_np(::pthread_self(), name);
-#elif defined(__linux__)
+#elif defined(__FreeBSD__) || defined(__linux__)
   ::pthread_setname_np(::pthread_self(), name);
 #elif defined(__NetBSD__)
   ::pthread_setname_np(::pthread_self(), "%s", const_cast(name));


Index: lldb/test/API/tools/lldb-server/thread-name/main.cpp
===
--- lldb/test/API/tools/lldb-server/thread-name/main.cpp
+++ lldb/test/API/tools/lldb-server/thread-name/main.cpp
@@ -4,9 +4,7 @@
 void set_thread_name(const char *name) {
 #if defined(__APPLE__)
   ::pthread_setname_np(name);
-#elif defined(__FreeBSD__)
-  ::pthread_set_name_np(::pthread_self(), name);
-#elif defined(__linux__)
+#elif defined(__FreeBSD__) || defined(__linux__)
   ::pthread_setname_np(::pthread_self(), name);
 #elif defined(__NetBSD__)
   ::pthread_setname_np(::pthread_self(), "%s", const_cast(name));
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D90863: [lldb] [Process/FreeBSDRemote] Remove thread name caching

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

mgorny created this revision.
mgorny added reviewers: krytarowski, emaste, labath.
Herald added a reviewer: JDevlieghere.
mgorny requested review of this revision.

Remove the thread name caching code.  It does not handle the possibility
of thread name changing between requests, therefore breaking
TestGdbRemoteThreadName.  While technically we could cache the results
and reset the cache on resuming process, the gain from doing that
does not seem worth the effort.


https://reviews.llvm.org/D90863

Files:
  lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.cpp
  lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.h
  lldb/test/API/tools/lldb-server/thread-name/TestGdbRemoteThreadName.py


Index: lldb/test/API/tools/lldb-server/thread-name/TestGdbRemoteThreadName.py
===
--- lldb/test/API/tools/lldb-server/thread-name/TestGdbRemoteThreadName.py
+++ lldb/test/API/tools/lldb-server/thread-name/TestGdbRemoteThreadName.py
@@ -29,7 +29,6 @@
 self.assertEqual(expected_name, kv_dict.get("name"))
 
 @skipIfWindows # the test is not updated for Windows.
-@expectedFailureAll(oslist=["freebsd"])
 @llgs_test
 def test(self):
 """ Make sure lldb-server can retrieve inferior thread name"""
Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.h
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.h
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.h
@@ -74,7 +74,6 @@
   using WatchpointIndexMap = std::map;
   WatchpointIndexMap m_watchpoint_index_map;
   WatchpointIndexMap m_hw_break_index_map;
-  llvm::Optional m_thread_name;
 };
 
 typedef std::shared_ptr NativeThreadFreeBSDSP;
Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.cpp
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.cpp
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.cpp
@@ -149,41 +149,35 @@
 }
 
 std::string NativeThreadFreeBSD::GetName() {
-  if (!m_thread_name) {
-Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_THREAD));
-
-std::vector kp;
-int mib[4] = {CTL_KERN, KERN_PROC, KERN_PROC_PID | KERN_PROC_INC_THREAD,
-  static_cast(GetProcess().GetID())};
-
-while (1) {
-  size_t len = kp.size() * sizeof(struct kinfo_proc);
-  void *ptr = len == 0 ? nullptr : kp.data();
-  int error = ::sysctl(mib, 4, ptr, &len, nullptr, 0);
-  if (ptr == nullptr || (error != 0 && errno == ENOMEM)) {
-kp.resize(len / sizeof(struct kinfo_proc));
-continue;
-  }
-  if (error != 0) {
-len = 0;
-LLDB_LOG(log, "tid = {0} in state {1} failed to get thread name: {2}", 
GetID(),
- m_state, strerror(errno));
-  }
+  Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_THREAD));
+
+  std::vector kp;
+  int mib[4] = {CTL_KERN, KERN_PROC, KERN_PROC_PID | KERN_PROC_INC_THREAD,
+static_cast(GetProcess().GetID())};
+
+  while (1) {
+size_t len = kp.size() * sizeof(struct kinfo_proc);
+void *ptr = len == 0 ? nullptr : kp.data();
+int error = ::sysctl(mib, 4, ptr, &len, nullptr, 0);
+if (ptr == nullptr || (error != 0 && errno == ENOMEM)) {
   kp.resize(len / sizeof(struct kinfo_proc));
-  break;
+  continue;
 }
-
-// empty == unknown
-m_thread_name = std::string();
-for (auto& procinfo : kp) {
-  if (procinfo.ki_tid == (lwpid_t)GetID()) {
-m_thread_name = procinfo.ki_tdname;
-break;
-  }
+if (error != 0) {
+  len = 0;
+  LLDB_LOG(log, "tid = {0} in state {1} failed to get thread name: {2}", 
GetID(),
+   m_state, strerror(errno));
 }
+kp.resize(len / sizeof(struct kinfo_proc));
+break;
+  }
+
+  for (auto& procinfo : kp) {
+if (procinfo.ki_tid == static_cast(GetID()))
+  return procinfo.ki_tdname;
   }
 
-  return m_thread_name.getValue();
+  return "";
 }
 
 lldb::StateType NativeThreadFreeBSD::GetState() { return m_state; }


Index: lldb/test/API/tools/lldb-server/thread-name/TestGdbRemoteThreadName.py
===
--- lldb/test/API/tools/lldb-server/thread-name/TestGdbRemoteThreadName.py
+++ lldb/test/API/tools/lldb-server/thread-name/TestGdbRemoteThreadName.py
@@ -29,7 +29,6 @@
 self.assertEqual(expected_name, kv_dict.get("name"))
 
 @skipIfWindows # the test is not updated for Windows.
-@expectedFailureAll(oslist=["freebsd"])
 @llgs_test
 def test(self):
 """ Make sure lldb-server can retrieve inferior thread name"""
Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.h
===
--- lldb/source/Plugins/Process/FreeBSDRemote/Native

[Lldb-commits] [PATCH] D90862: [lldb] [test] Fix TestGdbRemoteThreadName code on FreeBSD

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

mgorny added a comment.

Note that D90863  is also required to fix the 
test.


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

https://reviews.llvm.org/D90862

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

[Lldb-commits] [PATCH] D90769: [lldb][ObjectFile] Relocate sections for in-memory objects (e.g. received via JITLoaderGDB)

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz added a comment.

Thanks for having a look!

The JIT implementations in LLVM operate on relocatable object files, so someone 
needs to resolve them. MCJIT has a flag `ProcessAllSections` to control which 
sections will be loaded and relocated. As it is off by default, it will usually 
ignore sections that it doesn't need for execution. LLDB does it the other way 
around in ObjectFileELF::RelocateSection(), it only processes sections with the 
".debug" prefix. This seems to be a reasonable distribution of tasks. In 
general, relocations are resolved in place, so it doesn't rise memory 
consumption.

> I do wonder though if the jit could be changed to avoid relocation.

Well, I don't think we are anywhere close to optimizations like this, but it 
would be nice indeed. If we compile from bitcode on the JIT side, we could 
lookup external symbols at compile-time and don't produce relocations for them 
in the first place. I guess it would heavily reduce the number of relocations 
and potentially save time. On the other hand, thinking about concurrent compile 
jobs and cross-dependencies.. I can imagine it gets hairy quickly. Plus: the 
way it is now, we can cache the object files and reuse them thanks to 
position-independent code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90769

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

[Lldb-commits] [PATCH] D90863: [lldb] [Process/FreeBSDRemote] Remove thread name caching

[Ed Maste via Phabricator via lldb-commits]

emaste added a comment.

Does Linux fetch each time also?
I agree it's probably not worth the effort.


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

https://reviews.llvm.org/D90863

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

[Lldb-commits] [PATCH] D90863: [lldb] [Process/FreeBSDRemote] Remove thread name caching

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

mgorny added a comment.

In D90863#2376742 , @emaste wrote:

> Does Linux fetch each time also?
> I agree it's probably not worth the effort.

Yes. However, their logic is simpler (it boils down to opening a dedicated file 
in procfs (i.e. they don't have to iterate over all threads). Nevertheless, I 
don't think this is called frequently enough to justify optimization at this 
point.


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

https://reviews.llvm.org/D90863

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

[Lldb-commits] [PATCH] D90862: [lldb] [test] Fix TestGdbRemoteThreadName code on FreeBSD

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb643deb03fb9: [lldb] [test] Fix TestGdbRemoteThreadName code 
on FreeBSD (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90862

Files:
  lldb/test/API/tools/lldb-server/thread-name/main.cpp


Index: lldb/test/API/tools/lldb-server/thread-name/main.cpp
===
--- lldb/test/API/tools/lldb-server/thread-name/main.cpp
+++ lldb/test/API/tools/lldb-server/thread-name/main.cpp
@@ -4,9 +4,7 @@
 void set_thread_name(const char *name) {
 #if defined(__APPLE__)
   ::pthread_setname_np(name);
-#elif defined(__FreeBSD__)
-  ::pthread_set_name_np(::pthread_self(), name);
-#elif defined(__linux__)
+#elif defined(__FreeBSD__) || defined(__linux__)
   ::pthread_setname_np(::pthread_self(), name);
 #elif defined(__NetBSD__)
   ::pthread_setname_np(::pthread_self(), "%s", const_cast(name));


Index: lldb/test/API/tools/lldb-server/thread-name/main.cpp
===
--- lldb/test/API/tools/lldb-server/thread-name/main.cpp
+++ lldb/test/API/tools/lldb-server/thread-name/main.cpp
@@ -4,9 +4,7 @@
 void set_thread_name(const char *name) {
 #if defined(__APPLE__)
   ::pthread_setname_np(name);
-#elif defined(__FreeBSD__)
-  ::pthread_set_name_np(::pthread_self(), name);
-#elif defined(__linux__)
+#elif defined(__FreeBSD__) || defined(__linux__)
   ::pthread_setname_np(::pthread_self(), name);
 #elif defined(__NetBSD__)
   ::pthread_setname_np(::pthread_self(), "%s", const_cast(name));
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] b643deb - [lldb] [test] Fix TestGdbRemoteThreadName code on FreeBSD

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2020-11-05T20:45:34+01:00
New Revision: b643deb03fb935d414f74e07b702ebb4e5c33bf3

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

LOG: [lldb] [test] Fix TestGdbRemoteThreadName code on FreeBSD

Fix TestGdbRemoteThreadName to call ::pthread_setname_np instead
of ::pthread_set_name_np on FreeBSD.  While technically both names
are correct, the former is preferable because of compatibility
with Linux.  Furthermore, the latter requires `#include `
that was missing causing the test to fail to compile.

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

Added: 


Modified: 
lldb/test/API/tools/lldb-server/thread-name/main.cpp

Removed: 




diff  --git a/lldb/test/API/tools/lldb-server/thread-name/main.cpp 
b/lldb/test/API/tools/lldb-server/thread-name/main.cpp
index 898e9a35e9ac..02eea12ca98a 100644
--- a/lldb/test/API/tools/lldb-server/thread-name/main.cpp
+++ b/lldb/test/API/tools/lldb-server/thread-name/main.cpp
@@ -4,9 +4,7 @@
 void set_thread_name(const char *name) {
 #if defined(__APPLE__)
   ::pthread_setname_np(name);
-#elif defined(__FreeBSD__)
-  ::pthread_set_name_np(::pthread_self(), name);
-#elif defined(__linux__)
+#elif defined(__FreeBSD__) || defined(__linux__)
   ::pthread_setname_np(::pthread_self(), name);
 #elif defined(__NetBSD__)
   ::pthread_setname_np(::pthread_self(), "%s", const_cast(name));



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

[Lldb-commits] [PATCH] D90863: [lldb] [Process/FreeBSDRemote] Remove thread name caching

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rG40140e122f8b: [lldb] [Process/FreeBSDRemote] Remove thread 
name caching (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90863

Files:
  lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.cpp
  lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.h
  lldb/test/API/tools/lldb-server/thread-name/TestGdbRemoteThreadName.py


Index: lldb/test/API/tools/lldb-server/thread-name/TestGdbRemoteThreadName.py
===
--- lldb/test/API/tools/lldb-server/thread-name/TestGdbRemoteThreadName.py
+++ lldb/test/API/tools/lldb-server/thread-name/TestGdbRemoteThreadName.py
@@ -29,7 +29,6 @@
 self.assertEqual(expected_name, kv_dict.get("name"))
 
 @skipIfWindows # the test is not updated for Windows.
-@expectedFailureAll(oslist=["freebsd"])
 @llgs_test
 def test(self):
 """ Make sure lldb-server can retrieve inferior thread name"""
Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.h
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.h
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.h
@@ -74,7 +74,6 @@
   using WatchpointIndexMap = std::map;
   WatchpointIndexMap m_watchpoint_index_map;
   WatchpointIndexMap m_hw_break_index_map;
-  llvm::Optional m_thread_name;
 };
 
 typedef std::shared_ptr NativeThreadFreeBSDSP;
Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.cpp
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.cpp
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.cpp
@@ -149,41 +149,35 @@
 }
 
 std::string NativeThreadFreeBSD::GetName() {
-  if (!m_thread_name) {
-Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_THREAD));
-
-std::vector kp;
-int mib[4] = {CTL_KERN, KERN_PROC, KERN_PROC_PID | KERN_PROC_INC_THREAD,
-  static_cast(GetProcess().GetID())};
-
-while (1) {
-  size_t len = kp.size() * sizeof(struct kinfo_proc);
-  void *ptr = len == 0 ? nullptr : kp.data();
-  int error = ::sysctl(mib, 4, ptr, &len, nullptr, 0);
-  if (ptr == nullptr || (error != 0 && errno == ENOMEM)) {
-kp.resize(len / sizeof(struct kinfo_proc));
-continue;
-  }
-  if (error != 0) {
-len = 0;
-LLDB_LOG(log, "tid = {0} in state {1} failed to get thread name: {2}", 
GetID(),
- m_state, strerror(errno));
-  }
+  Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_THREAD));
+
+  std::vector kp;
+  int mib[4] = {CTL_KERN, KERN_PROC, KERN_PROC_PID | KERN_PROC_INC_THREAD,
+static_cast(GetProcess().GetID())};
+
+  while (1) {
+size_t len = kp.size() * sizeof(struct kinfo_proc);
+void *ptr = len == 0 ? nullptr : kp.data();
+int error = ::sysctl(mib, 4, ptr, &len, nullptr, 0);
+if (ptr == nullptr || (error != 0 && errno == ENOMEM)) {
   kp.resize(len / sizeof(struct kinfo_proc));
-  break;
+  continue;
 }
-
-// empty == unknown
-m_thread_name = std::string();
-for (auto& procinfo : kp) {
-  if (procinfo.ki_tid == (lwpid_t)GetID()) {
-m_thread_name = procinfo.ki_tdname;
-break;
-  }
+if (error != 0) {
+  len = 0;
+  LLDB_LOG(log, "tid = {0} in state {1} failed to get thread name: {2}", 
GetID(),
+   m_state, strerror(errno));
 }
+kp.resize(len / sizeof(struct kinfo_proc));
+break;
+  }
+
+  for (auto& procinfo : kp) {
+if (procinfo.ki_tid == static_cast(GetID()))
+  return procinfo.ki_tdname;
   }
 
-  return m_thread_name.getValue();
+  return "";
 }
 
 lldb::StateType NativeThreadFreeBSD::GetState() { return m_state; }


Index: lldb/test/API/tools/lldb-server/thread-name/TestGdbRemoteThreadName.py
===
--- lldb/test/API/tools/lldb-server/thread-name/TestGdbRemoteThreadName.py
+++ lldb/test/API/tools/lldb-server/thread-name/TestGdbRemoteThreadName.py
@@ -29,7 +29,6 @@
 self.assertEqual(expected_name, kv_dict.get("name"))
 
 @skipIfWindows # the test is not updated for Windows.
-@expectedFailureAll(oslist=["freebsd"])
 @llgs_test
 def test(self):
 """ Make sure lldb-server can retrieve inferior thread name"""
Index: lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.h
===
--- lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.h
+++ lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.h
@@ -74,7 +74,6 @@
   using WatchpointIndexMap = std::map;
   W

[Lldb-commits] [lldb] 40140e1 - [lldb] [Process/FreeBSDRemote] Remove thread name caching

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2020-11-05T20:45:34+01:00
New Revision: 40140e122f8b6512ebe22efc32dacf14f10117f6

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

LOG: [lldb] [Process/FreeBSDRemote] Remove thread name caching

Remove the thread name caching code.  It does not handle the possibility
of thread name changing between requests, therefore breaking
TestGdbRemoteThreadName.  While technically we could cache the results
and reset the cache on resuming process, the gain from doing that
does not seem worth the effort.

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

Added: 


Modified: 
lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.cpp
lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.h
lldb/test/API/tools/lldb-server/thread-name/TestGdbRemoteThreadName.py

Removed: 




diff  --git a/lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.cpp 
b/lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.cpp
index 2e62b0a25ed2..3c80f113b197 100644
--- a/lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.cpp
+++ b/lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.cpp
@@ -149,41 +149,35 @@ void NativeThreadFreeBSD::SetStepping() {
 }
 
 std::string NativeThreadFreeBSD::GetName() {
-  if (!m_thread_name) {
-Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_THREAD));
-
-std::vector kp;
-int mib[4] = {CTL_KERN, KERN_PROC, KERN_PROC_PID | KERN_PROC_INC_THREAD,
-  static_cast(GetProcess().GetID())};
-
-while (1) {
-  size_t len = kp.size() * sizeof(struct kinfo_proc);
-  void *ptr = len == 0 ? nullptr : kp.data();
-  int error = ::sysctl(mib, 4, ptr, &len, nullptr, 0);
-  if (ptr == nullptr || (error != 0 && errno == ENOMEM)) {
-kp.resize(len / sizeof(struct kinfo_proc));
-continue;
-  }
-  if (error != 0) {
-len = 0;
-LLDB_LOG(log, "tid = {0} in state {1} failed to get thread name: {2}", 
GetID(),
- m_state, strerror(errno));
-  }
+  Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_THREAD));
+
+  std::vector kp;
+  int mib[4] = {CTL_KERN, KERN_PROC, KERN_PROC_PID | KERN_PROC_INC_THREAD,
+static_cast(GetProcess().GetID())};
+
+  while (1) {
+size_t len = kp.size() * sizeof(struct kinfo_proc);
+void *ptr = len == 0 ? nullptr : kp.data();
+int error = ::sysctl(mib, 4, ptr, &len, nullptr, 0);
+if (ptr == nullptr || (error != 0 && errno == ENOMEM)) {
   kp.resize(len / sizeof(struct kinfo_proc));
-  break;
+  continue;
 }
-
-// empty == unknown
-m_thread_name = std::string();
-for (auto& procinfo : kp) {
-  if (procinfo.ki_tid == (lwpid_t)GetID()) {
-m_thread_name = procinfo.ki_tdname;
-break;
-  }
+if (error != 0) {
+  len = 0;
+  LLDB_LOG(log, "tid = {0} in state {1} failed to get thread name: {2}", 
GetID(),
+   m_state, strerror(errno));
 }
+kp.resize(len / sizeof(struct kinfo_proc));
+break;
+  }
+
+  for (auto& procinfo : kp) {
+if (procinfo.ki_tid == static_cast(GetID()))
+  return procinfo.ki_tdname;
   }
 
-  return m_thread_name.getValue();
+  return "";
 }
 
 lldb::StateType NativeThreadFreeBSD::GetState() { return m_state; }

diff  --git a/lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.h 
b/lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.h
index 665e4ea48971..e4d494174736 100644
--- a/lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.h
+++ b/lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.h
@@ -74,7 +74,6 @@ class NativeThreadFreeBSD : public NativeThreadProtocol {
   using WatchpointIndexMap = std::map;
   WatchpointIndexMap m_watchpoint_index_map;
   WatchpointIndexMap m_hw_break_index_map;
-  llvm::Optional m_thread_name;
 };
 
 typedef std::shared_ptr NativeThreadFreeBSDSP;

diff  --git 
a/lldb/test/API/tools/lldb-server/thread-name/TestGdbRemoteThreadName.py 
b/lldb/test/API/tools/lldb-server/thread-name/TestGdbRemoteThreadName.py
index c4f08da7099c..9ec40c117428 100644
--- a/lldb/test/API/tools/lldb-server/thread-name/TestGdbRemoteThreadName.py
+++ b/lldb/test/API/tools/lldb-server/thread-name/TestGdbRemoteThreadName.py
@@ -29,7 +29,6 @@ def run_and_check_name(self, expected_name):
 self.assertEqual(expected_name, kv_dict.get("name"))
 
 @skipIfWindows # the test is not updated for Windows.
-@expectedFailureAll(oslist=["freebsd"])
 @llgs_test
 def test(self):
 """ Make sure lldb-server can retrieve inferior thread name"""



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/

[Lldb-commits] [PATCH] D90872: [TargetList] Simplify dummy target creation

[Vedant Kumar via Phabricator via lldb-commits]

vsk created this revision.
vsk added reviewers: teemperor, JDevlieghere, jingham.
Herald added a project: LLDB.
vsk requested review of this revision.

Factor out dummy target creation from CreateTargetInternal.

This makes it impossible for dummy target creation to accidentally fail
due to too-strict checking in one of the CreateTargetInternal overloads.

Testing: check-lldb

rdar://70630655


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90872

Files:
  lldb/include/lldb/Target/TargetList.h
  lldb/source/Target/TargetList.cpp

Index: lldb/source/Target/TargetList.cpp
===
--- lldb/source/Target/TargetList.cpp
+++ lldb/source/Target/TargetList.cpp
@@ -55,8 +55,8 @@
 const OptionGroupPlatform *platform_options,
 TargetSP &target_sp) {
   return CreateTargetInternal(debugger, user_exe_path, triple_str,
-  load_dependent_files, platform_options, target_sp,
-  false);
+  load_dependent_files, platform_options,
+  target_sp);
 }
 
 Status TargetList::CreateTarget(Debugger &debugger,
@@ -65,15 +65,24 @@
 LoadDependentFiles load_dependent_files,
 PlatformSP &platform_sp, TargetSP &target_sp) {
   return CreateTargetInternal(debugger, user_exe_path, specified_arch,
-  load_dependent_files, platform_sp, target_sp,
-  false);
+  load_dependent_files, platform_sp, target_sp);
+}
+
+void TargetList::CreateDummyTarget(Debugger &debugger, ArchSpec arch,
+   lldb::TargetSP &target_sp) {
+  assert(!m_dummy_target_sp && "attempted to create a second dummy target");
+  const bool is_dummy_target = true;
+  PlatformSP host_platform_sp(Platform::GetHostPlatform());
+  target_sp.reset(
+  new Target(debugger, arch, host_platform_sp, is_dummy_target));
+  // Don't put the dummy target in the target list, it's held separately.
+  m_dummy_target_sp = target_sp;
 }
 
 Status TargetList::CreateTargetInternal(
 Debugger &debugger, llvm::StringRef user_exe_path,
 llvm::StringRef triple_str, LoadDependentFiles load_dependent_files,
-const OptionGroupPlatform *platform_options, TargetSP &target_sp,
-bool is_dummy_target) {
+const OptionGroupPlatform *platform_options, TargetSP &target_sp) {
   Status error;
 
   // Let's start by looking at the selected platform.
@@ -256,7 +265,7 @@
   if (!prefer_platform_arch && arch.IsValid()) {
 if (!platform_sp->IsCompatibleArchitecture(arch, false, nullptr)) {
   platform_sp = Platform::GetPlatformForArchitecture(arch, &platform_arch);
-  if (!is_dummy_target && platform_sp)
+  if (platform_sp)
 debugger.GetPlatformList().SetSelectedPlatform(platform_sp);
 }
   } else if (platform_arch.IsValid()) {
@@ -266,7 +275,7 @@
 if (!platform_sp->IsCompatibleArchitecture(platform_arch, false, nullptr)) {
   platform_sp = Platform::GetPlatformForArchitecture(platform_arch,
  &fixed_platform_arch);
-  if (!is_dummy_target && platform_sp)
+  if (platform_sp)
 debugger.GetPlatformList().SetSelectedPlatform(platform_sp);
 }
   }
@@ -274,9 +283,9 @@
   if (!platform_arch.IsValid())
 platform_arch = arch;
 
-  return TargetList::CreateTargetInternal(
-  debugger, user_exe_path, platform_arch, load_dependent_files, platform_sp,
-  target_sp, is_dummy_target);
+  return TargetList::CreateTargetInternal(debugger, user_exe_path,
+  platform_arch, load_dependent_files,
+  platform_sp, target_sp);
 }
 
 lldb::TargetSP TargetList::GetDummyTarget(lldb_private::Debugger &debugger) {
@@ -285,34 +294,25 @@
 ArchSpec arch(Target::GetDefaultArchitecture());
 if (!arch.IsValid())
   arch = HostInfo::GetArchitecture();
-Status err = CreateDummyTarget(
-debugger, arch.GetTriple().getTriple().c_str(), m_dummy_target_sp);
+assert(arch.IsValid() && "No valid default or host archspec");
+CreateDummyTarget(debugger, arch, m_dummy_target_sp);
   }
 
   return m_dummy_target_sp;
 }
 
-Status TargetList::CreateDummyTarget(Debugger &debugger,
- llvm::StringRef specified_arch_name,
- lldb::TargetSP &target_sp) {
-  PlatformSP host_platform_sp(Platform::GetHostPlatform());
-  return CreateTargetInternal(
-  debugger, (const char *)nullptr, specified_arch_name, eLoadDependentsNo,
-  (const OptionGroupPlatform *)nullptr, target_sp, true);
-}
-
 Status TargetList::CreateTargetInternal(Debugger &debugger,
 llvm::StringRef user_exe_path,
   

[Lldb-commits] [PATCH] D90872: [TargetList] Simplify dummy target creation

[Vedant Kumar via Phabricator via lldb-commits]

vsk updated this revision to Diff 303223.
vsk added a comment.

Delete some dead code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90872

Files:
  lldb/include/lldb/Target/TargetList.h
  lldb/source/Target/TargetList.cpp

Index: lldb/source/Target/TargetList.cpp
===
--- lldb/source/Target/TargetList.cpp
+++ lldb/source/Target/TargetList.cpp
@@ -55,8 +55,8 @@
 const OptionGroupPlatform *platform_options,
 TargetSP &target_sp) {
   return CreateTargetInternal(debugger, user_exe_path, triple_str,
-  load_dependent_files, platform_options, target_sp,
-  false);
+  load_dependent_files, platform_options,
+  target_sp);
 }
 
 Status TargetList::CreateTarget(Debugger &debugger,
@@ -65,15 +65,24 @@
 LoadDependentFiles load_dependent_files,
 PlatformSP &platform_sp, TargetSP &target_sp) {
   return CreateTargetInternal(debugger, user_exe_path, specified_arch,
-  load_dependent_files, platform_sp, target_sp,
-  false);
+  load_dependent_files, platform_sp, target_sp);
+}
+
+void TargetList::CreateDummyTarget(Debugger &debugger, ArchSpec arch,
+   lldb::TargetSP &target_sp) {
+  assert(!m_dummy_target_sp && "attempted to create a second dummy target");
+  const bool is_dummy_target = true;
+  PlatformSP host_platform_sp(Platform::GetHostPlatform());
+  target_sp.reset(
+  new Target(debugger, arch, host_platform_sp, is_dummy_target));
+  // Don't put the dummy target in the target list, it's held separately.
+  m_dummy_target_sp = target_sp;
 }
 
 Status TargetList::CreateTargetInternal(
 Debugger &debugger, llvm::StringRef user_exe_path,
 llvm::StringRef triple_str, LoadDependentFiles load_dependent_files,
-const OptionGroupPlatform *platform_options, TargetSP &target_sp,
-bool is_dummy_target) {
+const OptionGroupPlatform *platform_options, TargetSP &target_sp) {
   Status error;
 
   // Let's start by looking at the selected platform.
@@ -256,7 +265,7 @@
   if (!prefer_platform_arch && arch.IsValid()) {
 if (!platform_sp->IsCompatibleArchitecture(arch, false, nullptr)) {
   platform_sp = Platform::GetPlatformForArchitecture(arch, &platform_arch);
-  if (!is_dummy_target && platform_sp)
+  if (platform_sp)
 debugger.GetPlatformList().SetSelectedPlatform(platform_sp);
 }
   } else if (platform_arch.IsValid()) {
@@ -266,7 +275,7 @@
 if (!platform_sp->IsCompatibleArchitecture(platform_arch, false, nullptr)) {
   platform_sp = Platform::GetPlatformForArchitecture(platform_arch,
  &fixed_platform_arch);
-  if (!is_dummy_target && platform_sp)
+  if (platform_sp)
 debugger.GetPlatformList().SetSelectedPlatform(platform_sp);
 }
   }
@@ -274,45 +283,36 @@
   if (!platform_arch.IsValid())
 platform_arch = arch;
 
-  return TargetList::CreateTargetInternal(
-  debugger, user_exe_path, platform_arch, load_dependent_files, platform_sp,
-  target_sp, is_dummy_target);
+  return TargetList::CreateTargetInternal(debugger, user_exe_path,
+  platform_arch, load_dependent_files,
+  platform_sp, target_sp);
 }
 
 lldb::TargetSP TargetList::GetDummyTarget(lldb_private::Debugger &debugger) {
   // FIXME: Maybe the dummy target should be per-Debugger
-  if (!m_dummy_target_sp || !m_dummy_target_sp->IsValid()) {
+  if (!m_dummy_target_sp) {
 ArchSpec arch(Target::GetDefaultArchitecture());
 if (!arch.IsValid())
   arch = HostInfo::GetArchitecture();
-Status err = CreateDummyTarget(
-debugger, arch.GetTriple().getTriple().c_str(), m_dummy_target_sp);
+assert(arch.IsValid() && "No valid default or host archspec");
+CreateDummyTarget(debugger, arch, m_dummy_target_sp);
   }
 
   return m_dummy_target_sp;
 }
 
-Status TargetList::CreateDummyTarget(Debugger &debugger,
- llvm::StringRef specified_arch_name,
- lldb::TargetSP &target_sp) {
-  PlatformSP host_platform_sp(Platform::GetHostPlatform());
-  return CreateTargetInternal(
-  debugger, (const char *)nullptr, specified_arch_name, eLoadDependentsNo,
-  (const OptionGroupPlatform *)nullptr, target_sp, true);
-}
-
 Status TargetList::CreateTargetInternal(Debugger &debugger,
 llvm::StringRef user_exe_path,
 const ArchSpec &specified_arch,
 Lo

[Lldb-commits] [PATCH] D90875: [lldb] [test] Avoid double negatives in llgs/debugserver logic

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

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

Use positive logic (i.e. llgs_platform/debugserver_platform) for
indicating which platforms use the particular server variant.
Deduplicate the lists — it is rather expected that none of the platforms
using LLGS would use debugserver.


https://reviews.llvm.org/D90875

Files:
  lldb/packages/Python/lldbsuite/test/decorators.py
  lldb/packages/Python/lldbsuite/test/dotest.py


Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -945,19 +945,15 @@
 checkWatchpointSupport()
 checkDebugInfoSupport()
 
-# Don't do debugserver tests on anything except OS X.
-configuration.dont_do_debugserver_test = (
-"linux" in target_platform or
-"freebsd" in target_platform or
-"netbsd" in target_platform or
-"windows" in target_platform)
-
-# Don't do lldb-server (llgs) tests on platforms not supporting it.
-configuration.dont_do_llgs_test = not (
-"freebsd" in target_platform or
-"linux" in target_platform or
-"netbsd" in target_platform or
-"windows" in target_platform)
+# Perform LLGS tests only on platforms using it.
+configuration.llgs_platform = (
+target_platform in ["linux", "freebsd", "netbsd", "windows"])
+
+# Perform debugserver tests on Darwin platforms.  Currently, this
+# means all platform that do not use LLGS but additional platforms
+# may be excluded in the future.
+configuration.debugserver_platform = (
+not configuration.llgs_platform)
 
 for testdir in configuration.testdirs:
 for (dirpath, dirnames, filenames) in os.walk(testdir):
Index: lldb/packages/Python/lldbsuite/test/decorators.py
===
--- lldb/packages/Python/lldbsuite/test/decorators.py
+++ lldb/packages/Python/lldbsuite/test/decorators.py
@@ -375,14 +375,18 @@
 def debugserver_test(func):
 """Decorate the item as a debugserver test."""
 def should_skip_debugserver_test():
-return "debugserver tests" if configuration.dont_do_debugserver_test 
else None
+return ("debugserver tests"
+if not configuration.debugserver_platform
+else None)
 return skipTestIfFn(should_skip_debugserver_test)(func)
 
 
 def llgs_test(func):
 """Decorate the item as a lldb-server test."""
 def should_skip_llgs_tests():
-return "llgs tests" if configuration.dont_do_llgs_test else None
+return ("llgs tests"
+if not configuration.llgs_platform
+else None)
 return skipTestIfFn(should_skip_llgs_tests)(func)
 
 


Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -945,19 +945,15 @@
 checkWatchpointSupport()
 checkDebugInfoSupport()
 
-# Don't do debugserver tests on anything except OS X.
-configuration.dont_do_debugserver_test = (
-"linux" in target_platform or
-"freebsd" in target_platform or
-"netbsd" in target_platform or
-"windows" in target_platform)
-
-# Don't do lldb-server (llgs) tests on platforms not supporting it.
-configuration.dont_do_llgs_test = not (
-"freebsd" in target_platform or
-"linux" in target_platform or
-"netbsd" in target_platform or
-"windows" in target_platform)
+# Perform LLGS tests only on platforms using it.
+configuration.llgs_platform = (
+target_platform in ["linux", "freebsd", "netbsd", "windows"])
+
+# Perform debugserver tests on Darwin platforms.  Currently, this
+# means all platform that do not use LLGS but additional platforms
+# may be excluded in the future.
+configuration.debugserver_platform = (
+not configuration.llgs_platform)
 
 for testdir in configuration.testdirs:
 for (dirpath, dirnames, filenames) in os.walk(testdir):
Index: lldb/packages/Python/lldbsuite/test/decorators.py
===
--- lldb/packages/Python/lldbsuite/test/decorators.py
+++ lldb/packages/Python/lldbsuite/test/decorators.py
@@ -375,14 +375,18 @@
 def debugserver_test(func):
 """Decorate the item as a debugserver test."""
 def should_skip_debugserver_test():
-return "debugserver tests" if configuration.dont_do_debugserver_test else None
+return ("debugserver tests"
+if not configuration.debugserver_platform
+else None)
 return skipTestIfFn(should_skip_debugserver_te

[Lldb-commits] [PATCH] D90875: [lldb] [test] Avoid double negatives in llgs/debugserver logic

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

mgorny added inline comments.



Comment at: lldb/packages/Python/lldbsuite/test/dotest.py:953
+# Perform debugserver tests on Darwin platforms.  Currently, this
+# means all platform that do not use LLGS but additional platforms
+# may be excluded in the future.

@labath, can we assume that all future platforms will use LLGS?


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

https://reviews.llvm.org/D90875

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

[Lldb-commits] [PATCH] D90876: [lldb] [test] Improve comment on expr-after-step-after-crash tests

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

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

Make the comment indicate more strongly that this behavior
is Darwin-specific and it is not a bug for other platforms to behave
differently.


https://reviews.llvm.org/D90876

Files:
  lldb/test/API/functionalities/inferior-crashing/TestInferiorCrashingStep.py
  
lldb/test/API/functionalities/inferior-crashing/recursive-inferior/TestRecursiveInferiorStep.py


Index: 
lldb/test/API/functionalities/inferior-crashing/recursive-inferior/TestRecursiveInferiorStep.py
===
--- 
lldb/test/API/functionalities/inferior-crashing/recursive-inferior/TestRecursiveInferiorStep.py
+++ 
lldb/test/API/functionalities/inferior-crashing/recursive-inferior/TestRecursiveInferiorStep.py
@@ -25,8 +25,8 @@
 self.build()
 self.recursive_inferior_crashing_step_after_break()
 
-# Inferior exits after stepping after a segfault. This is working as
-# intended IMHO.
+# A test for Darwin-specific behavior.  On other platforms, inferior
+# exits after stepping after a segfault.
 @skipIf(oslist=["freebsd", "linux", "netbsd"])
 def test_recursive_inferior_crashing_expr_step_and_expr(self):
 """Test that lldb expressions work before and after stepping after a 
crash."""
Index: 
lldb/test/API/functionalities/inferior-crashing/TestInferiorCrashingStep.py
===
--- lldb/test/API/functionalities/inferior-crashing/TestInferiorCrashingStep.py
+++ lldb/test/API/functionalities/inferior-crashing/TestInferiorCrashingStep.py
@@ -49,8 +49,8 @@
 self.build()
 self.inferior_crashing_step_after_break()
 
-# Inferior exits after stepping after a segfault. This is working as
-# intended IMHO.
+# A test for Darwin-specific behavior.  On other platforms, inferior
+# exits after stepping after a segfault.
 @skipIf(oslist=["freebsd", "linux", "netbsd"])
 def test_inferior_crashing_expr_step_and_expr(self):
 """Test that lldb expressions work before and after stepping after a 
crash."""


Index: lldb/test/API/functionalities/inferior-crashing/recursive-inferior/TestRecursiveInferiorStep.py
===
--- lldb/test/API/functionalities/inferior-crashing/recursive-inferior/TestRecursiveInferiorStep.py
+++ lldb/test/API/functionalities/inferior-crashing/recursive-inferior/TestRecursiveInferiorStep.py
@@ -25,8 +25,8 @@
 self.build()
 self.recursive_inferior_crashing_step_after_break()
 
-# Inferior exits after stepping after a segfault. This is working as
-# intended IMHO.
+# A test for Darwin-specific behavior.  On other platforms, inferior
+# exits after stepping after a segfault.
 @skipIf(oslist=["freebsd", "linux", "netbsd"])
 def test_recursive_inferior_crashing_expr_step_and_expr(self):
 """Test that lldb expressions work before and after stepping after a crash."""
Index: lldb/test/API/functionalities/inferior-crashing/TestInferiorCrashingStep.py
===
--- lldb/test/API/functionalities/inferior-crashing/TestInferiorCrashingStep.py
+++ lldb/test/API/functionalities/inferior-crashing/TestInferiorCrashingStep.py
@@ -49,8 +49,8 @@
 self.build()
 self.inferior_crashing_step_after_break()
 
-# Inferior exits after stepping after a segfault. This is working as
-# intended IMHO.
+# A test for Darwin-specific behavior.  On other platforms, inferior
+# exits after stepping after a segfault.
 @skipIf(oslist=["freebsd", "linux", "netbsd"])
 def test_inferior_crashing_expr_step_and_expr(self):
 """Test that lldb expressions work before and after stepping after a crash."""
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D90872: [TargetList] Simplify dummy target creation

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/source/Target/TargetList.cpp:293
   // FIXME: Maybe the dummy target should be per-Debugger
-  if (!m_dummy_target_sp || !m_dummy_target_sp->IsValid()) {
+  if (!m_dummy_target_sp) {
 ArchSpec arch(Target::GetDefaultArchitecture());

I wonder if we gain anything by making this lazy. Maybe it's time to address 
that FIXME and always have a single DummyTarget held onto by the Debugger? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90872

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

[Lldb-commits] [lldb] ca17571 - [LLDB-lua] modify Lua's 'print' to respect 'io.stdout'

[Pedro Tammela via lldb-commits]

Author: Pedro Tammela
Date: 2020-11-05T21:23:20Z
New Revision: ca17571051d4e0a63e702371984dbd3671261f79

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

LOG: [LLDB-lua] modify Lua's 'print' to respect 'io.stdout'

This patch changes the implementation of Lua's `print()` function to
respect `io.stdout`.

The original implementation uses `lua_writestring()` internally, which is
hardcoded to `stdout`.

Reviewed By: JDevlieghere

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

Added: 
lldb/test/Shell/ScriptInterpreter/Lua/print.test

Modified: 
lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h

Removed: 




diff  --git a/lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp 
b/lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
index 2db44f2d29d0..dc3fd84a3bfb 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
@@ -14,6 +14,34 @@
 using namespace lldb_private;
 using namespace lldb;
 
+static int lldb_print(lua_State *L) {
+  int n = lua_gettop(L);
+  lua_getglobal(L, "io");
+  lua_getfield(L, -1, "stdout");
+  lua_getfield(L, -1, "write");
+  for (int i = 1; i <= n; i++) {
+lua_pushvalue(L, -1); // write()
+lua_pushvalue(L, -3); // io.stdout
+luaL_tolstring(L, i, nullptr);
+lua_pushstring(L, i != n ? "\t" : "\n");
+lua_call(L, 3, 0);
+  }
+  return 0;
+}
+
+Lua::Lua() : m_lua_state(luaL_newstate()) {
+  assert(m_lua_state);
+  luaL_openlibs(m_lua_state);
+  luaopen_lldb(m_lua_state);
+  lua_pushcfunction(m_lua_state, lldb_print);
+  lua_setglobal(m_lua_state, "print");
+}
+
+Lua::~Lua() {
+  assert(m_lua_state);
+  lua_close(m_lua_state);
+}
+
 llvm::Error Lua::Run(llvm::StringRef buffer) {
   int error =
   luaL_loadbuffer(m_lua_state, buffer.data(), buffer.size(), "buffer") ||

diff  --git a/lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h 
b/lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
index ff055d0cbb9e..83f836d8d78a 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
+++ b/lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
@@ -25,16 +25,8 @@ int luaopen_lldb(lua_State *L);
 
 class Lua {
 public:
-  Lua() : m_lua_state(luaL_newstate()) {
-assert(m_lua_state);
-luaL_openlibs(m_lua_state);
-luaopen_lldb(m_lua_state);
-  }
-
-  ~Lua() {
-assert(m_lua_state);
-lua_close(m_lua_state);
-  }
+  Lua();
+  ~Lua();
 
   llvm::Error Run(llvm::StringRef buffer);
   llvm::Error LoadModule(llvm::StringRef filename);

diff  --git a/lldb/test/Shell/ScriptInterpreter/Lua/print.test 
b/lldb/test/Shell/ScriptInterpreter/Lua/print.test
new file mode 100644
index ..fd457ecccec1
--- /dev/null
+++ b/lldb/test/Shell/ScriptInterpreter/Lua/print.test
@@ -0,0 +1,23 @@
+# REQUIRES: lua
+# UNSUPPORTED: lldb-repro
+#
+# RUN: rm -rf %t.stderr %t.stdout
+# RUN: cat %s | %lldb --script-language lua 2> %t.stderr > %t.stdout
+# RUN: cat %t.stdout | FileCheck %s --check-prefix STDOUT
+# RUN: cat %t.stderr | FileCheck %s --check-prefix STDERR
+script
+file = lldb.SBFile(2, "w", false)
+lldb.debugger:SetOutputFile(file)
+print(95000 + 126, nil, 'a')
+quit
+script
+print({})
+quit
+
+# STDOUT: 95126nil a
+# STDOUT-NOT: table: {{0x[[:xdigit:]]+}}
+# STDERR: table: {{0x[[:xdigit:]]+}}
+
+# RUN: rm -rf %t.stderr %t.stdout
+# RUN: %lldb --script-language lua -o 'script print(95000 + 126, nil, "a")' 2> 
%t.stderr > %t.stdout
+# RUN: cat %t.stdout | FileCheck %s --check-prefix STDOUT



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

[Lldb-commits] [PATCH] D90787: [LLDB-lua] modify Lua's 'print' to respect 'io.stdout'

[Pedro Tammela 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 rGca17571051d4: [LLDB-lua] modify Lua's 'print' 
to respect 'io.stdout' (authored by tammela).

Changed prior to commit:
  https://reviews.llvm.org/D90787?vs=303088&id=303249#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90787

Files:
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
  lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
  lldb/test/Shell/ScriptInterpreter/Lua/print.test


Index: lldb/test/Shell/ScriptInterpreter/Lua/print.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/print.test
@@ -0,0 +1,23 @@
+# REQUIRES: lua
+# UNSUPPORTED: lldb-repro
+#
+# RUN: rm -rf %t.stderr %t.stdout
+# RUN: cat %s | %lldb --script-language lua 2> %t.stderr > %t.stdout
+# RUN: cat %t.stdout | FileCheck %s --check-prefix STDOUT
+# RUN: cat %t.stderr | FileCheck %s --check-prefix STDERR
+script
+file = lldb.SBFile(2, "w", false)
+lldb.debugger:SetOutputFile(file)
+print(95000 + 126, nil, 'a')
+quit
+script
+print({})
+quit
+
+# STDOUT: 95126nil a
+# STDOUT-NOT: table: {{0x[[:xdigit:]]+}}
+# STDERR: table: {{0x[[:xdigit:]]+}}
+
+# RUN: rm -rf %t.stderr %t.stdout
+# RUN: %lldb --script-language lua -o 'script print(95000 + 126, nil, "a")' 2> 
%t.stderr > %t.stdout
+# RUN: cat %t.stdout | FileCheck %s --check-prefix STDOUT
Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
@@ -25,16 +25,8 @@
 
 class Lua {
 public:
-  Lua() : m_lua_state(luaL_newstate()) {
-assert(m_lua_state);
-luaL_openlibs(m_lua_state);
-luaopen_lldb(m_lua_state);
-  }
-
-  ~Lua() {
-assert(m_lua_state);
-lua_close(m_lua_state);
-  }
+  Lua();
+  ~Lua();
 
   llvm::Error Run(llvm::StringRef buffer);
   llvm::Error LoadModule(llvm::StringRef filename);
Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
@@ -14,6 +14,34 @@
 using namespace lldb_private;
 using namespace lldb;
 
+static int lldb_print(lua_State *L) {
+  int n = lua_gettop(L);
+  lua_getglobal(L, "io");
+  lua_getfield(L, -1, "stdout");
+  lua_getfield(L, -1, "write");
+  for (int i = 1; i <= n; i++) {
+lua_pushvalue(L, -1); // write()
+lua_pushvalue(L, -3); // io.stdout
+luaL_tolstring(L, i, nullptr);
+lua_pushstring(L, i != n ? "\t" : "\n");
+lua_call(L, 3, 0);
+  }
+  return 0;
+}
+
+Lua::Lua() : m_lua_state(luaL_newstate()) {
+  assert(m_lua_state);
+  luaL_openlibs(m_lua_state);
+  luaopen_lldb(m_lua_state);
+  lua_pushcfunction(m_lua_state, lldb_print);
+  lua_setglobal(m_lua_state, "print");
+}
+
+Lua::~Lua() {
+  assert(m_lua_state);
+  lua_close(m_lua_state);
+}
+
 llvm::Error Lua::Run(llvm::StringRef buffer) {
   int error =
   luaL_loadbuffer(m_lua_state, buffer.data(), buffer.size(), "buffer") ||


Index: lldb/test/Shell/ScriptInterpreter/Lua/print.test
===
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Lua/print.test
@@ -0,0 +1,23 @@
+# REQUIRES: lua
+# UNSUPPORTED: lldb-repro
+#
+# RUN: rm -rf %t.stderr %t.stdout
+# RUN: cat %s | %lldb --script-language lua 2> %t.stderr > %t.stdout
+# RUN: cat %t.stdout | FileCheck %s --check-prefix STDOUT
+# RUN: cat %t.stderr | FileCheck %s --check-prefix STDERR
+script
+file = lldb.SBFile(2, "w", false)
+lldb.debugger:SetOutputFile(file)
+print(95000 + 126, nil, 'a')
+quit
+script
+print({})
+quit
+
+# STDOUT: 95126	nil	a
+# STDOUT-NOT: table: {{0x[[:xdigit:]]+}}
+# STDERR: table: {{0x[[:xdigit:]]+}}
+
+# RUN: rm -rf %t.stderr %t.stdout
+# RUN: %lldb --script-language lua -o 'script print(95000 + 126, nil, "a")' 2> %t.stderr > %t.stdout
+# RUN: cat %t.stdout | FileCheck %s --check-prefix STDOUT
Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
+++ lldb/source/Plugins/ScriptInterpreter/Lua/Lua.h
@@ -25,16 +25,8 @@
 
 class Lua {
 public:
-  Lua() : m_lua_state(luaL_newstate()) {
-assert(m_lua_state);
-luaL_openlibs(m_lua_state);
-luaopen_lldb(m_lua_state);
-  }
-
-  ~Lua() {
-assert(m_lua_state);
-lua_close(m_lua_state);
-  }
+  Lua();
+  ~Lua();
 
   llvm::Error Run(llvm::StringRef buffer);
   llvm::Error LoadModule(llvm::StringRef filename);
Index: lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Lua/

[Lldb-commits] [PATCH] D90872: [TargetList] Simplify dummy target creation

[Vedant Kumar via Phabricator via lldb-commits]

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



Comment at: lldb/source/Target/TargetList.cpp:293
   // FIXME: Maybe the dummy target should be per-Debugger
-  if (!m_dummy_target_sp || !m_dummy_target_sp->IsValid()) {
+  if (!m_dummy_target_sp) {
 ArchSpec arch(Target::GetDefaultArchitecture());

JDevlieghere wrote:
> I wonder if we gain anything by making this lazy. Maybe it's time to address 
> that FIXME and always have a single DummyTarget held onto by the Debugger? 
You're right, that's even better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90872

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

[Lldb-commits] [PATCH] D90872: [TargetList] Simplify dummy target creation

[Vedant Kumar via Phabricator via lldb-commits]

vsk updated this revision to Diff 303252.
vsk marked an inline comment as done.
vsk added a comment.

Make the dummy target per-debugger.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90872

Files:
  lldb/include/lldb/Target/Target.h
  lldb/include/lldb/Target/TargetList.h
  lldb/source/Core/Debugger.cpp
  lldb/source/Target/TargetList.cpp

Index: lldb/source/Target/TargetList.cpp
===
--- lldb/source/Target/TargetList.cpp
+++ lldb/source/Target/TargetList.cpp
@@ -55,8 +55,8 @@
 const OptionGroupPlatform *platform_options,
 TargetSP &target_sp) {
   return CreateTargetInternal(debugger, user_exe_path, triple_str,
-  load_dependent_files, platform_options, target_sp,
-  false);
+  load_dependent_files, platform_options,
+  target_sp);
 }
 
 Status TargetList::CreateTarget(Debugger &debugger,
@@ -65,15 +65,13 @@
 LoadDependentFiles load_dependent_files,
 PlatformSP &platform_sp, TargetSP &target_sp) {
   return CreateTargetInternal(debugger, user_exe_path, specified_arch,
-  load_dependent_files, platform_sp, target_sp,
-  false);
+  load_dependent_files, platform_sp, target_sp);
 }
 
 Status TargetList::CreateTargetInternal(
 Debugger &debugger, llvm::StringRef user_exe_path,
 llvm::StringRef triple_str, LoadDependentFiles load_dependent_files,
-const OptionGroupPlatform *platform_options, TargetSP &target_sp,
-bool is_dummy_target) {
+const OptionGroupPlatform *platform_options, TargetSP &target_sp) {
   Status error;
 
   // Let's start by looking at the selected platform.
@@ -256,7 +254,7 @@
   if (!prefer_platform_arch && arch.IsValid()) {
 if (!platform_sp->IsCompatibleArchitecture(arch, false, nullptr)) {
   platform_sp = Platform::GetPlatformForArchitecture(arch, &platform_arch);
-  if (!is_dummy_target && platform_sp)
+  if (platform_sp)
 debugger.GetPlatformList().SetSelectedPlatform(platform_sp);
 }
   } else if (platform_arch.IsValid()) {
@@ -266,7 +264,7 @@
 if (!platform_sp->IsCompatibleArchitecture(platform_arch, false, nullptr)) {
   platform_sp = Platform::GetPlatformForArchitecture(platform_arch,
  &fixed_platform_arch);
-  if (!is_dummy_target && platform_sp)
+  if (platform_sp)
 debugger.GetPlatformList().SetSelectedPlatform(platform_sp);
 }
   }
@@ -274,31 +272,9 @@
   if (!platform_arch.IsValid())
 platform_arch = arch;
 
-  return TargetList::CreateTargetInternal(
-  debugger, user_exe_path, platform_arch, load_dependent_files, platform_sp,
-  target_sp, is_dummy_target);
-}
-
-lldb::TargetSP TargetList::GetDummyTarget(lldb_private::Debugger &debugger) {
-  // FIXME: Maybe the dummy target should be per-Debugger
-  if (!m_dummy_target_sp || !m_dummy_target_sp->IsValid()) {
-ArchSpec arch(Target::GetDefaultArchitecture());
-if (!arch.IsValid())
-  arch = HostInfo::GetArchitecture();
-Status err = CreateDummyTarget(
-debugger, arch.GetTriple().getTriple().c_str(), m_dummy_target_sp);
-  }
-
-  return m_dummy_target_sp;
-}
-
-Status TargetList::CreateDummyTarget(Debugger &debugger,
- llvm::StringRef specified_arch_name,
- lldb::TargetSP &target_sp) {
-  PlatformSP host_platform_sp(Platform::GetHostPlatform());
-  return CreateTargetInternal(
-  debugger, (const char *)nullptr, specified_arch_name, eLoadDependentsNo,
-  (const OptionGroupPlatform *)nullptr, target_sp, true);
+  return TargetList::CreateTargetInternal(debugger, user_exe_path,
+  platform_arch, load_dependent_files,
+  platform_sp, target_sp);
 }
 
 Status TargetList::CreateTargetInternal(Debugger &debugger,
@@ -306,13 +282,13 @@
 const ArchSpec &specified_arch,
 LoadDependentFiles load_dependent_files,
 lldb::PlatformSP &platform_sp,
-lldb::TargetSP &target_sp,
-bool is_dummy_target) {
+lldb::TargetSP &target_sp) {
   static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
   Timer scoped_timer(
   func_cat, "TargetList::CreateTarget (file = '%s', arch = '%s')",
   user_exe_path.str().c_str(), specified_arch.GetArchitectureName());
   Status error;
+  const bool is_dummy_target = false;
 
   Arc

[Lldb-commits] [PATCH] D90872: [TargetList] Simplify dummy target creation

[Jim Ingham via Phabricator via lldb-commits]

jingham added inline comments.



Comment at: lldb/source/Target/TargetList.cpp:293
   // FIXME: Maybe the dummy target should be per-Debugger
-  if (!m_dummy_target_sp || !m_dummy_target_sp->IsValid()) {
+  if (!m_dummy_target_sp) {
 ArchSpec arch(Target::GetDefaultArchitecture());

JDevlieghere wrote:
> I wonder if we gain anything by making this lazy. Maybe it's time to address 
> that FIXME and always have a single DummyTarget held onto by the Debugger? 
I see no reason not to just make a dummy target when you make a debugger.  That 
should be a very cheap operation, and the object itself should not be large.

I think for regularities sake it's better to have the TargetList have all the 
targets, including the Dummy target, rather than have the DummyTarget held 
specially by the debugger.  But it would be easy for the Debugger to make a 
DummyTarget and inject it into the TargetList as part of its startup.  We could 
even make the straight constructor of TargetList do this job.  We copy the 
debugger TargetList to iterate over it and for similar purposes in a bunch of 
places, but that goes through the copy constructor so it wouldn't interfere 
with this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90872

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

[Lldb-commits] [PATCH] D90872: [TargetList] Simplify dummy target creation

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

Oh, my bad, apparently we were leaving the dummy target out of the TargetList.  
That's a little odd, but I probably did it so that's not so unusual...  Ignore 
my comments...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90872

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

[Lldb-commits] [PATCH] D90872: [TargetList] Simplify dummy target creation

[Vedant Kumar via Phabricator via lldb-commits]

vsk added inline comments.



Comment at: lldb/source/Target/TargetList.cpp:293
   // FIXME: Maybe the dummy target should be per-Debugger
-  if (!m_dummy_target_sp || !m_dummy_target_sp->IsValid()) {
+  if (!m_dummy_target_sp) {
 ArchSpec arch(Target::GetDefaultArchitecture());

vsk wrote:
> jingham wrote:
> > JDevlieghere wrote:
> > > I wonder if we gain anything by making this lazy. Maybe it's time to 
> > > address that FIXME and always have a single DummyTarget held onto by the 
> > > Debugger? 
> > I see no reason not to just make a dummy target when you make a debugger.  
> > That should be a very cheap operation, and the object itself should not be 
> > large.
> > 
> > I think for regularities sake it's better to have the TargetList have all 
> > the targets, including the Dummy target, rather than have the DummyTarget 
> > held specially by the debugger.  But it would be easy for the Debugger to 
> > make a DummyTarget and inject it into the TargetList as part of its 
> > startup.  We could even make the straight constructor of TargetList do this 
> > job.  We copy the debugger TargetList to iterate over it and for similar 
> > purposes in a bunch of places, but that goes through the copy constructor 
> > so it wouldn't interfere with this.
> You're right, that's even better.
So far, the policy has been to not include the dummy target in the list of 
targets (i.e, you can't find it via TargetList::GetTargetAtIndex or 
TargetList::GetIndexOfTarget). If, down the line, we need to support that we 
can add the functionality though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90872

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

[Lldb-commits] [PATCH] D90872: [TargetList] Simplify dummy target creation

[Raphael Isemann via Phabricator via lldb-commits]

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

LGTM.




Comment at: lldb/source/Core/Debugger.cpp:685
 
-  m_dummy_target_sp = m_target_list.GetDummyTarget(*this);
+  {
+ArchSpec arch(Target::GetDefaultArchitecture());

Maybe have a small comment here that summarizes the block: `// Create dummy 
target`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90872

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

[Lldb-commits] [PATCH] D90872: [TargetList] Simplify dummy target creation

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/source/Core/Debugger.cpp:692
+m_dummy_target_sp.reset(
+new Target(*this, arch, default_platform_sp, is_dummy_target));
+  }

Should we move all this into `Target` and have a private static `TargetSP 
GetDummyTarget()`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90872

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

[Lldb-commits] [PATCH] D90872: [TargetList] Simplify dummy target creation

[Vedant Kumar via Phabricator via lldb-commits]

vsk added inline comments.



Comment at: lldb/source/Core/Debugger.cpp:685
 
-  m_dummy_target_sp = m_target_list.GetDummyTarget(*this);
+  {
+ArchSpec arch(Target::GetDefaultArchitecture());

teemperor wrote:
> Maybe have a small comment here that summarizes the block: `// Create dummy 
> target`?
I'll either fix this in a more substantial patch revision or before committing.



Comment at: lldb/source/Core/Debugger.cpp:692
+m_dummy_target_sp.reset(
+new Target(*this, arch, default_platform_sp, is_dummy_target));
+  }

JDevlieghere wrote:
> Should we move all this into `Target` and have a private static `TargetSP 
> GetDummyTarget()`?
I personally find that having the target creation inlined in the Debugger 
constructor is nice: it precludes some other code from setting up a separate 
dummy target.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90872

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

[Lldb-commits] [PATCH] D90876: [lldb] [test] Improve comment on expr-after-step-after-crash tests

[Ed Maste via Phabricator via lldb-commits]

emaste added a comment.

How does Windows fit into this? Other than that Q, LGTM.


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

https://reviews.llvm.org/D90876

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

[Lldb-commits] [PATCH] D90876: [lldb] [test] Improve comment on expr-after-step-after-crash tests

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

mgorny added a comment.

In D90876#2377541 , @emaste wrote:

> How does Windows fit into this? Other than that Q, LGTM.

@labath, any clue?


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

https://reviews.llvm.org/D90876

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

[Lldb-commits] [PATCH] D89283: [trace][intel-pt] Implement the basic decoding functionality

[Greg Clayton via Phabricator via lldb-commits]

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

I am good too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89283

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

[Lldb-commits] [lldb] 16e5a34 - [TargetList] Simplify dummy target creation

[Vedant Kumar via lldb-commits]

Author: Vedant Kumar
Date: 2020-11-05T16:04:02-08:00
New Revision: 16e5a347e70b4c819ff4402320457cc2c8ca

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

LOG: [TargetList] Simplify dummy target creation

Factor out dummy target creation from CreateTargetInternal.

This makes it impossible for dummy target creation to accidentally fail
due to too-strict checking in one of the CreateTargetInternal overloads.

Testing: check-lldb

rdar://70630655

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

Added: 


Modified: 
lldb/include/lldb/Target/Target.h
lldb/include/lldb/Target/TargetList.h
lldb/source/Core/Debugger.cpp
lldb/source/Target/TargetList.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/Target.h 
b/lldb/include/lldb/Target/Target.h
index 6a392a508f4c..41c07b7aa83f 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -441,6 +441,7 @@ class Target : public std::enable_shared_from_this,
public ModuleList::Notifier {
 public:
   friend class TargetList;
+  friend class Debugger;
 
   /// Broadcaster event bits definitions.
   enum {

diff  --git a/lldb/include/lldb/Target/TargetList.h 
b/lldb/include/lldb/Target/TargetList.h
index 5ed0344f175c..ff7fbd2753ab 100644
--- a/lldb/include/lldb/Target/TargetList.h
+++ b/lldb/include/lldb/Target/TargetList.h
@@ -183,28 +183,21 @@ class TargetList : public Broadcaster {
   typedef std::vector collection;
   // Member variables.
   collection m_target_list;
-  lldb::TargetSP m_dummy_target_sp;
   mutable std::recursive_mutex m_target_list_mutex;
   uint32_t m_selected_target_idx;
 
 private:
-  lldb::TargetSP GetDummyTarget(lldb_private::Debugger &debugger);
-
-  Status CreateDummyTarget(Debugger &debugger,
-   llvm::StringRef specified_arch_name,
-   lldb::TargetSP &target_sp);
-
   Status CreateTargetInternal(Debugger &debugger, llvm::StringRef 
user_exe_path,
   llvm::StringRef triple_str,
   LoadDependentFiles load_dependent_files,
   const OptionGroupPlatform *platform_options,
-  lldb::TargetSP &target_sp, bool is_dummy_target);
+  lldb::TargetSP &target_sp);
 
   Status CreateTargetInternal(Debugger &debugger, llvm::StringRef 
user_exe_path,
   const ArchSpec &arch,
   LoadDependentFiles get_dependent_modules,
   lldb::PlatformSP &platform_sp,
-  lldb::TargetSP &target_sp, bool is_dummy_target);
+  lldb::TargetSP &target_sp);
 
   TargetList(const TargetList &) = delete;
   const TargetList &operator=(const TargetList &) = delete;

diff  --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index f51754e15d9b..5f75ac246b80 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -682,7 +682,16 @@ Debugger::Debugger(lldb::LogOutputCallback log_callback, 
void *baton)
   assert(default_platform_sp);
   m_platform_list.Append(default_platform_sp, true);
 
-  m_dummy_target_sp = m_target_list.GetDummyTarget(*this);
+  // Create the dummy target.
+  {
+ArchSpec arch(Target::GetDefaultArchitecture());
+if (!arch.IsValid())
+  arch = HostInfo::GetArchitecture();
+assert(arch.IsValid() && "No valid default or host archspec");
+const bool is_dummy_target = true;
+m_dummy_target_sp.reset(
+new Target(*this, arch, default_platform_sp, is_dummy_target));
+  }
   assert(m_dummy_target_sp.get() && "Couldn't construct dummy target?");
 
   m_collection_sp->Initialize(g_debugger_properties);

diff  --git a/lldb/source/Target/TargetList.cpp 
b/lldb/source/Target/TargetList.cpp
index d4d3740286b7..ce9661ce63c2 100644
--- a/lldb/source/Target/TargetList.cpp
+++ b/lldb/source/Target/TargetList.cpp
@@ -55,8 +55,8 @@ Status TargetList::CreateTarget(Debugger &debugger,
 const OptionGroupPlatform *platform_options,
 TargetSP &target_sp) {
   return CreateTargetInternal(debugger, user_exe_path, triple_str,
-  load_dependent_files, platform_options, 
target_sp,
-  false);
+  load_dependent_files, platform_options,
+  target_sp);
 }
 
 Status TargetList::CreateTarget(Debugger &debugger,
@@ -65,15 +65,13 @@ Status TargetList::CreateTarget(Debugger &debugger,
 LoadDependentFiles load_dependent_files,
 PlatformSP &platform_sp, TargetSP &target_sp) 

[Lldb-commits] [PATCH] D90872: [TargetList] Simplify dummy target creation

[Vedant Kumar 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 rG16e5a347e70b: [TargetList] Simplify dummy target creation 
(authored by vsk).

Changed prior to commit:
  https://reviews.llvm.org/D90872?vs=303252&id=303290#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90872

Files:
  lldb/include/lldb/Target/Target.h
  lldb/include/lldb/Target/TargetList.h
  lldb/source/Core/Debugger.cpp
  lldb/source/Target/TargetList.cpp

Index: lldb/source/Target/TargetList.cpp
===
--- lldb/source/Target/TargetList.cpp
+++ lldb/source/Target/TargetList.cpp
@@ -55,8 +55,8 @@
 const OptionGroupPlatform *platform_options,
 TargetSP &target_sp) {
   return CreateTargetInternal(debugger, user_exe_path, triple_str,
-  load_dependent_files, platform_options, target_sp,
-  false);
+  load_dependent_files, platform_options,
+  target_sp);
 }
 
 Status TargetList::CreateTarget(Debugger &debugger,
@@ -65,15 +65,13 @@
 LoadDependentFiles load_dependent_files,
 PlatformSP &platform_sp, TargetSP &target_sp) {
   return CreateTargetInternal(debugger, user_exe_path, specified_arch,
-  load_dependent_files, platform_sp, target_sp,
-  false);
+  load_dependent_files, platform_sp, target_sp);
 }
 
 Status TargetList::CreateTargetInternal(
 Debugger &debugger, llvm::StringRef user_exe_path,
 llvm::StringRef triple_str, LoadDependentFiles load_dependent_files,
-const OptionGroupPlatform *platform_options, TargetSP &target_sp,
-bool is_dummy_target) {
+const OptionGroupPlatform *platform_options, TargetSP &target_sp) {
   Status error;
 
   // Let's start by looking at the selected platform.
@@ -256,7 +254,7 @@
   if (!prefer_platform_arch && arch.IsValid()) {
 if (!platform_sp->IsCompatibleArchitecture(arch, false, nullptr)) {
   platform_sp = Platform::GetPlatformForArchitecture(arch, &platform_arch);
-  if (!is_dummy_target && platform_sp)
+  if (platform_sp)
 debugger.GetPlatformList().SetSelectedPlatform(platform_sp);
 }
   } else if (platform_arch.IsValid()) {
@@ -266,7 +264,7 @@
 if (!platform_sp->IsCompatibleArchitecture(platform_arch, false, nullptr)) {
   platform_sp = Platform::GetPlatformForArchitecture(platform_arch,
  &fixed_platform_arch);
-  if (!is_dummy_target && platform_sp)
+  if (platform_sp)
 debugger.GetPlatformList().SetSelectedPlatform(platform_sp);
 }
   }
@@ -274,31 +272,9 @@
   if (!platform_arch.IsValid())
 platform_arch = arch;
 
-  return TargetList::CreateTargetInternal(
-  debugger, user_exe_path, platform_arch, load_dependent_files, platform_sp,
-  target_sp, is_dummy_target);
-}
-
-lldb::TargetSP TargetList::GetDummyTarget(lldb_private::Debugger &debugger) {
-  // FIXME: Maybe the dummy target should be per-Debugger
-  if (!m_dummy_target_sp || !m_dummy_target_sp->IsValid()) {
-ArchSpec arch(Target::GetDefaultArchitecture());
-if (!arch.IsValid())
-  arch = HostInfo::GetArchitecture();
-Status err = CreateDummyTarget(
-debugger, arch.GetTriple().getTriple().c_str(), m_dummy_target_sp);
-  }
-
-  return m_dummy_target_sp;
-}
-
-Status TargetList::CreateDummyTarget(Debugger &debugger,
- llvm::StringRef specified_arch_name,
- lldb::TargetSP &target_sp) {
-  PlatformSP host_platform_sp(Platform::GetHostPlatform());
-  return CreateTargetInternal(
-  debugger, (const char *)nullptr, specified_arch_name, eLoadDependentsNo,
-  (const OptionGroupPlatform *)nullptr, target_sp, true);
+  return TargetList::CreateTargetInternal(debugger, user_exe_path,
+  platform_arch, load_dependent_files,
+  platform_sp, target_sp);
 }
 
 Status TargetList::CreateTargetInternal(Debugger &debugger,
@@ -306,13 +282,13 @@
 const ArchSpec &specified_arch,
 LoadDependentFiles load_dependent_files,
 lldb::PlatformSP &platform_sp,
-lldb::TargetSP &target_sp,
-bool is_dummy_target) {
+lldb::TargetSP &target_sp) {
   static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
   Timer scoped_timer(
   func_cat, "TargetList::CreateTar

[Lldb-commits] [PATCH] D90490: [intel-pt][trace] Implement a "get supported trace type" packet

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: lldb/docs/lldb-gdb-remote.txt:248
+//  {
+//"name": 
+//"description": 

Should this specify that this name needs to match the name of the LLDB plug-in?



Comment at: lldb/include/lldb/Utility/StringExtractorGDBRemote.h:165-169
 eServerPacketType_jTraceStart,
 eServerPacketType_jTraceBufferRead,
 eServerPacketType_jTraceMetaRead,
 eServerPacketType_jTraceStop,
 eServerPacketType_jTraceConfigRead,

Mark as deprecated?



Comment at: lldb/include/lldb/Utility/StringExtractorGDBRemote.h:170
 eServerPacketType_jTraceConfigRead,
+eServerPacketType_jLLDBTraceSupportedType,
   };

Maybe add a space to separate the above deprecated stuff from this new stuff



Comment at: lldb/include/lldb/Utility/TraceOptions.h:21
+struct TraceTypeInfo {
+  /// The name of the technology, e.g. intel-pt or arm-coresight.
+  std::string name;

We should specify that this needs to match the LLDB trace plug-in name on the 
header doc.



Comment at: lldb/include/lldb/lldb-enumerations.h:772
 
 enum TraceType {
   eTraceTypeNone = 0,

Mark as deprecated?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90490

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

[Lldb-commits] [PATCH] D90895: [TargetList] Delete the destructor

[Vedant Kumar via Phabricator via lldb-commits]

vsk created this revision.
vsk added reviewers: teemperor, JDevlieghere, jingham.
Herald added a project: LLDB.
vsk requested review of this revision.

AFAICT, ~TargetList simply implements the default destructor, plus some
locking.

The history is murky, so I'm not sure why we do this locking. Perhaps,
at some point, it was possible to delete the same TargetList instance
from two different threads, setting up a race. If that were true, then
the locking would protect against the race.

Since TargetList is uniquely owned by Debugger (m_target_list), no such
race is possible today.

Testing: check-lldb


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90895

Files:
  lldb/include/lldb/Target/TargetList.h
  lldb/source/Target/TargetList.cpp


Index: lldb/source/Target/TargetList.cpp
===
--- lldb/source/Target/TargetList.cpp
+++ lldb/source/Target/TargetList.cpp
@@ -42,12 +42,6 @@
   CheckInWithManager();
 }
 
-// Destructor
-TargetList::~TargetList() {
-  std::lock_guard guard(m_target_list_mutex);
-  m_target_list.clear();
-}
-
 Status TargetList::CreateTarget(Debugger &debugger,
 llvm::StringRef user_exe_path,
 llvm::StringRef triple_str,
Index: lldb/include/lldb/Target/TargetList.h
===
--- lldb/include/lldb/Target/TargetList.h
+++ lldb/include/lldb/Target/TargetList.h
@@ -42,8 +42,6 @@
 return GetStaticBroadcasterClass();
   }
 
-  ~TargetList() override;
-
   /// Create a new Target.
   ///
   /// Clients must use this function to create a Target. This allows


Index: lldb/source/Target/TargetList.cpp
===
--- lldb/source/Target/TargetList.cpp
+++ lldb/source/Target/TargetList.cpp
@@ -42,12 +42,6 @@
   CheckInWithManager();
 }
 
-// Destructor
-TargetList::~TargetList() {
-  std::lock_guard guard(m_target_list_mutex);
-  m_target_list.clear();
-}
-
 Status TargetList::CreateTarget(Debugger &debugger,
 llvm::StringRef user_exe_path,
 llvm::StringRef triple_str,
Index: lldb/include/lldb/Target/TargetList.h
===
--- lldb/include/lldb/Target/TargetList.h
+++ lldb/include/lldb/Target/TargetList.h
@@ -42,8 +42,6 @@
 return GetStaticBroadcasterClass();
   }
 
-  ~TargetList() override;
-
   /// Create a new Target.
   ///
   /// Clients must use this function to create a Target. This allows
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D90895: [TargetList] Delete the destructor

[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/D90895/new/

https://reviews.llvm.org/D90895

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

[Lldb-commits] [lldb] 65d15fe - [TargetList] Delete the destructor

[Vedant Kumar via lldb-commits]

Author: Vedant Kumar
Date: 2020-11-05T16:56:37-08:00
New Revision: 65d15fefe3392b1db2f679b3df029d43d8d26d2d

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

LOG: [TargetList] Delete the destructor

AFAICT, ~TargetList simply implements the default destructor, plus some
locking.

The history is murky, so I'm not sure why we do this locking. Perhaps,
at some point, it was possible to delete the same TargetList instance
from two different threads, setting up a race. If that were true, then
the locking would protect against the race.

Since TargetList is uniquely owned by Debugger (m_target_list), no such
race is possible today.

Testing: check-lldb

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

Added: 


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

Removed: 




diff  --git a/lldb/include/lldb/Target/TargetList.h 
b/lldb/include/lldb/Target/TargetList.h
index ff7fbd2753ab..94b25f65863a 100644
--- a/lldb/include/lldb/Target/TargetList.h
+++ b/lldb/include/lldb/Target/TargetList.h
@@ -42,8 +42,6 @@ class TargetList : public Broadcaster {
 return GetStaticBroadcasterClass();
   }
 
-  ~TargetList() override;
-
   /// Create a new Target.
   ///
   /// Clients must use this function to create a Target. This allows

diff  --git a/lldb/source/Target/TargetList.cpp 
b/lldb/source/Target/TargetList.cpp
index ce9661ce63c2..2be32c53da9f 100644
--- a/lldb/source/Target/TargetList.cpp
+++ b/lldb/source/Target/TargetList.cpp
@@ -42,12 +42,6 @@ TargetList::TargetList(Debugger &debugger)
   CheckInWithManager();
 }
 
-// Destructor
-TargetList::~TargetList() {
-  std::lock_guard guard(m_target_list_mutex);
-  m_target_list.clear();
-}
-
 Status TargetList::CreateTarget(Debugger &debugger,
 llvm::StringRef user_exe_path,
 llvm::StringRef triple_str,



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

[Lldb-commits] [PATCH] D90895: [TargetList] Delete the destructor

[Vedant Kumar 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 rG65d15fefe339: [TargetList] Delete the destructor (authored 
by vsk).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90895

Files:
  lldb/include/lldb/Target/TargetList.h
  lldb/source/Target/TargetList.cpp


Index: lldb/source/Target/TargetList.cpp
===
--- lldb/source/Target/TargetList.cpp
+++ lldb/source/Target/TargetList.cpp
@@ -42,12 +42,6 @@
   CheckInWithManager();
 }
 
-// Destructor
-TargetList::~TargetList() {
-  std::lock_guard guard(m_target_list_mutex);
-  m_target_list.clear();
-}
-
 Status TargetList::CreateTarget(Debugger &debugger,
 llvm::StringRef user_exe_path,
 llvm::StringRef triple_str,
Index: lldb/include/lldb/Target/TargetList.h
===
--- lldb/include/lldb/Target/TargetList.h
+++ lldb/include/lldb/Target/TargetList.h
@@ -42,8 +42,6 @@
 return GetStaticBroadcasterClass();
   }
 
-  ~TargetList() override;
-
   /// Create a new Target.
   ///
   /// Clients must use this function to create a Target. This allows


Index: lldb/source/Target/TargetList.cpp
===
--- lldb/source/Target/TargetList.cpp
+++ lldb/source/Target/TargetList.cpp
@@ -42,12 +42,6 @@
   CheckInWithManager();
 }
 
-// Destructor
-TargetList::~TargetList() {
-  std::lock_guard guard(m_target_list_mutex);
-  m_target_list.clear();
-}
-
 Status TargetList::CreateTarget(Debugger &debugger,
 llvm::StringRef user_exe_path,
 llvm::StringRef triple_str,
Index: lldb/include/lldb/Target/TargetList.h
===
--- lldb/include/lldb/Target/TargetList.h
+++ lldb/include/lldb/Target/TargetList.h
@@ -42,8 +42,6 @@
 return GetStaticBroadcasterClass();
   }
 
-  ~TargetList() override;
-
   /// Create a new Target.
   ///
   /// Clients must use this function to create a Target. This allows
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D90729: [trace][intel-pt] Scaffold the 'thread trace start | stop' commands

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added inline comments.
This revision now requires changes to proceed.



Comment at: lldb/include/lldb/Target/Trace.h:183
+  /// llvm::Error::success() otherwise.
+  virtual llvm::Error StopTracingThread(const Thread &thread) {
+return llvm::make_error();

We need to have StartTracingThread() in this interface as well. I know right 
now we can just do the work in the command plug-in, but eventually we will need 
to expose the trace stuff through the SB API, so we need the StartTracingThread 
in the API here.



Comment at: lldb/source/Commands/CommandObjectThread.cpp:2126
+/// can support the current process.
+class CommandObjectTraceStart : public CommandObjectParsed {
+public:

We should probably create a CommandObjectDelegate in CommandObjectDelegate.cpp 
and CommandObjectDelegate.h. This class will have one pure virtual function 
named GetDelegateCommand() which we would override in this class and return 
GetTracePluginCommand(). Then this class will become much simpler and the next 
people that will create a similar kind of command that just forwards to another 
command can use it.



Comment at: lldb/source/Commands/CommandObjectThread.cpp:2139
+
+  Options *GetOptions() override {
+if (CommandObject *trace_plugin_command =

Move to CommandObjectDelegate.cpp



Comment at: lldb/source/Commands/CommandObjectThread.cpp:2140-2141
+  Options *GetOptions() override {
+if (CommandObject *trace_plugin_command =
+GetTracePluginCommand().command.get())
+  return trace_plugin_command->GetOptions();

Store as a shared pointer here please and use it:
```
CommandObjectSP delegate_cmd_sp = GetDelegateCommand();
if (delegate_cmd_sp)
  delegate_cmd_sp->GetOptions();
```



Comment at: lldb/source/Commands/CommandObjectThread.cpp:2146
+
+  llvm::StringRef GetSyntax() override {
+if (CommandObject *trace_plugin_command =

Move to CommandObjectDelegate.cpp





Comment at: lldb/source/Commands/CommandObjectThread.cpp:2147-2148
+  llvm::StringRef GetSyntax() override {
+if (CommandObject *trace_plugin_command =
+GetTracePluginCommand().command.get())
+  return trace_plugin_command->GetSyntax();

Store as a shared pointer like above comment.



Comment at: lldb/source/Commands/CommandObjectThread.cpp:2153
+
+  llvm::StringRef GetHelp() override {
+if (CommandObject *trace_plugin_command =

Move to CommandObjectDelegate.cpp





Comment at: lldb/source/Commands/CommandObjectThread.cpp:2154-2155
+  llvm::StringRef GetHelp() override {
+if (CommandObject *trace_plugin_command =
+GetTracePluginCommand().command.get())
+  return trace_plugin_command->GetHelp();

Store as a shared pointer like above comment.



Comment at: lldb/source/Commands/CommandObjectThread.cpp:2160
+
+  llvm::StringRef GetHelpLong() override {
+if (CommandObject *trace_plugin_command =

Move to CommandObjectDelegate.cpp





Comment at: lldb/source/Commands/CommandObjectThread.cpp:2161-2162
+  llvm::StringRef GetHelpLong() override {
+if (CommandObject *trace_plugin_command =
+GetTracePluginCommand().command.get())
+  return trace_plugin_command->GetHelpLong();

Store as a shared pointer like above comment.



Comment at: lldb/source/Commands/CommandObjectThread.cpp:2170
+
+  bool Execute(const char *args_string, CommandReturnObject &result) override {
+TracePluginCommandImplementation &trace_plugin_command =

Why are you taking over execute instead of DoExecute?



Comment at: lldb/source/Commands/CommandObjectThread.cpp:2176-2187
+if (const TraceSP &trace_sp =
+trace_plugin_command.process->GetTarget().GetTrace()) {
+  ConstString plugin_name = trace_sp->GetPluginName();
+  if (plugin_name.GetStringRef() != trace_plugin_command.plugin_name) {
+result.AppendErrorWithFormat(
+"error: attempted to trace with the '%s' plug-in, but this process 
"
+"is already being traced with the '%s' plug-in. Stop tracing "

If we require that only one tracing mechanism is available in a process, then 
why do we need this check?



Comment at: lldb/source/Commands/CommandObjectThread.cpp:2202
+  /// the trace plug-in that supports the current process.
+  TracePluginCommandImplementation &GetTracePluginCommand() {
+ExecutionContext exe_ctx = m_interpreter.GetExecutionContext();

make virtual override of CommandObjectDelegate::GetDelegateCommand()



Comment at: lldb/source/Commands/CommandObjectThread.h:16
 
+class CommandObjectI

[Lldb-commits] [lldb] 99a99c2 - [lldb] Remove Crashlog/interactive.test

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2020-11-05T17:10:52-08:00
New Revision: 99a99c29c6daef56a320a0d77bb6691aa66d81f1

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

LOG: [lldb] Remove Crashlog/interactive.test

This test requires running under the Python we built against (which is
easy) and setting up the PYTHONPATH (which is not worth it for this
simple test).

Added: 


Modified: 


Removed: 
lldb/test/Shell/ScriptInterpreter/Python/Crashlog/interactive.test



diff  --git 
a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/interactive.test 
b/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/interactive.test
deleted file mode 100644
index 2690b7fa2122..
--- a/lldb/test/Shell/ScriptInterpreter/Python/Crashlog/interactive.test
+++ /dev/null
@@ -1,8 +0,0 @@
-# RUN: echo "quit" | %S/../../../../../examples/python/crashlog.py -i %s 2>&1 
| FileCheck %s
-# CHECK: 1 crash logs are loaded:
-# CHECK: [0] = {{.*}}interactive.test
-# CHECK: Interactive crashlogs prompt, type "help" to see a list of supported 
commands.
-
-Binary Images:
-   0x10ab87000 -0x10abdafff +lldb (10.0.0) 
<87BD1384-BAE9-3625-A838-9D241CBAEF87> /Volumes/VOLUME/*/lldb
-   0x10ac45000 -0x10ae94fff  com.apple.python3 (3.8.2 - 3.8.2) 
<20BC3FC4-CAAD-3002-ACDF-423A3188F24C> 
/Applications/Xcode.app/Contents/Developer/Library/Frameworks/Python3.framework/Versions/3.8/Python3



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

[Lldb-commits] [lldb] cfd96f0 - [trace][intel-pt] Implement the basic decoding functionality

[Walter Erquinigo via lldb-commits]

Author: Walter Erquinigo
Date: 2020-11-05T18:38:03-08:00
New Revision: cfd96f057ba4256fef49518cad43d0a7f591da12

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

LOG: [trace][intel-pt] Implement the basic decoding functionality

Depends on D89408.

This diff finally implements trace decoding!

The current interface is

  $ trace load /path/to/trace/session/file.json
  $ thread trace dump instructions

  thread #1: tid = 3842849, total instructions = 22
[ 0] 0x40052d
[ 1] 0x40052d
...
[19] 0x400521

  $ # simply enter, which is a repeat command
[20] 0x40052d
[21] 0x400529
...

This doesn't do any disassembly, which will be done in the next diff.

Changes:
- Added an IntelPTDecoder class, that is a wrapper for libipt, which is the 
actual library that performs the decoding.
- Added TraceThreadDecoder class that decodes traces and memoizes the result to 
avoid repeating the decoding step.
- Added a DecodedThread class, which represents the output from decoding and 
that for the time being only stores the list of reconstructed instructions. 
Later it'll contain the function call hierarchy, which will enable 
reconstructing backtraces.
- Added basic APIs for accessing the trace in Trace.h:
  - GetInstructionCount, which counts the number of instructions traced for a 
given thread
  - IsTraceFailed, which returns an Error if decoding a thread failed
  - ForEachInstruction, which iterates on the instructions traced for a given 
thread, concealing the internal storage of threads, as plug-ins can decide to 
generate the instructions on the fly or to store them all in a vector, like I 
do.
- DumpTraceInstructions was updated to print the instructions or show an error 
message if decoding was impossible.
- Tests included

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

Added: 
lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp
lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.h
lldb/test/API/commands/trace/intelpt-trace-multi-file/a.out
lldb/test/API/commands/trace/intelpt-trace-multi-file/bar.cpp
lldb/test/API/commands/trace/intelpt-trace-multi-file/bar.h
lldb/test/API/commands/trace/intelpt-trace-multi-file/foo.cpp
lldb/test/API/commands/trace/intelpt-trace-multi-file/foo.h
lldb/test/API/commands/trace/intelpt-trace-multi-file/libbar.so
lldb/test/API/commands/trace/intelpt-trace-multi-file/libfoo.so
lldb/test/API/commands/trace/intelpt-trace-multi-file/main.cpp
lldb/test/API/commands/trace/intelpt-trace-multi-file/multi-file-no-ld.json
lldb/test/API/commands/trace/intelpt-trace-multi-file/multi-file.trace
lldb/test/API/commands/trace/intelpt-trace/trace_bad_image.json
lldb/test/API/commands/trace/intelpt-trace/trace_wrong_cpu.json

Modified: 
lldb/include/lldb/Core/Disassembler.h
lldb/include/lldb/Symbol/SymbolContext.h
lldb/include/lldb/Target/Trace.h
lldb/packages/Python/lldbsuite/test/lldbtest.py
lldb/source/Commands/CommandObjectThread.cpp
lldb/source/Commands/Options.td
lldb/source/Core/Disassembler.cpp
lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
lldb/source/Symbol/SymbolContext.cpp
lldb/source/Target/ProcessTrace.cpp
lldb/source/Target/Trace.cpp
lldb/source/Target/TraceSessionFileParser.cpp
lldb/test/API/commands/trace/TestTraceDumpInstructions.py

Removed: 




diff  --git a/lldb/include/lldb/Core/Disassembler.h 
b/lldb/include/lldb/Core/Disassembler.h
index c38f1f7975d5..9a694de0f60a 100644
--- a/lldb/include/lldb/Core/Disassembler.h
+++ b/lldb/include/lldb/Core/Disassembler.h
@@ -271,6 +271,13 @@ class InstructionList {
 
   lldb::InstructionSP GetInstructionAtIndex(size_t idx) const;
 
+  /// Get the instruction at the given address.
+  ///
+  /// \return
+  ///A valid \a InstructionSP if the address could be found, or null
+  ///otherwise.
+  lldb::InstructionSP GetInstructionAtAddress(const Address &addr);
+
   //--
   /// Get the index of the next branch instruction.
   ///

diff  --git a/lldb/include/lldb/Symbol/SymbolContext.h 
b/lldb/include/lldb/Symbol/SymbolContext.h
index 0f99364596c2..c513dbb447f8 100644
--- a/lldb/include/lldb/Symbol/SymbolContext.h
+++ b/lldb/include/lldb/Symbol/SymbolContext.h
@@ -139,11 +139,19 @@ class SymbolContext {
   /// be printed.  In disassembly formatting, where we want a format
   /// like "<*+36>", this should be false and "*" will be printed
   /// instead.
+  ///
+  /// \param[in] show_inlin

[Lldb-commits] [PATCH] D89283: [trace][intel-pt] Implement the basic decoding functionality

[walter erquinigo 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 rGcfd96f057ba4: [trace][intel-pt] Implement the basic decoding 
functionality (authored by Walter Erquinigo , committed 
by wallace).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89283

Files:
  lldb/include/lldb/Core/Disassembler.h
  lldb/include/lldb/Symbol/SymbolContext.h
  lldb/include/lldb/Target/Trace.h
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/Options.td
  lldb/source/Core/Disassembler.cpp
  lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
  lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Symbol/SymbolContext.cpp
  lldb/source/Target/ProcessTrace.cpp
  lldb/source/Target/Trace.cpp
  lldb/source/Target/TraceSessionFileParser.cpp
  lldb/test/API/commands/trace/TestTraceDumpInstructions.py
  lldb/test/API/commands/trace/intelpt-trace-multi-file/a.out
  lldb/test/API/commands/trace/intelpt-trace-multi-file/bar.cpp
  lldb/test/API/commands/trace/intelpt-trace-multi-file/bar.h
  lldb/test/API/commands/trace/intelpt-trace-multi-file/foo.cpp
  lldb/test/API/commands/trace/intelpt-trace-multi-file/foo.h
  lldb/test/API/commands/trace/intelpt-trace-multi-file/libbar.so
  lldb/test/API/commands/trace/intelpt-trace-multi-file/libfoo.so
  lldb/test/API/commands/trace/intelpt-trace-multi-file/main.cpp
  lldb/test/API/commands/trace/intelpt-trace-multi-file/multi-file-no-ld.json
  lldb/test/API/commands/trace/intelpt-trace-multi-file/multi-file.trace
  lldb/test/API/commands/trace/intelpt-trace/trace_bad_image.json
  lldb/test/API/commands/trace/intelpt-trace/trace_wrong_cpu.json

Index: lldb/test/API/commands/trace/intelpt-trace/trace_wrong_cpu.json
===
--- /dev/null
+++ lldb/test/API/commands/trace/intelpt-trace/trace_wrong_cpu.json
@@ -0,0 +1,31 @@
+{
+  "trace": {
+"type": "intel-pt",
+"pt_cpu": {
+  "vendor": "intel",
+  "family": 2123123,
+  "model": 12123123,
+  "stepping": 1231231
+}
+  },
+  "processes": [
+{
+  "pid": 1234,
+  "triple": "x86_64-*-linux",
+  "threads": [
+{
+  "tid": 3842849,
+  "traceFile": "3842849.trace"
+}
+  ],
+  "modules": [
+{
+  "file": "a.out",
+  "systemPath": "a.out",
+  "loadAddress": "0x0040",
+  "uuid": "6AA9A4E2-6F28-2F33-377D-59FECE874C71-5B41261A"
+}
+  ]
+}
+  ]
+}
Index: lldb/test/API/commands/trace/intelpt-trace/trace_bad_image.json
===
--- /dev/null
+++ lldb/test/API/commands/trace/intelpt-trace/trace_bad_image.json
@@ -0,0 +1,31 @@
+{
+  "trace": {
+"type": "intel-pt",
+"pt_cpu": {
+  "vendor": "intel",
+  "family": 6,
+  "model": 79,
+  "stepping": 1
+}
+  },
+  "processes": [
+{
+  "pid": 1234,
+  "triple": "x86_64-*-linux",
+  "threads": [
+{
+  "tid": 3842849,
+  "traceFile": "3842849.trace"
+}
+  ],
+  "modules": [
+{
+  "file": "a.out",
+  "systemPath": "a.out",
+  "loadAddress": "0x00F0",
+  "uuid": "6AA9A4E2-6F28-2F33-377D-59FECE874C71-5B41261A"
+}
+  ]
+}
+  ]
+}
Index: lldb/test/API/commands/trace/intelpt-trace-multi-file/multi-file-no-ld.json
===
--- /dev/null
+++ lldb/test/API/commands/trace/intelpt-trace-multi-file/multi-file-no-ld.json
@@ -0,0 +1,43 @@
+{
+  "trace": {
+"type": "intel-pt",
+"pt_cpu": {
+  "vendor": "intel",
+  "family": 6,
+  "model": 79,
+  "stepping": 1
+}
+  },
+  "processes": [
+{
+  "pid": 815455,
+  "triple": "x86_64-*-linux",
+  "threads": [
+{
+  "tid": 815455,
+  "traceFile": "multi-file.trace"
+}
+  ],
+  "modules": [
+{
+  "file": "a.out",
+  "systemPath": "a.out",
+  "loadAddress": "0x0040",
+  "uuid": "D2414468-7112-B7C5-408D-FF07E30D5B17-A5BFD2C4"
+},
+{
+  "file": "libfoo.so",
+  "systemPath": "libfoo.so",
+  "loadAddress": "0x77bd9000",
+  "uuid": "B30FFEDA-8BB2-3D08-4580-C5937ED11E2B-21BE778C"
+},
+{
+  "file": "libbar.so",
+  "systemPath": "libbar.so",
+  "loadAddress": "0x779d7000",
+  

[Lldb-commits] [PATCH] D90490: [intel-pt][trace] Implement a "get supported trace type" packet

[walter erquinigo via Phabricator via lldb-commits]

wallace updated this revision to Diff 303313.
wallace marked 5 inline comments as done.
wallace added a comment.

Address all comments, specially improved the documentation requested by 
@clayborg.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90490

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Target/Trace.h
  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
  lldb/include/lldb/Utility/TraceOptions.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
  lldb/source/Plugins/Process/Linux/ProcessorTrace.cpp
  lldb/source/Plugins/Process/Linux/ProcessorTrace.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/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/source/Utility/CMakeLists.txt
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/source/Utility/TraceOptions.cpp
  lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp

Index: lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
===
--- lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -362,6 +362,45 @@
   EXPECT_FALSE(result.get().Success());
 }
 
+TEST_F(GDBRemoteCommunicationClientTest, SendTraceSupportedTypePacket) {
+  // Success response
+  {
+std::future> result = std::async(
+std::launch::async, [&] { return client.SendGetSupportedTraceType(); });
+
+HandlePacket(
+server, "jLLDBTraceSupportedType",
+R"({"name":"intel-pt","description":"Intel Processor Trace"}])");
+
+llvm::Expected trace_type_or_err = result.get();
+EXPECT_THAT_EXPECTED(trace_type_or_err, llvm::Succeeded());
+ASSERT_STREQ(trace_type_or_err->name.c_str(), "intel-pt");
+ASSERT_STREQ(trace_type_or_err->description.c_str(),
+ "Intel Processor Trace");
+  }
+
+  // Error response - wrong json
+  {
+std::future> result = std::async(
+std::launch::async, [&] { return client.SendGetSupportedTraceType(); });
+
+HandlePacket(server, "jLLDBTraceSupportedType", R"({"type":"intel-pt"}])");
+
+llvm::Expected trace_type_or_err = result.get();
+EXPECT_THAT_EXPECTED(trace_type_or_err, llvm::Failed());
+  }
+
+  // Error response
+  {
+std::future> result = std::async(
+std::launch::async, [&] { return client.SendGetSupportedTraceType(); });
+
+HandlePacket(server, "jLLDBTraceSupportedType", "E23");
+llvm::Expected trace_type_or_err = result.get();
+ASSERT_THAT_EXPECTED(trace_type_or_err, llvm::Failed());
+  }
+}
+
 TEST_F(GDBRemoteCommunicationClientTest, SendStartTracePacket) {
   TraceOptions options;
   Status error;
Index: lldb/source/Utility/TraceOptions.cpp
===
--- /dev/null
+++ lldb/source/Utility/TraceOptions.cpp
@@ -0,0 +1,25 @@
+//===-- TraceOptions.cpp *- 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
+//
+//===--===//
+
+#include "lldb/Utility/TraceOptions.h"
+
+using namespace lldb_private;
+
+namespace llvm {
+namespace json {
+
+bool fromJSON(const Value &value, TraceTypeInfo &info, Path path) {
+  ObjectMapper o(value, path);
+  if (!o)
+return false;
+  o.map("description", info.description);
+  return o.map("name", info.name);
+}
+
+} // namespace json
+} // namespace llvm
Index: lldb/source/Utility/StringExtractorGDBRemote.cpp
===
--- lldb/source/Utility/StringExtractorGDBRemote.cpp
+++ lldb/source/Utility/StringExtractorGDBRemote.cpp
@@ -310,6 +310,8 @@
   return eServerPacketType_jTraceStart;
 if (PACKET_STARTS_WITH("jTraceStop:"))
   return eServerPacketType_jTraceStop;
+if (PACKET_MATCHES("jLLDBTraceSupportedType"))
+  return eServerPacketType_jLLDBTraceSupportedType;
 break;
 
   case 'v':
Index: lldb/source/Utility/CMakeLists.txt
===
--- lldb/source/Utility/CMakeLists.txt
+++ lldb/source/Utility/CMakeLists.txt
@@ -65,6 +65,7 @@
   StructuredData.cp

[Lldb-commits] [PATCH] D90729: [trace][intel-pt] Scaffold the 'thread trace start | stop' commands

[walter erquinigo via Phabricator via lldb-commits]

wallace added inline comments.



Comment at: lldb/include/lldb/Target/Trace.h:183
+  /// llvm::Error::success() otherwise.
+  virtual llvm::Error StopTracingThread(const Thread &thread) {
+return llvm::make_error();

clayborg wrote:
> We need to have StartTracingThread() in this interface as well. I know right 
> now we can just do the work in the command plug-in, but eventually we will 
> need to expose the trace stuff through the SB API, so we need the 
> StartTracingThread in the API here.
l'll add it later when I implement the actual StartTracingThread logic. I want 
to make sure I can cleanly allow each Trace plug-in to have its own set of 
tracing options for the StartTracingThread, and I'd rather do that in another 
diff.



Comment at: lldb/source/Commands/CommandObjectThread.cpp:2126
+/// can support the current process.
+class CommandObjectTraceStart : public CommandObjectParsed {
+public:

clayborg wrote:
> We should probably create a CommandObjectDelegate in 
> CommandObjectDelegate.cpp and CommandObjectDelegate.h. This class will have 
> one pure virtual function named GetDelegateCommand() which we would override 
> in this class and return GetTracePluginCommand(). Then this class will become 
> much simpler and the next people that will create a similar kind of command 
> that just forwards to another command can use it.
Great idea!



Comment at: lldb/source/Commands/CommandObjectThread.cpp:2170
+
+  bool Execute(const char *args_string, CommandReturnObject &result) override {
+TracePluginCommandImplementation &trace_plugin_command =

clayborg wrote:
> Why are you taking over execute instead of DoExecute?
because Execute in the delegate configures the ExecutionContext correctly, so I 
need to invoke Execute in the Trace plug-in command. With the Delegate idea 
that you mention, this will look cleaner



Comment at: lldb/source/Commands/CommandObjectThread.cpp:2176-2187
+if (const TraceSP &trace_sp =
+trace_plugin_command.process->GetTarget().GetTrace()) {
+  ConstString plugin_name = trace_sp->GetPluginName();
+  if (plugin_name.GetStringRef() != trace_plugin_command.plugin_name) {
+result.AppendErrorWithFormat(
+"error: attempted to trace with the '%s' plug-in, but this process 
"
+"is already being traced with the '%s' plug-in. Stop tracing "

clayborg wrote:
> If we require that only one tracing mechanism is available in a process, then 
> why do we need this check?
that's right!



Comment at: lldb/source/Interpreter/CommandObject.cpp:263-266
+if (!m_exe_ctx.GetProcessPtr()) {
+  result.SetError("Process must exist.");
+  return false;
+}

clayborg wrote:
> The command should specify it needs a process with eCommandRequiresProcess. 
> Remove these lines.
sure


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90729

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

[Lldb-commits] [PATCH] D90875: [lldb] [test] Avoid double negatives in llgs/debugserver logic

[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/packages/Python/lldbsuite/test/dotest.py:950
+configuration.llgs_platform = (
+target_platform in ["linux", "freebsd", "netbsd", "windows"])
+

This is not entirely equivalent to the previous version -- it won't match 
"platforms" like "freebsd4.7". Fortunately, we already have some 
canonicalization code in `lldbplatformutil.getPlatform()`, so if you switch to 
that, it should be fine.



Comment at: lldb/packages/Python/lldbsuite/test/dotest.py:953
+# Perform debugserver tests on Darwin platforms.  Currently, this
+# means all platform that do not use LLGS but additional platforms
+# may be excluded in the future.

mgorny wrote:
> @labath, can we assume that all future platforms will use LLGS?
I certainly hope so. I doubt anyone would port debugserver to another os, and I 
definitely would not want to carry another debugserver around.


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

https://reviews.llvm.org/D90875

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

[Lldb-commits] [PATCH] D90876: [lldb] [test] Improve comment on expr-after-step-after-crash tests

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D90876#2377544 , @mgorny wrote:

> In D90876#2377541 , @emaste wrote:
>
>> How does Windows fit into this? Other than that Q, LGTM.
>
> @labath, any clue?

I don't know... Since the test is not annotated, I guess it somehow makes this 
work, but I haven't been able to find the code that makes it happen.

The problem with this test is that the behavior it prescribes is not compatible 
with applications that handle SIGSEGV themselves. People have complained about 
that, including on macos (see D89315 ). If we 
were able to tell that the application will not actually handle the SEGV (or 
other signal), then displaying the process as "crashed" would actually be a 
good signal to the user. But I don't know of a reasonable way to check whether 
injecting a signal will cause the app to exit. The situation is probably the 
same on windows, as it also allows apps to handle their own exceptions (unless 
we're actually checking whether the application will handle it, which I doubt) 
, but noone probably noticed it yet...


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

https://reviews.llvm.org/D90876

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