[Lldb-commits] [PATCH] D105998: Create synthetic symbol names on demand to improve memory consumption and startup times.

[walter erquinigo via Phabricator via lldb-commits]

wallace closed this revision.
wallace added a comment.

This was commited by ec1a49170129d 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105998

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

[Lldb-commits] [PATCH] D109695: [lldb] [Process/gdb-remote] Add x31 AArch64 register for gdbserver

[Pavel Labath via Phabricator via lldb-commits]

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

Let's take a step back. First, I'd like to understand why are you adding a 
whole new register, instead of just an alias (alt_name) for an existing 
register. And second, have you considered putting this into the ABI plugin, as 
that's where the rest of the "augmentation" code lives (or most of it, anyway).


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

https://reviews.llvm.org/D109695

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

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

[Pavel Labath via Phabricator via lldb-commits]

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

seems reasonable


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] D108831: [lldb] [gdb-remote] Add x86_64 pseudo-registers when using gdbserver

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

This is an interesting problem. I don't yet know how, but as I alluded to in 
the other patch, my instinct would be to find a way to put this into the ABI 
class, in order to keep the "fill in bits not provided by debug server" code in 
a single place.


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] D109695: [lldb] [Process/gdb-remote] Add x31 AArch64 register for gdbserver

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

mgorny added a comment.

In D109695#2999171 , @labath wrote:

> Let's take a step back. First, I'd like to understand why are you adding a 
> whole new register, instead of just an alias (alt_name) for an existing 
> register.

Well, I was thinking 'what if the register has already another alt_name?' While 
it's unlikely that such a thing would happen with gdbserver, it's valid input 
in the end, and adding a new register seemed a more reliable solution for 
arbitrary input.

> And second, have you considered putting this into the ABI plugin, as that's 
> where the rest of the "augmentation" code lives (or most of it, anyway).

I initially did, yes, and I've figured out that it doesn't belong there since 
1) these assignments aren't really specific to a single ABI but are general 
architecture characteristics, and 2) the logic is really specific to the 
gdb-remote plugin.

I mean, putting things like generic regno assignments into ABI makes sense 
because different ABIs could use different registers for given purpose. 
However, things like ESP being derived from RSP or aliasing sp to x31 seem 
ABI-independent.

That said, I don't feel strongly about it.


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

https://reviews.llvm.org/D109695

___
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.

Another question: should we aim to remove the 'redundant afterwards' bits from 
lldb-server, and fill them in from client side entirely? I don't think our 
process plugins really need knowledge of EAX etc., and it would reduce the 
duplication a fair bit.

Also I'm not sure if I like my code here, two questions inline.




Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext_x86.cpp:37-54
+static std::array partial_gpr_regs = {{
+{"rax", {{"eax"}, {"ax"}, {"ah"}, {"al"}}},
+{"rbx", {{"ebx"}, {"bx"}, {"bh"}, {"bl"}}},
+{"rcx", {{"ecx"}, {"cx"}, {"ch"}, {"cl"}}},
+{"rdx", {{"edx"}, {"dx"}, {"dh"}, {"dl"}}},
+{"rdi", {{"edi"}, {"di"}, {nullptr}, {"dil"}}},
+{"rsi", {{"esi"}, {"si"}, {nullptr}, {"sil"}}},

Also here I'm wondering if it wouldn't be better to just have an array of 
`{"ax", "bx", ...}` and construct all other names from that base, plus r8..r15 
via numeric `for` loop.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext_x86.cpp:66-75
+static std::array mm_regs = {{
+{"st0", "mm0"},
+{"st1", "mm1"},
+{"st2", "mm2"},
+{"st3", "mm3"},
+{"st4", "mm4"},
+{"st5", "mm5"},

I've been somewhat basing this thing on how our process plugins do registers 
but I'm wondering if it wouldn't be better to just do a `for (int x = 0; i < 8; 
i++)` loop and dynamically generate `st${x}` and `mm${x}` names.


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] [lldb] f22c63b - [lldb/test] Start pexpect tests with a custom HOME

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2021-09-14T15:17:10+02:00
New Revision: f22c63b41bda01163a88b0bb9887a9324810732f

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

LOG: [lldb/test] Start pexpect tests with a custom HOME

This addresses the flakyness of (at least) TestMultilineNavigation,
which was failing when the editline history of a concurrently executing
test made leaked in. Using a test-specific home directory ensures the
tests are independent.

Added: 


Modified: 
lldb/packages/Python/lldbsuite/test/lldbpexpect.py

Removed: 




diff  --git a/lldb/packages/Python/lldbsuite/test/lldbpexpect.py 
b/lldb/packages/Python/lldbsuite/test/lldbpexpect.py
index f7c0e490105af..d3e3e8dde5d84 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbpexpect.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbpexpect.py
@@ -36,7 +36,8 @@ def launch(self, executable=None, extra_args=None, 
timeout=60, dimensions=None):
 args.extend(extra_args)
 
 env = dict(os.environ)
-env["TERM"]="vt100"
+env["TERM"] = "vt100"
+env["HOME"] = self.getBuildDir()
 
 import pexpect
 self.child = pexpect.spawn(



___
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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D108831#2999227 , @mgorny wrote:

> Another question: should we aim to remove the 'redundant afterwards' bits 
> from lldb-server, and fill them in from client side entirely? I don't think 
> our process plugins really need knowledge of EAX etc., and it would reduce 
> the duplication a fair bit.

I think that would be great. Besides the deduplication aspect, this will also 
decrease the chances of some gdb compatibility bug creeping in. I might 
consider waiting for one release, just in case someone wants to connect a newer 
lldb-server to an old lldb.

/And/, I would actually like to go further than that, and remove these register 
definitions from core file (&minidump, etc) plugins too. Since eax/ax/ah/al can 
be extracted from rax in a generic way, I don't see a reason for each plugin to 
have to redefine those (or forget doing like, like is the case with the windows 
plugin). This is the point where this logic stops being gdb-remote-specific. 
There are no compatibility issues here, though the process itself might be 
trickier, depending on how much the plugins are assuming a particular register 
order, etc.


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] D109695: [lldb] [Process/gdb-remote] Add x31 AArch64 register for gdbserver

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D109695#2999221 , @mgorny wrote:

> In D109695#2999171 , @labath wrote:
>
>> Let's take a step back. First, I'd like to understand why are you adding a 
>> whole new register, instead of just an alias (alt_name) for an existing 
>> register.
>
> Well, I was thinking 'what if the register has already another alt_name?' 
> While it's unlikely that such a thing would happen with gdbserver, it's valid 
> input in the end, and adding a new register seemed a more reliable solution 
> for arbitrary input.

While we shouldn't crash on such an input, I am not particularly worried about 
everything still functioning perfectly. And adding a new register has 
user-visible consequences (two entries in `register read`).

Overall, I think I'd be fine both with overwriting the alt-name in this case 
(this is an lldb construct anyway) and with just doing nothing.

>> And second, have you considered putting this into the ABI plugin, as that's 
>> where the rest of the "augmentation" code lives (or most of it, anyway).
>
> I initially did, yes, and I've figured out that it doesn't belong there since 
> 1) these assignments aren't really specific to a single ABI but are general 
> architecture characteristics, and 2) the logic is really specific to the 
> gdb-remote plugin.
>
> I mean, putting things like generic regno assignments into ABI makes sense 
> because different ABIs could use different registers for given purpose. 
> However, things like ESP being derived from RSP or aliasing sp to x31 seem 
> ABI-independent.

That is true, and it may actually make more sense to put this into the 
Architecture plugin. However, I am not sure that this architectural purity is 
worth the hassle of having RegisterInfo knowledge in two places, especially 
since the ABI plugins are nowadays structured in a way that the all ABIs for a 
given architecture share a common base class, and this common logic could 
easily go there. (For that reason, I am also not sure we really need a separate 
Architecture hierarchy, but anyway...)


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

https://reviews.llvm.org/D109695

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

[Lldb-commits] [PATCH] D62732: [RISCV] Add SystemV ABI

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added a subscriber: felixonmars.
MaskRay added a comment.

Hi Luis, is this still needed after D86292 ? 
Or are there missing pieces?
@felixonmars reported that https://archriscv.felixc.at/.status/logs/lldb.log 
still failed to build on riscv64 Arch Linux.
I thought that you may have an idea :)

  /usr/bin/ld: lib/liblldbPluginProcessLinux.a(NativeThreadLinux.cpp.o): in 
