This is an archive of the discontinued LLVM Phabricator instance.

[lldb] Do not deallocate memory after exec
ClosedPublic

Authored by bulbazord on Dec 16 2022, 3:01 PM.

Details

Summary

After an exec has occured, resources used to manage the state of a
Process are cleaned up. One such resource is the AllocatedMemoryCache
which keeps track of memory allocations made in the process for things
like expression evaluation. After an exec is performed, the allocated
memory regions in the process are gone, so it does not make sense to try
to deallocate those regions.

rdar://103188106

Diff Detail

Event Timeline

bulbazord created this revision.Dec 16 2022, 3:01 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 16 2022, 3:01 PM
bulbazord requested review of this revision.Dec 16 2022, 3:01 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 16 2022, 3:01 PM
jasonmolenda added inline comments.Dec 16 2022, 3:13 PM
lldb/source/Target/Memory.cpp
337

Should we comment that if we've just exec'ed, the inferior is a new process and does not have these memory regions allocated.

bulbazord added inline comments.Dec 16 2022, 4:21 PM
lldb/source/Target/Memory.cpp
337

Yeah, I think that would be helpful.

bulbazord updated this revision to Diff 483692.Dec 16 2022, 4:24 PM

Add a comment explaining why we do not dealloc after exec

jingham requested changes to this revision.Jan 10 2023, 3:50 PM

It doesn't seem unreasonable that in the course of time we'll come across another circumstance where it's not worth deallocating these memory regions (for instance, we really don't need to do that if we're planning to kill the process). So it seems awkward to encode this particular reason in the Memory::Clear function. I think it would be better to have the parameter be "deallocate_memory", and then move the comment to Process::DidExec.

This revision now requires changes to proceed.Jan 10 2023, 3:50 PM
bulbazord updated this revision to Diff 488035.Jan 10 2023, 4:45 PM

Address @jingham's comments

jasonmolenda accepted this revision.Jan 11 2023, 11:03 AM

LGTM. Jim?

jingham accepted this revision.Jan 11 2023, 11:28 AM

LGTM, thanks!

This revision is now accepted and ready to land.Jan 11 2023, 11:28 AM
This revision was automatically updated to reflect the committed changes.