[Lldb-commits] [PATCH] D127016: [lldb] Prevent crash due to reading memory from page zero.

[Med Ismail Bennani via Phabricator via lldb-commits]

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

LGTM with some minor feedbacks :) !



================
Comment at: lldb/source/Commands/CommandObjectMemory.cpp:597
+    ABISP abi;
+    if (Process * proc = m_exe_ctx.GetProcessPtr())
+      abi = proc->GetABI();
----------------
kastiglione wrote:
> Should `memory read` emit an error if there's no process (and no core file or any other memory to read from)?
Nit: I don't think this is formatted properly


================
Comment at: lldb/source/Commands/CommandObjectMemory.cpp:597-598
+    ABISP abi;
+    if (Process * proc = m_exe_ctx.GetProcessPtr())
+      abi = proc->GetABI();
+
----------------
mib wrote:
> kastiglione wrote:
> > Should `memory read` emit an error if there's no process (and no core file or any other memory to read from)?
> Nit: I don't think this is formatted properly
@kastiglione On Apple's lldb, we get this:

```
(lldb) x 0
error: error reading data from section __PAGEZERO
(lldb) target delete 0
1 targets deleted.
(lldb) x 0
error: invalid target, create a target using the 'target create' command
(lldb) 
```

Might be an oversight.


================
Comment at: lldb/test/Shell/Driver/TestPageZeroRead.test:3
+# RUN: %clang_host %p/Inputs/hello.c -g -o a.out
+# RUN: %lldb -b a.out -o 'x 0'
----------------
You probably want to check the error string


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127016