[Lldb-commits] [PATCH] D109691: [lldb] [ABI/AArch64] Recognize special regs by their xN names too

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

mgorny added a comment.

@labath, the tests here require D109272 , as 
otherwise LLDB won't recognize the architecture correctly. If you review that 
one, I can rebase its tests not to require other patches and merge these two.


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

https://reviews.llvm.org/D109691

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

[Lldb-commits] [PATCH] D109797: Fix rendezvous for rebase_exec=true case

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

mgorny added a comment.

I'm going to test this on FreeBSD in a minute. Could you fix clang-formatting 
in the meantime?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109797

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

[Lldb-commits] [PATCH] D108831: [lldb] [gdb-remote] Add x86_64 pseudo-registers when using gdbserver

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

mgorny added a comment.

@labath, could you look at my inline comments before I start moving stuff? ;-)


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

https://reviews.llvm.org/D108831

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

[Lldb-commits] [PATCH] D109778: [lldb] [Windows] Fix an incorrect assert in NativeRegisterContextWindows_arm

[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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109778

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

[Lldb-commits] [PATCH] D109777: [lldb] [Windows] Fix continuing from breakpoints and singlestepping on ARM/AArch64

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a reviewer: DavidSpickett.
DavidSpickett added a comment.

https://docs.microsoft.com/en-us/cpp/intrinsics/debugbreak?view=msvc-160 
confirms the AArch64 breakpoint instruction, were you able to find a source for 
the Arm/Thumb one?

Otherwise the logic seems fine. It would be good to dedupe these switches, in a 
file in `Plugins/Process/Windows/Common/`?
Or move some of this into the register context? Assuming you have access to a 
register context in all of these places. It's a bit weird to put it there since 
the only register related thing is the single step bit but that's the only per 
architecture class at the moment.

(also all the Windows code being in `Windows/Common` is odd but ok not a thing 
for now)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109777

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

[Lldb-commits] [PATCH] D109797: Fix rendezvous for rebase_exec=true case

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

mgorny added a comment.

Ok, I don't see any obvious regressions while testing, so LGTM modulo 
formatting changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109797

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

[Lldb-commits] [PATCH] D109777: [lldb] [Windows] Fix continuing from breakpoints and singlestepping on ARM/AArch64

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo added a comment.

In D109777#3001429 , @DavidSpickett 
wrote:

> https://docs.microsoft.com/en-us/cpp/intrinsics/debugbreak?view=msvc-160 
> confirms the AArch64 breakpoint instruction, were you able to find a source 
> for the Arm/Thumb one?

I don't have a formal source for that one, but compiling a `__debugbreak()` 
with MSVC produces that opcode. As Windows is thumb-only, we can always do a 
breakpoint with that single opcode (even if the targeted instruction happens to 
be a wide thumb2 instruction) without needing to detect betwen arm/thumb mode.

> Otherwise the logic seems fine. It would be good to dedupe these switches, in 
> a file in `Plugins/Process/Windows/Common/`?
> Or move some of this into the register context? Assuming you have access to a 
> register context in all of these places. It's a bit weird to put it there 
> since the only register related thing is the single step bit but that's the 
> only per architecture class at the moment.

You mean the duplication between the "native" and regular versions of the 
classes - or the arch specific switches in otherwise seemingly arch-independent 
code?

(In case it's not familiar to all - the former duplication is because lldb can 
operate in two modes - either with lldb directly controlling the debugged 
process (which is the case without the "native" prefix) or with lldb spawning 
lldb-server which then owns the debugged process (the classes with the "native" 
prefix).)

For the former, I'm not quite familiar enough the how the architecture of these 
things work in practice, but I think those two class hiearchies/families don't 
have much in common, so there's no immediate common place to put shared code 
between the two.

For the latter - I guess it'd maybe be possible to add some extra locally 
defined (not overriding a virtual method from the base classes) methods in the 
register contexts that return the breakpoint opcodes, their sizes and the right 
flag bits...

> (also all the Windows code being in `Windows/Common` is odd but ok not a 
> thing for now)

Yeah that's a bit odd indeed. There's also a few other oddities in the lldb 
tree, like some directiories being capitalized while others aren't, I guess 
it's one of the many cases where it's just not worth the churn to change it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109777

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

[Lldb-commits] [lldb] b4133a2 - [lldb] [Windows] Fix an incorrect assert in NativeRegisterContextWindows_arm

[Martin Storsjö via lldb-commits]

Author: Martin Storsjö
Date: 2021-09-15T15:03:20+03:00
New Revision: b4133a21cef49edb57cf96bb7d7518099d61e910

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

LOG: [lldb] [Windows] Fix an incorrect assert in 
NativeRegisterContextWindows_arm

This codepath hadn't been exercised in a build with asserts before.

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

Added: 


Modified: 

lldb/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_arm.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_arm.cpp
 
b/lldb/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_arm.cpp
index d2b1bc1a13c61..5cfd790c624a3 100644
--- 
a/lldb/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_arm.cpp
+++ 
b/lldb/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_arm.cpp
@@ -80,8 +80,8 @@ enum { k_num_register_sets = 2 };
 
 static RegisterInfoInterface *
 CreateRegisterInfoInterface(const ArchSpec &target_arch) {
-  assert((HostInfo::GetArchitecture().GetAddressByteSize() == 8) &&
- "Register setting path assumes this is a 64-bit host");
+  assert((HostInfo::GetArchitecture().GetAddressByteSize() == 4) &&
+ "Register setting path assumes this is a 32-bit host");
   return new RegisterInfoPOSIX_arm(target_arch);
 }
 



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

[Lldb-commits] [PATCH] D109778: [lldb] [Windows] Fix an incorrect assert in NativeRegisterContextWindows_arm

[Martin Storsjö via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb4133a21cef4: [lldb] [Windows] Fix an incorrect assert in 
NativeRegisterContextWindows_arm (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109778

Files:
  
lldb/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_arm.cpp


Index: 
lldb/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_arm.cpp
===
--- 
lldb/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_arm.cpp
+++ 
lldb/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_arm.cpp
@@ -80,8 +80,8 @@
 
 static RegisterInfoInterface *
 CreateRegisterInfoInterface(const ArchSpec &target_arch) {
-  assert((HostInfo::GetArchitecture().GetAddressByteSize() == 8) &&
- "Register setting path assumes this is a 64-bit host");
+  assert((HostInfo::GetArchitecture().GetAddressByteSize() == 4) &&
+ "Register setting path assumes this is a 32-bit host");
   return new RegisterInfoPOSIX_arm(target_arch);
 }
 


Index: lldb/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_arm.cpp
===
--- lldb/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_arm.cpp
+++ lldb/source/Plugins/Process/Windows/Common/NativeRegisterContextWindows_arm.cpp
@@ -80,8 +80,8 @@
 
 static RegisterInfoInterface *
 CreateRegisterInfoInterface(const ArchSpec &target_arch) {
-  assert((HostInfo::GetArchitecture().GetAddressByteSize() == 8) &&
- "Register setting path assumes this is a 64-bit host");
+  assert((HostInfo::GetArchitecture().GetAddressByteSize() == 4) &&
+ "Register setting path assumes this is a 32-bit host");
   return new RegisterInfoPOSIX_arm(target_arch);
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D109777: [lldb] [Windows] Fix continuing from breakpoints and singlestepping on ARM/AArch64

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> I don't have a formal source for that one, but compiling a __debugbreak() 
> with MSVC produces that opcode. As Windows is thumb-only, we can always do a 
> breakpoint with that single opcode (even if the targeted instruction happens 
> to be a wide thumb2 instruction) without needing to detect betwen arm/thumb 
> mode.

Great. (I had forgotten about being Thumb only in fact)

> You mean the duplication between the "native" and regular versions of the 
> classes - or the arch specific switches in otherwise seemingly 
> arch-independent code?

The first one. The switches follow the style of the generic methods so I'm cool 
with that.

> For the former, I'm not quite familiar enough the how the architecture of 
> these things work in practice, but I think those two class 
> hiearchies/families don't have much in common, so there's no immediate common 
> place to put shared code between the two.

Looking at the cmake it builds `lldbPluginProcessWindowsCommon` which includes 
native and process files so I think that means you can add a file there and 
both modes can use it, I think. `BreakpointInfo.cpp` ?

> For the latter - I guess it'd maybe be possible to add some extra locally 
> defined (not overriding a virtual method from the base classes) methods in 
> the register contexts that return the breakpoint opcodes, their sizes and the 
> right flag bits...

Now you mention the two modes, because each mode has its own register context 
class if you put the breakpoint info there it'll also be duplicated. Unless you 
add an intermediate RegisterContextWindowsBaseArm in the middle but that's 
blurring the register context's purpose further.

If the breakpoint info can go in a file that both modes can access that would 
be great, otherwise it's fine as is. Not likely to change this in a hurry 
anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109777

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

[Lldb-commits] [PATCH] D109797: Fix rendezvous for rebase_exec=true case

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 372711.
emrekultursay added a comment.

Applied clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109797

Files:
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp


Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -442,14 +442,18 @@
 if (module_sp->GetObjectFile()->GetBaseAddress().GetLoadAddress(
 &m_process->GetTarget()) == m_interpreter_base &&
 module_sp != m_interpreter_module.lock()) {
-  // If this is a duplicate instance of ld.so, unload it.  We may end 
up
-  // with it if we load it via a different path than before (symlink
-  // vs real path).
-  // TODO: remove this once we either fix library matching or avoid
-  // loading the interpreter when setting the rendezvous breakpoint.
-  UnloadSections(module_sp);
-  loaded_modules.Remove(module_sp);
-  continue;
+  if (m_interpreter_module.lock() == nullptr) {
+m_interpreter_module = module_sp;
+  } else {
+// If this is a duplicate instance of ld.so, unload it.  We may end
+// up with it if we load it via a different path than before
+// (symlink vs real path).
+// TODO: remove this once we either fix library matching or avoid
+// loading the interpreter when setting the rendezvous breakpoint.
+UnloadSections(module_sp);
+loaded_modules.Remove(module_sp);
+continue;
+  }
 }
 
 loaded_modules.AppendIfNeeded(module_sp);


Index: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
===
--- lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -442,14 +442,18 @@
 if (module_sp->GetObjectFile()->GetBaseAddress().GetLoadAddress(
 &m_process->GetTarget()) == m_interpreter_base &&
 module_sp != m_interpreter_module.lock()) {
-  // If this is a duplicate instance of ld.so, unload it.  We may end up
-  // with it if we load it via a different path than before (symlink
-  // vs real path).
-  // TODO: remove this once we either fix library matching or avoid
-  // loading the interpreter when setting the rendezvous breakpoint.
-  UnloadSections(module_sp);
-  loaded_modules.Remove(module_sp);
-  continue;
+  if (m_interpreter_module.lock() == nullptr) {
+m_interpreter_module = module_sp;
+  } else {
+// If this is a duplicate instance of ld.so, unload it.  We may end
+// up with it if we load it via a different path than before
+// (symlink vs real path).
+// TODO: remove this once we either fix library matching or avoid
+// loading the interpreter when setting the rendezvous breakpoint.
+UnloadSections(module_sp);
+loaded_modules.Remove(module_sp);
+continue;
+  }
 }
 
 loaded_modules.AppendIfNeeded(module_sp);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D109832: [lldb/win] Fix TestIRMemoryMapWindows.test when running tests in git bash

[Nico Weber via Phabricator via lldb-commits]

thakis created this revision.
thakis added a reviewer: amccarth.
Herald added a subscriber: pengfei.
thakis requested review of this revision.

lit.util.which('link') picks up the wrong link.exe in git bash, leading
to this error:

1. command stderr: /usr/bin/link: extra operand '/LIBPATH:C:\\Program Files 
(x86)\\Windows Kits\\10\\lib\\10.0.17763.0\\ucrt\\x64' Try '/usr/bin/link 
--help' for more information.

Instead, assume that link.exe is next to cl.exe.


https://reviews.llvm.org/D109832

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


Index: lldb/test/Shell/helper/toolchain.py
===
--- lldb/test/Shell/helper/toolchain.py
+++ lldb/test/Shell/helper/toolchain.py
@@ -90,11 +90,14 @@
 # detect the include and lib paths, and find cl.exe and link.exe and create
 # substitutions for each of them that explicitly specify /I and /L paths
 cl = lit.util.which('cl')
-link = lit.util.which('link')
 
-if not cl or not link:
+if not cl:
 return
 
+# Don't use lit.util.which() for link.exe: In `git bash`, it will pick
+# up /usr/bin/link (another name for ln).
+link = os.path.join(os.path.dirname(cl), 'link.exe')
+
 cl = '"' + cl + '"'
 link = '"' + link + '"'
 includes = os.getenv('INCLUDE', '').split(';')


Index: lldb/test/Shell/helper/toolchain.py
===
--- lldb/test/Shell/helper/toolchain.py
+++ lldb/test/Shell/helper/toolchain.py
@@ -90,11 +90,14 @@
 # detect the include and lib paths, and find cl.exe and link.exe and create
 # substitutions for each of them that explicitly specify /I and /L paths
 cl = lit.util.which('cl')
-link = lit.util.which('link')
 
-if not cl or not link:
+if not cl:
 return
 
+# Don't use lit.util.which() for link.exe: In `git bash`, it will pick
+# up /usr/bin/link (another name for ln).
+link = os.path.join(os.path.dirname(cl), 'link.exe')
+
 cl = '"' + cl + '"'
 link = '"' + link + '"'
 includes = os.getenv('INCLUDE', '').split(';')
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D109834: [lldb/win] Improve check-lldb-shell with LLVM_ENABLE_DIA_SDK=NO

[Nico Weber via Phabricator via lldb-commits]

thakis created this revision.
thakis added a reviewer: amccarth.
thakis requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Before this change, running check-lldb-shell with LLVM_ENABLE_DIA_SDK=NO would 
fail ~25 tests.

This change adds a lit feature based on if the DIA SDK is available and 
annotates
all tests that set breakpoints with this new feature. Now these tests are 
skipped
(with a warning message during test startup) instead of failing. This matches 
how
this is handled in check-llvm.

(This doesn't touch API or Unit tests for now.)


https://reviews.llvm.org/D109834

Files:
  lldb/test/Shell/Breakpoint/case-insensitive.test
  lldb/test/Shell/Breakpoint/dummy-target.test
  lldb/test/Shell/Commands/command-thread-select.test
  lldb/test/Shell/Driver/TestSingleQuote.test
  lldb/test/Shell/Expr/TestIRMemoryMapWindows.test
  lldb/test/Shell/Expr/nodefaultlib.cpp
  lldb/test/Shell/Settings/TestFrameFormatColor.test
  lldb/test/Shell/Settings/TestFrameFormatNoColor.test
  lldb/test/Shell/Settings/TestLineMarkerColor.test
  lldb/test/Shell/SymbolFile/NativePDB/stack_unwinding01.cpp
  lldb/test/Shell/SymbolFile/PDB/lit.local.cfg
  lldb/test/Shell/Watchpoint/SetErrorCases.test
  lldb/test/Shell/lit.cfg.py
  lldb/test/Shell/lit.site.cfg.py.in
  llvm/utils/gn/secondary/lldb/test/BUILD.gn

Index: llvm/utils/gn/secondary/lldb/test/BUILD.gn
===
--- llvm/utils/gn/secondary/lldb/test/BUILD.gn
+++ llvm/utils/gn/secondary/lldb/test/BUILD.gn
@@ -1,6 +1,5 @@
+import("//llvm/lib/DebugInfo/PDB/enable_dia.gni")
 import("//llvm/triples.gni")
-
-#import("//llvm/utils/gn/build/libs/xar/enable.gni")
 import("//llvm/utils/gn/build/libs/xml/enable.gni")
 import("//llvm/utils/gn/build/libs/zlib/enable.gni")
 import("//llvm/utils/gn/build/write_cmake_config.gni")
@@ -109,6 +108,12 @@
 "LLVM_HOST_TRIPLE=$llvm_current_triple",
   ]
 
+  if (llvm_enable_dia_sdk) {
+extra_values += [ "LLVM_ENABLE_DIA_SDK=1" ]
+  } else {
+extra_values += [ "LLVM_ENABLE_DIA_SDK=0" ]  # Must be 0.
+  }
+
   if (llvm_enable_zlib) {
 extra_values += [ "LLVM_ENABLE_ZLIB=1" ]
   } else {
Index: lldb/test/Shell/lit.site.cfg.py.in
===
--- lldb/test/Shell/lit.site.cfg.py.in
+++ lldb/test/Shell/lit.site.cfg.py.in
@@ -16,6 +16,7 @@
 config.target_triple = "@TARGET_TRIPLE@"
 config.python_executable = "@Python3_EXECUTABLE@"
 config.have_zlib = @LLVM_ENABLE_ZLIB@
+config.have_dia_sdk = @LLVM_ENABLE_DIA_SDK@
 config.lldb_enable_lzma = @LLDB_ENABLE_LZMA@
 config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.lldb_bitness = 64 if @LLDB_IS_64_BITS@ else 32
Index: lldb/test/Shell/lit.cfg.py
===
--- lldb/test/Shell/lit.cfg.py
+++ lldb/test/Shell/lit.cfg.py
@@ -56,6 +56,15 @@
   lit_config.note("Running Shell tests in {} mode.".format(lldb_repro_mode))
   toolchain.use_lldb_repro_substitutions(config, lldb_repro_mode)
 
+# Do DIA check early, so that the warning shows up above the config.py notes.
+if sys.platform != 'win32':
+  config.available_features.add('host-breakpoints')
+else:
+  if config.have_dia_sdk:
+config.available_features.add('host-breakpoints')
+  else:
+lit_config.warning('lldb built without DIA SDK, several tests will be skipped')
+
 llvm_config.use_default_substitutions()
 toolchain.use_lldb_substitutions(config)
 toolchain.use_support_substitutions(config)
Index: lldb/test/Shell/Watchpoint/SetErrorCases.test
===
--- lldb/test/Shell/Watchpoint/SetErrorCases.test
+++ lldb/test/Shell/Watchpoint/SetErrorCases.test
@@ -1,3 +1,4 @@
+# REQUIRES: host-breakpoints
 # RUN: %clangxx_host %p/Inputs/main.cpp -g -o %t.out
 # RUN: %lldb -b -o 'settings set interpreter.stop-command-source-on-error false' -s %s %t.out 2>&1 | FileCheck %s
 
Index: lldb/test/Shell/SymbolFile/PDB/lit.local.cfg
===
--- lldb/test/Shell/SymbolFile/PDB/lit.local.cfg
+++ lldb/test/Shell/SymbolFile/PDB/lit.local.cfg
@@ -1,2 +1,3 @@
-if 'lldb-repro' in config.available_features:
+if (not config.have_dia_sdk or
+'lldb-repro' in config.available_features):
   config.unsupported = True
Index: lldb/test/Shell/SymbolFile/NativePDB/stack_unwinding01.cpp
===
--- lldb/test/Shell/SymbolFile/NativePDB/stack_unwinding01.cpp
+++ lldb/test/Shell/SymbolFile/NativePDB/stack_unwinding01.cpp
@@ -1,5 +1,5 @@
 // clang-format off
-// REQUIRES: lld, system-windows
+// REQUIRES: lld, system-windows, host-breakpoints
 
 // RUN: %build --compiler=clang-cl --nodefaultlib -o %t.exe -- %s
 // RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe -s \
Index: lldb/test/Shell/Settings/TestLineMarkerColor.test
==

[Lldb-commits] [PATCH] D109738: [lldb] Fix bug 38317 - Address breakpoints don't work if set before the process launches

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

In D109738#3001122 , @vadimcn wrote:

>> In the future, can you generate patches with context (i.e. pass -U to
>> "git diff" or "git show"), it's a lot easier to read patches with context.
>
> Sure thing, will do.
>
> This doesn't seem like the right solution to the problem to me.  The way
>
>> address breakpoints SHOULD work  is that when you set the breakpoint, we
>> resolve the address you pass in against the images in the target.  If the
>> address is in an image in the list, then we convert the load address to a
>> section relative address before making the breakpoint.  Address breakpoints
>> made that way will always return true for m_addr.GetSection(), and so set
>> re_resolve to true before getting to the test you modified.
>
> No, actually.   In my scenario, the breakpoint is being created by load
> address (via SBTarget::BreakpointCreateByAddress(addr_t address)), right
> after the target has been created.   Since the process hasn't been launched
> at this point yet, there are no code sections mapped, so the breakpoint
> address stays non-section-relative.   My understanding is that in such a
> case, LLDB should attempt to re-resolve it upon loading each new module.

When the target is created, the modules for the target are loaded and mapped at 
whatever address is their default load address.
So all the libraries that lldb can find will be listed in the shared library 
list at that point, mapped at their pre-load addresses.  
On Darwin systems the dynamic loader plugin reads the closure of the loaded 
libraries from the binary's load commands, and loads them all.  
That's not a requirement of the dynamic loader, however, so you may just have 
the executable loaded.  But you will always have that.

The initial example you showed when proposing the patch has you finding the 
address of main, then getting its address and setting
an address breakpoint on that address.  So clearly there was some addresses 
associated with the main executable at that point, and
we could have re-resolved the address to a section relative one correctly.

There are problems with this approach for sure.  Mainly that there's no 
guarantee before run that all the libraries lldb detects
will have unique addresses.  It's pretty common for the linker to assign a 
default load address, often 0x0, for all shared libraries.
So before run, an address breakpoint is ambiguous, it could map to many 
section/offset pairs.

lldb doesn't usually fail to make breakpoints (it does, for instance, for a 
regex breakpoint where the regex doesn't compile...).  
But in this case, I think it should because there's no way to tell what the 
user's intentions were.  For instance, in your initial example 
where you got the symbol for main pre-run, then used that address to set an 
address breakpoint, say offset a bit from main or whatever.  
If on run, the breakpoint was resolved to whatever library happened to load at 
that address, you would not be best pleased.

>> OTOH, lldb is a bit hesitant to reset raw load address breakpoints that
>> aren't in any module.  After all, you have no idea, what with loading &
>> unloading & the effects of ASLR, a raw address doesn't have much meaning
>> run to run.
>
> LLDB disables ASLR by default, so load addresses are usually stable.

lldb ASKS to disable ASLR by default.  The systems that lldb runs on are under 
no obligation to honor 
that request, and haven't on all iOS devices for the past 5 years or 
thereabouts.  You can't assume this will
get you out of trouble.

> Also, please see Ted's comment about embedded.

If you mean the comment with the example of getting main's address and then 
setting an address
breakpoint there, as I argued above, I think that more tends to argue we want 
to convert addresses
to section relative whenever we can.  The user's clear intention was to set the 
breakpoint on the address
that "main" was going to show up at, not at whatever code happens to end up at 
the pre-load address of main.

>> In your case, you have an address that's clearly in a section, so it
>> should have been handled by the if(m_addr.GetSection() ||
>> m_module_filespec) test.  So the correct fix is to figure out how you ended
>> up with a non-section relative address for the breakpoint.
>>
>> I don't want lldb to silently reset non-section relative addresses.  The
>> only way those should show up is if you set the breakpoint on some code
>> that was NOT in any section, maybe JIT code or something, and lldb has no
>> way of knowing what to do with those addresses on re-run.  Refusing to
>> re-resolve them may be too harsh, but we should at least warn about them.
>> Whereas if the address is section relative, we can always do the right
>> thing.
>
> I don't see what's the problem with resolving them.  If I set a breakpoint
> on a raw memory address, I expect it to be hit as soon as there's some code
> mapped and executed at that loca

[Lldb-commits] [PATCH] D109738: [lldb] Fix bug 38317 - Address breakpoints don't work if set before the process launches

[Vadim Chugunov via Phabricator via lldb-commits]

vadimcn added a comment.

Hi Jim,
I think there's a bit of confusion going on here:
The original bug was opened by Ted Woodward, and I think his scenario was
motivated by embedded debugging.   He also provided that example with a
breakpoint being set in main.

I've rediscovered this bug independently, while working on an IDE
integration project.  In my case, the IDE saves just the address of the
instruction where a breakpoint was set in the previous debugging session.
In the next session, the debug adapter is expected to restore instruction
breakpoints using just the absolute addresses; the protocol does not
provide for any user interaction.  Also, this happens via the SB API, not
via LLDB commands.   Fortunately, when ASLR is disabled, modules are
usually loaded at the same address, so things tend to work out 99% of the
time.  The other 1%... well, if you are debugging on the assembly
instruction level, you are kinda expected to know what you are doing and be
prepared to deal with problems like those you've described above.

When the target is created, the modules for the target are loaded and

> mapped at whatever address is their default load address.

Clearly, this isn't happening.  From what I can see, section addresses are
not known until the process starts running.   Maybe this is another bug
that needs fixing?
Even so, I think that address breakpoints should still function when
section addresses aren't known at breakpoint creation time (for whatever
reason), if only on a best-effort basis.

3. If the address doesn't fall into any sections, then it would be fine to

> assign it to the first section that shows up at that address, but I would
> convert it to section relative at that point.

This is what I am attempting to do with my patch.  Is there something else
I should do to make sure I am not regressing other scenarios?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109738

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

[Lldb-commits] [PATCH] D109795: [MachCore] Report arm64 thread exception state

[Vedant Kumar via Phabricator via lldb-commits]

vsk updated this revision to Diff 372809.
vsk marked 3 inline comments as done.
vsk added a comment.

Address review feedback:

- Include far register in output, which now looks like e.g.:

  (lldb) bt all
  * thread #1, stop reason = ESR_EC_DABORT_EL0 (fault address: 
0x6261747563657860)
* frame #0: 0x0001aa5c2864 libsystem_platform.dylib`_platform_strlen + 4



- Include note that the exception class list can/does contain some 
Apple-specific classes, but that it's largely 1:1 with the ARM spec.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109795

Files:
  lldb/include/lldb/Target/AppleArm64ExceptionClass.def
  lldb/include/lldb/Target/AppleArm64ExceptionClass.h
  lldb/include/lldb/module.modulemap
  lldb/source/Plugins/Process/mach-core/ThreadMachCore.cpp

Index: lldb/source/Plugins/Process/mach-core/ThreadMachCore.cpp
===
--- lldb/source/Plugins/Process/mach-core/ThreadMachCore.cpp
+++ lldb/source/Plugins/Process/mach-core/ThreadMachCore.cpp
@@ -9,7 +9,9 @@
 #include "ThreadMachCore.h"
 
 #include "lldb/Breakpoint/Watchpoint.h"
+#include "lldb/Host/SafeMachO.h"
 #include "lldb/Symbol/ObjectFile.h"
+#include "lldb/Target/AppleArm64ExceptionClass.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/RegisterContext.h"
 #include "lldb/Target/StopInfo.h"
@@ -17,6 +19,7 @@
 #include "lldb/Target/Unwind.h"
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/DataExtractor.h"
+#include "lldb/Utility/RegisterValue.h"
 #include "lldb/Utility/State.h"
 #include "lldb/Utility/StreamString.h"
 
@@ -91,7 +94,40 @@
 bool ThreadMachCore::CalculateStopInfo() {
   ProcessSP process_sp(GetProcess());
   if (process_sp) {
-SetStopInfo(StopInfo::CreateStopReasonWithSignal(*this, SIGSTOP));
+StopInfoSP stop_info;
+RegisterContextSP reg_ctx_sp = GetRegisterContext();
+
+if (reg_ctx_sp) {
+  Target &target = process_sp->GetTarget();
+  const ArchSpec arch_spec = target.GetArchitecture();
+  const uint32_t cputype = arch_spec.GetMachOCPUType();
+
+  if (cputype == llvm::MachO::CPU_TYPE_ARM64 ||
+  cputype == llvm::MachO::CPU_TYPE_ARM64_32) {
+const RegisterInfo *esr_info = reg_ctx_sp->GetRegisterInfoByName("esr");
+const RegisterInfo *far_info = reg_ctx_sp->GetRegisterInfoByName("far");
+RegisterValue esr, far;
+if (reg_ctx_sp->ReadRegister(esr_info, esr) &&
+reg_ctx_sp->ReadRegister(far_info, far)) {
+  const uint32_t esr_val = esr.GetAsUInt32();
+  const AppleArm64ExceptionClass exception_class =
+  getAppleArm64ExceptionClass(esr_val);
+  if (exception_class !=
+  AppleArm64ExceptionClass::ESR_EC_UNCATEGORIZED) {
+StreamString S;
+S.Printf("%s (fault address: 0x%" PRIx64 ")",
+ toString(exception_class), far.GetAsUInt64());
+stop_info =
+StopInfo::CreateStopReasonWithException(*this, S.GetData());
+  }
+}
+  }
+}
+
+if (!stop_info)
+  stop_info = StopInfo::CreateStopReasonWithSignal(*this, SIGSTOP);
+
+SetStopInfo(stop_info);
 return true;
   }
   return false;
Index: lldb/include/lldb/module.modulemap
===
--- lldb/include/lldb/module.modulemap
+++ lldb/include/lldb/module.modulemap
@@ -119,6 +119,7 @@
 requires cplusplus
 
 umbrella "Target"
+textual header "Target/AppleArm64ExceptionClass.def"
 module * { export * }
   }
 }
Index: lldb/include/lldb/Target/AppleArm64ExceptionClass.h
===
--- /dev/null
+++ lldb/include/lldb/Target/AppleArm64ExceptionClass.h
@@ -0,0 +1,48 @@
+//===-- AppleArm64ExceptionClass.h --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_TARGET_APPLEARM64EXCEPTIONCLASS_H
+#define LLDB_TARGET_APPLEARM64EXCEPTIONCLASS_H
+
+namespace lldb_private {
+
+enum class AppleArm64ExceptionClass : unsigned {
+#define APPLE_ARM64_EXCEPTION_CLASS(Name, Code) Name = Code,
+#include "AppleArm64ExceptionClass.def"
+};
+
+/// Get the Apple ARM64 exception class encoded within \p esr.
+inline AppleArm64ExceptionClass getAppleArm64ExceptionClass(uint32_t esr) {
+  /*
+   * Exception Syndrome Register
+   *
+   *  31  26 25 24   0
+   * +--+--+--+
+   * |  EC  |IL|   ISS|
+   * +--+--+--+
+   *
+   * EC  - Exception Class
+   * IL  - Instruction Length
+   * ISS - Instruction Specific Syndrome
+   */
+  return

[Lldb-commits] [PATCH] D109795: [MachCore] Report arm64 thread exception state

[Vedant Kumar via Phabricator via lldb-commits]

vsk added inline comments.



Comment at: lldb/include/lldb/Target/AppleArm64ExceptionClass.h:14
+
+enum class AppleArm64ExceptionClass : unsigned {
+#define APPLE_ARM64_EXCEPTION_CLASS(Name, Code) Name = Code,

shafik wrote:
> We should use fixed sized integer types whenever possible, I suppose in this 
> case `uint32_t`.
I don't think we should use fixed size integer types in cases where the size 
doesn't matter (e.g. when uint8_t would do, as it would in this case), since 
it's distracting.



Comment at: lldb/source/Plugins/Process/mach-core/ThreadMachCore.cpp:112
+  auto exception_class =
+  static_cast(esr_val >> 26);
+  if (exception_class !=

jasonmolenda wrote:
> shafik wrote:
> > Does `26` have a meaning? I am guessing we are shifting to get the EC bits?
> Yeah I think the comment table in AppleArm64ExceptionClass.def would be 
> better placed here, showing the bit positions of the three fields in the esr 
> register probably.  AppleArm64ExceptionClass.def should make it clear that it 
> is operating on the exception class field of the esr register, but the bit 
> positions are more relevant here.
Good point - actually, maybe it's cleaner to keep knowledge of the EC bit 
position in the header. Lmk what you think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109795

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

[Lldb-commits] [PATCH] D109795: [MachCore] Report arm64 thread exception state

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added inline comments.



Comment at: lldb/source/Plugins/Process/mach-core/ThreadMachCore.cpp:112
+  auto exception_class =
+  static_cast(esr_val >> 26);
+  if (exception_class !=

vsk wrote:
> jasonmolenda wrote:
> > shafik wrote:
> > > Does `26` have a meaning? I am guessing we are shifting to get the EC 
> > > bits?
> > Yeah I think the comment table in AppleArm64ExceptionClass.def would be 
> > better placed here, showing the bit positions of the three fields in the 
> > esr register probably.  AppleArm64ExceptionClass.def should make it clear 
> > that it is operating on the exception class field of the esr register, but 
> > the bit positions are more relevant here.
> Good point - actually, maybe it's cleaner to keep knowledge of the EC bit 
> position in the header. Lmk what you think.
I was thinking the same thing, that `getAppleArm64ExceptionClass` might just 
take the $esr register value and get the necessary bits from it.  I guess every 
caller of this is probably starting with the ESR register value, so having 
every caller know how to grab the EC value from ESR doesn't seem good.  Some 
callers might want to use EC/IL/ISS to describe the nature of the error so they 
may need to know where these bits are in the register, but if they just want 
the EC name, it's unnecessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109795

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

[Lldb-commits] [PATCH] D109795: [MachCore] Report arm64 thread exception state

[Vedant Kumar via Phabricator via lldb-commits]

vsk added a comment.

FTR, I was able to write a test using `process save-core`, but even with `-s 
dirty` this is generating gigabytes of data and would presumably be super-flaky 
on the bots.
I also tried `-s stack`, but for the test program I passed in (just some 2-line 
.c file that dereferences NULL), `process save-core -s stack ...` generated an 
invalid .core with no sections, and lldb couldn't parse it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109795

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

[Lldb-commits] [PATCH] D109738: [lldb] Fix bug 38317 - Address breakpoints don't work if set before the process launches

[Ted Woodward via Phabricator via lldb-commits]

ted added a comment.

In D109738#3000608 , @jingham wrote:

> BTW, do you know what change made this stop working?

https://github.com/llvm-mirror/lldb/commit/43793406 in the old multirepo. In 
the monorepo, it's https://github.com/llvm/llvm-project/commit/5ec7cd7 .

We've got a conflict between "clean up old breakpoint addresses left behind 
when a section is deleted" vs "keep around breakpoint addressed that are set 
explicitly, but aren't part of a loaded section".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109738

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

[Lldb-commits] [lldb] 1758953 - [lldb-vscode] Fix focus thread when previous thread exits

[Ted Woodward via lldb-commits]

Author: Ted Woodward
Date: 2021-09-15T18:09:32-05:00
New Revision: 17589538aaef2b5ae27a0bfeb4346aff433aa59d

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

LOG: [lldb-vscode] Fix focus thread when previous thread exits

The thread that Visual Studio Code displays on a stop is called the focus 
thread. When the previous focus thread exits and we stop in a new thread, 
lldb-vscode does not tell vscode to set the new thread as the focus thread, so 
it selects the first thread in the thread list.

This patch changes lldb-vscode to tell vscode that the new thread is the focus 
thread. It also includes a test that verifies the DAP stop message for this 
case contains the correct values.

Reviewed By: clayborg, wallace

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

Added: 
lldb/test/API/tools/lldb-vscode/correct-thread/Makefile
lldb/test/API/tools/lldb-vscode/correct-thread/TestVSCode_correct_thread.py
lldb/test/API/tools/lldb-vscode/correct-thread/main.c

Modified: 
lldb/tools/lldb-vscode/lldb-vscode.cpp

Removed: 




diff  --git a/lldb/test/API/tools/lldb-vscode/correct-thread/Makefile 
b/lldb/test/API/tools/lldb-vscode/correct-thread/Makefile
new file mode 100644
index 0..121868fa8ec33
--- /dev/null
+++ b/lldb/test/API/tools/lldb-vscode/correct-thread/Makefile
@@ -0,0 +1,4 @@
+C_SOURCES := main.c
+CFLAGS_EXTRAS := -lpthread
+
+include Makefile.rules

diff  --git 
a/lldb/test/API/tools/lldb-vscode/correct-thread/TestVSCode_correct_thread.py 
b/lldb/test/API/tools/lldb-vscode/correct-thread/TestVSCode_correct_thread.py
new file mode 100644
index 0..4b7b03745692e
--- /dev/null
+++ 
b/lldb/test/API/tools/lldb-vscode/correct-thread/TestVSCode_correct_thread.py
@@ -0,0 +1,47 @@
+"""
+Test lldb-vscode setBreakpoints request
+"""
+
+from __future__ import print_function
+
+import unittest2
+import vscode
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+import lldbvscode_testcase
+
+
+class TestVSCode_correct_thread(lldbvscode_testcase.VSCodeTestCaseBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipIfWindows
+@skipIfRemote
+def test_correct_thread(self):
+'''
+Tests that the correct thread is selected if we continue from
+a thread that goes away and hit a breakpoint in another thread.
+In this case, the selected thread should be the thread that
+just hit the breakpoint, and not the first thread in the list.
+'''
+program = self.getBuildArtifact("a.out")
+self.build_and_launch(program)
+source = 'main.c'
+breakpoint_line = line_number(source, '// break here')
+lines = [breakpoint_line]
+# Set breakpoint in the thread function
+breakpoint_ids = self.set_source_breakpoints(source, lines)
+self.assertEqual(len(breakpoint_ids), len(lines),
+"expect correct number of breakpoints")
+self.continue_to_breakpoints(breakpoint_ids)
+# We're now stopped at the breakpoint in the first thread, thread #2.
+# Continue to join the first thread and hit the breakpoint in the
+# second thread, thread #3.
+self.vscode.request_continue()
+stopped_event = self.vscode.wait_for_stopped()
+# Verify that the description is the relevant breakpoint,
+# preserveFocusHint is False and threadCausedFocus is True
+
self.assertTrue(stopped_event[0]['body']['description'].startswith('breakpoint 
%s.' % breakpoint_ids[0]))
+self.assertFalse(stopped_event[0]['body']['preserveFocusHint'])
+self.assertTrue(stopped_event[0]['body']['threadCausedFocus'])

diff  --git a/lldb/test/API/tools/lldb-vscode/correct-thread/main.c 
b/lldb/test/API/tools/lldb-vscode/correct-thread/main.c
new file mode 100644
index 0..157c3f994db1e
--- /dev/null
+++ b/lldb/test/API/tools/lldb-vscode/correct-thread/main.c
@@ -0,0 +1,23 @@
+#include 
+#include 
+
+int state_var;
+
+void *thread (void *in)
+{
+  state_var++; // break here
+  return NULL;
+}
+
+int main(int argc, char **argv)
+{
+  pthread_t t1, t2;
+
+  pthread_create(&t1, NULL, *thread, NULL);
+  pthread_join(t1, NULL);
+  pthread_create(&t2, NULL, *thread, NULL);
+  pthread_join(t2, NULL);
+
+  printf("state_var is %d\n", state_var);
+  return 0;
+}

diff  --git a/lldb/tools/lldb-vscode/lldb-vscode.cpp 
b/lldb/tools/lldb-vscode/lldb-vscode.cpp
index 59d8debe6378c..5c237c00deabe 100644
--- a/lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ b/lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -232,13 +232,17 @@ void SendThreadStoppedEvent() {
   // set it as the focus thread if below if needed.
   lldb::tid_t first_tid

[Lldb-commits] [PATCH] D109633: [lldb-vscode] Fix focus thread when previous thread exits

[Ted Woodward via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG17589538aaef: [lldb-vscode] Fix focus thread when previous 
thread exits (authored by ted).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109633

Files:
  lldb/test/API/tools/lldb-vscode/correct-thread/Makefile
  lldb/test/API/tools/lldb-vscode/correct-thread/TestVSCode_correct_thread.py
  lldb/test/API/tools/lldb-vscode/correct-thread/main.c
  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
@@ -232,13 +232,17 @@
   // set it as the focus thread if below if needed.
   lldb::tid_t first_tid_with_reason = LLDB_INVALID_THREAD_ID;
   uint32_t num_threads_with_reason = 0;
+  bool focus_thread_exists = false;
   for (uint32_t thread_idx = 0; thread_idx < num_threads; ++thread_idx) {
 lldb::SBThread thread = process.GetThreadAtIndex(thread_idx);
 const lldb::tid_t tid = thread.GetThreadID();
 const bool has_reason = ThreadHasStopReason(thread);
 // If the focus thread doesn't have a stop reason, clear the thread ID
-if (tid == g_vsc.focus_tid && !has_reason)
-  g_vsc.focus_tid = LLDB_INVALID_THREAD_ID;
+if (tid == g_vsc.focus_tid) {
+  focus_thread_exists = true;
+  if (!has_reason)
+g_vsc.focus_tid = LLDB_INVALID_THREAD_ID;
+}
 if (has_reason) {
   ++num_threads_with_reason;
   if (first_tid_with_reason == LLDB_INVALID_THREAD_ID)
@@ -246,10 +250,10 @@
 }
   }
 
-  // We will have cleared g_vsc.focus_tid if he focus thread doesn't
-  // have a stop reason, so if it was cleared, or wasn't set, then set the
-  // focus thread to the first thread with a stop reason.
-  if (g_vsc.focus_tid == LLDB_INVALID_THREAD_ID)
+  // We will have cleared g_vsc.focus_tid if he focus thread doesn't have
+  // a stop reason, so if it was cleared, or wasn't set, or doesn't exist,
+  // then set the focus thread to the first thread with a stop reason.
+  if (!focus_thread_exists || g_vsc.focus_tid == LLDB_INVALID_THREAD_ID)
 g_vsc.focus_tid = first_tid_with_reason;
 
   // If no threads stopped with a reason, then report the first one so
Index: lldb/test/API/tools/lldb-vscode/correct-thread/main.c
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/correct-thread/main.c
@@ -0,0 +1,23 @@
+#include 
+#include 
+
+int state_var;
+
+void *thread (void *in)
+{
+  state_var++; // break here
+  return NULL;
+}
+
+int main(int argc, char **argv)
+{
+  pthread_t t1, t2;
+
+  pthread_create(&t1, NULL, *thread, NULL);
+  pthread_join(t1, NULL);
+  pthread_create(&t2, NULL, *thread, NULL);
+  pthread_join(t2, NULL);
+
+  printf("state_var is %d\n", state_var);
+  return 0;
+}
Index: lldb/test/API/tools/lldb-vscode/correct-thread/TestVSCode_correct_thread.py
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/correct-thread/TestVSCode_correct_thread.py
@@ -0,0 +1,47 @@
+"""
+Test lldb-vscode setBreakpoints request
+"""
+
+from __future__ import print_function
+
+import unittest2
+import vscode
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+import lldbvscode_testcase
+
+
+class TestVSCode_correct_thread(lldbvscode_testcase.VSCodeTestCaseBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipIfWindows
+@skipIfRemote
+def test_correct_thread(self):
+'''
+Tests that the correct thread is selected if we continue from
+a thread that goes away and hit a breakpoint in another thread.
+In this case, the selected thread should be the thread that
+just hit the breakpoint, and not the first thread in the list.
+'''
+program = self.getBuildArtifact("a.out")
+self.build_and_launch(program)
+source = 'main.c'
+breakpoint_line = line_number(source, '// break here')
+lines = [breakpoint_line]
+# Set breakpoint in the thread function
+breakpoint_ids = self.set_source_breakpoints(source, lines)
+self.assertEqual(len(breakpoint_ids), len(lines),
+"expect correct number of breakpoints")
+self.continue_to_breakpoints(breakpoint_ids)
+# We're now stopped at the breakpoint in the first thread, thread #2.
+# Continue to join the first thread and hit the breakpoint in the
+# second thread, thread #3.
+self.vscode.request_continue()
+stopped_event = 

[Lldb-commits] [PATCH] D109785: [lldb] Refactor and rename CPlusPlusLanguage::FindAlternateFunctionManglings

[Greg Clayton via Phabricator via lldb-commits]

clayborg accepted this revision.
clayborg added inline comments.
This revision is now accepted and ready to land.



Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h:130
 
-  // Given a mangled function name, calculates some alternative manglings since
-  // the compiler mangling may not line up with the symbol we are expecting
-  static uint32_t
-  FindAlternateFunctionManglings(const ConstString mangled,
- std::set &candidates);
+  std::set
+  GenerateAlternateFunctionManglings(const ConstString mangled) const override;

Do we need to be using a std::set? Do we need this to be an ordered result? I 
would think a std::vector would be better no?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109785

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

[Lldb-commits] [PATCH] D109785: [lldb] Refactor and rename CPlusPlusLanguage::FindAlternateFunctionManglings

[Alex Langford via Phabricator via lldb-commits]

bulbazord added inline comments.



Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h:130
 
-  // Given a mangled function name, calculates some alternative manglings since
-  // the compiler mangling may not line up with the symbol we are expecting
-  static uint32_t
-  FindAlternateFunctionManglings(const ConstString mangled,
- std::set &candidates);
+  std::set
+  GenerateAlternateFunctionManglings(const ConstString mangled) const override;

clayborg wrote:
> Do we need to be using a std::set? Do we need this to be an ordered result? I 
> would think a std::vector would be better no?
I'm not sure why it was a `std::set` to begin with but I don't see any reason 
why it can't be a `std::vector` instead. I'll update this before I land it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109785

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

[Lldb-commits] [PATCH] D109795: [MachCore] Report arm64 thread exception state

[Vedant Kumar via Phabricator via lldb-commits]

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

- Add a test (thanks @jasonmolenda!).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109795

Files:
  lldb/include/lldb/Target/AppleArm64ExceptionClass.def
  lldb/include/lldb/Target/AppleArm64ExceptionClass.h
  lldb/include/lldb/module.modulemap
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Plugins/Process/mach-core/ThreadMachCore.cpp
  lldb/test/API/macosx/corefile-exception-reason/Makefile
  lldb/test/API/macosx/corefile-exception-reason/TestCorefileExceptionReason.py
  lldb/test/API/macosx/corefile-exception-reason/main.c

Index: lldb/test/API/macosx/corefile-exception-reason/main.c
===
--- /dev/null
+++ lldb/test/API/macosx/corefile-exception-reason/main.c
@@ -0,0 +1,5 @@
+#include 
+int main() {
+  volatile int *p = NULL; // break here
+  return *p;
+}
Index: lldb/test/API/macosx/corefile-exception-reason/TestCorefileExceptionReason.py
===
--- /dev/null
+++ lldb/test/API/macosx/corefile-exception-reason/TestCorefileExceptionReason.py
@@ -0,0 +1,43 @@
+"""Test that lldb can report the exception reason for threads in a corefile."""
+
+import os
+import re
+import subprocess
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestCorefileExceptionReason(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipIfOutOfTreeDebugserver  # newer debugserver required for these qMemoryRegionInfo types
+@no_debug_info_test
+@skipUnlessDarwin
+@skipIf(archs=['i386','x86_64']) # exception codes not yet supported for Intel macs
+def test(self):
+
+corefile = self.getBuildArtifact("process.core")
+self.build()
+(target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
+self, "// break here", lldb.SBFileSpec("main.c"))
+
+self.runCmd("continue")
+
+self.runCmd("process save-core -s stack " + corefile)
+process.Kill()
+self.dbg.DeleteTarget(target)
+
+# Now load the corefile
+target = self.dbg.CreateTarget('')
+process = target.LoadCore(corefile)
+thread = process.GetSelectedThread()
+self.assertTrue(process.GetSelectedThread().IsValid())
+if self.TraceOn():
+self.runCmd("image list")
+self.runCmd("bt")
+self.runCmd("fr v")
+
+self.assertTrue(thread.GetStopDescription(256) == "ESR_EC_DABORT_EL0 (fault address: 0x0)")
Index: lldb/test/API/macosx/corefile-exception-reason/Makefile
===
--- /dev/null
+++ lldb/test/API/macosx/corefile-exception-reason/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES = main.c
+
+include Makefile.rules
Index: lldb/source/Plugins/Process/mach-core/ThreadMachCore.cpp
===
--- lldb/source/Plugins/Process/mach-core/ThreadMachCore.cpp
+++ lldb/source/Plugins/Process/mach-core/ThreadMachCore.cpp
@@ -9,7 +9,9 @@
 #include "ThreadMachCore.h"
 
 #include "lldb/Breakpoint/Watchpoint.h"
+#include "lldb/Host/SafeMachO.h"
 #include "lldb/Symbol/ObjectFile.h"
+#include "lldb/Target/AppleArm64ExceptionClass.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/RegisterContext.h"
 #include "lldb/Target/StopInfo.h"
@@ -17,6 +19,7 @@
 #include "lldb/Target/Unwind.h"
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/DataExtractor.h"
+#include "lldb/Utility/RegisterValue.h"
 #include "lldb/Utility/State.h"
 #include "lldb/Utility/StreamString.h"
 
@@ -91,7 +94,40 @@
 bool ThreadMachCore::CalculateStopInfo() {
   ProcessSP process_sp(GetProcess());
   if (process_sp) {
-SetStopInfo(StopInfo::CreateStopReasonWithSignal(*this, SIGSTOP));
+StopInfoSP stop_info;
+RegisterContextSP reg_ctx_sp = GetRegisterContext();
+
+if (reg_ctx_sp) {
+  Target &target = process_sp->GetTarget();
+  const ArchSpec arch_spec = target.GetArchitecture();
+  const uint32_t cputype = arch_spec.GetMachOCPUType();
+
+  if (cputype == llvm::MachO::CPU_TYPE_ARM64 ||
+  cputype == llvm::MachO::CPU_TYPE_ARM64_32) {
+const RegisterInfo *esr_info = reg_ctx_sp->GetRegisterInfoByName("esr");
+const RegisterInfo *far_info = reg_ctx_sp->GetRegisterInfoByName("far");
+RegisterValue esr, far;
+if (reg_ctx_sp->ReadRegister(esr_info, esr) &&
+reg_ctx_sp->ReadRegister(far_info, far)) {
+  const uint32_t esr_val = esr.GetAsUInt32();
+  const AppleArm64ExceptionClass exception_class =
+  getAppleArm64ExceptionClass(esr_val);
+  if (exception_class !=
+  AppleArm64ExceptionClass::ESR_EC_UNCATEGORIZED) {
+ 

[Lldb-commits] [PATCH] D109795: [MachCore] Report arm64 thread exception state

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

very nice.




Comment at: 
lldb/test/API/macosx/corefile-exception-reason/TestCorefileExceptionReason.py:19
+@skipUnlessDarwin
+@skipIf(archs=['i386','x86_64']) # exception codes not yet supported for 
Intel macs
+def test(self):

also not supported on armv7k.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109795

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

[Lldb-commits] [PATCH] D109795: [MachCore] Report arm64 thread exception state

[Vedant Kumar via Phabricator via lldb-commits]

vsk added inline comments.



Comment at: 
lldb/test/API/macosx/corefile-exception-reason/TestCorefileExceptionReason.py:19
+@skipUnlessDarwin
+@skipIf(archs=['i386','x86_64']) # exception codes not yet supported for 
Intel macs
+def test(self):

jasonmolenda wrote:
> also not supported on armv7k.
Thanks. I'll fix this before landing the patch (eta tomorrow morning).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109795

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

[Lldb-commits] [PATCH] D109834: [lldb/win] Improve check-lldb-shell with LLVM_ENABLE_DIA_SDK=NO

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I fear this is going to be an endless whack-a-mole. There's no way every 
contributor will remember to (correctly) add this feature.


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

https://reviews.llvm.org/D109834

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