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

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp:661-664
+if (mode != llvm::sys::fs::perms_not_known)
+  response.Printf("F%x", mode);
+else
+  response.Printf("F-1,%x", (int)Status(ec).GetError());

mgorny wrote:
> labath wrote:
> > Unless I'm mistaken, the test does not actually run this code (as it tests 
> > the client bits).
> Yes, this is the case. Adding tests for the server is a bit out of scope for 
> what I'm working on.
Why is that? You did have them in the other patches I looked at...

Batching these changes in this way is particularly unfortunate because a casual 
observer might conclude that you actualy _are_ including a test for the bug 
you've fixed.


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

https://reviews.llvm.org/D107809

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

[Lldb-commits] [PATCH] D101563: [lldb] [test] Extend aarch64-gp-read test to cover all registers

[Kamil Rytarowski via Phabricator via lldb-commits]

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

There is an unhandled comment.


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

https://reviews.llvm.org/D101563

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

[Lldb-commits] [PATCH] D109249: [lldb] Add Getdescription function for SBInstruction.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I'm not sure what's the exact use case here, but I /think/ that passing just 
the target part of the execution context will is not sufficient to enable all 
the disassembler bells and whistles. For example the regular "disassemble" 
command will also print a "-> " next to the instruction that references the 
current PC.

Even if that is not required for your use case, I think that a better (more 
generic) API would be for this function to take a SBExecutionContext argument, 
and let the user choose how it wants to populate it.




Comment at: lldb/source/API/SBInstruction.cpp:241-256
   lldb::InstructionSP inst_sp(GetOpaque());
   if (inst_sp) {
 SymbolContext sc;
 const Address &addr = inst_sp->GetAddress();
 ModuleSP module_sp(addr.GetModule());
 if (module_sp)
   module_sp->ResolveSymbolContextForAddress(addr, eSymbolContextEverything,

It seems like this function could now just delegate to the new version, passing 
a null/empty target or execution context


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109249

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

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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In the code and/or in the commit message.


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

https://reviews.llvm.org/D107811

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

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

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

mgorny added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp:661-664
+if (mode != llvm::sys::fs::perms_not_known)
+  response.Printf("F%x", mode);
+else
+  response.Printf("F-1,%x", (int)Status(ec).GetError());

labath wrote:
> mgorny wrote:
> > labath wrote:
> > > Unless I'm mistaken, the test does not actually run this code (as it 
> > > tests the client bits).
> > Yes, this is the case. Adding tests for the server is a bit out of scope 
> > for what I'm working on.
> Why is that? You did have them in the other patches I looked at...
> 
> Batching these changes in this way is particularly unfortunate because a 
> casual observer might conclude that you actualy _are_ including a test for 
> the bug you've fixed.
Just to be clear, are we talking of adding a generic server-side test for 
`vFile:mode`, or specifically one that triggers an error? Though thinking about 
it a bit more, using a file that does not exist should be good enough to 
trigger this.


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

https://reviews.llvm.org/D107809

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

[Lldb-commits] [PATCH] D107821: [lldb] [gdb-server] Add tests for more vFile packets

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

mgorny added a comment.

In D107821#2986450 , @labath wrote:

> Instead of the tempfile thingies I'd probably just use fixed files names in 
> the build directory -- it's specific to a single test, automatically cleaned 
> on each run and it's where all of our other "temporary" files end up

Done that. I'll do one more test run, then push.


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

https://reviews.llvm.org/D107821

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

[Lldb-commits] [PATCH] D109326: [lldb] [Process/FreeBSD] Support SaveCore() using PT_COREDUMP

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

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb07803ee2a97: [lldb] [Process/FreeBSD] Support SaveCore() 
using PT_COREDUMP (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109326

Files:
  lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h
  lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py
  lldb/test/API/tools/lldb-server/TestGdbRemoteSaveCore.py


Index: lldb/test/API/tools/lldb-server/TestGdbRemoteSaveCore.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteSaveCore.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteSaveCore.py
@@ -38,15 +38,15 @@
 self.assertTrue(process, PROCESS_IS_VALID)
 self.assertEqual(process.GetProcessID(), procs["inferior"].pid)
 
-@skipUnlessPlatform(oslist=["netbsd"])
+@skipUnlessPlatform(oslist=["freebsd", "netbsd"])
 def test_netbsd_path(self):
 core = lldbutil.append_to_process_working_directory(self, "core")
 self.coredump_test(core, core)
 
-@skipUnlessPlatform(oslist=["netbsd"])
+@skipUnlessPlatform(oslist=["freebsd", "netbsd"])
 def test_netbsd_no_path(self):
 self.coredump_test()
 
-@skipUnlessPlatform(oslist=["netbsd"])
+@skipUnlessPlatform(oslist=["freebsd", "netbsd"])
 def test_netbsd_bad_path(self):
 self.coredump_test("/dev/null/cantwritehere")
Index: lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py
===
--- lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py
+++ lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py
@@ -64,7 +64,7 @@
 if (os.path.isfile(core)):
 os.unlink(core)
 
-@skipUnlessPlatform(["netbsd"])
+@skipUnlessPlatform(["freebsd", "netbsd"])
 def test_save_core_via_process_plugin(self):
 self.build()
 exe = self.getBuildArtifact("a.out")
Index: lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h
===
--- lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h
+++ lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h
@@ -91,6 +91,8 @@
 
   bool SupportHardwareSingleStepping() const;
 
+  llvm::Expected SaveCore(llvm::StringRef path_hint) override;
+
 protected:
   llvm::Expected>
   GetSoftwareBreakpointTrapOpcode(size_t size_hint) override;
Index: lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
===
--- lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
+++ lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
@@ -131,7 +131,8 @@
 NativeProcessFreeBSD::Extension
 NativeProcessFreeBSD::Factory::GetSupportedExtensions() const {
   return Extension::multiprocess | Extension::fork | Extension::vfork |
- Extension::pass_signals | Extension::auxv | Extension::libraries_svr4;
+ Extension::pass_signals | Extension::auxv | Extension::libraries_svr4 
|
+ Extension::savecore;
 }
 
 // Public Instance Methods
@@ -1009,3 +1010,30 @@
 }
   }
 }
+
+llvm::Expected
+NativeProcessFreeBSD::SaveCore(llvm::StringRef path_hint) {
+  using namespace llvm::sys::fs;
+
+  llvm::SmallString<128> path{path_hint};
+  Status error;
+  struct ptrace_coredump pc = {};
+
+  // Try with the suggested path first.  If there is no suggested path or it
+  // failed to open, use a temporary file.
+  if (path.empty() ||
+  openFile(path, pc.pc_fd, CD_CreateNew, FA_Write, OF_None)) {
+if (std::error_code errc =
+createTemporaryFile("lldb", "core", pc.pc_fd, path))
+  return llvm::createStringError(errc, "Unable to create a temporary 
file");
+  }
+  error = PtraceWrapper(PT_COREDUMP, GetID(), &pc, sizeof(pc));
+
+  std::error_code close_err = closeFile(pc.pc_fd);
+  if (error.Fail())
+return error.ToError();
+  if (close_err)
+return llvm::createStringError(
+close_err, "Unable to close the core dump after writing");
+  return path.str().str();
+}


Index: lldb/test/API/tools/lldb-server/TestGdbRemoteSaveCore.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteSaveCore.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteSaveCore.py
@@ -38,15 +38,15 @@
 self.assertTrue(process, PROCESS_IS_VALID)
 self.assertEqual(process.GetProcessID(), procs["inferior"].pid)
 
-@skipUnlessPlatform(oslist=["netbsd"])
+@skipUnlessPlatform(oslist=["freebsd", "netbsd"])
 def test_netbsd_path(self):
 core = lldbutil.append_to

[Lldb-commits] [lldb] 39a2449 - [lldb] [Commands] Fix reporting errors in 'platform file read/write'

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-09-08T10:58:12+02:00
New Revision: 39a2449ea1333886a9c9104b5afb4ff9c4932403

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

LOG: [lldb] [Commands] Fix reporting errors in 'platform file read/write'

Fix 'platform file read' and 'platform file write' commands to correctly
detect erraneous return and report it as such.  Currently, errors were
implicitly printed as a return value of -1, and the commands were
assumed to be successful.

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

Added: 


Modified: 
lldb/source/Commands/CommandObjectPlatform.cpp
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectPlatform.cpp 
b/lldb/source/Commands/CommandObjectPlatform.cpp
index 68e1fa6c07b44..8dd0512141645 100644
--- a/lldb/source/Commands/CommandObjectPlatform.cpp
+++ b/lldb/source/Commands/CommandObjectPlatform.cpp
@@ -589,11 +589,15 @@ class CommandObjectPlatformFRead : public 
CommandObjectParsed {
   }
   std::string buffer(m_options.m_count, 0);
   Status error;
-  uint32_t retcode = platform_sp->ReadFile(
+  uint64_t retcode = platform_sp->ReadFile(
   fd, m_options.m_offset, &buffer[0], m_options.m_count, error);
-  result.AppendMessageWithFormat("Return = %d\n", retcode);
-  result.AppendMessageWithFormat("Data = \"%s\"\n", buffer.c_str());
-  result.SetStatus(eReturnStatusSuccessFinishResult);
+  if (retcode != UINT64_MAX) {
+result.AppendMessageWithFormat("Return = %" PRIu64 "\n", retcode);
+result.AppendMessageWithFormat("Data = \"%s\"\n", buffer.c_str());
+result.SetStatus(eReturnStatusSuccessFinishResult);
+  } else {
+result.AppendError(error.AsCString());
+  }
 } else {
   result.AppendError("no platform currently selected\n");
 }
@@ -678,11 +682,15 @@ class CommandObjectPlatformFWrite : public 
CommandObjectParsed {
   cmd_line);
 return result.Succeeded();
   }
-  uint32_t retcode =
+  uint64_t retcode =
   platform_sp->WriteFile(fd, m_options.m_offset, &m_options.m_data[0],
  m_options.m_data.size(), error);
-  result.AppendMessageWithFormat("Return = %d\n", retcode);
-  result.SetStatus(eReturnStatusSuccessFinishResult);
+  if (retcode != UINT64_MAX) {
+result.AppendMessageWithFormat("Return = %" PRIu64 "\n", retcode);
+result.SetStatus(eReturnStatusSuccessFinishResult);
+  } else {
+result.AppendError(error.AsCString());
+  }
 } else {
   result.AppendError("no platform currently selected\n");
 }

diff  --git 
a/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py 
b/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
index 264e11032ecaf..4ba23870e4a0e 100644
--- 
a/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
+++ 
b/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
@@ -60,11 +60,12 @@ def vFile(self, packet):
 self.match("platform file open /some/file.txt -v 0755",
[r"error: Invalid argument"],
error=True)
-# TODO: fix the commands to fail on unsuccessful result
 self.match("platform file read 16 -o 11 -c 13",
-   [r"Return = -1\nData = \"\""])
+   [r"error: Invalid argument"],
+   error=True)
 self.match("platform file write 16 -o 11 -d teststring",
-   [r"Return = -1"])
+   [r"error: Invalid argument"],
+   error=True)
 self.match("platform file close 16",
[r"error: Invalid argument"],
error=True)



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

[Lldb-commits] [lldb] b07803e - [lldb] [Process/FreeBSD] Support SaveCore() using PT_COREDUMP

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-09-08T10:58:12+02:00
New Revision: b07803ee2a97fdcf4ed6494d8d6593bf985a5150

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

LOG: [lldb] [Process/FreeBSD] Support SaveCore() using PT_COREDUMP

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

Added: 


Modified: 
lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h
lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py
lldb/test/API/tools/lldb-server/TestGdbRemoteSaveCore.py

Removed: 




diff  --git a/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp 
b/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
index d6426b3d23675..d795a43375ca7 100644
--- a/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
+++ b/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
@@ -131,7 +131,8 @@ NativeProcessFreeBSD::Factory::Attach(
 NativeProcessFreeBSD::Extension
 NativeProcessFreeBSD::Factory::GetSupportedExtensions() const {
   return Extension::multiprocess | Extension::fork | Extension::vfork |
- Extension::pass_signals | Extension::auxv | Extension::libraries_svr4;
+ Extension::pass_signals | Extension::auxv | Extension::libraries_svr4 
|
+ Extension::savecore;
 }
 
 // Public Instance Methods
@@ -1009,3 +1010,30 @@ void NativeProcessFreeBSD::MonitorClone(::pid_t 
child_pid, bool is_vfork,
 }
   }
 }
+
+llvm::Expected
+NativeProcessFreeBSD::SaveCore(llvm::StringRef path_hint) {
+  using namespace llvm::sys::fs;
+
+  llvm::SmallString<128> path{path_hint};
+  Status error;
+  struct ptrace_coredump pc = {};
+
+  // Try with the suggested path first.  If there is no suggested path or it
+  // failed to open, use a temporary file.
+  if (path.empty() ||
+  openFile(path, pc.pc_fd, CD_CreateNew, FA_Write, OF_None)) {
+if (std::error_code errc =
+createTemporaryFile("lldb", "core", pc.pc_fd, path))
+  return llvm::createStringError(errc, "Unable to create a temporary 
file");
+  }
+  error = PtraceWrapper(PT_COREDUMP, GetID(), &pc, sizeof(pc));
+
+  std::error_code close_err = closeFile(pc.pc_fd);
+  if (error.Fail())
+return error.ToError();
+  if (close_err)
+return llvm::createStringError(
+close_err, "Unable to close the core dump after writing");
+  return path.str().str();
+}

diff  --git a/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h 
b/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h
index 7ec9d17d4cf48..44b8a53699bb6 100644
--- a/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h
+++ b/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h
@@ -91,6 +91,8 @@ class NativeProcessFreeBSD : public NativeProcessELF,
 
   bool SupportHardwareSingleStepping() const;
 
+  llvm::Expected SaveCore(llvm::StringRef path_hint) override;
+
 protected:
   llvm::Expected>
   GetSoftwareBreakpointTrapOpcode(size_t size_hint) override;

diff  --git 
a/lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py 
b/lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py
index 9897fb6b4910c..42955975a71b8 100644
--- a/lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py
+++ b/lldb/test/API/functionalities/process_save_core/TestProcessSaveCore.py
@@ -64,7 +64,7 @@ def test_save_windows_mini_dump(self):
 if (os.path.isfile(core)):
 os.unlink(core)
 
-@skipUnlessPlatform(["netbsd"])
+@skipUnlessPlatform(["freebsd", "netbsd"])
 def test_save_core_via_process_plugin(self):
 self.build()
 exe = self.getBuildArtifact("a.out")

diff  --git a/lldb/test/API/tools/lldb-server/TestGdbRemoteSaveCore.py 
b/lldb/test/API/tools/lldb-server/TestGdbRemoteSaveCore.py
index 405a73bcdba8d..10ba1e91134f4 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemoteSaveCore.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemoteSaveCore.py
@@ -38,15 +38,15 @@ def coredump_test(self, core_path=None, expect_path=None):
 self.assertTrue(process, PROCESS_IS_VALID)
 self.assertEqual(process.GetProcessID(), procs["inferior"].pid)
 
-@skipUnlessPlatform(oslist=["netbsd"])
+@skipUnlessPlatform(oslist=["freebsd", "netbsd"])
 def test_netbsd_path(self):
 core = lldbutil.append_to_process_working_directory(self, "core")
 self.coredump_test(core, core)
 
-@skipUnlessPlatform(oslist=["netbsd"])
+@skipUnlessPlatform(oslist=["freebsd", "netbsd"])
 def test_netbsd_no_path(self):
 self.coredump_test()
 
-@skipUnlessPlatform(oslist=["netbsd"])
+@skipUnlessPlatform(oslist=["freebsd", "netbsd"])
 def test_netbsd_bad_path(self):
 self.coredump_tes

[Lldb-commits] [PATCH] D107665: [lldb] [Commands] Fix reporting errors in 'platform file read/write'

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

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG39a2449ea133: [lldb] [Commands] Fix reporting errors in 
'platform file read/write' (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107665

Files:
  lldb/source/Commands/CommandObjectPlatform.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py


Index: 
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
@@ -60,11 +60,12 @@
 self.match("platform file open /some/file.txt -v 0755",
[r"error: Invalid argument"],
error=True)
-# TODO: fix the commands to fail on unsuccessful result
 self.match("platform file read 16 -o 11 -c 13",
-   [r"Return = -1\nData = \"\""])
+   [r"error: Invalid argument"],
+   error=True)
 self.match("platform file write 16 -o 11 -d teststring",
-   [r"Return = -1"])
+   [r"error: Invalid argument"],
+   error=True)
 self.match("platform file close 16",
[r"error: Invalid argument"],
error=True)
Index: lldb/source/Commands/CommandObjectPlatform.cpp
===
--- lldb/source/Commands/CommandObjectPlatform.cpp
+++ lldb/source/Commands/CommandObjectPlatform.cpp
@@ -589,11 +589,15 @@
   }
   std::string buffer(m_options.m_count, 0);
   Status error;