function `.L181':
  
NativeThreadLinux.cpp:(.text._ZN12lldb_private13process_linux17NativeThreadLinuxC2ERNS0_18NativeProcessLinuxEm+0x7c):
 undefined reference to 
`lldb_private::process_linux::NativeRegisterContextLinux::CreateHostNativeRegisterContextLinux(lldb_private::ArchSpec
 const&, lldb_private::NativeThreadProtocol&)'
  collect2: error: ld returned 1 exit status


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62732

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

[Lldb-commits] [PATCH] D109483: [APInt] Normalize naming on keep constructors / predicate methods.

[Jay Foad via Phabricator via lldb-commits]

foad added a comment.

What is a "keep constructor"?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109483

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

[Lldb-commits] [PATCH] D62732: [RISCV] Add SystemV ABI

[Jessica Clarke via Phabricator via lldb-commits]

jrtc27 added a comment.

In D62732#2995111 , @MaskRay wrote:

> Hi Luís, is this still needed after D86292 ? 
> Or are there missing pieces?
> @felixonmars reported that https://archriscv.felixc.at/.status/logs/lldb.log 
> still failed to build on riscv64 Arch Linux.
> It's 12.0.0 but I thought that you may have an idea :)
>
>   /usr/bin/ld: lib/liblldbPluginProcessLinux.a(NativeThreadLinux.cpp.o): in 
> function `.L181':
>   
> NativeThreadLinux.cpp:(.text._ZN12lldb_private13process_linux17NativeThreadLinuxC2ERNS0_18NativeProcessLinuxEm+0x7c):
>  undefined reference to 
> `lldb_private::process_linux::NativeRegisterContextLinux::CreateHostNativeRegisterContextLinux(lldb_private::ArchSpec
>  const&, lldb_private::NativeThreadProtocol&)'
>   collect2: error: ld returned 1 exit status

That just teaches it that RISC-V exists, it can't really do anything useful 
with it, you need this patch to teach it about relevant registers, how to 
disassemble, etc, and even then only for bare-metal, there is no Linux or 
FreeBSD arch-specific plugin.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62732

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

[Lldb-commits] [PATCH] D109483: [APInt] Normalize naming on keep constructors / predicate methods.

[Chris Lattner via Phabricator via lldb-commits]

lattner added a comment.

> What is a "keep constructor"?

Good question, I'm not sure.  I think I meant to say "key constructors".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109483

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

Re: [Lldb-commits] [lldb] f22c63b - [lldb/test] Start pexpect tests with a custom HOME

[Raphael Isemann via lldb-commits]

Thanks! Just FYI, I was actually about to open a review for allowing
configuring the history directory which should also address this.

