This is an archive of the discontinued LLVM Phabricator instance.

[lldb] Prevent crash due to reading memory from page zero.
ClosedPublic

Authored by cassanova on Jun 3 2022, 3:45 PM.

Details

Summary

Adds a check to ensure that a process exists before attempting to get its ABI to prevent lldb from crash due to trying to read from page zero.

Diff Detail

Event Timeline

cassanova created this revision.Jun 3 2022, 3:45 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 3 2022, 3:45 PM
cassanova requested review of this revision.Jun 3 2022, 3:45 PM
kastiglione added inline comments.
lldb/source/Commands/CommandObjectMemory.cpp
596–597

Should memory read emit an error if there's no process (and no core file or any other memory to read from)?

mib accepted this revision.Jun 3 2022, 4:23 PM

LGTM with some minor feedbacks :) !

lldb/source/Commands/CommandObjectMemory.cpp
596

Nit: I don't think this is formatted properly

596–597

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

lldb/test/Shell/Driver/TestPageZeroRead.test
4

You probably want to check the error string

This revision is now accepted and ready to land.Jun 3 2022, 4:23 PM
cassanova added inline comments.Jun 3 2022, 4:23 PM
lldb/source/Commands/CommandObjectMemory.cpp
596–597

Right now running memory read without a target indicates that there's no target. The input that can cause lldb to crash is x 0 (such as ./bin/lldb -o 'x 0' ./bin/count.

jingham added a subscriber: jingham.EditedJun 3 2022, 5:34 PM

More generally, you can tell which elements of the exe_ctx you need to check in any given command by looking at requirements for the command which are passed into the constructor for the command. Any entity that's required there, you don't need to check for, but otherwise, you do have to check. "CommandObjectMemoryRead" has:

eCommandRequiresTarget | eCommandProcessMustBePaused

so you should not need to check the target, but any references to the process, thread or frame in the exe_ctx must be checked. And if there is a process, you don't need to check whether it is paused or not.