-  uint32_t retcode = platform_sp->ReadFile(
+  uint64_t retcode = platform_sp->ReadFile(
   fd, m_options.m_offset, &buffer[0], m_options.m_count, error);
-  result.AppendMessageWithFormat("Return = %d\n", retcode);
-  result.AppendMessageWithFormat("Data = \"%s\"\n", buffer.c_str());
-  result.SetStatus(eReturnStatusSuccessFinishResult);
+  if (retcode != UINT64_MAX) {
+result.AppendMessageWithFormat("Return = %" PRIu64 "\n", retcode);
+result.AppendMessageWithFormat("Data = \"%s\"\n", buffer.c_str());
+result.SetStatus(eReturnStatusSuccessFinishResult);
+  } else {
+result.AppendError(error.AsCString());
+  }
 } else {
   result.AppendError("no platform currently selected\n");
 }
@@ -678,11 +682,15 @@
   cmd_line);
 return result.Succeeded();
   }
-  uint32_t retcode =
+  uint64_t retcode =
   platform_sp->WriteFile(fd, m_options.m_offset, &m_options.m_data[0],
  m_options.m_data.size(), error);
-  result.AppendMessageWithFormat("Return = %d\n", retcode);
-  result.SetStatus(eReturnStatusSuccessFinishResult);
+  if (retcode != UINT64_MAX) {
+result.AppendMessageWithFormat("Return = %" PRIu64 "\n", retcode);
+result.SetStatus(eReturnStatusSuccessFinishResult);
+  } else {
+result.AppendError(error.AsCString());
+  }
 } else {
   result.AppendError("no platform currently selected\n");
 }


Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
@@ -60,11 +60,12 @@
 self.match("platform file open /some/file.txt -v 0755",
[r"error: Invalid argument"],
error=True)
-# TODO: fix the commands to fail on unsuccessful result
 self.match("platform file read 16 -o 11 -c 13",
-   [r"Return = -1\nData = \"\""])
+   [r"error: Invalid argument"],
+   error=True)
 self.match("platform file write 16 -o 11 -d teststring",
-   [r"Return = -1"])
+   [r"error: Invalid argument"],
+   error=True)
 self.match("platform file close 16",
[r"error: Invalid argument"],
error=True)
Index: lldb/source/Commands/CommandObjectPlatform.cpp
===
--- lldb/source/Commands/CommandObjectPlatform.cpp
+++ lldb/source/Commands/CommandObjectPlatform.cpp
@@ -589,11 +589,15 @@
   }
   std::string buffer(m_options.m_count, 0);
   Status error;
-  uint32_t r

[Lldb-commits] [lldb] 8872c9d - [lldb] [gdb-server] Add tests for more vFile packets

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-09-08T10:58:12+02:00
New Revision: 8872c9d1ca2189c55fda0246890b42f67a460598

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

LOG: [lldb] [gdb-server] Add tests for more vFile packets

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

Added: 


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

Removed: 




diff  --git a/lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py 
b/lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
index 4fc688deae7f..9a7d197f123d 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
@@ -10,7 +10,6 @@
 
 import binascii
 import stat
-import tempfile
 
 
 class TestGdbRemotePlatformFile(GdbRemoteTestCaseBase):
@@ -68,20 +67,18 @@ def test_platform_file_wronly_fail(self):
 server = self.connect_to_debug_monitor()
 self.assertIsNotNone(server)
 
-# create a temporary directory
-with tempfile.TemporaryDirectory() as temp_dir:
-temp_path = os.path.join(temp_dir, "test")
-self.assertFalse(os.path.exists(temp_path))
+temp_path = self.getBuildArtifact("test")
+self.assertFalse(os.path.exists(temp_path))
 
-# attempt to open the file without O_CREAT
-self.do_handshake()
-self.test_sequence.add_log_lines(
-["read packet: $vFile:open:%s,1,0#00" % (
-binascii.b2a_hex(temp_path.encode()).decode(),),
- {"direction": "send",
- "regex": r"^\$F-1,[0-9a-fA-F]+#[0-9a-fA-F]{2}$"}],
-True)
-self.expect_gdbremote_sequence()
+# attempt to open the file without O_CREAT
+self.do_handshake()
+self.test_sequence.add_log_lines(
+["read packet: $vFile:open:%s,1,0#00" % (
+binascii.b2a_hex(temp_path.encode()).decode(),),
+ {"direction": "send",
+ "regex": r"^\$F-1,[0-9a-fA-F]+#[0-9a-fA-F]{2}$"}],
+True)
+self.expect_gdbremote_sequence()
 
 @skipIfWindows
 @add_test_categories(["llgs"])
@@ -89,16 +86,97 @@ def test_platform_file_wronly_creat_excl_fail(self):
 server = self.connect_to_debug_monitor()
 self.assertIsNotNone(server)
 
-with tempfile.NamedTemporaryFile() as temp_file:
-# attempt to open the file with O_CREAT|O_EXCL
-self.do_handshake()
-self.test_sequence.add_log_lines(
-["read packet: $vFile:open:%s,a01,0#00" % (
-binascii.b2a_hex(temp_file.name.encode()).decode(),),
- {"direction": "send",
- "regex": r"^\$F-1,[0-9a-fA-F]+#[0-9a-fA-F]{2}$"}],
-True)
-self.expect_gdbremote_sequence()
+temp_file = self.getBuildArtifact("test")
+with open(temp_file, "wb"):
+pass
+
+# attempt to open the file with O_CREAT|O_EXCL
+self.do_handshake()
+self.test_sequence.add_log_lines(
+["read packet: $vFile:open:%s,a01,0#00" % (
+binascii.b2a_hex(temp_file.encode()).decode(),),
+ {"direction": "send",
+ "regex": r"^\$F-1,[0-9a-fA-F]+#[0-9a-fA-F]{2}$"}],
+True)
+self.expect_gdbremote_sequence()
+
+@skipIfWindows
+@add_test_categories(["llgs"])
+def test_platform_file_size(self):
+server = self.connect_to_debug_monitor()
+self.assertIsNotNone(server)
+
+temp_path = self.getBuildArtifact("test")
+test_data = b"test data of some length"
+with open(temp_path, "wb") as temp_file:
+temp_file.write(test_data)
+
+self.do_handshake()
+self.test_sequence.add_log_lines(
+["read packet: $vFile:size:%s#00" % (
+binascii.b2a_hex(temp_path.encode()).decode(),),
+ {"direction": "send",
+ "regex": r"^\$F([0-9a-fA-F]+)+#[0-9a-fA-F]{2}$",
+ "capture": {1: "size"}}],
+True)
+context = self.expect_gdbremote_sequence()
+self.assertEqual(int(context["size"], 16), len(test_data))
+
+@skipIfWindows
+@add_test_categories(["llgs"])
+def test_platform_file_mode(self):
+server = self.connect_to_debug_monitor()
+self.assertIsNotNone(server)
+
+temp_path = self.getBuildArtifact("test")
+test_mode = 0o751
+
+with open(temp_path, "wb") as temp_file:
+os.chmod(temp_file.fileno(), test_mode)
+
+self.do_handshake()
+self.test_sequence.add_log_lines(
+["read packet: $vFile:mode:%s#00" % (
+binascii.b2a_hex(temp_

[Lldb-commits] [PATCH] D107821: [lldb] [gdb-server] Add tests for more vFile packets

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8872c9d1ca21: [lldb] [gdb-server] Add tests for more vFile 
packets (authored by mgorny).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D107821?vs=365451&id=371284#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107821

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

Index: lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py
@@ -10,7 +10,6 @@
 
 import binascii
 import stat
-import tempfile
 
 
 class TestGdbRemotePlatformFile(GdbRemoteTestCaseBase):
@@ -68,20 +67,18 @@
 server = self.connect_to_debug_monitor()
 self.assertIsNotNone(server)
 
-# create a temporary directory
-with tempfile.TemporaryDirectory() as temp_dir:
-temp_path = os.path.join(temp_dir, "test")
-self.assertFalse(os.path.exists(temp_path))
+temp_path = self.getBuildArtifact("test")
+self.assertFalse(os.path.exists(temp_path))
 
-# attempt to open the file without O_CREAT
-self.do_handshake()
-self.test_sequence.add_log_lines(
-["read packet: $vFile:open:%s,1,0#00" % (
-binascii.b2a_hex(temp_path.encode()).decode(),),
- {"direction": "send",
- "regex": r"^\$F-1,[0-9a-fA-F]+#[0-9a-fA-F]{2}$"}],
-True)
-self.expect_gdbremote_sequence()
+# attempt to open the file without O_CREAT
+self.do_handshake()
+self.test_sequence.add_log_lines(
+["read packet: $vFile:open:%s,1,0#00" % (
+binascii.b2a_hex(temp_path.encode()).decode(),),
+ {"direction": "send",
+ "regex": r"^\$F-1,[0-9a-fA-F]+#[0-9a-fA-F]{2}$"}],
+True)
+self.expect_gdbremote_sequence()
 
 @skipIfWindows
 @add_test_categories(["llgs"])
@@ -89,16 +86,97 @@
 server = self.connect_to_debug_monitor()
 self.assertIsNotNone(server)
 
-with tempfile.NamedTemporaryFile() as temp_file:
-# attempt to open the file with O_CREAT|O_EXCL
-self.do_handshake()
-self.test_sequence.add_log_lines(
-["read packet: $vFile:open:%s,a01,0#00" % (
-binascii.b2a_hex(temp_file.name.encode()).decode(),),
- {"direction": "send",
- "regex": r"^\$F-1,[0-9a-fA-F]+#[0-9a-fA-F]{2}$"}],
-True)
-self.expect_gdbremote_sequence()
+temp_file = self.getBuildArtifact("test")
+with open(temp_file, "wb"):
+pass
+
+# attempt to open the file with O_CREAT|O_EXCL
+self.do_handshake()
+self.test_sequence.add_log_lines(
+["read packet: $vFile:open:%s,a01,0#00" % (
+binascii.b2a_hex(temp_file.encode()).decode(),),
+ {"direction": "send",
+ "regex": r"^\$F-1,[0-9a-fA-F]+#[0-9a-fA-F]{2}$"}],
+True)
+self.expect_gdbremote_sequence()
+
+@skipIfWindows
+@add_test_categories(["llgs"])
+def test_platform_file_size(self):
+server = self.connect_to_debug_monitor()
+self.assertIsNotNone(server)
+
+temp_path = self.getBuildArtifact("test")
+test_data = b"test data of some length"
+with open(temp_path, "wb") as temp_file:
+temp_file.write(test_data)
+
+self.do_handshake()
+self.test_sequence.add_log_lines(
+["read packet: $vFile:size:%s#00" % (
+binascii.b2a_hex(temp_path.encode()).decode(),),
+ {"direction": "send",
+ "regex": r"^\$F([0-9a-fA-F]+)+#[0-9a-fA-F]{2}$",
+ "capture": {1: "size"}}],
+True)
+context = self.expect_gdbremote_sequence()
+self.assertEqual(int(context["size"], 16), len(test_data))
+
+@skipIfWindows
+@add_test_categories(["llgs"])
+def test_platform_file_mode(self):
+server = self.connect_to_debug_monitor()
+self.assertIsNotNone(server)
+
+temp_path = self.getBuildArtifact("test")
+test_mode = 0o751
+
+with open(temp_path, "wb") as temp_file:
+os.chmod(temp_file.fileno(), test_mode)
+
+self.do_handshake()
+self.test_sequence.add_log_lines(
+["read packet: $vFile:mode:%s#00" % (
+binascii.b2a_hex(temp_path.encode()).decode(),),
+ {"direction": "send",
+ "regex": r"^\$F([0-9a-fA-F]+)+#[0-9a-fA-F]{2}$",
+ "capture": {1: "mode"}}],
+True)
+context = self.expect_gdbremote_sequence()

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

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

mgorny added a comment.

While technically this patch doesn't depend on these two, applying them out of 
order would cause unnecessary rebase hell.


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

https://reviews.llvm.org/D107931

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

[Lldb-commits] [PATCH] D102428: [StopInfoMachException] Summarize arm64e BLRAx/LDRAx auth failures

[David Spickett via Phabricator via lldb-commits]

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

Apologies, I forgot about this for way too long. I don't think Omair is going 
to object anyway so let's get this in.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102428

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

[Lldb-commits] [lldb] c01b76e - [lldb] Support "eflags" register name in generic reg fallback

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-09-08T11:33:29+02:00
New Revision: c01b76e733d6e5e2d21e4277dceaa1f319794c6a

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

LOG: [lldb] Support "eflags" register name in generic reg fallback

Enhance the generic register fallback code to support "eflags" register
name in addition to "rflags", as the former is used by gdbserver.  This
permits lldb-server to recognize the generic flags register when
interfacing with gdbserver-style target.xml (i.e. without generic=""
attributes), and therefore aligns ABI plugins' AugmentRegisterInfo()
between lldb-server and gdbserver.

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

Added: 


Modified: 
lldb/source/Plugins/ABI/X86/ABISysV_x86_64.cpp
lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.cpp
lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp

Removed: 




diff  --git a/lldb/source/Plugins/ABI/X86/ABISysV_x86_64.cpp 
b/lldb/source/Plugins/ABI/X86/ABISysV_x86_64.cpp
index 196b45b3b6da4..9af8aace9f12c 100644
--- a/lldb/source/Plugins/ABI/X86/ABISysV_x86_64.cpp
+++ b/lldb/source/Plugins/ABI/X86/ABISysV_x86_64.cpp
@@ -933,6 +933,8 @@ uint32_t ABISysV_x86_64::GetGenericNum(llvm::StringRef 
name) {
   .Case("rsp", LLDB_REGNUM_GENERIC_SP)
   .Case("rbp", LLDB_REGNUM_GENERIC_FP)
   .Case("rflags", LLDB_REGNUM_GENERIC_FLAGS)
+  // gdbserver uses eflags
+  .Case("eflags", LLDB_REGNUM_GENERIC_FLAGS)
   .Case("rdi", LLDB_REGNUM_GENERIC_ARG1)
   .Case("rsi", LLDB_REGNUM_GENERIC_ARG2)
   .Case("rdx", LLDB_REGNUM_GENERIC_ARG3)

diff  --git a/lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.cpp 
b/lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.cpp
index 6c473c652c5f8..dc8789820e3d0 100644
--- a/lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.cpp
+++ b/lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.cpp
@@ -806,6 +806,8 @@ uint32_t ABIWindows_x86_64::GetGenericNum(llvm::StringRef 
reg) {
   .Case("rsp", LLDB_REGNUM_GENERIC_SP)
   .Case("rbp", LLDB_REGNUM_GENERIC_FP)
   .Case("rflags", LLDB_REGNUM_GENERIC_FLAGS)
+  // gdbserver uses eflags
+  .Case("eflags", LLDB_REGNUM_GENERIC_FLAGS)
   .Case("rcx", LLDB_REGNUM_GENERIC_ARG1)
   .Case("rdx", LLDB_REGNUM_GENERIC_ARG2)
   .Case("r8", LLDB_REGNUM_GENERIC_ARG3)

diff  --git a/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp 
b/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
index a85d7bd6f525a..f3dfa5d57fc85 100644
--- a/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
+++ b/lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
@@ -603,6 +603,7 @@ void DynamicRegisterInfo::Finalize(const ArchSpec &arch) {
  (strcmp(reg.name, "fp") == 0))
   reg.kinds[eRegisterKindGeneric] = LLDB_REGNUM_GENERIC_FP;
 else if ((strcmp(reg.name, "rflags") == 0) ||
+ (strcmp(reg.name, "eflags") == 0) ||
  (strcmp(reg.name, "flags") == 0))
   reg.kinds[eRegisterKindGeneric] = LLDB_REGNUM_GENERIC_FLAGS;
   }



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

[Lldb-commits] [PATCH] D108548: [lldb] Support "eflags" register name in generic reg fallback

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

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc01b76e733d6: [lldb] Support "eflags" register 
name in generic reg fallback (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108548

Files:
  lldb/source/Plugins/ABI/X86/ABISysV_x86_64.cpp
  lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.cpp
  lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp


Index: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
===
--- lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
+++ lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
@@ -603,6 +603,7 @@
  (strcmp(reg.name, "fp") == 0))
   reg.kinds[eRegisterKindGeneric] = LLDB_REGNUM_GENERIC_FP;
 else if ((strcmp(reg.name, "rflags") == 0) ||
+ (strcmp(reg.name, "eflags") == 0) ||
  (strcmp(reg.name, "flags") == 0))
   reg.kinds[eRegisterKindGeneric] = LLDB_REGNUM_GENERIC_FLAGS;
   }
Index: lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.cpp
===
--- lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.cpp
+++ lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.cpp
@@ -806,6 +806,8 @@
   .Case("rsp", LLDB_REGNUM_GENERIC_SP)
   .Case("rbp", LLDB_REGNUM_GENERIC_FP)
   .Case("rflags", LLDB_REGNUM_GENERIC_FLAGS)
+  // gdbserver uses eflags
+  .Case("eflags", LLDB_REGNUM_GENERIC_FLAGS)
   .Case("rcx", LLDB_REGNUM_GENERIC_ARG1)
   .Case("rdx", LLDB_REGNUM_GENERIC_ARG2)
   .Case("r8", LLDB_REGNUM_GENERIC_ARG3)
Index: lldb/source/Plugins/ABI/X86/ABISysV_x86_64.cpp
===
--- lldb/source/Plugins/ABI/X86/ABISysV_x86_64.cpp
+++ lldb/source/Plugins/ABI/X86/ABISysV_x86_64.cpp
@@ -933,6 +933,8 @@
   .Case("rsp", LLDB_REGNUM_GENERIC_SP)
   .Case("rbp", LLDB_REGNUM_GENERIC_FP)
   .Case("rflags", LLDB_REGNUM_GENERIC_FLAGS)
+  // gdbserver uses eflags
+  .Case("eflags", LLDB_REGNUM_GENERIC_FLAGS)
   .Case("rdi", LLDB_REGNUM_GENERIC_ARG1)
   .Case("rsi", LLDB_REGNUM_GENERIC_ARG2)
   .Case("rdx", LLDB_REGNUM_GENERIC_ARG3)


Index: lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
===
--- lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
+++ lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
@@ -603,6 +603,7 @@
  (strcmp(reg.name, "fp") == 0))
   reg.kinds[eRegisterKindGeneric] = LLDB_REGNUM_GENERIC_FP;
 else if ((strcmp(reg.name, "rflags") == 0) ||
+ (strcmp(reg.name, "eflags") == 0) ||
  (strcmp(reg.name, "flags") == 0))
   reg.kinds[eRegisterKindGeneric] = LLDB_REGNUM_GENERIC_FLAGS;
   }
Index: lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.cpp
===
--- lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.cpp
+++ lldb/source/Plugins/ABI/X86/ABIWindows_x86_64.cpp
@@ -806,6 +806,8 @@
   .Case("rsp", LLDB_REGNUM_GENERIC_SP)
   .Case("rbp", LLDB_REGNUM_GENERIC_FP)
   .Case("rflags", LLDB_REGNUM_GENERIC_FLAGS)
+  // gdbserver uses eflags
+  .Case("eflags", LLDB_REGNUM_GENERIC_FLAGS)
   .Case("rcx", LLDB_REGNUM_GENERIC_ARG1)
   .Case("rdx", LLDB_REGNUM_GENERIC_ARG2)
   .Case("r8", LLDB_REGNUM_GENERIC_ARG3)
Index: lldb/source/Plugins/ABI/X86/ABISysV_x86_64.cpp
===
--- lldb/source/Plugins/ABI/X86/ABISysV_x86_64.cpp
+++ lldb/source/Plugins/ABI/X86/ABISysV_x86_64.cpp
@@ -933,6 +933,8 @@
   .Case("rsp", LLDB_REGNUM_GENERIC_SP)
   .Case("rbp", LLDB_REGNUM_GENERIC_FP)
   .Case("rflags", LLDB_REGNUM_GENERIC_FLAGS)
+  // gdbserver uses eflags
+  .Case("eflags", LLDB_REGNUM_GENERIC_FLAGS)
   .Case("rdi", LLDB_REGNUM_GENERIC_ARG1)
   .Case("rsi", LLDB_REGNUM_GENERIC_ARG2)
   .Case("rdx", LLDB_REGNUM_GENERIC_ARG3)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D109427: [lldb] Remove unused GDBRemoteCommunicationClient::SendAttach function

[David Spickett via Phabricator via lldb-commits]

DavidSpickett created this revision.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D109427

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


Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -147,21 +147,6 @@
   int SendLaunchEventDataPacket(const char *data,
 bool *was_supported = nullptr);
 
-  /// Sends a "vAttach:PID" where PID is in hex.
-  ///
-  /// \param[in] pid
-  /// A process ID for the remote gdb server to attach to.
-  ///
-  /// \param[out] response
-  /// The response received from the gdb server. If the return
-  /// value is zero, \a response will contain a stop reply
-  /// packet.
-  ///
-  /// \return
-  /// Zero if the attach was successful, or an error indicating
-  /// an error code.
-  int SendAttach(lldb::pid_t pid, StringExtractorGDBRemote &response);
-
   /// Sends a GDB remote protocol 'I' packet that delivers stdin
   /// data to the remote process.
   ///
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1369,24 +1369,6 @@
   return m_qHostInfo_is_valid == eLazyBoolYes;
 }
 
-int GDBRemoteCommunicationClient::SendAttach(
-lldb::pid_t pid, StringExtractorGDBRemote &response) {
-  if (pid != LLDB_INVALID_PROCESS_ID) {
-char packet[64];
-const int packet_len =
-::snprintf(packet, sizeof(packet), "vAttach;%" PRIx64, pid);
-UNUSED_IF_ASSERT_DISABLED(packet_len);
-assert(packet_len < (int)sizeof(packet));
-if (SendPacketAndWaitForResponse(packet, response) ==
-PacketResult::Success) {
-  if (response.IsErrorResponse())
-return response.GetError();
-  return 0;
-}
-  }
-  return -1;
-}
-
 int GDBRemoteCommunicationClient::SendStdinNotification(const char *data,
 size_t data_len) {
   StreamString packet;


Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -147,21 +147,6 @@
   int SendLaunchEventDataPacket(const char *data,
 bool *was_supported = nullptr);
 
-  /// Sends a "vAttach:PID" where PID is in hex.
-  ///
-  /// \param[in] pid
-  /// A process ID for the remote gdb server to attach to.
-  ///
-  /// \param[out] response
-  /// The response received from the gdb server. If the return
-  /// value is zero, \a response will contain a stop reply
-  /// packet.
-  ///
-  /// \return
-  /// Zero if the attach was successful, or an error indicating
-  /// an error code.
-  int SendAttach(lldb::pid_t pid, StringExtractorGDBRemote &response);
-
   /// Sends a GDB remote protocol 'I' packet that delivers stdin
   /// data to the remote process.
   ///
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1369,24 +1369,6 @@
   return m_qHostInfo_is_valid == eLazyBoolYes;
 }
 
-int GDBRemoteCommunicationClient::SendAttach(
-lldb::pid_t pid, StringExtractorGDBRemote &response) {
-  if (pid != LLDB_INVALID_PROCESS_ID) {
-char packet[64];
-const int packet_len =
-::snprintf(packet, sizeof(packet), "vAttach;%" PRIx64, pid);
-UNUSED_IF_ASSERT_DISABLED(packet_len);
-assert(packet_len < (int)sizeof(packet));
-if (SendPacketAndWaitForResponse(packet, response) ==
-PacketResult::Success) {
-  if (response.IsErrorResponse())
-return response.GetError();
-  return 0;
-}
-  }
-  return -1;
-}
-
 int GDBRemoteCommunicationClient::SendStdinNotification(const char *data,
 size_t data_len) {
   StreamString packet;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D109427: [lldb] Remove unused GDBRemoteCommunicationClient::SendAttach function

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a reviewer: teemperor.
DavidSpickett added a comment.
Herald added a subscriber: JDevlieghere.

Asking for review to double check. Nothing in lldb upstream calls this and I 
think that's because we switched to a style where we ask to attach then have a 
callback that will tell us once it's done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109427

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

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

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

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

Fix lint warnings. Add a note about not-full-replacement as requested in 
D107811 .


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

https://reviews.llvm.org/D107780

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

Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
@@ -77,3 +77,58 @@
 ])
 finally:
 self.dbg.GetSelectedPlatform().DisconnectRemote()