Am Di., 14. Sept. 2021 um 15:18 Uhr schrieb Pavel Labath via
lldb-commits :
>
>
> Author: Pavel Labath
> Date: 2021-09-14T15:17:10+02:00
> New Revision: f22c63b41bda01163a88b0bb9887a9324810732f
>
> URL: 
> https://github.com/llvm/llvm-project/commit/f22c63b41bda01163a88b0bb9887a9324810732f
> DIFF: 
> https://github.com/llvm/llvm-project/commit/f22c63b41bda01163a88b0bb9887a9324810732f.diff
>
> LOG: [lldb/test] Start pexpect tests with a custom HOME
>
> This addresses the flakyness of (at least) TestMultilineNavigation,
> which was failing when the editline history of a concurrently executing
> test made leaked in. Using a test-specific home directory ensures the
> tests are independent.
>
> Added:
>
>
> Modified:
> lldb/packages/Python/lldbsuite/test/lldbpexpect.py
>
> Removed:
>
>
>
> 
> diff  --git a/lldb/packages/Python/lldbsuite/test/lldbpexpect.py 
> b/lldb/packages/Python/lldbsuite/test/lldbpexpect.py
> index f7c0e490105af..d3e3e8dde5d84 100644
> --- a/lldb/packages/Python/lldbsuite/test/lldbpexpect.py
> +++ b/lldb/packages/Python/lldbsuite/test/lldbpexpect.py
> @@ -36,7 +36,8 @@ def launch(self, executable=None, extra_args=None, 
> timeout=60, dimensions=None):
>  args.extend(extra_args)
>
>  env = dict(os.environ)
> -env["TERM"]="vt100"
> +env["TERM"] = "vt100"
> +env["HOME"] = self.getBuildDir()
>
>  import pexpect
>  self.child = pexpect.spawn(
>
>
>
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
___
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 created this revision.
mstorsjo added a reviewer: labath.
Herald added a subscriber: kristof.beyls.
mstorsjo requested review of this revision.
Herald added a project: LLDB.

Based on suggestions by Eric Youngdale.

This fixes https://llvm.org/PR51673.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D109777

Files:
  lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp
  lldb/source/Plugins/Platform/Windows/PlatformWindows.h
  lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
  lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.h
  lldb/source/Plugins/Process/Windows/Common/NativeThreadWindows.cpp
  lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
  lldb/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp

Index: lldb/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp
===
--- lldb/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp
+++ lldb/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp
@@ -131,12 +131,29 @@
 return Status();
 
   if (resume_state == eStateStepping) {
+Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_THREAD));
+
 uint32_t flags_index =
 GetRegisterContext()->ConvertRegisterKindToRegisterNumber(
 eRegisterKindGeneric, LLDB_REGNUM_GENERIC_FLAGS);
 uint64_t flags_value =
 GetRegisterContext()->ReadRegisterAsUnsigned(flags_index, 0);
-flags_value |= 0x100; // Set the trap flag on the CPU
+ProcessSP process = GetProcess();
+const ArchSpec &arch = process->GetTarget().GetArchitecture();
+switch (arch.GetMachine()) {
+case llvm::Triple::x86:
+case llvm::Triple::x86_64:
+  flags_value |= 0x100; // Set the trap flag on the CPU
+  break;
+case llvm::Triple::aarch64:
+case llvm::Triple::arm:
+case llvm::Triple::thumb:
+  flags_value |= 0x20; // The SS bit in PState
+  break;
+default:
+  LLDB_LOG(log, "single stepping unsupported on this architecture");
+  break;
+}
 GetRegisterContext()->WriteRegisterFromUnsigned(flags_index, flags_value);
   }
 
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
@@ -438,8 +438,29 @@
   case EXCEPTION_BREAKPOINT: {
 RegisterContextSP register_context = stop_thread->GetRegisterContext();
 
-// The current EIP is AFTER the BP opcode, which is one byte.
-uint64_t pc = register_context->GetPC() - 1;
+int breakpoint_size = 1;
+switch (GetTarget().GetArchitecture().GetMachine()) {
+case llvm::Triple::aarch64:
+  breakpoint_size = 4;
+  break;
+
+case llvm::Triple::arm:
+case llvm::Triple::thumb:
+  breakpoint_size = 2;
+  break;
+
+case llvm::Triple::x86:
+case llvm::Triple::x86_64:
+  breakpoint_size = 1;
+  break;
+
+default:
+  LLDB_LOG(log, "Unknown breakpoint size for architecture");
+  break;
+}
+
+// The current EIP is AFTER the BP opcode.
+uint64_t pc = register_context->GetPC() - breakpoint_size;
 
 BreakpointSiteSP site(GetBreakpointSiteList().FindByAddress(pc));
 if (site) {
Index: lldb/source/Plugins/Process/Windows/Common/NativeThreadWindows.cpp
===
--- lldb/source/Plugins/Process/Windows/Common/NativeThreadWindows.cpp
+++ lldb/source/Plugins/Process/Windows/Common/NativeThreadWindows.cpp
@@ -48,12 +48,29 @@
 return Status();
 
   if (resume_state == eStateStepping) {
+Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_THREAD));
+
 uint32_t flags_index =
 GetRegisterContext().ConvertRegisterKindToRegisterNumber(
 eRegisterKindGeneric, LLDB_REGNUM_GENERIC_FLAGS);
 uint64_t flags_value =
 GetRegisterContext().ReadRegisterAsUnsigned(flags_index, 0);
-flags_value |= 0x100; // Set the trap flag on the CPU
+NativeProcessProtocol &process = GetProcess();
+const ArchSpec &arch = process.GetArchitecture();
+switch (arch.GetMachine()) {
+case llvm::Triple::x86:
+case llvm::Triple::x86_64:
+  flags_value |= 0x100; // Set the trap flag on the CPU
+  break;
+case llvm::Triple::aarch64:
+case llvm::Triple::arm:
+case llvm::Triple::thumb:
+  flags_value |= 0x20; // The SS bit in PState
+  break;
+default:
+  LLDB_LOG(log, "single stepping unsupported on this architecture");
+  break;
+}
 GetRegisterContext().WriteRegisterFromUnsigned(flags_index, flags_value);
   }
 
Index: lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.h
===
--- lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.h
+++ lldb/source/Plug

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

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added a reviewer: labath.
Herald added a subscriber: kristof.beyls.
mstorsjo requested review of this revision.
Herald added a project: LLDB.

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


Repository:
  rG LLVM Github Monorepo

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] D109779: [LLDB] [Minidump] Fix format string warnings on Windows

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo created this revision.
mstorsjo added a reviewer: labath.
mstorsjo requested review of this revision.
Herald added a project: LLDB.

These variables are 'size_t' and thus should use %zu. On Windows,
'long' is always 32 bit.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D109779

Files:
  lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp


Index: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
===
--- lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -729,7 +729,7 @@
   if (error.Fail() || bytes_written != header_size) {
 if (bytes_written != header_size)
   error.SetErrorStringWithFormat(
-  "Unable to write the header. (written %ld/%ld).", bytes_written,
+  "Unable to write the header. (written %zd/%zd).", bytes_written,
   header_size);
 return error;
   }
@@ -740,7 +740,7 @@
   if (error.Fail() || bytes_written != m_data.GetByteSize()) {
 if (bytes_written != m_data.GetByteSize())
   error.SetErrorStringWithFormat(
-  "Unable to write the data. (written %ld/%ld).", bytes_written,
+  "Unable to write the data. (written %zd/%zd).", bytes_written,
   m_data.GetByteSize());
 return error;
   }
