date:20220705

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

2022-07-05 Thread walter erquinigo via Phabricator via lldb-commits

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

much better! Thanks for doing this.

There are two main things to do.

1. The algorithm you are using to classify the instructions uses too many 
acronyms and it's very well documented. Try to make it understandable enough so 
that anyone with little familiarity can have an idea of what is happening
2. write a unit test. For this, you can follow any of the tests in 
`lldb/unittests/Disassembler/`. In order to run it, you'll need to do `ninja 
DisassemblerTests` and then run the output path printed by that command. It 
uses gtest. You can see the tests in that folder for inspiration.




Comment at: lldb/include/lldb/Core/Disassembler.h:82
 
+  /// Return InstructionControlFlowType of this instruction.
+  lldb::InstructionControlFlowType

better this



Comment at: lldb/include/lldb/Core/Disassembler.h:84
+  lldb::InstructionControlFlowType
+  GetControlFlowInstructionKind(const ExecutionContext *exe_ctx);
+

Rename this to
  GetControlFlowKind
because we are already inside the Instruction 



Comment at: lldb/include/lldb/lldb-enumerations.h:976
+  eInstructionControlFlowTypeUnknown = 0,
+  /// The instruction is something not listed below.
+  eInstructionControlFlowTypeOther,





Comment at: lldb/source/API/SBInstruction.cpp:244
 FormatEntity::Parse("${addr}: ", format);
-inst_sp->Dump(&s.ref(), 0, true, false, nullptr, &sc, nullptr, &format, 0);
+inst_sp->Dump(&s.ref(), 0, true, false, /*show_control_flow_type*/ false,
+  nullptr, &sc, nullptr, &format, 0);

the llvm style requires a =



Comment at: lldb/source/API/SBInstruction.cpp:279
 FormatEntity::Parse("${addr}: ", format);
-inst_sp->Dump(&out_stream, 0, true, false, nullptr, &sc, nullptr, &format,
-  0);
+inst_sp->Dump(&out_stream, 0, true, false, /*show_control_flow_type*/ 
false,
+  nullptr, &sc, nullptr, &format, 0);

same here



Comment at: lldb/source/API/SBInstructionList.cpp:169
+inst->Dump(&sref, max_opcode_byte_size, true, false,
+   /*show_control_flow_type*/ false, nullptr, &sc, &prev_sc,
+   &format, 0);

ditto



Comment at: lldb/source/Commands/Options.td:306
+"`InstructionControlFlowType` for a list of control flow kind. "
+"far jump and far return often indicate syscall and return.">;
   def disassemble_options_context : Option<"context", "C">, Arg<"NumLines">,





Comment at: lldb/source/Commands/Options.td:1160
+"`InstructionControlFlowType` enum has a list of control flow kind. "
+"far jump and far return often indicate syscall and return.">;
   def thread_trace_dump_instructions_show_tsc : Option<"tsc", "t">, Group<1>,

copy the changes from above



Comment at: lldb/source/Core/Disassembler.cpp:575
+namespace x86 {
+enum PTI_MAP {
+  PTI_MAP_0,/* 1-byte opcodes.   may have modrm */

add documentation mentioning what a PTI_MAP is. Try to make the documentation 
clear enough for anyone without knowledge of this matter. Also, mention what 
modrm is why it's important here.
Also, it's better if don't use the PTI acronym but instead use the full name. 
It'd make it easier to read.
Also, this is an enum and not a map, so better use an identifier different than 
a map



Comment at: lldb/source/Core/Disassembler.cpp:584-586
+/// Decide InstructionControlFlowType based on bytes of instruction.
+/// The insruction bytes are parsed beforehand into opcode, modrm and map byte
+/// to be passed as parameters.

let's be more specific



Comment at: lldb/source/Core/Disassembler.cpp:695-696
+  Opcode m_opcode) {
+  // inst_bytes will be parsed into opcode, modrm and map bytes.
+  // These are the three values deciding instruction control flow type.
+  uint8_t inst_bytes[16];

another reference to opcode, modrm, and map bytes. I recommend writing 
extensive documentation in PTI_MAP and then mention here that the full 
documentation can be found in the PTI_MAP definition



Comment at: lldb/source/Core/Disassembler.cpp:703-704
+  if (m_opcode.GetOpcodeBytes() == nullptr || m_opcode.GetByteSize() <= 0) {
+// x86_64 and i386 are the only ones that use bytes right now
+// Else, we have ARM or MIPS, not yet implemented
+return lldb::eInstructionControlFlowTypeUnknown;

this comment is not useful because this method is in the x86 namespace



Comment at: lldb/source/Core/Disassembler.cpp:710-713
+  // In most cases, opcode is the first byte of instruction (a

[Lldb-commits] [lldb] dc46ae6 - [lldb] Add support to load object files from thin archives

2022-07-05 Thread Nico Weber via lldb-commits


Author: Kaining Zhong
Date: 2022-07-05T10:52:26+02:00
New Revision: dc46ae6df5f769b80b56a30f6d77f962fa90833d

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

LOG: [lldb] Add support to load object files from thin archives

This fixes https://github.com/llvm/llvm-project/issues/50114 where lldb/mac
can't load object files from thin archives.  This patch allows lldb to identify
thin archives, and load object files contained in them.

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

Added: 
lldb/test/API/functionalities/archives/c.c

Modified: 

lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.cpp
lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.h
lldb/test/API/functionalities/archives/Makefile
lldb/test/API/functionalities/archives/TestBSDArchives.py
lldb/test/API/lit.cfg.py
lldb/test/CMakeLists.txt

Removed: 




diff  --git 
a/lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.cpp 
b/lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.cpp
index a2522372f5af4..9fe222eceedca 100644
--- 
a/lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.cpp
+++ 
b/lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.cpp
@@ -40,6 +40,8 @@ typedef struct ar_hdr {
 using namespace lldb;
 using namespace lldb_private;
 
+using namespace llvm::object;
+
 LLDB_PLUGIN_DEFINE(ObjectContainerBSDArchive)
 
 ObjectContainerBSDArchive::Object::Object() : ar_name() {}
@@ -55,6 +57,74 @@ void ObjectContainerBSDArchive::Object::Clear() {
   file_size = 0;
 }
 
+lldb::offset_t ObjectContainerBSDArchive::Object::ExtractFromThin(
+const DataExtractor &data, lldb::offset_t offset,
+llvm::StringRef stringTable) {
+  size_t ar_name_len = 0;
+  std::string str;
+  char *err;
+
+  // File header
+  //
+  // The common format is as follows.
+  //
+  //  Offset  Length   NameFormat
+  //  0   16  File name   ASCII right padded with spaces (no spaces
+  //  allowed in file name)
+  //  16  12  File modDecimal as cstring right padded with
+  //  spaces
+  //  28  6   Owner IDDecimal as cstring right padded with
+  //  spaces
+  //  34  6   Group IDDecimal as cstring right padded with
+  //  spaces
+  //  40  8   File mode   Octal   as cstring right padded with
+  //  spaces
+  //  48  10  File byte size  Decimal as cstring right padded with
+  //  spaces
+  //  58  2   File magic  0x60 0x0A
+
+  // Make sure there is enough data for the file header and bail if not
+  if (!data.ValidOffsetForDataOfSize(offset, 60))
+return LLDB_INVALID_OFFSET;
+
+  str.assign((const char *)data.GetData(&offset, 16), 16);
+  if (!(llvm::StringRef(str).startswith("//") || stringTable.empty())) {
+// Strip off any trailing spaces.
+const size_t last_pos = str.find_last_not_of(' ');
+if (last_pos != std::string::npos) {
+  if (last_pos + 1 < 16)
+str.erase(last_pos + 1);
+}
+int start = strtoul(str.c_str() + 1, &err, 10);
+int end = stringTable.find('\n', start);
+str.assign(stringTable.data() + start, end - start - 1);
+ar_name.SetCString(str.c_str());
+  }
+
+  str.assign((const char *)data.GetData(&offset, 12), 12);
+  modification_time = strtoul(str.c_str(), &err, 10);
+
+  str.assign((const char *)data.GetData(&offset, 6), 6);
+  uid = strtoul(str.c_str(), &err, 10);
+
+  str.assign((const char *)data.GetData(&offset, 6), 6);
+  gid = strtoul(str.c_str(), &err, 10);
+
+  str.assign((const char *)data.GetData(&offset, 8), 8);
+  mode = strtoul(str.c_str(), &err, 8);
+
+  str.assign((const char *)data.GetData(&offset, 10), 10);
+  size = strtoul(str.c_str(), &err, 10);
+
+  str.assign((const char *)data.GetData(&offset, 2), 2);
+  if (str == ARFMAG) {
+file_offset = offset;
+file_size = size - ar_name_len;
+return offset;
+  }
+  return LLDB_INVALID_OFFSET;
+}
+
 lldb::offset_t
 ObjectContainerBSDArchive::Object::Extract(const DataExtractor &data,
lldb::offset_t offset) {
@@ -136,9 +206,10 @@ ObjectContainerBSDArchive::Object::Extract(const 
DataExtractor &data,
 ObjectContainerBSDArchive::Archive::Archive(const lldb_private::ArchSpec &arch,
 const llvm::sys::TimePoint<> &time,
 lldb::offset_t file_offset,
-lldb_private::DataExtractor &data)
+lldb_private::DataExtractor &data,
+ArchiveType archive_type)
 : m_arch(arch), m_modification_time(tim

[Lldb-commits] [PATCH] D126464: [lldb] Add support to load object files from thin archives

2022-07-05 Thread Nico Weber via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGdc46ae6df5f7: [lldb] Add support to load object files from 
thin archives (authored by PRESIDENT810, committed by thakis).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126464

Files:
  lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.cpp
  lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.h
  lldb/test/API/functionalities/archives/Makefile
  lldb/test/API/functionalities/archives/TestBSDArchives.py
  lldb/test/API/functionalities/archives/c.c
  lldb/test/API/lit.cfg.py
  lldb/test/CMakeLists.txt

Index: lldb/test/CMakeLists.txt
===
--- lldb/test/CMakeLists.txt
+++ lldb/test/CMakeLists.txt
@@ -148,6 +148,7 @@
   llvm-objcopy
   llvm-pdbutil
   llvm-readobj
+  llvm-ar
   )
 
 if(TARGET lld)
Index: lldb/test/API/lit.cfg.py
===
--- lldb/test/API/lit.cfg.py
+++ lldb/test/API/lit.cfg.py
@@ -162,6 +162,10 @@
 if is_configured('llvm_libs_dir'):
   dotest_cmd += ['--env', 'LLVM_LIBS_DIR=' + config.llvm_libs_dir]
 
+# This path may be needed to locate required llvm tools
+if is_configured('llvm_tools_dir'):
+  dotest_cmd += ['--env', 'LLVM_TOOLS_DIR=' + config.llvm_tools_dir]
+
 # Forward ASan-specific environment variables to tests, as a test may load an
 # ASan-ified dylib.
 for env_var in ('ASAN_OPTIONS', 'DYLD_INSERT_LIBRARIES'):
Index: lldb/test/API/functionalities/archives/c.c
===
--- /dev/null
+++ lldb/test/API/functionalities/archives/c.c
@@ -0,0 +1,11 @@
+static int __c_global = 3;
+
+int c(int arg) {
+  int result = arg + __c_global;
+  return result;
+}
+
+int cc(int arg1) {
+  int result3 = arg1 - __c_global;
+  return result3;
+}
Index: lldb/test/API/functionalities/archives/TestBSDArchives.py
===
--- lldb/test/API/functionalities/archives/TestBSDArchives.py
+++ lldb/test/API/functionalities/archives/TestBSDArchives.py
@@ -58,3 +58,10 @@
 self.expect("frame variable", VARIABLES_DISPLAYED_CORRECTLY,
 substrs=['(int) arg = 2'])
 self.expect_var_path("__b_global", type="int", value="2")
+
+# Test loading thin archives
+archive_path = self.getBuildArtifact("libbar.a")
+module_specs = lldb.SBModuleSpecList.GetModuleSpecifications(archive_path)
+num_specs = module_specs.GetSize()
+self.assertEqual(num_specs, 1)
+self.assertEqual(module_specs.GetSpecAtIndex(0).GetObjectName(), "c.o")
Index: lldb/test/API/functionalities/archives/Makefile
===
--- lldb/test/API/functionalities/archives/Makefile
+++ lldb/test/API/functionalities/archives/Makefile
@@ -1,8 +1,8 @@
-C_SOURCES := main.c a.c b.c
+C_SOURCES := main.c a.c b.c c.c
 EXE :=  # Define a.out explicitly
 MAKE_DSYM := NO
 
-all: a.out
+all: a.out libbar.a
 
 a.out: main.o libfoo.a
 	$(LD) $(LDFLAGS) $^ -o $@
@@ -11,4 +11,11 @@
 	$(AR) $(ARFLAGS) $@ $^
 	$(RM) $^
 
+# This tests whether lldb can load a thin archive
+libbar.a: c.o
+	$(eval LLVM_AR := $(LLVM_TOOLS_DIR)/llvm-ar)
+	$(eval LLVM_ARFLAGS := -rcsDT)
+	$(LLVM_AR) $(LLVM_ARFLAGS) $@ $^
+	# Note for thin archive case, we cannot remove c.o
+
 include Makefile.rules
Index: lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.h
===
--- lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.h
+++ lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.h
@@ -15,19 +15,24 @@
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/FileSpec.h"
 
+#include "llvm/Object/Archive.h"
 #include "llvm/Support/Chrono.h"
+#include "llvm/Support/Path.h"
 
 #include 
 #include 
 #include 
 
+enum class ArchiveType { Invalid, Archive, ThinArchive };
+
 class ObjectContainerBSDArchive : public lldb_private::ObjectContainer {
 public:
   ObjectContainerBSDArchive(const lldb::ModuleSP &module_sp,
 lldb::DataBufferSP &data_sp,
 lldb::offset_t data_offset,
 const lldb_private::FileSpec *file,
-lldb::offset_t offset, lldb::offset_t length);
+lldb::offset_t offset, lldb::offset_t length,
+ArchiveType archive_type);
 
   ~ObjectContainerBSDArchive() override;
 
@@ -54,7 +59,7 @@
 lldb::offset_t length,
 lldb_private::ModuleSpecList &specs);
 
-  static bool MagicBytesMatch(const lldb_private::DataExtractor &data);
+  static ArchiveType Magic

[Lldb-commits] [PATCH] D126464: [lldb] Add support to load object files from thin archives

2022-07-05 Thread Nico Weber via Phabricator via lldb-commits

thakis added a comment.

Done.

Thanks for the fix! :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126464

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

[Lldb-commits] [lldb] ba14e4d - [LLDB] Disable TestGdbRemoteFork* for Arm/AArch64 Linux

2022-07-05 Thread Muhammad Omair Javaid via lldb-commits


Author: Muhammad Omair Javaid
Date: 2022-07-05T13:45:26+04:00
New Revision: ba14e4d65cddb6057374c71232e17cbc03be974d

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

LOG: [LLDB] Disable TestGdbRemoteFork* for Arm/AArch64 Linux

This test is causing some trouble with LLDB Arm/AArch64 Linux buildbot.
I am disabling is temporarily to make buildbot green.

Added: 


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

Removed: 




diff  --git a/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py 
b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
index 0c169de3f4d06..a67c566d265ba 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemoteFork.py
@@ -7,6 +7,11 @@
 
 
 class TestGdbRemoteFork(GdbRemoteForkTestBase):
+def setUp(self):
+GdbRemoteForkTestBase.setUp(self)
+if self.getPlatform() == "linux" and self.getArchitecture() in ['arm', 
'aarch64']:
+self.skipTest("Unsupported for Arm/AArch64 Linux")
+
 @add_test_categories(["fork"])
 def test_fork_multithreaded(self):
 _, _, child_pid, _ = self.start_fork_test(["thread:new"]*2 + ["fork"])

diff  --git a/lldb/test/API/tools/lldb-server/TestGdbRemoteForkNonStop.py 
b/lldb/test/API/tools/lldb-server/TestGdbRemoteForkNonStop.py
index 764f238a51eea..7ba3764855137 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemoteForkNonStop.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemoteForkNonStop.py
@@ -5,6 +5,11 @@
 
 
 class TestGdbRemoteForkNonStop(GdbRemoteForkTestBase):
+def setUp(self):
+GdbRemoteForkTestBase.setUp(self)
+if self.getPlatform() == "linux" and self.getArchitecture() in ['arm', 
'aarch64']:
+self.skipTest("Unsupported for Arm/AArch64 Linux")
+
 @add_test_categories(["fork"])
 def test_vfork_nonstop(self):
 parent_pid, parent_tid = self.fork_and_detach_test("vfork",



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

[Lldb-commits] [lldb] 3b2496e - [LLDB] Skip TestTwoHitsOneActual.py on Arm/AArch64 Linux

2022-07-05 Thread Muhammad Omair Javaid via lldb-commits


Author: Muhammad Omair Javaid
Date: 2022-07-05T15:01:51+04:00
New Revision: 3b2496e8faae2ba99f2d56f11f1cb44b68c353ae

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

LOG: [LLDB] Skip TestTwoHitsOneActual.py on Arm/AArch64 Linux

This test has some race condition which is making it hang on LLDB
Arm/AArch64 Linux buildbot. I am marking it as skipped until we
investigate whats going wrong.

Added: 


Modified: 

lldb/test/API/functionalities/breakpoint/two_hits_one_actual/TestTwoHitsOneActual.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/breakpoint/two_hits_one_actual/TestTwoHitsOneActual.py
 
b/lldb/test/API/functionalities/breakpoint/two_hits_one_actual/TestTwoHitsOneActual.py
index 530e715d3c3a9..cd3c314358f75 100644
--- 
a/lldb/test/API/functionalities/breakpoint/two_hits_one_actual/TestTwoHitsOneActual.py
+++ 
b/lldb/test/API/functionalities/breakpoint/two_hits_one_actual/TestTwoHitsOneActual.py
@@ -14,6 +14,7 @@ class TestTwoHitsOneActual(TestBase):
 
 NO_DEBUG_INFO_TESTCASE = True
 
+@skipIf(oslist=["linux"], archs=["arm", "aarch64"])
 def test_two_hits_one_actual(self):
 """There can be many tests in a test case - describe this test here."""
 self.build()



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

[Lldb-commits] [lldb] b7b1109 - [LLDB] Fix decorator import in TestTwoHitsOneActual.py

2022-07-05 Thread Muhammad Omair Javaid via lldb-commits


Author: Muhammad Omair Javaid
Date: 2022-07-05T15:26:26+04:00
New Revision: b7b11091efd2f9e5e90bc449b1c2591ae0af9648

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

LOG: [LLDB] Fix decorator import in TestTwoHitsOneActual.py

Added: 


Modified: 

lldb/test/API/functionalities/breakpoint/two_hits_one_actual/TestTwoHitsOneActual.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/breakpoint/two_hits_one_actual/TestTwoHitsOneActual.py
 
b/lldb/test/API/functionalities/breakpoint/two_hits_one_actual/TestTwoHitsOneActual.py
index cd3c314358f7..6e5c721b6900 100644
--- 
a/lldb/test/API/functionalities/breakpoint/two_hits_one_actual/TestTwoHitsOneActual.py
+++ 
b/lldb/test/API/functionalities/breakpoint/two_hits_one_actual/TestTwoHitsOneActual.py
@@ -7,6 +7,7 @@
 
 import lldb
 import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 
 



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

[Lldb-commits] [PATCH] D126655: [lldb] [gdb-remote] Be more explicit about notification reading

2022-07-05 Thread Michał Górny via Phabricator via lldb-commits

mgorny abandoned this revision.
mgorny added a comment.

I'm going to merge this into D126614 .


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

https://reviews.llvm.org/D126655

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

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

2022-07-05 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added inline comments.



Comment at: lldb/source/Core/Debugger.cpp:1840-1841
   return;
-if (data->GetCompleted())
-  m_current_event_id.reset();
   } else {

We should still reset the `m_current_event_id` here, otherwise we can get out 
of sync if someone changes the value of `GetShowProgress`. 


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

https://reviews.llvm.org/D128768

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

[Lldb-commits] [PATCH] D126614: [lldb] [gdb-remote] Client support for using the non-stop protocol

2022-07-05 Thread Michał Górny via Phabricator via lldb-commits

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



Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp:147
+// vCtrlC may not do anything, so timeout if we don't get notification
+if (ReadPacket(stop_response, milliseconds(500), false) ==
+PacketResult::Success) {

mgorny wrote:
> labath wrote:
> > This is unfortunate. Isn't there a better way to do this? Why aren't you 
> > using vCtrlC as all the comments say? Did you find any parallel to this in 
> > gdb?
> Yes, it is unfortunate. I'm going to think more and try to figure out a 
> better way. I was originally thinking of simply not waiting for the response 
> and instead letting it come after and then taking care of any pending 
> notifications before sending the next continue packet (this would require 
> merging D126655 first) but I'm somewhat afraid of race conditions. Though 
> maybe unnecessarily.
> 
> Though OTOH I guess 500 ms may be insufficient for debugging over the 
> Internet, and this would lead to even worse results.
> 
> As for `vCtrlC`, I've changed the code to use `vCont;t` as the former didn't 
> work well with gdbserver (and the latter is also more correct wrt the 
> protocol). I've forgotten to update the comments.
Ok, I've been thinking and thinking about this and I have a better idea. 
However, since this is not needed for LLGS and only for true gdbserver, I'm 
going to split it into a separate patch.

My idea is basically to, in a loop:

1. issue `vCont;t` to request stopping all threads.
2. issue `?` to get the list of all stopped threads.
3. issue `qfThreadInfo` to get list of all threads.

and basically do this until both lists are the same. That should take care of 
pretty much every corner case and race condition I can think of.



Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h:62
 
+  bool GetNonStopProtocol() { return m_non_stop_protocol; }
+

mgorny wrote:
> labath wrote:
> > Maybe call this `GetNonStopEnabled` or `GetNonStopInUse` ?
> I kinda wanted to emphasize 'protocol' here, to make it clear we aren't 
> introducing full-scale non-stop support.
Thinking about it again, I suppose you're right.


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

https://reviews.llvm.org/D126614

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

[Lldb-commits] [lldb] f51c47d - Revert "[lldb/test] Don't use preexec_fn for launching inferiors"

2022-07-05 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2022-07-05T10:12:57-07:00
New Revision: f51c47d987917d18108f0415334f47c75db9e908

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

LOG: Revert "[lldb/test] Don't use preexec_fn for launching inferiors"

This reverts commit b15b1421bc9a11b318b65b489e5fd58dd917db1f because it
breaks GreenDragon [1]. The bot has been red for several days, so
reverting to green while I take a look.

[1] https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/45012/

Added: 
lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.py

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

Removed: 
lldb/test/API/tools/lldb-server/attach-wait/Makefile
lldb/test/API/tools/lldb-server/attach-wait/TestGdbRemoteAttachWait.py
lldb/test/API/tools/lldb-server/attach-wait/main.cpp
lldb/test/API/tools/lldb-server/attach-wait/shim.cpp



diff  --git a/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py 
b/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
index 719131c9248e8..e14c4f8c0d383 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
@@ -4,11 +4,12 @@
 from __future__ import absolute_import
 
 # System modules
+import ctypes
 import itertools
+import os
 import re
 import subprocess
 import sys
-import os
 
 # Third-party modules
 import six
@@ -191,3 +192,14 @@ def hasChattyStderr(test_case):
 if match_android_device(test_case.getArchitecture(), ['aarch64'], 
range(22, 25+1)):
 return True  # The dynamic linker on the device will complain about 
unknown DT entries
 return False
+
+if getHostPlatform() == "linux":
+def enable_attach():
+"""Enable attaching to _this_ process, if host requires such an action.
+Suitable for use as a preexec_fn in subprocess.Popen and similar."""
+c = ctypes.CDLL(None)
+PR_SET_PTRACER = ctypes.c_int(0x59616d61)
+PR_SET_PTRACER_ANY = ctypes.c_ulong(-1)
+c.prctl(PR_SET_PTRACER, PR_SET_PTRACER_ANY)
+else:
+enable_attach = None

diff  --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py 
b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index d46e54f30bd55..22851730153e0 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -393,6 +393,7 @@ def launch(self, executable, args, extra_env):
 stdout=open(
 os.devnull) if not self._trace_on else None,
 stdin=PIPE,
+preexec_fn=lldbplatformutil.enable_attach,
 env=env)
 
 def terminate(self):

diff  --git 
a/lldb/test/API/tools/lldb-server/attach-wait/TestGdbRemoteAttachWait.py 
b/lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.py
similarity index 80%
rename from 
lldb/test/API/tools/lldb-server/attach-wait/TestGdbRemoteAttachWait.py
rename to lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.py
index abcc98d22090c..ed31b5645c336 100644
--- a/lldb/test/API/tools/lldb-server/attach-wait/TestGdbRemoteAttachWait.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.py
@@ -14,12 +14,10 @@ class 
TestGdbRemoteAttachWait(gdbremote_testcase.GdbRemoteTestCaseBase):
 @skipIfWindows # This test is flaky on Windows
 def test_attach_with_vAttachWait(self):
 exe = '%s_%d' % (self.testMethodName, os.getpid())
-exe_to_attach = exe
-args = []
 
 def launch_inferior():
 inferior = self.launch_process_for_attach(
-inferior_args=args,
+inferior_args=["sleep:60"],
 exe_path=self.getBuildArtifact(exe))
 self.assertIsNotNone(inferior)
 self.assertTrue(inferior.pid > 0)
@@ -28,14 +26,7 @@ def launch_inferior():
 inferior.pid, True))
 return inferior
 
-self.build(dictionary={'EXE': exe, 'CXX_SOURCES': 'main.cpp'})
-if self.getPlatform() != "windows":
-# Use a shim to ensure that the process is ready to be attached 
from
-# the get-go.
-args = [self.getBuildArtifact(exe)]
-exe = "shim"
-self.build(dictionary={'EXE': exe, 'CXX_SOURCES': 'shim.cpp'})
-
+self.build(dictionary={'EXE': exe})
 self.set_inferior_startup_attach_manually()
 
 server = self.connect_to_debug_monitor()
@@ -47,8 +38,7 @@ def launch_inferior():
 self.do_handshake()
 self.test_sequence.add_log_lines([
 # Do the attach.
-"read packet: $vAttachWait;{}#00".format(
-lldbgdbserverutils.gdbremote_hex_encode_string(exe_to_attach)),
+"read packet: 
$vAttachWa

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

2022-07-05 Thread Med Ismail Bennani via Phabricator via lldb-commits

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

Address @JDevlieghere comment


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

https://reviews.llvm.org/D128768

Files:
  lldb/source/Core/Debugger.cpp


Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1835,9 +1835,20 @@
   // going to show the progress.
   const uint64_t id = data->GetID();
   if (m_current_event_id) {
+Log *log = GetLog(LLDBLog::Events);
+if (log && log->GetVerbose()) {
+  StreamString log_stream;
+  log_stream.AsRawOstream()
+  << static_cast(this) << " Debugger(" << GetID()
+  << ")::HandleProgressEvent( m_current_event_id = "
+  << *m_current_event_id << ", data = { ";
+  data->Dump(&log_stream);
+  log_stream << " } )";
+  log->PutString(log_stream.GetString());
+}
 if (id != *m_current_event_id)
   return;
-if (data->GetCompleted())
+if (data->GetCompleted() == data->GetTotal())
   m_current_event_id.reset();
   } else {
 m_current_event_id = id;
@@ -1860,7 +1871,7 @@
   // Print over previous line, if any.
   output->Printf("\r");
 
-  if (data->GetCompleted()) {
+  if (data->GetCompleted() == data->GetTotal()) {
 // Clear the current line.
 output->Printf("\x1B[2K");
 output->Flush();


Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1835,9 +1835,20 @@
   // going to show the progress.
   const uint64_t id = data->GetID();
   if (m_current_event_id) {
+Log *log = GetLog(LLDBLog::Events);
+if (log && log->GetVerbose()) {
+  StreamString log_stream;
+  log_stream.AsRawOstream()
+  << static_cast(this) << " Debugger(" << GetID()
+  << ")::HandleProgressEvent( m_current_event_id = "
+  << *m_current_event_id << ", data = { ";
+  data->Dump(&log_stream);
+  log_stream << " } )";
+  log->PutString(log_stream.GetString());
+}
 if (id != *m_current_event_id)
   return;
-if (data->GetCompleted())
+if (data->GetCompleted() == data->GetTotal())
   m_current_event_id.reset();
   } else {
 m_current_event_id = id;
@@ -1860,7 +1871,7 @@
   // Print over previous line, if any.
   output->Printf("\r");
 
-  if (data->GetCompleted()) {
+  if (data->GetCompleted() == data->GetTotal()) {
 // Clear the current line.
 output->Printf("\x1B[2K");
 output->Flush();
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 5cca2ef - [LLDB] Remove TestLoadUnload.py Arm/Linux Xfail decorator

2022-07-05 Thread Muhammad Omair Javaid via lldb-commits


Author: Muhammad Omair Javaid
Date: 2022-07-06T01:14:40+04:00
New Revision: 5cca2ef3c35a08ce439e65e04a8d18d5bb4a6e4e

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

LOG: [LLDB] Remove TestLoadUnload.py Arm/Linux Xfail decorator

This patch removes Xfail decorator from TestLoadUnload.py as it is now
passing on Arm/Linux buildbot.

Added: 


Modified: 
lldb/test/API/functionalities/load_unload/TestLoadUnload.py

Removed: 




diff  --git a/lldb/test/API/functionalities/load_unload/TestLoadUnload.py 
b/lldb/test/API/functionalities/load_unload/TestLoadUnload.py
index eb5c5592cd8c7..58123251a64e5 100644
--- a/lldb/test/API/functionalities/load_unload/TestLoadUnload.py
+++ b/lldb/test/API/functionalities/load_unload/TestLoadUnload.py
@@ -295,7 +295,6 @@ def run_lldb_process_load_and_unload_commands(self):
 self.runCmd("process continue")
 
 @expectedFailureAll(oslist=["windows"]) # breakpoint not hit
-@expectedFailureAll(oslist=["linux"], archs=["arm"]) # Fails on ubuntu 
jammy
 def test_load_unload(self):
 self.setSvr4Support(False)
 self.run_load_unload()



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

[Lldb-commits] [lldb] bb9b30f - [LLDB] Remove TestLoadUnload.py Arm/Linux Xfail decorator

2022-07-05 Thread Muhammad Omair Javaid via lldb-commits


Author: Muhammad Omair Javaid
Date: 2022-07-06T01:39:52+04:00
New Revision: bb9b30ffbe7c80c94807f01eb31429e4f9f2ce2e

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

LOG: [LLDB] Remove TestLoadUnload.py Arm/Linux Xfail decorator

This is a follow up on my last commit where one of the decorator was
left unremoved.

This patch removes Xfail decorator from TestLoadUnload.py as it is now
passing on Arm/Linux buildbot.

Added: 


Modified: 
lldb/test/API/functionalities/load_unload/TestLoadUnload.py

Removed: 




diff  --git a/lldb/test/API/functionalities/load_unload/TestLoadUnload.py 
b/lldb/test/API/functionalities/load_unload/TestLoadUnload.py
index 58123251a64e..70d08f4884c0 100644
--- a/lldb/test/API/functionalities/load_unload/TestLoadUnload.py
+++ b/lldb/test/API/functionalities/load_unload/TestLoadUnload.py
@@ -201,7 +201,6 @@ def test_dyld_library_path(self):
 hostoslist=["windows"],
 triple='.*-android')
 @expectedFailureAll(oslist=["windows"]) # process load not implemented
-@expectedFailureAll(oslist=["linux"], archs=["arm"]) # Fails on ubuntu 
jammy
 def test_lldb_process_load_and_unload_commands(self):
 self.setSvr4Support(False)
 self.run_lldb_process_load_and_unload_commands()



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

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

2022-07-05 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

LGTM


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

https://reviews.llvm.org/D128768

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

[Lldb-commits] [lldb] ea8b811 - [lldb/Core] Fix finite progress event reporting

2022-07-05 Thread Med Ismail Bennani via lldb-commits


Author: Med Ismail Bennani
Date: 2022-07-05T16:25:40-07:00
New Revision: ea8b811bf800680e7d7bde1e8d6ff43f8ecf17cf

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

LOG: [lldb/Core] Fix finite progress event reporting

This patch should fix event handling for finite progress reports.

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

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

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

rdar://91788326

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

Signed-off-by: Med Ismail Bennani 

Added: 


Modified: 
lldb/source/Core/Debugger.cpp

Removed: 




diff  --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index f17cd8856a6db..62857c181af89 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -1835,9 +1835,20 @@ void Debugger::HandleProgressEvent(const lldb::EventSP 
&event_sp) {
   // going to show the progress.
   const uint64_t id = data->GetID();
   if (m_current_event_id) {
+Log *log = GetLog(LLDBLog::Events);
+if (log && log->GetVerbose()) {
+  StreamString log_stream;
+  log_stream.AsRawOstream()
+  << static_cast(this) << " Debugger(" << GetID()
+  << ")::HandleProgressEvent( m_current_event_id = "
+  << *m_current_event_id << ", data = { ";
+  data->Dump(&log_stream);
+  log_stream << " } )";
+  log->PutString(log_stream.GetString());
+}
 if (id != *m_current_event_id)
   return;
-if (data->GetCompleted())
+if (data->GetCompleted() == data->GetTotal())
   m_current_event_id.reset();
   } else {
 m_current_event_id = id;
@@ -1860,7 +1871,7 @@ void Debugger::HandleProgressEvent(const lldb::EventSP 
&event_sp) {
   // Print over previous line, if any.
   output->Printf("\r");
 
-  if (data->GetCompleted()) {
+  if (data->GetCompleted() == data->GetTotal()) {
 // Clear the current line.
 output->Printf("\x1B[2K");
 output->Flush();



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

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

2022-07-05 Thread Med Ismail Bennani via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGea8b811bf800: [lldb/Core] Fix finite progress event 
reporting (authored by mib).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128768

Files:
  lldb/source/Core/Debugger.cpp


Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1835,9 +1835,20 @@
   // going to show the progress.
   const uint64_t id = data->GetID();
   if (m_current_event_id) {
+Log *log = GetLog(LLDBLog::Events);
+if (log && log->GetVerbose()) {
+  StreamString log_stream;
+  log_stream.AsRawOstream()
+  << static_cast(this) << " Debugger(" << GetID()
+  << ")::HandleProgressEvent( m_current_event_id = "
+  << *m_current_event_id << ", data = { ";
+  data->Dump(&log_stream);
+  log_stream << " } )";
+  log->PutString(log_stream.GetString());
+}
 if (id != *m_current_event_id)
   return;
-if (data->GetCompleted())
+if (data->GetCompleted() == data->GetTotal())
   m_current_event_id.reset();
   } else {
 m_current_event_id = id;
@@ -1860,7 +1871,7 @@
   // Print over previous line, if any.
   output->Printf("\r");
 
-  if (data->GetCompleted()) {
+  if (data->GetCompleted() == data->GetTotal()) {
 // Clear the current line.
 output->Printf("\x1B[2K");
 output->Flush();


Index: lldb/source/Core/Debugger.cpp
===
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1835,9 +1835,20 @@
   // going to show the progress.
   const uint64_t id = data->GetID();
   if (m_current_event_id) {
+Log *log = GetLog(LLDBLog::Events);
+if (log && log->GetVerbose()) {
+  StreamString log_stream;
+  log_stream.AsRawOstream()
+  << static_cast(this) << " Debugger(" << GetID()
+  << ")::HandleProgressEvent( m_current_event_id = "
+  << *m_current_event_id << ", data = { ";
+  data->Dump(&log_stream);
+  log_stream << " } )";
+  log->PutString(log_stream.GetString());
+}
 if (id != *m_current_event_id)
   return;
-if (data->GetCompleted())
+if (data->GetCompleted() == data->GetTotal())
   m_current_event_id.reset();
   } else {
 m_current_event_id = id;
@@ -1860,7 +1871,7 @@
   // Print over previous line, if any.
   output->Printf("\r");
 
-  if (data->GetCompleted()) {
+  if (data->GetCompleted() == data->GetTotal()) {
 // Clear the current line.
 output->Printf("\x1B[2K");
 output->Flush();
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D129166: [lldb] Make sure we use the libc++ from the build dir

2022-07-05 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

Make sure we use the libc++ from the build dir. Currently, by passing 
`-stdlib=libc++`, we might pick up the system libc++ on macOS. This change 
ensures that if `LLVM_LIBS_DIR` is set, we try to use the libc++ from there.


https://reviews.llvm.org/D129166

Files:
  lldb/packages/Python/lldbsuite/test/make/Makefile.rules


Index: lldb/packages/Python/lldbsuite/test/make/Makefile.rules
===
--- lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -392,12 +392,13 @@
# Nothing to do, this is already handled in
# Android.rules.
else
-   CXXFLAGS += -stdlib=libc++
-   LDFLAGS += -stdlib=libc++
-   endif
-   ifneq (,$(filter $(OS), FreeBSD Linux NetBSD))
ifneq (,$(LLVM_LIBS_DIR))
-   LDFLAGS += -Wl,-rpath,$(LLVM_LIBS_DIR)
+   # Use the libc++ from the build dir
+   CXXFLAGS += -nostdlib++
+   LDFLAGS += -L$(LLVM_LIBS_DIR) 
-Wl,-rpath,$(LLVM_LIBS_DIR) -lc++
+   else
+   CXXFLAGS += -stdlib=libc++
+   LDFLAGS += -stdlib=libc++
endif
endif
 endif


Index: lldb/packages/Python/lldbsuite/test/make/Makefile.rules
===
--- lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -392,12 +392,13 @@
 		# Nothing to do, this is already handled in
 		# Android.rules.
 	else
-		CXXFLAGS += -stdlib=libc++
-		LDFLAGS += -stdlib=libc++
-	endif
-	ifneq (,$(filter $(OS), FreeBSD Linux NetBSD))
 		ifneq (,$(LLVM_LIBS_DIR))
-			LDFLAGS += -Wl,-rpath,$(LLVM_LIBS_DIR)
+			# Use the libc++ from the build dir
+			CXXFLAGS += -nostdlib++
+			LDFLAGS += -L$(LLVM_LIBS_DIR) -Wl,-rpath,$(LLVM_LIBS_DIR) -lc++
+		else
+			CXXFLAGS += -stdlib=libc++
+			LDFLAGS += -stdlib=libc++
 		endif
 	endif
 endif
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

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

2022-07-05 Thread Sujin Park via Phabricator via lldb-commits

persona0220 updated this revision to Diff 442417.
persona0220 marked 13 inline comments as done.
persona0220 added a comment.

- Terms ‘control flow kind’ and ‘control flow type’ was mixed across source 
codes -> Unify into ‘control flow kind’
- Divide the ‘GetControlFlowKind’ function into two functions’:
  - InstructionLengthDecode: Decode raw instruction bytes into opcode, modrm 
and map.
  - MapOpcodeIntoControlFlowKind: Get control flow find from opcode, modrm and 
map.
- Add detailed comments for each new functions.


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

https://reviews.llvm.org/D128477

Files:
  lldb/include/lldb/Core/Disassembler.h
  lldb/include/lldb/Target/TraceDumper.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/API/SBInstruction.cpp
  lldb/source/API/SBInstructionList.cpp
  lldb/source/Commands/CommandObjectDisassemble.cpp
  lldb/source/Commands/CommandObjectDisassemble.h
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/Options.td
  lldb/source/Core/Disassembler.cpp
  lldb/source/Core/DumpDataExtractor.cpp
  lldb/source/Expression/IRExecutionUnit.cpp
  lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp
  
lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
  lldb/source/Symbol/Function.cpp
  lldb/source/Symbol/Symbol.cpp
  lldb/source/Target/ThreadPlanTracer.cpp
  lldb/source/Target/TraceDumper.cpp

Index: lldb/source/Target/TraceDumper.cpp
===
--- lldb/source/Target/TraceDumper.cpp
+++ lldb/source/Target/TraceDumper.cpp
@@ -147,14 +147,14 @@
   m_s.Format("{0:x+16}", item.load_address);
   if (item.symbol_info) {
 m_s << "";
-item.symbol_info->instruction->Dump(&m_s, /*max_opcode_byte_size=*/0,
-/*show_address=*/false,
-/*show_bytes=*/false,
-&item.symbol_info->exe_ctx,
-&item.symbol_info->sc,
-/*prev_sym_ctx=*/nullptr,
-/*disassembly_addr_format=*/nullptr,
-/*max_address_text_size=*/0);
+item.symbol_info->instruction->Dump(
+&m_s, /*max_opcode_byte_size=*/0,
+/*show_address=*/false,
+/*show_bytes=*/false, m_options.show_control_flow_kind,
+&item.symbol_info->exe_ctx, &item.symbol_info->sc,
+/*prev_sym_ctx=*/nullptr,
+/*disassembly_addr_format=*/nullptr,
+/*max_address_text_size=*/0);
   }
 }
 
Index: lldb/source/Target/ThreadPlanTracer.cpp
===
--- lldb/source/Target/ThreadPlanTracer.cpp
+++ lldb/source/Target/ThreadPlanTracer.cpp
@@ -170,13 +170,14 @@
   if (instruction_list.GetSize()) {
 const bool show_bytes = true;
 const bool show_address = true;
+const bool show_control_flow_kind = true;
 Instruction *instruction =
 instruction_list.GetInstructionAtIndex(0).get();
 const FormatEntity::Entry *disassemble_format =
 m_process.GetTarget().GetDebugger().GetDisassemblyFormat();
 instruction->Dump(stream, max_opcode_byte_size, show_address,
-  show_bytes, nullptr, nullptr, nullptr,
-  disassemble_format, 0);
+  show_bytes, show_control_flow_kind, nullptr, nullptr,
+  nullptr, disassemble_format, 0);
   }
 }
   }
Index: lldb/source/Symbol/Symbol.cpp
===
--- lldb/source/Symbol/Symbol.cpp
+++ lldb/source/Symbol/Symbol.cpp
@@ -558,8 +558,9 @@
   if (disassembler_sp) {
 const bool show_address = true;
 const bool show_bytes = false;
-disassembler_sp->GetInstructionList().Dump(&strm, show_address, show_bytes,
-   &exe_ctx);
+const bool show_control_flow_kind = false;
+disassembler_sp->GetInstructionList().Dump(
+&strm, show_address, show_bytes, show_control_flow_kind, &exe_ctx);
 return true;
   }
   return false;
Index: lldb/source/Symbol/Function.cpp
===
--- lldb/source/Symbol/Function.cpp
+++ lldb/source/Symbol/Function.cpp
@@ -440,8 +440,9 @@
   if (disassembler_sp) {
 const bool show_address = true;
 const bool show_bytes = false;
-disassembler_sp->GetInstructionList().Dump(&strm, show_address, show_bytes,
-   &exe_ctx);
+const bool show_control_flow_kind = false;
+disassembler_sp->GetInstructionList().Dump(
+&strm, show_address, show_bytes, show_control_flow_kind, &exe_ctx);
 return true;
   }
   re

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

2022-07-05 Thread Sujin Park via Phabricator via lldb-commits

persona0220 updated this revision to Diff 442420.
persona0220 marked an inline comment as done.
persona0220 added a comment.

Add some more documentation


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

https://reviews.llvm.org/D128477

Files:
  lldb/include/lldb/Core/Disassembler.h
  lldb/include/lldb/Target/TraceDumper.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/API/SBInstruction.cpp
  lldb/source/API/SBInstructionList.cpp
  lldb/source/Commands/CommandObjectDisassemble.cpp
  lldb/source/Commands/CommandObjectDisassemble.h
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/Options.td
  lldb/source/Core/Disassembler.cpp
  lldb/source/Core/DumpDataExtractor.cpp
  lldb/source/Expression/IRExecutionUnit.cpp
  lldb/source/Plugins/TraceExporter/common/TraceHTR.cpp
  
lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
  lldb/source/Symbol/Function.cpp
  lldb/source/Symbol/Symbol.cpp
  lldb/source/Target/ThreadPlanTracer.cpp
  lldb/source/Target/TraceDumper.cpp

Index: lldb/source/Target/TraceDumper.cpp
===
--- lldb/source/Target/TraceDumper.cpp
+++ lldb/source/Target/TraceDumper.cpp
@@ -147,14 +147,14 @@
   m_s.Format("{0:x+16}", item.load_address);
   if (item.symbol_info) {
 m_s << "";
-item.symbol_info->instruction->Dump(&m_s, /*max_opcode_byte_size=*/0,
-/*show_address=*/false,
-/*show_bytes=*/false,
-&item.symbol_info->exe_ctx,
-&item.symbol_info->sc,
-/*prev_sym_ctx=*/nullptr,
-/*disassembly_addr_format=*/nullptr,
-/*max_address_text_size=*/0);
+item.symbol_info->instruction->Dump(
+&m_s, /*max_opcode_byte_size=*/0,
+/*show_address=*/false,
+/*show_bytes=*/false, m_options.show_control_flow_kind,
+&item.symbol_info->exe_ctx, &item.symbol_info->sc,
+/*prev_sym_ctx=*/nullptr,
+/*disassembly_addr_format=*/nullptr,
+/*max_address_text_size=*/0);
   }
 }
 
Index: lldb/source/Target/ThreadPlanTracer.cpp
===
--- lldb/source/Target/ThreadPlanTracer.cpp
+++ lldb/source/Target/ThreadPlanTracer.cpp
@@ -170,13 +170,14 @@
   if (instruction_list.GetSize()) {
 const bool show_bytes = true;
 const bool show_address = true;
+const bool show_control_flow_kind = true;
 Instruction *instruction =
 instruction_list.GetInstructionAtIndex(0).get();
 const FormatEntity::Entry *disassemble_format =
 m_process.GetTarget().GetDebugger().GetDisassemblyFormat();
 instruction->Dump(stream, max_opcode_byte_size, show_address,
-  show_bytes, nullptr, nullptr, nullptr,
-  disassemble_format, 0);
+  show_bytes, show_control_flow_kind, nullptr, nullptr,
+  nullptr, disassemble_format, 0);
   }
 }
   }
Index: lldb/source/Symbol/Symbol.cpp
===
--- lldb/source/Symbol/Symbol.cpp
+++ lldb/source/Symbol/Symbol.cpp
@@ -558,8 +558,9 @@
   if (disassembler_sp) {
 const bool show_address = true;
 const bool show_bytes = false;
-disassembler_sp->GetInstructionList().Dump(&strm, show_address, show_bytes,
-   &exe_ctx);
+const bool show_control_flow_kind = false;
+disassembler_sp->GetInstructionList().Dump(
+&strm, show_address, show_bytes, show_control_flow_kind, &exe_ctx);
 return true;
   }
   return false;
Index: lldb/source/Symbol/Function.cpp
===
--- lldb/source/Symbol/Function.cpp
+++ lldb/source/Symbol/Function.cpp
@@ -440,8 +440,9 @@
   if (disassembler_sp) {
 const bool show_address = true;
 const bool show_bytes = false;
-disassembler_sp->GetInstructionList().Dump(&strm, show_address, show_bytes,
-   &exe_ctx);
+const bool show_control_flow_kind = false;
+disassembler_sp->GetInstructionList().Dump(
+&strm, show_address, show_bytes, show_control_flow_kind, &exe_ctx);
 return true;
   }
   return false;
Index: lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
===
--- lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
+++ lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
@@ -83,6 +83,7 @@
   const uint3

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

2022-07-05 Thread walter erquinigo via Phabricator via lldb-commits

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

let's try to have tests soon. It seems that the code can be simplified and 
tests will be very handy




Comment at: lldb/include/lldb/lldb-enumerations.h:976
+  eInstructionControlFlowKindUnknown = 0,
+  /// The instruction is something not listed below, i.e. it's sequential
+  /// instruction that doesn't affect the control flow of the program.





Comment at: lldb/source/Core/Disassembler.cpp:730-740
+/// \param[out] opcode
+///Primary opcode of the instruction.
+///
+/// \param[out] modrm
+///ModR/M byte of the instruction.
+///Bit[7:6] indicates MOD. Bit[5:3] specifies a register and R/M bit[2:0]
+///may contain a register or specify an addressing mode, depending on MOD.

I'm noticing that MapOpcodeIntoControlFlowKind only uses PTI_MAP_0 and 
PTI_MAP_1, which are opcodes of 1 and 2 bytes. Any opcode of 3 bytes and even 
amd3dnow is not used at all. That means that you don't need that enum, so 
please delete it. Instead, use the length of the opcode throughout the code.




Comment at: lldb/source/Core/Disassembler.cpp:882
+
+  if (m_opcode.GetOpcodeBytes() == nullptr || m_opcode.GetByteSize() <= 0) {
+return lldb::eInstructionControlFlowKindUnknown;

does this mean that all x86 and x86_64 instructions are categorized as 
Opcode::Type::eTypeBytes?
I'm asking because after reading GetOpcodeBytes, it only returns data if the 
type is eTypeBytes. Did you check that?



Comment at: lldb/source/Core/Disassembler.cpp:885
+  }
+  memcpy(inst_bytes, m_opcode.GetOpcodeBytes(), m_opcode.GetByteSize());
+

you don't need to copy GetOpcodeBytes, you can just pass it directly to 
InstructionLengthDecode


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

https://reviews.llvm.org/D128477

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

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

[Lldb-commits] [lldb] dc46ae6 - [lldb] Add support to load object files from thin archives

[Lldb-commits] [PATCH] D126464: [lldb] Add support to load object files from thin archives

[Lldb-commits] [PATCH] D126464: [lldb] Add support to load object files from thin archives

[Lldb-commits] [lldb] ba14e4d - [LLDB] Disable TestGdbRemoteFork* for Arm/AArch64 Linux

[Lldb-commits] [lldb] 3b2496e - [LLDB] Skip TestTwoHitsOneActual.py on Arm/AArch64 Linux

[Lldb-commits] [lldb] b7b1109 - [LLDB] Fix decorator import in TestTwoHitsOneActual.py

[Lldb-commits] [PATCH] D126655: [lldb] [gdb-remote] Be more explicit about notification reading

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

[Lldb-commits] [PATCH] D126614: [lldb] [gdb-remote] Client support for using the non-stop protocol

[Lldb-commits] [lldb] f51c47d - Revert "[lldb/test] Don't use preexec_fn for launching inferiors"

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

[Lldb-commits] [lldb] 5cca2ef - [LLDB] Remove TestLoadUnload.py Arm/Linux Xfail decorator

[Lldb-commits] [lldb] bb9b30f - [LLDB] Remove TestLoadUnload.py Arm/Linux Xfail decorator

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

[Lldb-commits] [lldb] ea8b811 - [lldb/Core] Fix finite progress event reporting

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

[Lldb-commits] [PATCH] D129166: [lldb] Make sure we use the libc++ from the build dir

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

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

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

21 matches

Site Navigation

Mail list logo

Footer information