+
+def test_file_size(self):
+"""Test 'platform get-size'"""
+
+class Responder(MockGDBServerResponder):
+def vFile(self, packet):
+return "F1000"
+
+self.server.responder = Responder()
+
+try:
+self.runCmd("platform select remote-gdb-server")
+self.runCmd("platform connect connect://" +
+self.server.get_connect_address())
+self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
+
+self.match("platform get-size /some/file.txt",
+   [r"File size of /some/file\.txt \(remote\): 4096"])
+self.assertPacketLogContains([
+"vFile:size:2f736f6d652f66696c652e747874",
+])
+finally:
+self.dbg.GetSelectedPlatform().DisconnectRemote()
+
+def test_file_size_fallback(self):
+"""Test 'platform get-size fallback to vFile:fstat'"""
+
+class Responder(MockGDBServerResponder):
+def vFile(self, packet):
+if packet.startswith("vFile:open:"):
+return "F5"
+elif packet.startswith("vFile:fstat:"):
+return "F40;" + 28 * "\0" + "\0\0\0\0\0\1\2\3" + 28 * "\0"
+if packet.startswith("vFile:close:"):
+return "F0"
+return ""
+
+self.server.responder = Responder()
+
+try:
+self.runCmd("platform select remote-gdb-server")
+self.runCmd("platform connect connect://" +
+self.server.get_connect_address())
+self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
+
+self.match("platform get-size /some/file.txt",
+   [r"File size of /some/file\.txt \(remote\): 66051"])
+self.assertPacketLogContains([
+"vFile:size:2f736f6d652f66696c652e747874",
+"vFile:open:2f736f6d652f66696c652e747874,,",
+"vFile:fstat:5",
+"vFile:close:5",
+])
+finally:
+self.dbg.GetSelectedPlatform().DisconnectRemote()
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -391,6 +391,10 @@
 
   bool CloseFile(lldb::user_id_t fd, Status &error);
 
+  llvm::Optional FStat(lldb::user_id_t fd);
+
+  llvm::Optional Stat(const FileSpec &file_spec);
+
   lldb::user_id_t GetFileSize(const FileSpec &file_spec);
 
   void AutoCompleteDiskFileOrDirectory(CompletionRequest &request,
@@ -596,7 +600,7 @@
   m_supports_QEnvironment : 1, m_supports_QEnvironmentHexEncoded : 1,
   m_supports_qSymbol : 1, m_qSymbol_requests_done : 1,
   m_supports_qModuleInfo : 1, m_supports_jThreadsInfo : 1,
-  m_supports_jModulesInfo : 1;
+  m_supports_jModulesInfo : 1, m_supports_vFileSize : 1;
 
   /// Current gdb remote protocol process identifier for all other operations
   lldb::pid_t m_curr_pid = LLDB_INVALID_PROCESS_ID;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -65,6 +65,7 @@
   m_supports_QEnvironmentHexEncoded(true), m_supports_qSymbol(true),
   m_qSymbol_requests_done(false), m_supports_qModuleInfo(true),
   m_supports_jThreadsInfo(true), m_supports_jModulesInfo(true),
+  m_supports_vFileSize(true),
 
   m_host_arch(), m_process_arch(), m_os_build(), m_os_kernel(),
   m_hostname(), m_gdb_server_name(), m_default_packet_timeout(0),
@@ -3068,22 +3069,83

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

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

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

Add fallback notes, as requested.


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

https://reviews.llvm.org/D107811

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

Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
@@ -156,6 +156,38 @@
 finally:
 self.dbg.GetSelectedPlatform().DisconnectRemote()
 
+def test_file_permissions_fallback(self):
+"""Test 'platform get-permissions' fallback to fstat"""
+
+class Responder(MockGDBServerResponder):
+def vFile(self, packet):
+if packet.startswith("vFile:open:"):
+return "F5"
+elif packet.startswith("vFile:fstat:"):
+return "F40;" + 8 * "\0\0\1\xA4" + 4 * "\0" + 52 * "\0"
+if packet.startswith("vFile:close:"):
+return "F0"
+return ""
+
+self.server.responder = Responder()
+
+try:
+self.runCmd("platform select remote-gdb-server")
+self.runCmd("platform connect connect://" +
+self.server.get_connect_address())
+self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
+
+self.match("platform get-permissions /some/file.txt",
+   [r"File permissions of /some/file\.txt \(remote\): 0o0644"])
+self.assertPacketLogContains([
+"vFile:mode:2f736f6d652f66696c652e747874",
+"vFile:open:2f736f6d652f66696c652e747874,,",
+"vFile:fstat:5",
+"vFile:close:5",
+])
+finally:
+self.dbg.GetSelectedPlatform().DisconnectRemote()
+
 def test_file_exists(self):
 """Test 'platform file-exists'"""
 
@@ -201,3 +233,58 @@
 ])
 finally:
 self.dbg.GetSelectedPlatform().DisconnectRemote()
