[Lldb-commits] [PATCH] D90729: [trace][intel-pt] Scaffold the 'thread trace start | stop' commands
Greg Clayton via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Tue Nov 17 21:01:08 PST 2020
clayborg added a comment.
LGTM. A few inline nits, but nothing major. I only question if "RecordedProcess" is the right name for the base class for a core file/trace file, but don't have much of a better name. Maybe SerializedProcess? SavedProcess? Don't we usually have Pavel in on these reviews?
================
Comment at: lldb/include/lldb/Target/RecordedProcess.h:18
+/// Base class for all processes that don't represent a live process, such as
+/// coredumps or processes traced in the past.
+class RecordedProcess : public Process {
----------------
Might want to comment something like "common lldb_private::Process virtual functions overrides that are common between these kinds of processes can have default implementations in this class".
================
Comment at: lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp:209
GetTarget().SetArchitecture(target_arch);
-
+
SetUnixSignals(UnixSignals::Create(GetArchitecture()));
----------------
undo space only change
================
Comment at: lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp:246-247
if (exe_module_spec.GetFileSpec()) {
- exe_module_sp = GetTarget().GetOrCreateModule(exe_module_spec,
- true /* notify */);
+ exe_module_sp =
+ GetTarget().GetOrCreateModule(exe_module_spec, true /* notify */);
if (exe_module_sp)
----------------
undo space only change
================
Comment at: lldb/source/Plugins/Process/elf-core/ProcessElfCore.cpp:766
/// - NT_FILE - Files mapped into memory
-///
+///
/// Additionally, for each thread in the process the core file will contain at
----------------
undo space only change
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D90729/new/
https://reviews.llvm.org/D90729
More information about the lldb-commits
mailing list