@@ -752,7 +752,7 @@
 if (error.Fail() || bytes_written != directory_size) {
   if (bytes_written != directory_size)
 error.SetErrorStringWithFormat(
-"Unable to write the directory. (written %ld/%ld).", bytes_written,
+"Unable to write the directory. (written %zd/%zd).", bytes_written,
 directory_size);
   return error;
 }


Index: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
===
--- lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -729,7 +729,7 @@
   if (error.Fail() || bytes_written != header_size) {
 if (bytes_written != header_size)
   error.SetErrorStringWithFormat(
-  "Unable to write the header. (written %ld/%ld).", bytes_written,
+  "Unable to write the header. (written %zd/%zd).", bytes_written,
   header_size);
 return error;
   }
@@ -740,7 +740,7 @@
   if (error.Fail() || bytes_written != m_data.GetByteSize()) {
 if (bytes_written != m_data.GetByteSize())
   error.SetErrorStringWithFormat(
-  "Unable to write the data. (written %ld/%ld).", bytes_written,
+  "Unable to write the data. (written %zd/%zd).", bytes_written,
   m_data.GetByteSize());
 return error;
   }
@@ -752,7 +752,7 @@
 if (error.Fail() || bytes_written != directory_size) {
   if (bytes_written != directory_size)
 error.SetErrorStringWithFormat(
-"Unable to write the directory. (written %ld/%ld).", bytes_written,
+"Unable to write the directory. (written %zd/%zd).", bytes_written,
 directory_size);
   return error;
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D109779: [LLDB] [Minidump] Fix format string warnings on Windows

[Alex Langford via Phabricator via lldb-commits]

bulbazord added a reviewer: MaskRay.
bulbazord added a comment.

Adding MaskRay who fixed something similar to this yesterday.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109779

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

[Lldb-commits] [PATCH] D109779: [LLDB] [Minidump] Fix format string warnings on Windows

[Martin Storsjö via Phabricator via lldb-commits]

mstorsjo abandoned this revision.
mstorsjo added a comment.

Oh, sorry, I worked on a checkout a couple weeks old, where I ran into this 
warning. It does indeed seem to be fixed in the current git main branch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109779

___
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 created this revision.
bulbazord added reviewers: clayborg, teemperor, jingham, JDevlieghere, labath.
bulbazord requested review of this revision.
Herald added a project: LLDB.

I have 2 goals with this change:

1. Disambiguate between CPlusPlus::FindAlternateFunctionManglings and 
IRExecutionUnit::FindBestAlternateMangledName. These are named very similar 
things, they try to do very similar things, but their approaches are different. 
This change should make it clear that one is generating possible alternate 
manglings (through some heuristics-based approach) and the other is finding 
alternate manglings (through searching the SymbolFile for potential matches).
2. Change GenerateAlternateFunctionManglings from a static method in 
CPlusPlusLanguage to a virtual method in Language. This will allow us to remove 
a direct use of CPlusPlusLanguage in IRExecutionUnit, further pushing it to be 
more general. This change doesn't meet this goal completely but allows for it 
to happen later.

Though this doesn't remove IRExecutionUnit's dependency on
CPlusPlusLanguage, it does bring us closer to that goal.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D109785

Files:
  lldb/include/lldb/Target/Language.h
  lldb/source/Expression/IRExecutionUnit.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
  lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp

Index: lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
===
--- lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
+++ lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
@@ -7,6 +7,8 @@
 //===--===//
 #include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
 #include "Plugins/Language/CPlusPlus/CPlusPlusNameParser.h"
+#include "TestingSupport/SubsystemRAII.h"
+#include "lldb/lldb-enumerations.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -185,29 +187,32 @@
   "operator<=>", context, basename));
 }
 
-static std::set FindAlternate(llvm::StringRef Name) {
-  std::set Results;
-  uint32_t Count = CPlusPlusLanguage::FindAlternateFunctionManglings(
-  ConstString(Name), Results);
-  EXPECT_EQ(Count, Results.size());
+static std::set GenerateAlternate(llvm::StringRef Name) {
   std::set Strings;
-  for (ConstString Str : Results)
-Strings.insert(std::string(Str.GetStringRef()));
+  if (Language *CPlusPlusLang =
+  Language::FindPlugin(lldb::eLanguageTypeC_plus_plus)) {
+std::set Results =
+CPlusPlusLang->GenerateAlternateFunctionManglings(ConstString(Name));
+for (ConstString Str : Results)
+  Strings.insert(std::string(Str.GetStringRef()));
+  }
   return Strings;
 }
 
-TEST(CPlusPlusLanguage, FindAlternateFunctionManglings) {
+TEST(CPlusPlusLanguage, GenerateAlternateFunctionManglings) {
   using namespace testing;
 
-  EXPECT_THAT(FindAlternate("_ZN1A1fEv"),
+  SubsystemRAII lang;
+
+  EXPECT_THAT(GenerateAlternate("_ZN1A1fEv"),
   UnorderedElementsAre("_ZNK1A1fEv", "_ZLN1A1fEv"));
-  EXPECT_THAT(FindAlternate("_ZN1A1fEa"), Contains("_ZN1A1fEc"));
-  EXPECT_THAT(FindAlternate("_ZN1A1fEx"), Contains("_ZN1A1fEl"));
-  EXPECT_THAT(FindAlternate("_ZN1A1fEy"), Contains("_ZN1A1fEm"));
-  EXPECT_THAT(FindAlternate("_ZN1A1fEai"), Contains("_ZN1A1fEci"));
-  EXPECT_THAT(FindAlternate("_ZN1AC1Ev"), Contains("_ZN1AC2Ev"));
-  EXPECT_THAT(FindAlternate("_ZN1AD1Ev"), Contains("_ZN1AD2Ev"));
-  EXPECT_THAT(FindAlternate("_bogus"), IsEmpty());
+  EXPECT_THAT(GenerateAlternate("_ZN1A1fEa"), Contains("_ZN1A1fEc"));
+  EXPECT_THAT(GenerateAlternate("_ZN1A1fEx"), Contains("_ZN1A1fEl"));
+  EXPECT_THAT(GenerateAlternate("_ZN1A1fEy"), Contains("_ZN1A1fEm"));
+  EXPECT_THAT(GenerateAlternate("_ZN1A1fEai"), Contains("_ZN1A1fEci"));
+  EXPECT_THAT(GenerateAlternate("_ZN1AC1Ev"), Contains("_ZN1AC2Ev"));
+  EXPECT_THAT(GenerateAlternate("_ZN1AD1Ev"), Contains("_ZN1AD2Ev"));
+  EXPECT_THAT(GenerateAlternate("_bogus"), IsEmpty());
 }
 
 TEST(CPlusPlusLanguage, CPlusPlusNameParser) {
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
===
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
@@ -127,11 +127,8 @@
   llvm::StringRef &context,
   llvm::StringRef &identifier);
 
-  // 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)

[Lldb-commits] [PATCH] D109779: [LLDB] [Minidump] Fix format string warnings on Windows

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added a comment.

I fixed this in e69d359841b6358f1d17569212ef8cf91244ca11 
 and fixed 
some style issues.

---

This needs extra care.

While Clang -Wformat flags

  printf("%llu", (size_t)3);
  
  warning: format specifies type 'unsigned long long' but the argument has type 
'size_t' (aka 'unsigned long') [-Wformat]

This is silent:

  printf("%ld", (size_t)3);


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109779

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

[Lldb-commits] [lldb] 66902a3 - [StopInfoMachException] Summarize arm64e BLRAx/LDRAx auth failures

[Vedant Kumar via lldb-commits]

Author: Vedant Kumar
Date: 2021-09-14T13:31:52-07:00
New Revision: 66902a32c83809d26662f76e4107d5dd777610c3

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

LOG: [StopInfoMachException] Summarize arm64e BLRAx/LDRAx auth failures

Upstream lldb support for summarizing BLRAx and LDRAx auth failures.

rdar://41615322

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

Added: 
lldb/test/API/functionalities/ptrauth_diagnostics/BLRAA_error/Makefile

lldb/test/API/functionalities/ptrauth_diagnostics/BLRAA_error/TestPtrauthBLRAADiagnostic.py
lldb/test/API/functionalities/ptrauth_diagnostics/BLRAA_error/blraa.c
lldb/test/API/functionalities/ptrauth_diagnostics/BRAA_error/Makefile

lldb/test/API/functionalities/ptrauth_diagnostics/BRAA_error/TestPtrauthBRAADiagnostic.py
lldb/test/API/functionalities/ptrauth_diagnostics/BRAA_error/braa.c
lldb/test/API/functionalities/ptrauth_diagnostics/LDRAA_error/Makefile

lldb/test/API/functionalities/ptrauth_diagnostics/LDRAA_error/TestPtrauthLDRAADiagnostic.py
lldb/test/API/functionalities/ptrauth_diagnostics/LDRAA_error/ldraa.c
lldb/test/API/functionalities/ptrauth_diagnostics/brkC47x_code/Makefile

lldb/test/API/functionalities/ptrauth_diagnostics/brkC47x_code/TestPtrauthBRKc47xDiagnostic.py
lldb/test/API/functionalities/ptrauth_diagnostics/brkC47x_code/brkC47x.c

lldb/test/API/functionalities/ptrauth_diagnostics/brkC47x_x16_invalid/Makefile

lldb/test/API/functionalities/ptrauth_diagnostics/brkC47x_x16_invalid/TestPtrauthBRKc47xX16Invalid.py

lldb/test/API/functionalities/ptrauth_diagnostics/brkC47x_x16_invalid/brkC47x.c

Modified: 
lldb/include/lldb/Core/Address.h
lldb/include/lldb/Core/Disassembler.h
lldb/source/Core/Address.cpp
lldb/source/Core/Disassembler.cpp
lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
lldb/source/Plugins/Process/Utility/StopInfoMachException.h

Removed: 




diff  --git a/lldb/include/lldb/Core/Address.h 
b/lldb/include/lldb/Core/Address.h
index ec393a1871e35..dc50e27ca277a 100644
--- a/lldb/include/lldb/Core/Address.h
+++ b/lldb/include/lldb/Core/Address.h
@@ -210,6 +210,10 @@ class Address {
 }
   };
 
+  /// Write a description of this object to a Stream.
+  bool GetDescription(Stream &s, Target &target,
+  lldb::DescriptionLevel level) const;
+
   /// Dump a description of this object to a Stream.
   ///
   /// Dump a description of the contents of this object to the supplied stream

diff  --git a/lldb/include/lldb/Core/Disassembler.h 
b/lldb/include/lldb/Core/Disassembler.h
index 622c23ff64928..0925bf358b9c3 100644
--- a/lldb/include/lldb/Core/Disassembler.h
+++ b/lldb/include/lldb/Core/Disassembler.h
@@ -150,6 +150,10 @@ class Instruction {
 
   virtual bool HasDelaySlot();
 
+  virtual bool IsLoad() = 0;
+
+  virtual bool IsAuthenticated() = 0;
+
   bool CanSetBreakpoint ();
 
   virtual size_t Decode(const Disassembler &disassembler,
@@ -336,6 +340,10 @@ class PseudoInstruction : public Instruction {
 
   bool HasDelaySlot() override;
 
+  bool IsLoad() override;
+
+  bool IsAuthenticated() override;
+
   void CalculateMnemonicOperandsAndComment(
   const ExecutionContext *exe_ctx) override {
 // TODO: fill this in and put opcode name into Instruction::m_opcode_name,

diff  --git a/lldb/source/Core/Address.cpp b/lldb/source/Core/Address.cpp
index f0c7e2b34f99c..122bed924b420 100644
--- a/lldb/source/Core/Address.cpp
+++ b/lldb/source/Core/Address.cpp
@@ -389,6 +389,19 @@ bool Address::SetOpcodeLoadAddress(lldb::addr_t load_addr, 
Target *target,
   return false;
 }
 
+bool Address::GetDescription(Stream &s, Target &target,
+ DescriptionLevel level) const {
+  assert(level == eDescriptionLevelBrief &&
+ "Non-brief descriptions not implemented");
+  LineEntry line_entry;
+  if (CalculateSymbolContextLineEntry(line_entry)) {
+s.Printf(" (%s:%u:%u)", line_entry.file.GetFilename().GetCString(),
+ line_entry.line, line_entry.column);
+return true;
+  }
+  return false;
+}
+
 bool Address::Dump(Stream *s, ExecutionContextScope *exe_scope, DumpStyle 
style,
DumpStyle fallback_style, uint32_t addr_size) const {
   // If the section was nullptr, only load address is going to work unless we

diff  --git a/lldb/source/Core/Disassembler.cpp 
b/lldb/source/Core/Disassembler.cpp
index 704b3df4b2ac8..d147f8fbb0dba 100644
--- a/lldb/source/Core/Disassembler.cpp
+++ b/lldb/source/Core/Disassembler.cpp
@@ -1123,6 +1123,10 @@ bool PseudoInstruction::HasDelaySlot() {
   return false;
 }
 
+bool PseudoInstruction::IsLoad() { return false; }
+
+bool PseudoInstruction::Is

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

[Vedant Kumar 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 rG66902a32c838: [StopInfoMachException] Summarize arm64e 
BLRAx/LDRAx auth failures (authored by vsk).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102428

Files:
  lldb/include/lldb/Core/Address.h
  lldb/include/lldb/Core/Disassembler.h
  lldb/source/Core/Address.cpp
  lldb/source/Core/Disassembler.cpp
  lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp
  lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
  lldb/source/Plugins/Process/Utility/StopInfoMachException.h
  lldb/test/API/functionalities/ptrauth_diagnostics/BLRAA_error/Makefile
  
lldb/test/API/functionalities/ptrauth_diagnostics/BLRAA_error/TestPtrauthBLRAADiagnostic.py
  lldb/test/API/functionalities/ptrauth_diagnostics/BLRAA_error/blraa.c
  lldb/test/API/functionalities/ptrauth_diagnostics/BRAA_error/Makefile
  
lldb/test/API/functionalities/ptrauth_diagnostics/BRAA_error/TestPtrauthBRAADiagnostic.py
  lldb/test/API/functionalities/ptrauth_diagnostics/BRAA_error/braa.c
  lldb/test/API/functionalities/ptrauth_diagnostics/LDRAA_error/Makefile
  
lldb/test/API/functionalities/ptrauth_diagnostics/LDRAA_error/TestPtrauthLDRAADiagnostic.py
  lldb/test/API/functionalities/ptrauth_diagnostics/LDRAA_error/ldraa.c
  lldb/test/API/functionalities/ptrauth_diagnostics/brkC47x_code/Makefile
  
lldb/test/API/functionalities/ptrauth_diagnostics/brkC47x_code/TestPtrauthBRKc47xDiagnostic.py
  lldb/test/API/functionalities/ptrauth_diagnostics/brkC47x_code/brkC47x.c
  lldb/test/API/functionalities/ptrauth_diagnostics/brkC47x_x16_invalid/Makefile
  
lldb/test/API/functionalities/ptrauth_diagnostics/brkC47x_x16_invalid/TestPtrauthBRKc47xX16Invalid.py
  
lldb/test/API/functionalities/ptrauth_diagnostics/brkC47x_x16_invalid/brkC47x.c

Index: lldb/test/API/functionalities/ptrauth_diagnostics/brkC47x_x16_invalid/brkC47x.c
===
--- /dev/null
+++ lldb/test/API/functionalities/ptrauth_diagnostics/brkC47x_x16_invalid/brkC47x.c
@@ -0,0 +1,14 @@
+int main() {
+  //% self.filecheck("c", "brkC47x.c")
+  // CHECK: stop reason = EXC_BAD_ACCESS
+  // CHECK-NOT: Note: Possible pointer authentication failure detected.
+  asm volatile (
+  "mov x16, #0xbad \n"
+  "brk 0xc470 \n"
+  /* Outputs */  :
+  /* Inputs */   :
+  /* Clobbers */ : "x16"
+  );
+
+  return 1;
+}
Index: lldb/test/API/functionalities/ptrauth_diagnostics/brkC47x_x16_invalid/TestPtrauthBRKc47xX16Invalid.py
===
--- /dev/null
+++ lldb/test/API/functionalities/ptrauth_diagnostics/brkC47x_x16_invalid/TestPtrauthBRKc47xX16Invalid.py
@@ -0,0 +1,5 @@
+from lldbsuite.test import lldbinline
+from lldbsuite.test import decorators
+
+lldbinline.MakeInlineTest(__file__, globals(),
+[decorators.skipIf(archs=decorators.no_match(['arm64e']))])
Index: lldb/test/API/functionalities/ptrauth_diagnostics/brkC47x_x16_invalid/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/ptrauth_diagnostics/brkC47x_x16_invalid/Makefile
@@ -0,0 +1,2 @@
+C_SOURCES := brkC47x.c
+include Makefile.rules
Index: lldb/test/API/functionalities/ptrauth_diagnostics/brkC47x_code/brkC47x.c
===
--- /dev/null
+++ lldb/test/API/functionalities/ptrauth_diagnostics/brkC47x_code/brkC47x.c
@@ -0,0 +1,17 @@
+void foo() {}
+
+int main() {
+  //% self.filecheck("c", "brkC47x.c")
+  // CHECK: stop reason = EXC_BAD_ACCESS
+  // CHECK-NEXT: Note: Possible pointer authentication failure detected.
+  // CHECK-NEXT: Found value that failed to authenticate at address=0x{{.*}} (brkC47x.c:1:13).
+  asm volatile (
+  "mov x16, %[target] \n"
+  "brk 0xc470 \n"
+  /* Outputs */  :
+  /* Inputs */   : [target] "r"(&foo)
+  /* Clobbers */ : "x16"
+  );
+
+  return 1;
+}
Index: lldb/test/API/functionalities/ptrauth_diagnostics/brkC47x_code/TestPtrauthBRKc47xDiagnostic.py
===
--- /dev/null
+++ lldb/test/API/functionalities/ptrauth_diagnostics/brkC47x_code/TestPtrauthBRKc47xDiagnostic.py
@@ -0,0 +1,5 @@
+from lldbsuite.test import lldbinline
+from lldbsuite.test import decorators
+
+lldbinline.MakeInlineTest(__file__, globals(),
+[decorators.skipIf(archs=decorators.no_match(['arm64e']))])
Index: lldb/test/API/functionalities/ptrauth_diagnostics/brkC47x_code/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/ptrauth_diagnostics/brkC47x_code/Makefile
@@ -0,0 +1,2 @@
+C_SOURCES := brkC47x.c
+include Makefile.rules
Index: lldb/test/API/functionalities/ptrauth_diagnostics/LDRAA_error/ldraa

[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.

I created the original bug because a change made LLDB stop working on address 
breakpoints if they were created before the program is run. Setting a 
breakpoint on an address is a typical case with embedded applications, 
especially if you want to debug startup code.

I took this patch and applied it to the latest downstream Hexagon repo. It 
behaved as expected - I set a breakpoint at the address of main, ran, and it 
stopped at the breakpoint. Before this patch, the breakpoint wouldn't be 
resolved.

Thanks, Vadim!

(lldb) p main
(int (*)(int, unsigned char **)) $0 = 0x5128
(lldb) b 0x5128
Breakpoint 1: address = 0x5128
(lldb) br l
Current breakpoints:
1: address = 0x5128, locations = 1

  1.1: address = 0x5128, unresolved, hit count = 0 

(lldb) r
Process 1 launched: '/usr2/tedwood/lldb_test/factorial' (hexagon)
Process 1 stopped

- thread #1, name = 'T1', stop reason = breakpoint 1.1 frame #0: 0x5128 
factorial`main(argc=-1161904401, argv=0x5bac) at factorial.c:13 10   } 11 
12   int main(int argc, char **argv)

-> 13   {

  14 unsigned base;
  15  
  16   /*

(lldb) re r pc

  pc = 0x5128  factorial`main at factorial.c:13

(lldb) br l
Current breakpoints:
1: address = 0x5128, locations = 1, resolved = 1, hit count = 1

  1.1: address = 0x5128, resolved, hardware, hit count = 1 


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] D109738: [lldb] Fix bug 38317 - Address breakpoints don't work if set before the process launches

[Jim Ingham via Phabricator via lldb-commits]

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

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.

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.

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.

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.

So the most important thing here is to make sure that address breakpoints 
always have section relative addresses if we can by any means identify the 
section they belong in.


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] 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.

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


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 created this revision.
vsk added reviewers: jasonmolenda, jingham, JDevlieghere.
Herald added subscribers: zzheng, omjavaid, pengfei, kristof.beyls.
vsk requested review of this revision.
Herald added a project: LLDB.

A MachO userspace corefile may contain LC_THREAD commands which specify
thread exception state.

For arm64* only (for now), report a human-readable version of this state
as the thread stop reason, instead of 'SIGSTOP'.

As a follow-up, similar functionality can be implemented for x86 cores
by translating the trapno/err exception registers.

rdar://82898146


Repository:
  rG LLVM Github Monorepo

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,36 @@
 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();
+  ArchSpec arch_spec = target.GetArchitecture();
+  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");
+RegisterValue esr;
+if (reg_ctx_sp->ReadRegister(esr_info, esr)) {
+  uint32_t esr_val = esr.GetAsUInt32();
+  auto exception_class =
+  static_cast(esr_val >> 26);
+  if (exception_class !=
+  AppleArm64ExceptionClass::ESR_EC_UNCATEGORIZED) {
+const char *exception_name = toString(exception_class);
+stop_info =
+StopInfo::CreateStopReasonWithException(*this, exception_name);
+  }
+}
+  }
+}
+
+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,31 @@
+//===-- 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"
+};
+
+inline const char *toString(AppleArm64ExceptionClass EC) {
+  switch (EC) {
+#define APPLE_ARM64_EXCEPTION_CLASS(Name, Code)\
+  case AppleArm64ExceptionClass::Name: \
+return #Name;
+#include "AppleArm64ExceptionClass.def"
+  }
+  return "Unknown Exception Class";
+}
+
+} // namespace lldb_private
+
+#endif // LLDB_TARGET_APPLEARM64EXCEPTIONCLASS_H
Index: lldb/include/lldb/Target/AppleArm64ExceptionClass.def
===
--- /dev/null
+++ lldb/include/lldb/Target/AppleArm64ExceptionClass.def
@@ -0,0 +1,51 @@
+#ifndef APPLE_ARM64_EXCEPTION_CLASS
+#error "APPLE_ARM64_EXCEPTION_CLASS(Name, Code) no

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

[Vedant Kumar via Phabricator via lldb-commits]

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

- Add license header to the .def file.
- (I'm not sure whether/how this change can be tested, any pointers 
appreciated.)


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,36 @@
 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();
+  ArchSpec arch_spec = target.GetArchitecture();
+  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");
+RegisterValue esr;
+if (reg_ctx_sp->ReadRegister(esr_info, esr)) {
+  uint32_t esr_val = esr.GetAsUInt32();
+  auto exception_class =
+  static_cast(esr_val >> 26);
+  if (exception_class !=
+  AppleArm64ExceptionClass::ESR_EC_UNCATEGORIZED) {
+const char *exception_name = toString(exception_class);
+stop_info =
+StopInfo::CreateStopReasonWithException(*this, exception_name);
+  }
+}
+  }
+}
+
+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,31 @@
+//===-- 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"
+};
+
+inline const char *toString(AppleArm64ExceptionClass EC) {
+  switch (EC) {
+#define APPLE_ARM64_EXCEPTION_CLASS(Name, Code)\
+  case AppleArm64ExceptionClass::Name: \
+return #Name;
+#include "AppleArm64ExceptionClass.def"
+  }
+  return "Unknown Exception Class";
+}
+
+} // namespace lldb_private
+
+#endif // LLDB_TARGET_APPLEARM64EXCEPTIONCLASS_H
Index: lldb/include/lldb/Target/AppleArm64ExceptionClass.def
===
--- /dev/null
+++ lldb/include/lldb/Target/AppleArm64ExceptionClass.def
@@ -0,0 +1,61 @@
+/*===-- AppleArm64ExceptionClass.def ---*- C++ -*-=== *\
+|*
+|* Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+|* See https://llvm.org/LICENSE.txt for license information.
+|* SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+|*
+\*===--===*/
+
+// Defines ESR exception classes for A

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

[Emre Kultursay via Phabricator via lldb-commits]

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

When rebase_exec=true in DidAttach(), all modules are loaded
before the rendezvous breakpoint is set, which means the
LoadInterpreterModule() method is not called and m_interpreter_module
is not initialized.

This causes the very first rendezvous breakpoint hit (with
m_initial_modules_added=false) to accidentally unload the
module_sp that corresponds to the dynamic loader.

This bug was causing the rendezvous mechanism to not work
in Android 28, which this CL fixes.

Test: Verified rendezvous on Android 28 and 29


Repository:
  rG LLVM Github Monorepo

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] D109795: [MachCore] Report arm64 thread exception state

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik 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,

We should use fixed sized integer types whenever possible, I suppose in this 
case `uint32_t`.



Comment at: lldb/source/Plugins/Process/mach-core/ThreadMachCore.cpp:111
+  uint32_t esr_val = esr.GetAsUInt32();
+  auto exception_class =
+  static_cast(esr_val >> 26);

`const`



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

Does `26` have a meaning? I am guessing we are shifting to get the EC bits?


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 a comment.

This is much more comprehensive than what I was thinking of, nicely done.  The 
mapping of the Exception Class in the esr reg to a name is AArch64 specific, 
not Apple specific, isn't it?  I haven't looked in the ARM ARM, but I suspect 
it's just the specific names you're using here which might be what we use at 
apple, but the exception codes are the same for any AArch64 target.

