Aj0SK added inline comments.
================
Comment at: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:519
+lldb_private::Status
+MinidumpFileBuilder::AddMemoryList(const lldb::ProcessSP &process_sp) {
+ Status error;
----------------
Aj0SK wrote:
> clayborg wrote:
> > This should take in the CoreStyle and only emit what was asked for.
> >
> > If style is set to "eSaveCoreStackOnly", then grab only the memory regions
> > for each thread stack pointer and save only those.
> >
> > We can't tell from LLDB APIs if a page is dirty, so if we get
> > "eSaveCoreDirtyOnly", then we will need to save all memory regions that
> > have write permissions.
> >
> > If it is either "eSaveCoreFull" or "eSaveCoreUnspecified" then save
> > everything.
> I agree that this code should take care of a CoreStyle but it poses a
> significant problem to this implementation as it was mainly designed to save
> smaller Minidumps. This solution stores all of the information in memory
> before dumping them (this eases an implementation quite a bit because of the
> way how Minidump pointers (offsets) are working). This implementation, on my
> machine, exhausted memory before the minidump could be written in case of a
> full memory dump. At this time, we don't plan to reimplement this solution in
> the way it would allow to store all the data on disc at the time of minidump
> creation so there are two possible solutions that I see:
>
> 1. If the type of the CoreStyle is full or dirty, this plugin will return
> false from the SaveCore function. Then maybe the default CoreStyle for this
> plugin should be changed to "eSaveCoreStackOnly".
>
> 2. To the best of my knowledge, it should be theoretically possible to
> reimplement Dump() method to sort of take special care of a MemoryListStream,
> dumping also the memory information at the end of the minidump file as I
> think the memory dump is the most stressful for the memory and otherwise
> there is no problem with this.
To state my preference, I would rather stick to the first option and landed a
minimum viable product. The only reason for this is that I have this version
way better tested and also I am not entirely sure about how minidump deals with
the whole memory dumps as it can be a little problematic for a big memory
chunks... Then, I would rather add the necessary changes in the next patch...
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108233/new/
https://reviews.llvm.org/D108233
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
