MaskRay added inline comments.
Comment at: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:120
+ if (error.Fail()) {
+error.SetErrorString("Unable to convert the csd string to UTF16.");
+return error;
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGeee687a66d76: [lldb] Add minidump save-core functionality to
ELF object files (authored by Aj0SK, committed by werat).
Repository:
rG LLVM Github
Aj0SK added inline comments.
Comment at:
lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py:17
+
+@skipUnlessArch("x86_64")
+@skipUnlessPlatform(["linux"])
clayborg wrote:
> Do we only support x86_64 right now? If we
clayborg added inline comments.
Comment at:
lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py:17
+
+@skipUnlessArch("x86_64")
+@skipUnlessPlatform(["linux"])
Do we only support x86_64 right now? If we are actually
Aj0SK updated this revision to Diff 369722.
Aj0SK added a comment.
Fix not-correctly applied change from review regarding memory reading.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108233/new/
https://reviews.llvm.org/D108233
Files:
Aj0SK updated this revision to Diff 369686.
Aj0SK added a comment.
Fixes arm and aarch64 build run fails. Adding aarch64 to the matching in
SystemInfo stream and activating test only on x86_64 as this is the only
platform where also thread info etc. is being saved.
Repository:
rG LLVM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGaafa05e03d62: [lldb] Add minidump save-core functionality to
ELF object files (authored by Aj0SK, committed by werat).
Changed prior to commit:
clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.
Thanks for all of the changes! Looks great.
Comment at: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:549
+bool is_interesting = false;
+for
Aj0SK updated this revision to Diff 369410.
Aj0SK marked 2 inline comments as done and an inline comment as not done.
Aj0SK added a comment.
Add error for dump core style other than stack and change procedure getting the
memory list stream.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE
Aj0SK marked 8 inline comments as done and an inline comment as not done.
Aj0SK added inline comments.
Comment at: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:519
+lldb_private::Status
+MinidumpFileBuilder::AddMemoryList(const lldb::ProcessSP _sp) {
+ Status
Aj0SK added inline comments.
Comment at: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:549
+bool is_interesting = false;
+for (size_t interesting_address : interesting_addresses)
+ if (interesting_address >= addr && interesting_address < addr +
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
No worries on only saving out the minimal for now, but just make sure that you
error out if CoreStyle is anything but "eSaveCoreUnspecified" or
"eSaveCoreStackOnly".
Aj0SK added inline comments.
Comment at: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:519
+lldb_private::Status
+MinidumpFileBuilder::AddMemoryList(const lldb::ProcessSP _sp) {
+ Status error;
Aj0SK wrote:
> clayborg wrote:
> > This should
Aj0SK added inline comments.
Comment at: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:225
+m.SizeOfImage = static_cast(
+mod->GetObjectFile()->GetByteSize());
+m.Checksum = static_cast(0);
clayborg wrote:
> I am not sure if the
clayborg added a comment.
Very nice! Structure is good now. I found a few other issue we should fix as
well.
Comment at: lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp:225
+m.SizeOfImage = static_cast(
+mod->GetObjectFile()->GetByteSize());
+
Aj0SK updated this revision to Diff 368765.
Aj0SK added a comment.
Change ObjectFileMinidump plugin to inherit from PluginInterface
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108233/new/
https://reviews.llvm.org/D108233
Files:
clayborg added inline comments.
Comment at: lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp:31
+ /*create_memory_callback=*/nullptr,
+ /*get_module_specifications=*/nullptr, SaveCore);
+}
It should be fine to not inherit from ObjectFile
Aj0SK marked 9 inline comments as done.
Aj0SK added a comment.
Thanks for the review! Some requested changes need to be clarified for me. I
have a problem mainly with successfully registering the Plugin when it inherits
only from PluginInterface.
Comment at:
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
Thanks for moving over into object file. See inlined comments, we should be
able to get this down to just the SaveCore and other static functions that just
return nothing. Let
Aj0SK updated this revision to Diff 368110.
Aj0SK added a comment.
Herald added a subscriber: dang.
Move minidump save-core functionality to separate plugin
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108233/new/
https://reviews.llvm.org/D108233
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
I like the idea of this! I have a minidump creation tool I wrote in python for
making tests.
ELF files support core files in their native file format, so I think the
Aj0SK created this revision.
Aj0SK added a reviewer: clayborg.
Herald added subscribers: pengfei, mgorny, emaste.
Aj0SK requested review of this revision.
Herald added subscribers: lldb-commits, MaskRay.
Herald added a project: LLDB.
This change adds save-core functionality into the ObjectFileELF
22 matches
Mail list logo