What do you think about printing the FAR register value (in particular) in the 
stop message?  For many exception types, it contains the address that was 
relevant to the fault iirc, and even if we don't explicitly spell out what that 
relationship is (it's prob going to be different for each exception code), just 
having it printed may be helpful for a human reading the message.

I think the patch is fine if you prefer it this way, just giving some other 
ideas.


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 !=

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.


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

[Vadim Chugunov via Phabricator via lldb-commits]

vadimcn added a comment.

> 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.

> 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.
Also, please see Ted's comment about embedded.

> 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 location.   Anything else would be surprising
to me.


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] D109738: [lldb] Fix bug 38317 - Address breakpoints don't work if set before the process launches

[Vadim Chugunov via Phabricator via lldb-commits]

vadimcn updated this revision to Diff 372636.
vadimcn added a comment.

Added patch context.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109738

Files:
  lldb/source/Breakpoint/BreakpointResolverAddress.cpp
  
lldb/test/API/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py


Index: 
lldb/test/API/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py
===
--- 
lldb/test/API/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py
+++ 
lldb/test/API/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py
@@ -79,10 +79,33 @@
 process = target.Launch(launch_info, error)
 self.assertTrue(process, PROCESS_IS_VALID)
 
-thread = get_threads_stopped_at_breakpoint(process, breakpoint)
+threads = get_threads_stopped_at_breakpoint(process, breakpoint)
 self.assertEqual(
 len(threads), 1,
 "There should be a thread stopped at our breakpoint")
 
 # The hit count for the breakpoint should now be 2.
 self.assertEquals(breakpoint.GetHitCount(), 2)
