DavidSpickett added inline comments.
================
Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:1400
+ .ToError())
+ return std::move(Err);
+
----------------
Is it possible that the page we find is executable but not writeable and is
this part intended to handle that?
(maybe this is a silly question, the code had to be written into that page in
the first place I guess)
================
Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:1404
+
+ int req = SupportHardwareSingleStepping() ? PTRACE_SINGLESTEP : PTRACE_CONT;
+ if (llvm::Error Err =
----------------
Could you add a comment here explaining this? Like "either we single step just
syscall, or continue then hit a trap instruction after the syscall"
(if I have that right)
The description for PTRACE_SINGLESTEP confused me a bit with "next entry to":
> Restart the stopped tracee as for PTRACE_CONT, but arrange for
> the tracee to be stopped at the next entry to or exit from a
> system call, or after execution of a single instruction,
> respectively.
But I assume what happens here is you step "syscall" and then the whole mmap
call happens, then you come back. Instead of getting kept before the mmap
actually happens.
================
Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:1419
+ uint64_t result = reg_ctx.ReadRegisterAsUnsigned(syscall_data.Result,
-ESRCH);
+ uint64_t errno_threshold =
+ (uint64_t(-1) >> (64 - 8 * m_arch.GetAddressByteSize())) - 0x1000;
----------------
What does this calculation do/mean? (0x1000 is 4k which is a page size maybe?)
================
Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:1421
+ (uint64_t(-1) >> (64 - 8 * m_arch.GetAddressByteSize())) - 0x1000;
+ if (result > errno_threshold) {
+ return llvm::errorCodeToError(
----------------
For these ifs, are you putting {} because the return is split over two lines? I
think it would compile without but is this one of the exceptions to the coding
standard? (if clang-format isn't doing this for you that is)
================
Comment at:
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp:1249
+ case llvm::Triple::x86:
+ return MmapData{192, 91};
+ case llvm::Triple::x86_64:
----------------
What's the source for these numbers?
================
Comment at:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:2346
+
+ const lldb::addr_t size = packet.GetHexMaxU64(false, LLDB_INVALID_ADDRESS);
+ if (size == LLDB_INVALID_ADDRESS)
----------------
(assuming this packet type is supported by GDB already) Does it also use hex
for the size field? I ask because I came across some file loading packets that
were hex for lldb, int for gdb.
Obviously we don't have a hard requirement to match but if we're adding
something new might as well.
================
Comment at:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:2366
+ }
+ }
+
----------------
Is no permissions here a valid packet? (as long as mmap doesn't mind I guess it
is)
================
Comment at: lldb/test/API/lang/c/stepping/TestStepAndBreakpoints.py:23
@expectedFailureAll(oslist=['freebsd'], bugnumber='llvm.org/pr17932')
- @expectedFailureAll(oslist=["linux"], bugnumber="llvm.org/pr14437")
@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24777")
----------------
This test now passes *because* we have the ability to allocate memory, correct?
(I thought it was a stray change at first)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D89124/new/
https://reviews.llvm.org/D89124
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
