[Lldb-commits] [PATCH] D134585: [lldb][COFF] Map symbols without base+complex type as 'Data' type

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

This LGTM, but I'll leave it to some LLDB maintainer to formally approve.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134585

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

[Lldb-commits] [PATCH] D128504: debugserver: spawn process in its own process group

[Alessandro Arzilli via Phabricator via lldb-commits]

aarzilli added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128504

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

[Lldb-commits] [PATCH] D134037: [lldb] Fix CommandInterpreter::DidProcessStopAbnormally() with multiple threads

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added inline comments.



Comment at: lldb/test/Shell/Driver/CommandOnCrashMultiThreaded.test:1
+# XFAIL: system-windows
+# REQUIRES: native && (target-x86 || target-x86_64)

DavidSpickett wrote:
> mstorsjo wrote:
> > DavidSpickett wrote:
> > > Isn't this fixing an issue that you saw on Windows as well?
> > Yes, that was the place where I observed the issue (as Windows apparently, 
> > under some circumstances, can have a couple of extra threads running that 
> > haven't been spawned by the code of the process itself), but it's easily 
> > reproducible on any platform as long as you make sure there's more than one 
> > thread running.
> > 
> > I copied the basis for the test from 
> > `Shell/Register/x86-multithreaded-read.test` - most of the tests there 
> > under `Shell/Register` seem to have such an XFAIL, not sure exactly why. 
> > (In this particular test, at least the `-pthread` linker flag might be a 
> > trivial platform specific difference that might break it, which would need 
> > to be abstracted somewhere.)
> Yeah I didn't notice the pthread flag, that could be it. Makes sense to me.
Actually, it turns out that this test does pass on Windows (I hadn't tested 
before, I had just inherited the XFAIL from the test I based it on) - as 
`-pthread` isn't a linker flag but a compiler driver flag, the msvc target in 
clang just does nothing about it. So I'll remove the XFAIL here before pushing 
it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134037

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

[Lldb-commits] [lldb] 8a3597d - [lldb] Fix CommandInterpreter::DidProcessStopAbnormally() with multiple threads

[Martin Storsjö via lldb-commits]

Author: Martin Storsjö
Date: 2022-09-26T11:05:42+03:00
New Revision: 8a3597d73c8f694ca8c991d8cb4bb97a4ac8ba8c

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

LOG: [lldb] Fix CommandInterpreter::DidProcessStopAbnormally() with multiple 
threads

If a process has multiple threads, the thread with the stop
info might not be the first one in the thread list.

On Windows, under certain circumstances, processes seem to have
one or more extra threads that haven't been launched by the
executable itself, waiting in NtWaitForWorkViaWorkerFactory. If the
main (stopped) thread isn't the first one in the list (the order
seems nondeterministic), DidProcessStopAbnormally() would return
false prematurely, instead of inspecting later threads.

The main observable effect of DidProcessStopAbnormally() erroneously
returning false, is when running lldb with multiple "-o" parameters
to specify multiple commands to execute on the command line.