+
+process.Kill()
+
+# Create a breakpoint again, this time using the load address
+load_address = address.GetLoadAddress(target)
+
+# Re-create the target to make sure that nothing is cached
+target = self.createTestTarget()
+breakpoint = target.BreakpointCreateByAddress(load_address)
+
+launch_info.Clear()
+launch_info.SetLaunchFlags(flags)
+
+process = target.Launch(launch_info, error)
+self.assertTrue(process, PROCESS_IS_VALID)
+
+threads = get_threads_stopped_at_breakpoint(process, breakpoint)
+self.assertEqual(
+len(threads), 1,
+"There should be a thread stopped at our breakpoint")
+
+# The hit count for the breakpoint should be 1.
+self.assertEquals(breakpoint.GetHitCount(), 1)
Index: lldb/source/Breakpoint/BreakpointResolverAddress.cpp
===
--- lldb/source/Breakpoint/BreakpointResolverAddress.cpp
+++ lldb/source/Breakpoint/BreakpointResolverAddress.cpp
@@ -97,8 +97,12 @@
   bool re_resolve = false;
   if (m_addr.GetSection() || m_module_filespec)
 re_resolve = true;
-  else if (GetBreakpoint()->GetNumLocations() == 0)
-re_resolve = true;
+  else {
+BreakpointSP breakpoint = GetBreakpoint();
+if (breakpoint->GetNumLocations() == 0 ||
+breakpoint->GetNumResolvedLocations() < breakpoint->GetNumLocations())
+  re_resolve = true;
+  }
 
   if (re_resolve)
 BreakpointResolver::ResolveBreakpoint(filter);
@@ -110,8 +114,12 @@
   bool re_resolve = false;
   if (m_addr.GetSection())
 re_resolve = true;
-  else if (GetBreakpoint()->GetNumLocations() == 0)
-re_resolve = true;
+  else {
+BreakpointSP breakpoint = GetBreakpoint();
+if (breakpoint->GetNumLocations() == 0 ||
+breakpoint->GetNumResolvedLocations() < breakpoint->GetNumLocations())
+  re_resolve = true;
+  }
 
   if (re_resolve)
 BreakpointResolver::ResolveBreakpointInModules(filter, modules);
@@ -151,7 +159,7 @@
   BreakpointLocationSP loc_sp = breakpoint.GetLocationAtIndex(0);
   lldb::addr_t cur_load_location =
   m_addr.GetLoadAddress(&breakpoint.GetTarget());
-  if (cur_load_location != m_resolved_addr) {
+  if (cur_load_location != m_resolved_addr || !loc_sp->IsResolved()) {
 m_resolved_addr = cur_load_location;
 loc_sp->ClearBreakpointSite();
 loc_sp->ResolveBreakpointSite();


Index: lldb/test/API/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py
===
--- lldb/test/API/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py
+++ lldb/test/API/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py
@@ -79,10 +79,33 @@
 process = target.Launch(launch_info, error)
 self.assertTrue(process, PROCESS_IS_VALID)
 
-thread = get_threads_stopped_at_breakpoint(process, breakpoint)
+threads = get_threads_stopped_at_breakpoint(process, breakpoint)
 self.assertEqual(
 len(threads), 1,
 "There should be a thread stopped at our breakpoint")
 
 # The hit count for the breakpoint should now be 2.
 self.assertEquals(breakpoint.GetHitCount(), 2)
+
+process.Kill()
+
+# Create a breakpoint again, this time using the load address
+load_address = address.GetLoadAddress(target)
+
+# Re-create the target to make sure that nothing is cached
+target = self.createTestTarget()
+breakpoint = target.BreakpointCreateByAddress(load_address)
+
+launch_info.Clear()
+launch_info.SetLaun