date:20240801


DavidSpickett wrote:

I thought I saw that fixed once already, but perhaps that was just a full 
revert.

We (Linaro) can test a patch for you if it's fiddly to verify, we just need to 
know the general idea of it.

https://github.com/llvm/llvm-project/pull/97273
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb/Commands] Add `scripting template list` command with auto discovery (PR #97273)


DavidSpickett wrote:

Actually I'm looking at old results, the latest build makes it to tests 
https://lab.llvm.org/buildbot/#/builders/141/builds/1285 (which fail for 
reasons unrelated to this patch).

https://github.com/llvm/llvm-project/pull/97273
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test][win][x86_64] Fix XFAIL and XPASS on LLDB API tests (PR #100477)


DavidSpickett wrote:

Thanks for your efforts @kendalharland. If you think any of these details are 
worth documenting (https://lldb.llvm.org/index.html), please do so. The page 
sources are in `lldb/docs`.

https://github.com/llvm/llvm-project/pull/100477
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test] Support remote run of Shell tests (PR #95986)



@@ -2,6 +2,8 @@
 # RUN: %lldb -x -b -o 'settings set interpreter.echo-comment-commands false' 
-s %S/Inputs/EchoCommandsTest.in | FileCheck 
%S/Inputs/EchoCommandsNoComments.out
 # RUN: %lldb -x -b -o 'settings set interpreter.echo-commands false' 
-s %S/Inputs/EchoCommandsTest.in | FileCheck %S/Inputs/EchoCommandsNone.out
 
+XFAIL: remote{{.*}}

DavidSpickett wrote:

Why exactly does this fail when run remote? I get that in some sense it doesn't 
apply because nothing would happen on the remote, but I'm surprised that it 
fails.

Is it because with a remote, %lldb is actually `lldb -o platform connect` etc 
and that screws up the output?

https://github.com/llvm/llvm-project/pull/95986
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test] Support remote run of Shell tests (PR #95986)



@@ -2,6 +2,8 @@
 # RUN: %lldb -x -b -o 'settings set interpreter.echo-comment-commands false' 
-s %S/Inputs/EchoCommandsTest.in | FileCheck 
%S/Inputs/EchoCommandsNoComments.out
 # RUN: %lldb -x -b -o 'settings set interpreter.echo-commands false' 
-s %S/Inputs/EchoCommandsTest.in | FileCheck %S/Inputs/EchoCommandsNone.out
 
+XFAIL: remote{{.*}}

DavidSpickett wrote:

I would add the explanation as a comment, even if it is obvious in hindsight.

https://github.com/llvm/llvm-project/pull/95986
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test] Support remote run of Shell tests (PR #95986)



@@ -22,6 +25,60 @@ def _disallow(config, execName):
 config.substitutions.append((" {0} ".format(execName), 
warning.format(execName)))
 
 
+def get_lldb_args(config, suffix=""):
+lldb_args = []
+if "remote-linux" in config.available_features:
+lldb_args += [
+"-O",
+'"platform select remote-linux"',
+"-O",
+f'"platform connect {config.lldb_platform_url}"',
+]
+if config.lldb_platform_working_dir:
+dir = posixpath.join(f"{config.lldb_platform_working_dir}", 
"shell")
+if suffix:
+dir += posixpath.join(dir, f"{suffix}")
+lldb_args += [
+"-O",
+f'"platform shell mkdir -p {dir}"',
+"-O",
+f'"platform settings -w {dir}"',
+]
+lldb_args += ["--no-lldbinit", "-S", _get_lldb_init_path(config)]
+return lldb_args
+
+
+class ShTestLldb(ShTest):
+def __init__(
+self, execute_external=False, extra_substitutions=[], 
preamble_commands=[]
+):
+super().__init__(execute_external, extra_substitutions, 
preamble_commands)
+
+def execute(self, test, litConfig):
+# Run each Shell test in a separate directory (on remote).
+
+# Find directory change command in %lldb substitution.
+for i, t in enumerate(test.config.substitutions):
+if re.match(t[0], "%lldb"):
+cmd = t[1]
+if '-O "platform settings -w ' in cmd:
+# If command is present, it is added by get_lldb_args.

DavidSpickett wrote:

I'm struggling to see exactly what this is replacing and with what.

Can you add a comment giving an example? You say replace the path with the 
tests' path in the suite but it seems like you'd want to append to the path 
originally set wouldn't you?

Also adding something in get_lldb_args to immediately modify it here seems like 
one or the other should do it not both, but without an example that's just a 
feeling I have here.

https://github.com/llvm/llvm-project/pull/95986
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test] Support remote run of Shell tests (PR #95986)



@@ -22,6 +25,60 @@ def _disallow(config, execName):
 config.substitutions.append((" {0} ".format(execName), 
warning.format(execName)))
 
 
+def get_lldb_args(config, suffix=""):
+lldb_args = []
+if "remote-linux" in config.available_features:
+lldb_args += [
+"-O",
+'"platform select remote-linux"',
+"-O",
+f'"platform connect {config.lldb_platform_url}"',
+]
+if config.lldb_platform_working_dir:
+dir = posixpath.join(f"{config.lldb_platform_working_dir}", 
"shell")
+if suffix:
+dir += posixpath.join(dir, f"{suffix}")
+lldb_args += [
+"-O",
+f'"platform shell mkdir -p {dir}"',
+"-O",
+f'"platform settings -w {dir}"',
+]
+lldb_args += ["--no-lldbinit", "-S", _get_lldb_init_path(config)]
+return lldb_args
+
+
+class ShTestLldb(ShTest):
+def __init__(
+self, execute_external=False, extra_substitutions=[], 
preamble_commands=[]
+):
+super().__init__(execute_external, extra_substitutions, 
preamble_commands)
+
+def execute(self, test, litConfig):
+# Run each Shell test in a separate directory (on remote).
+
+# Find directory change command in %lldb substitution.
+for i, t in enumerate(test.config.substitutions):
+if re.match(t[0], "%lldb"):

DavidSpickett wrote:

Why is this re.match not `in` or `startswith`, normal string comparison stuff.

https://github.com/llvm/llvm-project/pull/95986
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test] Support remote run of Shell tests (PR #95986)


DavidSpickett wrote:

Ok I see what `LLDB_SHELL_TESTS_DISABLE_REMOTE` does but I don't get why. 
Perhaps you can show how a developer would:
* Hit the problem that this option addresses
* Apply the option
* What problems it would solve and what that final build looks like
* What the value of the results of that build are to the developer

I'm thinking that final build has unit and shell tests run locally, API tests 
run on the remote? I suppose we already support unit tests locally and API run 
remote, so this carries on that idea, but I've just never tried to do that 
because I'm generally x86 -> AArch64 where the cross compiler is never 
compatible.

https://github.com/llvm/llvm-project/pull/95986
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb/Commands] Add `scripting template list` command with auto discovery (PR #97273)

2024-08-01 Thread Med Ismail Bennani via lldb-commits

medismailben wrote:

Hey @DavidSpickett! Thanks for the help! I don't have a system to try this on
and it only fails on windows :/

> Actually I'm looking at old results, the latest build makes it to tests
> https://lab.llvm.org/buildbot/#/builders/141/builds/1285 (which fail for
> reasons unrelated to this patch).

My patch has already been reverted in 9effefbae8d96006a4dd29bb9ab8532fd408559d
and the failure you're talking about was introduced in #100710. I believe
@Michael137 is trying to fix it.

If you have a recent build of lldb on windows (with python enabled), could you
run the `scripting template list` command and paste the output here.

I've split this patch in smaller chunks (bccff3baeff8, ecf125eaf0b1,
995f643f5aa7) and they've all landed without any build failure on windows
except for 2402b3213c2f. I've noticed however that the cmake configure phase
would produce some warnings on the bot regarding the files related to my
changes, about the fact that they'd exceed `CMAKE_OBJECT_PATH_MAX` limit (250
on windows apparently).

You can see it happening here :
https://lab.llvm.org/buildbot/#/builders/141/builds/1222

```
CMake Warning in
C:/Users/tcwg/llvm-worker/lldb-aarch64-windows/llvm-project/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedProcessPythonInterface/CMakeLists.txt:
The object file directory

C:/Users/tcwg/llvm-worker/lldb-aarch64-windows/build/tools/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedProcessPythonInterface/CMakeFiles/lldbPluginScriptInterpreterPythonScriptedProcessPythonInterface.dir/./
has 227 characters. The maximum full path to an object file is 250
characters (see CMAKE_OBJECT_PATH_MAX). Object file
ScriptedProcessPythonInterface.cpp.obj
cannot be safely placed under this directory. The build may not work
correctly.
```

Also, if you could re-apply e72cdae47b4e263ea97b2bdd75cf44c1510cf3be and attach
your build log here, that would be very helpful, and hopefully help us narrow
down the ambiguous typedef issue.

If I'm not able to make it work on windows, I'll unfortunately have to compile
it out of lldb for that platform, since I don't have access to a machine to
investigate the issue further.

Thank you in advance!

https://github.com/llvm/llvm-project/pull/97273
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Multithreading lldb-server works on Windows now; fixed gdb port mapping (PR #100670)


https://github.com/slydiman updated 
https://github.com/llvm/llvm-project/pull/100670

>From 7fb5b2ee53f3125dc2b93cb6fba28b35c70b70e1 Mon Sep 17 00:00:00 2001
From: Dmitry Vasilyev 
Date: Thu, 25 Jul 2024 00:34:34 +0400
Subject: [PATCH 1/2] [lldb] Multithreading lldb-server works on Windows now;
 fixed gdb port mapping

Removed fork(). Used threads and the common thread-safe port map for all 
platform connections.

Updated lldb::FileSystem to use llvm::vfs::createPhysicalFileSystem() with an 
own virtual working directory per thread.

This patch depends on #100659, #100666.

This patch fixes #97537, #90923, #56346.

lldb-server has been tested on Windows with 50 connections and 100 processes 
launched simultaneously. Tested also the cross build with Linux x86_64 host and 
Linux Aarch64 target.
---
 lldb/include/lldb/Host/FileSystem.h   |   7 +
 lldb/source/Host/common/FileSystem.cpp|   8 +
 lldb/source/Host/posix/PipePosix.cpp  |  12 +
 .../GDBRemoteCommunicationServerCommon.cpp|  15 +-
 .../GDBRemoteCommunicationServerPlatform.cpp  | 309 --
 .../GDBRemoteCommunicationServerPlatform.h|  31 +-
 lldb/tools/lldb-server/lldb-platform.cpp  |  99 ++
 7 files changed, 298 insertions(+), 183 deletions(-)

diff --git a/lldb/include/lldb/Host/FileSystem.h 
b/lldb/include/lldb/Host/FileSystem.h
index 640f3846e448c..5e25414a894d3 100644
--- a/lldb/include/lldb/Host/FileSystem.h
+++ b/lldb/include/lldb/Host/FileSystem.h
@@ -47,6 +47,12 @@ class FileSystem {
 
   static FileSystem &Instance();
 
+  static void InitializePerThread() {
+lldbassert(!InstancePerThread() && "Already initialized.");
+
InstancePerThread().emplace(llvm::IntrusiveRefCntPtr(
+llvm::vfs::createPhysicalFileSystem().release()));
+  }
+
   template  static void Initialize(T &&...t) {
 lldbassert(!InstanceImpl() && "Already initialized.");
 InstanceImpl().emplace(std::forward(t)...);
@@ -206,6 +212,7 @@ class FileSystem {
 
 private:
   static std::optional &InstanceImpl();
+  static std::optional &InstancePerThread();
   llvm::IntrusiveRefCntPtr m_fs;
   std::unique_ptr m_tilde_resolver;
   std::string m_home_directory;
diff --git a/lldb/source/Host/common/FileSystem.cpp 
b/lldb/source/Host/common/FileSystem.cpp
index 5153a0a9ec513..cb76086616d6b 100644
--- a/lldb/source/Host/common/FileSystem.cpp
+++ b/lldb/source/Host/common/FileSystem.cpp
@@ -49,7 +49,15 @@ void FileSystem::Terminate() {
   InstanceImpl().reset();
 }
 
+std::optional &FileSystem::InstancePerThread() {
+  static thread_local std::optional t_fs;
+  return t_fs;
+}
+
 std::optional &FileSystem::InstanceImpl() {
+  std::optional &fs = InstancePerThread();
+  if (fs)
+return fs;
   static std::optional g_fs;
   return g_fs;
 }
diff --git a/lldb/source/Host/posix/PipePosix.cpp 
b/lldb/source/Host/posix/PipePosix.cpp
index f35c348990df6..1aa02efe86610 100644
--- a/lldb/source/Host/posix/PipePosix.cpp
+++ b/lldb/source/Host/posix/PipePosix.cpp
@@ -324,6 +324,18 @@ Status PipePosix::ReadWithTimeout(void *buf, size_t size,
 bytes_read += result;
 if (bytes_read == size || result == 0)
   break;
+
+// This is the workaround for the following bug in Linux multithreading
+// select() https://bugzilla.kernel.org/show_bug.cgi?id=546
+// ReadWithTimeout() with a non-zero timeout is used only to
+// read the port number from the gdbserver pipe
+// in GDBRemoteCommunication::StartDebugserverProcess().
+// The port number may be "1024\0".."65535\0".
+if (timeout.count() > 0 && size == 6 && bytes_read == 5 &&
+static_cast(buf)[4] == '\0') {
+  break;
+}
+
   } else if (errno == EINTR) {
 continue;
   } else {
diff --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
index f9d37490e16ae..cef836e001adf 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -646,7 +646,9 @@ GDBRemoteCommunicationServerCommon::Handle_vFile_Size(
   packet.GetHexByteString(path);
   if (!path.empty()) {
 uint64_t Size;
-if (llvm::sys::fs::file_size(path, Size))
+FileSpec file_spec(path);
+FileSystem::Instance().Resolve(file_spec);
+if (llvm::sys::fs::file_size(file_spec.GetPath(), Size))
   return SendErrorResponse(5);
 StreamString response;
 response.PutChar('F');
@@ -725,7 +727,9 @@ GDBRemoteCommunicationServerCommon::Handle_vFile_unlink(
   packet.SetFilePos(::strlen("vFile:unlink:"));
   std::string path;
   packet.GetHexByteString(path);
-  Status error(llvm::sys::fs::remove(path));
+  FileSpec file_spec(path);
+  FileSystem::Instance().Resolve(file_spec);
+  Status error(llvm::sys::fs::remove(file_spec.GetPath()));
   StreamString response;

[Lldb-commits] [lldb] [lldb][TypeSystemClang] Create VLAs of explicitly 0-size as ConstantArrayType (PR #100710)


DavidSpickett wrote:

I'm going to look into this now.

https://github.com/llvm/llvm-project/pull/100710
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 229a165 - [lldb][test] Disable vla test on Windows


Author: David Spickett
Date: 2024-08-01T12:59:49Z
New Revision: 229a16590a3cd65da77bb868498d3eed63bf6263

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

LOG: [lldb][test] Disable vla test on Windows

For the same reasons as 6cfac497e96978f2bfc50a00b51c198f2ed50f82.

This test was added in https://github.com/llvm/llvm-project/pull/100710.

It fails because when we're linking with link.exe, -gdwarf has no
effect and we get a PDB file anyway. The Windows on Arm lldb bot
uses link.exe.

 "C:\\Program Files\\Microsoft Visual 
Studio\\2022\\Community\\VC\\Tools\\MSVC\\14.34.31933\\bin\\Hostx86\\arm64\\link.exe"
 <...>

08/01/2024  01:47 PM 2,956,488 vla.cpp.ilk
08/01/2024  01:47 PM 6,582,272 vla.cpp.pdb
08/01/2024  01:47 PM   734,208 vla.cpp.tmp

Added: 


Modified: 
lldb/test/Shell/SymbolFile/DWARF/vla.cpp

Removed: 




diff  --git a/lldb/test/Shell/SymbolFile/DWARF/vla.cpp 
b/lldb/test/Shell/SymbolFile/DWARF/vla.cpp
index 344b100efd9f9..47a4aa1836899 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/vla.cpp
+++ b/lldb/test/Shell/SymbolFile/DWARF/vla.cpp
@@ -1,3 +1,6 @@
+// When linking with link.exe, -gdwarf still produces PDB instead.
+// UNSUPPORTED: system-windows
+
 // RUN: %clangxx_host -gdwarf -std=c++11 -o %t %s
 // RUN: %lldb %t \
 // RUN:   -o run \



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

[Lldb-commits] [lldb] [lldb][TypeSystemClang] Create VLAs of explicitly 0-size as ConstantArrayType (PR #100710)


DavidSpickett wrote:

Yeah, won't work with link.exe, so I've just disabled it. Nothing more for you 
to do here.

https://github.com/llvm/llvm-project/pull/100710
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb/Commands] Add `scripting template list` command with auto discovery (PR #97273)


DavidSpickett wrote:

> If you have a recent build of lldb on windows (with python enabled), could 
> you run the scripting template list command and paste the output here.

As of 229a16590a3cd65da77bb868498d3eed63bf6263:
```
C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build>.\bin\lldb.exe
(lldb) scripting template list
Available scripted extension templates:
  Name: OperatingSystemPythonInterface
  Language: Python
  Description: Mock thread state
  API Usages: None
  Command Interpreter Usages:
settings set target.process.python-os-plugin-path 
settings set process.experimental.os-plugin-reports-all-threads [0/1]

  Name: ScriptedPlatformPythonInterface
  Language: Python
  Description: Mock platform and interact with its processes.
  API Usages: None
  Command Interpreter Usages: None

  Name: ScriptedThreadPlanPythonInterface
  Language: Python
  Description: Alter thread stepping logic and stop reason
  API Usages: SBThread.StepUsingScriptedThreadPlan
  Command Interpreter Usages: thread step-scripted -C  [-k key -v 
value ...]
```


https://github.com/llvm/llvm-project/pull/97273
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test] Support remote run of Shell tests (PR #95986)

2024-08-01 Thread Vladislav Dzhidzhoev via lldb-commits


https://github.com/dzhidzhoev updated 
https://github.com/llvm/llvm-project/pull/95986

>From b747d2cf8d648e0dadda4adaaf2e0ef41d4ebd34 Mon Sep 17 00:00:00 2001
From: Vladislav Dzhidzhoev 
Date: Fri, 31 May 2024 21:39:56 +
Subject: [PATCH 1/5] [lldb][test] Support remote run of Shell tests

1. This commit adds LLDB_TEST_PLATFORM_URL, LLDB_TEST_SYSROOT,
   LLDB_TEST_PLATFORM_WORKING_DIR, LLDB_SHELL_TESTS_DISABLE_REMOTE cmake flags
   to pass arguments for cross-compilation and remote running of both Shell&API
   tests.
2. To run Shell tests remotely, it adds 'platform select' and 'platform 
connect' commands to %lldb
   substitution.
3. 'remote-linux' feature added to lit to disable tests failing with remote 
execution.
4. A separate working directory is assigned to each test to avoid
   conflicts during parallel test execution.
5. Remote Shell testing is run only when LLDB_TEST_SYSROOT is set for
   building test sources. Recommended compiler for that is Clang.
---
 lldb/docs/resources/test.rst  | 20 +++---
 lldb/test/API/lit.cfg.py  |  7 ++
 lldb/test/API/lit.site.cfg.py.in  |  3 +
 lldb/test/CMakeLists.txt  |  1 +
 .../test/Shell/Settings/TestEchoCommands.test |  2 +
 lldb/test/Shell/Target/target-label.test  | 20 +++---
 lldb/test/Shell/helper/toolchain.py   | 67 ++-
 lldb/test/Shell/lit.cfg.py|  9 ++-
 lldb/test/Shell/lit.site.cfg.py.in|  7 +-
 9 files changed, 112 insertions(+), 24 deletions(-)

diff --git a/lldb/docs/resources/test.rst b/lldb/docs/resources/test.rst
index 382e42bf22b10..23b6ab03559d4 100644
--- a/lldb/docs/resources/test.rst
+++ b/lldb/docs/resources/test.rst
@@ -592,15 +592,17 @@ test suite, but there are two things to have in mind:
multiple connections. For more information on how to setup remote debugging
see the Remote debugging page.
 2. You must tell the test-suite how to connect to the remote system. This is
-   achieved using the ``--platform-name``, ``--platform-url`` and
-   ``--platform-working-dir`` parameters to ``dotest.py``. These parameters
-   correspond to the platform select and platform connect LLDB commands. You
-   will usually also need to specify the compiler and architecture for the
-   remote system.
-
-Currently, running the remote test suite is supported only with ``dotest.py`` 
(or
-dosep.py with a single thread), but we expect this issue to be addressed in the
-near future.
+   achieved using the ``LLDB_TEST_PLATFORM_URL``, 
``LLDB_TEST_PLATFORM_WORKING_DIR``
+   flags to cmake, and ``--platform-name`` parameter to ``dotest.py``.
+   These parameters correspond to the platform select and platform connect
+   LLDB commands. You will usually also need to specify the compiler and
+   architecture for the remote system.
+3. Remote Shell tests execution is currently supported only for Linux target
+   platform. It's triggered when ``LLDB_TEST_SYSROOT`` is provided for building
+   test sources. It can be disabled by setting 
``LLDB_SHELL_TESTS_DISABLE_REMOTE=On``.
+   Shell tests are not guaranteed to pass against remote target if the compiler
+   being used is other than Clang.
+
 
 Running tests in QEMU System Emulation Environment
 ``
diff --git a/lldb/test/API/lit.cfg.py b/lldb/test/API/lit.cfg.py
index 96520c7c82624..6a0a1b0a76675 100644
--- a/lldb/test/API/lit.cfg.py
+++ b/lldb/test/API/lit.cfg.py
@@ -303,6 +303,13 @@ def delete_module_cache(path):
 # In particular, (1) is visited at the top of the file, since the script
 # derives other information from it.
 
+if is_configured("lldb_platform_url"):
+dotest_cmd += ["--platform-url", config.lldb_platform_url]
+if is_configured("lldb_platform_working_dir"):
+dotest_cmd += ["--platform-working-dir", config.lldb_platform_working_dir]
+if is_configured("cmake_sysroot"):
+dotest_cmd += ["--sysroot", config.cmake_sysroot]
+
 if is_configured("dotest_user_args_str"):
 dotest_cmd.extend(config.dotest_user_args_str.split(";"))
 
diff --git a/lldb/test/API/lit.site.cfg.py.in b/lldb/test/API/lit.site.cfg.py.in
index 8b2d09ae41cd2..db3cd2971f347 100644
--- a/lldb/test/API/lit.site.cfg.py.in
+++ b/lldb/test/API/lit.site.cfg.py.in
@@ -24,6 +24,9 @@ config.lua_executable = "@Lua_EXECUTABLE@"
 config.lua_test_entry = "TestLuaAPI.py"
 config.dotest_common_args_str = 
lit_config.substitute("@LLDB_TEST_COMMON_ARGS@")
 config.dotest_user_args_str = lit_config.substitute("@LLDB_TEST_USER_ARGS@")
+config.lldb_platform_url = lit_config.substitute("@LLDB_TEST_PLATFORM_URL@")
+config.lldb_platform_working_dir = 
lit_config.substitute("@LLDB_TEST_PLATFORM_WORKING_DIR@")
+config.cmake_sysroot = lit_config.substitute("@LLDB_TEST_SYSROOT@" or 
"@DEFAULT_SYSROOT@")
 config.lldb_enable_python = @LLDB_ENABLE_PYTHON@
 config.dotest_lit_args_str = None
 config.enabled_plugins = []
diff --git a/lldb/test/CMakeLists.txt b/lldb/test/CMakeLi

[Lldb-commits] [lldb] [lldb][test] Support remote run of Shell tests (PR #95986)

2024-08-01 Thread Vladislav Dzhidzhoev via lldb-commits

dzhidzhoev wrote:

> Ok I see what `LLDB_SHELL_TESTS_DISABLE_REMOTE` does but I don't get why. 
> Perhaps you can show how a developer would:
> 
> * Hit the problem that this option addresses
> * Apply the option
> * What problems it would solve and what that final build looks like
> * What the value of the results of that build are to the developer

For example:
1. A developer uses gcc to cross-compile lldb and run remote testing. 
2. API tests pass because Makefile.rules provides the ability to use different 
compilers/adjusts all necessary flags.
3. Shell tests fail for some reason (--sysroot is not enough or not 
provided/some tests use clang-specific flags/etc). The developer thinks that 
it's sufficient for them to get away with Shell tests running only for the host 
platform.
4. LLDB_SHELL_TESTS_DISABLE_REMOTE is set to on.
5. Shell tests are compiled and run locally => tests are green.

https://github.com/llvm/llvm-project/pull/95986
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test] Support remote run of Shell tests (PR #95986)



@@ -22,6 +25,60 @@ def _disallow(config, execName):
 config.substitutions.append((" {0} ".format(execName), 
warning.format(execName)))
 
 
+def get_lldb_args(config, suffix=""):
+lldb_args = []
+if "remote-linux" in config.available_features:
+lldb_args += [
+"-O",
+'"platform select remote-linux"',
+"-O",
+f'"platform connect {config.lldb_platform_url}"',
+]
+if config.lldb_platform_working_dir:
+dir = posixpath.join(f"{config.lldb_platform_working_dir}", 
"shell")
+if suffix:
+dir += posixpath.join(dir, f"{suffix}")
+lldb_args += [
+"-O",
+f'"platform shell mkdir -p {dir}"',
+"-O",
+f'"platform settings -w {dir}"',
+]
+lldb_args += ["--no-lldbinit", "-S", _get_lldb_init_path(config)]
+return lldb_args
+
+
+class ShTestLldb(ShTest):
+def __init__(
+self, execute_external=False, extra_substitutions=[], 
preamble_commands=[]
+):
+super().__init__(execute_external, extra_substitutions, 
preamble_commands)
+
+def execute(self, test, litConfig):
+# Run each Shell test in a separate directory (on remote).
+
+# Find directory change command in %lldb substitution.
+for i, t in enumerate(test.config.substitutions):
+if re.match(t[0], "%lldb"):

DavidSpickett wrote:

Oh I see, I have this the wrong way around.

You're not looking for `%lldb` in something, you're checking whether the 
pattern in `t[0]` finds a match when applied to `%lldb`. A bit cryptic but ok, 
if that's what you've got then it makes sense to do it that way.

https://github.com/llvm/llvm-project/pull/95986
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test] Support remote run of Shell tests (PR #95986)


DavidSpickett wrote:

Ok, that seems reasonable. I'm still in a mode of thinking where a given build 
is all remote or all native. `LLDB_SHELL_TESTS_DISABLE_REMOTE` allows you to 
reuse all the settings you went to the trouble to set up, but on the host not 
the remote.

Though another way of thinking about it might be, I have a custom compiler 
setup but only intend to do host testing. This variable lets me tell the test 
runner not to assume I'm going to do any remote testing?

And if this compiler were the one from the toolchain you were going to ship, 
and include lldb with it, this lets you test host and remote then ship what 
you've tested, right?

https://github.com/llvm/llvm-project/pull/95986
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test] Support remote run of Shell tests (PR #95986)

2024-08-01 Thread Vladislav Dzhidzhoev via lldb-commits

dzhidzhoev wrote:

> Though another way of thinking about it might be, I have a custom compiler 
> setup but only intend to do host testing. This variable lets me tell the test 
> runner not to assume I'm going to do any remote testing?

Yes, considering only Shell tests. By default, host testing is assumed. 
LLDB_TEST_PLATFORM_NAME/LLDB_TEST_PLATFORM_URL must be set to execute remote 
API&Shell testing. LLDB_SHELL_TESTS_DISABLE_REMOTE may be used to disable it 
for Shell tests.

> And if this compiler were the one from the toolchain you were going to ship, 
> and include lldb with it, this lets you test host and remote then ship what 
> you've tested, right? Instead of testing host with the system compiler and 
> hoping nothing changes when you use the newly built cross compiler.

Technically, yes. But I think Shell tests don't aim to test a cross-compiler. 
It's more like "if the cross-compiler is used, that is not compatible with 
Shell tests, we can run them only on the host to check at least the part of the 
functionality". It may make sense since only one-fifth of Shell tests run 
something remotely.

https://github.com/llvm/llvm-project/pull/95986
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Added Pipe::WriteWithTimeout() (PR #101383)


https://github.com/slydiman updated 
https://github.com/llvm/llvm-project/pull/101383

>From 459303737e728a2cda683eb73f46763661126efa Mon Sep 17 00:00:00 2001
From: Dmitry Vasilyev 
Date: Wed, 31 Jul 2024 22:02:53 +0400
Subject: [PATCH] [lldb] Added Pipe::WriteWithTimeout()

Fixed few bugs in PipeWindows. Added the test for async read/write.
---
 lldb/include/lldb/Host/PipeBase.h|   5 +-
 lldb/include/lldb/Host/posix/PipePosix.h |   4 +-
 lldb/include/lldb/Host/windows/PipeWindows.h |   5 +-
 lldb/source/Host/common/PipeBase.cpp |   5 +
 lldb/source/Host/posix/PipePosix.cpp |   6 +-
 lldb/source/Host/windows/PipeWindows.cpp | 124 ---
 lldb/unittests/Host/PipeTest.cpp |  67 +-
 7 files changed, 163 insertions(+), 53 deletions(-)

diff --git a/lldb/include/lldb/Host/PipeBase.h 
b/lldb/include/lldb/Host/PipeBase.h
index 48c19b899cef6..d51d0cd54e036 100644
--- a/lldb/include/lldb/Host/PipeBase.h
+++ b/lldb/include/lldb/Host/PipeBase.h
@@ -56,7 +56,10 @@ class PipeBase {
   // Delete named pipe.
   virtual Status Delete(llvm::StringRef name) = 0;
 
-  virtual Status Write(const void *buf, size_t size, size_t &bytes_written) = 
0;
+  virtual Status WriteWithTimeout(const void *buf, size_t size,
+  const std::chrono::microseconds &timeout,
+  size_t &bytes_written) = 0;
+  Status Write(const void *buf, size_t size, size_t &bytes_written);
   virtual Status ReadWithTimeout(void *buf, size_t size,
  const std::chrono::microseconds &timeout,
  size_t &bytes_read) = 0;
diff --git a/lldb/include/lldb/Host/posix/PipePosix.h 
b/lldb/include/lldb/Host/posix/PipePosix.h
index ec4c752a24e94..2e291160817c4 100644
--- a/lldb/include/lldb/Host/posix/PipePosix.h
+++ b/lldb/include/lldb/Host/posix/PipePosix.h
@@ -64,7 +64,9 @@ class PipePosix : public PipeBase {
 
   Status Delete(llvm::StringRef name) override;
 
-  Status Write(const void *buf, size_t size, size_t &bytes_written) override;
+  Status WriteWithTimeout(const void *buf, size_t size,
+  const std::chrono::microseconds &timeout,
+  size_t &bytes_written) override;
   Status ReadWithTimeout(void *buf, size_t size,
  const std::chrono::microseconds &timeout,
  size_t &bytes_read) override;
diff --git a/lldb/include/lldb/Host/windows/PipeWindows.h 
b/lldb/include/lldb/Host/windows/PipeWindows.h
index 4b5be28d7ae6c..e28d104cc60ec 100644
--- a/lldb/include/lldb/Host/windows/PipeWindows.h
+++ b/lldb/include/lldb/Host/windows/PipeWindows.h
@@ -32,7 +32,6 @@ class PipeWindows : public PipeBase {
   Status CreateNew(bool child_process_inherit) override;
 
   // Create a named pipe.
-  Status CreateNewNamed(bool child_process_inherit);
   Status CreateNew(llvm::StringRef name, bool child_process_inherit) override;
   Status CreateWithUniqueName(llvm::StringRef prefix,
   bool child_process_inherit,
@@ -60,7 +59,9 @@ class PipeWindows : public PipeBase {
 
   Status Delete(llvm::StringRef name) override;
 
-  Status Write(const void *buf, size_t size, size_t &bytes_written) override;
+  Status WriteWithTimeout(const void *buf, size_t size,
+  const std::chrono::microseconds &timeout,
+  size_t &bytes_written) override;
   Status ReadWithTimeout(void *buf, size_t size,
  const std::chrono::microseconds &timeout,
  size_t &bytes_read) override;
diff --git a/lldb/source/Host/common/PipeBase.cpp 
b/lldb/source/Host/common/PipeBase.cpp
index b3e0ab34a58df..904a2df12392d 100644
--- a/lldb/source/Host/common/PipeBase.cpp
+++ b/lldb/source/Host/common/PipeBase.cpp
@@ -18,6 +18,11 @@ Status PipeBase::OpenAsWriter(llvm::StringRef name,
  std::chrono::microseconds::zero());
 }
 
+Status PipeBase::Write(const void *buf, size_t size, size_t &bytes_written) {
+  return WriteWithTimeout(buf, size, std::chrono::microseconds::zero(),
+  bytes_written);
+}
+
 Status PipeBase::Read(void *buf, size_t size, size_t &bytes_read) {
   return ReadWithTimeout(buf, size, std::chrono::microseconds::zero(),
  bytes_read);
diff --git a/lldb/source/Host/posix/PipePosix.cpp 
b/lldb/source/Host/posix/PipePosix.cpp
index f35c348990df6..00c6242f3f2e8 100644
--- a/lldb/source/Host/posix/PipePosix.cpp
+++ b/lldb/source/Host/posix/PipePosix.cpp
@@ -335,7 +335,9 @@ Status PipePosix::ReadWithTimeout(void *buf, size_t size,
   return error;
 }
 
-Status PipePosix::Write(const void *buf, size_t size, size_t &bytes_written) {
+Status PipePosix::WriteWithTimeout(const void *buf, size_t size,
+   const std::chrono::microseconds &timeout,
+   size_t &bytes_written) {

[Lldb-commits] [lldb] [lldb] Added Pipe::WriteWithTimeout() (PR #101383)


slydiman wrote:

@labath 
Note I have updated PipeTest.OpenAsReader. The behavior of the creating named 
pipe is different on Windows and Posix.
PipePosix calls mkfifo() and does not open the pipe. But PipeWindows really 
creates the pipe, which is already opened for read and write. Currently lldb is 
not using named pipes on Windows. But it must be fixed and the behavior must be 
the same.

https://github.com/llvm/llvm-project/pull/101383
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test] Support remote run of Shell tests (PR #95986)

2024-08-01 Thread Vladislav Dzhidzhoev via lldb-commits



@@ -4,16 +4,16 @@
 
 target create -l "ls" /bin/ls
 target list
-# CHECK: * target #0 (ls): /bin/ls
+# CHECK: * target #0 (ls): [[LS_PATH:.*]]

dzhidzhoev wrote:

Sorry, I was wrong here.

`target create` command accepts remote path. But `target list` prints full path 
to host's cached copy of the executable:
```
(lldb) target list
Current targets:
* target #0: 
C:\Users\vdzhidzhoev\.lldb\module_cache\remote-linux\.cache\16852127-4D24-D03C-BAB1-365C28DC2007-8BF46835\ls
 ( arch=aarch64-*-linux, platform=remote-linux )
```

So we can't just place 'ls' in the pattern matching line.

https://github.com/llvm/llvm-project/pull/95986
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][TypeSystemClang] Create VLAs of explicitly 0-size as ConstantArrayType (PR #100710)

2024-08-01 Thread Michael Buch via lldb-commits


Michael137 wrote:

> Yeah, won't work with link.exe, so I've just disabled it. Nothing more for 
> you to do here.

Awesome, thanks for the fix!

Out of curiosity, what's stopping us from using `lld` on Windows?

https://github.com/llvm/llvm-project/pull/100710
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test] Support remote run of Shell tests (PR #95986)



@@ -244,6 +244,7 @@ llvm_canonicalize_cmake_booleans(
   LLVM_ENABLE_ZLIB
   LLVM_ENABLE_SHARED_LIBS
   LLDB_HAS_LIBCXX
+  LLDB_SHELL_TESTS_DISABLE_REMOTE

DavidSpickett wrote:

It would be good to declare this and the other new options in CMake with 
docstrings so that they show up in tools like ccmake and just generally have 
another way to be discovered. `test/API/CMakeLists.txt` does this for example:
```
set(LLDB_TEST_COMPILER "${LLDB_DEFAULT_TEST_COMPILER}" CACHE PATH "C Compiler 
to use for building LLDB test inferiors")
```

https://github.com/llvm/llvm-project/pull/95986
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test] Support remote run of Shell tests (PR #95986)


https://github.com/DavidSpickett edited 
https://github.com/llvm/llvm-project/pull/95986
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Added Pipe::WriteWithTimeout() (PR #101383)


https://github.com/slydiman updated 
https://github.com/llvm/llvm-project/pull/101383

>From f88510ee4ae348b62550d785d3efaa4a4a7ee050 Mon Sep 17 00:00:00 2001
From: Dmitry Vasilyev 
Date: Wed, 31 Jul 2024 22:02:53 +0400
Subject: [PATCH] [lldb] Added Pipe::WriteWithTimeout()

Fixed few bugs in PipeWindows. Added the test for async read/write.
---
 lldb/include/lldb/Host/PipeBase.h|   5 +-
 lldb/include/lldb/Host/posix/PipePosix.h |   4 +-
 lldb/include/lldb/Host/windows/PipeWindows.h |   5 +-
 lldb/source/Host/common/PipeBase.cpp |   5 +
 lldb/source/Host/posix/PipePosix.cpp |   6 +-
 lldb/source/Host/windows/PipeWindows.cpp | 128 ---
 lldb/unittests/Host/PipeTest.cpp |  66 +-
 7 files changed, 165 insertions(+), 54 deletions(-)

diff --git a/lldb/include/lldb/Host/PipeBase.h 
b/lldb/include/lldb/Host/PipeBase.h
index 48c19b899cef6..d51d0cd54e036 100644
--- a/lldb/include/lldb/Host/PipeBase.h
+++ b/lldb/include/lldb/Host/PipeBase.h
@@ -56,7 +56,10 @@ class PipeBase {
   // Delete named pipe.
   virtual Status Delete(llvm::StringRef name) = 0;
 
-  virtual Status Write(const void *buf, size_t size, size_t &bytes_written) = 
0;
+  virtual Status WriteWithTimeout(const void *buf, size_t size,
+  const std::chrono::microseconds &timeout,
+  size_t &bytes_written) = 0;
+  Status Write(const void *buf, size_t size, size_t &bytes_written);
   virtual Status ReadWithTimeout(void *buf, size_t size,
  const std::chrono::microseconds &timeout,
  size_t &bytes_read) = 0;
diff --git a/lldb/include/lldb/Host/posix/PipePosix.h 
b/lldb/include/lldb/Host/posix/PipePosix.h
index ec4c752a24e94..2e291160817c4 100644
--- a/lldb/include/lldb/Host/posix/PipePosix.h
+++ b/lldb/include/lldb/Host/posix/PipePosix.h
@@ -64,7 +64,9 @@ class PipePosix : public PipeBase {
 
   Status Delete(llvm::StringRef name) override;
 
-  Status Write(const void *buf, size_t size, size_t &bytes_written) override;
+  Status WriteWithTimeout(const void *buf, size_t size,
+  const std::chrono::microseconds &timeout,
+  size_t &bytes_written) override;
   Status ReadWithTimeout(void *buf, size_t size,
  const std::chrono::microseconds &timeout,
  size_t &bytes_read) override;
diff --git a/lldb/include/lldb/Host/windows/PipeWindows.h 
b/lldb/include/lldb/Host/windows/PipeWindows.h
index 4b5be28d7ae6c..e28d104cc60ec 100644
--- a/lldb/include/lldb/Host/windows/PipeWindows.h
+++ b/lldb/include/lldb/Host/windows/PipeWindows.h
@@ -32,7 +32,6 @@ class PipeWindows : public PipeBase {
   Status CreateNew(bool child_process_inherit) override;
 
   // Create a named pipe.
-  Status CreateNewNamed(bool child_process_inherit);
   Status CreateNew(llvm::StringRef name, bool child_process_inherit) override;
   Status CreateWithUniqueName(llvm::StringRef prefix,
   bool child_process_inherit,
@@ -60,7 +59,9 @@ class PipeWindows : public PipeBase {
 
   Status Delete(llvm::StringRef name) override;
 
-  Status Write(const void *buf, size_t size, size_t &bytes_written) override;
+  Status WriteWithTimeout(const void *buf, size_t size,
+  const std::chrono::microseconds &timeout,
+  size_t &bytes_written) override;
   Status ReadWithTimeout(void *buf, size_t size,
  const std::chrono::microseconds &timeout,
  size_t &bytes_read) override;
diff --git a/lldb/source/Host/common/PipeBase.cpp 
b/lldb/source/Host/common/PipeBase.cpp
index b3e0ab34a58df..904a2df12392d 100644
--- a/lldb/source/Host/common/PipeBase.cpp
+++ b/lldb/source/Host/common/PipeBase.cpp
@@ -18,6 +18,11 @@ Status PipeBase::OpenAsWriter(llvm::StringRef name,
  std::chrono::microseconds::zero());
 }
 
+Status PipeBase::Write(const void *buf, size_t size, size_t &bytes_written) {
+  return WriteWithTimeout(buf, size, std::chrono::microseconds::zero(),
+  bytes_written);
+}
+
 Status PipeBase::Read(void *buf, size_t size, size_t &bytes_read) {
   return ReadWithTimeout(buf, size, std::chrono::microseconds::zero(),
  bytes_read);
diff --git a/lldb/source/Host/posix/PipePosix.cpp 
b/lldb/source/Host/posix/PipePosix.cpp
index f35c348990df6..00c6242f3f2e8 100644
--- a/lldb/source/Host/posix/PipePosix.cpp
+++ b/lldb/source/Host/posix/PipePosix.cpp
@@ -335,7 +335,9 @@ Status PipePosix::ReadWithTimeout(void *buf, size_t size,
   return error;
 }
 
-Status PipePosix::Write(const void *buf, size_t size, size_t &bytes_written) {
+Status PipePosix::WriteWithTimeout(const void *buf, size_t size,
+   const std::chrono::microseconds &timeout,
+   size_t &bytes_written) {

[Lldb-commits] [lldb] [lldb][test] Support remote run of Shell tests (PR #95986)


DavidSpickett wrote:

Looks good to me. @labath should give this another look as the biggest skeptic 
here :)

And @JDevlieghere who was also tagged earlier (start with the RFC though 
https://discourse.llvm.org/t/rfc-lldb-support-remote-run-of-shell-tests/80072).

https://github.com/llvm/llvm-project/pull/95986
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][TypeSystemClang] Create VLAs of explicitly 0-size as ConstantArrayType (PR #100710)


DavidSpickett wrote:

I don't know, it never occurred to me to do so before now. @omjavaid mentioned 
it so maybe he knows something about it.

https://github.com/llvm/llvm-project/pull/100710
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [lldb] [HLSL] Implement intangible AST type (PR #97362)

2024-08-01 Thread Damyan Pepper via lldb-commits


https://github.com/damyanp approved this pull request.


https://github.com/llvm/llvm-project/pull/97362
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)

2024-08-01 Thread Jacob Lalonde via lldb-commits


https://github.com/Jlalond updated 
https://github.com/llvm/llvm-project/pull/101272

>From 78ab13e3da15832117a7e2f8f85090a63482ca41 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde 
Date: Tue, 30 Jul 2024 11:04:45 -0700
Subject: [PATCH 1/8] Squash 64b-memory-regions-minidump and take only
 llvm-changes

---
 llvm/include/llvm/BinaryFormat/Minidump.h   | 12 
 llvm/include/llvm/Object/Minidump.h | 16 --
 llvm/include/llvm/ObjectYAML/MinidumpYAML.h | 28 +
 llvm/lib/Object/Minidump.cpp| 19 ++--
 llvm/lib/ObjectYAML/MinidumpEmitter.cpp | 11 +++
 llvm/lib/ObjectYAML/MinidumpYAML.cpp| 34 +
 6 files changed, 115 insertions(+), 5 deletions(-)

diff --git a/llvm/include/llvm/BinaryFormat/Minidump.h 
b/llvm/include/llvm/BinaryFormat/Minidump.h
index 9669303252615..8054e81322a92 100644
--- a/llvm/include/llvm/BinaryFormat/Minidump.h
+++ b/llvm/include/llvm/BinaryFormat/Minidump.h
@@ -74,6 +74,18 @@ struct MemoryDescriptor_64 {
   support::ulittle64_t StartOfMemoryRange;
   support::ulittle64_t DataSize;
 };
+static_assert(sizeof(MemoryDescriptor_64) == 16);
+
+struct MemoryListHeader {
+  support::ulittle32_t NumberOfMemoryRanges;
+};
+static_assert(sizeof(MemoryListHeader) == 4);
+
+struct Memory64ListHeader {
+  support::ulittle64_t NumberOfMemoryRanges;
+  support::ulittle64_t BaseRVA;
+};
+static_assert(sizeof(Memory64ListHeader) == 16);
 
 struct MemoryInfoListHeader {
   support::ulittle32_t SizeOfHeader;
diff --git a/llvm/include/llvm/Object/Minidump.h 
b/llvm/include/llvm/Object/Minidump.h
index e45d4de0090de..6ca9eb2afe150 100644
--- a/llvm/include/llvm/Object/Minidump.h
+++ b/llvm/include/llvm/Object/Minidump.h
@@ -103,6 +103,15 @@ class MinidumpFile : public Binary {
 minidump::StreamType::MemoryList);
   }
 
+  /// Returns the header to the memory 64 list stream. An error is returned if
+  /// the file does not contain this stream.
+  Expected getMemoryList64Header() const {
+return getStream(
+minidump::StreamType::Memory64List);
+  }
+
+  Expected> getMemory64List() const;
+
   class MemoryInfoIterator
   : public iterator_facade_base> getDataSlice(ArrayRef Data,
-  size_t Offset, size_t Size);
+  static Expected>
+  getDataSlice(ArrayRef Data, uint64_t Offset, uint64_t Size);
 
   /// Return the slice of the given data array as an array of objects of the
   /// given type. The function checks that the input array is large enough to
   /// contain the correct number of objects of the given type.
   template 
   static Expected> getDataSliceAs(ArrayRef Data,
-  size_t Offset, size_t Count);
+  uint64_t Offset, uint64_t Count);
 
   MinidumpFile(MemoryBufferRef Source, const minidump::Header &Header,
ArrayRef Streams,
@@ -208,6 +217,7 @@ Expected> 
MinidumpFile::getDataSliceAs(ArrayRef Data,
   getDataSlice(Data, Offset, sizeof(T) * Count);
   if (!Slice)
 return Slice.takeError();
+
   return ArrayRef(reinterpret_cast(Slice->data()), Count);
 }
 
diff --git a/llvm/include/llvm/ObjectYAML/MinidumpYAML.h 
b/llvm/include/llvm/ObjectYAML/MinidumpYAML.h
index b0cee541cef20..2cccf7fba5853 100644
--- a/llvm/include/llvm/ObjectYAML/MinidumpYAML.h
+++ b/llvm/include/llvm/ObjectYAML/MinidumpYAML.h
@@ -29,6 +29,7 @@ struct Stream {
 Exception,
 MemoryInfoList,
 MemoryList,
+Memory64List,
 ModuleList,
 RawContent,
 SystemInfo,
@@ -104,6 +105,25 @@ using ModuleListStream = 
detail::ListStream;
 using ThreadListStream = detail::ListStream;
 using MemoryListStream = detail::ListStream;
 
+/// Memory64ListStream minidump stream.
+struct Memory64ListStream : public Stream {
+  minidump::Memory64ListHeader Header;
+  std::vector Entries;
+  yaml::BinaryRef Content;
+
+  Memory64ListStream()
+  : Stream(StreamKind::Memory64List, minidump::StreamType::Memory64List) {}
+
+  explicit Memory64ListStream(
+  std::vector Entries)
+  : Stream(StreamKind::Memory64List, minidump::StreamType::Memory64List),
+Entries(Entries) {}
+
+  static bool classof(const Stream *S) {
+return S->Kind == StreamKind::Memory64List;
+  }
+};
+
 /// ExceptionStream minidump stream.
 struct ExceptionStream : public Stream {
   minidump::ExceptionStream MDExceptionStream;
@@ -244,6 +264,12 @@ template <> struct 
MappingContextTraits {
   BinaryRef &Content);
 };
 
+template <>
+struct MappingContextTraits {
+  static void mapping(IO &IO, minidump::MemoryDescriptor_64 &Memory,
+  BinaryRef &Content);
+};
+
 } // namespace yaml
 
 } // namespace llvm
@@ -262,6 +288,7 @@ 
LLVM_YAML_DECLARE_MAPPING_TRAITS(llvm::minidump::CPUInfo::X86Info)
 LLVM_YAML_DECLARE_MAPPING_TRAITS(llvm::minidump::Exception)
 LLVM_YAML_DECLARE_MAPPING_TRAITS(llvm::minidump::MemoryInfo)
 LLVM_YAML_DECLARE_MAPPING_TRAIT

[Lldb-commits] [lldb] [lldb] Added Pipe::WriteWithTimeout() (PR #101383)


https://github.com/slydiman updated 
https://github.com/llvm/llvm-project/pull/101383

>From 9c5005d9f28bf1419250a3b3d0d2f5e2ab34b58d Mon Sep 17 00:00:00 2001
From: Dmitry Vasilyev 
Date: Wed, 31 Jul 2024 22:02:53 +0400
Subject: [PATCH] [lldb] Added Pipe::WriteWithTimeout()

Fixed few bugs in PipeWindows. Added the test for async read/write.
---
 lldb/include/lldb/Host/PipeBase.h|   5 +-
 lldb/include/lldb/Host/posix/PipePosix.h |   4 +-
 lldb/include/lldb/Host/windows/PipeWindows.h |   5 +-
 lldb/source/Host/common/PipeBase.cpp |   5 +
 lldb/source/Host/posix/PipePosix.cpp |   6 +-
 lldb/source/Host/windows/PipeWindows.cpp | 128 ---
 lldb/unittests/Host/PipeTest.cpp |  70 +-
 7 files changed, 169 insertions(+), 54 deletions(-)

diff --git a/lldb/include/lldb/Host/PipeBase.h 
b/lldb/include/lldb/Host/PipeBase.h
index 48c19b899cef6..d51d0cd54e036 100644
--- a/lldb/include/lldb/Host/PipeBase.h
+++ b/lldb/include/lldb/Host/PipeBase.h
@@ -56,7 +56,10 @@ class PipeBase {
   // Delete named pipe.
   virtual Status Delete(llvm::StringRef name) = 0;
 
-  virtual Status Write(const void *buf, size_t size, size_t &bytes_written) = 
0;
+  virtual Status WriteWithTimeout(const void *buf, size_t size,
+  const std::chrono::microseconds &timeout,
+  size_t &bytes_written) = 0;
+  Status Write(const void *buf, size_t size, size_t &bytes_written);
   virtual Status ReadWithTimeout(void *buf, size_t size,
  const std::chrono::microseconds &timeout,
  size_t &bytes_read) = 0;
diff --git a/lldb/include/lldb/Host/posix/PipePosix.h 
b/lldb/include/lldb/Host/posix/PipePosix.h
index ec4c752a24e94..2e291160817c4 100644
--- a/lldb/include/lldb/Host/posix/PipePosix.h
+++ b/lldb/include/lldb/Host/posix/PipePosix.h
@@ -64,7 +64,9 @@ class PipePosix : public PipeBase {
 
   Status Delete(llvm::StringRef name) override;
 
-  Status Write(const void *buf, size_t size, size_t &bytes_written) override;
+  Status WriteWithTimeout(const void *buf, size_t size,
+  const std::chrono::microseconds &timeout,
+  size_t &bytes_written) override;
   Status ReadWithTimeout(void *buf, size_t size,
  const std::chrono::microseconds &timeout,
  size_t &bytes_read) override;
diff --git a/lldb/include/lldb/Host/windows/PipeWindows.h 
b/lldb/include/lldb/Host/windows/PipeWindows.h
index 4b5be28d7ae6c..e28d104cc60ec 100644
--- a/lldb/include/lldb/Host/windows/PipeWindows.h
+++ b/lldb/include/lldb/Host/windows/PipeWindows.h
@@ -32,7 +32,6 @@ class PipeWindows : public PipeBase {
   Status CreateNew(bool child_process_inherit) override;
 
   // Create a named pipe.
-  Status CreateNewNamed(bool child_process_inherit);
   Status CreateNew(llvm::StringRef name, bool child_process_inherit) override;
   Status CreateWithUniqueName(llvm::StringRef prefix,
   bool child_process_inherit,
@@ -60,7 +59,9 @@ class PipeWindows : public PipeBase {
 
   Status Delete(llvm::StringRef name) override;
 
-  Status Write(const void *buf, size_t size, size_t &bytes_written) override;
+  Status WriteWithTimeout(const void *buf, size_t size,
+  const std::chrono::microseconds &timeout,
+  size_t &bytes_written) override;
   Status ReadWithTimeout(void *buf, size_t size,
  const std::chrono::microseconds &timeout,
  size_t &bytes_read) override;
diff --git a/lldb/source/Host/common/PipeBase.cpp 
b/lldb/source/Host/common/PipeBase.cpp
index b3e0ab34a58df..904a2df12392d 100644
--- a/lldb/source/Host/common/PipeBase.cpp
+++ b/lldb/source/Host/common/PipeBase.cpp
@@ -18,6 +18,11 @@ Status PipeBase::OpenAsWriter(llvm::StringRef name,
  std::chrono::microseconds::zero());
 }
 
+Status PipeBase::Write(const void *buf, size_t size, size_t &bytes_written) {
+  return WriteWithTimeout(buf, size, std::chrono::microseconds::zero(),
+  bytes_written);
+}
+
 Status PipeBase::Read(void *buf, size_t size, size_t &bytes_read) {
   return ReadWithTimeout(buf, size, std::chrono::microseconds::zero(),
  bytes_read);
diff --git a/lldb/source/Host/posix/PipePosix.cpp 
b/lldb/source/Host/posix/PipePosix.cpp
index f35c348990df6..00c6242f3f2e8 100644
--- a/lldb/source/Host/posix/PipePosix.cpp
+++ b/lldb/source/Host/posix/PipePosix.cpp
@@ -335,7 +335,9 @@ Status PipePosix::ReadWithTimeout(void *buf, size_t size,
   return error;
 }
 
-Status PipePosix::Write(const void *buf, size_t size, size_t &bytes_written) {
+Status PipePosix::WriteWithTimeout(const void *buf, size_t size,
+   const std::chrono::microseconds &timeout,
+   size_t &bytes_written) {

[Lldb-commits] [lldb] [lldb][test][x86_64][win] Split assertion in TestBreakpointConditions (PR #100487)

2024-08-01 Thread Kendal Harland via lldb-commits


kendalharland wrote:

Hi @JDevlieghere, friendly reminder to PTAL when you can.

https://github.com/llvm/llvm-project/pull/100487
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)

2024-08-01 Thread Jacob Lalonde via lldb-commits


https://github.com/Jlalond edited 
https://github.com/llvm/llvm-project/pull/101272
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB] Add a StackFrameRecognizer for the Darwin specific abort_with_payload… (PR #101365)

2024-08-01 Thread Jonas Devlieghere via lldb-commits



@@ -2570,6 +2570,18 @@ void PruneThreadPlans();
   /// information related to the process.
   virtual StructuredData::DictionarySP GetMetadata() { return nullptr; }
 
+  /// Fetch extended crash information held by the process.  This will never be
+  /// an empty shared pointer, it will always have a dict, though it may be
+  /// empty.
+  StructuredData::DictionarySP GetExtendedCrashInfoDict() {

JDevlieghere wrote:

Why not add an `assert(m_crash_info_dict_sp && "DictionarySP must be valid")` 
to enforce that precondition? 

https://github.com/llvm/llvm-project/pull/101365
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB] Add a StackFrameRecognizer for the Darwin specific abort_with_payload… (PR #101365)

2024-08-01 Thread Jonas Devlieghere via lldb-commits


https://github.com/JDevlieghere edited 
https://github.com/llvm/llvm-project/pull/101365
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB] Add a StackFrameRecognizer for the Darwin specific abort_with_payload… (PR #101365)

2024-08-01 Thread Jonas Devlieghere via lldb-commits


https://github.com/JDevlieghere approved this pull request.

LGTM if @medismailben is happy!

https://github.com/llvm/llvm-project/pull/101365
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB] Impove ObjectFileELF's .dynamic parsing and usage. (PR #101237)



@@ -3664,7 +3730,27 @@ llvm::ArrayRef 
ObjectFileELF::ProgramHeaders() {
 }
 
 DataExtractor ObjectFileELF::GetSegmentData(const ELFProgramHeader &H) {
-  return DataExtractor(m_data, H.p_offset, H.p_filesz);
+  // Try and read the program header from our cached m_data which can come from
+  // the file on disk being mmap'ed or from the initial part of the ELF file we
+  // read from memory and cached.
+  DataExtractor data = DataExtractor(m_data, H.p_offset, H.p_filesz);
+  if (data.GetByteSize() == H.p_filesz)
+return data;
+  if (IsInMemory()) {
+// We have a ELF file in process memory, read the program header data from
+// the process.
+ProcessSP process_sp(m_process_wp.lock());
+if (process_sp) {
+  const lldb::offset_t base_file_addr = GetBaseAddress().GetFileAddress();
+  // const addr_t data_addr = m_memory_addr + H.p_offset; // Not correct 
for

clayborg wrote:

will do

https://github.com/llvm/llvm-project/pull/101237
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB] Impove ObjectFileELF's .dynamic parsing and usage. (PR #101237)



@@ -3704,3 +3790,83 @@ ObjectFileELF::MapFileDataWritable(const FileSpec &file, 
uint64_t Size,
   return FileSystem::Instance().CreateWritableDataBuffer(file.GetPath(), Size,
  Offset);
 }
+
+std::optional ObjectFileELF::GetDynstrData() {
+
+  SectionList *section_list = GetSectionList();
+  if (section_list) {
+// Find the SHT_DYNAMIC section.
+Section *dynamic =
+section_list->FindSectionByType(eSectionTypeELFDynamicLinkInfo, true)
+.get();
+if (dynamic) {
+  assert(dynamic->GetObjectFile() == this);
+  const ELFSectionHeaderInfo *header =
+  GetSectionHeaderByIndex(dynamic->GetID());
+  if (header) {
+// sh_link: section header index of string table used by entries in
+// the section.
+Section *dynstr = section_list->FindSectionByID(header->sh_link).get();
+DataExtractor data;
+if (dynstr && ReadSectionData(dynstr, data))
+  return data;
+  }
+}
+  }
+
+  // Every ELF file which represents an executable or shared library has
+  // mandatory .dynamic entries. Two of these values are DT_STRTAB and DT_STRSZ
+  // and represent the dynamic symbol tables's string table. These are needed
+  // by the dynamic loader and we can read them from a process' address space.
+  //
+  // When loading and ELF file from memory, only the program headers end up
+  // being mapped into memory, and we can find these values in the PT_DYNAMIC
+  // segment.
+  if (!IsInMemory())
+return std::nullopt;
+  ProcessSP process_sp(m_process_wp.lock());
+  if (!process_sp)
+return std::nullopt;
+
+  const ELFDynamic *strtab = FindDynamicSymbol(DT_STRTAB);
+  const ELFDynamic *strsz = FindDynamicSymbol(DT_STRSZ);
+  if (strtab == nullptr || strsz == nullptr)
+return std::nullopt;
+
+  DataBufferSP data_sp = ReadMemory(process_sp, strtab->d_ptr, strsz->d_val);

clayborg wrote:

The code above this checks that the ELF file was in memory, so this was only 
designed to work for in memory ELF files. But as you say we might be able to 
make this work for on disk files that have no section headers as well. Can we 
do this in a separate patch? I would like to make forward progress on ELF core 
files without making up new work.

https://github.com/llvm/llvm-project/pull/101237
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB] Impove ObjectFileELF's .dynamic parsing and usage. (PR #101237)



@@ -0,0 +1,58 @@
+// REQUIRES: system-linux, native

clayborg wrote:

Adding support for memory ELF files is my primary concern with this patch as 
you know. Testing that in memory ELF files work as well as they can and that we 
can find the DT_DEBUG is the primary goal of this patch, so I would still like 
to concentrate on that. I have yet to run into an ELF file in the wild that 
doesn't have section headers, though I know they are possible. Testing I can 
load a ELF file from disk with no section headers is a whole other code path 
that introduces changes and time to this patch which I would rather not take on 
at this point if possible. Getting core files to be able to have a shared 
library list without having the main executable makes ELF core file actually 
useful now instead of being useless unless you get the correct executable. So 
although testing an on disk ELF file with no section headers is nice for 
testing, it can't replace the test I have written because we need this to test 
the in memory ELF files and testing the file itself is whole other code path 
that isn't needed.

I will be happy to add support for on disk ELF files after this patch, but I 
would like to get the useful part of this patch checked in so we can get ELF 
core files to be useful with no executable.

https://github.com/llvm/llvm-project/pull/101237
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB] Impove ObjectFileELF's .dynamic parsing and usage. (PR #101237)



@@ -384,6 +392,9 @@ class ObjectFileELF : public lldb_private::ObjectFile {
   /// ELF dependent module dump routine.
   void DumpDependentModules(lldb_private::Stream *s);
 
+  /// ELF dump the .dynamic section
+  void DumpELFDynamic(lldb_private::Stream *s);

clayborg wrote:

All other dump calls use a "Stream *" so I mirrored those. I can make a 
separate patch to switch these over to "Stream &". Would that be ok? I didn't 
want to make the patch bigger

https://github.com/llvm/llvm-project/pull/101237
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB] Impove ObjectFileELF's .dynamic parsing and usage. (PR #101237)


https://github.com/clayborg updated 
https://github.com/llvm/llvm-project/pull/101237

>From f0cd3ef613b2da145b14a3d79d6810cc19e9b198 Mon Sep 17 00:00:00 2001
From: Greg Clayton 
Date: Tue, 30 Jul 2024 13:37:44 -0700
Subject: [PATCH 1/3] Impove ObjectFileELF's .dynamic parsing and usage.

This patch improves the ability of a ObjectFileELF instance to read the 
.dynamic section. It adds the ability to read the .dynamic section from a ELF 
file that is read from memory, cleans up the usage of the .dynamic entries so 
that ObjectFileELF::ParseDynamicSymbols() is the only code that parses .dynamic 
entries, teaches that function the read and store the string values for each 
.dynamic entry, and dumps the .dynamic entries in the output of "image dump 
objfile". It also cleans up the code that gets the dynamic string table and the 
.dynamic data from the ELF file.
---
 lldb/include/lldb/Target/Process.h|   2 +-
 .../Plugins/ObjectFile/ELF/ObjectFileELF.cpp  | 356 +-
 .../Plugins/ObjectFile/ELF/ObjectFileELF.h|  38 +-
 lldb/source/Target/Process.cpp|  15 +
 .../ObjectFile/ELF/Inputs/memory-elf.cpp  |   6 +
 .../test/Shell/ObjectFile/ELF/elf-memory.test |  58 +++
 6 files changed, 378 insertions(+), 97 deletions(-)
 create mode 100644 lldb/test/Shell/ObjectFile/ELF/Inputs/memory-elf.cpp
 create mode 100644 lldb/test/Shell/ObjectFile/ELF/elf-memory.test

diff --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index c8475db8ae1609..17e18261b4752d 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -1952,7 +1952,7 @@ class Process : public 
std::enable_shared_from_this,
 
   lldb::ModuleSP ReadModuleFromMemory(const FileSpec &file_spec,
   lldb::addr_t header_addr,
-  size_t size_to_read = 512);
+  size_t size_to_read = 0);
 
   /// Attempt to get the attributes for a region of memory in the process.
   ///
diff --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp 
b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
index 890db5c2748146..becee73f962058 100644
--- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -873,33 +873,29 @@ Address ObjectFileELF::GetImageInfoAddress(Target 
*target) {
   if (!section_list)
 return Address();
 
-  // Find the SHT_DYNAMIC (.dynamic) section.
-  SectionSP dynsym_section_sp(
-  section_list->FindSectionByType(eSectionTypeELFDynamicLinkInfo, true));
-  if (!dynsym_section_sp)
-return Address();
-  assert(dynsym_section_sp->GetObjectFile() == this);
-
-  user_id_t dynsym_id = dynsym_section_sp->GetID();
-  const ELFSectionHeaderInfo *dynsym_hdr = GetSectionHeaderByIndex(dynsym_id);
-  if (!dynsym_hdr)
-return Address();
-
   for (size_t i = 0; i < m_dynamic_symbols.size(); ++i) {
-ELFDynamic &symbol = m_dynamic_symbols[i];
+const ELFDynamic &symbol = m_dynamic_symbols[i].symbol;
 
 if (symbol.d_tag == DT_DEBUG) {
   // Compute the offset as the number of previous entries plus the size of
   // d_tag.
-  addr_t offset = i * dynsym_hdr->sh_entsize + GetAddressByteSize();
-  return Address(dynsym_section_sp, offset);
+  addr_t offset = (i * 2 + 1) * GetAddressByteSize();
+  addr_t file_addr = m_dynamic_base_addr + offset;
+  Address addr;
+  if (addr.ResolveAddressUsingFileSections(file_addr, GetSectionList()))
+return addr;
 }
 // MIPS executables uses DT_MIPS_RLD_MAP_REL to support PIE. 
DT_MIPS_RLD_MAP
 // exists in non-PIE.
 else if ((symbol.d_tag == DT_MIPS_RLD_MAP ||
   symbol.d_tag == DT_MIPS_RLD_MAP_REL) &&
  target) {
-  addr_t offset = i * dynsym_hdr->sh_entsize + GetAddressByteSize();
+  SectionSP dynsym_section_sp(section_list->FindSectionByType(
+  eSectionTypeELFDynamicLinkInfo, true));
+  if (!dynsym_section_sp)
+return Address();
+
+  addr_t offset = (i * 2 + 1) * GetAddressByteSize();
   addr_t dyn_base = dynsym_section_sp->GetLoadBaseAddress(target);
   if (dyn_base == LLDB_INVALID_ADDRESS)
 return Address();
@@ -970,62 +966,23 @@ Address ObjectFileELF::GetBaseAddress() {
   return LLDB_INVALID_ADDRESS;
 }
 
-// ParseDependentModules
 size_t ObjectFileELF::ParseDependentModules() {
   if (m_filespec_up)
 return m_filespec_up->GetSize();
 
   m_filespec_up = std::make_unique();
 
-  if (!ParseSectionHeaders())
-return 0;
-
-  SectionList *section_list = GetSectionList();
-  if (!section_list)
-return 0;
-
-  // Find the SHT_DYNAMIC section.
-  Section *dynsym =
-  section_list->FindSectionByType(eSectionTypeELFDynamicLinkInfo, true)
-  .get();
-  if (!dynsym)
-return 0;
-  assert(dynsym->GetObjectFile() == this);
-
-  const ELFSectionHeaderInfo *header = 
GetSectionHeaderByIndex(dynsym

[Lldb-commits] [lldb] [LLDB] Add a StackFrameRecognizer for the Darwin specific abort_with_payload… (PR #101365)

2024-08-01 Thread Med Ismail Bennani via lldb-commits


https://github.com/medismailben approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/101365
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test][x86_64][win] Split assertion in TestBreakpointConditions (PR #100487)

2024-08-01 Thread Vladislav Dzhidzhoev via lldb-commits


https://github.com/dzhidzhoev approved this pull request.


https://github.com/llvm/llvm-project/pull/100487
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Change Module to have a concrete UnwindTable, update (PR #101130)

2024-08-01 Thread Jason Molenda via lldb-commits

https://github.com/jasonmolenda updated 
https://github.com/llvm/llvm-project/pull/101130

>From 55b1ac1fbad89ebc50038738c1e09045e9884be8 Mon Sep 17 00:00:00 2001
From: Jason Molenda 
Date: Mon, 29 Jul 2024 22:37:43 -0700
Subject: [PATCH 1/4] [lldb] Change Module to have a concrete UnwindTable,
 update

Currently a Module has a std::optional which is
created when the UnwindTable is requested from outside the
Module.  The idea is to delay its creation until the Module
has an ObjectFile initialized, which will have been done by
the time we're doing an unwind.

However, Module::GetUnwindTable wasn't doing any locking, so it was
possible for two threads to ask for the UnwindTable for the first
time, one would be created and returned while another thread would
create one, destroy the first in the process of emplacing it.
It was an uncommon crash, but it was possible.

Grabbing the Module's mutex would be one way to address it, but
when loading ELF binaries, we start creating the SymbolTable on one
thread (ObjectFileELF) grabbing the Module's mutex, and then spin
up worker threads to parse the individual DWARF compilation units,
which then try to also get the UnwindTable and deadlock if they try
to get the Module's mutex.

This changes Module to have a concrete UnwindTable as an ivar, and
when it adds an ObjectFile or SymbolFileVendor, it will call the
Update method on it, which will re-evaluate which sections exist
in the ObjectFile/SymbolFile. UnwindTable used to have an Initialize
method which set all the sections, and an Update method which would
set some of them if they weren't set.  I unified these with the
Initialize method taking a `force` option to re-initialize the
section pointers even if they had been done already before.

This is addressing a rare crash report we've received, and also a
failure Adrian spotted on the -fsanitize=address CI bot last week,
it's still uncommon with ASAN but it can happen with the standard
testsuite.

rdar://132514736
---
 lldb/include/lldb/Core/Module.h|  6 +--
 lldb/include/lldb/Symbol/UnwindTable.h |  2 +-
 lldb/source/Core/Module.cpp| 26 +++--
 lldb/source/Symbol/UnwindTable.cpp | 51 ++
 4 files changed, 23 insertions(+), 62 deletions(-)

diff --git a/lldb/include/lldb/Core/Module.h b/lldb/include/lldb/Core/Module.h
index 0188057247a68..5589c1c9a350d 100644
--- a/lldb/include/lldb/Core/Module.h
+++ b/lldb/include/lldb/Core/Module.h
@@ -1021,9 +1021,9 @@ class Module : public 
std::enable_shared_from_this,
   lldb::ObjectFileSP m_objfile_sp; ///< A shared pointer to the object file
/// parser for this module as it may or may
/// not be shared with the SymbolFile
-  std::optional m_unwind_table; ///< Table of FuncUnwinders
- /// objects created for this
- /// Module's functions
+  UnwindTable m_unwind_table;  ///< Table of FuncUnwinders
+   /// objects created for this
+   /// Module's functions
   lldb::SymbolVendorUP
   m_symfile_up; ///< A pointer to the symbol vendor for this module.
   std::vector
diff --git a/lldb/include/lldb/Symbol/UnwindTable.h 
b/lldb/include/lldb/Symbol/UnwindTable.h
index 26826e5d1b497..0e7d76b1b896c 100644
--- a/lldb/include/lldb/Symbol/UnwindTable.h
+++ b/lldb/include/lldb/Symbol/UnwindTable.h
@@ -64,7 +64,7 @@ class UnwindTable {
 private:
   void Dump(Stream &s);

-  void Initialize();
+  void Initialize(bool force = false);
   std::optional GetAddressRange(const Address &addr,
   const SymbolContext &sc);

diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index 9c105b3f0e57a..acc17ecad015b 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -131,7 +131,8 @@ Module *Module::GetAllocatedModuleAtIndex(size_t idx) {
 }

 Module::Module(const ModuleSpec &module_spec)
-: m_file_has_changed(false), m_first_file_changed_log(false) {
+: m_unwind_table(*this), m_file_has_changed(false),
+  m_first_file_changed_log(false) {
   // Scope for locker below...
   {
 std::lock_guard guard(
@@ -238,7 +239,8 @@ Module::Module(const FileSpec &file_spec, const ArchSpec 
&arch,
 : m_mod_time(FileSystem::Instance().GetModificationTime(file_spec)),
   m_arch(arch), m_file(file_spec), m_object_name(object_name),
   m_object_offset(object_offset), m_object_mod_time(object_mod_time),
-  m_file_has_changed(false), m_first_file_changed_log(false) {
+  m_unwind_table(*this), m_file_has_changed(false),
+  m_first_file_changed_log(false) {
   // Scope for locker below...
   {
 std::lock_guard guard(
@@ -254,7 +256,9 @@ Module::Module(const FileSpec &file_spec, const ArchSpec 
&arch,
   m_object_name.AsCString(""), m_object_name.IsEmpty()

[Lldb-commits] [lldb] [LLDB] Impove ObjectFileELF's .dynamic parsing and usage. (PR #101237)


https://github.com/clayborg updated 
https://github.com/llvm/llvm-project/pull/101237

>From f0cd3ef613b2da145b14a3d79d6810cc19e9b198 Mon Sep 17 00:00:00 2001
From: Greg Clayton 
Date: Tue, 30 Jul 2024 13:37:44 -0700
Subject: [PATCH 1/4] Impove ObjectFileELF's .dynamic parsing and usage.

This patch improves the ability of a ObjectFileELF instance to read the 
.dynamic section. It adds the ability to read the .dynamic section from a ELF 
file that is read from memory, cleans up the usage of the .dynamic entries so 
that ObjectFileELF::ParseDynamicSymbols() is the only code that parses .dynamic 
entries, teaches that function the read and store the string values for each 
.dynamic entry, and dumps the .dynamic entries in the output of "image dump 
objfile". It also cleans up the code that gets the dynamic string table and the 
.dynamic data from the ELF file.
---
 lldb/include/lldb/Target/Process.h|   2 +-
 .../Plugins/ObjectFile/ELF/ObjectFileELF.cpp  | 356 +-
 .../Plugins/ObjectFile/ELF/ObjectFileELF.h|  38 +-
 lldb/source/Target/Process.cpp|  15 +
 .../ObjectFile/ELF/Inputs/memory-elf.cpp  |   6 +
 .../test/Shell/ObjectFile/ELF/elf-memory.test |  58 +++
 6 files changed, 378 insertions(+), 97 deletions(-)
 create mode 100644 lldb/test/Shell/ObjectFile/ELF/Inputs/memory-elf.cpp
 create mode 100644 lldb/test/Shell/ObjectFile/ELF/elf-memory.test

diff --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index c8475db8ae160..17e18261b4752 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -1952,7 +1952,7 @@ class Process : public 
std::enable_shared_from_this,
 
   lldb::ModuleSP ReadModuleFromMemory(const FileSpec &file_spec,
   lldb::addr_t header_addr,
-  size_t size_to_read = 512);
+  size_t size_to_read = 0);
 
   /// Attempt to get the attributes for a region of memory in the process.
   ///
diff --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp 
b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
index 890db5c274814..becee73f96205 100644
--- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -873,33 +873,29 @@ Address ObjectFileELF::GetImageInfoAddress(Target 
*target) {
   if (!section_list)
 return Address();
 
-  // Find the SHT_DYNAMIC (.dynamic) section.
-  SectionSP dynsym_section_sp(
-  section_list->FindSectionByType(eSectionTypeELFDynamicLinkInfo, true));
-  if (!dynsym_section_sp)
-return Address();
-  assert(dynsym_section_sp->GetObjectFile() == this);
-
-  user_id_t dynsym_id = dynsym_section_sp->GetID();
-  const ELFSectionHeaderInfo *dynsym_hdr = GetSectionHeaderByIndex(dynsym_id);
-  if (!dynsym_hdr)
-return Address();
-
   for (size_t i = 0; i < m_dynamic_symbols.size(); ++i) {
-ELFDynamic &symbol = m_dynamic_symbols[i];
+const ELFDynamic &symbol = m_dynamic_symbols[i].symbol;
 
 if (symbol.d_tag == DT_DEBUG) {
   // Compute the offset as the number of previous entries plus the size of
   // d_tag.
-  addr_t offset = i * dynsym_hdr->sh_entsize + GetAddressByteSize();
-  return Address(dynsym_section_sp, offset);
+  addr_t offset = (i * 2 + 1) * GetAddressByteSize();
+  addr_t file_addr = m_dynamic_base_addr + offset;
+  Address addr;
+  if (addr.ResolveAddressUsingFileSections(file_addr, GetSectionList()))
+return addr;
 }
 // MIPS executables uses DT_MIPS_RLD_MAP_REL to support PIE. 
DT_MIPS_RLD_MAP
 // exists in non-PIE.
 else if ((symbol.d_tag == DT_MIPS_RLD_MAP ||
   symbol.d_tag == DT_MIPS_RLD_MAP_REL) &&
  target) {
-  addr_t offset = i * dynsym_hdr->sh_entsize + GetAddressByteSize();
+  SectionSP dynsym_section_sp(section_list->FindSectionByType(
+  eSectionTypeELFDynamicLinkInfo, true));
+  if (!dynsym_section_sp)
+return Address();
+
+  addr_t offset = (i * 2 + 1) * GetAddressByteSize();
   addr_t dyn_base = dynsym_section_sp->GetLoadBaseAddress(target);
   if (dyn_base == LLDB_INVALID_ADDRESS)
 return Address();
@@ -970,62 +966,23 @@ Address ObjectFileELF::GetBaseAddress() {
   return LLDB_INVALID_ADDRESS;
 }
 
-// ParseDependentModules
 size_t ObjectFileELF::ParseDependentModules() {
   if (m_filespec_up)
 return m_filespec_up->GetSize();
 
   m_filespec_up = std::make_unique();
 
-  if (!ParseSectionHeaders())
-return 0;
-
-  SectionList *section_list = GetSectionList();
-  if (!section_list)
-return 0;
-
-  // Find the SHT_DYNAMIC section.
-  Section *dynsym =
-  section_list->FindSectionByType(eSectionTypeELFDynamicLinkInfo, true)
-  .get();
-  if (!dynsym)
-return 0;
-  assert(dynsym->GetObjectFile() == this);
-
-  const ELFSectionHeaderInfo *header = 
GetSectionHeaderByIndex(dynsym->Ge

[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)


https://github.com/clayborg commented:

Code looks fine to me now. I will let Pavel give the final OK as I don't know 
much about the YAML layering in LLVM.

https://github.com/llvm/llvm-project/pull/101272
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)



@@ -10,6 +10,7 @@
 #define LLDB_API_SBSAVECOREOPTIONS_H
 
 #include "lldb/API/SBDefines.h"
+#include "lldb/API/SBThread.h"

clayborg wrote:

probably should include SBProcess.h here as well since we are using it.

https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)


https://github.com/clayborg requested changes to this pull request.


https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)



@@ -9,6 +9,8 @@
 #include "lldb/API/SBSaveCoreOptions.h"
 #include "lldb/API/SBError.h"
 #include "lldb/API/SBFileSpec.h"
+#include "lldb/API/SBProcess.h"
+#include "lldb/API/SBThread.h"

clayborg wrote:

remove these, they are in the `SBSaveCoreOptions.h` header file

https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)



@@ -46,8 +48,83 @@ SaveCoreOptions::GetOutputFile() const {
   return m_file;
 }
 
+Status SaveCoreOptions::SetProcess(lldb::ProcessSP process_sp) {
+  Status error;
+  if (!process_sp) {
+ClearProcessSpecificData();
+m_process_sp.reset();
+return error;
+  }
+
+  if (!process_sp->IsValid()) {
+error.SetErrorString("Cannot assign an invalid process.");
+return error;
+  }
+
+  if (m_process_sp == process_sp)
+return error;
+
+  ClearProcessSpecificData();
+  m_process_sp = process_sp;
+  return error;
+}
+
+Status SaveCoreOptions::AddThread(lldb::ThreadSP thread_sp) {
+  Status error;
+  if (!thread_sp) {
+error.SetErrorString("invalid thread");
+return error;
+  }
+
+  if (m_process_sp) {
+if (m_process_sp != thread_sp->GetProcess()) {
+  error.SetErrorString("Cannot add a thread from a different process.");
+  return error;
+}
+  } else {
+m_process_sp = thread_sp->GetProcess();
+  }
+
+  m_threads_to_save[thread_sp->GetID()];

clayborg wrote:

This might insert a default constructed ThreadSP. If we need the ThreadSP as 
the value then this should be:
```
m_threads_to_save[thread_sp->GetID()] = thread_sp;
```
But seeing as you must have a test for this, I am guessing we don't need the 
thread SP and `m_threads_to_save` can switch to being:
```
std::set m_tids_to_save;
```
Then the code above becomes:
```
m_threads_to_save.insert(thread_sp->GetID());
```

https://github.com/llvm/llvm-project/pull/100443
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB][SBSaveCore] Implement a selectable threadlist for Core Options. (PR #100443)