[Lldb-commits] [lldb] [lldb] Add RegisterContextPOSIXCore for RISC-V 64 (PR #93297)
jasonmolenda wrote: This patch look good to me, do you have necessary permissions to squash & merge? https://github.com/llvm/llvm-project/pull/93297 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add RegisterContextPOSIXCore for RISC-V 64 (PR #93297)
@@ -0,0 +1,84 @@
+//===-- RegisterContextPOSIXCore_riscv64.cpp
--===//
+//
+// 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
+//
+//===--===//
+
+#include "RegisterContextPOSIXCore_riscv64.h"
+
+#include "lldb/Utility/DataBufferHeap.h"
+
+using namespace lldb_private;
+
+std::unique_ptr
+RegisterContextCorePOSIX_riscv64::Create(
+lldb_private::Thread , const lldb_private::ArchSpec ,
+const lldb_private::DataExtractor ,
+llvm::ArrayRef notes) {
+ Flags flags = 0;
+
+ auto register_info_up =
+ std::make_unique(arch, flags);
+ return std::unique_ptr(
+ new RegisterContextCorePOSIX_riscv64(thread, std::move(register_info_up),
+ gpregset, notes));
+}
+
+RegisterContextCorePOSIX_riscv64::RegisterContextCorePOSIX_riscv64(
+Thread , std::unique_ptr register_info,
+const DataExtractor , llvm::ArrayRef notes)
+: RegisterContextPOSIX_riscv64(thread, std::move(register_info)) {
+
+ m_gpr_buffer = std::make_shared(gpregset.GetDataStart(),
+ gpregset.GetByteSize());
+ m_gpr.SetData(m_gpr_buffer);
+ m_gpr.SetByteOrder(gpregset.GetByteOrder());
+
+ ArchSpec arch = m_register_info_up->GetTargetArchitecture();
+ DataExtractor fpregset = getRegset(notes, arch.GetTriple(), FPR_Desc);
+ m_fpr_buffer = std::make_shared(fpregset.GetDataStart(),
jasonmolenda wrote:
Good point, no need to change this PR. I don't think you should have to lldb's
Linux support to handle a no-FPU environment, unless you'd like to -- if
someone is bringing up Linux on a target like that, I think it's fine for them
to do the work.
https://github.com/llvm/llvm-project/pull/93297
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add RegisterContextPOSIXCore for RISC-V 64 (PR #93297)
@@ -0,0 +1,84 @@
+//===-- RegisterContextPOSIXCore_riscv64.cpp
--===//
+//
+// 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
+//
+//===--===//
+
+#include "RegisterContextPOSIXCore_riscv64.h"
+
+#include "lldb/Utility/DataBufferHeap.h"
+
+using namespace lldb_private;
+
+std::unique_ptr
+RegisterContextCorePOSIX_riscv64::Create(
+lldb_private::Thread , const lldb_private::ArchSpec ,
+const lldb_private::DataExtractor ,
+llvm::ArrayRef notes) {
+ Flags flags = 0;
+
+ auto register_info_up =
+ std::make_unique(arch, flags);
+ return std::unique_ptr(
+ new RegisterContextCorePOSIX_riscv64(thread, std::move(register_info_up),
+ gpregset, notes));
+}
+
+RegisterContextCorePOSIX_riscv64::RegisterContextCorePOSIX_riscv64(
+Thread , std::unique_ptr register_info,
+const DataExtractor , llvm::ArrayRef notes)
+: RegisterContextPOSIX_riscv64(thread, std::move(register_info)) {
+
+ m_gpr_buffer = std::make_shared(gpregset.GetDataStart(),
+ gpregset.GetByteSize());
+ m_gpr.SetData(m_gpr_buffer);
+ m_gpr.SetByteOrder(gpregset.GetByteOrder());
+
+ ArchSpec arch = m_register_info_up->GetTargetArchitecture();
+ DataExtractor fpregset = getRegset(notes, arch.GetTriple(), FPR_Desc);
+ m_fpr_buffer = std::make_shared(fpregset.GetDataStart(),
jasonmolenda wrote:
I see in the aarch64 elf core plugin that it checks the size of the
DataExtractor object before setting the buffer to its contents, with a "must be
bigger than the smallest possible size", e.g.
```
DataExtractor mte_data = getRegset(notes, arch.GetTriple(), AARCH64_MTE_Desc);
if (mte_data.GetByteSize() >= sizeof(uint64_t))
opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskMTE);
```
Is it possible this could be called without a floating point register context?
I know in RISC-V nearly anything is possible :) but maybe realistically we're
always going to have an fpr.
https://github.com/llvm/llvm-project/pull/93297
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add RegisterContextPOSIXCore for RISC-V 64 (PR #93297)
https://github.com/jasonmolenda approved this pull request. This looks good to me, I'd like to see the std::make_unique in `RegisterContextCorePOSIX_riscv64::Create` and a sanity check for fetching the fpr, but maybe that's just unnecessary I don't work with RV64 targets myself, if you're comfortable with it as-is, that's fine. https://github.com/llvm/llvm-project/pull/93297 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add RegisterContextPOSIXCore for RISC-V 64 (PR #93297)
https://github.com/jasonmolenda edited https://github.com/llvm/llvm-project/pull/93297 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add RegisterContextPOSIXCore for RISC-V 64 (PR #93297)
@@ -0,0 +1,84 @@
+//===-- RegisterContextPOSIXCore_riscv64.cpp
--===//
+//
+// 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
+//
+//===--===//
+
+#include "RegisterContextPOSIXCore_riscv64.h"
+
+#include "lldb/Utility/DataBufferHeap.h"
+
+using namespace lldb_private;
+
+std::unique_ptr
+RegisterContextCorePOSIX_riscv64::Create(
+lldb_private::Thread , const lldb_private::ArchSpec ,
+const lldb_private::DataExtractor ,
+llvm::ArrayRef notes) {
+ Flags flags = 0;
+
+ auto register_info_up =
+ std::make_unique(arch, flags);
+ return std::unique_ptr(
jasonmolenda wrote:
Please make this return object a `std::make_unique` too, it will be shorter.
https://github.com/llvm/llvm-project/pull/93297
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Reapply "[lldb/aarch64] Fix unwinding when signal interrupts a leaf f… (PR #92503)
jasonmolenda wrote: > The way I see it, this check (at least the part about the RA register(*)) is > heuristic that's impossible to get always right. Like, I could construct a > test case using functions with non-standard ABIs where a non-leaf function > legitimately has a `lr=` rule. Such code would execute correctly but > lldb would refuse to unwind it due to the `lr=` restriction. This would be an interesting idea. I don't think there's any unwind format which allows you to specify that a different register holds the return address, and lr is IsSame. You could say that lr=x9 to say that the return address is in x9, but you can't express that the return address is stored in a non-lr register. You could add an unwind rule for pc=x9 to say that the return address is in x9, and depend on the unwinder to not look for lr, but to try retrieving pc first. > > All of this is to say that I don't think there is a way to change this piece > of code to be correct all the time -- we'd just be trading one set of edge > cases for the other. I think that the most correct solution would be to > remove this check altogether. I'm not sure why it exists, but I expect it has > something to do with preventing looping stacks. The original goal was that if we're on frame 1, we don't want to surface a register value from frame 0 and use it in frame 1 unless it's a callee-spilled register. e.g. x0 on frame 1 may have been overwritten while frame 0 was executing, there is no unwind rule for x0 and the unwinder can't show frame 0's x0 value in frame 1. But if we're above a sigtramp etc frame which has the entire register context from when a function was interrupted, we can retrieve all the registers and want to show them. > (*) I'm only talking about the `lr` rule everywhere. I _think_ that a > `pc=` rule would always be an error (even in signal handlers), so we > should be able to keep that here. OTOH, if our loop detection code is robust > enough, then there should be no harm in letting this through either... Yeah I think there's improvements that can be made here for sure. https://github.com/llvm/llvm-project/pull/92503 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Reapply "[lldb/aarch64] Fix unwinding when signal interrupts a leaf f… (PR #92503)
jasonmolenda wrote: as cool as this idea is, I do worry that it will make the code less readable, where instead of saying "BehavesLikeFrameZero / HasAllRegisters", we now need to ask "can the frame below supply register x", I don't know. it's just something I have running around my head today. https://github.com/llvm/llvm-project/pull/92503 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Reapply "[lldb/aarch64] Fix unwinding when signal interrupts a leaf f… (PR #92503)
jasonmolenda wrote: (with the caveat that a register location of IsSame for a volatile aka non-callee-spilled register would be treated as "did not have a location") https://github.com/llvm/llvm-project/pull/92503 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Reapply "[lldb/aarch64] Fix unwinding when signal interrupts a leaf f… (PR #92503)
jasonmolenda wrote: We should be able to work correctly with a trap handler that has full eh_frame without knowing the function name. And we shouldn't treat a sigtramp missing eh_frame as having all registers. https://github.com/llvm/llvm-project/pull/92503 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Reapply "[lldb/aarch64] Fix unwinding when signal interrupts a leaf f… (PR #92503)
jasonmolenda wrote: btw @labath I was thinking about "sigtramp routines" and fault / trap / interrupt handlers in general, and how lldb has this list of function names that it treats as fault handlers (`target.trap-handler-names`). And in the unwinder we have the same idea of "the frame above a designed sigtramp/fault/trap/interrupt function can retrieve all registers". But what if the unwinder had a method saying "Do you have an unwind rule to give me a value for register n, without iterating down the stack?" e.g. frame 5 is interrupted and frame 4 is sigtramp, with full eh_frame. From frame 5, I can say "does frame 4 have an unwind rule to provide x0, without iterating down to frame 3, etc." This also means if we have a sigtramp which is missing its eh_frame, we won't apply our "all registers available" rules to the frame above. An interesting case for a return-address target like aarch64 where normally when we ask for a caller frame's pc, we fetch the link register. But above a fault/trap/interrupt frame, we can retrieve both the pc and the link register and they are different values. Just something I started kicking around in my head, I don't have concrete plans to implement an overhaul like this but the more I think about it, the more I like it. https://github.com/llvm/llvm-project/pull/92503 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][riscv] Fix setting breakpoint for undecoded instruction (PR #90075)
https://github.com/jasonmolenda edited https://github.com/llvm/llvm-project/pull/90075 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][riscv] Fix setting breakpoint for undecoded instruction (PR #90075)
@@ -115,8 +148,23 @@ Status
NativeProcessSoftwareSingleStep::SetupSoftwareSingleStepping(
emulator_up->SetWriteMemCallback();
emulator_up->SetWriteRegCallback();
- if (!emulator_up->ReadInstruction())
-return Status("Read instruction failed!");
+ if (!emulator_up->ReadInstruction()) {
jasonmolenda wrote:
Ah wait, I see. This method is trying to decode where the next instruction
will go, with branches and jumps decoded, so we can put a breakpoint there.
And you're handling the case where we can't decode the current instruction (I
now understand why you used that in your test case). It seems harmless to call
GetLastInstrSize() if the instruction that couldn't be decoded, and add the
length of the instruction to pc. We can assume the emulation engine will
emulate all branching instructions. I could imagine the RISCV emulation plugin
didn't have decoding for an instruction that doesn't branch, it could fail, but
we can still decode the size of that unknown instruction successfully, and
assume that it does not branch.
https://github.com/llvm/llvm-project/pull/90075
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add the word "backtrace" to bt help string (PR #92618)
https://github.com/jasonmolenda approved this pull request. looks good, thanks. https://github.com/llvm/llvm-project/pull/92618 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Reapply "[lldb/aarch64] Fix unwinding when signal interrupts a leaf f… (PR #92503)
https://github.com/jasonmolenda approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/92503 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Reapply "[lldb/aarch64] Fix unwinding when signal interrupts a leaf f… (PR #92503)
jasonmolenda wrote: Yes, originally lldb's unwinder was recursive for any register propagation and it was easy to hit the problem of lldb blowing out its own stack on a recursive inferior that had crashed. I changed most of the propagation to a loop to solve this (years and years ago) but it looks like we still have a case where it is recursing. We still need to skip the test case on macOS until I can come up with some idea to get proper unwind instruction for sigtramp on arm64. Most of the CI bots are x86_64 so it may pass on them, but that's The Past and I would prefer to skip this on Darwin until I can figure something out, I wrote a little TODO on myself in rdar://128031075 This looks good to me. https://github.com/llvm/llvm-project/pull/92503 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][riscv] Fix setting breakpoint for undecoded instruction (PR #90075)
https://github.com/jasonmolenda edited https://github.com/llvm/llvm-project/pull/90075 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][riscv] Fix setting breakpoint for undecoded instruction (PR #90075)
@@ -115,8 +148,23 @@ Status
NativeProcessSoftwareSingleStep::SetupSoftwareSingleStepping(
emulator_up->SetWriteMemCallback();
emulator_up->SetWriteRegCallback();
- if (!emulator_up->ReadInstruction())
-return Status("Read instruction failed!");
+ if (!emulator_up->ReadInstruction()) {
jasonmolenda wrote:
Shouldn't this block now be `if (emulator_up->ReadInstruction())` now? We're
going to get the size of the last decoded instruction here.
https://github.com/llvm/llvm-project/pull/90075
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][riscv] Fix setting breakpoint for undecoded instruction (PR #90075)
@@ -115,8 +148,23 @@ Status
NativeProcessSoftwareSingleStep::SetupSoftwareSingleStepping(
emulator_up->SetWriteMemCallback();
emulator_up->SetWriteRegCallback();
- if (!emulator_up->ReadInstruction())
-return Status("Read instruction failed!");
+ if (!emulator_up->ReadInstruction()) {
+// try to get at least the size of next instruction to set breakpoint.
+auto instrSizeOpt = emulator_up->GetLastInstrSize();
+if (!instrSizeOpt)
+ return Status("Read instruction failed!");
+bool success = false;
+auto pc = emulator_up->ReadRegisterUnsigned(eRegisterKindGeneric,
+LLDB_REGNUM_GENERIC_PC,
+LLDB_INVALID_ADDRESS,
);
+if (!success)
+ return Status("Reading pc failed!");
+lldb::addr_t next_pc = pc + *instrSizeOpt;
+auto Result =
+SetSoftwareBreakPointOnPC(arch, next_pc, /* next_flags */ 0x0,
process);
jasonmolenda wrote:
We've decoded the length of the instruction at `pc` at this point, and them to
get `next_pc`. Then we pass `next_pc` to this method which has a hardcoded
size of 4 for RISCV. It's only a hint that is sent to lldb-server as it tries
to step over the instruction. With armv7/aarch32 we had to get arm/thumb
breakpoint instructions correct because an arm breakpoint wasn't valid when the
processor was in thumb mode (iirc) but RISC-V doesn't have a processor mode
like that iiuc. So maybe it's fine to have `SetSoftwareBreakPointOnPC`
hardcoding 4 for the next RISCV breakpoint.
https://github.com/llvm/llvm-project/pull/90075
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][riscv] Fix setting breakpoint for undecoded instruction (PR #90075)
@@ -115,8 +148,23 @@ Status
NativeProcessSoftwareSingleStep::SetupSoftwareSingleStepping(
emulator_up->SetWriteMemCallback();
emulator_up->SetWriteRegCallback();
- if (!emulator_up->ReadInstruction())
-return Status("Read instruction failed!");
+ if (!emulator_up->ReadInstruction()) {
+// try to get at least the size of next instruction to set breakpoint.
+auto instrSizeOpt = emulator_up->GetLastInstrSize();
+if (!instrSizeOpt)
+ return Status("Read instruction failed!");
jasonmolenda wrote:
We've defined the new GetLastInstrSize() method for the RISCV
EmulateInstruction plugin, but others like AArch64 won't have that, so this
will error out on them, won't it? What we really want to express is "if
`arch.GetTriple().isRISCV()` and we couldn't decode the length of the last
instruction, then error out" isn't it?
https://github.com/llvm/llvm-project/pull/90075
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][riscv] Fix setting breakpoint for undecoded instruction (PR #90075)
https://github.com/jasonmolenda commented: I read this patch while trying to keep the RVC instruction set in mind because it's an interesting case to consider. Maybe the test file could have a compressed instruction? The one instruction there now ends in 0b11 so I don't think it will decode that way. https://github.com/llvm/llvm-project/pull/90075 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][riscv] Fix setting breakpoint for undecoded instruction (PR #90075)
@@ -99,6 +100,8 @@ class EmulateInstructionRISCV : public EmulateInstruction {
private:
/// Last decoded instruction from m_opcode
DecodeResult m_decoded;
+ /// Last tried to be decoded instruction expected size.
jasonmolenda wrote:
I might call this `Last decoded instruction size`.
https://github.com/llvm/llvm-project/pull/90075
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/aarch64] Allow unaligned PC addresses below a trap handler (PR #92093)
https://github.com/jasonmolenda approved this pull request. Good change, thanks for fixing this. https://github.com/llvm/llvm-project/pull/92093 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/aarch64] Allow unaligned PC addresses below a trap handler (PR #92093)
@@ -0,0 +1,26 @@
+# REQUIRES: (target-aarch64 || target-arm) && native
+# UNSUPPORTED: system-windows
+
+# RUN: %clang_host %S/Inputs/unaligned-pc-sigbus.c -o %t
+# RUN: %lldb -s %s -o exit %t | FileCheck %s
+
+breakpoint set -n sigbus_handler
+# CHECK: Breakpoint 1: where = {{.*}}`sigbus_handler
+
+run
+# CHECK: thread #1, {{.*}} stop reason = signal SIGBUS
jasonmolenda wrote:
Yes doing `b sigbus_handler; r` stops with an EXC_BAD_ACCESS without it being
delivered to the process.
`b sigbus_handler; settings set platform.plugin.darwin.ignored-exceptions
EXC_BAD_ACCESS; r` will stop when the SIGBUS is delivered.
`b sigbus_handler; settings set platform.plugin.darwin.ignored-exceptions
EXC_BAD_ACCESS; process handle -p true -s false SIGBUS; r` will stop in
sigbus_handler.
On macOS we hit the same failure we saw in
https://github.com/llvm/llvm-project/pull/91321 where we don't have eh_frame
details for _sigtramp so this frameless leaf function that crashed is not
discovered when we do the stack walk. This will need to be xfailed on macOS
for the same reason as 91321. FWIW I filed a little work item on myself to
figure out Something That Can Be Done in rdar://128031075 if you want to
annotate the xfail.
https://github.com/llvm/llvm-project/pull/92093
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/aarch64] Fix unwinding when signal interrupts a leaf function (PR #91321)
jasonmolenda wrote: (and it turns out the reason we don't have eh_frame is because _sigtramp on arm64 is written in C, and I'm not sure how I'm going to track which callee-saved register the argument is copied into, so this is definitely not something I can get fixed quickly.) https://github.com/llvm/llvm-project/pull/91321 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/aarch64] Fix unwinding when signal interrupts a leaf function (PR #91321)
jasonmolenda wrote: Ah, so the problem here is that we're missing the eh_frame instructions for _sigtramp on arm64 with macOS 14. `signal_generating_add` is a frameless function (a great stress test in this instance), and _sigtramp is called with enough of a faked-up stack that a stack walk will find the last frame that set up a stack frame before faulting, main in this case. But we skip `signal_generating_add`. We're going to need to skip this test on macOS for now, and I'll dig in to where the eh_frame instructions went and see about getting them re-added, but it may take a while to get that into builds and on to the bots. https://github.com/llvm/llvm-project/pull/91321 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/aarch64] Fix unwinding when signal interrupts a leaf function (PR #91321)
jasonmolenda wrote: > It's not completely correct -- the frame for `signal_generating_add` is > missing. That's what the error message is complaining about. Ah, thanks, I missed that! Let me debug it and comment further https://github.com/llvm/llvm-project/pull/91321 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/aarch64] Fix unwinding when signal interrupts a leaf function (PR #91321)
jasonmolenda wrote: maybe the shell test is building without debug info, I am surprised to see assembly there. If I build it like that and run it by hand, ``` (lldb) settings set platform.plugin.darwin.ignored-exceptions EXC_BAD_INSTRUCTION (lldb) b sigill_handler Breakpoint 1: where = a.out`sigill_handler, address = 0x00013f38 (lldb) r Process 25891 launched: '/tmp/a.out' (arm64) Process 25891 stopped * thread #1, queue = 'com.apple.main-thread', stop reason = signal SIGILL frame #0: 0x00013f2c a.out`signal_generating_add + 4 a.out`signal_generating_add: -> 0x13f2c <+4>: udf#0xdead 0x13f30 <+8>: ret 0x13f34 <+12>: brk#0x1 a.out`sigill_handler: 0x13f38 <+0>: subsp, sp, #0x20 (lldb) c Process 25891 resuming Process 25891 stopped * thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1 frame #0: 0x00013f38 a.out`sigill_handler a.out`sigill_handler: -> 0x13f38 <+0>: subsp, sp, #0x20 0x13f3c <+4>: stpx29, x30, [sp, #0x10] 0x13f40 <+8>: addx29, sp, #0x10 0x13f44 <+12>: stur w0, [x29, #-0x4] (lldb) bt * thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1 * frame #0: 0x00013f38 a.out`sigill_handler frame #1: 0x000197f93584 libsystem_platform.dylib`_sigtramp + 56 frame #2: 0x00013f7c a.out`main + 44 frame #3: 0x000197bda0e0 dyld`start + 2360 (lldb) ``` https://github.com/llvm/llvm-project/pull/91321 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/aarch64] Fix unwinding when signal interrupts a leaf function (PR #91321)
jasonmolenda wrote:
Ah, I misunderstood what the nature of the failure was. I tried running the
shell test, and it's failing for different reasons. I almost never touch shell
tests, I find them really hard to debug so I'm not sure what the problem is.
If I run it by hand,
```
(lldb) settings set platform.plugin.darwin.ignored-exceptions
EXC_BAD_INSTRUCTION
(lldb) b sigill_handler
Breakpoint 1: where = a.out`sigill_handler + 20 at
signal-in-leaf-function-aarch64.c:10:34, address = 0x00013f4c
(lldb) r
Process 25854 launched: '/tmp/a.out' (arm64)
Process 25854 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = signal SIGILL
frame #0: 0x00013f2c a.out`signal_generating_add at
signal-in-leaf-function-aarch64.c:5:3
2#include
3
4int __attribute__((naked)) signal_generating_add(int a, int b) {
-> 5 asm("add w0, w1, w0\n\t"
6 "udf #0xdead\n\t"
7 "ret");
8}
(lldb) c
Process 25854 resuming
Process 25854 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
frame #0: 0x00013f4c a.out`sigill_handler(signo=4) at
signal-in-leaf-function-aarch64.c:10:34
7 "ret");
8}
9
-> 10 void sigill_handler(int signo) { _exit(0); }
11
12 int main() {
13 signal(SIGILL, sigill_handler);
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
* frame #0: 0x00013f4c a.out`sigill_handler(signo=4) at
signal-in-leaf-function-aarch64.c:10:34
frame #1: 0x000197f93584 libsystem_platform.dylib`_sigtramp + 56
frame #2: 0x00013f7c a.out`main at
signal-in-leaf-function-aarch64.c:14:3
frame #3: 0x000197bda0e0 dyld`start + 2360
(lldb)
```
which all looks good to me, but it the shell test fails with
```
/Volumes/work/llvm/llvm-project/lldb/test/Shell/Unwind/signal-in-leaf-function-aarch64.test:26:10:
error: CHECK: expected string not found in input
# CHECK: frame #{{[0-9]+}}: [[ADD]] {{.*}}`signal_generating_add
^
:32:86: note: scanning from here
frame #0: 0x00013f38
signal-in-leaf-function-aarch64.test.tmp`sigill_handler
^
:32:86: note: with "ADD" equal to "0x00013f2c"
frame #0: 0x00013f38
signal-in-leaf-function-aarch64.test.tmp`sigill_handler
^
:33:23: note: possible intended match here
signal-in-leaf-function-aarch64.test.tmp`sigill_handler:
^
Input file:
Check file:
/Volumes/work/llvm/llvm-project/lldb/test/Shell/Unwind/signal-in-leaf-function-aarch64.test
-dump-input=help explains the following input dump.
Input was:
<<
.
.
.
27: frame #2: 0x000197bda0e0 dyld`start + 2360
28: (lldb) continue
29: Process 23467 resuming
30: Process 23467 stopped
31: * thread #1, queue = 'com.apple.main-thread', stop reason =
breakpoint 1.1
32: frame #0: 0x00013f38
signal-in-leaf-function-aarch64.test.tmp`sigill_handler
check:26'0
X error: no match found
check:26'1
with "ADD" equal to "0x00013f2c"
33: signal-in-leaf-function-aarch64.test.tmp`sigill_handler:
check:26'0 ~
check:26'2 ?
possible intended match
34: -> 0x13f38 <+0>: sub sp, sp, #0x20
check:26'0 ~~~
35: 0x13f3c <+4>: stp x29, x30, [sp, #0x10]
check:26'0 ~
36: 0x13f40 <+8>: add x29, sp, #0x10
check:26'0 ~~
37: 0x13f44 <+12>: stur w0, [x29, #-0x4]
check:26'0 ~~
38: (lldb) thread backtrace
check:26'0
.
.
.
>>
```
https://github.com/llvm/llvm-project/pull/91321
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/aarch64] Fix unwinding when signal interrupts a leaf function (PR #91321)
jasonmolenda wrote: > I have fixed/worked around the mach exception issue in a [followup > commit](https://github.com/llvm/llvm-project/commit/b903badd73a2467fdd4e363231f2bf9b0704b546) > with a `settings set platform.plugin.darwin.ignored-exceptions > EXC_BAD_INSTRUCTION`. Now the process gets a SIGILL as expected, but it still > fails at the backtrace step (the second one, after stopping inside the > handler). Oh, that's a clever idea, I forgot about that setting. If you add `process handle -p true -s false SIGILL` (lldb was stopping on the SIGILL signal before sigtramp / the registered handler ran), then sigill_handler is called, ``` (lldb) bt * thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1 * frame #0: 0x00013f4c a.out`sigill_handler(signo=4) + 20 at signal-in-leaf-function-aarch64.c:10 frame #1: 0x000197f93584 libsystem_platform.dylib`_sigtramp + 56 frame #2: 0x00013f7c a.out`main + 44 at signal-in-leaf-function-aarch64.c:14 frame #3: 0x000197bda0e0 dyld`start + 2360 ``` https://github.com/llvm/llvm-project/pull/91321 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/aarch64] Fix unwinding when signal interrupts a leaf function (PR #91321)
jasonmolenda wrote: Ah, so the problem is, ``` (lldb) bt * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_INSTRUCTION (code=1, subcode=0xdead) * frame #0: 0x00013f2c b.out`signal_generating_add + 4 at signal-in-leaf-function-aarch64.c:5 frame #1: 0x00013f7c b.out`main + 44 at signal-in-leaf-function-aarch64.c:14 frame #2: 0x000185c76244 dyld`start + 2792 ``` The bad instruction in `signal_generating_add()`, is sent to the program master a EXC_BAD_INSTRUCTION mach exception, not a posix signal (or maybe it was received by lldb as a mach exception and if it had continued to the process it would be recieved as a signal). When I've done these kinds of test cases in the past, I usually add a signal handler and then send the signal to the inferior, e.g. `lldb/test/API/functionalities/unwind/sigtramp/main.c` -- this test is marked `@skipUnlessDarwin` with a comment of ``` # On different platforms the "_sigtramp" and "__kill" frames are likely to be different. # This test could probably be adapted to run on linux/*bsd easily enough. ``` (the test explicitly checks the stack for the function names that should appear above the signal handler) https://github.com/llvm/llvm-project/pull/91321 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/aarch64] Fix unwinding when signal interrupts a leaf function (PR #91321)
jasonmolenda wrote: Will debug this today. https://github.com/llvm/llvm-project/pull/91321 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/aarch64] Fix unwinding when signal interrupts a leaf function (PR #91321)
https://github.com/jasonmolenda approved this pull request. This looks good to me, I know there are other codepaths that handle this correctly, where we can backtrace out of a frameless function that faults into a trap handler and we have the entire register state available in the trap handler. Looking at this, I'm a little uncertain why we have `m_behaves_like_zeroth_frame` and `m_all_registers_available` which are both set to true under the same conditions, and then we sometimes use `m_behaves_like_zeroth_frame`, sometimes `m_all_registers_available `, and sometimes call `RegisterContextUnwind::BehavesLikeZerothFrame` which has a redundant check if the frame number is 0, sigh. Looks like some accumulated nonsense that you shouldn't have to deal with in this patch. https://github.com/llvm/llvm-project/pull/91321 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [debugserver] address preprocessor warning, extra arg (PR #90808)
https://github.com/jasonmolenda closed https://github.com/llvm/llvm-project/pull/90808 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 954d00e - [lldb] MachO delay-init binaries don't load as dependent
Author: Jason Molenda
Date: 2024-05-02T15:20:17-07:00
New Revision: 954d00e87cdd77d0e9e367be52e62340467bd779
URL:
https://github.com/llvm/llvm-project/commit/954d00e87cdd77d0e9e367be52e62340467bd779
DIFF:
https://github.com/llvm/llvm-project/commit/954d00e87cdd77d0e9e367be52e62340467bd779.diff
LOG: [lldb] MachO delay-init binaries don't load as dependent
Added:
Modified:
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
Removed:
diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 1caf93659956b4..4dd23bb1e4dbec 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -5140,8 +5140,17 @@ uint32_t
ObjectFileMachO::GetDependentModules(FileSpecList ) {
case LC_LOADFVMLIB:
case LC_LOAD_UPWARD_DYLIB: {
uint32_t name_offset = cmd_offset + m_data.GetU32();
+bool is_delayed_init = false;
+uint32_t use_command_marker = m_data.GetU32();
+if (use_command_marker == 0x1a741800 /* DYLIB_USE_MARKER */) {
+ offset += 4; /* uint32_t current_version */
+ offset += 4; /* uint32_t compat_version */
+ uint32_t flags = m_data.GetU32();
+ if (flags & 0x08 /* DYLIB_USE_DELAYED_INIT */)
+is_delayed_init = true;
+}
const char *path = m_data.PeekCStr(name_offset);
-if (path) {
+if (path && !is_delayed_init) {
if (load_cmd.cmd == LC_RPATH)
rpath_paths.push_back(path);
else {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [debugserver] address preprocessor warning, extra arg (PR #90808)
@@ -169,25 +173,28 @@ kern_return_t DNBArchMachARM64::GetGPRState(bool force) {
(thread_state_t)_state.context.gpr, );
if (DNBLogEnabledForAny(LOG_THREAD)) {
uint64_t *x = _state.context.gpr.__x[0];
+
+#if defined(DEBUGSERVER_IS_ARM64E)
DNBLogThreaded("thread_get_state signed regs "
"\n fp=%16.16llx"
"\n lr=%16.16llx"
"\n sp=%16.16llx"
"\n pc=%16.16llx",
-#if __has_feature(ptrauth_calls) && defined(__LP64__)
reinterpret_cast(m_state.context.gpr.__opaque_fp),
reinterpret_cast(m_state.context.gpr.__opaque_lr),
reinterpret_cast(m_state.context.gpr.__opaque_sp),
- reinterpret_cast(m_state.context.gpr.__opaque_pc)
+
reinterpret_cast(m_state.context.gpr.__opaque_pc));
#else
- m_state.context.gpr.__fp,
- m_state.context.gpr.__lr,
- m_state.context.gpr.__sp,
- m_state.context.gpr.__pc
+DNBLogThreaded("thread_get_state signed regs "
+ "\n fp=%16.16llx"
+ "\n lr=%16.16llx"
+ "\n sp=%16.16llx"
+ "\n pc=%16.16llx",
+ m_state.context.gpr.__fp, m_state.context.gpr.__lr,
+ m_state.context.gpr.__sp, m_state.context.gpr.__pc);
jasonmolenda wrote:
good idea, done.
https://github.com/llvm/llvm-project/pull/90808
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [debugserver] address preprocessor warning, extra arg (PR #90808)
https://github.com/jasonmolenda updated
https://github.com/llvm/llvm-project/pull/90808
>From 8145e9faaa52209f9800d473fb75f7cfbd2a1185 Mon Sep 17 00:00:00 2001
From: Jason Molenda
Date: Wed, 1 May 2024 18:06:50 -0700
Subject: [PATCH 1/2] [lldb] [debugserver] address preprocessor warning, extra
arg
In DNBArchImplARM64.cpp I'm doing
And the preprocessor warns that this is not defined behavior. This
checks if ptrauth_calls is available and if this is being compiled
64-bit (i.e. arm64e), and defines a single DEBUGSERVER_IS_ARM64E
when those are both true.
I did have to duplicate one DNBLogThreaded() call which itself is a
macro, and using an ifdef in the middle of macro arguments also got
me a warning from the preprocessor.
While testing this for all the different targets, I found a DNBError
initialization that accepts a c-string but I'm passing in a
printf-style formatter c-string and an argument. Create the string
before the call and pass in the constructed string.
rdar://127129242
---
.../debugserver/source/MacOSX/MachProcess.mm | 8 ++---
.../source/MacOSX/arm64/DNBArchImplARM64.cpp | 31 ---
2 files changed, 23 insertions(+), 16 deletions(-)
diff --git a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
b/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
index 70b4564a027b1b..cbe3c5459e91fc 100644
--- a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ b/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -4070,10 +4070,10 @@ static CFStringRef CopyBundleIDForPath(const char
*app_bundle_path,
m_flags |= eMachProcessFlagsAttached;
DNBLog("[LaunchAttach] successfully attached to pid %d", m_pid);
} else {
- launch_err.SetErrorString(
- "Failed to attach to pid %d, BoardServiceLaunchForDebug() unable to "
- "ptrace(PT_ATTACHEXC)",
- m_pid);
+ std::string errmsg = "Failed to attach to pid ";
+ errmsg += std::to_string(m_pid);
+ errmsg += ", BoardServiceLaunchForDebug() unable to
ptrace(PT_ATTACHEXC)";
+ launch_err.SetErrorString(errmsg.c_str());
SetState(eStateExited);
DNBLog("[LaunchAttach] END (%d) error: failed to attach to pid %d",
getpid(), m_pid);
diff --git a/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
b/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
index 57dd2dce6bf5ad..3b1147c69c666b 100644
--- a/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
+++ b/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
@@ -26,8 +26,12 @@
#include
#include
+#undef DEBUGSERVER_IS_ARM64E
#if __has_feature(ptrauth_calls)
#include
+#if defined(__LP64__)
+#define DEBUGSERVER_IS_ARM64E 1
+#endif
#endif
// Break only in privileged or user mode
@@ -115,7 +119,7 @@ static uint64_t clear_pac_bits(uint64_t value) {
uint64_t DNBArchMachARM64::GetPC(uint64_t failValue) {
// Get program counter
if (GetGPRState(false) == KERN_SUCCESS)
-#if __has_feature(ptrauth_calls) && defined(__LP64__)
+#if defined(DEBUGSERVER_IS_ARM64E)
return clear_pac_bits(
reinterpret_cast(m_state.context.gpr.__opaque_pc));
#else
@@ -147,7 +151,7 @@ kern_return_t DNBArchMachARM64::SetPC(uint64_t value) {
uint64_t DNBArchMachARM64::GetSP(uint64_t failValue) {
// Get stack pointer
if (GetGPRState(false) == KERN_SUCCESS)
-#if __has_feature(ptrauth_calls) && defined(__LP64__)
+#if defined(DEBUGSERVER_IS_ARM64E)
return clear_pac_bits(
reinterpret_cast(m_state.context.gpr.__opaque_sp));
#else
@@ -169,25 +173,28 @@ kern_return_t DNBArchMachARM64::GetGPRState(bool force) {
(thread_state_t)_state.context.gpr, );
if (DNBLogEnabledForAny(LOG_THREAD)) {
uint64_t *x = _state.context.gpr.__x[0];
+
+#if defined(DEBUGSERVER_IS_ARM64E)
DNBLogThreaded("thread_get_state signed regs "
"\n fp=%16.16llx"
"\n lr=%16.16llx"
"\n sp=%16.16llx"
"\n pc=%16.16llx",
-#if __has_feature(ptrauth_calls) && defined(__LP64__)
reinterpret_cast(m_state.context.gpr.__opaque_fp),
reinterpret_cast(m_state.context.gpr.__opaque_lr),
reinterpret_cast(m_state.context.gpr.__opaque_sp),
- reinterpret_cast(m_state.context.gpr.__opaque_pc)
+
reinterpret_cast(m_state.context.gpr.__opaque_pc));
#else
- m_state.context.gpr.__fp,
- m_state.context.gpr.__lr,
- m_state.context.gpr.__sp,
- m_state.context.gpr.__pc
+DNBLogThreaded("thread_get_state signed regs "
+ "\n fp=%16.16llx"
+ "\n lr=%16.16llx"
+ "\n sp=%16.16llx"
+ "\n pc=%16.16llx",
+ m_state.context.gpr.__fp, m_state.context.gpr.__lr,
+ m_state.context.gpr.__sp,
[Lldb-commits] [lldb] [lldb] [debugserver] address preprocessor warning, extra arg (PR #90808)
https://github.com/jasonmolenda edited https://github.com/llvm/llvm-project/pull/90808 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [debugserver] address preprocessor warning, extra arg (PR #90808)
https://github.com/jasonmolenda created
https://github.com/llvm/llvm-project/pull/90808
In DNBArchImplARM64.cpp I'm doing
And the preprocessor warns that this is not defined behavior. This checks if
ptrauth_calls is available and if this is being compiled 64-bit (i.e. arm64e),
and defines a single DEBUGSERVER_IS_ARM64E when those are both true.
I did have to duplicate one DNBLogThreaded() call which itself is a macro, and
using an ifdef in the middle of macro arguments also got me a warning from the
preprocessor.
While testing this for all the different targets, I found a DNBError
initialization that accepts a c-string but I'm passing in a printf-style
formatter c-string and an argument. Create the string before the call and pass
in the constructed string.
rdar://127129242
>From 8145e9faaa52209f9800d473fb75f7cfbd2a1185 Mon Sep 17 00:00:00 2001
From: Jason Molenda
Date: Wed, 1 May 2024 18:06:50 -0700
Subject: [PATCH] [lldb] [debugserver] address preprocessor warning, extra arg
In DNBArchImplARM64.cpp I'm doing
And the preprocessor warns that this is not defined behavior. This
checks if ptrauth_calls is available and if this is being compiled
64-bit (i.e. arm64e), and defines a single DEBUGSERVER_IS_ARM64E
when those are both true.
I did have to duplicate one DNBLogThreaded() call which itself is a
macro, and using an ifdef in the middle of macro arguments also got
me a warning from the preprocessor.
While testing this for all the different targets, I found a DNBError
initialization that accepts a c-string but I'm passing in a
printf-style formatter c-string and an argument. Create the string
before the call and pass in the constructed string.
rdar://127129242
---
.../debugserver/source/MacOSX/MachProcess.mm | 8 ++---
.../source/MacOSX/arm64/DNBArchImplARM64.cpp | 31 ---
2 files changed, 23 insertions(+), 16 deletions(-)
diff --git a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
b/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
index 70b4564a027b1b..cbe3c5459e91fc 100644
--- a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ b/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -4070,10 +4070,10 @@ static CFStringRef CopyBundleIDForPath(const char
*app_bundle_path,
m_flags |= eMachProcessFlagsAttached;
DNBLog("[LaunchAttach] successfully attached to pid %d", m_pid);
} else {
- launch_err.SetErrorString(
- "Failed to attach to pid %d, BoardServiceLaunchForDebug() unable to "
- "ptrace(PT_ATTACHEXC)",
- m_pid);
+ std::string errmsg = "Failed to attach to pid ";
+ errmsg += std::to_string(m_pid);
+ errmsg += ", BoardServiceLaunchForDebug() unable to
ptrace(PT_ATTACHEXC)";
+ launch_err.SetErrorString(errmsg.c_str());
SetState(eStateExited);
DNBLog("[LaunchAttach] END (%d) error: failed to attach to pid %d",
getpid(), m_pid);
diff --git a/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
b/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
index 57dd2dce6bf5ad..3b1147c69c666b 100644
--- a/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
+++ b/lldb/tools/debugserver/source/MacOSX/arm64/DNBArchImplARM64.cpp
@@ -26,8 +26,12 @@
#include
#include
+#undef DEBUGSERVER_IS_ARM64E
#if __has_feature(ptrauth_calls)
#include
+#if defined(__LP64__)
+#define DEBUGSERVER_IS_ARM64E 1
+#endif
#endif
// Break only in privileged or user mode
@@ -115,7 +119,7 @@ static uint64_t clear_pac_bits(uint64_t value) {
uint64_t DNBArchMachARM64::GetPC(uint64_t failValue) {
// Get program counter
if (GetGPRState(false) == KERN_SUCCESS)
-#if __has_feature(ptrauth_calls) && defined(__LP64__)
+#if defined(DEBUGSERVER_IS_ARM64E)
return clear_pac_bits(
reinterpret_cast(m_state.context.gpr.__opaque_pc));
#else
@@ -147,7 +151,7 @@ kern_return_t DNBArchMachARM64::SetPC(uint64_t value) {
uint64_t DNBArchMachARM64::GetSP(uint64_t failValue) {
// Get stack pointer
if (GetGPRState(false) == KERN_SUCCESS)
-#if __has_feature(ptrauth_calls) && defined(__LP64__)
+#if defined(DEBUGSERVER_IS_ARM64E)
return clear_pac_bits(
reinterpret_cast(m_state.context.gpr.__opaque_sp));
#else
@@ -169,25 +173,28 @@ kern_return_t DNBArchMachARM64::GetGPRState(bool force) {
(thread_state_t)_state.context.gpr, );
if (DNBLogEnabledForAny(LOG_THREAD)) {
uint64_t *x = _state.context.gpr.__x[0];
+
+#if defined(DEBUGSERVER_IS_ARM64E)
DNBLogThreaded("thread_get_state signed regs "
"\n fp=%16.16llx"
"\n lr=%16.16llx"
"\n sp=%16.16llx"
"\n pc=%16.16llx",
-#if __has_feature(ptrauth_calls) && defined(__LP64__)
reinterpret_cast(m_state.context.gpr.__opaque_fp),
reinterpret_cast(m_state.context.gpr.__opaque_lr),
[Lldb-commits] [lldb] [lldb] Be conversative about setting highmem address masks (PR #90533)
jasonmolenda wrote:
> On the face of it
>
> > When we have an unset high memory address mask, and we are told to set low-
> > and high-memory to the same new address mask, maintain the high memory mask
> > as unset in Process. The same thing is done with the
> > SBProcess::SetAddressMask API when the user specifies
> > lldb.eAddressMaskRangeAll.
>
> Seems to itself be confusing and if you're doing this to address
>
> > When high memory has a different set of masks, those become active in
> > Process and the user can use highmem-virtual-addressable-bits to override
> > only the highmem value, if it is wrong.
>
> You're adding one gotcha to avoid another.
>
> However, I think the result of the highmem mask being unset is the same as
> setting both masks to the same value. Except that `virtual-addressable-bits`
> will now effectively apply to both masks, until the user sets _only_ the high
> mask to some value. At that point, `highmem-virtual-addressable-bits` must be
> used to modify the high mem value.
>
> Correct?
>
> So this doesn't actually change anything for an API user that was setting
> address masks for high and low all at once. They'll still think both are the
> same value, but _internally_, lldb tries high, sees that it's unset, and
> falls back to the low mask.
I'm not thrilled about where I'm ending up.
For the vast majority of our users, the separate address masks for high and low
memory will never occur, it's an unusual environment that sets up the MMU this
way, and operates in both halves of memory. The goal of this patch is to keep
the high memory masks unset (set to LLDB_INVALID_MASK) unless someone sets them
to a value distinct from the "low memory" masks.
I almost think about changing these masks in Process to be a class which holds
either {code, data} or {low code, low data, high code, high data} internally,
with methods to get/set the masks and the requirement that high and low be set
simultaneously when they're both going to be specified so we can detect if it's
genuinely a split-address space situation.
https://github.com/llvm/llvm-project/pull/90533
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Be conversative about setting highmem address masks (PR #90533)
@@ -1465,6 +1465,20 @@ class Process : public
std::enable_shared_from_this,
/// platforms where there is a difference (only Arm Thumb at this time).
lldb::addr_t FixAnyAddress(lldb::addr_t pc);
+ /// Retrieve the actual address masks for high memory code/data,
+ /// without the normal fallbacks of returning the low-memory masks
+ /// or the user settings. Needed by SBProcess to determine if
+ /// the high address masks have actually been set, and should
+ /// be updated when "set all masks" is requested. In most cases,
+ /// the highmem masks should be left to have LLDB_INVALID_ADDRESS_MASK,
+ /// unset.
+ lldb::addr_t GetConcreteHighmemCodeAddressMask() {
jasonmolenda wrote:
I think I also thought about moving to a std::optional for all the masks, but
in the SB API I need a way to say "no mask available" and without creating an
SBAddressMask object with an IsValid() method, I can't represent that without a
magic invalid value.
https://github.com/llvm/llvm-project/pull/90533
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Be conversative about setting highmem address masks (PR #90533)
@@ -1465,6 +1465,20 @@ class Process : public
std::enable_shared_from_this,
/// platforms where there is a difference (only Arm Thumb at this time).
lldb::addr_t FixAnyAddress(lldb::addr_t pc);
+ /// Retrieve the actual address masks for high memory code/data,
+ /// without the normal fallbacks of returning the low-memory masks
+ /// or the user settings. Needed by SBProcess to determine if
+ /// the high address masks have actually been set, and should
+ /// be updated when "set all masks" is requested. In most cases,
+ /// the highmem masks should be left to have LLDB_INVALID_ADDRESS_MASK,
+ /// unset.
+ lldb::addr_t GetConcreteHighmemCodeAddressMask() {
jasonmolenda wrote:
Yeah this is a weird API I almost thought to make it a protected method because
the only user should be SBFrame where we need to distinguish between the
"currently in-effect high memory mask" and "the current Process mask setting,
which may be unset". For the effective high memory mask, if the user hasn't
specified a high memory addressable bits setting, and the process doesn't have
a mask set, then we are in a "one-mask-for-all-addresses" mode and we return
the low memory address mask.
https://github.com/llvm/llvm-project/pull/90533
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Be conversative about setting highmem address masks (PR #90533)
https://github.com/jasonmolenda updated
https://github.com/llvm/llvm-project/pull/90533
>From 3c272e99326a287f0a61c1f8c2c7ee790e1aeb48 Mon Sep 17 00:00:00 2001
From: Jason Molenda
Date: Mon, 29 Apr 2024 16:45:36 -0700
Subject: [PATCH 1/2] [lldb] Be conversative about setting highmem address
masks
The most common case with address masks/addressable bits is that
we have one value/mask that applies across the entire address space,
for code and data. On AArch64, we can have separate masks for high
and low address spaces. In the normal "one mask for all memory"
case, we use the low memory masks in Process, and we use the
`virtual-addressable-bits` setting for the user to override that
value. When high memory has a different set of masks, those become
active in Process and the user can use `highmem-virtual-addressable-bits`
to override only the highmem value, if it is wrong.
This patch is handling a case where a gdb stub sent incorrect address
masks, for both high and low memory:
```
(lldb) process plugin packet send qHostInfo
packet: qHostInfo
response:
cputype:16777228;cpusubtype:0;endian:little;ptrsize:8;low_mem_addressing_bits:64;high_mem_addressing_bits:64;
```
When in fact this target was using 47 bits of addressing. This
qHostInfo response set the Process low and high address masks for
code and data so that no bit-clearing is performed. The user tried
to override this with the virtual-addressable-bits setting, and was
surprised when it had no affect on code running in high memory.
Because the high memory code mask now has a setting (all bits are
addressable) and they needed to use the very-uncommon
highmem-virtual-addressable-bits setting.
When we have an *unset* high memory address mask, and we are told
to set low- and high-memory to the same new address mask, maintain
the high memory mask as unset in Process. The same thing is done
with the SBProcess::SetAddressMask API when the user specifies
lldb.eAddressMaskRangeAll.
Added a new test case to TestAddressMasks.py to test that the
low-memory override works correctly. Also fixed a bug in the
testsuite that I did where I added `@skipIf(archs=["arm"])`
to avoid a problem on the Linaro 32-bit arm Ubuntu CI bot. I
forgot that this is a regex, and so the tests were being skipped
entirely on any arm.* system.
---
lldb/source/API/SBProcess.cpp | 26 ---
lldb/source/Target/Process.cpp| 20 +++---
.../process/address-masks/TestAddressMasks.py | 21 ---
3 files changed, 56 insertions(+), 11 deletions(-)
diff --git a/lldb/source/API/SBProcess.cpp b/lldb/source/API/SBProcess.cpp
index c37c111c5a58e0..a0654d23e67efd 100644
--- a/lldb/source/API/SBProcess.cpp
+++ b/lldb/source/API/SBProcess.cpp
@@ -1298,7 +1298,12 @@ void SBProcess::SetAddressMask(AddressMaskType type,
addr_t mask,
case eAddressMaskTypeCode:
if (addr_range == eAddressMaskRangeAll) {
process_sp->SetCodeAddressMask(mask);
-process_sp->SetHighmemCodeAddressMask(mask);
+// If the highmem mask is the default-invalid,
+// don't change it, keep everything consistently
+// using the lowmem all-address-space mask.
+if (process_sp->GetHighmemCodeAddressMask() !=
+LLDB_INVALID_ADDRESS_MASK)
+ process_sp->SetHighmemCodeAddressMask(mask);
} else if (addr_range == eAddressMaskRangeHigh) {
process_sp->SetHighmemCodeAddressMask(mask);
} else {
@@ -1308,7 +1313,12 @@ void SBProcess::SetAddressMask(AddressMaskType type,
addr_t mask,
case eAddressMaskTypeData:
if (addr_range == eAddressMaskRangeAll) {
process_sp->SetDataAddressMask(mask);
-process_sp->SetHighmemDataAddressMask(mask);
+// If the highmem mask is the default-invalid,
+// don't change it, keep everything consistently
+// using the lowmem all-address-space mask.
+if (process_sp->GetHighmemDataAddressMask() !=
+LLDB_INVALID_ADDRESS_MASK)
+ process_sp->SetHighmemDataAddressMask(mask);
} else if (addr_range == eAddressMaskRangeHigh) {
process_sp->SetHighmemDataAddressMask(mask);
} else {
@@ -1319,8 +1329,16 @@ void SBProcess::SetAddressMask(AddressMaskType type,
addr_t mask,
if (addr_range == eAddressMaskRangeAll) {
process_sp->SetCodeAddressMask(mask);
process_sp->SetDataAddressMask(mask);
-process_sp->SetHighmemCodeAddressMask(mask);
-process_sp->SetHighmemDataAddressMask(mask);
+// If the highmem masks are the default-invalid,
+// don't change them, keep everything consistently
+// using the lowmem all-address-space masks.
+if (process_sp->GetHighmemDataAddressMask() !=
+LLDB_INVALID_ADDRESS_MASK ||
+process_sp->GetHighmemCodeAddressMask() !=
+LLDB_INVALID_ADDRESS_MASK) {
+ process_sp->SetHighmemCodeAddressMask(mask);
+
[Lldb-commits] [lldb] [lldb] Be conversative about setting highmem address masks (PR #90533)
https://github.com/jasonmolenda created
https://github.com/llvm/llvm-project/pull/90533
The most common case with address masks/addressable bits is that we have one
value/mask that applies across the entire address space, for code and data. On
AArch64, we can have separate masks for high and low address spaces. In the
normal "one mask for all memory" case, we use the low memory masks in Process,
and we use the `virtual-addressable-bits` setting for the user to override that
value. When high memory has a different set of masks, those become active in
Process and the user can use `highmem-virtual-addressable-bits` to override
only the highmem value, if it is wrong.
This patch is handling a case where a gdb stub sent incorrect address masks,
for both high and low memory:
```
(lldb) process plugin packet send qHostInfo
packet: qHostInfo
response:
cputype:16777228;cpusubtype:0;endian:little;ptrsize:8;low_mem_addressing_bits:64;high_mem_addressing_bits:64;
```
When in fact this target was using 47 bits of addressing. This qHostInfo
response set the Process low and high address masks for code and data so that
no bit-clearing is performed. The user tried to override this with the
virtual-addressable-bits setting, and was surprised when it had no affect on
code running in high memory. Because the high memory code mask now has a
setting (all bits are addressable) and they needed to use the very-uncommon
highmem-virtual-addressable-bits setting.
When we have an *unset* high memory address mask, and we are told to set low-
and high-memory to the same new address mask, maintain the high memory mask as
unset in Process. The same thing is done with the SBProcess::SetAddressMask
API when the user specifies lldb.eAddressMaskRangeAll.
Added a new test case to TestAddressMasks.py to test that the low-memory
override works correctly. Also fixed a bug in the testsuite that I did where I
added `@skipIf(archs=["arm"])` to avoid a problem on the Linaro 32-bit arm
Ubuntu CI bot. I forgot that this is a regex, and so the tests were being
skipped entirely on any arm.* system.
>From 3c272e99326a287f0a61c1f8c2c7ee790e1aeb48 Mon Sep 17 00:00:00 2001
From: Jason Molenda
Date: Mon, 29 Apr 2024 16:45:36 -0700
Subject: [PATCH] [lldb] Be conversative about setting highmem address masks
The most common case with address masks/addressable bits is that
we have one value/mask that applies across the entire address space,
for code and data. On AArch64, we can have separate masks for high
and low address spaces. In the normal "one mask for all memory"
case, we use the low memory masks in Process, and we use the
`virtual-addressable-bits` setting for the user to override that
value. When high memory has a different set of masks, those become
active in Process and the user can use `highmem-virtual-addressable-bits`
to override only the highmem value, if it is wrong.
This patch is handling a case where a gdb stub sent incorrect address
masks, for both high and low memory:
```
(lldb) process plugin packet send qHostInfo
packet: qHostInfo
response:
cputype:16777228;cpusubtype:0;endian:little;ptrsize:8;low_mem_addressing_bits:64;high_mem_addressing_bits:64;
```
When in fact this target was using 47 bits of addressing. This
qHostInfo response set the Process low and high address masks for
code and data so that no bit-clearing is performed. The user tried
to override this with the virtual-addressable-bits setting, and was
surprised when it had no affect on code running in high memory.
Because the high memory code mask now has a setting (all bits are
addressable) and they needed to use the very-uncommon
highmem-virtual-addressable-bits setting.
When we have an *unset* high memory address mask, and we are told
to set low- and high-memory to the same new address mask, maintain
the high memory mask as unset in Process. The same thing is done
with the SBProcess::SetAddressMask API when the user specifies
lldb.eAddressMaskRangeAll.
Added a new test case to TestAddressMasks.py to test that the
low-memory override works correctly. Also fixed a bug in the
testsuite that I did where I added `@skipIf(archs=["arm"])`
to avoid a problem on the Linaro 32-bit arm Ubuntu CI bot. I
forgot that this is a regex, and so the tests were being skipped
entirely on any arm.* system.
---
lldb/source/API/SBProcess.cpp | 26 ---
lldb/source/Target/Process.cpp| 20 +++---
.../process/address-masks/TestAddressMasks.py | 21 ---
3 files changed, 56 insertions(+), 11 deletions(-)
diff --git a/lldb/source/API/SBProcess.cpp b/lldb/source/API/SBProcess.cpp
index c37c111c5a58e0..a0654d23e67efd 100644
--- a/lldb/source/API/SBProcess.cpp
+++ b/lldb/source/API/SBProcess.cpp
@@ -1298,7 +1298,12 @@ void SBProcess::SetAddressMask(AddressMaskType type,
addr_t mask,
case eAddressMaskTypeCode:
if (addr_range == eAddressMaskRangeAll) {
[Lldb-commits] [lldb] [lldb] Recognize DW_TAG_LLVM_ptrauth_type as a type qual (PR #90140)
https://github.com/jasonmolenda closed https://github.com/llvm/llvm-project/pull/90140 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add SBValue::GetValueAsAddress API (PR #90144)
https://github.com/jasonmolenda closed https://github.com/llvm/llvm-project/pull/90144 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add SBValue::GetValueAsAddress API (PR #90144)
https://github.com/jasonmolenda updated
https://github.com/llvm/llvm-project/pull/90144
>From 3b66943d9c49fd5e71c2eb136b5bb3311e932bbc Mon Sep 17 00:00:00 2001
From: Jason Molenda
Date: Thu, 25 Apr 2024 15:51:44 -0700
Subject: [PATCH 1/2] [lldb] Add SBValue::GetValueAsAddress API
I previously added this API via https://reviews.llvm.org/D142792
in 2023, along with changes to the ValueObject class to treat
pointer types as addresses, and to annotate those ValueObjects
with the original uint64_t byte sequence AND the name of the
symbol once stripped, if that points to a symbol.
I did this unconditionally for all pointer type ValueObjects, and
it caused several regressions in the Objective-C data formatters
which have a ValueObject of an object, it has the address of its
class -- but with ObjC, sometimes it is a "tagged pointer" which
is metadata, not an actual pointer. (e.g. a small NSInteger value
is stored entirely in the tagged pointer, instead of a separate
object) Treating these not-addresses as addresses -- clearing the
non-addressable-bits -- is invalid.
The original version of this patch we're using downstream only
does this bits clearing for pointer types that are specifically
decorated with the pointerauth typequal, but not all of those
clang changes are upstreamed to github main yet, so I tried this
simpler approach and hit the tagged pointer issue and bailed on
the whole patch.
This patch, however, is simply adding SBValue::GetValueAsAddress
so script writers who know that an SBValue has an address in
memory, can strip off any metadata. It's an important API to have
for script writers when AArch64 ptrauth is in use, so I'm
going to put this part of the patch back on github main now until
we can get the rest of that original patch upstreamed.
---
lldb/bindings/interface/SBValueDocstrings.i | 20 +++
lldb/include/lldb/API/SBValue.h | 2 +
lldb/source/API/SBValue.cpp | 20 +++
.../Makefile | 3 +
.../TestClearSBValueNonAddressableBits.py | 60 +++
.../clear-sbvalue-nonaddressable-bits/main.c | 27 +
6 files changed, 132 insertions(+)
create mode 100644 lldb/test/API/clear-sbvalue-nonaddressable-bits/Makefile
create mode 100644
lldb/test/API/clear-sbvalue-nonaddressable-bits/TestClearSBValueNonAddressableBits.py
create mode 100644 lldb/test/API/clear-sbvalue-nonaddressable-bits/main.c
diff --git a/lldb/bindings/interface/SBValueDocstrings.i
b/lldb/bindings/interface/SBValueDocstrings.i
index 6bab923e8b35a6..59fa807f5ec95c 100644
--- a/lldb/bindings/interface/SBValueDocstrings.i
+++ b/lldb/bindings/interface/SBValueDocstrings.i
@@ -135,6 +135,26 @@ linked list."
%feature("docstring", "Expands nested expressions like .a->b[0].c[1]->d."
) lldb::SBValue::GetValueForExpressionPath;
+%feature("docstring", "
+ Return the value as an address. On failure, LLDB_INVALID_ADDRESS
+ will be returned. On architectures like AArch64, where the
+ top (unaddressable) bits can be used for authentication,
+ memory tagging, or top byte ignore, this method will return
+ the value with those top bits cleared.
+
+ GetValueAsUnsigned returns the actual value, with the
+ authentication/Top Byte Ignore/Memory Tagging Extension bits.
+
+ Calling this on a random value which is not a pointer is
+ incorrect. Call GetType().IsPointerType() if in doubt.
+
+ An SB API program may want to show both the literal byte value
+ and the address it refers to in memory. These two SBValue
+ methods allow SB API writers to behave appropriately for their
+ interface."
+) lldb::SBValue::GetValueAsAddress;
+
+
%feature("doctstring", "
Returns the number for children.
diff --git a/lldb/include/lldb/API/SBValue.h b/lldb/include/lldb/API/SBValue.h
index bbcccaab51aaee..6fac0961910540 100644
--- a/lldb/include/lldb/API/SBValue.h
+++ b/lldb/include/lldb/API/SBValue.h
@@ -68,6 +68,8 @@ class LLDB_API SBValue {
uint64_t GetValueAsUnsigned(uint64_t fail_value = 0);
+ lldb::addr_t GetValueAsAddress();
+
ValueType GetValueType();
// If you call this on a newly created ValueObject, it will always return
diff --git a/lldb/source/API/SBValue.cpp b/lldb/source/API/SBValue.cpp
index 94a8f3ea319e89..c8b664c86ec2d9 100644
--- a/lldb/source/API/SBValue.cpp
+++ b/lldb/source/API/SBValue.cpp
@@ -909,6 +909,26 @@ uint64_t SBValue::GetValueAsUnsigned(uint64_t fail_value) {
return fail_value;
}
+lldb::addr_t SBValue::GetValueAsAddress() {
+ addr_t fail_value = LLDB_INVALID_ADDRESS;
+ ValueLocker locker;
+ lldb::ValueObjectSP value_sp(GetSP(locker));
+ if (value_sp) {
+bool success = true;
+uint64_t ret_val = fail_value;
+ret_val = value_sp->GetValueAsUnsigned(fail_value, );
+if (!success) {
+ return fail_value;
+}
+ProcessSP process_sp = m_opaque_sp->GetProcessSP();
+if (!process_sp)
+ return ret_val;
+
[Lldb-commits] [lldb] [lldb] Add SBValue::GetValueAsAddress API (PR #90144)
https://github.com/jasonmolenda created
https://github.com/llvm/llvm-project/pull/90144
I previously added this API via https://reviews.llvm.org/D142792 in 2023, along
with changes to the ValueObject class to treat pointer types as addresses, and
to annotate those ValueObjects with the original uint64_t byte sequence AND the
name of the symbol once stripped, if that points to a symbol.
I did this unconditionally for all pointer type ValueObjects, and it caused
several regressions in the Objective-C data formatters which have a ValueObject
of an object, it has the address of its class -- but with ObjC, sometimes it is
a "tagged pointer" which is metadata, not an actual pointer. (e.g. a small
NSInteger value is stored entirely in the tagged pointer, instead of a separate
object) Treating these not-addresses as addresses -- clearing the
non-addressable-bits -- is invalid.
The original version of this patch we're using downstream only does this bits
clearing for pointer types that are specifically decorated with the pointerauth
typequal, but not all of those clang changes are upstreamed to github main yet,
so I tried this simpler approach and hit the tagged pointer issue and bailed on
the whole patch.
This patch, however, is simply adding SBValue::GetValueAsAddress so script
writers who know that an SBValue has an address in memory, can strip off any
metadata. It's an important API to have for script writers when AArch64
ptrauth is in use, so I'm going to put this part of the patch back on github
main now until we can get the rest of that original patch upstreamed.
>From 3b66943d9c49fd5e71c2eb136b5bb3311e932bbc Mon Sep 17 00:00:00 2001
From: Jason Molenda
Date: Thu, 25 Apr 2024 15:51:44 -0700
Subject: [PATCH] [lldb] Add SBValue::GetValueAsAddress API
I previously added this API via https://reviews.llvm.org/D142792
in 2023, along with changes to the ValueObject class to treat
pointer types as addresses, and to annotate those ValueObjects
with the original uint64_t byte sequence AND the name of the
symbol once stripped, if that points to a symbol.
I did this unconditionally for all pointer type ValueObjects, and
it caused several regressions in the Objective-C data formatters
which have a ValueObject of an object, it has the address of its
class -- but with ObjC, sometimes it is a "tagged pointer" which
is metadata, not an actual pointer. (e.g. a small NSInteger value
is stored entirely in the tagged pointer, instead of a separate
object) Treating these not-addresses as addresses -- clearing the
non-addressable-bits -- is invalid.
The original version of this patch we're using downstream only
does this bits clearing for pointer types that are specifically
decorated with the pointerauth typequal, but not all of those
clang changes are upstreamed to github main yet, so I tried this
simpler approach and hit the tagged pointer issue and bailed on
the whole patch.
This patch, however, is simply adding SBValue::GetValueAsAddress
so script writers who know that an SBValue has an address in
memory, can strip off any metadata. It's an important API to have
for script writers when AArch64 ptrauth is in use, so I'm
going to put this part of the patch back on github main now until
we can get the rest of that original patch upstreamed.
---
lldb/bindings/interface/SBValueDocstrings.i | 20 +++
lldb/include/lldb/API/SBValue.h | 2 +
lldb/source/API/SBValue.cpp | 20 +++
.../Makefile | 3 +
.../TestClearSBValueNonAddressableBits.py | 60 +++
.../clear-sbvalue-nonaddressable-bits/main.c | 27 +
6 files changed, 132 insertions(+)
create mode 100644 lldb/test/API/clear-sbvalue-nonaddressable-bits/Makefile
create mode 100644
lldb/test/API/clear-sbvalue-nonaddressable-bits/TestClearSBValueNonAddressableBits.py
create mode 100644 lldb/test/API/clear-sbvalue-nonaddressable-bits/main.c
diff --git a/lldb/bindings/interface/SBValueDocstrings.i
b/lldb/bindings/interface/SBValueDocstrings.i
index 6bab923e8b35a6..59fa807f5ec95c 100644
--- a/lldb/bindings/interface/SBValueDocstrings.i
+++ b/lldb/bindings/interface/SBValueDocstrings.i
@@ -135,6 +135,26 @@ linked list."
%feature("docstring", "Expands nested expressions like .a->b[0].c[1]->d."
) lldb::SBValue::GetValueForExpressionPath;
+%feature("docstring", "
+ Return the value as an address. On failure, LLDB_INVALID_ADDRESS
+ will be returned. On architectures like AArch64, where the
+ top (unaddressable) bits can be used for authentication,
+ memory tagging, or top byte ignore, this method will return
+ the value with those top bits cleared.
+
+ GetValueAsUnsigned returns the actual value, with the
+ authentication/Top Byte Ignore/Memory Tagging Extension bits.
+
+ Calling this on a random value which is not a pointer is
+ incorrect. Call GetType().IsPointerType() if in doubt.
+
+ An SB
[Lldb-commits] [lldb] [lldb] Recognize DW_TAG_LLVM_ptrauth_type as a type qual (PR #90140)
https://github.com/jasonmolenda created
https://github.com/llvm/llvm-project/pull/90140
Jonas upstreamed recognition of DW_TAG_LLVM_ptrauth_type
https://reviews.llvm.org/D130215 but it isn't recognized as a type qualifier
tag in DWARFASTParserClang::ParseTypeFromDWARF.
>From b5e6f50797ddde235dde86f8e2f15a9fd6632092 Mon Sep 17 00:00:00 2001
From: Jason Molenda
Date: Thu, 25 Apr 2024 15:31:51 -0700
Subject: [PATCH] [lldb] Recognize DW_TAG_LLVM_ptrauth_type as a type qual
Jonas upstreamed recognition of DW_TAG_LLVM_ptrauth_type
https://reviews.llvm.org/D130215 but it isn't recognized as a type
qualifier tag in DWARFASTParserClang::ParseTypeFromDWARF.
---
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 41d81fbcf1b087..12dafd3f5d5d51 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -495,6 +495,7 @@ TypeSP DWARFASTParserClang::ParseTypeFromDWARF(const
SymbolContext ,
case DW_TAG_const_type:
case DW_TAG_restrict_type:
case DW_TAG_volatile_type:
+ case DW_TAG_LLVM_ptrauth_type:
case DW_TAG_atomic_type:
case DW_TAG_unspecified_type: {
type_sp = ParseTypeModifier(sc, die, attrs);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Docs] Make formatting regular in lldb-gdb-remote.txt (PR #89587)
https://github.com/jasonmolenda approved this pull request. Yes please, very long overdue, I've felt bad about not trying to do something here myself. https://github.com/llvm/llvm-project/pull/89587 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DynamicLoader] Fix lldb unable to stop at _dl_debug_state if user set it before the process launched. (PR #88792)
https://github.com/jasonmolenda approved this pull request. This looks good, thanks for the revisions. https://github.com/llvm/llvm-project/pull/88792 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DynamicLoader] Fix lldb unable to stop at _dl_debug_state if user set it before the process launched. (PR #88792)
jasonmolenda wrote: Skimming DynamicLoaderPOSIXDYLD (as it is written today), it looks like attach/launch call `SetRendezvousBreakpoint()` which (1) loads ld.so into lldb if it isn't already there, and (2) sets the breakpoint to be notified about future binaries loading. It loads ld.so via `LoadInterpreterModule` which Finds It somehow (I stopped reading), adds it to the Target, and sets the section load addresses, returns the module_sp. Then it sets breakpoints on a half dozen notification function names in the ld.so Module. This is all special-cased for ld.so, and we do not evaluate user breakpoints because we didn't call `Target::ModulesDidLoad` for ld.so. The code path for other binaries goes through `DynamicLoaderPOSIXDYLD::RefreshModules` which does the normal approach of adding binaries to the Target, setting the load addresses, then calling ModulesDidLoad which will evaluate breakpoints that may need to be set in the new binaries. It seems like a simpler fix would be to have `DynamicLoaderPOSIXDYLD::LoadInterpreterModule` create a ModuleList with the ModuleSP of ld.so that it has just added to Target and set its section load addresses, then call `ModulesDidLoad` on it so that user breakpoints are evaluated and inserted. The patch as it is written right now is taking one part of ModulesDidLoad and putting it in a separate method that `DynamicLoaderPOSIXDYLD::LoadInterpreterModule` is calling -- why not just call ModulesDidLoad and delegate this (and possibly other binary-just-loaded) behaviors to it? https://github.com/llvm/llvm-project/pull/88792 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix nullptr dereference on running x86 binary with x86-disabled llvm (PR #82603)
https://github.com/jasonmolenda approved this pull request. Looks good, please land it. Thanks for the pings and rewriting the test case! https://github.com/llvm/llvm-project/pull/82603 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][MinidumpFileBuilder] Fix addition of MemoryList steam (PR #88564)
@@ -655,9 +655,10 @@ MinidumpFileBuilder::AddMemoryList(const lldb::ProcessSP _sp, const addr_t addr = core_range.range.start(); const addr_t size = core_range.range.size(); auto data_up = std::make_unique(size, 0); +Status read_error; jasonmolenda wrote: I don't work on this plugin myself, I'm sure the way you expressed the idea is fine, it's little more than a style difference. https://github.com/llvm/llvm-project/pull/88564 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][MinidumpFileBuilder] Fix addition of MemoryList steam (PR #88564)
@@ -655,9 +655,10 @@ MinidumpFileBuilder::AddMemoryList(const lldb::ProcessSP
_sp,
const addr_t addr = core_range.range.start();
const addr_t size = core_range.range.size();
auto data_up = std::make_unique(size, 0);
+Status read_error;
jasonmolenda wrote:
The goal is to skip memory ranges that couldn't be read, without surfacing an
error about them, right. I don't mind it that much, but another way to be to
use the existing `error` Status object (which we know is state==Success at this
point), and if our memory read does fail, we could do
```
if (error.Fail() || bytes_read == 0) {
error.Clear();
continue;
}
```
To make it clear that we don't want to surface a memory read failure for a
region to the caller. But this is more of a style preference I think.
https://github.com/llvm/llvm-project/pull/88564
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [libcxx] [lldb] [libc++][CI] Tests LLDB libc++ data formatters. (PR #88312)
jasonmolenda wrote: The lldb change part of this PR looks good to me. https://github.com/llvm/llvm-project/pull/88312 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix nullptr dereference on running x86 binary with x86-disabled llvm (PR #82603)
jasonmolenda wrote:
Hi sorry @kovdan01 I missed this one in the emails. You're using an lldb which
was built without the `LLVM_TARGETS_TO_BUILD` including X86, and running that
lldb on an x86 corefile, got it. I have low confidence how well lldb will work
in this situation, e.g. inferior function calls are obviously going to fail
completely, and possibly not in a graceful way, but that doesn't impact
corefiles. I'm less thrilled about adding a 570kb corefile to the repository
to check this combination doesn't crash the unwinder. In
lldb/unittest/UnwindAssembly we build the `x86` directory when
```
if ("X86" IN_LIST LLVM_TARGETS_TO_BUILD)
add_subdirectory(x86)
endif()
```
In Testx86AssemblyInspectionEngine.cpp we initialize llvm state in
`Testx86AssemblyInspectionEngine::SetUpTestCase` and then run individual tests
in the `TEST_F()` entries, creating a byte stream of prologues like
```
// 'int main() { }' compiled for x86_64-apple-macosx with clang
uint8_t data[] = {
0x55, // offset 0 -- pushq %rbp
0x48, 0x89, 0xe5, // offset 1 -- movq %rsp, %rbp
0x31, 0xc0, // offset 4 -- xorl %eax, %eax
0x5d, // offset 6 -- popq %rbp
0xc3 // offset 7 -- retq
};
```
and run the unwind engine on those bytes.
Could we add a `x86-but-no-x86-target` directory, write one test to see that
the unwind engine can run against a byte buffer like this and not crash instead
maybe?
https://github.com/llvm/llvm-project/pull/82603
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Reland "[lldb][lit] Add MallocNanoZone envvar to Darwin ASan builds" … (PR #88442)
https://github.com/jasonmolenda approved this pull request. This looks good to me. https://github.com/llvm/llvm-project/pull/88442 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix error in unrecognized register name handling for "SBFrame.register" (PR #88047)
https://github.com/jasonmolenda approved this pull request. Looks good. https://github.com/llvm/llvm-project/pull/88047 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 41dc04e - [lldb] Add swig doc for SBProcess address mask methods
Author: Jason Molenda
Date: 2024-04-08T18:56:39-07:00
New Revision: 41dc04e5283adef9979cad2b126ab3e6c156034a
URL:
https://github.com/llvm/llvm-project/commit/41dc04e5283adef9979cad2b126ab3e6c156034a
DIFF:
https://github.com/llvm/llvm-project/commit/41dc04e5283adef9979cad2b126ab3e6c156034a.diff
LOG: [lldb] Add swig doc for SBProcess address mask methods
Add descriptions of `GetAddressMask`, `SetAddressMask`,
`SetAddressableBits`, and `FixAddress` SBProcess methods.
Added:
Modified:
lldb/bindings/interface/SBProcessDocstrings.i
Removed:
diff --git a/lldb/bindings/interface/SBProcessDocstrings.i
b/lldb/bindings/interface/SBProcessDocstrings.i
index c20ef3e4655bd4..1b98a79e4f6d36 100644
--- a/lldb/bindings/interface/SBProcessDocstrings.i
+++ b/lldb/bindings/interface/SBProcessDocstrings.i
@@ -199,6 +199,47 @@ SBProcess supports thread iteration. For example (from
test/lldbutil.py), ::
process_info.GetProcessID()"
) lldb::SBProcess::GetProcessInfo;
+%feature("docstring", "
+Get the current address mask in this Process of a given type.
+There are lldb.eAddressMaskTypeCode and lldb.eAddressMaskTypeData address
+masks, and on most Targets, the the Data address mask is more general
+because there are no alignment restrictions, as there can be with Code
+addresses.
+lldb.eAddressMaskTypeAny may be used to get the most general mask.
+The bits which are not used for addressing are set to 1 in the returned
+mask.
+In an unusual environment with
diff erent address masks for high and low
+memory, this may also be specified. This is uncommon, default is
+lldb.eAddressMaskRangeLow."
+) lldb::SBProcess::GetAddressMask;
+
+%feature("docstring", "
+Set the current address mask in this Process for a given type,
+lldb.eAddressMaskTypeCode or lldb.eAddressMaskTypeData. Bits that are not
+used for addressing should be set to 1 in the mask.
+When setting all masks, lldb.eAddressMaskTypeAll may be specified.
+In an unusual environment with
diff erent address masks for high and low
+memory, this may also be specified. This is uncommon, default is
+lldb.eAddressMaskRangeLow."
+) lldb::SBProcess::SetAddressMask;
+
+%feature("docstring", "
+Set the number of low bits relevant for addressing in this Process
+for a given type, lldb.eAddressMaskTypeCode or lldb.eAddressMaskTypeData.
+When setting all masks, lldb.eAddressMaskTypeAll may be specified.
+In an unusual environment with
diff erent address masks for high and low
+memory, the address range may also be specified. This is uncommon,
+default is lldb.eAddressMaskRangeLow."
+) lldb::SBProcess::SetAddressableBits;
+
+%feature("docstring", "
+Given a virtual address, clear the bits that are not used for addressing
+(and may be used for metadata, memory tagging, point authentication, etc).
+By default the most general mask, lldb.eAddressMaskTypeAny is used to
+process the address, but lldb.eAddressMaskTypeData and
+lldb.eAddressMaskTypeCode may be specified if the type of address is
known."
+) lldb::SBProcess::FixAddress;
+
%feature("docstring", "
Allocates a block of memory within the process, with size and
access permissions specified in the arguments. The permissions
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [NFC] Fix swig docstring annotations (PR #88073)
https://github.com/jasonmolenda closed https://github.com/llvm/llvm-project/pull/88073 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [NFC] Fix swig docstring annotations (PR #88073)
jasonmolenda wrote: An example of the difference in the output this makes for `script help (lldb.SBProcess)`. Old: ``` | PutSTDIN(self, src) | Writes data into the current process's stdin. API client specifies a Python | string as the only argument. ``` new: ``` | PutSTDIN(self, src) | PutSTDIN(SBProcess self, char const * src) -> size_t | | Writes data into the current process's stdin. API client specifies a Python | string as the only argument. ``` https://github.com/llvm/llvm-project/pull/88073 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [NFC] Fix swig docstring annotations (PR #88073)
https://github.com/jasonmolenda created
https://github.com/llvm/llvm-project/pull/88073
Some of the SB API method description docstrings for swing are annotated as
`%feature("autodoc")` - but `"autodoc"` annotations are only to substitute a
string showing the arguments and return variables - either in a single line, or
in multiple lines. SBMemoryRegionInfo used `"autodoc"` correctly describing the
parameters and return type, but then it added a description too which is not
correct either.
Change all of these that are adding a method description to use
`%feature("docstring")` instead. There were a half dozen instances where
`"autodoc"` was correctly being used and we have overriden the parameter and
return types with a more readable version.
>From 97f8f87fc4b14a973eb0bb0ac67ff36f1f4ef08a Mon Sep 17 00:00:00 2001
From: Jason Molenda
Date: Mon, 8 Apr 2024 17:17:16 -0700
Subject: [PATCH] [lldb] [NFC] Fix swig docstring annotations
Some of the SB API method description docstrings for swing are
annotated as `%feature("autodoc")` - but `"autodoc"` annotations
are only to substitute a string showing the arguments and return
variables - either in a single line, or in multiple lines.
SBMemoryRegionInfo used `"autodoc"` correctly describing the
parameters and return type, but then it added a description too
which is not correct either.
Change all of these that are adding a method description to use
`%feature("docstring")` instead. There were a half dozen instances
where `"autodoc"` was correctly being used and we have overriden
the parameter and return types with a more readable version.
---
.../interface/SBMemoryRegionInfoDocstrings.i | 12 ++---
lldb/bindings/interface/SBProcessDocstrings.i | 50 +--
lldb/bindings/interface/SBQueueDocstrings.i | 4 +-
lldb/bindings/interface/SBThreadDocstrings.i | 36 +++--
4 files changed, 48 insertions(+), 54 deletions(-)
diff --git a/lldb/bindings/interface/SBMemoryRegionInfoDocstrings.i
b/lldb/bindings/interface/SBMemoryRegionInfoDocstrings.i
index bd80740f3fdd3b..d7c68baf100e27 100644
--- a/lldb/bindings/interface/SBMemoryRegionInfoDocstrings.i
+++ b/lldb/bindings/interface/SBMemoryRegionInfoDocstrings.i
@@ -2,8 +2,7 @@
"API clients can get information about memory regions in processes."
) lldb::SBMemoryRegionInfo;
-%feature("autodoc", "
-GetRegionEnd(SBMemoryRegionInfo self) -> lldb::addr_t
+%feature("docstring", "
Returns whether this memory region has a list of modified (dirty)
pages available or not. When calling GetNumDirtyPages(), you will
have 0 returned for both \"dirty page list is not known\" and
@@ -11,8 +10,7 @@
memory region). You must use this method to disambiguate."
) lldb::SBMemoryRegionInfo::HasDirtyMemoryPageList;
-%feature("autodoc", "
-GetNumDirtyPages(SBMemoryRegionInfo self) -> uint32_t
+%feature("docstring", "
Return the number of dirty (modified) memory pages in this
memory region, if available. You must use the
SBMemoryRegionInfo::HasDirtyMemoryPageList() method to
@@ -20,16 +18,14 @@
on the target system can provide this information."
) lldb::SBMemoryRegionInfo::GetNumDirtyPages;
-%feature("autodoc", "
-GetDirtyPageAddressAtIndex(SBMemoryRegionInfo self, uint32_t idx) ->
lldb::addr_t
+%feature("docstring", "
Return the address of a modified, or dirty, page of memory.
If the provided index is out of range, or this memory region
does not have dirty page information, LLDB_INVALID_ADDRESS
is returned."
) lldb::SBMemoryRegionInfo::GetDirtyPageAddressAtIndex;
-%feature("autodoc", "
-GetPageSize(SBMemoryRegionInfo self) -> int
+%feature("docstring", "
Return the size of pages in this memory region. 0 will be returned
if this information was unavailable."
) lldb::SBMemoryRegionInfo::GetPageSize();
diff --git a/lldb/bindings/interface/SBProcessDocstrings.i
b/lldb/bindings/interface/SBProcessDocstrings.i
index 3ee17e0c7f2fbb..c20ef3e4655bd4 100644
--- a/lldb/bindings/interface/SBProcessDocstrings.i
+++ b/lldb/bindings/interface/SBProcessDocstrings.i
@@ -20,18 +20,18 @@ SBProcess supports thread iteration. For example (from
test/lldbutil.py), ::
"
) lldb::SBProcess;
-%feature("autodoc", "
+%feature("docstring", "
Writes data into the current process's stdin. API client specifies a Python
string as the only argument."
) lldb::SBProcess::PutSTDIN;
-%feature("autodoc", "
+%feature("docstring", "
Reads data from the current process's stdout stream. API client specifies
the size of the buffer to read data into. It returns the byte buffer in a
Python string."
) lldb::SBProcess::GetSTDOUT;
-%feature("autodoc", "
+%feature("docstring", "
Reads data from the current process's stderr stream. API client specifies
the size of the buffer to read data into. It returns the byte buffer in a
Python
[Lldb-commits] [lldb] [lldb] [ObjC runtime] Don't cast to signed when left shifting (PR #86605)
https://github.com/jasonmolenda closed https://github.com/llvm/llvm-project/pull/86605 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Revive shell test after updating UnwindTable (PR #86770)
https://github.com/jasonmolenda closed https://github.com/llvm/llvm-project/pull/86770 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Revive shell test after updating UnwindTable (PR #86770)
https://github.com/jasonmolenda edited https://github.com/llvm/llvm-project/pull/86770 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Revive shell test after updating UnwindTable (PR #86770)
https://github.com/jasonmolenda edited https://github.com/llvm/llvm-project/pull/86770 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Revive shell test after updating UnwindTable (PR #86770)
jasonmolenda wrote: I developed & tested this on aarch64 linux, the only system I have here that runs this shell test. It's a little trickier to test on macOS, we basically only use compact_unwind and it's in the executable binary, not the dSYM SymbolFile. https://github.com/llvm/llvm-project/pull/86770 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Revive shell test after updating UnwindTable (PR #86770)
https://github.com/jasonmolenda created
https://github.com/llvm/llvm-project/pull/86770
In
commit 2f63718f8567413a1c596bda803663eb58d6da5a
Author: Jason Molenda
Date: Tue Mar 26 09:07:15 2024 -0700
[lldb] Don't clear a Module's UnwindTable when adding a SymbolFile (#86603)
I stopped clearing a Module's UnwindTable when we add a SymbolFile to avoid the
memory management problems with adding a symbol file asynchronously while the
UnwindTable is being accessed on another thread. This broke the
target-symbols-add-unwind.test shell test on Linux which removes the DWARF
debub_frame section from a binary, loads it, then loads the unstripped binary
with the DWARF debug_frame section and checks that the UnwindPlans for a
function include debug_frame.
I originally decided that I was willing to sacrifice the possiblity of
additional unwind sources from a symbol file because we rely on assembly
emulation so heavily, they're rarely critical. But there are targets where we
we don't have emluation and rely on things like DWARF debug_frame a lot more,
so this probably wasn't a good choice.
This patch adds a new UnwindTable::Update method which looks for any new
sources of unwind information and adds it to the UnwindTable, and calls that
after a new SymbolFile has been added to a Module.
>From 450f75c9fcc68bc3590afd21e6eb1abb85d987eb Mon Sep 17 00:00:00 2001
From: Jason Molenda
Date: Tue, 26 Mar 2024 22:48:58 -0700
Subject: [PATCH] [lldb] Revive shell test after updating UnwindTable
In
commit 2f63718f8567413a1c596bda803663eb58d6da5a
Author: Jason Molenda
Date: Tue Mar 26 09:07:15 2024 -0700
[lldb] Don't clear a Module's UnwindTable when adding a SymbolFile (#86603)
I stopped clearing a Module's UnwindTable when we add a SymbolFile
to avoid the memory management problems with adding a symbol file
asynchronously while the UnwindTable is being accessed on another
thread. This broke the target-symbols-add-unwind.test shell test
on Linux which removes the DWARF debub_frame section from a binary,
loads it, then loads the unstripped binary with the DWARF debug_frame
section and checks that the UnwindPlans for a function include
debug_frame.
I originally decided that I was willing to sacrifice the possiblity
of additional unwind sources from a symbol file because we rely on
assembly emulation so heavily, they're rarely critical. But there
are targets where we we don't have emluation and rely on things
like DWARF debug_frame a lot more, so this probably wasn't a good
choice.
This patch adds a new UnwindTable::Update method which looks for any
new sources of unwind information and adds it to the UnwindTable,
and calls that after a new SymbolFile has been added to a Module.
---
lldb/include/lldb/Symbol/UnwindTable.h| 4 ++
lldb/source/Core/Module.cpp | 2 +
lldb/source/Symbol/UnwindTable.cpp| 45 +++
.../SymbolFile/target-symbols-add-unwind.test | 27 +++
4 files changed, 78 insertions(+)
create mode 100644 lldb/test/Shell/SymbolFile/target-symbols-add-unwind.test
diff --git a/lldb/include/lldb/Symbol/UnwindTable.h
b/lldb/include/lldb/Symbol/UnwindTable.h
index f0ce7047de2d1e..26826e5d1b497c 100644
--- a/lldb/include/lldb/Symbol/UnwindTable.h
+++ b/lldb/include/lldb/Symbol/UnwindTable.h
@@ -57,6 +57,10 @@ class UnwindTable {
ArchSpec GetArchitecture();
+ /// Called after a SymbolFile has been added to a Module to add any new
+ /// unwind sections that may now be available.
+ void Update();
+
private:
void Dump(Stream );
diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index a520523a96521a..9c105b3f0e57a1 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -1009,6 +1009,8 @@ SymbolFile *Module::GetSymbolFile(bool can_create, Stream
*feedback_strm) {
m_symfile_up.reset(
SymbolVendor::FindPlugin(shared_from_this(), feedback_strm));
m_did_load_symfile = true;
+if (m_unwind_table)
+ m_unwind_table->Update();
}
}
}
diff --git a/lldb/source/Symbol/UnwindTable.cpp
b/lldb/source/Symbol/UnwindTable.cpp
index 3c1a5187b11054..11bedf3d6052e7 100644
--- a/lldb/source/Symbol/UnwindTable.cpp
+++ b/lldb/source/Symbol/UnwindTable.cpp
@@ -84,6 +84,51 @@ void UnwindTable::Initialize() {
}
}
+void UnwindTable::Update() {
+ if (!m_initialized)
+return Initialize();
+
+ std::lock_guard guard(m_mutex);
+
+ ObjectFile *object_file = m_module.GetObjectFile();
+ if (!object_file)
+return;
+
+ if (!m_object_file_unwind_up)
+m_object_file_unwind_up = object_file->CreateCallFrameInfo();
+
+ SectionList *sl = m_module.GetSectionList();
+ if (!sl)
+return;
+
+ SectionSP sect = sl->FindSectionByType(eSectionTypeEHFrame, true);
+ if (!m_eh_frame_up && sect) {
+m_eh_frame_up = std::make_unique(
+*object_file, sect, DWARFCallFrameInfo::EH);
+ }
+
+ sect =
[Lldb-commits] [lldb] 29318ab - [lldb] Remove test for add-symbol-file adds unwind source
Author: Jason Molenda
Date: 2024-03-26T10:54:26-07:00
New Revision: 29318abe1d2c55e8543255d70f26ac93261b74a4
URL:
https://github.com/llvm/llvm-project/commit/29318abe1d2c55e8543255d70f26ac93261b74a4
DIFF:
https://github.com/llvm/llvm-project/commit/29318abe1d2c55e8543255d70f26ac93261b74a4.diff
LOG: [lldb] Remove test for add-symbol-file adds unwind source
In
commit 2f63718f8567413a1c596bda803663eb58d6da5a
Author: Jason Molenda
Date: Tue Mar 26 09:07:15 2024 -0700
[lldb] Don't clear a Module's UnwindTable when adding a SymbolFile (#86603)
I changed lldb to not clear a Module's UnwindTable when we add a
SymbolFile to a binary, because the added benefit is marginal, and
handling this reconstruction correctly is difficult. This test was
written to explicitly create a test without unwind info in the
binary, then add a symbol file with the unwind info, and check that
it is present. I've intentionally broken this, so I'm removing the
test.
Added:
Modified:
Removed:
lldb/test/Shell/SymbolFile/target-symbols-add-unwind.test
diff --git a/lldb/test/Shell/SymbolFile/target-symbols-add-unwind.test
b/lldb/test/Shell/SymbolFile/target-symbols-add-unwind.test
deleted file mode 100644
index 5420213d405e86..00
--- a/lldb/test/Shell/SymbolFile/target-symbols-add-unwind.test
+++ /dev/null
@@ -1,27 +0,0 @@
-# TODO: When it's possible to run "image show-unwind" without a running
-# process, we can remove the unsupported line below, and hard-code an ELF
-# triple in the test.
-# UNSUPPORTED: system-windows, system-darwin
-
-# RUN: cd %T
-# RUN: %clang_host %S/Inputs/target-symbols-add-unwind.c -g \
-# RUN: -fno-unwind-tables -fno-asynchronous-unwind-tables \
-# RUN: -o target-symbols-add-unwind.debug
-# RUN: llvm-objcopy --strip-debug target-symbols-add-unwind.debug \
-# RUN: target-symbols-add-unwind.stripped
-# RUN: %lldb target-symbols-add-unwind.stripped -s %s -o quit | FileCheck %s
-
-process launch --stop-at-entry
-image show-unwind -n main
-# CHECK-LABEL: image show-unwind -n main
-# CHECK-NOT: debug_frame UnwindPlan:
-
-target symbols add -s target-symbols-add-unwind.stripped
target-symbols-add-unwind.debug
-# CHECK-LABEL: target symbols add
-# CHECK: symbol file {{.*}} has been added to {{.*}}
-
-image show-unwind -n main
-# CHECK-LABEL: image show-unwind -n main
-# CHECK: debug_frame UnwindPlan:
-# CHECK-NEXT: This UnwindPlan originally sourced from DWARF CFI
-# CHECK-NEXT: This UnwindPlan is sourced from the compiler: yes.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [ObjC runtime] Don't cast to signed when left shifting (PR #86605)
https://github.com/jasonmolenda edited https://github.com/llvm/llvm-project/pull/86605 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [ObjC runtime] Don't cast to signed when left shifting (PR #86605)
@@ -3154,7 +3154,7 @@ AppleObjCRuntimeV2::TaggedPointerVendorExtended::GetClassDescriptor( << m_objc_debug_taggedpointer_ext_payload_lshift) >> m_objc_debug_taggedpointer_ext_payload_rshift); int64_t data_payload_signed = - ((int64_t)((int64_t)unobfuscated + ((int64_t)((uint64_t)unobfuscated << m_objc_debug_taggedpointer_ext_payload_lshift) >> jasonmolenda wrote: e.g. ``` (lldb) p (-5LL <<60) >> 60 (long long) -5 ``` if the bit slice we're pulling out has its high bit set, that'll shfit down and be sign extended. https://github.com/llvm/llvm-project/pull/86605 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [ObjC runtime] Don't cast to signed when left shifting (PR #86605)
@@ -3154,7 +3154,7 @@ AppleObjCRuntimeV2::TaggedPointerVendorExtended::GetClassDescriptor( << m_objc_debug_taggedpointer_ext_payload_lshift) >> m_objc_debug_taggedpointer_ext_payload_rshift); int64_t data_payload_signed = - ((int64_t)((int64_t)unobfuscated + ((int64_t)((uint64_t)unobfuscated << m_objc_debug_taggedpointer_ext_payload_lshift) >> jasonmolenda wrote: Probably the one interesting thing is that the value we're slicing out is, apparently, signed, so we want it to sign extend to Int64 when it is done. That would take a little more care with a bit slice approach. https://github.com/llvm/llvm-project/pull/86605 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [ObjC runtime] Don't cast to signed when left shifting (PR #86605)
@@ -3154,7 +3154,7 @@ AppleObjCRuntimeV2::TaggedPointerVendorExtended::GetClassDescriptor( << m_objc_debug_taggedpointer_ext_payload_lshift) >> m_objc_debug_taggedpointer_ext_payload_rshift); int64_t data_payload_signed = - ((int64_t)((int64_t)unobfuscated + ((int64_t)((uint64_t)unobfuscated << m_objc_debug_taggedpointer_ext_payload_lshift) >> jasonmolenda wrote: I suspect the runtime gave us these "left & right shift" values, and the person who wrote this used the obvious implementation. It could be expressed as a bit slice with a little subtraction, true. Just to be clear, we're not fixing a real bug here, we were saying a UInt64 was signed and then ubsan got all shirty when we shifted away some bits that would indicate sign. https://github.com/llvm/llvm-project/pull/86605 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Don't clear a Module's UnwindTable when adding a SymbolFile (PR #86603)
https://github.com/jasonmolenda closed https://github.com/llvm/llvm-project/pull/86603 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Don't clear a Module's UnwindTable when adding a SymbolFile (PR #86603)
https://github.com/jasonmolenda updated
https://github.com/llvm/llvm-project/pull/86603
>From b6fcac7d6bb48b7fb665034712407c9ad7e4053a Mon Sep 17 00:00:00 2001
From: Jason Molenda
Date: Mon, 25 Mar 2024 16:47:11 -0700
Subject: [PATCH] [lldb] Don't clear a Module's UnwindTable when adding a
SymbolFile
Fixing a crash in lldb when `symbols.auto-download` setting is
enabled. When doing a backtrace, this feature has lldb search
for a SymbolFile for stack frames when we are backtracing, and
add them either synchoronously or asynchronously, depending
on the specific setting used.
Module::SetSymbolFileFileSpec clears the Module's UnwindTable, once
we find a new SymbolFile. We may be adding a source of unwind
information that we did not have when lldb was working only with
the executable binary.
What happens in practice is that we're using a reference to the
Module's UnwindTable, and then the other thread getting the SymbolFile
clears it and now the first thread is referring to freed memory
and we can crash. When built with address sanitizer, it crashes
much more reliably.
Given that unwind information used for exception handling -- eh_frame,
compact unwind -- is present in executable binaries, the only thing
we're likely to *add* would be DWARF's `debug_frame` if that was
also available. The actual value of re-creating the UnwindTable
when we have added a SymbolFile is not large.
I also tried fixing this by changing the Module to have a shared_ptr
to the UnwindTable, so we could have two different UnwindTable's
in use simultaneously for a brief period. This would be fine TODAY,
but it introduces a very subtle bug that someone will have a heck
of a time figuring out in the future.
In the end, I believe the safest approach is to sacrifice the possible
marginal gain of reconstructing the UnwindTable once a SymbolFile has
been added, to sidestep this whole problem area.
---
lldb/source/Core/Module.cpp | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index 8ffa35518b3c36..a520523a96521a 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -1239,9 +1239,9 @@ void Module::SectionFileAddressesChanged() {
UnwindTable ::GetUnwindTable() {
if (!m_unwind_table) {
-m_unwind_table.emplace(*this);
if (!m_symfile_spec)
SymbolLocator::DownloadSymbolFileAsync(GetUUID());
+m_unwind_table.emplace(*this);
}
return *m_unwind_table;
}
@@ -1359,15 +1359,10 @@ void Module::SetSymbolFileFileSpec(const FileSpec
) {
// one
obj_file->ClearSymtab();
-// Clear the unwind table too, as that may also be affected by the
-// symbol file information.
-m_unwind_table.reset();
-
// The symbol file might be a directory bundle ("/tmp/a.out.dSYM")
// instead of a full path to the symbol file within the bundle
// ("/tmp/a.out.dSYM/Contents/Resources/DWARF/a.out"). So we need to
// check this
-
if (FileSystem::Instance().IsDirectory(file)) {
std::string new_path(file.GetPath());
std::string old_path(obj_file->GetFileSpec().GetPath());
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [ObjC runtime] Don't cast to signed when left shifting (PR #86605)
jasonmolenda wrote: I've seen this ubsan error every time I run the testsuite for years, i just finally sat down and figured out what was going on. https://github.com/llvm/llvm-project/pull/86605 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [ObjectFileMachO] LLVM_COV is not mapped into firmware memory (PR #86359)
https://github.com/jasonmolenda closed https://github.com/llvm/llvm-project/pull/86359 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [ObjectFileMachO] LLVM_COV is not mapped into firmware memory (PR #86359)
https://github.com/jasonmolenda updated
https://github.com/llvm/llvm-project/pull/86359
>From 47bf1259289986cdc0df706aa40a18e58e94eee5 Mon Sep 17 00:00:00 2001
From: Jason Molenda
Date: Fri, 22 Mar 2024 16:31:07 -0700
Subject: [PATCH] [lldb] [ObjectFileMachO] LLVM_COV is not mapped into firmware
memory
It is possible to gather code coverage in a firmware environment,
where the __LLVM_COV segment will not be mapped in memory but does
exist in the binary, see
https://llvm.org/devmtg/2020-09/slides/PhippsAlan_EmbeddedCodeCoverage_LLVM_Conf_Talk_final.pdf
The __LLVM_COV segment in the binary happens to be at the same
address as the __DATA segment, so if lldb treats this segment as
loaded, it shadows the __DATA segment and address->symbol resolution
can fail.
For these non-userland code cases, we need to mark __LLVM_COV as
not a loadable segment.
rdar://124475661
---
.../Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp| 12
.../Plugins/ObjectFile/Mach-O/ObjectFileMachO.h | 1 +
2 files changed, 13 insertions(+)
diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index bcf3a3274cf3a0..1caf93659956b4 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -905,6 +905,11 @@ ConstString ObjectFileMachO::GetSegmentNameDWARF() {
return g_section_name;
}
+ConstString ObjectFileMachO::GetSegmentNameLLVM_COV() {
+ static ConstString g_section_name("__LLVM_COV");
+ return g_section_name;
+}
+
ConstString ObjectFileMachO::GetSectionNameEHFrame() {
static ConstString g_section_name_eh_frame("__eh_frame");
return g_section_name_eh_frame;
@@ -6145,6 +6150,13 @@ bool ObjectFileMachO::SectionIsLoadable(const Section
*section) {
return false;
if (GetModule().get() != section->GetModule().get())
return false;
+ // firmware style binaries with llvm gcov segment do
+ // not have that segment mapped into memory.
+ if (section->GetName() == GetSegmentNameLLVM_COV()) {
+const Strata strata = GetStrata();
+if (strata == eStrataKernel || strata == eStrataRawImage)
+ return false;
+ }
// Be careful with __LINKEDIT and __DWARF segments
if (section->GetName() == GetSegmentNameLINKEDIT() ||
section->GetName() == GetSegmentNameDWARF()) {
diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
index 0a47f3a7dd1861..55bc688126eb36 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
@@ -271,6 +271,7 @@ class ObjectFileMachO : public lldb_private::ObjectFile {
static lldb_private::ConstString GetSegmentNameOBJC();
static lldb_private::ConstString GetSegmentNameLINKEDIT();
static lldb_private::ConstString GetSegmentNameDWARF();
+ static lldb_private::ConstString GetSegmentNameLLVM_COV();
static lldb_private::ConstString GetSectionNameEHFrame();
llvm::MachO::dysymtab_command m_dysymtab;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [ObjectFileMachO] LLVM_COV is not mapped into firmware memory (PR #86359)
https://github.com/jasonmolenda created
https://github.com/llvm/llvm-project/pull/86359
It is possible to gather code coverage in a firmware environment, where the
__LLVM_COV segment will not be mapped in memory but does exist in the binary,
see
https://llvm.org/devmtg/2020-09/slides/PhippsAlan_EmbeddedCodeCoverage_LLVM_Conf_Talk_final.pdf
The __LLVM_COV segment in the binary happens to be at the same address as the
__DATA segment, so if lldb treats this segment as loaded, it shadows the __DATA
segment and address->symbol resolution can fail.
For these non-userland code cases, we need to mark __LLVM_COV as not a loadable
segment.
rdar://124475661
>From 47bf1259289986cdc0df706aa40a18e58e94eee5 Mon Sep 17 00:00:00 2001
From: Jason Molenda
Date: Fri, 22 Mar 2024 16:31:07 -0700
Subject: [PATCH] [lldb] [ObjectFileMachO] LLVM_COV is not mapped into firmware
memory
It is possible to gather code coverage in a firmware environment,
where the __LLVM_COV segment will not be mapped in memory but does
exist in the binary, see
https://llvm.org/devmtg/2020-09/slides/PhippsAlan_EmbeddedCodeCoverage_LLVM_Conf_Talk_final.pdf
The __LLVM_COV segment in the binary happens to be at the same
address as the __DATA segment, so if lldb treats this segment as
loaded, it shadows the __DATA segment and address->symbol resolution
can fail.
For these non-userland code cases, we need to mark __LLVM_COV as
not a loadable segment.
rdar://124475661
---
.../Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp| 12
.../Plugins/ObjectFile/Mach-O/ObjectFileMachO.h | 1 +
2 files changed, 13 insertions(+)
diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index bcf3a3274cf3a0..1caf93659956b4 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -905,6 +905,11 @@ ConstString ObjectFileMachO::GetSegmentNameDWARF() {
return g_section_name;
}
+ConstString ObjectFileMachO::GetSegmentNameLLVM_COV() {
+ static ConstString g_section_name("__LLVM_COV");
+ return g_section_name;
+}
+
ConstString ObjectFileMachO::GetSectionNameEHFrame() {
static ConstString g_section_name_eh_frame("__eh_frame");
return g_section_name_eh_frame;
@@ -6145,6 +6150,13 @@ bool ObjectFileMachO::SectionIsLoadable(const Section
*section) {
return false;
if (GetModule().get() != section->GetModule().get())
return false;
+ // firmware style binaries with llvm gcov segment do
+ // not have that segment mapped into memory.
+ if (section->GetName() == GetSegmentNameLLVM_COV()) {
+const Strata strata = GetStrata();
+if (strata == eStrataKernel || strata == eStrataRawImage)
+ return false;
+ }
// Be careful with __LINKEDIT and __DWARF segments
if (section->GetName() == GetSegmentNameLINKEDIT() ||
section->GetName() == GetSegmentNameDWARF()) {
diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
index 0a47f3a7dd1861..55bc688126eb36 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
@@ -271,6 +271,7 @@ class ObjectFileMachO : public lldb_private::ObjectFile {
static lldb_private::ConstString GetSegmentNameOBJC();
static lldb_private::ConstString GetSegmentNameLINKEDIT();
static lldb_private::ConstString GetSegmentNameDWARF();
+ static lldb_private::ConstString GetSegmentNameLLVM_COV();
static lldb_private::ConstString GetSectionNameEHFrame();
llvm::MachO::dysymtab_command m_dysymtab;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Invert relationship between Process and AddressableBits (PR #85858)
https://github.com/jasonmolenda approved this pull request. Thanks for fixing this, I hadn't been paying attention to the separation when I added this method. https://github.com/llvm/llvm-project/pull/85858 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [Mach-O] ProcessMachCore needs to strip TBI data from addrs (PR #84998)
https://github.com/jasonmolenda closed https://github.com/llvm/llvm-project/pull/84998 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [Mach-O] ProcessMachCore needs to strip TBI data from addrs (PR #84998)
jasonmolenda wrote: Thanks for all the helpful comments as always, David. I updated the test case to fix the issues you identified and added comments that I think make it clearer that it is testing both a live process and a corefile. https://github.com/llvm/llvm-project/pull/84998 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [Mach-O] ProcessMachCore needs to strip TBI data from addrs (PR #84998)
https://github.com/jasonmolenda updated
https://github.com/llvm/llvm-project/pull/84998
>From 4278537c262b01b1d6432391bd9d8017eb96c60a Mon Sep 17 00:00:00 2001
From: Jason Molenda
Date: Tue, 12 Mar 2024 17:09:30 -0700
Subject: [PATCH 1/2] [lldb] [Mach-O] ProcessMachCore needs to strip TBI data
from addrs
Darwin AArch64 application processors are run with Top Byte Ignore
mode enabled so metadata may be stored in the top byte, it needs
to be ignored when reading/writing memory. David Spickett handled
this already in the base class Process::ReadMemory but ProcessMachCore
overrides that method (to avoid the memory cache) and did not pick
up the same change. I add a test case that creates a pointer with
metadata in the top byte and dereferences it with a live process and
with a corefile.
rdar://123784501
---
.../Process/mach-core/ProcessMachCore.cpp | 2 +-
lldb/test/API/macosx/tbi-honored/Makefile | 3 ++
.../API/macosx/tbi-honored/TestTBIHonored.py | 49 +++
lldb/test/API/macosx/tbi-honored/main.c | 13 +
4 files changed, 66 insertions(+), 1 deletion(-)
create mode 100644 lldb/test/API/macosx/tbi-honored/Makefile
create mode 100644 lldb/test/API/macosx/tbi-honored/TestTBIHonored.py
create mode 100644 lldb/test/API/macosx/tbi-honored/main.c
diff --git a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
index 3961dcf0fbcc0e..7b9938d4f02020 100644
--- a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
+++ b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
@@ -652,7 +652,7 @@ size_t ProcessMachCore::ReadMemory(addr_t addr, void *buf,
size_t size,
Status ) {
// Don't allow the caching that lldb_private::Process::ReadMemory does since
// in core files we have it all cached our our core file anyway.
- return DoReadMemory(addr, buf, size, error);
+ return DoReadMemory(FixAnyAddress(addr), buf, size, error);
}
size_t ProcessMachCore::DoReadMemory(addr_t addr, void *buf, size_t size,
diff --git a/lldb/test/API/macosx/tbi-honored/Makefile
b/lldb/test/API/macosx/tbi-honored/Makefile
new file mode 100644
index 00..10495940055b63
--- /dev/null
+++ b/lldb/test/API/macosx/tbi-honored/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules
diff --git a/lldb/test/API/macosx/tbi-honored/TestTBIHonored.py
b/lldb/test/API/macosx/tbi-honored/TestTBIHonored.py
new file mode 100644
index 00..d38685359af6d1
--- /dev/null
+++ b/lldb/test/API/macosx/tbi-honored/TestTBIHonored.py
@@ -0,0 +1,49 @@
+"""Test that lldb on Darwin ignores metadata in the top byte of addresses."""
+
+import os
+import re
+import subprocess
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestTBIHonored(TestBase):
+@no_debug_info_test
+@skipUnlessDarwin
+@skipIf(archs=no_match(["arm64", "arm64e"]))
+@skipIfRemote
+def do_variable_access_tests(self, frame):
+self.assertEqual(
+frame.variables["pb"][0]
+.GetChildMemberWithName("p")
+.Dereference()
+.GetValueAsUnsigned(),
+15,
+)
+addr =
frame.variables["pb"][0].GetChildMemberWithName("p").GetValueAsUnsigned()
+self.expect("expr -- *pb.p", substrs=["15"])
+self.expect("frame variable *pb.p", substrs=["15"])
+self.expect("expr -- *(int*)0x%x" % addr, substrs=["15"])
+
+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.do_variable_access_tests(thread.GetFrameAtIndex(0))
+
+self.runCmd("process save-core -s stack " + corefile)
+self.dbg.DeleteTarget(target)
+
+# Now load the corefile
+target = self.dbg.CreateTarget("")
+process = target.LoadCore(corefile)
+thread = process.GetSelectedThread()
+self.assertTrue(process.GetSelectedThread().IsValid())
+
+self.do_variable_access_tests(thread.GetFrameAtIndex(0))
diff --git a/lldb/test/API/macosx/tbi-honored/main.c
b/lldb/test/API/macosx/tbi-honored/main.c
new file mode 100644
index 00..3d7ad0b04cd664
--- /dev/null
+++ b/lldb/test/API/macosx/tbi-honored/main.c
@@ -0,0 +1,13 @@
+#include
+#include
+union ptrbytes {
+ int *p;
+ uint8_t bytes[8];
+};
+int main() {
+ int c = 15;
+ union ptrbytes pb;
+ pb.p =
+ pb.bytes[7] = 0xfe;
+ printf("%d\n", *pb.p); // break here
+}
>From 145e4161a311956ee15c4631667f332d50ef4e0b Mon Sep 17 00:00:00 2001
From: Jason Molenda
Date: Wed, 13 Mar 2024 15:40:08 -0700
Subject: [PATCH 2/2] Address David Spickett's feedback on the testcase
Lots of fixes to the test case for correctness and
to make it
[Lldb-commits] [lldb] e2468bf - [lldb][debugserver] Update flags past to app launch request
Author: Jason Molenda Date: 2024-03-12T17:19:46-07:00 New Revision: e2468bf16a0c1f63a39aa417c15c03ebd77fab9e URL: https://github.com/llvm/llvm-project/commit/e2468bf16a0c1f63a39aa417c15c03ebd77fab9e DIFF: https://github.com/llvm/llvm-project/commit/e2468bf16a0c1f63a39aa417c15c03ebd77fab9e.diff LOG: [lldb][debugserver] Update flags past to app launch request rdar://117421999 Added: Modified: lldb/tools/debugserver/source/MacOSX/MachProcess.mm Removed: diff --git a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm b/lldb/tools/debugserver/source/MacOSX/MachProcess.mm index 87bdbf835bfd10..70b4564a027b1b 100644 --- a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm +++ b/lldb/tools/debugserver/source/MacOSX/MachProcess.mm @@ -472,6 +472,8 @@ static bool FBSAddEventDataToOptions(NSMutableDictionary *options, // And there are some other options at the top level in this dictionary: [options setObject:[NSNumber numberWithBool:YES] forKey:FBSOpenApplicationOptionKeyUnlockDevice]; + [options setObject:[NSNumber numberWithBool:YES] + forKey:FBSOpenApplicationOptionKeyPromptUnlockDevice]; // We have to get the "sequence ID & UUID" for this app bundle path and send // them to FBS: ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [Mach-O] ProcessMachCore needs to strip TBI data from addrs (PR #84998)
https://github.com/jasonmolenda created
https://github.com/llvm/llvm-project/pull/84998
Darwin AArch64 application processors are run with Top Byte Ignore mode enabled
so metadata may be stored in the top byte, it needs to be ignored when
reading/writing memory. David Spickett handled this already in the base class
Process::ReadMemory but ProcessMachCore overrides that method (to avoid the
memory cache) and did not pick up the same change. I add a test case that
creates a pointer with metadata in the top byte and dereferences it with a live
process and with a corefile.
rdar://123784501
>From 4278537c262b01b1d6432391bd9d8017eb96c60a Mon Sep 17 00:00:00 2001
From: Jason Molenda
Date: Tue, 12 Mar 2024 17:09:30 -0700
Subject: [PATCH] [lldb] [Mach-O] ProcessMachCore needs to strip TBI data from
addrs
Darwin AArch64 application processors are run with Top Byte Ignore
mode enabled so metadata may be stored in the top byte, it needs
to be ignored when reading/writing memory. David Spickett handled
this already in the base class Process::ReadMemory but ProcessMachCore
overrides that method (to avoid the memory cache) and did not pick
up the same change. I add a test case that creates a pointer with
metadata in the top byte and dereferences it with a live process and
with a corefile.
rdar://123784501
---
.../Process/mach-core/ProcessMachCore.cpp | 2 +-
lldb/test/API/macosx/tbi-honored/Makefile | 3 ++
.../API/macosx/tbi-honored/TestTBIHonored.py | 49 +++
lldb/test/API/macosx/tbi-honored/main.c | 13 +
4 files changed, 66 insertions(+), 1 deletion(-)
create mode 100644 lldb/test/API/macosx/tbi-honored/Makefile
create mode 100644 lldb/test/API/macosx/tbi-honored/TestTBIHonored.py
create mode 100644 lldb/test/API/macosx/tbi-honored/main.c
diff --git a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
index 3961dcf0fbcc0e..7b9938d4f02020 100644
--- a/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
+++ b/lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
@@ -652,7 +652,7 @@ size_t ProcessMachCore::ReadMemory(addr_t addr, void *buf,
size_t size,
Status ) {
// Don't allow the caching that lldb_private::Process::ReadMemory does since
// in core files we have it all cached our our core file anyway.
- return DoReadMemory(addr, buf, size, error);
+ return DoReadMemory(FixAnyAddress(addr), buf, size, error);
}
size_t ProcessMachCore::DoReadMemory(addr_t addr, void *buf, size_t size,
diff --git a/lldb/test/API/macosx/tbi-honored/Makefile
b/lldb/test/API/macosx/tbi-honored/Makefile
new file mode 100644
index 00..10495940055b63
--- /dev/null
+++ b/lldb/test/API/macosx/tbi-honored/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules
diff --git a/lldb/test/API/macosx/tbi-honored/TestTBIHonored.py
b/lldb/test/API/macosx/tbi-honored/TestTBIHonored.py
new file mode 100644
index 00..d38685359af6d1
--- /dev/null
+++ b/lldb/test/API/macosx/tbi-honored/TestTBIHonored.py
@@ -0,0 +1,49 @@
+"""Test that lldb on Darwin ignores metadata in the top byte of addresses."""
+
+import os
+import re
+import subprocess
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestTBIHonored(TestBase):
+@no_debug_info_test
+@skipUnlessDarwin
+@skipIf(archs=no_match(["arm64", "arm64e"]))
+@skipIfRemote
+def do_variable_access_tests(self, frame):
+self.assertEqual(
+frame.variables["pb"][0]
+.GetChildMemberWithName("p")
+.Dereference()
+.GetValueAsUnsigned(),
+15,
+)
+addr =
frame.variables["pb"][0].GetChildMemberWithName("p").GetValueAsUnsigned()
+self.expect("expr -- *pb.p", substrs=["15"])
+self.expect("frame variable *pb.p", substrs=["15"])
+self.expect("expr -- *(int*)0x%x" % addr, substrs=["15"])
+
+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.do_variable_access_tests(thread.GetFrameAtIndex(0))
+
+self.runCmd("process save-core -s stack " + corefile)
+self.dbg.DeleteTarget(target)
+
+# Now load the corefile
+target = self.dbg.CreateTarget("")
+process = target.LoadCore(corefile)
+thread = process.GetSelectedThread()
+self.assertTrue(process.GetSelectedThread().IsValid())
+
+self.do_variable_access_tests(thread.GetFrameAtIndex(0))
diff --git a/lldb/test/API/macosx/tbi-honored/main.c
b/lldb/test/API/macosx/tbi-honored/main.c
new file mode 100644
index 00..3d7ad0b04cd664
--- /dev/null
+++
[Lldb-commits] [lldb] [lldb] [debugserver] Handle interrupted reads correctly (PR #84872)
https://github.com/jasonmolenda closed https://github.com/llvm/llvm-project/pull/84872 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [debugserver] Handle interrupted reads correctly (PR #84872)
https://github.com/jasonmolenda created
https://github.com/llvm/llvm-project/pull/84872
The first half of this patch is a long-standing annoyance, if I attach to
debugserver with lldb while it is waiting for an lldb connection, the syscall
is interrupted and it doesn't retry, debugserver exits immediately.
The second half is a request from another tool that is communicating with
debugserver, that we retry reads on our sockets in the same way. I haven't dug
in to the details of how they're communicating that this is necessary, but the
best I've been able to find reading the POSIX API docs, this is fine.
rdar://117113298
>From b2ed63b985c2509594ef647f8258a0854ff404a7 Mon Sep 17 00:00:00 2001
From: Jason Molenda
Date: Mon, 11 Mar 2024 22:19:49 -0700
Subject: [PATCH] [lldb] [debugserver] Handle interrupted reads correctly
The first half of this patch is a long-standing annoyance,
if I attach to debugserver with lldb while it is waiting
for an lldb connection, the syscall is interrupted and it
doesn't retry, debugserver exits immediately.
The second half is a request from another tool that is
communicating with debugserver, that we retry reads on
our sockets in the same way. I haven't dug in to the
details of how they're communicating that this is necessary,
but the best I've been able to find reading the POSIX API
docs, this is fine.
rdar://117113298
---
lldb/tools/debugserver/source/RNBSocket.cpp | 16 +---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/lldb/tools/debugserver/source/RNBSocket.cpp
b/lldb/tools/debugserver/source/RNBSocket.cpp
index 1282ea221625a0..fc55dbf2e1f1b0 100644
--- a/lldb/tools/debugserver/source/RNBSocket.cpp
+++ b/lldb/tools/debugserver/source/RNBSocket.cpp
@@ -120,8 +120,13 @@ rnb_err_t RNBSocket::Listen(const char *listen_host,
uint16_t port,
while (!accept_connection) {
struct kevent event_list[4];
-int num_events =
-kevent(queue_id, events.data(), events.size(), event_list, 4, NULL);
+int num_events;
+do {
+ errno = 0;
+ num_events =
+ kevent(queue_id, events.data(), events.size(), event_list, 4, NULL);
+} while (num_events == -1 &&
+ (errno == EAGAIN || errno == EWOULDBLOCK || errno == EINTR));
if (num_events < 0) {
err.SetError(errno, DNBError::MachKernel);
@@ -291,7 +296,12 @@ rnb_err_t RNBSocket::Read(std::string ) {
// DNBLogThreadedIf(LOG_RNB_COMM, "%8u RNBSocket::%s calling read()",
// (uint32_t)m_timer.ElapsedMicroSeconds(true), __FUNCTION__);
DNBError err;
- ssize_t bytesread = read(m_fd, buf, sizeof(buf));
+ ssize_t bytesread;
+ do {
+errno = 0;
+bytesread = read(m_fd, buf, sizeof(buf));
+ } while (bytesread == -1 &&
+ (errno == EAGAIN || errno == EWOULDBLOCK || errno == EINTR));
if (bytesread <= 0)
err.SetError(errno, DNBError::POSIX);
else
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Turn off instruction flow control annotations by default (PR #84607)
https://github.com/jasonmolenda closed https://github.com/llvm/llvm-project/pull/84607 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][doc] Updates build instructions. (PR #84630)
https://github.com/jasonmolenda approved this pull request. Looks good. I see the PR test for "Test documentation build" is failing, but that's an issue with the bot and pexpect. https://github.com/llvm/llvm-project/pull/84630 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Turn off instruction flow control annotations by default (PR #84607)
https://github.com/jasonmolenda created https://github.com/llvm/llvm-project/pull/84607 Walter Erquinigo added optional instruction annotations for x86 instructions in 2022 for the `thread trace dump instruction` command, and code to DisassemblerLLVMC to add annotations for instructions that change flow control, v. https://reviews.llvm.org/D128477 This was added as an option to `disassemble`, and the trace dump command enables it by default, but several other instruction dumpers were changed to display them by default as well. These are only implemented for Intel instructions, so our disassembly on other targets ends up looking like ``` (lldb) x/5i 0x186e4 0x186e4: 0xa9be6ffc unknown stpx28, x27, [sp, #-0x20]! 0x186e8: 0xa9017bfd unknown stpx29, x30, [sp, #0x10] 0x186ec: 0x910043fd unknown addx29, sp, #0x10 0x186f0: 0xd11843ff unknown subsp, sp, #0x610 0x186f4: 0x910c63e8 unknown addx8, sp, #0x318 ``` instead of `disassemble`'s output style of ``` lldb`main: lldb[0x186e4] <+0>: stpx28, x27, [sp, #-0x20]! lldb[0x186e8] <+4>: stpx29, x30, [sp, #0x10] lldb[0x186ec] <+8>: addx29, sp, #0x10 lldb[0x186f0] <+12>: subsp, sp, #0x610 lldb[0x186f4] <+16>: addx8, sp, #0x318 ``` Adding symbolic annotations for assembly instructions is something I'm interested in too, because we may have users investigating a crash or apparent-incorrect behavior who must debug optimized assembly and they may not be familiar with the ISA they're using, so short of flipping through a many-thousand-page PDF to understand each instruction, they're lost. They don't write assembly or work at that level, but to understand a bug, they have to understand what the instructions are actually doing. But the annotations that exist today don't move us forward much on that front - I'd argue that the flow control instructions on Intel are not hard to understand from their names, but that might just be my personal bias. Much trickier instructions exist in any event. Displaying this information by default for all targets when we only have one class of instructions on one target is not a good default. Also, in 2011 when Greg implemented the `memory read -f i` (aka `x/i`) command ``` commit 5009f9d5010a7e34ae15f962dac8505ea11a8716 Author: Greg Clayton Date: Thu Oct 27 17:55:14 2011 + [...] eFormatInstruction will print out disassembly with bytes and it will use the current target's architecture. The format character for this is "i" (which used to be being used for the integer format, but the integer format also has "d", so we gave the "i" format to disassembly), the long format is "instruction". ``` he had DumpDataExtractor's DumpInstructions print the bytes of the instruction -- that's the first field we see above for the `x/5i` after the address -- and this is only useful for people who are debugging the disassembler itself, I would argue. I don't want this displayed by default either. tl;dr this patch removes both fields from `memory read -f -i` and I think this is the right call today. While I'm really interested in instruction annotation, I don't think `x/i` is the right place to have it enabled by default unless it's really compelling on at least some of our major targets. >From 3969aaf70b22e2551e5d30a87cc80fa45ca83080 Mon Sep 17 00:00:00 2001 From: Jason Molenda Date: Fri, 8 Mar 2024 21:50:28 -0800 Subject: [PATCH] Turn off instruction flow control annotations by default Walter Erquinigo added optional instruction annotations for x86 instructions in 2022 for the `thread trace dump instruction` command, and code to DisassemblerLLVMC to add annotations for instructions that change flow control, v. https://reviews.llvm.org/D128477 This was added as an option to `disassemble`, and the trace dump command enables it by default, but several other instruction dumpers were changed to display them by default as well. These are only implemented for Intel instructions, so our disassembly on other targets ends up looking like ``` (lldb) x/5i 0x186e4 0x186e4: 0xa9be6ffc unknown stpx28, x27, [sp, #-0x20]! 0x186e8: 0xa9017bfd unknown stpx29, x30, [sp, #0x10] 0x186ec: 0x910043fd unknown addx29, sp, #0x10 0x186f0: 0xd11843ff unknown subsp, sp, #0x610 0x186f4: 0x910c63e8 unknown addx8, sp, #0x318 ``` instead of `disassemble`'s output style of ``` lldb`main: lldb[0x186e4] <+0>: stpx28, x27, [sp, #-0x20]! lldb[0x186e8] <+4>: stpx29, x30, [sp, #0x10] lldb[0x186ec] <+8>: addx29, sp, #0x10 lldb[0x186f0] <+12>: subsp, sp, #0x610 lldb[0x186f4] <+16>: addx8, sp, #0x318 ``` Adding symbolic annotations for assembly instructions is something I'm interested in too, because we may have users investigating a crash or apparent-incorrect behavior who must debug optimized
[Lldb-commits] [lldb] [lldb] Address mask sbprocess apis and new mask invalid const (PR #83663)
jasonmolenda wrote: nb: this got a bot failure on the arm-linux-ubuntu bot, ``` mask = process.GetAddressMask(lldb.eAddressMaskTypeAny) process.SetAddressMask(lldb.eAddressMaskTypeCode, mask | 0x3) self.assertEqual( 0x02950001F694, process.FixAddress(0x00265E950001F697, lldb.eAddressMaskTypeCode), ) The API returned 0x02950001f694 instead of the expected 0x00265e950001f696. The low bits differ because ABISysV_arm hardcodes the Code address mask to clear the 0th bit, it doesn't use the Process code mask. I didn't debug why some of the high bytes were dropped. ``` I'm skipping TestAddressMasks for arch=arm now, but I don't even know if we should be able to set a mask which preserves the high 32-bits of an addr_t on a 32-bit host; all masks have to mask off the high word of addr_t to be a valid addresss, at a minimum. I don't have access to a 32-bit host that lldb runs on myself so my position is that I'll skip this API test entirely on 32-bit targets, I don't think the concepts make much sense there. https://github.com/llvm/llvm-project/pull/83663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 04bbbba - Skip TestAddressMasks API tests on 32-bit arm
Author: Jason Molenda
Date: 2024-03-06T10:58:03-08:00
New Revision: 04a271ebe4c2421f34a4fbf34c328df9f111
URL:
https://github.com/llvm/llvm-project/commit/04a271ebe4c2421f34a4fbf34c328df9f111
DIFF:
https://github.com/llvm/llvm-project/commit/04a271ebe4c2421f34a4fbf34c328df9f111.diff
LOG: Skip TestAddressMasks API tests on 32-bit arm
TestAddressMasks failed on the lldb-arm-buntu bot with the
Code address mask test,
mask = process.GetAddressMask(lldb.eAddressMaskTypeAny)
process.SetAddressMask(lldb.eAddressMaskTypeCode, mask | 0x3)
self.assertEqual(
0x02950001F694,
process.FixAddress(0x00265E950001F697, lldb.eAddressMaskTypeCode),
)
The API returned 0x02950001f694 instead of the expected
0x00265e950001f696. The low bits differ because ABISysV_arm hardcodes
the Code address mask to clear the 0th bit, it doesn't use the
Process code mask. I didn't debug why some of the high bytes were
dropped. The address mask APIs are only important on 64-bit targets,
where many of the bits are not used for addressing and are used for
metadata instead, so I'm going to skip these tests on 32-bit arm
instead of debugging.
Added:
Modified:
lldb/test/API/python_api/process/address-masks/TestAddressMasks.py
Removed:
diff --git
a/lldb/test/API/python_api/process/address-masks/TestAddressMasks.py
b/lldb/test/API/python_api/process/address-masks/TestAddressMasks.py
index e0a570c15961c2..152776efc726f2 100644
--- a/lldb/test/API/python_api/process/address-masks/TestAddressMasks.py
+++ b/lldb/test/API/python_api/process/address-masks/TestAddressMasks.py
@@ -19,6 +19,7 @@ def reset_all_masks(self, process):
self.runCmd("settings set target.process.virtual-addressable-bits 0")
self.runCmd("settings set
target.process.highmem-virtual-addressable-bits 0")
+@skipIf(archs=["arm"]) # 32-bit arm ABI hardcodes Code mask, is 32-bit
def test_address_masks(self):
self.build()
(target, process, t, bp) = lldbutil.run_to_source_breakpoint(
@@ -79,6 +80,7 @@ def test_address_masks(self):
# AArch64 can have
diff erent address masks for high and low memory, when
diff erent
# page tables are set up.
@skipIf(archs=no_match(["arm64", "arm64e", "aarch64"]))
+@skipIf(archs=["arm"]) # 32-bit arm ABI hardcodes Code mask, is 32-bit
def test_address_masks_target_supports_highmem_tests(self):
self.build()
(target, process, t, bp) = lldbutil.run_to_source_breakpoint(
@@ -111,6 +113,7 @@ def test_address_masks_target_supports_highmem_tests(self):
# On most targets where we have a single mask for all address range,
confirm
# that the high memory masks are ignored.
@skipIf(archs=["arm64", "arm64e", "aarch64"])
+@skipIf(archs=["arm"]) # 32-bit arm ABI hardcodes Code mask, is 32-bit
def test_address_masks_target_no_highmem(self):
self.build()
(target, process, t, bp) = lldbutil.run_to_source_breakpoint(
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Address mask sbprocess apis and new mask invalid const (PR #83663)
https://github.com/jasonmolenda closed https://github.com/llvm/llvm-project/pull/83663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Address mask sbprocess apis and new mask invalid const (PR #83663)
jasonmolenda wrote: > Still not sure this is the ideal API but we can't know that until someone > (aka you :) ) has used it for a bit, so let's get this in. Thanks for all the help in finishing this one. I think this approach with enums to select the different types/ranges, with defaulted arguments so the most frequent use case is easy, is not bad. My original patch had a lot of different methods for the different operations and that was a lot less clear for script writers I think. https://github.com/llvm/llvm-project/pull/83663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Address mask sbprocess apis and new mask invalid const (PR #83663)
@@ -0,0 +1,130 @@
+"""Test Python APIs for setting, getting, and using address masks."""
+
+import os
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class AddressMasksTestCase(TestBase):
+NO_DEBUG_INFO_TESTCASE = True
+
+def reset_all_masks(self, process):
+process.SetAddressMask(
+lldb.eAddressMaskTypeAll,
+lldb.LLDB_INVALID_ADDRESS_MASK,
+lldb.eAddressMaskRangeAll,
+)
+
+def test_address_masks(self):
+self.build()
+(target, process, t, bp) = lldbutil.run_to_source_breakpoint(
+self, "break here", lldb.SBFileSpec("main.c")
+)
+
+process.SetAddressableBits(lldb.eAddressMaskTypeAll, 42)
+self.assertEqual(0x02953F94,
process.FixAddress(0x00265E953F94))
+self.reset_all_masks(process)
+
+# ~((1ULL<<42)-1) == 0xfc00
+process.SetAddressMask(lldb.eAddressMaskTypeAll, 0xFC00)
+self.assertEqual(0x02953F94,
process.FixAddress(0x00265E953F94))
+self.reset_all_masks(process)
+
+# Check that all bits can pass through unmodified
+process.SetAddressableBits(lldb.eAddressMaskTypeAll, 64)
+self.assertEqual(0x00265E953F94,
process.FixAddress(0x00265E953F94))
+self.reset_all_masks(process)
+
+process.SetAddressableBits(
+lldb.eAddressMaskTypeAll, 42, lldb.eAddressMaskRangeAll
+)
+self.assertEqual(0x02950001F694,
process.FixAddress(0x00265E950001F694))
+self.assertEqual(0xFE95F694,
process.FixAddress(0xFFA65E95F694))
+self.reset_all_masks(process)
+
+# Set a eAddressMaskTypeCode which has the low 3 bits marked as
non-address
+# bits, confirm that they're cleared by FixAddress.
+process.SetAddressableBits(
+lldb.eAddressMaskTypeAll, 42, lldb.eAddressMaskRangeAll
+)
+mask = process.GetAddressMask(lldb.eAddressMaskTypeAny)
+process.SetAddressMask(lldb.eAddressMaskTypeCode, mask | 0x3)
+process.SetAddressMask(lldb.eAddressMaskTypeCode, 0xFC03)
jasonmolenda wrote:
Ah, thanks for spotting that. I originally wrote it as a hex literal and then
I thought "Oh it would be clearer to bitmask in the 0b111" and forgot to remove
the lit.
https://github.com/llvm/llvm-project/pull/83663
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Address mask sbprocess apis and new mask invalid const (PR #83663)
https://github.com/jasonmolenda updated
https://github.com/llvm/llvm-project/pull/83663
>From c993c7cc7c1669ca7d06e52f1a1ff8dbefe9ebc9 Mon Sep 17 00:00:00 2001
From: Jason Molenda
Date: Thu, 29 Feb 2024 17:02:42 -0800
Subject: [PATCH 1/5] [lldb] Add SBProcess methods for get/set/use address
masks (#83095)
I'm reviving a patch from phabracator, https://reviews.llvm.org/D155905
which was approved but I wasn't thrilled with all the API I was adding
to SBProcess for all of the address mask types / memory regions. In this
update, I added enums to control type address mask type (code, data,
any) and address space specifiers (low, high, all) with defaulted
arguments for the most common case.
This patch is also fixing a bug in the "addressable bits to address
mask" calculation I added in AddressableBits::SetProcessMasks. If lldb
were told that 64 bits are valid for addressing, this method would
overflow the calculation and set an invalid mask. Added tests to check
this specific bug while I was adding these APIs.
rdar://123530562
---
lldb/include/lldb/API/SBProcess.h | 114 ++
lldb/include/lldb/Utility/AddressableBits.h | 3 +
lldb/include/lldb/lldb-defines.h | 5 +
lldb/include/lldb/lldb-enumerations.h | 16 +++
lldb/source/API/SBProcess.cpp | 92 ++
lldb/source/Target/Process.cpp| 10 +-
lldb/source/Utility/AddressableBits.cpp | 12 +-
.../python_api/process/address-masks/Makefile | 3 +
.../process/address-masks/TestAddressMasks.py | 74
.../python_api/process/address-masks/main.c | 5 +
10 files changed, 328 insertions(+), 6 deletions(-)
create mode 100644 lldb/test/API/python_api/process/address-masks/Makefile
create mode 100644
lldb/test/API/python_api/process/address-masks/TestAddressMasks.py
create mode 100644 lldb/test/API/python_api/process/address-masks/main.c
diff --git a/lldb/include/lldb/API/SBProcess.h
b/lldb/include/lldb/API/SBProcess.h
index 4f92a41f3028a2..7da3335a7234b7 100644
--- a/lldb/include/lldb/API/SBProcess.h
+++ b/lldb/include/lldb/API/SBProcess.h
@@ -407,6 +407,120 @@ class LLDB_API SBProcess {
/// the process isn't loaded from a core file.
lldb::SBFileSpec GetCoreFile();
+ /// \{
+ /// \group Mask Address Methods
+ ///
+ /// \a type
+ /// All of the methods in this group take \a type argument
+ /// which is an AddressMaskType enum value.
+ /// There can be different address masks for code addresses and
+ /// data addresses, this argument can select which to get/set,
+ /// or to use when clearing non-addressable bits from an address.
+ /// This choice of mask can be important for example on AArch32
+ /// systems. Where instructions where instructions start on even addresses,
+ /// the 0th bit may be used to indicate that a function is thumb code. On
+ /// such a target, the eAddressMaskTypeCode may clear the 0th bit from an
+ /// address to get the actual address Whereas eAddressMaskTypeData would not.
+ ///
+ /// \a addr_range
+ /// Many of the methods in this group take an \a addr_range argument
+ /// which is an AddressMaskRange enum value.
+ /// Needing to specify the address range is highly unusual, and the
+ /// default argument can be used in nearly all circumstances.
+ /// On some architectures (e.g., AArch64), it is possible to have
+ /// different page table setups for low and high memory, so different
+ /// numbers of bits relevant to addressing. It is possible to have
+ /// a program running in one half of memory and accessing the other
+ /// as heap, so we need to maintain two different sets of address masks
+ /// to debug this correctly.
+
+ /// Get the current address mask that will be applied to addresses
+ /// before reading from memory.
+ ///
+ /// \param[in] type
+ /// See \ref Mask Address Methods description of this argument.
+ /// eAddressMaskTypeAny is often a suitable value when code and
+ /// data masks are the same on a given target.
+ ///
+ /// \param[in] addr_range
+ /// See \ref Mask Address Methods description of this argument.
+ /// This will default to eAddressMaskRangeLow which is the
+ /// only set of masks used normally.
+ ///
+ /// \return
+ /// The address mask currently in use. Bits which are not used
+ /// for addressing will be set to 1 in the mask.
+ lldb::addr_t GetAddressMask(
+ lldb::AddressMaskType type,
+ lldb::AddressMaskRange addr_range = lldb::eAddressMaskRangeLow);
+
+ /// Set the current address mask that can be applied to addresses
+ /// before reading from memory.
+ ///
+ /// \param[in] type
+ /// See \ref Mask Address Methods description of this argument.
+ /// eAddressMaskTypeAll is often a suitable value when the
+ /// same mask is being set for both code and data.
+ ///
+ /// \param[in] mask
+ /// The address mask to set. Bits which are not used for addressing
+
[Lldb-commits] [lldb] Address mask sbprocess apis and new mask invalid const (PR #83663)
https://github.com/jasonmolenda updated
https://github.com/llvm/llvm-project/pull/83663
>From c993c7cc7c1669ca7d06e52f1a1ff8dbefe9ebc9 Mon Sep 17 00:00:00 2001
From: Jason Molenda
Date: Thu, 29 Feb 2024 17:02:42 -0800
Subject: [PATCH 1/4] [lldb] Add SBProcess methods for get/set/use address
masks (#83095)
I'm reviving a patch from phabracator, https://reviews.llvm.org/D155905
which was approved but I wasn't thrilled with all the API I was adding
to SBProcess for all of the address mask types / memory regions. In this
update, I added enums to control type address mask type (code, data,
any) and address space specifiers (low, high, all) with defaulted
arguments for the most common case.
This patch is also fixing a bug in the "addressable bits to address
mask" calculation I added in AddressableBits::SetProcessMasks. If lldb
were told that 64 bits are valid for addressing, this method would
overflow the calculation and set an invalid mask. Added tests to check
this specific bug while I was adding these APIs.
rdar://123530562
---
lldb/include/lldb/API/SBProcess.h | 114 ++
lldb/include/lldb/Utility/AddressableBits.h | 3 +
lldb/include/lldb/lldb-defines.h | 5 +
lldb/include/lldb/lldb-enumerations.h | 16 +++
lldb/source/API/SBProcess.cpp | 92 ++
lldb/source/Target/Process.cpp| 10 +-
lldb/source/Utility/AddressableBits.cpp | 12 +-
.../python_api/process/address-masks/Makefile | 3 +
.../process/address-masks/TestAddressMasks.py | 74
.../python_api/process/address-masks/main.c | 5 +
10 files changed, 328 insertions(+), 6 deletions(-)
create mode 100644 lldb/test/API/python_api/process/address-masks/Makefile
create mode 100644
lldb/test/API/python_api/process/address-masks/TestAddressMasks.py
create mode 100644 lldb/test/API/python_api/process/address-masks/main.c
diff --git a/lldb/include/lldb/API/SBProcess.h
b/lldb/include/lldb/API/SBProcess.h
index 4f92a41f3028a2..7da3335a7234b7 100644
--- a/lldb/include/lldb/API/SBProcess.h
+++ b/lldb/include/lldb/API/SBProcess.h
@@ -407,6 +407,120 @@ class LLDB_API SBProcess {
/// the process isn't loaded from a core file.
lldb::SBFileSpec GetCoreFile();
+ /// \{
+ /// \group Mask Address Methods
+ ///
+ /// \a type
+ /// All of the methods in this group take \a type argument
+ /// which is an AddressMaskType enum value.
+ /// There can be different address masks for code addresses and
+ /// data addresses, this argument can select which to get/set,
+ /// or to use when clearing non-addressable bits from an address.
+ /// This choice of mask can be important for example on AArch32
+ /// systems. Where instructions where instructions start on even addresses,
+ /// the 0th bit may be used to indicate that a function is thumb code. On
+ /// such a target, the eAddressMaskTypeCode may clear the 0th bit from an
+ /// address to get the actual address Whereas eAddressMaskTypeData would not.
+ ///
+ /// \a addr_range
+ /// Many of the methods in this group take an \a addr_range argument
+ /// which is an AddressMaskRange enum value.
+ /// Needing to specify the address range is highly unusual, and the
+ /// default argument can be used in nearly all circumstances.
+ /// On some architectures (e.g., AArch64), it is possible to have
+ /// different page table setups for low and high memory, so different
+ /// numbers of bits relevant to addressing. It is possible to have
+ /// a program running in one half of memory and accessing the other
+ /// as heap, so we need to maintain two different sets of address masks
+ /// to debug this correctly.
+
+ /// Get the current address mask that will be applied to addresses
+ /// before reading from memory.
+ ///
+ /// \param[in] type
+ /// See \ref Mask Address Methods description of this argument.
+ /// eAddressMaskTypeAny is often a suitable value when code and
+ /// data masks are the same on a given target.
+ ///
+ /// \param[in] addr_range
+ /// See \ref Mask Address Methods description of this argument.
+ /// This will default to eAddressMaskRangeLow which is the
+ /// only set of masks used normally.
+ ///
+ /// \return
+ /// The address mask currently in use. Bits which are not used
+ /// for addressing will be set to 1 in the mask.
+ lldb::addr_t GetAddressMask(
+ lldb::AddressMaskType type,
+ lldb::AddressMaskRange addr_range = lldb::eAddressMaskRangeLow);
+
+ /// Set the current address mask that can be applied to addresses
+ /// before reading from memory.
+ ///
+ /// \param[in] type
+ /// See \ref Mask Address Methods description of this argument.
+ /// eAddressMaskTypeAll is often a suitable value when the
+ /// same mask is being set for both code and data.
+ ///
+ /// \param[in] mask
+ /// The address mask to set. Bits which are not used for addressing
+
[Lldb-commits] [lldb] Address mask sbprocess apis and new mask invalid const (PR #83663)
jasonmolenda wrote: I pushed an update to this PR where I have the base class Fix methods in ABI.cpp ignore the highmem masks completely, I added highmem mask use when it's a highmem address and they are set to the AArch64ABI class so all the AArch64 ABI plugins are checking this. I changed the API test to only do highmem tests on AArch64 targets, and on other targets they will test that highmem masks are ignored. If any target wants this feature, I would like them to have to explicitly declare it in the TestAddressMasks.py so things don't kinda-work/fail unintentionally. https://github.com/llvm/llvm-project/pull/83663 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Address mask sbprocess apis and new mask invalid const (PR #83663)
https://github.com/jasonmolenda updated
https://github.com/llvm/llvm-project/pull/83663
>From c993c7cc7c1669ca7d06e52f1a1ff8dbefe9ebc9 Mon Sep 17 00:00:00 2001
From: Jason Molenda
Date: Thu, 29 Feb 2024 17:02:42 -0800
Subject: [PATCH 1/3] [lldb] Add SBProcess methods for get/set/use address
masks (#83095)
I'm reviving a patch from phabracator, https://reviews.llvm.org/D155905
which was approved but I wasn't thrilled with all the API I was adding
to SBProcess for all of the address mask types / memory regions. In this
update, I added enums to control type address mask type (code, data,
any) and address space specifiers (low, high, all) with defaulted
arguments for the most common case.
This patch is also fixing a bug in the "addressable bits to address
mask" calculation I added in AddressableBits::SetProcessMasks. If lldb
were told that 64 bits are valid for addressing, this method would
overflow the calculation and set an invalid mask. Added tests to check
this specific bug while I was adding these APIs.
rdar://123530562
---
lldb/include/lldb/API/SBProcess.h | 114 ++
lldb/include/lldb/Utility/AddressableBits.h | 3 +
lldb/include/lldb/lldb-defines.h | 5 +
lldb/include/lldb/lldb-enumerations.h | 16 +++
lldb/source/API/SBProcess.cpp | 92 ++
lldb/source/Target/Process.cpp| 10 +-
lldb/source/Utility/AddressableBits.cpp | 12 +-
.../python_api/process/address-masks/Makefile | 3 +
.../process/address-masks/TestAddressMasks.py | 74
.../python_api/process/address-masks/main.c | 5 +
10 files changed, 328 insertions(+), 6 deletions(-)
create mode 100644 lldb/test/API/python_api/process/address-masks/Makefile
create mode 100644
lldb/test/API/python_api/process/address-masks/TestAddressMasks.py
create mode 100644 lldb/test/API/python_api/process/address-masks/main.c
diff --git a/lldb/include/lldb/API/SBProcess.h
b/lldb/include/lldb/API/SBProcess.h
index 4f92a41f3028a2..7da3335a7234b7 100644
--- a/lldb/include/lldb/API/SBProcess.h
+++ b/lldb/include/lldb/API/SBProcess.h
@@ -407,6 +407,120 @@ class LLDB_API SBProcess {
/// the process isn't loaded from a core file.
lldb::SBFileSpec GetCoreFile();
+ /// \{
+ /// \group Mask Address Methods
+ ///
+ /// \a type
+ /// All of the methods in this group take \a type argument
+ /// which is an AddressMaskType enum value.
+ /// There can be different address masks for code addresses and
+ /// data addresses, this argument can select which to get/set,
+ /// or to use when clearing non-addressable bits from an address.
+ /// This choice of mask can be important for example on AArch32
+ /// systems. Where instructions where instructions start on even addresses,
+ /// the 0th bit may be used to indicate that a function is thumb code. On
+ /// such a target, the eAddressMaskTypeCode may clear the 0th bit from an
+ /// address to get the actual address Whereas eAddressMaskTypeData would not.
+ ///
+ /// \a addr_range
+ /// Many of the methods in this group take an \a addr_range argument
+ /// which is an AddressMaskRange enum value.
+ /// Needing to specify the address range is highly unusual, and the
+ /// default argument can be used in nearly all circumstances.
+ /// On some architectures (e.g., AArch64), it is possible to have
+ /// different page table setups for low and high memory, so different
+ /// numbers of bits relevant to addressing. It is possible to have
+ /// a program running in one half of memory and accessing the other
+ /// as heap, so we need to maintain two different sets of address masks
+ /// to debug this correctly.
+
+ /// Get the current address mask that will be applied to addresses
+ /// before reading from memory.
+ ///
+ /// \param[in] type
+ /// See \ref Mask Address Methods description of this argument.
+ /// eAddressMaskTypeAny is often a suitable value when code and
+ /// data masks are the same on a given target.
+ ///
+ /// \param[in] addr_range
+ /// See \ref Mask Address Methods description of this argument.
+ /// This will default to eAddressMaskRangeLow which is the
+ /// only set of masks used normally.
+ ///
+ /// \return
+ /// The address mask currently in use. Bits which are not used
+ /// for addressing will be set to 1 in the mask.
+ lldb::addr_t GetAddressMask(
+ lldb::AddressMaskType type,
+ lldb::AddressMaskRange addr_range = lldb::eAddressMaskRangeLow);
+
+ /// Set the current address mask that can be applied to addresses
+ /// before reading from memory.
+ ///
+ /// \param[in] type
+ /// See \ref Mask Address Methods description of this argument.
+ /// eAddressMaskTypeAll is often a suitable value when the
+ /// same mask is being set for both code and data.
+ ///
+ /// \param[in] mask
+ /// The address mask to set. Bits which are not used for addressing
+