After an abnormal stop, lldb would stop executing "-o" parameters
and execute "-k" parameters instead - but due to this issue, it
would instead keep executing "-o" parameters as if there was no
abnormal stop. (If multiple parameters are specified via a script
file via the "-s" option, all of the commands in that file are
executed regardless of whether there's an abnormal stop inbetween.)

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

Added: 
lldb/test/Shell/Driver/CommandOnCrashMultiThreaded.test
lldb/test/Shell/Driver/Inputs/CommandOnCrashMultiThreaded.cpp

Modified: 
lldb/source/Interpreter/CommandInterpreter.cpp

Removed: 




diff  --git a/lldb/source/Interpreter/CommandInterpreter.cpp 
b/lldb/source/Interpreter/CommandInterpreter.cpp
index fa6511635e287..5c11b87dcbe03 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2471,8 +2471,12 @@ bool CommandInterpreter::DidProcessStopAbnormally() 
const {
 
   for (const auto &thread_sp : process_sp->GetThreadList().Threads()) {
 StopInfoSP stop_info = thread_sp->GetStopInfo();
-if (!stop_info)
-  return false;
+if (!stop_info) {
+  // If there's no stop_info, keep iterating through the other threads;
+  // it's enough that any thread has got a stop_info that indicates
+  // an abnormal stop, to consider the process to be stopped abnormally.
+  continue;
+}
 
 const StopReason reason = stop_info->GetStopReason();
 if (reason == eStopReasonException ||

diff  --git a/lldb/test/Shell/Driver/CommandOnCrashMultiThreaded.test 
b/lldb/test/Shell/Driver/CommandOnCrashMultiThreaded.test
new file mode 100644
index 0..b16cfc5763715
--- /dev/null
+++ b/lldb/test/Shell/Driver/CommandOnCrashMultiThreaded.test
@@ -0,0 +1,5 @@
+# REQUIRES: native && (target-x86 || target-x86_64)
+# RUN: %clangxx_host %p/Inputs/CommandOnCrashMultiThreaded.cpp -o %t -pthread
+# RUN: %lldb -b -o "process launch" -k "process continue" -k "exit" %t | 
FileCheck %s
+
+# CHECK: Process {{[0-9]+}} exited with status = 0

diff  --git a/lldb/test/Shell/Driver/Inputs/CommandOnCrashMultiThreaded.cpp 
b/lldb/test/Shell/Driver/Inputs/CommandOnCrashMultiThreaded.cpp
new file mode 100644
index 0..f469d82fbbef9
--- /dev/null
+++ b/lldb/test/Shell/Driver/Inputs/CommandOnCrashMultiThreaded.cpp
@@ -0,0 +1,13 @@
+#include 
+
+void t_func() {
+  asm volatile(
+"int3\n\t"
+  );
+}
+
+int main() {
+  std::thread t(t_func);
+  t.join();
+  return 0;
+}



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

[Lldb-commits] [PATCH] D134037: [lldb] Fix CommandInterpreter::DidProcessStopAbnormally() with multiple threads

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8a3597d73c8f: [lldb] Fix 
CommandInterpreter::DidProcessStopAbnormally() with multiple threads (authored 
by mstorsjo).

Changed prior to commit:
  https://reviews.llvm.org/D134037?vs=462454&id=462836#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134037

Files:
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/test/Shell/Driver/CommandOnCrashMultiThreaded.test
  lldb/test/Shell/Driver/Inputs/CommandOnCrashMultiThreaded.cpp


Index: lldb/test/Shell/Driver/Inputs/CommandOnCrashMultiThreaded.cpp
===
--- /dev/null
+++ lldb/test/Shell/Driver/Inputs/CommandOnCrashMultiThreaded.cpp
@@ -0,0 +1,13 @@
+#include 
+
+void t_func() {
+  asm volatile(
+"int3\n\t"
+  );
+}
+
+int main() {
+  std::thread t(t_func);
+  t.join();
+  return 0;
+}
Index: lldb/test/Shell/Driver/CommandOnCrashMultiThreaded.test
===
--- /dev/null
+++ lldb/test/Shell/Driver/CommandOnCrashMultiThreaded.test
@@ -0,0 +1,5 @@
+# REQUIRES: native && (target-x86 || target-x86_64)
+# RUN: %clangxx_host %p/Inputs/CommandOnCrashMultiThreaded.cpp -o %t -pthread
+# RUN: %lldb -b -o "process launch" -k "process continue" -k "exit" %t | 
FileCheck %s
+
+# CHECK: Process {{[0-9]+}} exited with status = 0
Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2471,8 +2471,12 @@
 
   for (const auto &thread_sp : process_sp->GetThreadList().Threads()) {
 StopInfoSP stop_info = thread_sp->GetStopInfo();
-if (!stop_info)
-  return false;
+if (!stop_info) {
+  // If there's no stop_info, keep iterating through the other threads;
+  // it's enough that any thread has got a stop_info that indicates
+  // an abnormal stop, to consider the process to be stopped abnormally.
+  continue;
+}
 
 const StopReason reason = stop_info->GetStopReason();
 if (reason == eStopReasonException ||


Index: lldb/test/Shell/Driver/Inputs/CommandOnCrashMultiThreaded.cpp
===
--- /dev/null
+++ lldb/test/Shell/Driver/Inputs/CommandOnCrashMultiThreaded.cpp
@@ -0,0 +1,13 @@
+#include 
+
+void t_func() {
+  asm volatile(
+"int3\n\t"
+  );
+}
+
+int main() {
+  std::thread t(t_func);
+  t.join();
+  return 0;
+}
Index: lldb/test/Shell/Driver/CommandOnCrashMultiThreaded.test
===
--- /dev/null
+++ lldb/test/Shell/Driver/CommandOnCrashMultiThreaded.test
@@ -0,0 +1,5 @@
+# REQUIRES: native && (target-x86 || target-x86_64)
+# RUN: %clangxx_host %p/Inputs/CommandOnCrashMultiThreaded.cpp -o %t -pthread
+# RUN: %lldb -b -o "process launch" -k "process continue" -k "exit" %t | FileCheck %s
+
+# CHECK: Process {{[0-9]+}} exited with status = 0
Index: lldb/source/Interpreter/CommandInterpreter.cpp
===
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2471,8 +2471,12 @@
 
   for (const auto &thread_sp : process_sp->GetThreadList().Threads()) {
 StopInfoSP stop_info = thread_sp->GetStopInfo();
-if (!stop_info)
-  return false;
+if (!stop_info) {
+  // If there's no stop_info, keep iterating through the other threads;
+  // it's enough that any thread has got a stop_info that indicates
+  // an abnormal stop, to consider the process to be stopped abnormally.
+  continue;
+}
 
 const StopReason reason = stop_info->GetStopReason();
 if (reason == eStopReasonException ||
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D134515: [lldb] Fix completion of 'settings set' values

[David Spickett via Phabricator via lldb-commits]

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

LGTM, thanks for the fix!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134515

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

[Lldb-commits] [PATCH] D134585: [lldb][COFF] Map symbols without base+complex type as 'Data' type

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

You're the expert on what the linker does and the code looks fine.

Is it possible that msvc uses these `IMAGE_SYM_TYPE_NULL` in a different way 
that could cause a problem? Worst that happens is we get a bunch of symbols 
available in expressions that shouldn't be?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134585

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

[Lldb-commits] [PATCH] D134585: [lldb][COFF] Map symbols without base+complex type as 'Data' type

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D134585#3814458 , @DavidSpickett 
wrote:

> You're the expert on what the linker does and the code looks fine.
>
> Is it possible that msvc uses these `IMAGE_SYM_TYPE_NULL` in a different way 
> that could cause a problem? Worst that happens is we get a bunch of symbols 
> available in expressions that shouldn't be?

MSVC/link.exe doesn't write symbols into linked PE images at all. But at the 
object file stage, they do express data symbols in the same way (which is the 
default marking for symbols that have no other types set).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134585

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

[Lldb-commits] [PATCH] D134585: [lldb][COFF] Map symbols without base+complex type as 'Data' type

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> MSVC/link.exe doesn't write symbols into linked PE images at all.

So by the time we get to a debugger, it's not an issue anyway.

Then this LGTM.

Thanks for all these improvements btw, I expect Linaro's Windows on Arm efforts 
appreciate them too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134585

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

[Lldb-commits] [PATCH] D134581: [lldb] Prevent re-adding a module that is already loaded

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added inline comments.



Comment at: lldb/test/Shell/Target/dependent-modules-nodupe-windows.test:7
+# RUN: %clang_host -g0 -O0 -shared %S/Inputs/shlib.c -o %t.shlib.dll
+# RUN: %clang_host -g0 -O0 %S/Inputs/main.c %t.shlib.dll -o %t.main.exe
+# RUN: %lldb -b -o "#before" -o "target modules list" -o "b main" -o run \

Unfortunately, this aspect of the test doesn't work as is on Windows.

By default (in msvc builds), clang invokes link.exe here, but link.exe can't 
take `%t.shlib.dll` as input file to the linker, as it requires an import 
library.

If we would assume that lld is available, we could run the linking command with 
`-fuse-ld=lld` - however that on its own isn't enough, as lld only allows 
linking against DLLs when run with the `-lldmingw` flag. We can add 
`-fuse-ld=lld -Wl,-lldmingw` here to fix that, but that would break testing in 
mingw environments, as `-lldmingw` isn't a valid option on the mingw lld driver.

Likewise, we could create a proper import library by adding 
`-Wl,-implib:%t.shlib.lib` on the first command above, but that doesn't work in 
mingw mode either, as it would have to be `-Wl,--out-implib=%t.shlib.lib` 
instead.

In practice, running the lldb tests in mingw mode is not entirely supported, 
while they do pass cleanly in MSVC mode (and there's a buildbot testing this 
configuration) - but I would like to work towards making things work better in 
the mingw configuration too.

There's a different substitution, `%build`, which invokes the 
`lldb/test/Shell/helpers/build.py` script, which abstracts a bunch of 
boilerplate details mostly relevant for windows targets (like creating PDB 
debug info files); the easiest at this point would probably be to extend that 
script with options for creating import libraries.

For testing with mingw, I'm currently using this out of tree patch for that 
script too:
```
diff --git a/lldb/test/Shell/helper/build.py b/lldb/test/Shell/helper/build.py
index 96684b7b3e66..f138b00bee9e 100755
--- a/lldb/test/Shell/helper/build.py
+++ b/lldb/test/Shell/helper/build.py
@@ -191,7 +191,10 @@ def find_toolchain(compiler, tools_dir):
 if compiler == 'any':
 priorities = []
 if sys.platform == 'win32':
-priorities = ['clang-cl', 'msvc', 'clang', 'gcc']
+if 'gcc' in sys.version.lower():
+priorities = ['clang', 'gcc', 'clang-cl', 'msvc']
+else:
+priorities = ['clang-cl', 'msvc', 'clang', 'gcc']
 else:
 priorities = ['clang', 'gcc', 'clang-cl']
 for toolchain in priorities:
```
(This is a bit hacky, since it uses the build type of the python interpreter to 
switch the preference between clang-cl and clang.)

Alternatively, we could maybe add a separate substitution that expands into 
either `-implib:` or `--out-implib=`, but I'm not sure if we can guess which 
one will be the right one either, while the build.py helper script probably can 
do better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134581

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

[Lldb-commits] [PATCH] D134585: [lldb][COFF] Map symbols without base+complex type as 'Data' type

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D134585#3814463 , @DavidSpickett 
wrote:

>> MSVC/link.exe doesn't write symbols into linked PE images at all.
>
> So by the time we get to a debugger, it's not an issue anyway.

Yep, most of these patches about symbol table handling doesn't make any 
difference for the MSVC ecosystem usage at all (although some of these patches 
fix other generic windows-specific bugs noticed too).

> Then this LGTM.

Thanks! Can you mark it formally approved too? :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134585

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

[Lldb-commits] [PATCH] D134581: [lldb] Prevent re-adding a module that is already loaded

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added inline comments.



Comment at: lldb/test/Shell/Target/dependent-modules-nodupe-windows.test:7
+# RUN: %clang_host -g0 -O0 -shared %S/Inputs/shlib.c -o %t.shlib.dll
+# RUN: %clang_host -g0 -O0 %S/Inputs/main.c %t.shlib.dll -o %t.main.exe
+# RUN: %lldb -b -o "#before" -o "target modules list" -o "b main" -o run \

mstorsjo wrote:
> Unfortunately, this aspect of the test doesn't work as is on Windows.
> 
> By default (in msvc builds), clang invokes link.exe here, but link.exe can't 
> take `%t.shlib.dll` as input file to the linker, as it requires an import 
> library.
> 
> If we would assume that lld is available, we could run the linking command 
> with `-fuse-ld=lld` - however that on its own isn't enough, as lld only 
> allows linking against DLLs when run with the `-lldmingw` flag. We can add 
> `-fuse-ld=lld -Wl,-lldmingw` here to fix that, but that would break testing 
> in mingw environments, as `-lldmingw` isn't a valid option on the mingw lld 
> driver.
> 
> Likewise, we could create a proper import library by adding 
> `-Wl,-implib:%t.shlib.lib` on the first command above, but that doesn't work 
> in mingw mode either, as it would have to be `-Wl,--out-implib=%t.shlib.lib` 
> instead.
> 
> In practice, running the lldb tests in mingw mode is not entirely supported, 
> while they do pass cleanly in MSVC mode (and there's a buildbot testing this 
> configuration) - but I would like to work towards making things work better 
> in the mingw configuration too.
> 
> There's a different substitution, `%build`, which invokes the 
> `lldb/test/Shell/helpers/build.py` script, which abstracts a bunch of 
> boilerplate details mostly relevant for windows targets (like creating PDB 
> debug info files); the easiest at this point would probably be to extend that 
> script with options for creating import libraries.
> 
> For testing with mingw, I'm currently using this out of tree patch for that 
> script too:
> ```
> diff --git a/lldb/test/Shell/helper/build.py b/lldb/test/Shell/helper/build.py
> index 96684b7b3e66..f138b00bee9e 100755
> --- a/lldb/test/Shell/helper/build.py
> +++ b/lldb/test/Shell/helper/build.py
> @@ -191,7 +191,10 @@ def find_toolchain(compiler, tools_dir):
>  if compiler == 'any':
>  priorities = []
>  if sys.platform == 'win32':
> -priorities = ['clang-cl', 'msvc', 'clang', 'gcc']
> +if 'gcc' in sys.version.lower():
> +priorities = ['clang', 'gcc', 'clang-cl', 'msvc']
> +else:
> +priorities = ['clang-cl', 'msvc', 'clang', 'gcc']
>  else:
>  priorities = ['clang', 'gcc', 'clang-cl']
>  for toolchain in priorities:
> ```
> (This is a bit hacky, since it uses the build type of the python interpreter 
> to switch the preference between clang-cl and clang.)
> 
> Alternatively, we could maybe add a separate substitution that expands into 
> either `-implib:` or `--out-implib=`, but I'm not sure if we can guess which 
> one will be the right one either, while the build.py helper script probably 
> can do better.
I forgot to mention; see https://reviews.llvm.org/D129455 for some earlier 
discussion about other aspects (symbol table vs dwarf vs PDB) for windows 
testcases in lldb.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134581

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

[Lldb-commits] [PATCH] D134585: [lldb][COFF] Map symbols without base+complex type as 'Data' type

[David Spickett via Phabricator via lldb-commits]

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

Would help if I actually clicked the buttons yes :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134585

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

[Lldb-commits] [PATCH] D134581: [lldb] Prevent re-adding a module that is already loaded

[Alvin Wong via Phabricator via lldb-commits]

alvinhochun added inline comments.



Comment at: lldb/test/Shell/Target/dependent-modules-nodupe-windows.test:7
+# RUN: %clang_host -g0 -O0 -shared %S/Inputs/shlib.c -o %t.shlib.dll
+# RUN: %clang_host -g0 -O0 %S/Inputs/main.c %t.shlib.dll -o %t.main.exe
+# RUN: %lldb -b -o "#before" -o "target modules list" -o "b main" -o run \

mstorsjo wrote:
> mstorsjo wrote:
> > Unfortunately, this aspect of the test doesn't work as is on Windows.
> > 
> > By default (in msvc builds), clang invokes link.exe here, but link.exe 
> > can't take `%t.shlib.dll` as input file to the linker, as it requires an 
> > import library.
> > 
> > If we would assume that lld is available, we could run the linking command 
> > with `-fuse-ld=lld` - however that on its own isn't enough, as lld only 
> > allows linking against DLLs when run with the `-lldmingw` flag. We can add 
> > `-fuse-ld=lld -Wl,-lldmingw` here to fix that, but that would break testing 
> > in mingw environments, as `-lldmingw` isn't a valid option on the mingw lld 
> > driver.
> > 
> > Likewise, we could create a proper import library by adding 
> > `-Wl,-implib:%t.shlib.lib` on the first command above, but that doesn't 
> > work in mingw mode either, as it would have to be 
> > `-Wl,--out-implib=%t.shlib.lib` instead.
> > 
> > In practice, running the lldb tests in mingw mode is not entirely 
> > supported, while they do pass cleanly in MSVC mode (and there's a buildbot 
> > testing this configuration) - but I would like to work towards making 
> > things work better in the mingw configuration too.
> > 
> > There's a different substitution, `%build`, which invokes the 
> > `lldb/test/Shell/helpers/build.py` script, which abstracts a bunch of 
> > boilerplate details mostly relevant for windows targets (like creating PDB 
> > debug info files); the easiest at this point would probably be to extend 
> > that script with options for creating import libraries.
> > 
> > For testing with mingw, I'm currently using this out of tree patch for that 
> > script too:
> > ```
> > diff --git a/lldb/test/Shell/helper/build.py 
> > b/lldb/test/Shell/helper/build.py
> > index 96684b7b3e66..f138b00bee9e 100755
> > --- a/lldb/test/Shell/helper/build.py
> > +++ b/lldb/test/Shell/helper/build.py
> > @@ -191,7 +191,10 @@ def find_toolchain(compiler, tools_dir):
> >  if compiler == 'any':
> >  priorities = []
> >  if sys.platform == 'win32':
> > -priorities = ['clang-cl', 'msvc', 'clang', 'gcc']
> > +if 'gcc' in sys.version.lower():
> > +priorities = ['clang', 'gcc', 'clang-cl', 'msvc']
> > +else:
> > +priorities = ['clang-cl', 'msvc', 'clang', 'gcc']
> >  else:
> >  priorities = ['clang', 'gcc', 'clang-cl']
> >  for toolchain in priorities:
> > ```
> > (This is a bit hacky, since it uses the build type of the python 
> > interpreter to switch the preference between clang-cl and clang.)
> > 
> > Alternatively, we could maybe add a separate substitution that expands into 
> > either `-implib:` or `--out-implib=`, but I'm not sure if we can guess 
> > which one will be the right one either, while the build.py helper script 
> > probably can do better.
> I forgot to mention; see https://reviews.llvm.org/D129455 for some earlier 
> discussion about other aspects (symbol table vs dwarf vs PDB) for windows 
> testcases in lldb.
Re different options on msvc vs mingw, how about using the `%if` substitution 
(https://www.llvm.org/docs/TestingGuide.html)? We can use it to check for the 
`windows-msvc` feature to apply options conditionally.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134581

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

[Lldb-commits] [PATCH] D134581: [lldb] Prevent re-adding a module that is already loaded

[Alvin Wong via Phabricator via lldb-commits]

alvinhochun added inline comments.



Comment at: lldb/test/Shell/Target/dependent-modules-nodupe-windows.test:7
+# RUN: %clang_host -g0 -O0 -shared %S/Inputs/shlib.c -o %t.shlib.dll
+# RUN: %clang_host -g0 -O0 %S/Inputs/main.c %t.shlib.dll -o %t.main.exe
+# RUN: %lldb -b -o "#before" -o "target modules list" -o "b main" -o run \

alvinhochun wrote:
> mstorsjo wrote:
> > mstorsjo wrote:
> > > Unfortunately, this aspect of the test doesn't work as is on Windows.
> > > 
> > > By default (in msvc builds), clang invokes link.exe here, but link.exe 
> > > can't take `%t.shlib.dll` as input file to the linker, as it requires an 
> > > import library.
> > > 
> > > If we would assume that lld is available, we could run the linking 
> > > command with `-fuse-ld=lld` - however that on its own isn't enough, as 
> > > lld only allows linking against DLLs when run with the `-lldmingw` flag. 
> > > We can add `-fuse-ld=lld -Wl,-lldmingw` here to fix that, but that would 
> > > break testing in mingw environments, as `-lldmingw` isn't a valid option 
> > > on the mingw lld driver.
> > > 
> > > Likewise, we could create a proper import library by adding 
> > > `-Wl,-implib:%t.shlib.lib` on the first command above, but that doesn't 
> > > work in mingw mode either, as it would have to be 
> > > `-Wl,--out-implib=%t.shlib.lib` instead.
> > > 
> > > In practice, running the lldb tests in mingw mode is not entirely 
> > > supported, while they do pass cleanly in MSVC mode (and there's a 
> > > buildbot testing this configuration) - but I would like to work towards 
> > > making things work better in the mingw configuration too.
> > > 
> > > There's a different substitution, `%build`, which invokes the 
> > > `lldb/test/Shell/helpers/build.py` script, which abstracts a bunch of 
> > > boilerplate details mostly relevant for windows targets (like creating 
> > > PDB debug info files); the easiest at this point would probably be to 
> > > extend that script with options for creating import libraries.
> > > 
> > > For testing with mingw, I'm currently using this out of tree patch for 
> > > that script too:
> > > ```
> > > diff --git a/lldb/test/Shell/helper/build.py 
> > > b/lldb/test/Shell/helper/build.py
> > > index 96684b7b3e66..f138b00bee9e 100755
> > > --- a/lldb/test/Shell/helper/build.py
> > > +++ b/lldb/test/Shell/helper/build.py
> > > @@ -191,7 +191,10 @@ def find_toolchain(compiler, tools_dir):
> > >  if compiler == 'any':
> > >  priorities = []
> > >  if sys.platform == 'win32':
> > > -priorities = ['clang-cl', 'msvc', 'clang', 'gcc']
> > > +if 'gcc' in sys.version.lower():
> > > +priorities = ['clang', 'gcc', 'clang-cl', 'msvc']
> > > +else:
> > > +priorities = ['clang-cl', 'msvc', 'clang', 'gcc']
> > >  else:
> > >  priorities = ['clang', 'gcc', 'clang-cl']
> > >  for toolchain in priorities:
> > > ```
> > > (This is a bit hacky, since it uses the build type of the python 
> > > interpreter to switch the preference between clang-cl and clang.)
> > > 
> > > Alternatively, we could maybe add a separate substitution that expands 
> > > into either `-implib:` or `--out-implib=`, but I'm not sure if we can 
> > > guess which one will be the right one either, while the build.py helper 
> > > script probably can do better.
> > I forgot to mention; see https://reviews.llvm.org/D129455 for some earlier 
> > discussion about other aspects (symbol table vs dwarf vs PDB) for windows 
> > testcases in lldb.
> Re different options on msvc vs mingw, how about using the `%if` substitution 
> (https://www.llvm.org/docs/TestingGuide.html)? We can use it to check for the 
> `windows-msvc` feature to apply options conditionally.
The commands should become this:

```
# RUN: %clang_host -g0 -O0 -shared %S/Inputs/shlib.c -o %t.shlib.dll %if 
windows-msvc %{-Wl,-implib:%t.shlib.lib}
# RUN: %clang_host -g0 -O0 %S/Inputs/main.c -o %t.main.exe %if windows-msvc 
%{%t.shlib.lib} %else %{%t.shlib.dll}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134581

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

[Lldb-commits] [PATCH] D134581: [lldb] Prevent re-adding a module that is already loaded

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added inline comments.



Comment at: lldb/test/Shell/Target/dependent-modules-nodupe-windows.test:7
+# RUN: %clang_host -g0 -O0 -shared %S/Inputs/shlib.c -o %t.shlib.dll
+# RUN: %clang_host -g0 -O0 %S/Inputs/main.c %t.shlib.dll -o %t.main.exe
+# RUN: %lldb -b -o "#before" -o "target modules list" -o "b main" -o run \

alvinhochun wrote:
> mstorsjo wrote:
> > mstorsjo wrote:
> > > Unfortunately, this aspect of the test doesn't work as is on Windows.
> > > 
> > > By default (in msvc builds), clang invokes link.exe here, but link.exe 
> > > can't take `%t.shlib.dll` as input file to the linker, as it requires an 
> > > import library.
> > > 
> > > If we would assume that lld is available, we could run the linking 
> > > command with `-fuse-ld=lld` - however that on its own isn't enough, as 
> > > lld only allows linking against DLLs when run with the `-lldmingw` flag. 
> > > We can add `-fuse-ld=lld -Wl,-lldmingw` here to fix that, but that would 
> > > break testing in mingw environments, as `-lldmingw` isn't a valid option 
> > > on the mingw lld driver.
> > > 
> > > Likewise, we could create a proper import library by adding 
> > > `-Wl,-implib:%t.shlib.lib` on the first command above, but that doesn't 
> > > work in mingw mode either, as it would have to be 
> > > `-Wl,--out-implib=%t.shlib.lib` instead.
> > > 
> > > In practice, running the lldb tests in mingw mode is not entirely 
> > > supported, while they do pass cleanly in MSVC mode (and there's a 
> > > buildbot testing this configuration) - but I would like to work towards 
> > > making things work better in the mingw configuration too.
> > > 
> > > There's a different substitution, `%build`, which invokes the 
> > > `lldb/test/Shell/helpers/build.py` script, which abstracts a bunch of 
> > > boilerplate details mostly relevant for windows targets (like creating 
> > > PDB debug info files); the easiest at this point would probably be to 
> > > extend that script with options for creating import libraries.
> > > 
> > > For testing with mingw, I'm currently using this out of tree patch for 
> > > that script too:
> > > ```
> > > diff --git a/lldb/test/Shell/helper/build.py 
> > > b/lldb/test/Shell/helper/build.py
> > > index 96684b7b3e66..f138b00bee9e 100755
> > > --- a/lldb/test/Shell/helper/build.py
> > > +++ b/lldb/test/Shell/helper/build.py
> > > @@ -191,7 +191,10 @@ def find_toolchain(compiler, tools_dir):
> > >  if compiler == 'any':
> > >  priorities = []
> > >  if sys.platform == 'win32':
> > > -priorities = ['clang-cl', 'msvc', 'clang', 'gcc']
> > > +if 'gcc' in sys.version.lower():
> > > +priorities = ['clang', 'gcc', 'clang-cl', 'msvc']
> > > +else:
> > > +priorities = ['clang-cl', 'msvc', 'clang', 'gcc']
> > >  else:
> > >  priorities = ['clang', 'gcc', 'clang-cl']
> > >  for toolchain in priorities:
> > > ```
> > > (This is a bit hacky, since it uses the build type of the python 
> > > interpreter to switch the preference between clang-cl and clang.)
> > > 
> > > Alternatively, we could maybe add a separate substitution that expands 
> > > into either `-implib:` or `--out-implib=`, but I'm not sure if we can 
> > > guess which one will be the right one either, while the build.py helper 
> > > script probably can do better.
> > I forgot to mention; see https://reviews.llvm.org/D129455 for some earlier 
> > discussion about other aspects (symbol table vs dwarf vs PDB) for windows 
> > testcases in lldb.
> Re different options on msvc vs mingw, how about using the `%if` substitution 
> (https://www.llvm.org/docs/TestingGuide.html)? We can use it to check for the 
> `windows-msvc` feature to apply options conditionally.
Oh, I guess that would work too - it does litter the individual tests with the 
conditions instead of hiding it in `build.py`, but probably is acceptable too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134581

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

[Lldb-commits] [PATCH] D134581: [lldb] Prevent re-adding a module that is already loaded

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added inline comments.



Comment at: lldb/test/Shell/Target/dependent-modules-nodupe-windows.test:7
+# RUN: %clang_host -g0 -O0 -shared %S/Inputs/shlib.c -o %t.shlib.dll
+# RUN: %clang_host -g0 -O0 %S/Inputs/main.c %t.shlib.dll -o %t.main.exe
+# RUN: %lldb -b -o "#before" -o "target modules list" -o "b main" -o run \

alvinhochun wrote:
> mstorsjo wrote:
> > alvinhochun wrote:
> > > mstorsjo wrote:
> > > > mstorsjo wrote:
> > > > > Unfortunately, this aspect of the test doesn't work as is on Windows.
> > > > > 
> > > > > By default (in msvc builds), clang invokes link.exe here, but 
> > > > > link.exe can't take `%t.shlib.dll` as input file to the linker, as it 
> > > > > requires an import library.
> > > > > 
> > > > > If we would assume that lld is available, we could run the linking 
> > > > > command with `-fuse-ld=lld` - however that on its own isn't enough, 
> > > > > as lld only allows linking against DLLs when run with the `-lldmingw` 
> > > > > flag. We can add `-fuse-ld=lld -Wl,-lldmingw` here to fix that, but 
> > > > > that would break testing in mingw environments, as `-lldmingw` isn't 
> > > > > a valid option on the mingw lld driver.
> > > > > 
> > > > > Likewise, we could create a proper import library by adding 
> > > > > `-Wl,-implib:%t.shlib.lib` on the first command above, but that 
> > > > > doesn't work in mingw mode either, as it would have to be 
> > > > > `-Wl,--out-implib=%t.shlib.lib` instead.
> > > > > 
> > > > > In practice, running the lldb tests in mingw mode is not entirely 
> > > > > supported, while they do pass cleanly in MSVC mode (and there's a 
> > > > > buildbot testing this configuration) - but I would like to work 
> > > > > towards making things work better in the mingw configuration too.
> > > > > 
> > > > > There's a different substitution, `%build`, which invokes the 
> > > > > `lldb/test/Shell/helpers/build.py` script, which abstracts a bunch of 
> > > > > boilerplate details mostly relevant for windows targets (like 
> > > > > creating PDB debug info files); the easiest at this point would 
> > > > > probably be to extend that script with options for creating import 
> > > > > libraries.
> > > > > 
> > > > > For testing with mingw, I'm currently using this out of tree patch 
> > > > > for that script too:
> > > > > ```
> > > > > diff --git a/lldb/test/Shell/helper/build.py 
> > > > > b/lldb/test/Shell/helper/build.py
> > > > > index 96684b7b3e66..f138b00bee9e 100755
> > > > > --- a/lldb/test/Shell/helper/build.py
> > > > > +++ b/lldb/test/Shell/helper/build.py
> > > > > @@ -191,7 +191,10 @@ def find_toolchain(compiler, tools_dir):
> > > > >  if compiler == 'any':
> > > > >  priorities = []
> > > > >  if sys.platform == 'win32':
> > > > > -priorities = ['clang-cl', 'msvc', 'clang', 'gcc']
> > > > > +if 'gcc' in sys.version.lower():
> > > > > +priorities = ['clang', 'gcc', 'clang-cl', 'msvc']
> > > > > +else:
> > > > > +priorities = ['clang-cl', 'msvc', 'clang', 'gcc']
> > > > >  else:
> > > > >  priorities = ['clang', 'gcc', 'clang-cl']
> > > > >  for toolchain in priorities:
> > > > > ```
> > > > > (This is a bit hacky, since it uses the build type of the python 
> > > > > interpreter to switch the preference between clang-cl and clang.)
> > > > > 
> > > > > Alternatively, we could maybe add a separate substitution that 
> > > > > expands into either `-implib:` or `--out-implib=`, but I'm not sure 
> > > > > if we can guess which one will be the right one either, while the 
> > > > > build.py helper script probably can do better.
> > > > I forgot to mention; see https://reviews.llvm.org/D129455 for some 
> > > > earlier discussion about other aspects (symbol table vs dwarf vs PDB) 
> > > > for windows testcases in lldb.
> > > Re different options on msvc vs mingw, how about using the `%if` 
> > > substitution (https://www.llvm.org/docs/TestingGuide.html)? We can use it 
> > > to check for the `windows-msvc` feature to apply options conditionally.
> > Oh, I guess that would work too - it does litter the individual tests with 
> > the conditions instead of hiding it in `build.py`, but probably is 
> > acceptable too.
> The commands should become this:
> 
> ```
> # RUN: %clang_host -g0 -O0 -shared %S/Inputs/shlib.c -o %t.shlib.dll %if 
> windows-msvc %{-Wl,-implib:%t.shlib.lib}
> # RUN: %clang_host -g0 -O0 %S/Inputs/main.c -o %t.main.exe %if windows-msvc 
> %{%t.shlib.lib} %else %{%t.shlib.dll}
> ```
This form works, and keeps it to just one conditional:
```
# RUN: %clang_host -g0 -O0 -shared %S/Inputs/shlib.c -o %t.shlib.dll %if 
windows-msvc %{-Wl,-implib:%t.shlib.lib%} %else 
%{-Wl,--out-implib=%t.shlib.lib%}
# RUN: %clang_host -g0 -O0 %S/Inputs/main.c %t.shlib.lib -o %t.main.exe
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews

[Lldb-commits] [PATCH] D134581: [lldb] Prevent re-adding a module that is already loaded

[Alvin Wong via Phabricator via lldb-commits]

alvinhochun added inline comments.



Comment at: lldb/test/Shell/Target/dependent-modules-nodupe-windows.test:7
+# RUN: %clang_host -g0 -O0 -shared %S/Inputs/shlib.c -o %t.shlib.dll
+# RUN: %clang_host -g0 -O0 %S/Inputs/main.c %t.shlib.dll -o %t.main.exe
+# RUN: %lldb -b -o "#before" -o "target modules list" -o "b main" -o run \

mstorsjo wrote:
> alvinhochun wrote:
> > mstorsjo wrote:
> > > alvinhochun wrote:
> > > > mstorsjo wrote:
> > > > > mstorsjo wrote:
> > > > > > Unfortunately, this aspect of the test doesn't work as is on 
> > > > > > Windows.
> > > > > > 
> > > > > > By default (in msvc builds), clang invokes link.exe here, but 
> > > > > > link.exe can't take `%t.shlib.dll` as input file to the linker, as 
> > > > > > it requires an import library.
> > > > > > 
> > > > > > If we would assume that lld is available, we could run the linking 
> > > > > > command with `-fuse-ld=lld` - however that on its own isn't enough, 
> > > > > > as lld only allows linking against DLLs when run with the 
> > > > > > `-lldmingw` flag. We can add `-fuse-ld=lld -Wl,-lldmingw` here to 
> > > > > > fix that, but that would break testing in mingw environments, as 
> > > > > > `-lldmingw` isn't a valid option on the mingw lld driver.
> > > > > > 
> > > > > > Likewise, we could create a proper import library by adding 
> > > > > > `-Wl,-implib:%t.shlib.lib` on the first command above, but that 
> > > > > > doesn't work in mingw mode either, as it would have to be 
> > > > > > `-Wl,--out-implib=%t.shlib.lib` instead.
> > > > > > 
> > > > > > In practice, running the lldb tests in mingw mode is not entirely 
> > > > > > supported, while they do pass cleanly in MSVC mode (and there's a 
> > > > > > buildbot testing this configuration) - but I would like to work 
> > > > > > towards making things work better in the mingw configuration too.
> > > > > > 
> > > > > > There's a different substitution, `%build`, which invokes the 
> > > > > > `lldb/test/Shell/helpers/build.py` script, which abstracts a bunch 
> > > > > > of boilerplate details mostly relevant for windows targets (like 
> > > > > > creating PDB debug info files); the easiest at this point would 
> > > > > > probably be to extend that script with options for creating import 
> > > > > > libraries.
> > > > > > 
> > > > > > For testing with mingw, I'm currently using this out of tree patch 
> > > > > > for that script too:
> > > > > > ```
> > > > > > diff --git a/lldb/test/Shell/helper/build.py 
> > > > > > b/lldb/test/Shell/helper/build.py
> > > > > > index 96684b7b3e66..f138b00bee9e 100755
> > > > > > --- a/lldb/test/Shell/helper/build.py
> > > > > > +++ b/lldb/test/Shell/helper/build.py
> > > > > > @@ -191,7 +191,10 @@ def find_toolchain(compiler, tools_dir):
> > > > > >  if compiler == 'any':
> > > > > >  priorities = []
> > > > > >  if sys.platform == 'win32':
> > > > > > -priorities = ['clang-cl', 'msvc', 'clang', 'gcc']
> > > > > > +if 'gcc' in sys.version.lower():
> > > > > > +priorities = ['clang', 'gcc', 'clang-cl', 'msvc']
> > > > > > +else:
> > > > > > +priorities = ['clang-cl', 'msvc', 'clang', 'gcc']
> > > > > >  else:
> > > > > >  priorities = ['clang', 'gcc', 'clang-cl']
> > > > > >  for toolchain in priorities:
> > > > > > ```
> > > > > > (This is a bit hacky, since it uses the build type of the python 
> > > > > > interpreter to switch the preference between clang-cl and clang.)
> > > > > > 
> > > > > > Alternatively, we could maybe add a separate substitution that 
> > > > > > expands into either `-implib:` or `--out-implib=`, but I'm not sure 
> > > > > > if we can guess which one will be the right one either, while the 
> > > > > > build.py helper script probably can do better.
> > > > > I forgot to mention; see https://reviews.llvm.org/D129455 for some 
> > > > > earlier discussion about other aspects (symbol table vs dwarf vs PDB) 
> > > > > for windows testcases in lldb.
> > > > Re different options on msvc vs mingw, how about using the `%if` 
> > > > substitution (https://www.llvm.org/docs/TestingGuide.html)? We can use 
> > > > it to check for the `windows-msvc` feature to apply options 
> > > > conditionally.
> > > Oh, I guess that would work too - it does litter the individual tests 
> > > with the conditions instead of hiding it in `build.py`, but probably is 
> > > acceptable too.
> > The commands should become this:
> > 
> > ```
> > # RUN: %clang_host -g0 -O0 -shared %S/Inputs/shlib.c -o %t.shlib.dll %if 
> > windows-msvc %{-Wl,-implib:%t.shlib.lib}
> > # RUN: %clang_host -g0 -O0 %S/Inputs/main.c -o %t.main.exe %if windows-msvc 
> > %{%t.shlib.lib} %else %{%t.shlib.dll}
> > ```
> This form works, and keeps it to just one conditional:
> ```
> # RUN: %clang_host -g0 -O0 -shared %S/Inputs/shlib.c -o %t.shlib.dll %if 
> windows-msvc %{-Wl,-implib:%t.shlib.lib%} %else 
> %{-Wl,--out-implib=%t.

[Lldb-commits] [PATCH] D134536: [LLDB] Add an llvm::Optional version of GetRegisterInfo

[David Spickett via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0e912417c67d: [LLDB] Add an llvm::Optional version of 
GetRegisterInfo (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134536

Files:
  lldb/include/lldb/Core/EmulateInstruction.h
  lldb/source/Core/EmulateInstruction.cpp
  lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
  lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.h
  lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
  lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.h
  lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp
  lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.h
  lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp
  lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.h
  lldb/source/Plugins/Instruction/PPC64/EmulateInstructionPPC64.cpp
  lldb/source/Plugins/Instruction/PPC64/EmulateInstructionPPC64.h
  lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
  lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h

Index: lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h
===
--- lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h
+++ lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h
@@ -76,8 +76,10 @@
   bool EvaluateInstruction(uint32_t options) override;
   bool TestEmulation(Stream *out_stream, ArchSpec &arch,
  OptionValueDictionary *test_data) override;
-  bool GetRegisterInfo(lldb::RegisterKind reg_kind, uint32_t reg_num,
-   RegisterInfo ®_info) override;
+  using EmulateInstruction::GetRegisterInfo;
+
+  llvm::Optional GetRegisterInfo(lldb::RegisterKind reg_kind,
+   uint32_t reg_num) override;
 
   lldb::addr_t ReadPC(bool &success);
   bool WritePC(lldb::addr_t pc);
Index: lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
===
--- lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
+++ lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
@@ -1286,9 +1286,9 @@
LLDB_REGNUM_GENERIC_PC, pc);
 }
 
-bool EmulateInstructionRISCV::GetRegisterInfo(lldb::RegisterKind reg_kind,
-  uint32_t reg_index,
-  RegisterInfo ®_info) {
+llvm::Optional
+EmulateInstructionRISCV::GetRegisterInfo(lldb::RegisterKind reg_kind,
+ uint32_t reg_index) {
   if (reg_kind == eRegisterKindGeneric) {
 switch (reg_index) {
 case LLDB_REGNUM_GENERIC_PC:
@@ -1320,10 +1320,9 @@
   RegisterInfoPOSIX_riscv64::GetRegisterInfoCount(m_arch);
 
   if (reg_index >= length || reg_kind != eRegisterKindLLDB)
-return false;
+return {};
 
-  reg_info = array[reg_index];
-  return true;
+  return array[reg_index];
 }
 
 bool EmulateInstructionRISCV::SetTargetTriple(const ArchSpec &arch) {
Index: lldb/source/Plugins/Instruction/PPC64/EmulateInstructionPPC64.h
===
--- lldb/source/Plugins/Instruction/PPC64/EmulateInstructionPPC64.h
+++ lldb/source/Plugins/Instruction/PPC64/EmulateInstructionPPC64.h
@@ -61,8 +61,10 @@
 return false;
   }
 
-  bool GetRegisterInfo(lldb::RegisterKind reg_kind, uint32_t reg_num,
-   RegisterInfo ®_info) override;
+  using EmulateInstruction::GetRegisterInfo;
+
+  llvm::Optional GetRegisterInfo(lldb::RegisterKind reg_kind,
+   uint32_t reg_num) override;
 
   bool CreateFunctionEntryUnwind(UnwindPlan &unwind_plan) override;
 
Index: lldb/source/Plugins/Instruction/PPC64/EmulateInstructionPPC64.cpp
===
--- lldb/source/Plugins/Instruction/PPC64/EmulateInstructionPPC64.cpp
+++ lldb/source/Plugins/Instruction/PPC64/EmulateInstructionPPC64.cpp
@@ -58,16 +58,15 @@
   return arch.GetTriple().isPPC64();
 }
 
-static bool LLDBTableGetRegisterInfo(uint32_t reg_num, RegisterInfo ®_info) {
+static llvm::Optional LLDBTableGetRegisterInfo(uint32_t reg_num) {
   if (reg_num >= std::size(g_register_infos_ppc64le))
-return false;
-  reg_info = g_register_infos_ppc64le[reg_num];
-  return true;
+return {};
+  return g_register_infos_ppc64le[reg_num];
 }
 
-bool EmulateInstructionPPC64::GetRegisterInfo(RegisterKind reg_kind,
-  uint32_t reg_num,
-  RegisterInfo ®_info) {
+llvm::Optional
+EmulateInstructionPPC64::GetRegisterInfo(RegisterKind reg_kind,
+ uint32

[Lldb-commits] [lldb] 0e91241 - [LLDB] Add an llvm::Optional version of GetRegisterInfo

[David Spickett via lldb-commits]

Author: David Spickett
Date: 2022-09-26T10:54:34Z
New Revision: 0e912417c67db2dc32d32c98213dba42b9b607a6

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

LOG: [LLDB] Add an llvm::Optional version of GetRegisterInfo

We have some 500 ish uses of the bool plus ref version
so changing them all at once isn't a great idea.

This adds an overload that doesn't take a RegisterInfo&
and returns an optional.

Once I'm done switching all the existing callers I'll
remove the original function.

Benefits of optional over bool plus ref:
* The intent of the function is clear from the prototype.
* It's harder to forget to check if the return is valid,
  and if you do you'll get an assert.
* You don't hide ununsed variables, which happens because
  passing by ref marks a variable used.
* You can't forget to reset the RegisterInfo in between
  calls.

Reviewed By: clayborg

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

Added: 


Modified: 
lldb/include/lldb/Core/EmulateInstruction.h
lldb/source/Core/EmulateInstruction.cpp
lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.h
lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.h
lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp
lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.h
lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp
lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.h
lldb/source/Plugins/Instruction/PPC64/EmulateInstructionPPC64.cpp
lldb/source/Plugins/Instruction/PPC64/EmulateInstructionPPC64.h
lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.cpp
lldb/source/Plugins/Instruction/RISCV/EmulateInstructionRISCV.h

Removed: 




diff  --git a/lldb/include/lldb/Core/EmulateInstruction.h 
b/lldb/include/lldb/Core/EmulateInstruction.h
index a710c866d9803..fa049d4180fbf 100644
--- a/lldb/include/lldb/Core/EmulateInstruction.h
+++ b/lldb/include/lldb/Core/EmulateInstruction.h
@@ -375,8 +375,11 @@ class EmulateInstruction : public PluginInterface {
   virtual bool TestEmulation(Stream *out_stream, ArchSpec &arch,
  OptionValueDictionary *test_data) = 0;
 
-  virtual bool GetRegisterInfo(lldb::RegisterKind reg_kind, uint32_t reg_num,
-   RegisterInfo ®_info) = 0;
+  bool GetRegisterInfo(lldb::RegisterKind reg_kind, uint32_t reg_num,
+   RegisterInfo ®_info);
+
+  virtual llvm::Optional
+  GetRegisterInfo(lldb::RegisterKind reg_kind, uint32_t reg_num) = 0;
 
   // Optional overrides
   virtual bool SetInstruction(const Opcode &insn_opcode,

diff  --git a/lldb/source/Core/EmulateInstruction.cpp 
b/lldb/source/Core/EmulateInstruction.cpp
index 1320e8925553e..271301b9d3831 100644
--- a/lldb/source/Core/EmulateInstruction.cpp
+++ b/lldb/source/Core/EmulateInstruction.cpp
@@ -582,3 +582,12 @@ bool 
EmulateInstruction::CreateFunctionEntryUnwind(UnwindPlan &unwind_plan) {
   unwind_plan.Clear();
   return false;
 }
+
+bool EmulateInstruction::GetRegisterInfo(lldb::RegisterKind reg_kind,
+ uint32_t reg_num,
+ RegisterInfo ®_info) {
+  llvm::Optional info = GetRegisterInfo(reg_kind, reg_num);
+  if (info)
+reg_info = *info;
+  return info.has_value();
+}
\ No newline at end of file

diff  --git a/lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp 
b/lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
index 0abfefa43e099..54aec79d24773 100644
--- a/lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
+++ b/lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
@@ -42,7 +42,8 @@ LLDB_PLUGIN_DEFINE_ADV(EmulateInstructionARM, InstructionARM)
 // ITSession implementation
 //
 
-static bool GetARMDWARFRegisterInfo(unsigned reg_num, RegisterInfo ®_info) {
+static llvm::Optional GetARMDWARFRegisterInfo(unsigned reg_num) {
+  RegisterInfo reg_info;
   ::memset(®_info, 0, sizeof(RegisterInfo));
   ::memset(reg_info.kinds, LLDB_INVALID_REGNUM, sizeof(reg_info.kinds));
 
@@ -594,9 +595,9 @@ static bool GetARMDWARFRegisterInfo(unsigned reg_num, 
RegisterInfo ®_info) {
 break;
 
   default:
-return false;
+return {};
   }
-  return true;
+  return reg_info;
 }
 
 // A8.6.50
@@ -782,9 +783,9 @@ bool EmulateInstructionARM::WriteBits32Unknown(int n) {
   return true;
 }
 
-bool EmulateInstructionARM::GetRegisterInfo(lldb::RegisterKind reg_kind,
-uint32_t reg_num,
-RegisterInfo ®_info) {
+llvm::Optional
+Emulat

[Lldb-commits] [PATCH] D134581: [lldb] Prevent re-adding a module that is already loaded

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added inline comments.



Comment at: lldb/test/Shell/Target/dependent-modules-nodupe-windows.test:7
+# RUN: %clang_host -g0 -O0 -shared %S/Inputs/shlib.c -o %t.shlib.dll
+# RUN: %clang_host -g0 -O0 %S/Inputs/main.c %t.shlib.dll -o %t.main.exe
+# RUN: %lldb -b -o "#before" -o "target modules list" -o "b main" -o run \

alvinhochun wrote:
> mstorsjo wrote:
> > alvinhochun wrote:
> > > mstorsjo wrote:
> > > > alvinhochun wrote:
> > > > > mstorsjo wrote:
> > > > > > mstorsjo wrote:
> > > > > > > Unfortunately, this aspect of the test doesn't work as is on 
> > > > > > > Windows.
> > > > > > > 
> > > > > > > By default (in msvc builds), clang invokes link.exe here, but 
> > > > > > > link.exe can't take `%t.shlib.dll` as input file to the linker, 
> > > > > > > as it requires an import library.
> > > > > > > 
> > > > > > > If we would assume that lld is available, we could run the 
> > > > > > > linking command with `-fuse-ld=lld` - however that on its own 
> > > > > > > isn't enough, as lld only allows linking against DLLs when run 
> > > > > > > with the `-lldmingw` flag. We can add `-fuse-ld=lld 
> > > > > > > -Wl,-lldmingw` here to fix that, but that would break testing in 
> > > > > > > mingw environments, as `-lldmingw` isn't a valid option on the 
> > > > > > > mingw lld driver.
> > > > > > > 
> > > > > > > Likewise, we could create a proper import library by adding 
> > > > > > > `-Wl,-implib:%t.shlib.lib` on the first command above, but that 
> > > > > > > doesn't work in mingw mode either, as it would have to be 
> > > > > > > `-Wl,--out-implib=%t.shlib.lib` instead.
> > > > > > > 
> > > > > > > In practice, running the lldb tests in mingw mode is not entirely 
> > > > > > > supported, while they do pass cleanly in MSVC mode (and there's a 
> > > > > > > buildbot testing this configuration) - but I would like to work 
> > > > > > > towards making things work better in the mingw configuration too.
> > > > > > > 
> > > > > > > There's a different substitution, `%build`, which invokes the 
> > > > > > > `lldb/test/Shell/helpers/build.py` script, which abstracts a 
> > > > > > > bunch of boilerplate details mostly relevant for windows targets 
> > > > > > > (like creating PDB debug info files); the easiest at this point 
> > > > > > > would probably be to extend that script with options for creating 
> > > > > > > import libraries.
> > > > > > > 
> > > > > > > For testing with mingw, I'm currently using this out of tree 
> > > > > > > patch for that script too:
> > > > > > > ```
> > > > > > > diff --git a/lldb/test/Shell/helper/build.py 
> > > > > > > b/lldb/test/Shell/helper/build.py
> > > > > > > index 96684b7b3e66..f138b00bee9e 100755
> > > > > > > --- a/lldb/test/Shell/helper/build.py
> > > > > > > +++ b/lldb/test/Shell/helper/build.py
> > > > > > > @@ -191,7 +191,10 @@ def find_toolchain(compiler, tools_dir):
> > > > > > >  if compiler == 'any':
> > > > > > >  priorities = []
> > > > > > >  if sys.platform == 'win32':
> > > > > > > -priorities = ['clang-cl', 'msvc', 'clang', 'gcc']
> > > > > > > +if 'gcc' in sys.version.lower():
> > > > > > > +priorities = ['clang', 'gcc', 'clang-cl', 'msvc']
> > > > > > > +else:
> > > > > > > +priorities = ['clang-cl', 'msvc', 'clang', 'gcc']
> > > > > > >  else:
> > > > > > >  priorities = ['clang', 'gcc', 'clang-cl']
> > > > > > >  for toolchain in priorities:
> > > > > > > ```
> > > > > > > (This is a bit hacky, since it uses the build type of the python 
> > > > > > > interpreter to switch the preference between clang-cl and clang.)
> > > > > > > 
> > > > > > > Alternatively, we could maybe add a separate substitution that 
> > > > > > > expands into either `-implib:` or `--out-implib=`, but I'm not 
> > > > > > > sure if we can guess which one will be the right one either, 
> > > > > > > while the build.py helper script probably can do better.
> > > > > > I forgot to mention; see https://reviews.llvm.org/D129455 for some 
> > > > > > earlier discussion about other aspects (symbol table vs dwarf vs 
> > > > > > PDB) for windows testcases in lldb.
> > > > > Re different options on msvc vs mingw, how about using the `%if` 
> > > > > substitution (https://www.llvm.org/docs/TestingGuide.html)? We can 
> > > > > use it to check for the `windows-msvc` feature to apply options 
> > > > > conditionally.
> > > > Oh, I guess that would work too - it does litter the individual tests 
> > > > with the conditions instead of hiding it in `build.py`, but probably is 
> > > > acceptable too.
> > > The commands should become this:
> > > 
> > > ```
> > > # RUN: %clang_host -g0 -O0 -shared %S/Inputs/shlib.c -o %t.shlib.dll %if 
> > > windows-msvc %{-Wl,-implib:%t.shlib.lib}
> > > # RUN: %clang_host -g0 -O0 %S/Inputs/main.c -o %t.main.exe %if 
> > > windows-msvc %{%t.shlib.lib} %else %{%t.shlib.dll}
> > > ```
> > This form wor

[Lldb-commits] [PATCH] D134581: [lldb] Prevent re-adding a module that is already loaded

[Alvin Wong via Phabricator via lldb-commits]

alvinhochun updated this revision to Diff 462858.
alvinhochun added a comment.

Updated test, thanks to @mstorsjo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134581

Files:
  lldb/source/Target/Target.cpp
  lldb/test/Shell/Target/Inputs/main.c
  lldb/test/Shell/Target/Inputs/shlib.c
  lldb/test/Shell/Target/dependent-modules-nodupe-windows.test


Index: lldb/test/Shell/Target/dependent-modules-nodupe-windows.test
===
--- /dev/null
+++ lldb/test/Shell/Target/dependent-modules-nodupe-windows.test
@@ -0,0 +1,22 @@
+# REQUIRES: system-windows
+
+# Checks that dependent modules preloaded by LLDB are not duplicated when the
+# process actually loads the DLL.
+
+# RUN: %clang_host -g0 -O0 -shared %S/Inputs/shlib.c -o %t.shlib.dll \
+# RUN: %if windows-msvc %{-Wl,-implib:%t.shlib.lib} \
+# RUN: %else %{-Wl,--out-implib=%t.shlib.lib%}
+# RUN: %clang_host -g0 -O0 %S/Inputs/main.c %t.shlib.lib -o %t.main.exe
+# RUN: %lldb -b -o "#before" -o "target modules list" -o "b main" -o run \
+# RUN:   -o "#after" -o "target modules list" %t.main.exe | FileCheck %s
+
+# CHECK-LABEL: #before
+# CHECK-NEXT: target modules list
+# CHECK-NEXT: .main.exe
+# CHECK-NEXT: .shlib.dll
+
+# CHECK-LABEL: #after
+# CHECK-NEXT: target modules list
+# CHECK-NEXT: .main.exe
+# CHECK-NEXT: .shlib.dll
+# CHECK-NOT:  .shlib.dll
Index: lldb/test/Shell/Target/Inputs/shlib.c
===
--- /dev/null
+++ lldb/test/Shell/Target/Inputs/shlib.c
@@ -0,0 +1 @@
+__declspec(dllexport) void exportFunc(void) {}
Index: lldb/test/Shell/Target/Inputs/main.c
===
--- /dev/null
+++ lldb/test/Shell/Target/Inputs/main.c
@@ -0,0 +1,2 @@
+__declspec(dllimport) void exportFunc(void);
+int main() { exportFunc(); }
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -2134,6 +2134,14 @@
   }
 }
 
+// If we found a pre-existing module, just return it.
+if (module_sp && !did_create_module &&
+m_images.FindModule(module_sp.get())) {
+  if (error_ptr)
+*error_ptr = error;
+  return module_sp;
+}
+
 // We found a module that wasn't in our target list.  Let's make sure that
 // there wasn't an equivalent module in the list already, and if there was,
 // let's remove it.


Index: lldb/test/Shell/Target/dependent-modules-nodupe-windows.test
===
--- /dev/null
+++ lldb/test/Shell/Target/dependent-modules-nodupe-windows.test
@@ -0,0 +1,22 @@
+# REQUIRES: system-windows
+
+# Checks that dependent modules preloaded by LLDB are not duplicated when the
+# process actually loads the DLL.
+
+# RUN: %clang_host -g0 -O0 -shared %S/Inputs/shlib.c -o %t.shlib.dll \
+# RUN: %if windows-msvc %{-Wl,-implib:%t.shlib.lib} \
+# RUN: %else %{-Wl,--out-implib=%t.shlib.lib%}
+# RUN: %clang_host -g0 -O0 %S/Inputs/main.c %t.shlib.lib -o %t.main.exe
+# RUN: %lldb -b -o "#before" -o "target modules list" -o "b main" -o run \
+# RUN:   -o "#after" -o "target modules list" %t.main.exe | FileCheck %s
+
+# CHECK-LABEL: #before
+# CHECK-NEXT: target modules list
+# CHECK-NEXT: .main.exe
+# CHECK-NEXT: .shlib.dll
+
+# CHECK-LABEL: #after
+# CHECK-NEXT: target modules list
+# CHECK-NEXT: .main.exe
+# CHECK-NEXT: .shlib.dll
+# CHECK-NOT:  .shlib.dll
Index: lldb/test/Shell/Target/Inputs/shlib.c
===
--- /dev/null
+++ lldb/test/Shell/Target/Inputs/shlib.c
@@ -0,0 +1 @@
+__declspec(dllexport) void exportFunc(void) {}
Index: lldb/test/Shell/Target/Inputs/main.c
===
--- /dev/null
+++ lldb/test/Shell/Target/Inputs/main.c
@@ -0,0 +1,2 @@
+__declspec(dllimport) void exportFunc(void);
+int main() { exportFunc(); }
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -2134,6 +2134,14 @@
   }
 }
 
+// If we found a pre-existing module, just return it.
+if (module_sp && !did_create_module &&
+m_images.FindModule(module_sp.get())) {
+  if (error_ptr)
+*error_ptr = error;
+  return module_sp;
+}
+
 // We found a module that wasn't in our target list.  Let's make sure that
 // there wasn't an equivalent module in the list already, and if there was,
 // let's remove it.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D134581: [lldb] Prevent re-adding a module that is already loaded

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added inline comments.



Comment at: lldb/test/Shell/Target/dependent-modules-nodupe-windows.test:7
+# RUN: %clang_host -g0 -O0 -shared %S/Inputs/shlib.c -o %t.shlib.dll \
+# RUN: %if windows-msvc %{-Wl,-implib:%t.shlib.lib} \
+# RUN: %else %{-Wl,--out-implib=%t.shlib.lib%}

This closing `}` is lacking the preceding `%`, i.e. it should be `%}`, like on 
the second row.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134581

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

[Lldb-commits] [PATCH] D134581: [lldb] Prevent re-adding a module that is already loaded

[Alvin Wong via Phabricator via lldb-commits]

alvinhochun updated this revision to Diff 462860.
alvinhochun added a comment.

Fixed test again


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134581

Files:
  lldb/source/Target/Target.cpp
  lldb/test/Shell/Target/Inputs/main.c
  lldb/test/Shell/Target/Inputs/shlib.c
  lldb/test/Shell/Target/dependent-modules-nodupe-windows.test


Index: lldb/test/Shell/Target/dependent-modules-nodupe-windows.test
===
--- /dev/null
+++ lldb/test/Shell/Target/dependent-modules-nodupe-windows.test
@@ -0,0 +1,22 @@
+# REQUIRES: system-windows
+
+# Checks that dependent modules preloaded by LLDB are not duplicated when the
+# process actually loads the DLL.
+
+# RUN: %clang_host -g0 -O0 -shared %S/Inputs/shlib.c -o %t.shlib.dll \
+# RUN: %if windows-msvc %{-Wl,-implib:%t.shlib.lib%} \
+# RUN: %else %{-Wl,--out-implib=%t.shlib.lib%}
+# RUN: %clang_host -g0 -O0 %S/Inputs/main.c %t.shlib.lib -o %t.main.exe
+# RUN: %lldb -b -o "#before" -o "target modules list" -o "b main" -o run \
+# RUN:   -o "#after" -o "target modules list" %t.main.exe | FileCheck %s
+
+# CHECK-LABEL: #before
+# CHECK-NEXT: target modules list
+# CHECK-NEXT: .main.exe
+# CHECK-NEXT: .shlib.dll
+
+# CHECK-LABEL: #after
+# CHECK-NEXT: target modules list
+# CHECK-NEXT: .main.exe
+# CHECK-NEXT: .shlib.dll
+# CHECK-NOT:  .shlib.dll
Index: lldb/test/Shell/Target/Inputs/shlib.c
===
--- /dev/null
+++ lldb/test/Shell/Target/Inputs/shlib.c
@@ -0,0 +1 @@
+__declspec(dllexport) void exportFunc(void) {}
Index: lldb/test/Shell/Target/Inputs/main.c
===
--- /dev/null
+++ lldb/test/Shell/Target/Inputs/main.c
@@ -0,0 +1,2 @@
+__declspec(dllimport) void exportFunc(void);
+int main() { exportFunc(); }
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -2134,6 +2134,14 @@
   }
 }
 
+// If we found a pre-existing module, just return it.
+if (module_sp && !did_create_module &&
+m_images.FindModule(module_sp.get())) {
+  if (error_ptr)
+*error_ptr = error;
+  return module_sp;
+}
+
 // We found a module that wasn't in our target list.  Let's make sure that
 // there wasn't an equivalent module in the list already, and if there was,
 // let's remove it.


Index: lldb/test/Shell/Target/dependent-modules-nodupe-windows.test
===
--- /dev/null
+++ lldb/test/Shell/Target/dependent-modules-nodupe-windows.test
@@ -0,0 +1,22 @@
+# REQUIRES: system-windows
+
+# Checks that dependent modules preloaded by LLDB are not duplicated when the
+# process actually loads the DLL.
+
+# RUN: %clang_host -g0 -O0 -shared %S/Inputs/shlib.c -o %t.shlib.dll \
+# RUN: %if windows-msvc %{-Wl,-implib:%t.shlib.lib%} \
+# RUN: %else %{-Wl,--out-implib=%t.shlib.lib%}
+# RUN: %clang_host -g0 -O0 %S/Inputs/main.c %t.shlib.lib -o %t.main.exe
+# RUN: %lldb -b -o "#before" -o "target modules list" -o "b main" -o run \
+# RUN:   -o "#after" -o "target modules list" %t.main.exe | FileCheck %s
+
+# CHECK-LABEL: #before
+# CHECK-NEXT: target modules list
+# CHECK-NEXT: .main.exe
+# CHECK-NEXT: .shlib.dll
+
+# CHECK-LABEL: #after
+# CHECK-NEXT: target modules list
+# CHECK-NEXT: .main.exe
+# CHECK-NEXT: .shlib.dll
+# CHECK-NOT:  .shlib.dll
Index: lldb/test/Shell/Target/Inputs/shlib.c
===
--- /dev/null
+++ lldb/test/Shell/Target/Inputs/shlib.c
@@ -0,0 +1 @@
+__declspec(dllexport) void exportFunc(void) {}
Index: lldb/test/Shell/Target/Inputs/main.c
===
--- /dev/null
+++ lldb/test/Shell/Target/Inputs/main.c
@@ -0,0 +1,2 @@
+__declspec(dllimport) void exportFunc(void);
+int main() { exportFunc(); }
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -2134,6 +2134,14 @@
   }
 }
 
+// If we found a pre-existing module, just return it.
+if (module_sp && !did_create_module &&
+m_images.FindModule(module_sp.get())) {
+  if (error_ptr)
+*error_ptr = error;
+  return module_sp;
+}
+
 // We found a module that wasn't in our target list.  Let's make sure that
 // there wasn't an equivalent module in the list already, and if there was,
 // let's remove it.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D134636: [lldb][Windows] Always call SetExecutableModule on debugger connected

[Alvin Wong via Phabricator via lldb-commits]

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

In `ProcessWindows::OnDebuggerConnected` (triggered from
`CREATE_PROCESS_DEBUG_EVENT`), we should always call
`Target::SetExecutableModule` regardless of whether LLDB has already
preloaded the executable modules. `SetExecutableModule` has the side
effect of clearing the module list of the Target, which help make sure
that module #0 is the executable module and the rest of the modules are
listed according to the DLL load order in the process (technically this
has no real consequences but it seems to make more sense anyway.)

Depends on D134581 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134636

Files:
  lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
  lldb/test/Shell/Target/dependent-modules-nodupe-windows.test


Index: lldb/test/Shell/Target/dependent-modules-nodupe-windows.test
===
--- lldb/test/Shell/Target/dependent-modules-nodupe-windows.test
+++ lldb/test/Shell/Target/dependent-modules-nodupe-windows.test
@@ -18,5 +18,7 @@
 # CHECK-LABEL: #after
 # CHECK-NEXT: target modules list
 # CHECK-NEXT: .main.exe
-# CHECK-NEXT: .shlib.dll
+# CHECK-NEXT: ntdll.dll
+# CHECK-NEXT: kernel32.dll
+# CHECK:  .shlib.dll
 # CHECK-NOT:  .shlib.dll
Index: lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
===
--- lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
+++ lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
@@ -653,28 +653,26 @@
   LLDB_LOG(log, "Debugger connected to process {0}.  Image base = {1:x}",
debugger->GetProcess().GetProcessId(), image_base);
 
-  ModuleSP module = GetTarget().GetExecutableModule();
-  if (!module) {
-// During attach, we won't have the executable module, so find it now.
-const DWORD pid = debugger->GetProcess().GetProcessId();
-const std::string file_name = GetProcessExecutableName(pid);
-if (file_name.empty()) {
-  return;
-}
-
-FileSpec executable_file(file_name);
-FileSystem::Instance().Resolve(executable_file);
-ModuleSpec module_spec(executable_file);
-Status error;
-module =
-GetTarget().GetOrCreateModule(module_spec, true /* notify */, &error);
-if (!module) {
-  return;
-}
+  ModuleSP module;
+  // During attach, we won't have the executable module, so find it now.
+  const DWORD pid = debugger->GetProcess().GetProcessId();
+  const std::string file_name = GetProcessExecutableName(pid);
+  if (file_name.empty()) {
+return;
+  }
 
-GetTarget().SetExecutableModule(module, eLoadDependentsNo);
+  FileSpec executable_file(file_name);
+  FileSystem::Instance().Resolve(executable_file);
+  ModuleSpec module_spec(executable_file);
+  Status error;
+  module =
+  GetTarget().GetOrCreateModule(module_spec, true /* notify */, &error);
+  if (!module) {
+return;
   }
 
+  GetTarget().SetExecutableModule(module, eLoadDependentsNo);
+
   if (auto dyld = GetDynamicLoader())
 dyld->OnLoadModule(module, ModuleSpec(), image_base);
 


Index: lldb/test/Shell/Target/dependent-modules-nodupe-windows.test
===
--- lldb/test/Shell/Target/dependent-modules-nodupe-windows.test
+++ lldb/test/Shell/Target/dependent-modules-nodupe-windows.test
@@ -18,5 +18,7 @@
 # CHECK-LABEL: #after
 # CHECK-NEXT: target modules list
 # CHECK-NEXT: .main.exe
-# CHECK-NEXT: .shlib.dll
+# CHECK-NEXT: ntdll.dll
+# CHECK-NEXT: kernel32.dll
+# CHECK:  .shlib.dll
 # CHECK-NOT:  .shlib.dll
Index: lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
===
--- lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
+++ lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
@@ -653,28 +653,26 @@
   LLDB_LOG(log, "Debugger connected to process {0}.  Image base = {1:x}",
debugger->GetProcess().GetProcessId(), image_base);
 
-  ModuleSP module = GetTarget().GetExecutableModule();
-  if (!module) {
-// During attach, we won't have the executable module, so find it now.
-const DWORD pid = debugger->GetProcess().GetProcessId();
-const std::string file_name = GetProcessExecutableName(pid);
-if (file_name.empty()) {
-  return;
-}
-
-FileSpec executable_file(file_name);
-FileSystem::Instance().Resolve(executable_file);
-ModuleSpec module_spec(executable_file);
-Status error;
-module =
-GetTarget().GetOrCreateModule(module_spec, true /* notify */, &error);
-if (!module) {
-  return;
-}
+  ModuleSP module;
+  // During attach, we won't have the executable module, so find it now.
+  const DWORD pid = debugger-

[Lldb-commits] [PATCH] D134581: [lldb] Prevent re-adding a module that is already loaded

[Alvin Wong via Phabricator via lldb-commits]

alvinhochun added a comment.

In D134581#3813757 , @alvinhochun 
wrote:

> In D134581#3813484 , @jasonmolenda 
> wrote:
>
>> I probably misunderstand the situation DynamicLoaderWindows finds itself in, 
>> but in DynamicLoaderDarwin we have similar behavior - you run 'lldb binary' 
>> and lldb loads all the directly linked dynamic libraries into the target it 
>> creates, pre-execution.  When execution starts, the dynamic linker tells us 
>> where the binary is loaded in memory and we call Target::SetExecutableModule 
>> with it.  Target::SetExecutableModule has a side effect of clearing the 
>> Target's module list - you now have only one binary in the Target, your 
>> executable module.  (this is done so the 0th image in the image list is your 
>> executable module)
>>
>> Why aren't your pre-loaded DLLs unloaded when you call 
>> Target::SetExecutableModule?
>
> In `ProcessWindows::OnDebuggerConnected`, it first checks 
> `GetTarget().GetExecutableModule()`. Only when the returned module is null 
> (i.e. the binary hasn't already been loaded) then it calls 
> `GetTarget().SetExecutableModule(module, eLoadDependentsNo)`. If I understood 
> you correctly, then the right thing to do there should be to call 
> `GetTarget().SetExecutableModule(module, eLoadDependentsNo)` in all cases, 
> without checking `GetExecutableModule`, right?
>
> It seems to make sense, but I may need some comments from other reviewers on 
> this.

I made a follow-up patch D134636  for this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134581

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

[Lldb-commits] [PATCH] D134636: [lldb][Windows] Always call SetExecutableModule on debugger connected

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a reviewer: jasonmolenda.
mstorsjo added a subscriber: jasonmolenda.
mstorsjo added a comment.

The test runs fine on Windows, but I'm not familiar with these aspects of lldb 
to (yet) have a good opinion on it; adding @jasonmolenda to the review who 
commented on the previous one too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134636

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

[Lldb-commits] [PATCH] D134493: [lldb][TypeSystemClang] Deduce lldb::eEncodingUint for unsigned enum types

[Andy Yankovsky via Phabricator via lldb-commits]

werat added a comment.

In D134493#3811253 , @labath wrote:

> lol, I knew about the last two parts (not that I agree with them, but I think 
> we have about an equal amount of code which relies on it, and that which 
> tries to work around it), but the first one is really weird. I think we have 
> invented a middle ground between sign- and zero-extension.

haha, so this mean no chance of fixing this? I have a workaround for this as 
well -- 
https://github.com/google/lldb-eval/blob/04a73616c012c3dac7fb11206511bd2a9fe16db4/lldb-eval/value.cc#L146
I can live with that, but current behaviour does look like a bug.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134493

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

[Lldb-commits] [PATCH] D134493: [lldb][TypeSystemClang] Deduce lldb::eEncodingUint for unsigned enum types

[Michael Buch via Phabricator via lldb-commits]

Michael137 added a comment.

In D134493#3814761 , @werat wrote:

> In D134493#3811253 , @labath wrote:
>
>> lol, I knew about the last two parts (not that I agree with them, but I 
>> think we have about an equal amount of code which relies on it, and that 
>> which tries to work around it), but the first one is really weird. I think 
>> we have invented a middle ground between sign- and zero-extension.
>
> haha, so this mean no chance of fixing this? I have a workaround for this as 
> well -- 
> https://github.com/google/lldb-eval/blob/04a73616c012c3dac7fb11206511bd2a9fe16db4/lldb-eval/value.cc#L146
> I can live with that, but current behaviour does look like a bug.

That does look like a bug, thanks for reporting. Was going to open an issue and 
take a look at it this week


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134493

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

[Lldb-commits] [PATCH] D134642: Propagate FORK events back to client

[Iryna Shakhova via Phabricator via lldb-commits]

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

We want to allow the client to detect when the process forks and to attach the 
debugger to a newly created process. For this we propose to implement two 
options in LLDB*: to propagate fork/vfork events to the client and to 
automatically detach the child/parent in a _stopped_ state:

- stop-on-clone-events (**false**/true);
- detach-keeps-stopped (**false**/true).

The client (IDE in our case) can then decide to attach another debugger to that 
process.

- By default these options will be disabled, so the default behavior won't 
change.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134642

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
  lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.h
  lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
  lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h
  lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
  lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/StopInfo.cpp
  lldb/source/Target/TargetProperties.td
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/test/API/linux/clone-events-notification/Makefile
  
lldb/test/API/linux/clone-events-notification/TestCloneEventSentAndChildStopped.py
  lldb/test/API/linux/clone-events-notification/main.cpp
  lldb/test/Shell/Subprocess/clone-stops-on-fork
  lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
  lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h

Index: lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h
===
--- lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h
+++ lldb/unittests/TestingSupport/Host/NativeProcessTestUtils.h
@@ -47,7 +47,7 @@
 
   MOCK_METHOD1(Resume, Status(const ResumeActionList &ResumeActions));
   MOCK_METHOD0(Halt, Status());
-  MOCK_METHOD0(Detach, Status());
+  MOCK_METHOD1(Detach, Status(bool KeepStopped));
   MOCK_METHOD1(Signal, Status(int Signo));
   MOCK_METHOD0(Kill, Status());
   MOCK_METHOD0(GetSharedLibraryInfoAddress, addr_t());
Index: lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
===
--- lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -591,3 +591,22 @@
  std::vector{0x99}, "QMemTags:456789,0:8000:99",
  "E03", false);
 }
+
+TEST_F(GDBRemoteCommunicationClientTest, DetachAndKeepStopped) {
+  llvm::Triple triple("i386-pc-linux");
+  const lldb::tid_t tid = 0x18d915;
+  std::future async_result = std::async(
+  std::launch::async, [&] { return client.Detach(true, tid); });
+  HandlePacket(server, "qSupportsDetachAndStayStopped:", "OK");
+  HandlePacket(server, "D1;0018d915", "OK");
+  EXPECT_TRUE(async_result.get().Success());
+}
+
+TEST_F(GDBRemoteCommunicationClientTest, DetachAndContinue) {
+  llvm::Triple triple("i386-pc-linux");
+  const lldb::tid_t tid = 0x18d915;
+  std::future async_result = std::async(
+  std::launch::async, [&] { return client.Detach(false, tid); });
+  HandlePacket(server, "D;0018d915", "OK");
+  EXPECT_TRUE(async_result.get().Success());
+}
Index: lldb/test/Shell/Subprocess/clone-stops-on-fork
===
--- /dev/null
+++ lldb/test/Shell/Subprocess/clone-stops-on-fork
@@ -0,0 +1,10 @@
+# REQUIRES: native && system-linux
+# RUN: %clangxx_host -g %p/Inputs/fork.cpp -DTEST_CLONE -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+settings set target.process.stop-on-clone-events true
+process launch -s
+continue
+# CHECK: stop reason = fork
+continue
+# CHECK: function run in parent
+# CHECK: function run in exec'd child
\ No newline at end of file
Index: lldb/test/API/linux/clone-events-notification/main.cpp
===
--- /dev/null
+++ lldb/test/API/linux/clone-events-notification/main.cpp
@@ -0,0 +1,90 @@
+/* This sample is a modified version of the example in
+https://linux.die.net/man/2/waitpid. The expected output
+when running in the shell is:
+(lldb) 

[Lldb-commits] [PATCH] D134645: [lldb] Skip tests incompatible with older versions of Clang

[Felipe de Azevedo Piovezan via Phabricator via lldb-commits]

fdeazeve created this revision.
fdeazeve added a reviewer: aprantl.
Herald added a project: All.
fdeazeve requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

The coroutine tests require a standard library implementation of
coroutines, which was only made available some time _after_ Clang 13.
The first such Clang tested by the LLDB matrix bot is 15.0.1

The TestObjCExceptions test forces the use of the system's libcxx. For
the lldb matrix bot, the first Clang version compatible with the bot's
libraries is 13.0.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134645

Files:
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
  lldb/test/API/lang/objc/exceptions/TestObjCExceptions.py


Index: lldb/test/API/lang/objc/exceptions/TestObjCExceptions.py
===
--- lldb/test/API/lang/objc/exceptions/TestObjCExceptions.py
+++ lldb/test/API/lang/objc/exceptions/TestObjCExceptions.py
@@ -13,6 +13,7 @@
 
 class ObjCExceptionsTestCase(TestBase):
 
+@skipIf(compiler="clang", compiler_version=['<', '13.0'])
 def test_objc_exceptions_at_throw(self):
 self.build()
 
@@ -123,6 +124,7 @@
 for n in ["objc_exception_throw", "foo(int)", "main"]:
 self.assertIn(n, names, "%s is in the exception backtrace (%s)" % 
(n, names))
 
+@skipIf(compiler="clang", compiler_version=['<', '13.0'])
 def test_objc_exceptions_at_abort(self):
 self.build()
 
@@ -179,6 +181,7 @@
 for n in ["objc_exception_throw", "foo(int)", "rethrow(int)", "main"]:
 self.assertEqual(len([f for f in history_thread.frames if 
f.GetFunctionName() == n]), 1)
 
+@skipIf(compiler="clang", compiler_version=['<', '13.0'])
 def test_cxx_exceptions_at_abort(self):
 self.build()
 
Index: 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
===
--- 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
+++ 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
@@ -79,5 +79,6 @@
 self.do_test(USE_LIBSTDCPP)
 
 @add_test_categories(["libc++"])
+@skipIf(compiler="clang", compiler_version=['<', '15.0'])
 def test_libcpp(self):
 self.do_test(USE_LIBCPP)


Index: lldb/test/API/lang/objc/exceptions/TestObjCExceptions.py
===
--- lldb/test/API/lang/objc/exceptions/TestObjCExceptions.py
+++ lldb/test/API/lang/objc/exceptions/TestObjCExceptions.py
@@ -13,6 +13,7 @@
 
 class ObjCExceptionsTestCase(TestBase):
 
+@skipIf(compiler="clang", compiler_version=['<', '13.0'])
 def test_objc_exceptions_at_throw(self):
 self.build()
 
@@ -123,6 +124,7 @@
 for n in ["objc_exception_throw", "foo(int)", "main"]:
 self.assertIn(n, names, "%s is in the exception backtrace (%s)" % (n, names))
 
+@skipIf(compiler="clang", compiler_version=['<', '13.0'])
 def test_objc_exceptions_at_abort(self):
 self.build()
 
@@ -179,6 +181,7 @@
 for n in ["objc_exception_throw", "foo(int)", "rethrow(int)", "main"]:
 self.assertEqual(len([f for f in history_thread.frames if f.GetFunctionName() == n]), 1)
 
+@skipIf(compiler="clang", compiler_version=['<', '13.0'])
 def test_cxx_exceptions_at_abort(self):
 self.build()
 
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
@@ -79,5 +79,6 @@
 self.do_test(USE_LIBSTDCPP)
 
 @add_test_categories(["libc++"])
+@skipIf(compiler="clang", compiler_version=['<', '15.0'])
 def test_libcpp(self):
 self.do_test(USE_LIBCPP)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D134037: [lldb] Fix CommandInterpreter::DidProcessStopAbnormally() with multiple threads

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> I might add the AArch64 and Arm equivalent if I have some spare time.

I looked into this and there's a detail that makes this work on x86 only. So 
I'll leave it as is, it's getting tested somewhere, and that's what matters.

If you're curious this is the spanner in the works:

  size_t NativeProcessProtocol::GetSoftwareBreakpointPCOffset() {
switch (GetArchitecture().GetMachine()) {
case llvm::Triple::x86:
case llvm::Triple::x86_64:
case llvm::Triple::systemz:
  // These architectures report increment the PC after breakpoint is hit.
  return cantFail(GetSoftwareBreakpointTrapOpcode(0)).size();

Meaning AArch64 doesn't. lldb on x86 can continue past a break in the program 
because of this:

  (lldb) run
  Process 31032 launched: '/tmp/test.o' (x86_64)
  Process 31032 stopped
  * thread #1, name = 'test.o', stop reason = signal SIGTRAP
  frame #0: 0x460f test.o`main at test.c:3
 1int main() {
 2__asm__ __volatile__("int3");
  -> 3  return 0;
 4}
  (lldb) c
  Process 31032 resuming
  Process 31032 exited with status = 0 (0x)

On AArch64 we just stick on the break:

  (lldb) run
  Process 2162869 launched: '/tmp/test.o' (aarch64)
  Process 2162869 stopped
  * thread #1, name = 'test.o', stop reason = signal SIGTRAP
  frame #0: 0xa71c test.o`main at test.c:2:3
 1int main() {
  -> 2  asm volatile("brk #0xf000\n\t");
 3  return 0;
 4}
  (lldb) c
  Process 2162869 resuming
  Process 2162869 stopped
  * thread #1, name = 'test.o', stop reason = signal SIGTRAP
  frame #0: 0xa71c test.o`main at test.c:2:3
 1int main() {
  -> 2  asm volatile("brk #0xf000\n\t");
 3  return 0;
 4}

gdb has the same behaviour on AArch64 so I'll leave it as is until someone 
complains.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134037

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

[Lldb-commits] [PATCH] D134574: [lldb][test] 2 - Add gmodules test category explicitly where previously done implicitly

[Adrian Prantl via Phabricator via lldb-commits]

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

Looks good!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134574

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

[Lldb-commits] [PATCH] D134656: [LLDB][NativePDB] Add class/union layout bit size.

[Zequan Wu via Phabricator via lldb-commits]

zequanwu created this revision.
zequanwu added reviewers: labath, rnk.
Herald added a project: All.
zequanwu requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Missing it causes lldb to crash or give incorrect result.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134656

Files:
  lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
  lldb/test/Shell/SymbolFile/NativePDB/packed_class_layout.cpp


Index: lldb/test/Shell/SymbolFile/NativePDB/packed_class_layout.cpp
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/NativePDB/packed_class_layout.cpp
@@ -0,0 +1,34 @@
+// clang-format off
+// REQUIRES: lld, x86
+
+// Make sure class layout is correct.
+// RUN: %clang_cl --target=x86_64-windows-msvc -Od -Z7 -c /Fo%t.obj -- %s
+// RUN: lld-link -debug:full -nodefaultlib -entry:main %t.obj -out:%t.exe 
-pdb:%t.pdb
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe \
+// RUN: -o "expr a" -o "expr u.c" -o "expr u.i" -o "exit" | FileCheck %s
+
+// CHECK:  (lldb) expr a
+// CHECK-NEXT: (A) $0 = (d1 = 'a', d2 = 1, d3 = 2, d4 = 'b')
+// CHECK-NEXT: (lldb) expr u.c
+// CHECK-NEXT: (char[3]) $1 = "a"
+// CHECK-NEXT: (lldb) expr u.i
+// CHECK-NEXT: (int) $2 = 97
+
+struct __attribute__((packed, aligned(1))) A {
+  char d1;
+  int d2;
+  int d3;
+  char d4;
+};
+
+union __attribute__((packed, aligned(1))) U {
+  char c[2];
+  int i;
+};
+
+A a = {'a', 1, 2, 'b'};
+U u = {"a"};
+
+int main() {
+  return 0;
+}
Index: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
===
--- lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
+++ lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
@@ -45,10 +45,12 @@
 break;
   case LF_UNION:
 llvm::cantFail(TypeDeserializer::deserializeAs(cvt, 
m_cvr.ur));
+ m_layout.bit_size = m_cvr.ur.getSize() * 8;
 break;
   case LF_CLASS:
   case LF_STRUCTURE:
 llvm::cantFail(TypeDeserializer::deserializeAs(cvt, 
m_cvr.cr));
+ m_layout.bit_size = m_cvr.cr.getSize() * 8;
 break;
   default:
 llvm_unreachable("unreachable!");


Index: lldb/test/Shell/SymbolFile/NativePDB/packed_class_layout.cpp
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/NativePDB/packed_class_layout.cpp
@@ -0,0 +1,34 @@
+// clang-format off
+// REQUIRES: lld, x86
+
+// Make sure class layout is correct.
+// RUN: %clang_cl --target=x86_64-windows-msvc -Od -Z7 -c /Fo%t.obj -- %s
+// RUN: lld-link -debug:full -nodefaultlib -entry:main %t.obj -out:%t.exe -pdb:%t.pdb
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe \
+// RUN: -o "expr a" -o "expr u.c" -o "expr u.i" -o "exit" | FileCheck %s
+
+// CHECK:  (lldb) expr a
+// CHECK-NEXT: (A) $0 = (d1 = 'a', d2 = 1, d3 = 2, d4 = 'b')
+// CHECK-NEXT: (lldb) expr u.c
+// CHECK-NEXT: (char[3]) $1 = "a"
+// CHECK-NEXT: (lldb) expr u.i
+// CHECK-NEXT: (int) $2 = 97
+
+struct __attribute__((packed, aligned(1))) A {
+  char d1;
+  int d2;
+  int d3;
+  char d4;
+};
+
+union __attribute__((packed, aligned(1))) U {
+  char c[2];
+  int i;
+};
+
+A a = {'a', 1, 2, 'b'};
+U u = {"a"};
+
+int main() {
+  return 0;
+}
Index: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
===
--- lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
+++ lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
@@ -45,10 +45,12 @@
 break;
   case LF_UNION:
 llvm::cantFail(TypeDeserializer::deserializeAs(cvt, m_cvr.ur));
+ m_layout.bit_size = m_cvr.ur.getSize() * 8;
 break;
   case LF_CLASS:
   case LF_STRUCTURE:
 llvm::cantFail(TypeDeserializer::deserializeAs(cvt, m_cvr.cr));
+ m_layout.bit_size = m_cvr.cr.getSize() * 8;
 break;
   default:
 llvm_unreachable("unreachable!");
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 10a3563 - [lldb] Skip tests incompatible with older versions of Clang

[Felipe de Azevedo Piovezan via lldb-commits]

Author: Felipe de Azevedo Piovezan
Date: 2022-09-26T13:27:51-04:00
New Revision: 10a35632d55bb05004fe3d0c2d4432bb74897ee7

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

LOG: [lldb] Skip tests incompatible with older versions of Clang

The coroutine tests require a standard library implementation of
coroutines, which was only made available some time _after_ Clang 13.
The first such Clang tested by the LLDB matrix bot is 15.0.1

The TestObjCExceptions test forces the use of the system's libcxx. For
the lldb matrix bot, the first Clang version compatible with the bot's
libraries is 13.0.

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

Added: 


Modified: 

lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
lldb/test/API/lang/objc/exceptions/TestObjCExceptions.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
index 8288fcff6aae4..224a550a5346d 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
@@ -79,5 +79,6 @@ def test_libstdcpp(self):
 self.do_test(USE_LIBSTDCPP)
 
 @add_test_categories(["libc++"])
+@skipIf(compiler="clang", compiler_version=['<', '15.0'])
 def test_libcpp(self):
 self.do_test(USE_LIBCPP)

diff  --git a/lldb/test/API/lang/objc/exceptions/TestObjCExceptions.py 
b/lldb/test/API/lang/objc/exceptions/TestObjCExceptions.py
index 663ab11ddfd56..23d94663c1d07 100644
--- a/lldb/test/API/lang/objc/exceptions/TestObjCExceptions.py
+++ b/lldb/test/API/lang/objc/exceptions/TestObjCExceptions.py
@@ -13,6 +13,7 @@
 
 class ObjCExceptionsTestCase(TestBase):
 
+@skipIf(compiler="clang", compiler_version=['<', '13.0'])
 def test_objc_exceptions_at_throw(self):
 self.build()
 
@@ -123,6 +124,7 @@ def test_objc_exceptions_at_throw(self):
 for n in ["objc_exception_throw", "foo(int)", "main"]:
 self.assertIn(n, names, "%s is in the exception backtrace (%s)" % 
(n, names))
 
+@skipIf(compiler="clang", compiler_version=['<', '13.0'])
 def test_objc_exceptions_at_abort(self):
 self.build()
 
@@ -179,6 +181,7 @@ def test_objc_exceptions_at_abort(self):
 for n in ["objc_exception_throw", "foo(int)", "rethrow(int)", "main"]:
 self.assertEqual(len([f for f in history_thread.frames if 
f.GetFunctionName() == n]), 1)
 
+@skipIf(compiler="clang", compiler_version=['<', '13.0'])
 def test_cxx_exceptions_at_abort(self):
 self.build()
 



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

[Lldb-commits] [PATCH] D134645: [lldb] Skip tests incompatible with older versions of Clang

[Felipe de Azevedo Piovezan via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG10a35632d55b: [lldb] Skip tests incompatible with older 
versions of Clang (authored by fdeazeve).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134645

Files:
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
  lldb/test/API/lang/objc/exceptions/TestObjCExceptions.py


Index: lldb/test/API/lang/objc/exceptions/TestObjCExceptions.py
===
--- lldb/test/API/lang/objc/exceptions/TestObjCExceptions.py
+++ lldb/test/API/lang/objc/exceptions/TestObjCExceptions.py
@@ -13,6 +13,7 @@
 
 class ObjCExceptionsTestCase(TestBase):
 
+@skipIf(compiler="clang", compiler_version=['<', '13.0'])
 def test_objc_exceptions_at_throw(self):
 self.build()
 
@@ -123,6 +124,7 @@
 for n in ["objc_exception_throw", "foo(int)", "main"]:
 self.assertIn(n, names, "%s is in the exception backtrace (%s)" % 
(n, names))
 
+@skipIf(compiler="clang", compiler_version=['<', '13.0'])
 def test_objc_exceptions_at_abort(self):
 self.build()
 
@@ -179,6 +181,7 @@
 for n in ["objc_exception_throw", "foo(int)", "rethrow(int)", "main"]:
 self.assertEqual(len([f for f in history_thread.frames if 
f.GetFunctionName() == n]), 1)
 
+@skipIf(compiler="clang", compiler_version=['<', '13.0'])
 def test_cxx_exceptions_at_abort(self):
 self.build()
 
Index: 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
===
--- 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
+++ 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
@@ -79,5 +79,6 @@
 self.do_test(USE_LIBSTDCPP)
 
 @add_test_categories(["libc++"])
+@skipIf(compiler="clang", compiler_version=['<', '15.0'])
 def test_libcpp(self):
 self.do_test(USE_LIBCPP)


Index: lldb/test/API/lang/objc/exceptions/TestObjCExceptions.py
===
--- lldb/test/API/lang/objc/exceptions/TestObjCExceptions.py
+++ lldb/test/API/lang/objc/exceptions/TestObjCExceptions.py
@@ -13,6 +13,7 @@
 
 class ObjCExceptionsTestCase(TestBase):
 
+@skipIf(compiler="clang", compiler_version=['<', '13.0'])
 def test_objc_exceptions_at_throw(self):
 self.build()
 
@@ -123,6 +124,7 @@
 for n in ["objc_exception_throw", "foo(int)", "main"]:
 self.assertIn(n, names, "%s is in the exception backtrace (%s)" % (n, names))
 
+@skipIf(compiler="clang", compiler_version=['<', '13.0'])
 def test_objc_exceptions_at_abort(self):
 self.build()
 
@@ -179,6 +181,7 @@
 for n in ["objc_exception_throw", "foo(int)", "rethrow(int)", "main"]:
 self.assertEqual(len([f for f in history_thread.frames if f.GetFunctionName() == n]), 1)
 
+@skipIf(compiler="clang", compiler_version=['<', '13.0'])
 def test_cxx_exceptions_at_abort(self):
 self.build()
 
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/coroutine_handle/TestCoroutineHandle.py
@@ -79,5 +79,6 @@
 self.do_test(USE_LIBSTDCPP)
 
 @add_test_categories(["libc++"])
+@skipIf(compiler="clang", compiler_version=['<', '15.0'])
 def test_libcpp(self):
 self.do_test(USE_LIBCPP)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D134581: [lldb] Prevent re-adding a module that is already loaded

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

In D134581#3813757 , @alvinhochun 
wrote:

> Thanks for the comment.
>
> In D134581#3813484 , @jasonmolenda 
> wrote:
>
>> I probably misunderstand the situation DynamicLoaderWindows finds itself in, 
>> but in DynamicLoaderDarwin we have similar behavior - you run 'lldb binary' 
>> and lldb loads all the directly linked dynamic libraries into the target it 
>> creates, pre-execution.  When execution starts, the dynamic linker tells us 
>> where the binary is loaded in memory and we call Target::SetExecutableModule 
>> with it.  Target::SetExecutableModule has a side effect of clearing the 
>> Target's module list - you now have only one binary in the Target, your 
>> executable module.  (this is done so the 0th image in the image list is your 
>> executable module)
>>
>> Why aren't your pre-loaded DLLs unloaded when you call 
>> Target::SetExecutableModule?
>
> In `ProcessWindows::OnDebuggerConnected`, it first checks 
> `GetTarget().GetExecutableModule()`. Only when the returned module is null 
> (i.e. the binary hasn't already been loaded) then it calls 
> `GetTarget().SetExecutableModule(module, eLoadDependentsNo)`. If I understood 
> you correctly, then the right thing to do there should be to call 
> `GetTarget().SetExecutableModule(module, eLoadDependentsNo)` in all cases, 
> without checking `GetExecutableModule`, right?
>
> It seems to make sense, but I may need some comments from other reviewers on 
> this.

Just my opinion, but I know how DynamicDarwinLoader works is that when it 
starts the actual debug session, it clears the image list entirely (iirc or 
maybe it just calls Target::SetExecutableModule - which is effectively the same 
thing).  I don't know how Windows works, but on Darwin we pre-load the binaries 
we THINK will be loaded, but when the process actually runs, different binaries 
may end up getting loaded, and we need to use what the dynamic linker tells us 
instead of our original logic.  `GetTarget().SetExecutableModule(module, 
eLoadDependentsNo)` would be one option, or clear the list and start adding, 
effectively the same thing.  I think it would be more straightforward than 
adding this change to Target::GetOrAddModule.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134581

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

[Lldb-commits] [PATCH] D134515: [lldb] Fix completion of 'settings set' values

[Dave Lee via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc418f0053600: [lldb] Fix completion of 'settings 
set' values (authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134515

Files:
  lldb/source/Commands/CommandObjectSettings.cpp
  lldb/test/API/functionalities/completion/TestCompletion.py


Index: lldb/test/API/functionalities/completion/TestCompletion.py
===
--- lldb/test/API/functionalities/completion/TestCompletion.py
+++ lldb/test/API/functionalities/completion/TestCompletion.py
@@ -405,6 +405,11 @@
   ['target.process.thread.step-avoid-regexp',
'target.process.thread.trace-thread'])
 
+def test_settings_set_can_complete_setting_enum_values(self):
+"""Checks that we can complete the values of an enum setting."""
+self.complete_from_to('settings set stop-disassembly-display ',
+  ['never', 'always', 'no-debuginfo', 'no-source'])
+
 def test_thread_plan_discard(self):
 self.build()
 (_, _, thread, _) = lldbutil.run_to_source_breakpoint(self,
Index: lldb/source/Commands/CommandObjectSettings.cpp
===
--- lldb/source/Commands/CommandObjectSettings.cpp
+++ lldb/source/Commands/CommandObjectSettings.cpp
@@ -154,7 +154,7 @@
   return;
 
 // Complete option name
-if (arg[0] != '-')
+if (arg[0] == '-')
   return;
 
 // Complete setting value


Index: lldb/test/API/functionalities/completion/TestCompletion.py
===
--- lldb/test/API/functionalities/completion/TestCompletion.py
+++ lldb/test/API/functionalities/completion/TestCompletion.py
@@ -405,6 +405,11 @@
   ['target.process.thread.step-avoid-regexp',
'target.process.thread.trace-thread'])
 
+def test_settings_set_can_complete_setting_enum_values(self):
+"""Checks that we can complete the values of an enum setting."""
+self.complete_from_to('settings set stop-disassembly-display ',
+  ['never', 'always', 'no-debuginfo', 'no-source'])
+
 def test_thread_plan_discard(self):
 self.build()
 (_, _, thread, _) = lldbutil.run_to_source_breakpoint(self,
Index: lldb/source/Commands/CommandObjectSettings.cpp
===
--- lldb/source/Commands/CommandObjectSettings.cpp
+++ lldb/source/Commands/CommandObjectSettings.cpp
@@ -154,7 +154,7 @@
   return;
 
 // Complete option name
-if (arg[0] != '-')
+if (arg[0] == '-')
   return;
 
 // Complete setting value
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] c418f00 - [lldb] Fix completion of 'settings set' values

[Dave Lee via lldb-commits]

Author: Dave Lee
Date: 2022-09-26T11:26:13-07:00
New Revision: c418f00536001169f40d09bf3bff568aacfc9c30

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

LOG: [lldb] Fix completion of 'settings set' values

Some time ago, a refactor (1153dc960) broke completion for assigning settings
values (`settings set`). This was most annoying for enum settings, where you'd
have to get the valid enum names separately.

This restores the logic in the post-refactor completion function, as well as
adding a test to catch future regressions.

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

Added: 


Modified: 
lldb/source/Commands/CommandObjectSettings.cpp
lldb/test/API/functionalities/completion/TestCompletion.py

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectSettings.cpp 
b/lldb/source/Commands/CommandObjectSettings.cpp
index 86194664bf5dc..9420beaad372b 100644
--- a/lldb/source/Commands/CommandObjectSettings.cpp
+++ b/lldb/source/Commands/CommandObjectSettings.cpp
@@ -154,7 +154,7 @@ insert-before or insert-after.");
   return;
 
 // Complete option name
-if (arg[0] != '-')
+if (arg[0] == '-')
   return;
 
 // Complete setting value

diff  --git a/lldb/test/API/functionalities/completion/TestCompletion.py 
b/lldb/test/API/functionalities/completion/TestCompletion.py
index 49950e0cec13a..f2d3fb704900a 100644
--- a/lldb/test/API/functionalities/completion/TestCompletion.py
+++ b/lldb/test/API/functionalities/completion/TestCompletion.py
@@ -405,6 +405,11 @@ def test_settings_set_target_process_thread_dot(self):
   ['target.process.thread.step-avoid-regexp',
'target.process.thread.trace-thread'])
 
+def test_settings_set_can_complete_setting_enum_values(self):
+"""Checks that we can complete the values of an enum setting."""
+self.complete_from_to('settings set stop-disassembly-display ',
+  ['never', 'always', 'no-debuginfo', 'no-source'])
+
 def test_thread_plan_discard(self):
 self.build()
 (_, _, thread, _) = lldbutil.run_to_source_breakpoint(self,



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

[Lldb-commits] [PATCH] D134661: [lldb][TypeSystemClang] Honor DW_AT_rvalue_reference when creating C++ FunctionPrototypes

[Michael Buch via Phabricator via lldb-commits]

Michael137 created this revision.
Michael137 added reviewers: aprantl, labath.
Herald added a reviewer: shafik.
Herald added a project: All.
Michael137 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Currently funciton lookup in the expression evaluator
fails to disambiguate member functions the are overloaded
on lvalue/rvalue reference-qualifiers. This happens because
we unconditionally set a `FunctionPrototype`s
`ExtProtoInfo::RefQualifier` to `RQ_None`. We lose
the ref-qualifiers in the synthesized AST and `clang::Sema`
fails to pick a correct overload candidate.

DWARF emits information about a function's ref-qualifiers
in the form of a boolean `DW_AT_rvalue_reference` (for rvalues)
and `DW_AT_reference` (for lvalues).

This patch sets the `FunctionPrototype::ExtProtoInfo::RefQualifier`
based on the DWARF attributes above.

**Testing**

- Added API test

llvm/llvm-project issue #57866


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134661

Files:
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
  lldb/test/API/lang/cpp/function-ref-qualifiers/Makefile
  lldb/test/API/lang/cpp/function-ref-qualifiers/TestCppFunctionQualifiers.py
  lldb/test/API/lang/cpp/function-ref-qualifiers/main.cpp

Index: lldb/test/API/lang/cpp/function-ref-qualifiers/main.cpp
===
--- /dev/null
+++ lldb/test/API/lang/cpp/function-ref-qualifiers/main.cpp
@@ -0,0 +1,19 @@
+#include 
+#include 
+
+struct Foo {
+  uint32_t func() const & { return 0; }
+  int64_t func() const && { return 1; }
+  uint32_t func() & { return 2; }
+  int64_t func() && { return 3; }
+};
+
+int main() {
+  Foo foo;
+  const Foo const_foo;
+  auto res = foo.func() + const_foo.func() + Foo{}.func() +
+ static_cast(Foo{}).func();
+
+  std::puts("Break here");
+  return res;
+}
Index: lldb/test/API/lang/cpp/function-ref-qualifiers/TestCppFunctionQualifiers.py
===
--- /dev/null
+++ lldb/test/API/lang/cpp/function-ref-qualifiers/TestCppFunctionQualifiers.py
@@ -0,0 +1,39 @@
+"""
+Tests that C++ expression evaluation can
+disambiguate between rvalue and lvalue
+reference-qualified functions.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCase(TestBase):
+
+def test(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "Break here", lldb.SBFileSpec("main.cpp"))
+
+# const lvalue
+self.expect_expr("const_foo.func()", result_type="uint32_t", result_value="0")
+
+# const rvalue
+self.expect_expr("static_cast(Foo{}).func()",
+ result_type="int64_t", result_value="1")
+
+# non-const lvalue
+self.expect_expr("foo.func()", result_type="uint32_t", result_value="2")
+
+# non-const rvalue
+self.expect_expr("Foo{}.func()", result_type="int64_t", result_value="3")
+
+self.filecheck("target modules dump ast", __file__)
+# CHECK:  |-CXXMethodDecl {{.*}} func 'uint32_t () const &'
+# CHECK-NEXT: | `-AsmLabelAttr {{.*}}
+# CHECK-NEXT: |-CXXMethodDecl {{.*}} func 'int64_t () const &&'
+# CHECK-NEXT: | `-AsmLabelAttr {{.*}}
+# CHECK-NEXT: |-CXXMethodDecl {{.*}} func 'uint32_t () &'
+# CHECK-NEXT: | `-AsmLabelAttr {{.*}}
+# CHECK-NEXT: `-CXXMethodDecl {{.*}} func 'int64_t () &&'
+# CHECK-NEXT:   `-AsmLabelAttr {{.*}}
Index: lldb/test/API/lang/cpp/function-ref-qualifiers/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/cpp/function-ref-qualifiers/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -23,6 +23,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTFwd.h"
 #include "clang/AST/TemplateBase.h"
+#include "clang/AST/Type.h"
 #include "clang/Basic/TargetInfo.h"
 #include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/SmallVector.h"
@@ -398,10 +399,11 @@
   llvm::StringRef name, const CompilerType &function_Type,
   clang::StorageClass storage, bool is_inline);
 
-  CompilerType CreateFunctionType(const CompilerType &result_type,
-  const CompilerType *args, unsigned num_args,
-  bool is_variadic, unsigned type_quals,
-  clang::CallingConv cc = clang::

[Lldb-commits] [lldb] 12502ee - [lldb][test] 1 - Don't do test-replication for gmodules debug_info variant

[Michael Buch via lldb-commits]

Author: Michael Buch
Date: 2022-09-26T19:54:24+01:00
New Revision: 12502ee551236375516b714cfdfb75d85b3b7e3e

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

LOG: [lldb][test] 1 - Don't do test-replication for gmodules debug_info variant

Currently, by default LLDB runs an API test with 3 variants,
one of which, depending on platform, is `gmodules`. However,
most tests don't actually make use of any specific `gmodules`
feature since they compile a single `main.cpp` file without
imporint types from other modules.

Instead of running all tests an extra time with `-gmodules`
enabled, we plan to test `gmodules` features with dedicated
API tests that explicitly opt-into compiling with `-gmodules`.
One of the main benefits of this will be a reduction in total
API test-suite run-time (by around 1/3).

This patch adds a flag to `debug_info_categories` that indicates
whether a category is eligible to be replicated by `lldbtest`.

Keeping `gmodules` a debug-info category is desirable because
`builder.py` knows how to inject the appropriate Makefile flags
into the build command for debug-info categories already. Whereas
for non-debug-info categories we'd have to teach it how to. The
category is inferred from the test-name debug-info suffix currently.

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

Added: 


Modified: 
lldb/docs/resources/test.rst
lldb/packages/Python/lldbsuite/test/lldbtest.py
lldb/packages/Python/lldbsuite/test/test_categories.py

Removed: 




diff  --git a/lldb/docs/resources/test.rst b/lldb/docs/resources/test.rst
index d0e94fbe1f9dc..2ecce85fd9dbf 100644
--- a/lldb/docs/resources/test.rst
+++ b/lldb/docs/resources/test.rst
@@ -173,9 +173,9 @@ but often it's easier to find an existing ``Makefile`` that 
does something
 similar to what you want to do.
 
 Another thing this enables is having 
diff erent variants for the same test
-case. By default, we run every test for all 3 debug info formats, so once with
-DWARF from the object files, once with gmodules and finally with a dSYM on
-macOS or split DWARF (DWO) on Linux. But there are many more things we can test
+case. By default, we run every test for two debug info formats, once with
+DWARF from the object files and another with a dSYM on macOS or split
+DWARF (DWO) on Linux. But there are many more things we can test
 that are orthogonal to the test itself. On GreenDragon we have a matrix bot
 that runs the test suite under 
diff erent configurations, with older host
 compilers and 
diff erent DWARF versions.

diff  --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py 
b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index 5c058769ff3f9..1ea29b3f5a39d 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -1664,14 +1664,16 @@ def __new__(cls, name, bases, attrs):
 # If any debug info categories were explicitly tagged, assume 
that list to be
 # authoritative.  If none were specified, try with all debug
 # info formats.
-all_dbginfo_categories = 
set(test_categories.debug_info_categories)
+all_dbginfo_categories = 
set(test_categories.debug_info_categories.keys())
 categories = set(
 getattr(
 attrvalue,
 "categories",
 [])) & all_dbginfo_categories
 if not categories:
-categories = all_dbginfo_categories
+categories = [category for category, can_replicate \
+  in 
test_categories.debug_info_categories.items() \
+  if can_replicate]
 
 for cat in categories:
 @decorators.add_test_categories([cat])

diff  --git a/lldb/packages/Python/lldbsuite/test/test_categories.py 
b/lldb/packages/Python/lldbsuite/test/test_categories.py
index a396741ccf8f3..f3d8b0749f2f6 100644
--- a/lldb/packages/Python/lldbsuite/test/test_categories.py
+++ b/lldb/packages/Python/lldbsuite/test/test_categories.py
@@ -13,10 +13,14 @@
 # LLDB modules
 from lldbsuite.support import gmodules
 
-
-debug_info_categories = [
-'dwarf', 'dwo', 'dsym', 'gmodules'
-]
+# Key: Category name
+# Value: should be used in lldbtest's debug-info replication
+debug_info_categories = {
+'dwarf': True,
+'dwo'  : True,
+'dsym' : True,
+'gmodules' : False
+}
 
 all_categories = {
 'basic_process': 'Basic process execution sniff tests.',



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

[Lldb-commits] [lldb] 1c6826e - [lldb][test] 2 - Add gmodules test category explicitly where previously done implicitly

[Michael Buch via lldb-commits]

Author: Michael Buch
Date: 2022-09-26T19:54:24+01:00
New Revision: 1c6826e8fc6d5c47c3573efd8d080939a932

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

LOG: [lldb][test] 2 - Add gmodules test category explicitly where previously 
done implicitly

Since we don't compile with `gmodules` implicitly via
debug-info test replication, we should mark all implicit
`gmodules` tests with the appropriate category so the API
tests get actually run as intended.

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

Added: 


Modified: 
lldb/test/API/lang/cpp/gmodules/templates/TestGModules.py
lldb/test/API/lang/objc/modules-app-update/TestClangModulesAppUpdate.py

lldb/test/API/lang/objc/modules-hash-mismatch/TestClangModulesHashMismatch.py
lldb/test/API/lang/objc/modules-incomplete/TestIncompleteModules.py

lldb/test/API/lang/objc/modules-inline-functions/TestModulesInlineFunctions.py
lldb/test/API/lang/objc/modules-update/TestClangModulesUpdate.py
lldb/test/API/lang/objc/objc-struct-argument/TestObjCStructArgument.py

Removed: 




diff  --git a/lldb/test/API/lang/cpp/gmodules/templates/TestGModules.py 
b/lldb/test/API/lang/cpp/gmodules/templates/TestGModules.py
index 69d04636d86b6..7be986f4fcf08 100644
--- a/lldb/test/API/lang/cpp/gmodules/templates/TestGModules.py
+++ b/lldb/test/API/lang/cpp/gmodules/templates/TestGModules.py
@@ -2,5 +2,5 @@
 from lldbsuite.test.decorators import *
 
 lldbinline.MakeInlineTest(__file__, globals(), [
-expectedFailureAll(oslist=["linux"], bugnumber="llvm.org/pr36107",
-debug_info="gmodules")])
+expectedFailureAll(oslist=["linux"], bugnumber="llvm.org/pr36107"),
+add_test_categories(["gmodules"])])

diff  --git 
a/lldb/test/API/lang/objc/modules-app-update/TestClangModulesAppUpdate.py 
b/lldb/test/API/lang/objc/modules-app-update/TestClangModulesAppUpdate.py
index d520082443470..629317192d85f 100644
--- a/lldb/test/API/lang/objc/modules-app-update/TestClangModulesAppUpdate.py
+++ b/lldb/test/API/lang/objc/modules-app-update/TestClangModulesAppUpdate.py
@@ -10,7 +10,7 @@
 
 class TestClangModuleAppUpdate(TestBase):
 
-@skipIf(debug_info=no_match(["gmodules"]))
+@add_test_categories(["gmodules"])
 def test_rebuild_app_modules_untouched(self):
 with open(self.getBuildArtifact("module.modulemap"), "w") as f:
 f.write("""

diff  --git 
a/lldb/test/API/lang/objc/modules-hash-mismatch/TestClangModulesHashMismatch.py 
b/lldb/test/API/lang/objc/modules-hash-mismatch/TestClangModulesHashMismatch.py
index 2ce885370cf14..eb58851ee3ee5 100644
--- 
a/lldb/test/API/lang/objc/modules-hash-mismatch/TestClangModulesHashMismatch.py
+++ 
b/lldb/test/API/lang/objc/modules-hash-mismatch/TestClangModulesHashMismatch.py
@@ -10,7 +10,7 @@
 
 class TestClangModuleHashMismatch(TestBase):
 
-@skipIf(debug_info=no_match(["gmodules"]))
+@add_test_categories(["gmodules"])
 def test_expr(self):
 with open(self.getBuildArtifact("module.modulemap"), "w") as f:
 f.write("""

diff  --git 
a/lldb/test/API/lang/objc/modules-incomplete/TestIncompleteModules.py 
b/lldb/test/API/lang/objc/modules-incomplete/TestIncompleteModules.py
index 95802b4e10122..acb961b57ff92 100644
--- a/lldb/test/API/lang/objc/modules-incomplete/TestIncompleteModules.py
+++ b/lldb/test/API/lang/objc/modules-incomplete/TestIncompleteModules.py
@@ -15,7 +15,7 @@ def setUp(self):
 # Find the line number to break inside main().
 self.line = line_number('main.m', '// Set breakpoint 0 here.')
 
-@skipIf(debug_info=no_match(["gmodules"]))
+@add_test_categories(["gmodules"])
 def test_expr(self):
 self.build()
 exe = self.getBuildArtifact("a.out")

diff  --git 
a/lldb/test/API/lang/objc/modules-inline-functions/TestModulesInlineFunctions.py
 
b/lldb/test/API/lang/objc/modules-inline-functions/TestModulesInlineFunctions.py
index 2db0f87a325bc..81f67867eb525 100644
--- 
a/lldb/test/API/lang/objc/modules-inline-functions/TestModulesInlineFunctions.py
+++ 
b/lldb/test/API/lang/objc/modules-inline-functions/TestModulesInlineFunctions.py
@@ -11,7 +11,8 @@
 
 class ModulesInlineFunctionsTestCase(TestBase):
 
-@skipIf(macos_version=["<", "10.12"], debug_info=no_match(["gmodules"]))
+@add_test_categories(["gmodules"])
+@skipIf(macos_version=["<", "10.12"])
 def test_expr(self):
 self.build()
 exe = self.getBuildArtifact("a.out")

diff  --git a/lldb/test/API/lang/objc/modules-update/TestClangModulesUpdate.py 
b/lldb/test/API/lang/objc/modules-update/TestClangModulesUpdate.py
index 4056741716b82..e1b5c510a2db0 100644
--- a/lldb/test/API/lang/objc/modules-update/TestClangModulesUpdate.py
+++ b/lldb/test/API/lang/objc/modules-update/TestClangModulesUpd

[Lldb-commits] [PATCH] D134524: [lldb][test] 1 - Don't do test-replication for gmodules debug_info variant

[Michael Buch via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG12502ee55123: [lldb][test] 1 - Don't do 
test-replication for gmodules debug_info variant (authored by Michael137).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134524

Files:
  lldb/docs/resources/test.rst
  lldb/packages/Python/lldbsuite/test/lldbtest.py
  lldb/packages/Python/lldbsuite/test/test_categories.py


Index: lldb/packages/Python/lldbsuite/test/test_categories.py
===
--- lldb/packages/Python/lldbsuite/test/test_categories.py
+++ lldb/packages/Python/lldbsuite/test/test_categories.py
@@ -13,10 +13,14 @@
 # LLDB modules
 from lldbsuite.support import gmodules
 
-
-debug_info_categories = [
-'dwarf', 'dwo', 'dsym', 'gmodules'
-]
+# Key: Category name
+# Value: should be used in lldbtest's debug-info replication
+debug_info_categories = {
+'dwarf': True,
+'dwo'  : True,
+'dsym' : True,
+'gmodules' : False
+}
 
 all_categories = {
 'basic_process': 'Basic process execution sniff tests.',
Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -1664,14 +1664,16 @@
 # If any debug info categories were explicitly tagged, assume 
that list to be
 # authoritative.  If none were specified, try with all debug
 # info formats.
-all_dbginfo_categories = 
set(test_categories.debug_info_categories)
+all_dbginfo_categories = 
set(test_categories.debug_info_categories.keys())
 categories = set(
 getattr(
 attrvalue,
 "categories",
 [])) & all_dbginfo_categories
 if not categories:
-categories = all_dbginfo_categories
+categories = [category for category, can_replicate \
+  in 
test_categories.debug_info_categories.items() \
+  if can_replicate]
 
 for cat in categories:
 @decorators.add_test_categories([cat])
Index: lldb/docs/resources/test.rst
===
--- lldb/docs/resources/test.rst
+++ lldb/docs/resources/test.rst
@@ -173,9 +173,9 @@
 similar to what you want to do.
 
 Another thing this enables is having different variants for the same test
-case. By default, we run every test for all 3 debug info formats, so once with
-DWARF from the object files, once with gmodules and finally with a dSYM on
-macOS or split DWARF (DWO) on Linux. But there are many more things we can test
+case. By default, we run every test for two debug info formats, once with
+DWARF from the object files and another with a dSYM on macOS or split
+DWARF (DWO) on Linux. But there are many more things we can test
 that are orthogonal to the test itself. On GreenDragon we have a matrix bot
 that runs the test suite under different configurations, with older host
 compilers and different DWARF versions.


Index: lldb/packages/Python/lldbsuite/test/test_categories.py
===
--- lldb/packages/Python/lldbsuite/test/test_categories.py
+++ lldb/packages/Python/lldbsuite/test/test_categories.py
@@ -13,10 +13,14 @@
 # LLDB modules
 from lldbsuite.support import gmodules
 
-
-debug_info_categories = [
-'dwarf', 'dwo', 'dsym', 'gmodules'
-]
+# Key: Category name
+# Value: should be used in lldbtest's debug-info replication
+debug_info_categories = {
+'dwarf': True,
+'dwo'  : True,
+'dsym' : True,
+'gmodules' : False
+}
 
 all_categories = {
 'basic_process': 'Basic process execution sniff tests.',
Index: lldb/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -1664,14 +1664,16 @@
 # If any debug info categories were explicitly tagged, assume that list to be
 # authoritative.  If none were specified, try with all debug
 # info formats.
-all_dbginfo_categories = set(test_categories.debug_info_categories)
+all_dbginfo_categories = set(test_categories.debug_info_categories.keys())
 categories = set(
 getattr(
 attrvalue,
 "categories",
 [])) & all_dbginfo_categories
 if not categories:
-categories = all_dbginfo_categories
+   

[Lldb-commits] [PATCH] D134574: [lldb][test] 2 - Add gmodules test category explicitly where previously done implicitly

[Michael Buch via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1c6826e8fc6d: [lldb][test] 2 - Add gmodules test category 
explicitly where previously done… (authored by Michael137).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134574

Files:
  lldb/test/API/lang/cpp/gmodules/templates/TestGModules.py
  lldb/test/API/lang/objc/modules-app-update/TestClangModulesAppUpdate.py
  lldb/test/API/lang/objc/modules-hash-mismatch/TestClangModulesHashMismatch.py
  lldb/test/API/lang/objc/modules-incomplete/TestIncompleteModules.py
  lldb/test/API/lang/objc/modules-inline-functions/TestModulesInlineFunctions.py
  lldb/test/API/lang/objc/modules-update/TestClangModulesUpdate.py
  lldb/test/API/lang/objc/objc-struct-argument/TestObjCStructArgument.py


Index: lldb/test/API/lang/objc/objc-struct-argument/TestObjCStructArgument.py
===
--- lldb/test/API/lang/objc/objc-struct-argument/TestObjCStructArgument.py
+++ lldb/test/API/lang/objc/objc-struct-argument/TestObjCStructArgument.py
@@ -18,8 +18,9 @@
 self.break_line = line_number(
 self.main_source, '// Set breakpoint here.')
 
-@add_test_categories(['pyapi'])
-@skipIf(debug_info=no_match(["gmodules"]), oslist=['ios', 'watchos', 
'tvos', 'bridgeos'], archs=['armv7', 'arm64'])  # this test program only builds 
for ios with -gmodules
+# this test program only builds for ios with -gmodules
+@add_test_categories(['gmodules', 'pyapi'])
+@skipIf(oslist=['ios', 'watchos', 'tvos', 'bridgeos'], archs=['armv7', 
'arm64'])
 def test_with_python_api(self):
 """Test passing structs to Objective-C methods."""
 self.build()
Index: lldb/test/API/lang/objc/modules-update/TestClangModulesUpdate.py
===
--- lldb/test/API/lang/objc/modules-update/TestClangModulesUpdate.py
+++ lldb/test/API/lang/objc/modules-update/TestClangModulesUpdate.py
@@ -10,7 +10,7 @@
 
 class TestClangModuleUpdate(TestBase):
 
-@skipIf(debug_info=no_match(["gmodules"]))
+@add_test_categories(["gmodules"])
 @skipIfDarwin # rdar://76540904
 def test_expr(self):
 with open(self.getBuildArtifact("module.modulemap"), "w") as f:
Index: 
lldb/test/API/lang/objc/modules-inline-functions/TestModulesInlineFunctions.py
===
--- 
lldb/test/API/lang/objc/modules-inline-functions/TestModulesInlineFunctions.py
+++ 
lldb/test/API/lang/objc/modules-inline-functions/TestModulesInlineFunctions.py
@@ -11,7 +11,8 @@
 
 class ModulesInlineFunctionsTestCase(TestBase):
 
-@skipIf(macos_version=["<", "10.12"], debug_info=no_match(["gmodules"]))
+@add_test_categories(["gmodules"])
+@skipIf(macos_version=["<", "10.12"])
 def test_expr(self):
 self.build()
 exe = self.getBuildArtifact("a.out")
Index: lldb/test/API/lang/objc/modules-incomplete/TestIncompleteModules.py
===
--- lldb/test/API/lang/objc/modules-incomplete/TestIncompleteModules.py
+++ lldb/test/API/lang/objc/modules-incomplete/TestIncompleteModules.py
@@ -15,7 +15,7 @@
 # Find the line number to break inside main().
 self.line = line_number('main.m', '// Set breakpoint 0 here.')
 
-@skipIf(debug_info=no_match(["gmodules"]))
+@add_test_categories(["gmodules"])
 def test_expr(self):
 self.build()
 exe = self.getBuildArtifact("a.out")
Index: 
lldb/test/API/lang/objc/modules-hash-mismatch/TestClangModulesHashMismatch.py
===
--- 
lldb/test/API/lang/objc/modules-hash-mismatch/TestClangModulesHashMismatch.py
+++ 
lldb/test/API/lang/objc/modules-hash-mismatch/TestClangModulesHashMismatch.py
@@ -10,7 +10,7 @@
 
 class TestClangModuleHashMismatch(TestBase):
 
-@skipIf(debug_info=no_match(["gmodules"]))
+@add_test_categories(["gmodules"])
 def test_expr(self):
 with open(self.getBuildArtifact("module.modulemap"), "w") as f:
 f.write("""
Index: lldb/test/API/lang/objc/modules-app-update/TestClangModulesAppUpdate.py
===
--- lldb/test/API/lang/objc/modules-app-update/TestClangModulesAppUpdate.py
+++ lldb/test/API/lang/objc/modules-app-update/TestClangModulesAppUpdate.py
@@ -10,7 +10,7 @@
 
 class TestClangModuleAppUpdate(TestBase):
 
-@skipIf(debug_info=no_match(["gmodules"]))
+@add_test_categories(["gmodules"])
 def test_rebuild_app_modules_untouched(self):
 with open(self.getBuildArtifact("module.modulemap"), "w") as f:
 f.write("""
Index: lldb/test/API/lang/cpp/gmodules/templates/TestGModules.py
===
--- lldb/test/API/lang/

[Lldb-commits] [PATCH] D134037: [lldb] Fix CommandInterpreter::DidProcessStopAbnormally() with multiple threads

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D134037#3815240 , @DavidSpickett 
wrote:

>> I might add the AArch64 and Arm equivalent if I have some spare time.
>
> I looked into this and there's a detail that makes this work on x86 only. So 
> I'll leave it as is, it's getting tested somewhere, and that's what matters.
>
> If you're curious this is the spanner in the works:
>
>   size_t NativeProcessProtocol::GetSoftwareBreakpointPCOffset() {
> switch (GetArchitecture().GetMachine()) {
> case llvm::Triple::x86:
> case llvm::Triple::x86_64:
> case llvm::Triple::systemz:
>   // These architectures report increment the PC after breakpoint is hit.
>   return cantFail(GetSoftwareBreakpointTrapOpcode(0)).size();
>
> Meaning AArch64 doesn't. lldb on x86 can continue past a break in the program 
> because of this:
>
>   (lldb) run
>   Process 31032 launched: '/tmp/test.o' (x86_64)
>   Process 31032 stopped
>   * thread #1, name = 'test.o', stop reason = signal SIGTRAP
>   frame #0: 0x460f test.o`main at test.c:3
>  1int main() {
>  2__asm__ __volatile__("int3");
>   -> 3  return 0;
>  4}
>   (lldb) c
>   Process 31032 resuming
>   Process 31032 exited with status = 0 (0x)
>
> On AArch64 we just stick on the break:
>
>   (lldb) run
>   Process 2162869 launched: '/tmp/test.o' (aarch64)
>   Process 2162869 stopped
>   * thread #1, name = 'test.o', stop reason = signal SIGTRAP
>   frame #0: 0xa71c test.o`main at test.c:2:3
>  1int main() {
>   -> 2  asm volatile("brk #0xf000\n\t");
>  3  return 0;
>  4}
>   (lldb) c
>   Process 2162869 resuming
>   Process 2162869 stopped
>   * thread #1, name = 'test.o', stop reason = signal SIGTRAP
>   frame #0: 0xa71c test.o`main at test.c:2:3
>  1int main() {
>   -> 2  asm volatile("brk #0xf000\n\t");
>  3  return 0;
>  4}
>
> gdb has the same behaviour on AArch64 so I'll leave it as is until someone 
> complains.

Ah, yes, I remember hitting that issue occasionally. On the other hand, if it 
would be somewhat doable to fix it, it'd would be a great value add IMO - I've 
had to do acrobatics to set the program counter to the next instruction to 
resume running after such a breakpoint a couple times...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134037

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

[Lldb-commits] [PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

[Arthur Eubanks via Phabricator via lldb-commits]

aeubanks added a comment.

@vitalybuka can we turn on -fsanitize-memory-param-retval by default upstream?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133036

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

[Lldb-commits] [PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

[Arthur Eubanks via Phabricator via lldb-commits]

aeubanks added a comment.

In D133036#3816002 , @aeubanks wrote:

> @vitalybuka can we turn on -fsanitize-memory-param-retval by default upstream?

attempt in https://reviews.llvm.org/D134669


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133036

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

[Lldb-commits] [PATCH] D133858: [lldb] Reset breakpoint hit count before new runs

[Felipe de Azevedo Piovezan via Phabricator via lldb-commits]

fdeazeve added a comment.

In D133858#3805630 , @labath wrote:

> I am afraid that this patch misses one method of initiating a debug session 
> -- connecting to a running debug server (`process connect`, 
> `SBTarget::ConnectRemote`). The breakpoint hit counts don't get reset in case 
> of a reconnect. This isn't a particularly common use case (and the only 
> reason I've noticed it is that for `PlatformQemuUser`, all "launches" are 
> actually "connects" under the hood 
> ),
>  but I've verified that this problem can be reproduced by issuing connect 
> commands manually (on the regular host platform). I'm pretty sure that was 
> not intentional.
>
> Fixing this by adding another callout to `ResetBreakpointHitCounts` would be 
> easy enough, but I'm also thinking if there isn't a better place from which 
> to call this function, one that would capture all three scenarios in a single 
> statement. I think that one such place could be `Target::CreateProcess`. This 
> function is called by all three code paths, and it's a very good indicator 
> that we will be starting a new debug session.
>
> What do you think?

My understanding is that there is an obligation of calling the WillXX methods 
before calling XX, so by placing the reset code in the WillXX functions we can 
rely on that guarantee. Right now, the same is implicit for "one must call 
Target::CreateProcess before launching or attaching". We could instead define a 
WillConnect and have the usual contract for that.

The code is fairly new to me, so I'm not confident enough to make a judgement 
call here. Thoughts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133858

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

[Lldb-commits] [lldb] 79e522f - Revert "Skip crashing test"

[Adrian Prantl via lldb-commits]

Author: Adrian Prantl
Date: 2022-09-26T17:17:47-07:00
New Revision: 79e522fb44e1761e5ad90835f95c5dadcf8d8406

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

LOG: Revert "Skip crashing test"

This reverts commit c1ce19021da0cf1c88722024e6ff9cee7aabc7b6.

Added: 


Modified: 

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/TestDataFormatterLibcxxSpan.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/TestDataFormatterLibcxxSpan.py
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/TestDataFormatterLibcxxSpan.py
index e37f1a9799fca..3236544e21859 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/TestDataFormatterLibcxxSpan.py
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/span/TestDataFormatterLibcxxSpan.py
@@ -44,7 +44,6 @@ def check_numbers(self, var_name):
 
 @add_test_categories(['libc++'])
 @skipIf(compiler='clang', compiler_version=['<', '11.0'])
-@skipIf(debug_info='gmodules', bugnumber="rdar://99758046") # Crashes 
Clang while compiling module. 
 def test_with_run_command(self):
 """Test that std::span variables are formatted correctly when 
printed."""
 self.build()
@@ -136,7 +135,6 @@ def test_with_run_command(self):
 
 @add_test_categories(['libc++'])
 @skipIf(compiler='clang', compiler_version=['<', '11.0'])
-@skipIf(debug_info='gmodules', bugnumber="rdar://99758046") # Crashes 
Clang while compiling module. 
 def test_ref_and_ptr(self):
 """Test that std::span is correctly formatted when passed by ref and 
ptr"""
 self.build()



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

[Lldb-commits] [PATCH] D134333: When there are variable errors, display an error in VS Code's local variables view.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D134333#3812653 , @jingham wrote:

> In D134333#3812448 , @clayborg 
> wrote:
>
>> I just did a search through our test sources and we use 
>> GetError().GetCString() 34 times in our test suites python files. So the 
>> SBError::GetCString() is being misused a lot like this already and is 
>> probably making some tests flaky. I would highly recommend we fix 
>> SBError::GetCString() to make sure things work correctly. If people are 
>> already doing this, or if they can do this with our API, then we should make 
>> our API as stable as possible for users even if it costs a bit more memory 
>> in our string pools.
>
> When we return Python strings from the SWIG'ed version of 
> GetError().GetCString() does the Python string we make & return copy the 
> string data, or is it just holding onto the pointer?  In C/C++ if someone 
> returns you a char * unless explicitly specified you assume that string is 
> only going to live as long as the object that handed it out.  But a python 
> string is generally self-contained, so you wouldn't expect it to lose its 
> data when the generating object goes away.  I haven't checked yet, but if 
> this is how SWIG handles making python strings from C strings, then this 
> wouldn't be an error in the testsuite, only in C++ code.  If that's not how 
> it works, that might be a good way to finesse the issue.

The main issue is people are already using this API this way. We can fix this 
by adding documentation and saying "don't do that", but that will lead to bugs 
and our API appearing flaky. Many strings we return have no lifetime issues due 
to string constification. We can't really even stop people from doing this by 
making sure the string is emptied when the object destructs because another 
object could come and live in its place quickly on the stack and we could be 
using a bogus data as the error string.

All that said, I am fine going with what the majority of people want on this. I 
will remove this issue from this patch so it doesn't hold up this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134333

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

[Lldb-commits] [PATCH] D134333: When there are variable errors, display an error in VS Code's local variables view.

[Greg Clayton via Phabricator via lldb-commits]

clayborg updated this revision to Diff 463102.
clayborg added a comment.

Don't constify SBError::GetCString anymore. We can fix this in another patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134333

Files:
  lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -2953,6 +2953,31 @@
 }
 
 num_children = top_scope->GetSize();
+if (num_children == 0 && variablesReference == VARREF_LOCALS) {
+  // Check for an error in the SBValueList that might explain why we don't
+  // have locals. If we have an error display it as the sole value in the
+  // the locals.
+
+  // "error" owns the error string so we must keep it alive as long as we
+  // want to use the returns "const char *"
+  SBError error = top_scope->GetError();
+  const char *var_err = error.GetCString();
+  if (var_err) {
+// Create a fake variable named "error" to explain why variables were
+// not available. This new error will help let users know when there was
+// a problem that kept variables from being available for display and
+// allow users to fix this issue instead of seeing no variables. The
+// errors are only set when there is a problem that the user could
+// fix, so no error will show up when you have no debug info, only when
+// we do have debug info and something that is fixable can be done.
+llvm::json::Object object;
+EmplaceSafeString(object, "name", "");
+EmplaceSafeString(object, "type", "const char *");
+EmplaceSafeString(object, "value", var_err);
+object.try_emplace("variablesReference", (int64_t)0);
+variables.emplace_back(std::move(object));
+  }
+}
 const int64_t end_idx = start_idx + ((count == 0) ? num_children : count);
 
 // We first find out which variable names are duplicated
Index: lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
===
--- lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
+++ lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
@@ -7,7 +7,7 @@
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 import lldbvscode_testcase
-
+import os
 
 def make_buffer_verify_dict(start_idx, count, offset=0):
 verify_dict = {}
@@ -38,6 +38,19 @@
  ' "%s")') % (
 key, actual_value,
 verify_value))
+if 'contains' in verify_dict:
+verify = verify_dict['contains']
+for key in verify:
+contains_array = verify[key]
+actual_value = actual[key]
+self.assertTrue(isinstance(contains_array, list))
+for verify_value in contains_array:
+contains = verify_value in actual_value
+self.assertTrue(contains,
+('"%s" value "%s" doesn\'t contain'
+ ' "%s")') % (
+key, actual_value,
+verify_value))
 if 'missing' in verify_dict:
 missing = verify_dict['missing']
 for key in missing:
@@ -508,3 +521,46 @@
 self.assertTrue(value.startswith('0x'))
 self.assertTrue('a.out`main + ' in value)
 self.assertTrue('at main.cpp:' in value)
+
+@no_debug_info_test
+@skipUnlessDarwin
+def test_darwin_dwarf_missing_obj(self):
+'''
+Test that if we build a binary with DWARF in .o files and we remove
+the .o file for main.cpp, that we get a variable named ""
+whose value matches the appriopriate error. Errors when getting
+variables are returned in the LLDB API when the user should be
+notified of issues that can easily be solved by rebuilding or
+changing compiler options and are designed to give better feedback
+to the user.
+'''
+self.build(debug_info="dwarf")
+program = self.getBuildArtifact("a.out")
+main_obj = self.getBuildArtifact("main.o")
+self.assertTrue(os.path.exists(main_obj))
+# Delete the main.o file that contains the debug info so we force an
+# error when we run to main and try to get variables
+os.unlink(main_obj)
+
+self.create_debug_adaptor()
+self.assertTrue(os.path.exists(program), 'executable must exist')
+self.launch(prog