+
+def test_file_exists_fallback(self):
+"""Test 'platform file-exists' fallback to open"""
+
+class Responder(MockGDBServerResponder):
+def vFile(self, packet):
+if packet.startswith("vFile:open:"):
+return "F5"
+if packet.startswith("vFile:close:"):
+return "F0"
+return ""
+
+self.server.responder = Responder()
+
+try:
+self.runCmd("platform select remote-gdb-server")
+self.runCmd("platform connect connect://" +
+self.server.get_connect_address())
+self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
+
+self.match("platform file-exists /some/file.txt",
+   [r"File /some/file\.txt \(remote\) exists"])
+self.assertPacketLogContains([
+"vFile:exists:2f736f6d652f66696c652e747874",
+"vFile:open:2f736f6d652f66696c652e747874,,",
+"vFile:close:5",
+])
+finally:
+self.dbg.GetSelectedPlatform().DisconnectRemote()
+
+def test_file_exists_not_fallback(self):
+"""Test 'platform file-exists' fallback to open with non-existing file"""
+
+class Responder(MockGDBServerResponder):
+def vFile(self, packet):
+if packet.startswith("vFile:open:"):
+return "F-1,2"
+return ""
+
+self.server.responder = Responder()
+
+try:
+self.runCmd("platform select remote-gdb-server")
+self.runCmd("platform connect connect://" +
+self.server.get_connect_address())
+self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
+
+self.match("platform file-exists /some/file.txt",
+   [r"File /some/file\.txt \(remote\) does not exist"])
+self.assertPacketLogContains([
+"vFile:exists:2f736f6d652f66696c652e747874",
+"vFile:open:2f736f6d652f66696c652e747874,,",
+])
+finally:
+self.dbg.GetSelectedPlatform().DisconnectRemote()
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -600,7 +600,8 @@
   m_supports_

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

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h:56-70
+struct GDBRemoteFStatData {
+  uint32_t gdb_st_dev;
+  uint32_t gdb_st_ino;
+  uint32_t gdb_st_mode;
+  uint32_t gdb_st_nlink;
+  uint32_t gdb_st_uid;
+  uint32_t gdb_st_gid;

If we're going to have a struct for this anyway, then I think we should 
definitely use llvm's endian-specific types here. Then, instead of playing with 
DataExtractors, we can just memcpy the data here (the memcpy could go away as 
well with some changes to the StringExtractorGDBRemote interface)..



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h:396
+
+  llvm::Optional Stat(const FileSpec &file_spec);
+

If you make a note here that this is only a convenience wrapper around FStat 
(which opens the file in read mode), then the other comments are probably 
unnecessary...


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

https://reviews.llvm.org/D107780

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

[Lldb-commits] [PATCH] D108768: [lldb] DynamicRegisterInfo: fix wrong regnos in Dump()

[Pavel Labath via Phabricator via lldb-commits]

labath added a reviewer: omjavaid.
labath added a comment.

I'm not sure if this is correct, or if I want it to be correct. I think that a 
lot of the confusion comes from the fact that this structure is used both in 
lldb-server and lldb, and I'm not sure that the fields have the same meaning in 
both cases (specifically, the "process plugin" concept is not really a thing in 
lldb-server). I think it would be better to standardize on the "lldb" numbers, 
as those should be available everywhere, but I may be forgetting some important 
issues. Omair may remember this in more detail.


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

https://reviews.llvm.org/D108768

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

[Lldb-commits] [PATCH] D107664: [lldb] [Commands] Remove 'append' from 'platform file open' mode

[Pavel Labath via Phabricator via lldb-commits]

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

Sounds reasonable.

> The Linux manpage for pwrite(2) suggests that Linux does not respect O_APPEND 
> there,

I guess it would be more correct to say that it "respects" O_APPEND, when it 
fact POSIX says it shouldn't.


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

https://reviews.llvm.org/D107664

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

[Lldb-commits] [PATCH] D107079: [lldb] Add an option for emitting LLDB logs as JSON

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I think that a structured logging format could be very useful. I am not sure 
what to make of the trailing "," in the json format though. It seems its 
usefulness is fairly limited, given that additional post processing is still 
needed to turn it into valid json. One could envision printing the surrounding 
`[`/`]` when logging is enabled/disabled, but I'm not sure that's such a good 
idea since one often inspects logs from crashing lldb instances. I have a 
feeling it would be cleanest to just drop the comma entirely. That way at least 
each individual log message would be valid json (so one could feed it to some 
log analyzer/storage backend in real time), and adding the trailing comma would 
would still be fairly trivial in post processing.

I too think that we should keep the textual log format, because it's much 
easier to read when printing to the terminal interactively. One thing to note 
there is that this patch removes the column alignment code from the textual 
output. With the advent of the structured format, this may not be so useful, as 
uninteresting fields can be removed differently (right now, I use vi block 
editing commands for that), but some alignment might still be useful for the 
ease of reading.

In D107079#2939328 , @teemperor wrote:

> - Added the log format as an option to `log enable` (`o`, `f` and `F` are 
> taken, so I just picked `O` as the short option).

This seems like a good time to remind everyone that not every option needs to 
have a short form. I think that skipping one is a better idea than picking a 
random letter that noone is going to remember.

In D107079#2913864 , @mib wrote:

> This looks like a great effort to improve logging and debugging! Have you 
> considered also adding logging levels (info, debug, warning, error ...) ? I 
> think the `CommandReturnObject` and `Status` classes already makes the 
> distinction between warnings and errors messages. We could enrich logging 
> further more by having logging levels to filter / color-code the messages.

We had a concept of different log levels at one point, but it wasn't really 
used, so it was removed during some refactoring. Though, with a structured log 
format, I can see how it could be useful to reintroduce that.

In D107079#2914111 , @aprantl wrote:

> More a comment than anything else: One thing I always wanted to explore was 
> to implement LLDB's logging on Darwin on top of os_log 
> (https://developer.apple.com/documentation/os/logging) which is also 
> structured and also faster since it moves the the formatting out of process. 
> This patch is great because it also works for non-Darwin platforms, and I 
> guess there isn't anything preventing us from moving to os_log even if we 
> take patch.

BTW, there's a function called Host::SystemLog, which (I think) logs to some 
darwin-specific facility on darwin hosts. Personally, I think that having two 
logging mechanisms is just confusing, but I think I would be interesting if you 
could redirect the "normal" lldb logs to some os-specific target (like you can 
do with a file).




Comment at: lldb/docs/design/logging.rst:65
+set.
+
+.. list-table:: JSON message fields

jingham wrote:
> We always know which channel/category pair produces the message.  I think it 
> would be handy to also always include the channel/category pairs.  That seems 
> like it would be useful information.
The way logging works right now, we only know which channel the message came 
from. Category is lost, and sometimes it's never really clear because of stuff 
like GetLogIfAny/AllCategoriesSet(CAT1 | CAT2).

To be able to do this (which I think is a great idea), we'd need to slightly 
refactor the logging code, so thateach message can be uniquely associated with 
a specific category.



Comment at: lldb/source/Utility/Log.cpp:314-320
+  void PrintSeparatorIfNeeded() {
+// The first output at the start of the line doesn't need a separating
+// space.
+if (!m_anything_written)
+  return;
+m_output << " ";
+  }

I guess you should use llvm::ListSeparator for this.



Comment at: lldb/source/Utility/Log.cpp:378-380
 llvm::SmallString<12> format_str;
 llvm::raw_svector_ostream format_os(format_str);
 format_os << "{0,-" << llvm::alignTo<16>(thread_name.size()) << "} ";

This is now dead code.


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

https://reviews.llvm.org/D107079

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

[Lldb-commits] [PATCH] D108468: [lldb] [gdb-remote] Fix displaying i387_ext & vec regs with gdbserver

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Is printing STi as a vector actually desired? It seems to me that printing them 
as floating point would be more useful. Have you looked at what it would take 
to make lldb-server print them as FP as well?


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

https://reviews.llvm.org/D108468

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

[Lldb-commits] [PATCH] D107664: [lldb] [Commands] Remove 'append' from 'platform file open' mode

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

mgorny added a comment.

Thanks, I'll improve the commit message as suggested.


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

https://reviews.llvm.org/D107664

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

[Lldb-commits] [lldb] 2c6d90d - [lldb] [Commands] Remove 'append' from 'platform file open' mode

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2021-09-08T15:28:03+02:00
New Revision: 2c6d90d7410b77798699ff2fff4182d83db79164

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

LOG: [lldb] [Commands] Remove 'append' from 'platform file open' mode

Remove File::eOpenOptionAppend from the mode used by 'platform file
open' command.  According to POSIX, O_APPEND causes all successive
writes to be done at the end of the file.  This effectively makes
the offset argument to 'platform file write' meaningless.

Furthermore, apparently O_APPEND is not implemented reliably everywhere.
The Linux manpage for pwrite(2) suggests that Linux does respect
O_APPEND there while according to POSIX it should not, so the actual
behavior would be dependent on how the vFile:pwrite packet is
implemented on the server.

Ideally, the mode used for opening flags would be provided via options.
However, changing the default mode seems to be a reasonable intermediate
solution.

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

Added: 


Modified: 
lldb/source/Commands/CommandObjectPlatform.cpp
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectPlatform.cpp 
b/lldb/source/Commands/CommandObjectPlatform.cpp
index 8dd0512141645..0274605f2e329 100644
--- a/lldb/source/Commands/CommandObjectPlatform.cpp
+++ b/lldb/source/Commands/CommandObjectPlatform.cpp
@@ -498,8 +498,7 @@ class CommandObjectPlatformFOpen : public 
CommandObjectParsed {
 lldb::eFilePermissionsWorldRead;
   lldb::user_id_t fd = platform_sp->OpenFile(
   FileSpec(cmd_line),
-  File::eOpenOptionReadWrite | File::eOpenOptionAppend |
-   File::eOpenOptionCanCreate,
+  File::eOpenOptionReadWrite | File::eOpenOptionCanCreate,
   perms, error);
   if (error.Success()) {
 result.AppendMessageWithFormat("File Descriptor = %" PRIu64 "\n", fd);

diff  --git 
a/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py 
b/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
index 4ba23870e4a0e..b0ecddbdc5579 100644
--- 
a/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
+++ 
b/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
@@ -34,7 +34,7 @@ def vFile(self, packet):
 self.match("platform file close 16",
[r"file 16 closed."])
 self.assertPacketLogContains([
-"vFile:open:2f736f6d652f66696c652e747874,020a,01ed",
+"vFile:open:2f736f6d652f66696c652e747874,0202,01ed",
 "vFile:pread:10,d,b",
 "vFile:pwrite:10,b,teststring",
 "vFile:close:10",
@@ -70,7 +70,7 @@ def vFile(self, packet):
[r"error: Invalid argument"],
error=True)
 self.assertPacketLogContains([
-"vFile:open:2f736f6d652f66696c652e747874,020a,01ed",
+"vFile:open:2f736f6d652f66696c652e747874,0202,01ed",
 "vFile:pread:10,d,b",
 "vFile:pwrite:10,b,teststring",
 "vFile:close:10",



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

[Lldb-commits] [PATCH] D107664: [lldb] [Commands] Remove 'append' from 'platform file open' mode

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

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG2c6d90d7410b: [lldb] [Commands] Remove 'append' 
from 'platform file open' mode (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107664

Files:
  lldb/source/Commands/CommandObjectPlatform.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py


Index: 
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
@@ -34,7 +34,7 @@
 self.match("platform file close 16",
[r"file 16 closed."])
 self.assertPacketLogContains([
-"vFile:open:2f736f6d652f66696c652e747874,020a,01ed",
+"vFile:open:2f736f6d652f66696c652e747874,0202,01ed",
 "vFile:pread:10,d,b",
 "vFile:pwrite:10,b,teststring",
 "vFile:close:10",
@@ -70,7 +70,7 @@
[r"error: Invalid argument"],
error=True)
 self.assertPacketLogContains([
-"vFile:open:2f736f6d652f66696c652e747874,020a,01ed",
+"vFile:open:2f736f6d652f66696c652e747874,0202,01ed",
 "vFile:pread:10,d,b",
 "vFile:pwrite:10,b,teststring",
 "vFile:close:10",
Index: lldb/source/Commands/CommandObjectPlatform.cpp
===
--- lldb/source/Commands/CommandObjectPlatform.cpp
+++ lldb/source/Commands/CommandObjectPlatform.cpp
@@ -498,8 +498,7 @@
 lldb::eFilePermissionsWorldRead;
   lldb::user_id_t fd = platform_sp->OpenFile(
   FileSpec(cmd_line),
-  File::eOpenOptionReadWrite | File::eOpenOptionAppend |
-   File::eOpenOptionCanCreate,
+  File::eOpenOptionReadWrite | File::eOpenOptionCanCreate,
   perms, error);
   if (error.Success()) {
 result.AppendMessageWithFormat("File Descriptor = %" PRIu64 "\n", fd);


Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
@@ -34,7 +34,7 @@
 self.match("platform file close 16",
[r"file 16 closed."])
 self.assertPacketLogContains([
-"vFile:open:2f736f6d652f66696c652e747874,020a,01ed",
+"vFile:open:2f736f6d652f66696c652e747874,0202,01ed",
 "vFile:pread:10,d,b",
 "vFile:pwrite:10,b,teststring",
 "vFile:close:10",
@@ -70,7 +70,7 @@
[r"error: Invalid argument"],
error=True)
 self.assertPacketLogContains([
-"vFile:open:2f736f6d652f66696c652e747874,020a,01ed",
+"vFile:open:2f736f6d652f66696c652e747874,0202,01ed",
 "vFile:pread:10,d,b",
 "vFile:pwrite:10,b,teststring",
 "vFile:close:10",
Index: lldb/source/Commands/CommandObjectPlatform.cpp
===
--- lldb/source/Commands/CommandObjectPlatform.cpp
+++ lldb/source/Commands/CommandObjectPlatform.cpp
@@ -498,8 +498,7 @@
 lldb::eFilePermissionsWorldRead;
   lldb::user_id_t fd = platform_sp->OpenFile(
   FileSpec(cmd_line),
-  File::eOpenOptionReadWrite | File::eOpenOptionAppend |
-   File::eOpenOptionCanCreate,
+  File::eOpenOptionReadWrite | File::eOpenOptionCanCreate,
   perms, error);
   if (error.Success()) {
 result.AppendMessageWithFormat("File Descriptor = %" PRIu64 "\n", fd);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D108468: [lldb] [gdb-remote] Fix displaying i387_ext & vec regs with gdbserver

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

mgorny added a comment.

At a quick glance, we'd have to enhance `Scalar` to fully support x87 extended 
type. FWICS it's currently modeled to use convert `APFloat` from/to 
`float`/`double`/`long double` types. Maybe we could get away with some lossy 
rounding but it doesn't like a good way forward. It would probably be a better 
idea to drop some of the abstraction and/or use `APFloat` more.

I don't think an effort to getting proper x87 float support should be blocking 
this. If anyone is sufficiently motivated to implement it, it will be easy to 
switch the client to it, both for lldb-server and gdbserver usage.


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

https://reviews.llvm.org/D108468

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

[Lldb-commits] [lldb] d2189b5 - [lldb] Remove unused GDBRemoteCommunicationClient::SendAttach function

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2021-09-08T15:14:02Z
New Revision: d2189b5c4b011201f784ed7f503287c543076f91

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

LOG: [lldb] Remove unused GDBRemoteCommunicationClient::SendAttach function

Reviewed By: labath

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

Added: 


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

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index c38a4ee19d152..6dcc73c3e43de 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1369,24 +1369,6 @@ bool GDBRemoteCommunicationClient::GetHostInfo(bool 
force) {
   return m_qHostInfo_is_valid == eLazyBoolYes;
 }
 
-int GDBRemoteCommunicationClient::SendAttach(
-lldb::pid_t pid, StringExtractorGDBRemote &response) {
-  if (pid != LLDB_INVALID_PROCESS_ID) {
-char packet[64];
-const int packet_len =
-::snprintf(packet, sizeof(packet), "vAttach;%" PRIx64, pid);
-UNUSED_IF_ASSERT_DISABLED(packet_len);
-assert(packet_len < (int)sizeof(packet));
-if (SendPacketAndWaitForResponse(packet, response) ==
-PacketResult::Success) {
-  if (response.IsErrorResponse())
-return response.GetError();
-  return 0;
-}
-  }
-  return -1;
-}
-
 int GDBRemoteCommunicationClient::SendStdinNotification(const char *data,
 size_t data_len) {
   StreamString packet;

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
index 63bccc8781c8a..00625088d42a0 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -147,21 +147,6 @@ class GDBRemoteCommunicationClient : public 
GDBRemoteClientBase {
   int SendLaunchEventDataPacket(const char *data,
 bool *was_supported = nullptr);
 
-  /// Sends a "vAttach:PID" where PID is in hex.
-  ///
-  /// \param[in] pid
-  /// A process ID for the remote gdb server to attach to.
-  ///
-  /// \param[out] response
-  /// The response received from the gdb server. If the return
-  /// value is zero, \a response will contain a stop reply
-  /// packet.
-  ///
-  /// \return
-  /// Zero if the attach was successful, or an error indicating
-  /// an error code.
-  int SendAttach(lldb::pid_t pid, StringExtractorGDBRemote &response);
-
   /// Sends a GDB remote protocol 'I' packet that delivers stdin
   /// data to the remote process.
   ///



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

[Lldb-commits] [PATCH] D109427: [lldb] Remove unused GDBRemoteCommunicationClient::SendAttach function

[David Spickett via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd2189b5c4b01: [lldb] Remove unused 
GDBRemoteCommunicationClient::SendAttach function (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109427

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


Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -147,21 +147,6 @@
   int SendLaunchEventDataPacket(const char *data,
 bool *was_supported = nullptr);
 
-  /// Sends a "vAttach:PID" where PID is in hex.
-  ///
-  /// \param[in] pid
-  /// A process ID for the remote gdb server to attach to.
-  ///
-  /// \param[out] response
-  /// The response received from the gdb server. If the return
-  /// value is zero, \a response will contain a stop reply
-  /// packet.
-  ///
-  /// \return
-  /// Zero if the attach was successful, or an error indicating
-  /// an error code.
-  int SendAttach(lldb::pid_t pid, StringExtractorGDBRemote &response);
-
   /// Sends a GDB remote protocol 'I' packet that delivers stdin
   /// data to the remote process.
   ///
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1369,24 +1369,6 @@
   return m_qHostInfo_is_valid == eLazyBoolYes;
 }
 
-int GDBRemoteCommunicationClient::SendAttach(
-lldb::pid_t pid, StringExtractorGDBRemote &response) {
-  if (pid != LLDB_INVALID_PROCESS_ID) {
-char packet[64];
-const int packet_len =
-::snprintf(packet, sizeof(packet), "vAttach;%" PRIx64, pid);
-UNUSED_IF_ASSERT_DISABLED(packet_len);
-assert(packet_len < (int)sizeof(packet));
-if (SendPacketAndWaitForResponse(packet, response) ==
-PacketResult::Success) {
-  if (response.IsErrorResponse())
-return response.GetError();
-  return 0;
-}
-  }
-  return -1;
-}
-
 int GDBRemoteCommunicationClient::SendStdinNotification(const char *data,
 size_t data_len) {
   StreamString packet;


Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -147,21 +147,6 @@
   int SendLaunchEventDataPacket(const char *data,
 bool *was_supported = nullptr);
 
-  /// Sends a "vAttach:PID" where PID is in hex.
-  ///
-  /// \param[in] pid
-  /// A process ID for the remote gdb server to attach to.
-  ///
-  /// \param[out] response
-  /// The response received from the gdb server. If the return
-  /// value is zero, \a response will contain a stop reply
-  /// packet.
-  ///
-  /// \return
-  /// Zero if the attach was successful, or an error indicating
-  /// an error code.
-  int SendAttach(lldb::pid_t pid, StringExtractorGDBRemote &response);
-
   /// Sends a GDB remote protocol 'I' packet that delivers stdin
   /// data to the remote process.
   ///
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1369,24 +1369,6 @@
   return m_qHostInfo_is_valid == eLazyBoolYes;
 }
 
-int GDBRemoteCommunicationClient::SendAttach(
-lldb::pid_t pid, StringExtractorGDBRemote &response) {
-  if (pid != LLDB_INVALID_PROCESS_ID) {
-char packet[64];
-const int packet_len =
-::snprintf(packet, sizeof(packet), "vAttach;%" PRIx64, pid);
-UNUSED_IF_ASSERT_DISABLED(packet_len);
-assert(packet_len < (int)sizeof(packet));
-if (SendPacketAndWaitForResponse(packet, response) ==
-PacketResult::Success) {
-  if (response.IsErrorResponse())
-return response.GetError();
-  return 0;
-}
-  }
-  return -1;
-}
-
 int GDBRemoteCommunicationClient::SendStdinNotification(const char *data,
 size_t data_len) {
   StreamString packet;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.

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

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

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

Use a .def file.


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

https://reviews.llvm.org/D108148

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

Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
@@ -15,7 +15,7 @@
 return "Fa"
 elif packet.startswith("vFile:close:"):
 return "F0"
-return "F-1,16"
+return "F-1,58"
 
 self.server.responder = Responder()
 
@@ -47,7 +47,9 @@
 
 class Responder(MockGDBServerResponder):
 def vFile(self, packet):
-return "F-1,16"
+# use ENOSYS as this constant differs between GDB Remote
+# Protocol and Linux, so we can test the translation
+return "F-1,58"
 
 self.server.responder = Responder()
 
@@ -58,16 +60,16 @@
 self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
 
 self.match("platform file open /some/file.txt -v 0755",
-   [r"error: Invalid argument"],
+   [r"error: Function not implemented"],
error=True)
 self.match("platform file read 16 -o 11 -c 13",
-   [r"error: Invalid argument"],
+   [r"error: Function not implemented"],
error=True)
 self.match("platform file write 16 -o 11 -d teststring",
-   [r"error: Invalid argument"],
+   [r"error: Function not implemented"],
error=True)
 self.match("platform file close 16",
-   [r"error: Invalid argument"],
+   [r"error: Function not implemented"],
error=True)
 self.assertPacketLogContains([
 "vFile:open:2f736f6d652f66696c652e747874,0202,01ed",
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteErrno.def
===
--- /dev/null
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteErrno.def
@@ -0,0 +1,37 @@
+//===-- GDBRemoteErrno.def --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+// NOTE: NO INCLUDE GUARD DESIRED!
+
+// HANDLE_ERRNO(name, value)
+#ifndef HANDLE_ERRNO
+#error "HANDLE_ERRNO must be defined"
+#endif
+
+// from gdb's include/gdb/fileio.h
+HANDLE_ERRNO(EPERM, 1)
+HANDLE_ERRNO(ENOENT,2)
+HANDLE_ERRNO(EINTR, 4)
+HANDLE_ERRNO(EIO,   5)
+HANDLE_ERRNO(EBADF, 9)
+HANDLE_ERRNO(EACCES,   13)
+HANDLE_ERRNO(EFAULT,   14)
+HANDLE_ERRNO(EBUSY,16)
+HANDLE_ERRNO(EEXIST,   17)
+HANDLE_ERRNO(ENODEV,   19)
+HANDLE_ERRNO(ENOTDIR,  20)
+HANDLE_ERRNO(EISDIR,   21)
+HANDLE_ERRNO(EINVAL,   22)
+HANDLE_ERRNO(ENFILE,   23)
+HANDLE_ERRNO(EMFILE,   24)
+HANDLE_ERRNO(EFBIG,27)
+HANDLE_ERRNO(ENOSPC,   28)
+HANDLE_ERRNO(ESPIPE,   29)
+HANDLE_ERRNO(EROFS,30)
+HANDLE_ERRNO(ENOSYS,   88)
+HANDLE_ERRNO(ENAMETOOLONG, 91)
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -535,6 +535,18 @@
   return SendErrorResponse(18);
 }
 
+static GDBErrno system_errno_to_gdb(int err) {
+  switch (err) {
+#define HANDLE_ERRNO(name, value)  \
+  case name:   \
+return GDB_##name;
+#include "Plugins/Process/gdb-remote/GDBRemoteErrno.def"
+#undef HANDLE_ERRNO
+  default:
+return GDB_EUNKNOWN;
+  }
+}
+
 GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerCommon::Handle_vFile_Close(
 StringExtractorGDBRemote &packet) {

[Lldb-commits] [PATCH] D108739: [LLDB] AArch64 SVE restore SVE registers after expression

[Muhammad Omair Javaid via Phabricator via lldb-commits]

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

This update addresses review comments.


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

https://reviews.llvm.org/D108739

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/main.c

Index: lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/main.c
===
--- lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/main.c
+++ lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/main.c
@@ -1,4 +1,6 @@
-int main() {
+#include 
+
+void write_sve_regs () {
   asm volatile("setffr\n\t");
   asm volatile("ptrue p0.b\n\t");
   asm volatile("ptrue p1.h\n\t");
@@ -49,5 +51,20 @@
   asm volatile("cpy  z29.b, p5/z, #30\n\t");
   asm volatile("cpy  z30.b, p10/z, #31\n\t");
   asm volatile("cpy  z31.b, p15/z, #32\n\t");
+}
+
+// This function will be called using jitted expression call. We change vector
+// length and write SVE registers. Our program context should restore to
+// orignal vector length and register values after expression evaluation.
+int expr_eval_func() {
+  prctl(PR_SVE_SET_VL, 8 * 2);
+  write_sve_regs();
+  prctl(PR_SVE_SET_VL, 8 * 4);
+  write_sve_regs();
+  return 1;
+}
+
+int main() {
+  write_sve_regs();
   return 0; // Set a break point here.
 }
Index: lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py
===
--- lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py
+++ lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py
@@ -17,6 +17,51 @@
 self.assertEqual(reg_value.GetByteSize(), expected,
  'Verify "%s" == %i' % (name, expected))
 
+def check_sve_regs_read(self, z_reg_size):
+p_reg_size = int(z_reg_size / 8)
+
+for i in range(32):
+z_regs_value = '{' + \
+' '.join('0x{:02x}'.format(i + 1)
+ for _ in range(z_reg_size)) + '}'
+self.expect("register read " + 'z%i' %
+(i), substrs=[z_regs_value])
+
+p_value_bytes = ['0xff', '0x55', '0x11', '0x01', '0x00']
+for i in range(16):
+p_regs_value = '{' + \
+' '.join(p_value_bytes[i % 5] for _ in range(p_reg_size)) + '}'
+self.expect("register read " + 'p%i' % (i), substrs=[p_regs_value])
+
+self.expect("register read ffr", substrs=[p_regs_value])
+
+def check_sve_regs_read_after_write(self, z_reg_size):
+p_reg_size = int(z_reg_size / 8)
+
+z_regs_value = '{' + \
+' '.join(('0x9d' for _ in range(z_reg_size))) + '}'
+
+p_regs_value = '{' + \
+' '.join(('0xee' for _ in range(p_reg_size))) + '}'
+
+for i in range(32):
+self.runCmd('register write ' + 'z%i' %
+(i) + " '" + z_regs_value + "'")
+
+for i in range(32):
+self.expect("register read " + 'z%i' % (i), substrs=[z_regs_value])
+
+for i in range(16):
+self.runCmd('register write ' + 'p%i' %
+(i) + " '" + p_regs_value + "'")
+
+for i in range(16):
+self.expect("register read " + 'p%i' % (i), substrs=[p_regs_value])
+
+self.runCmd('register write ' + 'ffr ' + "'" + p_regs_value + "'")
+
+self.expect("register read " + 'ffr', substrs=[p_regs_value])
+
 mydir = TestBase.compute_mydir(__file__)
 
 @no_debug_info_test
@@ -117,43 +162,17 @@
 
 z_reg_size = vg_reg_value * 8
 
-p_reg_size = int(z_reg_size / 8)
-
-for i in range(32):
-z_regs_value = '{' + \
-' '.join('0x{:02x}'.format(i + 1)
- for _ in range(z_reg_size)) + '}'
-self.expect("register read " + 'z%i' %
-(i), substrs=[z_regs_value])
+self.check_sve_regs_read(z_reg_size)
 
-p_value_bytes = ['0xff', '0x55', '0x11', '0x01', '0x00']
-for i in range(16):
-p_regs_value = '{' + \
-' '.join(p_value_bytes[i % 5] for _ in range(p_reg_size)) + '}'
-self.expect("register read " + 'p%i' % (i), substrs=[p_regs_value])
+# Evaluate simple expression and print function expr_eval_func address.
+self.expect("p expr_eval_func", substrs=["= 0x"])
 
-  

[Lldb-commits] [PATCH] D109249: [lldb] Add Getdescription function for SBInstruction.

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

Would it be better to make an API that takes an SBExecutionContext?  If you 
knew the frame that held

In D109249#2988808 , @labath wrote:

> I'm not sure what's the exact use case here, but I /think/ that passing just 
> the target part of the execution context will is not sufficient to enable all 
> the disassembler bells and whistles. For example the regular "disassemble" 
> command will also print a "-> " next to the instruction that references the 
> current PC.
>
> Even if that is not required for your use case, I think that a better (more 
> generic) API would be for this function to take a SBExecutionContext 
> argument, and let the user choose how it wants to populate it.

I thought the same thing, but then I got to wondering what you should do when 
the SBExecutionContext you were passed in has a frame with a PC that doesn't 
point to the instruction you were passed.  Is that an error?  Does it matter?

I still think the SBExecutionContext is a better choice, and will prevent us 
from having to add another override later on when we find we need the thread or 
frame.  But we should decide up front whether we want there to be any linkage 
between the exact ExecutionContext and the instruction you are dumping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109249

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

[Lldb-commits] [PATCH] D108768: [lldb] DynamicRegisterInfo: fix wrong regnos in Dump()

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid added a comment.

This is complicated mainly because we are trying to merge two different 
register numbering scheme. On LLDB (host) side we try to construct register 
number from parsing register xml assigning register numbers in the increasing 
order. But we also have target supplied register numbers in regnum field. 
However registers on LLDB server side have fixed numbering although it may look 
like assigned in increasing order but this may not always be the case. Presence 
of optional register sets also doesnt help so we have the mess on managing 
target vs host register numbers not always in sync.

For invalidate_regs and value_regs register numbers should always be the ones 
assigned by remote i-e eRegisterKindProcessPlugin. So I would say this patch is 
ok as it corrects that info.

Broadly speaking we should do something about integrating user typed registers 
which may be unions of various other registers but should not require numbering.


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

https://reviews.llvm.org/D108768

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

[Lldb-commits] [lldb] 303b27f - [lldb] Delete IRExecutionUnit::SearchSpec

[Alex Langford via lldb-commits]

Author: Alex Langford
Date: 2021-09-08T11:27:10-07:00
New Revision: 303b27f21b98a45621d39f941c88c7e30d69fbe7

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

LOG: [lldb] Delete IRExecutionUnit::SearchSpec

IRExecutionUnit::SearchSpec is a struct that encapsulates information
needed to look for a symbol. Specifically, it is comprised of a name
represented with a ConstString and a FunctionNameType mask.
Because the mask is unused (effectively always set to
eFunctionNameTypeFull), we can remove the mask and replace all uses with
eFunctionNameTypeFull.  After doing that, SearchSpec is effectively a
wrapper around a ConstString.

As an aside, SearchSpec is similar in purpose to Module::LookupInfo. I
briefly considered replacing uses of SearchSpec with LookupInfo, but
the current code only cares about symbol names (treating them as
eFunctionNameTypeFull). This code does care about language type, so
LookupInfo may be appropriate for IRExecutionUnit in the future.

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

Added: 


Modified: 
lldb/include/lldb/Expression/IRExecutionUnit.h
lldb/source/Expression/IRExecutionUnit.cpp

Removed: 




diff  --git a/lldb/include/lldb/Expression/IRExecutionUnit.h 
b/lldb/include/lldb/Expression/IRExecutionUnit.h
index ad3c7372e67dc..29dae4dc50fa8 100644
--- a/lldb/include/lldb/Expression/IRExecutionUnit.h
+++ b/lldb/include/lldb/Expression/IRExecutionUnit.h
@@ -214,26 +214,24 @@ class IRExecutionUnit : public 
std::enable_shared_from_this,
 
   Status DisassembleFunction(Stream &stream, lldb::ProcessSP &process_sp);
 
-  struct SearchSpec;
-
-  void CollectCandidateCNames(std::vector &C_specs,
+  void CollectCandidateCNames(std::vector &C_names,
   ConstString name);
 
-  void CollectCandidateCPlusPlusNames(std::vector &CPP_specs,
-  const std::vector &C_specs,
+  void CollectCandidateCPlusPlusNames(std::vector &CPP_names,
+  const std::vector &C_names,
   const SymbolContext &sc);
 
-  void CollectFallbackNames(std::vector &fallback_specs,
-const std::vector &C_specs);
+  void CollectFallbackNames(std::vector &fallback_names,
+const std::vector &C_names);
 
-  lldb::addr_t FindInSymbols(const std::vector &specs,
+  lldb::addr_t FindInSymbols(const std::vector &names,
  const lldb_private::SymbolContext &sc,
  bool &symbol_was_missing_weak);
 
-  lldb::addr_t FindInRuntimes(const std::vector &specs,
+  lldb::addr_t FindInRuntimes(const std::vector &names,
   const lldb_private::SymbolContext &sc);
 
-  lldb::addr_t FindInUserDefinedSymbols(const std::vector &specs,
+  lldb::addr_t FindInUserDefinedSymbols(const std::vector &names,
 const lldb_private::SymbolContext &sc);
 
   void ReportSymbolLookupError(ConstString name);

diff  --git a/lldb/source/Expression/IRExecutionUnit.cpp 
b/lldb/source/Expression/IRExecutionUnit.cpp
index 77207320ea105..5c58ee6fcaea0 100644
--- a/lldb/source/Expression/IRExecutionUnit.cpp
+++ b/lldb/source/Expression/IRExecutionUnit.cpp
@@ -698,29 +698,17 @@ static ConstString 
FindBestAlternateMangledName(ConstString demangled,
 return ConstString();
 }
 
-struct IRExecutionUnit::SearchSpec {
-  ConstString name;
-  lldb::FunctionNameType mask;
-
-  SearchSpec(ConstString n,
- lldb::FunctionNameType m = lldb::eFunctionNameTypeFull)
-  : name(n), mask(m) {}
-};
-
-void IRExecutionUnit::CollectCandidateCNames(
-std::vector &C_specs,
-ConstString name) {
+void IRExecutionUnit::CollectCandidateCNames(std::vector &C_names,
+ ConstString name) {
   if (m_strip_underscore && name.AsCString()[0] == '_')
-C_specs.insert(C_specs.begin(), ConstString(&name.AsCString()[1]));
-  C_specs.push_back(SearchSpec(name));
+C_names.insert(C_names.begin(), ConstString(&name.AsCString()[1]));
+  C_names.push_back(name);
 }
 
 void IRExecutionUnit::CollectCandidateCPlusPlusNames(
-std::vector &CPP_specs,
-const std::vector &C_specs, const SymbolContext &sc) {
-  for (const SearchSpec &C_spec : C_specs) {
-ConstString name = C_spec.name;
-
+std::vector &CPP_names,
+const std::vector &C_names, const SymbolContext &sc) {
+  for (const ConstString &name : C_names) {
 if (CPlusPlusLanguage::IsCPPMangledName(name.GetCString())) {
   Mangled mangled(name);
   ConstString demangled = mangled.GetDemangledName();
@@ -730,26 +718,24 @@ void IRExecutionUnit::CollectCandidateCPlusPlusNames(
 FindBestAlternateMangledNa

[Lldb-commits] [PATCH] D109384: [lldb] Delete IRExecutionUnit::SearchSpec

[Alex Langford 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 rG303b27f21b98: [lldb] Delete IRExecutionUnit::SearchSpec 
(authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109384

Files:
  lldb/include/lldb/Expression/IRExecutionUnit.h
  lldb/source/Expression/IRExecutionUnit.cpp

Index: lldb/source/Expression/IRExecutionUnit.cpp
===
--- lldb/source/Expression/IRExecutionUnit.cpp
+++ lldb/source/Expression/IRExecutionUnit.cpp
@@ -698,29 +698,17 @@
 return ConstString();
 }
 
-struct IRExecutionUnit::SearchSpec {
-  ConstString name;
-  lldb::FunctionNameType mask;
-
-  SearchSpec(ConstString n,
- lldb::FunctionNameType m = lldb::eFunctionNameTypeFull)
-  : name(n), mask(m) {}
-};
-
-void IRExecutionUnit::CollectCandidateCNames(
-std::vector &C_specs,
-ConstString name) {
+void IRExecutionUnit::CollectCandidateCNames(std::vector &C_names,
+ ConstString name) {
   if (m_strip_underscore && name.AsCString()[0] == '_')
-C_specs.insert(C_specs.begin(), ConstString(&name.AsCString()[1]));
-  C_specs.push_back(SearchSpec(name));
+C_names.insert(C_names.begin(), ConstString(&name.AsCString()[1]));
+  C_names.push_back(name);
 }
 
 void IRExecutionUnit::CollectCandidateCPlusPlusNames(
-std::vector &CPP_specs,
-const std::vector &C_specs, const SymbolContext &sc) {
-  for (const SearchSpec &C_spec : C_specs) {
-ConstString name = C_spec.name;
-
+std::vector &CPP_names,
+const std::vector &C_names, const SymbolContext &sc) {
+  for (const ConstString &name : C_names) {
 if (CPlusPlusLanguage::IsCPPMangledName(name.GetCString())) {
   Mangled mangled(name);
   ConstString demangled = mangled.GetDemangledName();
@@ -730,26 +718,24 @@
 FindBestAlternateMangledName(demangled, sc);
 
 if (best_alternate_mangled_name) {
-  CPP_specs.push_back(best_alternate_mangled_name);
+  CPP_names.push_back(best_alternate_mangled_name);
 }
   }
 }
 
 std::set alternates;
 CPlusPlusLanguage::FindAlternateFunctionManglings(name, alternates);
-CPP_specs.insert(CPP_specs.end(), alternates.begin(), alternates.end());
+CPP_names.insert(CPP_names.end(), alternates.begin(), alternates.end());
   }
 }
 
 void IRExecutionUnit::CollectFallbackNames(
-std::vector &fallback_specs,
-const std::vector &C_specs) {
+std::vector &fallback_names,
+const std::vector &C_names) {
   // As a last-ditch fallback, try the base name for C++ names.  It's terrible,
   // but the DWARF doesn't always encode "extern C" correctly.
 
-  for (const SearchSpec &C_spec : C_specs) {
-ConstString name = C_spec.name;
-
+  for (const ConstString &name : C_names) {
 if (!CPlusPlusLanguage::IsCPPMangledName(name.GetCString()))
   continue;
 
@@ -763,9 +749,8 @@
 if (!lparen_loc)
   continue;
 
-llvm::StringRef base_name(demangled_cstr,
-  lparen_loc - demangled_cstr);
-fallback_specs.push_back(ConstString(base_name));
+llvm::StringRef base_name(demangled_cstr, lparen_loc - demangled_cstr);
+fallback_names.push_back(ConstString(base_name));
   }
 }
 
@@ -843,9 +828,10 @@
   lldb::addr_t m_best_internal_load_address = LLDB_INVALID_ADDRESS;
 };
 
-lldb::addr_t IRExecutionUnit::FindInSymbols(
-const std::vector &specs,
-const lldb_private::SymbolContext &sc, bool &symbol_was_missing_weak) {
+lldb::addr_t
+IRExecutionUnit::FindInSymbols(const std::vector &names,
+   const lldb_private::SymbolContext &sc,
+   bool &symbol_was_missing_weak) {
   symbol_was_missing_weak = false;
 
   Target *target = sc.target_sp.get();
@@ -860,18 +846,19 @@
   function_options.include_symbols = true;
   function_options.include_inlines = false;
 
-  for (const SearchSpec &spec : specs) {
+  for (const ConstString &name : names) {
 if (sc.module_sp) {
   SymbolContextList sc_list;
-  sc.module_sp->FindFunctions(spec.name, CompilerDeclContext(), spec.mask,
-  function_options, sc_list);
+  sc.module_sp->FindFunctions(name, CompilerDeclContext(),
+  lldb::eFunctionNameTypeFull, function_options,
+  sc_list);
   if (auto load_addr = resolver.Resolve(sc_list))
 return *load_addr;
 }
 
 if (sc.target_sp) {
   SymbolContextList sc_list;
-  sc.target_sp->GetImages().FindFunctions(spec.name, spec.mask,
+  sc.target_sp->GetImages().FindFunctions(name, lldb::eFunctionNameTypeFull,
   function_options, sc_list);
   if (auto load_addr = resolver.Resolve(s

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

[Rumeet Dhindsa via Phabricator via lldb-commits]

rdhindsa updated this revision to Diff 371425.
rdhindsa marked 2 inline comments as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108061

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

Index: lldb/test/API/functionalities/dyld-launch-linux/signal_file.h
===
--- /dev/null
+++ lldb/test/API/functionalities/dyld-launch-linux/signal_file.h
@@ -0,0 +1 @@
+int get_signal_crash();
Index: lldb/test/API/functionalities/dyld-launch-linux/signal_file.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/dyld-launch-linux/signal_file.cpp
@@ -0,0 +1,7 @@
+#include "signal_file.h"
+#include 
+
+int get_signal_crash(void) {
+  raise(SIGSEGV);
+  return 0;
+}
Index: lldb/test/API/functionalities/dyld-launch-linux/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/dyld-launch-linux/main.cpp
@@ -0,0 +1,6 @@
+#include "signal_file.h"
+
+int main() {
+  // Break here
+  return get_signal_crash();
+}
Index: lldb/test/API/functionalities/dyld-launch-linux/TestDyldLaunchLinux.py
===
--- /dev/null
+++ lldb/test/API/functionalities/dyld-launch-linux/TestDyldLaunchLinux.py
@@ -0,0 +1,52 @@
+"""
+Test that LLDB can launch a linux executable through the dynamic loader and still hit a breakpoint.
+"""
+
+import lldb
+import os
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+class TestLinux64LaunchingViaDynamicLoader(TestBase):
+mydir = TestBase.compute_mydir(__file__)
+
+@skipIf(oslist=no_match(['linux']))
+@no_debug_info_test
+def test(self):
+self.build()
+candidates = [
+"/lib64/ld-linux-x86-64.so.2",
+"/usr/lib/ld-linux-x86-64.so.2"
+]
+exe = next((c for c in candidates if os.path.exists(c)), None)
+if not os.path.exists(exe):
+return
+target = self.dbg.CreateTarget(exe)
+self.assertTrue(target, VALID_TARGET)
+
+# Set breakpoints both on shared library function as well as on
+# main. Both of them will be pending breakpoints.
+breakpoint_main = target.BreakpointCreateBySourceRegex("// Break here", lldb.SBFileSpec("main.cpp"))
+breakpoint_shared_library = target.BreakpointCreateBySourceRegex("get_signal_crash", lldb.SBFileSpec("signal_file.cpp"))
+launch_info = lldb.SBLaunchInfo([ "--library-path", self.get_process_working_directory(), self.getBuildArtifact("a.out")])
+launch_info.SetWorkingDirectory(self.get_process_working_directory())
+error = lldb.SBError()
+process = target.Launch(launch_info, error)
+self.assertTrue(error.Success())
+
+# Stopped on main here.
+self.assertEqual(process.GetState(), lldb.eStateStopped)
+thread = process.GetSelectedThread()
+self.assertIn("main", thread.GetFrameAtIndex(0).GetDisplayFunctionName())
+process.Continue()
+
+# Stopped on get_signal_crash function here.
+self.assertEqual(process.GetState(), lldb.eStateStopped)
+self.assertIn("get_signal_crash", thread.GetFrameAtIndex(0).GetDisplayFunctionName())
+process.Continue()
+
+# Stopped because of generated signal.
+self.assertEqual(process.GetState(), lldb.eStateStopped)
+self.assertIn("raise", thread.GetFrameAtIndex(0).GetDisplayFunctionName())
+self.assertIn("get_signal_crash", thread.GetFrameAtIndex(1).GetDisplayFunctionName())
Index: lldb/test/API/functionalities/dyld-launch-linux/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/dyld-launch-linux/Makefile
@@ -0,0 +1,4 @@
+CXX_SOURCES := main.cpp
+DYLIB_NAME := signal_file
+DYLIB_CXX_SOURCES := signal_file.cpp
+include Makefile.rules
Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -333,28 +333,37 @@
 LLDB_LOG(log, "Rendezvous structure is not set up yet. "
   "Trying to locate rendezvous breakpo

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

[Peter Collingbourne via Phabricator via lldb-commits]

pcc added a comment.

In D103127#2985203 , @labath wrote:

> How exactly are you building this? CMAKE_SYSTEM_NAME is set in the official 
> android cmake toolchain file 
> (https://android.googlesource.com/platform/ndk/+/refs/heads/ndk-release-r23/build/cmake/android.toolchain.cmake#213)

I configured cmake using a script that looks like this:

  #!/bin/sh
  
  target_flags='--target=aarch64-linux-android21 
--sysroot=/path/to/android-ndk-r21-beta1/toolchains/llvm/prebuilt/linux-x86_64/sysroot
 
--gcc-toolchain=/path/to/android-ndk-r21-beta1/toolchains/llvm/prebuilt/linux-x86_64
 -mno-outline-atomics'
  
  cmake \
-GNinja \
-DCMAKE_BUILD_TYPE=Release \
-DLLVM_ENABLE_ASSERTIONS=ON \
-DCMAKE_C_COMPILER=`pwd`/../ra/bin/clang \
-DCMAKE_CXX_COMPILER=`pwd`/../ra/bin/clang++ \
-DLLVM_ENABLE_THREADS=OFF \
-DLLVM_CONFIG_PATH=`pwd`/../ra/bin/llvm-config \
"-DCMAKE_C_FLAGS=$target_flags" \
"-DCMAKE_CXX_FLAGS=$target_flags" \
"-DCMAKE_ASM_FLAGS=$target_flags" \
-DSANITIZER_CXX_ABI=none \
-DSANITIZER_CXX_ABI_LIBRARY=c++abi \
-DCMAKE_EXE_LINKER_FLAGS="-fuse-ld=lld -Wl,-lstdc++ -static-libstdc++ 
--rtlib=libgcc" \
-DCMAKE_SHARED_LINKER_FLAGS=-Wl,-u__cxa_demangle \
-DANDROID=1 \
-DCOMPILER_RT_INCLUDE_TESTS=ON \
"-DCOMPILER_RT_TEST_COMPILER_CFLAGS=$target_flags" \
-DLLVM_TABLEGEN=`pwd`/../ra/bin/llvm-tblgen \
-DLLDB_TABLEGEN=`pwd`/../ra/bin/lldb-tblgen \
-DCLANG_TABLEGEN=`pwd`/../ra/bin/clang-tblgen \
-DLLVM_ENABLE_PROJECTS='clang;lldb' \
-DLLDB_ENABLE_LIBEDIT=0 \
-DLLDB_ENABLE_LIBXML2=0 \
-DLLVM_HOST_TRIPLE=aarch64-linux-android21 \
-DCMAKE_SYSTEM_NAME=Android \
-DLLVM_TARGETS_TO_BUILD='X86;AArch64' \
../../llvm

But now I'm switching to gn to build LLDB, so I don't need to worry about this 
any more.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103127

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

[Lldb-commits] [lldb] d1d4f36 - [lldb] Make sure there's a value for the key before dereferencing.

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2021-09-08T13:46:09-07:00
New Revision: d1d4f365566ce9aae40d1f46cae517e4fe8fe6ee

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

LOG: [lldb] Make sure there's a value for the key before dereferencing.

Make sure there's a value for the shared_cache_base_address key exists
in the dictionary before trying to dereference the value.

rdar://76894476

Added: 


Modified: 

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
index b4b6c8bf74861..a00b421c9f29b 100644
--- 
a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ 
b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -2188,8 +2188,12 @@ lldb::addr_t 
AppleObjCRuntimeV2::GetSharedCacheBaseAddress() {
   if (!info_dict)
 return LLDB_INVALID_ADDRESS;
 
-  return info_dict->GetValueForKey("shared_cache_base_address")
-  ->GetIntegerValue(LLDB_INVALID_ADDRESS);
+  StructuredData::ObjectSP value =
+  info_dict->GetValueForKey("shared_cache_base_address");
+  if (!value)
+return LLDB_INVALID_ADDRESS;
+
+  return value->GetIntegerValue(LLDB_INVALID_ADDRESS);
 }
 
 void AppleObjCRuntimeV2::UpdateISAToDescriptorMapIfNeeded() {



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

[Lldb-commits] [PATCH] D109463: gn build: Add support for building LLDB on Linux.

[Peter Collingbourne via Phabricator via lldb-commits]

pcc created this revision.
pcc added a reviewer: thakis.
Herald added a subscriber: emaste.
pcc requested review of this revision.
Herald added a project: LLVM.

On Linux, LLDB depends on lldb-server at runtime (is it different on
Mac?), so I created a top-level lldb target that will build both lldb
and lldb-server.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D109463

Files:
  llvm/utils/gn/secondary/BUILD.gn
  llvm/utils/gn/secondary/lldb/BUILD.gn
  llvm/utils/gn/secondary/lldb/include/lldb/Host/BUILD.gn
  llvm/utils/gn/secondary/lldb/source/Initialization/BUILD.gn
  llvm/utils/gn/secondary/lldb/source/Plugins/BUILD.gn
  llvm/utils/gn/secondary/lldb/source/Plugins/ObjectFile/JIT/BUILD.gn
  llvm/utils/gn/secondary/lldb/source/Plugins/Platform/Linux/BUILD.gn
  llvm/utils/gn/secondary/lldb/source/Plugins/Process/Linux/BUILD.gn
  llvm/utils/gn/secondary/lldb/source/Plugins/Process/POSIX/BUILD.gn
  llvm/utils/gn/secondary/lldb/tools/lldb-server/BUILD.gn

Index: llvm/utils/gn/secondary/lldb/tools/lldb-server/BUILD.gn
===
--- llvm/utils/gn/secondary/lldb/tools/lldb-server/BUILD.gn
+++ llvm/utils/gn/secondary/lldb/tools/lldb-server/BUILD.gn
@@ -29,13 +29,13 @@
 "//lldb/source/Plugins/Language/CPlusPlus",
   ]
 
-  #if (current_os == "android" || current_os == "linux") {
-#deps += [ "//lldb/source/Plugins/Process/Linux" ]
+  if (current_os == "android" || current_os == "linux") {
+deps += [ "//lldb/source/Plugins/Process/Linux" ]
 #} else if (current_os == "freebsd") {
 #deps += [ "//lldb/source/Plugins/Process/FreeBSD" ]
 #} else if (current_os == "netbsd") {
 #deps += [ "//lldb/source/Plugins/Process/NetBSD" ]
-  #}
+  }
 
   if (current_os == "mac") {
 deps += [ "//lldb/source/Plugins/ObjectFile/Mach-O" ]
Index: llvm/utils/gn/secondary/lldb/source/Plugins/Process/POSIX/BUILD.gn
===
--- /dev/null
+++ llvm/utils/gn/secondary/lldb/source/Plugins/Process/POSIX/BUILD.gn
@@ -0,0 +1,17 @@
+static_library("POSIX") {
+  output_name = "lldbPluginProcessPOSIX"
+  configs += [ "//llvm/utils/gn/build:lldb_code" ]
+  deps = [
+"//lldb/source/Utility",
+"//lldb/source/Plugins/Process/Utility",
+"//llvm/lib/Support",
+  ]
+  # Reaches into Plugins/Platform/Process/Utility.
+  include_dirs = [ "//lldb/source" ]
+  sources = [
+"CrashReason.cpp",
+"NativeProcessELF.cpp",
+"ProcessMessage.cpp",
+"ProcessPOSIXLog.cpp",
+  ]
+}
Index: llvm/utils/gn/secondary/lldb/source/Plugins/Process/Linux/BUILD.gn
===
--- /dev/null
+++ llvm/utils/gn/secondary/lldb/source/Plugins/Process/Linux/BUILD.gn
@@ -0,0 +1,29 @@
+static_library("Linux") {
+  output_name = "lldbPluginProcessLinux"
+  configs += [ "//llvm/utils/gn/build:lldb_code" ]
+  deps = [
+"//lldb/source/Core",
+"//lldb/source/Host",
+"//lldb/source/Symbol",
+"//lldb/source/Target",
+"//lldb/source/Utility",
+"//lldb/source/Plugins/Process/POSIX",
+"//lldb/source/Plugins/Process/Utility",
+"//llvm/lib/Support",
+  ]
+  # Uses source-relative includes for own headers.
+  include_dirs = [ "//lldb/source" ]
+  sources = [
+"IntelPTManager.cpp",
+"NativeProcessLinux.cpp",
+"NativeRegisterContextLinux.cpp",
+"NativeRegisterContextLinux_arm.cpp",
+"NativeRegisterContextLinux_arm64.cpp",
+"NativeRegisterContextLinux_ppc64le.cpp",
+"NativeRegisterContextLinux_s390x.cpp",
+"NativeRegisterContextLinux_x86_64.cpp",
+"NativeThreadLinux.cpp",
+"SingleStepCheck.cpp",
+  ]
+}
+
Index: llvm/utils/gn/secondary/lldb/source/Plugins/Platform/Linux/BUILD.gn
===
--- /dev/null
+++ llvm/utils/gn/secondary/lldb/source/Plugins/Platform/Linux/BUILD.gn
@@ -0,0 +1,18 @@
+static_library("Linux") {
+  output_name = "lldbPluginPlatformLinux"
+  configs += [
+"//llvm/utils/gn/build:clang_code",
+"//llvm/utils/gn/build:lldb_code",
+  ]
+  deps = [
+"//lldb/source/Breakpoint",
+"//lldb/source/Core",
+"//lldb/source/Host",
+"//lldb/source/Interpreter",
+"//lldb/source/Target",
+"//lldb/source/Plugins/Platform/POSIX",
+  ]
+  # Reaches into Plugins/Platform/POSIX.
+  include_dirs = [ "//lldb/source" ]
+  sources = [ "PlatformLinux.cpp" ]
+}
Index: llvm/utils/gn/secondary/lldb/source/Plugins/ObjectFile/JIT/BUILD.gn
===
--- llvm/utils/gn/secondary/lldb/source/Plugins/ObjectFile/JIT/BUILD.gn
+++ llvm/utils/gn/secondary/lldb/source/Plugins/ObjectFile/JIT/BUILD.gn
@@ -10,4 +10,6 @@
 "//llvm/lib/Support",
   ]
   sources = [ "ObjectFileJIT.cpp" ]
+  # For Utility/UuidCompatibility.h.
+  include_dirs = [ "//lldb/source" ]
 }
Index: llvm/utils/gn/secondary/lldb/source/Plugins/BUILD.gn
=

[Lldb-commits] [PATCH] D109464: gn build: Add support for building lldb-server on Android.

[Peter Collingbourne via Phabricator via lldb-commits]

pcc created this revision.
pcc added a reviewer: thakis.
Herald added a subscriber: danielkiss.
pcc requested review of this revision.
Herald added a project: LLVM.

The cross-compiled lldb-server targets are added to the top-level
lldb target if Android cross compilation is enabled.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D109464

Files:
  llvm/utils/gn/build/libs/xml/enable.gni
  llvm/utils/gn/secondary/lldb/BUILD.gn
  llvm/utils/gn/secondary/lldb/tools/lldb-server/BUILD.gn


Index: llvm/utils/gn/secondary/lldb/tools/lldb-server/BUILD.gn
===
--- llvm/utils/gn/secondary/lldb/tools/lldb-server/BUILD.gn
+++ llvm/utils/gn/secondary/lldb/tools/lldb-server/BUILD.gn
@@ -1,3 +1,4 @@
+import("//compiler-rt/target.gni")
 import("//llvm/utils/TableGen/tablegen.gni")
 
 tablegen("LLGSOptions") {
@@ -57,4 +58,8 @@
 "lldb-platform.cpp",
 "lldb-server.cpp",
   ]
+
+  if (current_os == "android") {
+output_dir = "$root_build_dir/runtimes_ndk_cxx/$crt_current_target_arch"
+  }
 }
Index: llvm/utils/gn/secondary/lldb/BUILD.gn
===
--- llvm/utils/gn/secondary/lldb/BUILD.gn
+++ llvm/utils/gn/secondary/lldb/BUILD.gn
@@ -1,6 +1,19 @@
+import("//llvm/utils/gn/build/toolchain/compiler.gni")
+
+supported_toolchains = []
+if (android_ndk_path != "") {
+  supported_toolchains += [
+"//llvm/utils/gn/build/toolchain:stage2_android_aarch64",
+"//llvm/utils/gn/build/toolchain:stage2_android_arm",
+  ]
+}
+
 group("lldb") {
   deps = [ "tools/driver:lldb" ]
   if (current_os == "linux") {
 deps += [ "tools/lldb-server" ]
   }
+  foreach (toolchain, supported_toolchains) {
+deps += [ "tools/lldb-server($toolchain)" ]
+  }
 }
Index: llvm/utils/gn/build/libs/xml/enable.gni
===
--- llvm/utils/gn/build/libs/xml/enable.gni
+++ llvm/utils/gn/build/libs/xml/enable.gni
@@ -1,4 +1,4 @@
 declare_args() {
   # Whether to include code that links against libxml2.
-  llvm_enable_libxml2 = host_os != "win"
+  llvm_enable_libxml2 = current_os != "win" && current_os != "android"
 }


Index: llvm/utils/gn/secondary/lldb/tools/lldb-server/BUILD.gn
===
--- llvm/utils/gn/secondary/lldb/tools/lldb-server/BUILD.gn
+++ llvm/utils/gn/secondary/lldb/tools/lldb-server/BUILD.gn
@@ -1,3 +1,4 @@
+import("//compiler-rt/target.gni")
 import("//llvm/utils/TableGen/tablegen.gni")
 
 tablegen("LLGSOptions") {
@@ -57,4 +58,8 @@
 "lldb-platform.cpp",
 "lldb-server.cpp",
   ]
+
+  if (current_os == "android") {
+output_dir = "$root_build_dir/runtimes_ndk_cxx/$crt_current_target_arch"
+  }
 }
Index: llvm/utils/gn/secondary/lldb/BUILD.gn
===
--- llvm/utils/gn/secondary/lldb/BUILD.gn
+++ llvm/utils/gn/secondary/lldb/BUILD.gn
@@ -1,6 +1,19 @@
+import("//llvm/utils/gn/build/toolchain/compiler.gni")
+
+supported_toolchains = []
+if (android_ndk_path != "") {
+  supported_toolchains += [
+"//llvm/utils/gn/build/toolchain:stage2_android_aarch64",
+"//llvm/utils/gn/build/toolchain:stage2_android_arm",
+  ]
+}
+
 group("lldb") {
   deps = [ "tools/driver:lldb" ]
   if (current_os == "linux") {
 deps += [ "tools/lldb-server" ]
   }
+  foreach (toolchain, supported_toolchains) {
+deps += [ "tools/lldb-server($toolchain)" ]
+  }
 }
Index: llvm/utils/gn/build/libs/xml/enable.gni
===
--- llvm/utils/gn/build/libs/xml/enable.gni
+++ llvm/utils/gn/build/libs/xml/enable.gni
@@ -1,4 +1,4 @@
 declare_args() {
   # Whether to include code that links against libxml2.
-  llvm_enable_libxml2 = host_os != "win"
+  llvm_enable_libxml2 = current_os != "win" && current_os != "android"
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D109463: gn build: Add support for building LLDB on Linux.

[Nico Weber via Phabricator via lldb-commits]

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

Very cool. LG with top-level BUILD.gn comment addressed :)




Comment at: llvm/utils/gn/secondary/BUILD.gn:15
 "//lld/test",
+"//lldb",
 "//llvm/test",

Does this build fine on windows?

Generally this only depends on the test targets which in turn depend on the 
binaries, so probably should only have the old/test dep in this file anyways.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109463

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

[Lldb-commits] [PATCH] D109464: gn build: Add support for building lldb-server on Android.

[Nico Weber via Phabricator via lldb-commits]

thakis added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109464

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

[Lldb-commits] [PATCH] D109463: gn build: Add support for building LLDB on Linux.

[Peter Collingbourne via Phabricator via lldb-commits]

pcc added inline comments.



Comment at: llvm/utils/gn/secondary/BUILD.gn:15
 "//lld/test",
+"//lldb",
 "//llvm/test",

thakis wrote:
> Does this build fine on windows?
> 
> Generally this only depends on the test targets which in turn depend on the 
> binaries, so probably should only have the old/test dep in this file anyways.
You're right, this needs to avoid the dependency on lldb on Windows targets.

The problem with only depending on `//lldb/test` is that nothing else refers to 
`//lldb`, so `ninja lldb` wouldn't also build `lldb-server`. Maybe it would be 
better to add `lldb-server` to the `data_deps` of `//lldb/tools/driver:lldb` 
instead then.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109463

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

[Lldb-commits] [PATCH] D109463: gn build: Add support for building LLDB on Linux.

[Peter Collingbourne via Phabricator via lldb-commits]

pcc updated this revision to Diff 371464.
pcc added a comment.

Move to data_deps


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109463

Files:
  llvm/utils/gn/secondary/BUILD.gn
  llvm/utils/gn/secondary/lldb/include/lldb/Host/BUILD.gn
  llvm/utils/gn/secondary/lldb/source/Initialization/BUILD.gn
  llvm/utils/gn/secondary/lldb/source/Plugins/BUILD.gn
  llvm/utils/gn/secondary/lldb/source/Plugins/ObjectFile/JIT/BUILD.gn
  llvm/utils/gn/secondary/lldb/source/Plugins/Platform/Linux/BUILD.gn
  llvm/utils/gn/secondary/lldb/source/Plugins/Process/Linux/BUILD.gn
  llvm/utils/gn/secondary/lldb/source/Plugins/Process/POSIX/BUILD.gn
  llvm/utils/gn/secondary/lldb/tools/driver/BUILD.gn
  llvm/utils/gn/secondary/lldb/tools/lldb-server/BUILD.gn

Index: llvm/utils/gn/secondary/lldb/tools/lldb-server/BUILD.gn
===
--- llvm/utils/gn/secondary/lldb/tools/lldb-server/BUILD.gn
+++ llvm/utils/gn/secondary/lldb/tools/lldb-server/BUILD.gn
@@ -29,13 +29,13 @@
 "//lldb/source/Plugins/Language/CPlusPlus",
   ]
 
-  #if (current_os == "android" || current_os == "linux") {
-#deps += [ "//lldb/source/Plugins/Process/Linux" ]
+  if (current_os == "android" || current_os == "linux") {
+deps += [ "//lldb/source/Plugins/Process/Linux" ]
 #} else if (current_os == "freebsd") {
 #deps += [ "//lldb/source/Plugins/Process/FreeBSD" ]
 #} else if (current_os == "netbsd") {
 #deps += [ "//lldb/source/Plugins/Process/NetBSD" ]
-  #}
+  }
 
   if (current_os == "mac") {
 deps += [ "//lldb/source/Plugins/ObjectFile/Mach-O" ]
Index: llvm/utils/gn/secondary/lldb/tools/driver/BUILD.gn
===
--- llvm/utils/gn/secondary/lldb/tools/driver/BUILD.gn
+++ llvm/utils/gn/secondary/lldb/tools/driver/BUILD.gn
@@ -7,7 +7,7 @@
   args = [ "-gen-opt-parser-defs" ]
 }
 
-if (host_os == "mac") {
+if (current_os == "mac") {
   write_cmake_config("write_info_plist") {
 input = "lldb-Info.plist.in"
 output = "$target_gen_dir/lldb-Info.plist"
@@ -25,12 +25,16 @@
 "//llvm/lib/Support",
 "//llvm/utils/gn/build/libs/xar",
   ]
-  if (host_os == "mac") {
+  if (current_os == "mac") {
 deps += [ ":write_info_plist" ]
 plist = get_target_outputs(":write_info_plist")
 ldflags = [ "-Wl,-sectcreate,__TEXT,__info_plist," +
 rebase_path(plist[0], root_out_dir) ]
   }
+  data_deps = []
+  if (current_os == "linux") {
+data_deps += [ "//lldb/tools/lldb-server" ]
+  }
 
   sources = [
 "Driver.cpp",
Index: llvm/utils/gn/secondary/lldb/source/Plugins/Process/POSIX/BUILD.gn
===
--- /dev/null
+++ llvm/utils/gn/secondary/lldb/source/Plugins/Process/POSIX/BUILD.gn
@@ -0,0 +1,17 @@
+static_library("POSIX") {
+  output_name = "lldbPluginProcessPOSIX"
+  configs += [ "//llvm/utils/gn/build:lldb_code" ]
+  deps = [
+"//lldb/source/Utility",
+"//lldb/source/Plugins/Process/Utility",
+"//llvm/lib/Support",
+  ]
+  # Reaches into Plugins/Platform/Process/Utility.
+  include_dirs = [ "//lldb/source" ]
+  sources = [
+"CrashReason.cpp",
+"NativeProcessELF.cpp",
+"ProcessMessage.cpp",
+"ProcessPOSIXLog.cpp",
+  ]
+}
Index: llvm/utils/gn/secondary/lldb/source/Plugins/Process/Linux/BUILD.gn
===
--- /dev/null
+++ llvm/utils/gn/secondary/lldb/source/Plugins/Process/Linux/BUILD.gn
@@ -0,0 +1,29 @@
+static_library("Linux") {
+  output_name = "lldbPluginProcessLinux"
+  configs += [ "//llvm/utils/gn/build:lldb_code" ]
+  deps = [
+"//lldb/source/Core",
+"//lldb/source/Host",
+"//lldb/source/Symbol",
+"//lldb/source/Target",
+"//lldb/source/Utility",
+"//lldb/source/Plugins/Process/POSIX",
+"//lldb/source/Plugins/Process/Utility",
+"//llvm/lib/Support",
+  ]
+  # Uses source-relative includes for own headers.
+  include_dirs = [ "//lldb/source" ]
+  sources = [
+"IntelPTManager.cpp",
+"NativeProcessLinux.cpp",
+"NativeRegisterContextLinux.cpp",
+"NativeRegisterContextLinux_arm.cpp",
+"NativeRegisterContextLinux_arm64.cpp",
+"NativeRegisterContextLinux_ppc64le.cpp",
+"NativeRegisterContextLinux_s390x.cpp",
+"NativeRegisterContextLinux_x86_64.cpp",
+"NativeThreadLinux.cpp",
+"SingleStepCheck.cpp",
+  ]
+}
+
Index: llvm/utils/gn/secondary/lldb/source/Plugins/Platform/Linux/BUILD.gn
===
--- /dev/null
+++ llvm/utils/gn/secondary/lldb/source/Plugins/Platform/Linux/BUILD.gn
@@ -0,0 +1,18 @@
+static_library("Linux") {
+  output_name = "lldbPluginPlatformLinux"
+  configs += [
+"//llvm/utils/gn/build:clang_code",
+"//llvm/utils/gn/build:lldb_code",
+  ]
+  deps = [
+"//lldb/source/Breakpoint",
+"//lldb/source/Core",
+

[Lldb-commits] [PATCH] D109464: gn build: Add support for building lldb-server on Android.

[Peter Collingbourne via Phabricator via lldb-commits]

pcc updated this revision to Diff 371465.
pcc added a comment.

Move to data_deps


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109464

Files:
  llvm/utils/gn/build/libs/xml/enable.gni
  llvm/utils/gn/secondary/lldb/tools/driver/BUILD.gn
  llvm/utils/gn/secondary/lldb/tools/lldb-server/BUILD.gn


Index: llvm/utils/gn/secondary/lldb/tools/lldb-server/BUILD.gn
===
--- llvm/utils/gn/secondary/lldb/tools/lldb-server/BUILD.gn
+++ llvm/utils/gn/secondary/lldb/tools/lldb-server/BUILD.gn
@@ -1,3 +1,4 @@
+import("//compiler-rt/target.gni")
 import("//llvm/utils/TableGen/tablegen.gni")
 
 tablegen("LLGSOptions") {
@@ -57,4 +58,8 @@
 "lldb-platform.cpp",
 "lldb-server.cpp",
   ]
+
+  if (current_os == "android") {
+output_dir = "$root_build_dir/runtimes_ndk_cxx/$crt_current_target_arch"
+  }
 }
Index: llvm/utils/gn/secondary/lldb/tools/driver/BUILD.gn
===
--- llvm/utils/gn/secondary/lldb/tools/driver/BUILD.gn
+++ llvm/utils/gn/secondary/lldb/tools/driver/BUILD.gn
@@ -1,4 +1,5 @@
 import("//llvm/utils/TableGen/tablegen.gni")
+import("//llvm/utils/gn/build/toolchain/compiler.gni")
 import("//llvm/utils/gn/build/write_cmake_config.gni")
 import("//llvm/version.gni")
 
@@ -15,6 +16,14 @@
   }
 }
 
+supported_toolchains = []
+if (android_ndk_path != "") {
+  supported_toolchains += [
+"//llvm/utils/gn/build/toolchain:stage2_android_aarch64",
+"//llvm/utils/gn/build/toolchain:stage2_android_arm",
+  ]
+}
+
 executable("lldb") {
   configs += [ "//llvm/utils/gn/build:lldb_code" ]
   deps = [
@@ -35,6 +44,9 @@
   if (current_os == "linux") {
 data_deps += [ "//lldb/tools/lldb-server" ]
   }
+  foreach (toolchain, supported_toolchains) {
+data_deps += [ "//lldb/tools/lldb-server($toolchain)" ]
+  }
 
   sources = [
 "Driver.cpp",
Index: llvm/utils/gn/build/libs/xml/enable.gni
===
--- llvm/utils/gn/build/libs/xml/enable.gni
+++ llvm/utils/gn/build/libs/xml/enable.gni
@@ -1,4 +1,4 @@
 declare_args() {
   # Whether to include code that links against libxml2.
-  llvm_enable_libxml2 = host_os != "win"
+  llvm_enable_libxml2 = current_os != "win" && current_os != "android"
 }


Index: llvm/utils/gn/secondary/lldb/tools/lldb-server/BUILD.gn
===
--- llvm/utils/gn/secondary/lldb/tools/lldb-server/BUILD.gn
+++ llvm/utils/gn/secondary/lldb/tools/lldb-server/BUILD.gn
@@ -1,3 +1,4 @@
+import("//compiler-rt/target.gni")
 import("//llvm/utils/TableGen/tablegen.gni")
 
 tablegen("LLGSOptions") {
@@ -57,4 +58,8 @@
 "lldb-platform.cpp",
 "lldb-server.cpp",
   ]
+
+  if (current_os == "android") {
+output_dir = "$root_build_dir/runtimes_ndk_cxx/$crt_current_target_arch"
+  }
 }
Index: llvm/utils/gn/secondary/lldb/tools/driver/BUILD.gn
===
--- llvm/utils/gn/secondary/lldb/tools/driver/BUILD.gn
+++ llvm/utils/gn/secondary/lldb/tools/driver/BUILD.gn
@@ -1,4 +1,5 @@
 import("//llvm/utils/TableGen/tablegen.gni")
+import("//llvm/utils/gn/build/toolchain/compiler.gni")
 import("//llvm/utils/gn/build/write_cmake_config.gni")
 import("//llvm/version.gni")
 
@@ -15,6 +16,14 @@
   }
 }
 
+supported_toolchains = []
+if (android_ndk_path != "") {
+  supported_toolchains += [
+"//llvm/utils/gn/build/toolchain:stage2_android_aarch64",
+"//llvm/utils/gn/build/toolchain:stage2_android_arm",
+  ]
+}
+
 executable("lldb") {
   configs += [ "//llvm/utils/gn/build:lldb_code" ]
   deps = [
@@ -35,6 +44,9 @@
   if (current_os == "linux") {
 data_deps += [ "//lldb/tools/lldb-server" ]
   }
+  foreach (toolchain, supported_toolchains) {
+data_deps += [ "//lldb/tools/lldb-server($toolchain)" ]
+  }
 
   sources = [
 "Driver.cpp",
Index: llvm/utils/gn/build/libs/xml/enable.gni
===
--- llvm/utils/gn/build/libs/xml/enable.gni
+++ llvm/utils/gn/build/libs/xml/enable.gni
@@ -1,4 +1,4 @@
 declare_args() {
   # Whether to include code that links against libxml2.
-  llvm_enable_libxml2 = host_os != "win"
+  llvm_enable_libxml2 = current_os != "win" && current_os != "android"
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D109463: gn build: Add support for building LLDB on Linux.

[Nico Weber via Phabricator via lldb-commits]

thakis added inline comments.



Comment at: llvm/utils/gn/secondary/BUILD.gn:15
 "//lld/test",
+"//lldb",
 "//llvm/test",

pcc wrote:
> thakis wrote:
> > Does this build fine on windows?
> > 
> > Generally this only depends on the test targets which in turn depend on the 
> > binaries, so probably should only have the old/test dep in this file 
> > anyways.
> You're right, this needs to avoid the dependency on lldb on Windows targets.
> 
> The problem with only depending on `//lldb/test` is that nothing else refers 
> to `//lldb`, so `ninja lldb` wouldn't also build `lldb-server`. Maybe it 
> would be better to add `lldb-server` to the `data_deps` of 
> `//lldb/tools/driver:lldb` instead then.
I guess `//lldb/test` could depend on `//:lldb` instead of 
`//lldb/tools/driver`?

We currently don't use data_deps anywhere else. (I have 
https://github.com/nico/llvm-project/commit/7246393c6bbc270044641415ffb0db93ffee3e29
 on a branch, but uploads with static links take so long that it isn't really 
worth it. Maybe I should revisit that with `-fno-semantic-interposition`…)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109463

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

[Lldb-commits] [PATCH] D109463: gn build: Add support for building LLDB on Linux.

[Peter Collingbourne via Phabricator via lldb-commits]

pcc added inline comments.



Comment at: llvm/utils/gn/secondary/BUILD.gn:15
 "//lld/test",
+"//lldb",
 "//llvm/test",

thakis wrote:
> pcc wrote:
> > thakis wrote:
> > > Does this build fine on windows?
> > > 
> > > Generally this only depends on the test targets which in turn depend on 
> > > the binaries, so probably should only have the old/test dep in this file 
> > > anyways.
> > You're right, this needs to avoid the dependency on lldb on Windows targets.
> > 
> > The problem with only depending on `//lldb/test` is that nothing else 
> > refers to `//lldb`, so `ninja lldb` wouldn't also build `lldb-server`. 
> > Maybe it would be better to add `lldb-server` to the `data_deps` of 
> > `//lldb/tools/driver:lldb` instead then.
> I guess `//lldb/test` could depend on `//:lldb` instead of 
> `//lldb/tools/driver`?
> 
> We currently don't use data_deps anywhere else. (I have 
> https://github.com/nico/llvm-project/commit/7246393c6bbc270044641415ffb0db93ffee3e29
>  on a branch, but uploads with static links take so long that it isn't really 
> worth it. Maybe I should revisit that with `-fno-semantic-interposition`…)
And then `//:lldb` would depend on lldb and the various lldb-servers? I suppose 
that could work.

It seems like clang's existing dependency on e.g. clang-offload-bundler, which 
is the sort of dependency we want here, is currently added via deps, and I see 
that your change moves that to data_deps. For executable->executable 
dependencies I suppose that deps means the same thing as data_deps (test 
isolation aside) but to be consistent with what's in clang maybe we should just 
stick to what I have here except with s/data_deps/deps/g.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109463

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

[Lldb-commits] [PATCH] D109463: gn build: Add support for building LLDB on Linux.

[Nico Weber via Phabricator via lldb-commits]

thakis added inline comments.



Comment at: llvm/utils/gn/secondary/BUILD.gn:15
 "//lld/test",
+"//lldb",
 "//llvm/test",

pcc wrote:
> thakis wrote:
> > pcc wrote:
> > > thakis wrote:
> > > > Does this build fine on windows?
> > > > 
> > > > Generally this only depends on the test targets which in turn depend on 
> > > > the binaries, so probably should only have the old/test dep in this 
> > > > file anyways.
> > > You're right, this needs to avoid the dependency on lldb on Windows 
> > > targets.
> > > 
> > > The problem with only depending on `//lldb/test` is that nothing else 
> > > refers to `//lldb`, so `ninja lldb` wouldn't also build `lldb-server`. 
> > > Maybe it would be better to add `lldb-server` to the `data_deps` of 
> > > `//lldb/tools/driver:lldb` instead then.
> > I guess `//lldb/test` could depend on `//:lldb` instead of 
> > `//lldb/tools/driver`?
> > 
> > We currently don't use data_deps anywhere else. (I have 
> > https://github.com/nico/llvm-project/commit/7246393c6bbc270044641415ffb0db93ffee3e29
> >  on a branch, but uploads with static links take so long that it isn't 
> > really worth it. Maybe I should revisit that with 
> > `-fno-semantic-interposition`…)
> And then `//:lldb` would depend on lldb and the various lldb-servers? I 
> suppose that could work.
> 
> It seems like clang's existing dependency on e.g. clang-offload-bundler, 
> which is the sort of dependency we want here, is currently added via deps, 
> and I see that your change moves that to data_deps. For 
> executable->executable dependencies I suppose that deps means the same thing 
> as data_deps (test isolation aside) but to be consistent with what's in clang 
> maybe we should just stick to what I have here except with s/data_deps/deps/g.
sgtm.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109463

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

[Lldb-commits] [PATCH] D109463: gn build: Add support for building LLDB on Linux.

[Nico Weber via Phabricator via lldb-commits]

thakis added a comment.

(Please watch http://45.33.8.238/ when landing this :) )


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109463

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

[Lldb-commits] [PATCH] D109463: gn build: Add support for building LLDB on Linux.

[Peter Collingbourne 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 rG9449f441fc9b: gn build: Add support for building LLDB on 
Linux. (authored by pcc).

Changed prior to commit:
  https://reviews.llvm.org/D109463?vs=371464&id=371490#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109463

Files:
  llvm/utils/gn/secondary/BUILD.gn
  llvm/utils/gn/secondary/lldb/include/lldb/Host/BUILD.gn
  llvm/utils/gn/secondary/lldb/source/Initialization/BUILD.gn
  llvm/utils/gn/secondary/lldb/source/Plugins/BUILD.gn
  llvm/utils/gn/secondary/lldb/source/Plugins/ObjectFile/JIT/BUILD.gn
  llvm/utils/gn/secondary/lldb/source/Plugins/Platform/Linux/BUILD.gn
  llvm/utils/gn/secondary/lldb/source/Plugins/Process/Linux/BUILD.gn
  llvm/utils/gn/secondary/lldb/source/Plugins/Process/POSIX/BUILD.gn
  llvm/utils/gn/secondary/lldb/tools/driver/BUILD.gn
  llvm/utils/gn/secondary/lldb/tools/lldb-server/BUILD.gn

Index: llvm/utils/gn/secondary/lldb/tools/lldb-server/BUILD.gn
===
--- llvm/utils/gn/secondary/lldb/tools/lldb-server/BUILD.gn
+++ llvm/utils/gn/secondary/lldb/tools/lldb-server/BUILD.gn
@@ -29,13 +29,13 @@
 "//lldb/source/Plugins/Language/CPlusPlus",
   ]
 
-  #if (current_os == "android" || current_os == "linux") {
-#deps += [ "//lldb/source/Plugins/Process/Linux" ]
+  if (current_os == "android" || current_os == "linux") {
+deps += [ "//lldb/source/Plugins/Process/Linux" ]
 #} else if (current_os == "freebsd") {
 #deps += [ "//lldb/source/Plugins/Process/FreeBSD" ]
 #} else if (current_os == "netbsd") {
 #deps += [ "//lldb/source/Plugins/Process/NetBSD" ]
-  #}
+  }
 
   if (current_os == "mac") {
 deps += [ "//lldb/source/Plugins/ObjectFile/Mach-O" ]
Index: llvm/utils/gn/secondary/lldb/tools/driver/BUILD.gn
===
--- llvm/utils/gn/secondary/lldb/tools/driver/BUILD.gn
+++ llvm/utils/gn/secondary/lldb/tools/driver/BUILD.gn
@@ -7,7 +7,7 @@
   args = [ "-gen-opt-parser-defs" ]
 }
 
-if (host_os == "mac") {
+if (current_os == "mac") {
   write_cmake_config("write_info_plist") {
 input = "lldb-Info.plist.in"
 output = "$target_gen_dir/lldb-Info.plist"
@@ -25,12 +25,15 @@
 "//llvm/lib/Support",
 "//llvm/utils/gn/build/libs/xar",
   ]
-  if (host_os == "mac") {
+  if (current_os == "mac") {
 deps += [ ":write_info_plist" ]
 plist = get_target_outputs(":write_info_plist")
 ldflags = [ "-Wl,-sectcreate,__TEXT,__info_plist," +
 rebase_path(plist[0], root_out_dir) ]
   }
+  if (current_os == "linux") {
+deps += [ "//lldb/tools/lldb-server" ]
+  }
 
   sources = [
 "Driver.cpp",
Index: llvm/utils/gn/secondary/lldb/source/Plugins/Process/POSIX/BUILD.gn
===
--- /dev/null
+++ llvm/utils/gn/secondary/lldb/source/Plugins/Process/POSIX/BUILD.gn
@@ -0,0 +1,17 @@
+static_library("POSIX") {
+  output_name = "lldbPluginProcessPOSIX"
+  configs += [ "//llvm/utils/gn/build:lldb_code" ]
+  deps = [
+"//lldb/source/Utility",
+"//lldb/source/Plugins/Process/Utility",
+"//llvm/lib/Support",
+  ]
+  # Reaches into Plugins/Platform/Process/Utility.
+  include_dirs = [ "//lldb/source" ]
+  sources = [
+"CrashReason.cpp",
+"NativeProcessELF.cpp",
+"ProcessMessage.cpp",
+"ProcessPOSIXLog.cpp",
+  ]
+}
Index: llvm/utils/gn/secondary/lldb/source/Plugins/Process/Linux/BUILD.gn
===
--- /dev/null
+++ llvm/utils/gn/secondary/lldb/source/Plugins/Process/Linux/BUILD.gn
@@ -0,0 +1,29 @@
+static_library("Linux") {
+  output_name = "lldbPluginProcessLinux"
+  configs += [ "//llvm/utils/gn/build:lldb_code" ]
+  deps = [
+"//lldb/source/Core",
+"//lldb/source/Host",
+"//lldb/source/Symbol",
+"//lldb/source/Target",
+"//lldb/source/Utility",
+"//lldb/source/Plugins/Process/POSIX",
+"//lldb/source/Plugins/Process/Utility",
+"//llvm/lib/Support",
+  ]
+  # Uses source-relative includes for own headers.
+  include_dirs = [ "//lldb/source" ]
+  sources = [
+"IntelPTManager.cpp",
+"NativeProcessLinux.cpp",
+"NativeRegisterContextLinux.cpp",
+"NativeRegisterContextLinux_arm.cpp",
+"NativeRegisterContextLinux_arm64.cpp",
+"NativeRegisterContextLinux_ppc64le.cpp",
+"NativeRegisterContextLinux_s390x.cpp",
+"NativeRegisterContextLinux_x86_64.cpp",
+"NativeThreadLinux.cpp",
+"SingleStepCheck.cpp",
+  ]
+}
+
Index: llvm/utils/gn/secondary/lldb/source/Plugins/Platform/Linux/BUILD.gn
===
--- /dev/null
+++ llvm/utils/gn/secondary/lldb/source/Plugins/Platform/Linux/BUILD.gn
@@ -0,0 +1,18 @@
+static_library("Linux") {
+  output_

[Lldb-commits] [PATCH] D109464: gn build: Add support for building lldb-server on Android.

[Peter Collingbourne 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 rG883e93cb280e: gn build: Add support for building lldb-server 
on Android. (authored by pcc).

Changed prior to commit:
  https://reviews.llvm.org/D109464?vs=371465&id=371491#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109464

Files:
  llvm/utils/gn/build/libs/xml/enable.gni
  llvm/utils/gn/secondary/lldb/tools/driver/BUILD.gn
  llvm/utils/gn/secondary/lldb/tools/lldb-server/BUILD.gn


Index: llvm/utils/gn/secondary/lldb/tools/lldb-server/BUILD.gn
===
--- llvm/utils/gn/secondary/lldb/tools/lldb-server/BUILD.gn
+++ llvm/utils/gn/secondary/lldb/tools/lldb-server/BUILD.gn
@@ -1,3 +1,4 @@
+import("//compiler-rt/target.gni")
 import("//llvm/utils/TableGen/tablegen.gni")
 
 tablegen("LLGSOptions") {
@@ -57,4 +58,8 @@
 "lldb-platform.cpp",
 "lldb-server.cpp",
   ]
+
+  if (current_os == "android") {
+output_dir = "$root_build_dir/runtimes_ndk_cxx/$crt_current_target_arch"
+  }
 }
Index: llvm/utils/gn/secondary/lldb/tools/driver/BUILD.gn
===
--- llvm/utils/gn/secondary/lldb/tools/driver/BUILD.gn
+++ llvm/utils/gn/secondary/lldb/tools/driver/BUILD.gn
@@ -1,4 +1,5 @@
 import("//llvm/utils/TableGen/tablegen.gni")
+import("//llvm/utils/gn/build/toolchain/compiler.gni")
 import("//llvm/utils/gn/build/write_cmake_config.gni")
 import("//llvm/version.gni")
 
@@ -15,6 +16,14 @@
   }
 }
 
+supported_toolchains = []
+if (android_ndk_path != "") {
+  supported_toolchains += [
+"//llvm/utils/gn/build/toolchain:stage2_android_aarch64",
+"//llvm/utils/gn/build/toolchain:stage2_android_arm",
+  ]
+}
+
 executable("lldb") {
   configs += [ "//llvm/utils/gn/build:lldb_code" ]
   deps = [
@@ -34,6 +43,9 @@
   if (current_os == "linux") {
 deps += [ "//lldb/tools/lldb-server" ]
   }
+  foreach (toolchain, supported_toolchains) {
+deps += [ "//lldb/tools/lldb-server($toolchain)" ]
+  }
 
   sources = [
 "Driver.cpp",
Index: llvm/utils/gn/build/libs/xml/enable.gni
===
--- llvm/utils/gn/build/libs/xml/enable.gni
+++ llvm/utils/gn/build/libs/xml/enable.gni
@@ -1,4 +1,4 @@
 declare_args() {
   # Whether to include code that links against libxml2.
-  llvm_enable_libxml2 = host_os != "win"
+  llvm_enable_libxml2 = current_os != "win" && current_os != "android"
 }


Index: llvm/utils/gn/secondary/lldb/tools/lldb-server/BUILD.gn
===
--- llvm/utils/gn/secondary/lldb/tools/lldb-server/BUILD.gn
+++ llvm/utils/gn/secondary/lldb/tools/lldb-server/BUILD.gn
@@ -1,3 +1,4 @@
+import("//compiler-rt/target.gni")
 import("//llvm/utils/TableGen/tablegen.gni")
 
 tablegen("LLGSOptions") {
@@ -57,4 +58,8 @@
 "lldb-platform.cpp",
 "lldb-server.cpp",
   ]
+
+  if (current_os == "android") {
+output_dir = "$root_build_dir/runtimes_ndk_cxx/$crt_current_target_arch"
+  }
 }
Index: llvm/utils/gn/secondary/lldb/tools/driver/BUILD.gn
===
--- llvm/utils/gn/secondary/lldb/tools/driver/BUILD.gn
+++ llvm/utils/gn/secondary/lldb/tools/driver/BUILD.gn
@@ -1,4 +1,5 @@
 import("//llvm/utils/TableGen/tablegen.gni")
+import("//llvm/utils/gn/build/toolchain/compiler.gni")
 import("//llvm/utils/gn/build/write_cmake_config.gni")
 import("//llvm/version.gni")
 
@@ -15,6 +16,14 @@
   }
 }
 
+supported_toolchains = []
+if (android_ndk_path != "") {
+  supported_toolchains += [
+"//llvm/utils/gn/build/toolchain:stage2_android_aarch64",
+"//llvm/utils/gn/build/toolchain:stage2_android_arm",
+  ]
+}
+
 executable("lldb") {
   configs += [ "//llvm/utils/gn/build:lldb_code" ]
   deps = [
@@ -34,6 +43,9 @@
   if (current_os == "linux") {
 deps += [ "//lldb/tools/lldb-server" ]
   }
+  foreach (toolchain, supported_toolchains) {
+deps += [ "//lldb/tools/lldb-server($toolchain)" ]
+  }
 
   sources = [
 "Driver.cpp",
Index: llvm/utils/gn/build/libs/xml/enable.gni
===
--- llvm/utils/gn/build/libs/xml/enable.gni
+++ llvm/utils/gn/build/libs/xml/enable.gni
@@ -1,4 +1,4 @@
 declare_args() {
   # Whether to include code that links against libxml2.
-  llvm_enable_libxml2 = host_os != "win"
+  llvm_enable_libxml2 = current_os != "win" && current_os != "android"
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits