wallace marked 8 inline comments as done.
wallace added a comment.
> When you are dumping instructions we are only showing one hex value. Is this
> the instruction address or the opcode itself?
That's the instruction load address. In a later diff I'll implement
pretty-printing similar to the "disassembly" command, but for now this is a
good start.
================
Comment at: lldb/source/Target/ProcessTrace.cpp:151-152
size_t ProcessTrace::DoReadMemory(addr_t addr, void *buf, size_t size,
Status &error) {
+ const std::map<MemoryRegionInfo::RangeType, SectionSP> §ion_map =
----------------
clayborg wrote:
> You should be able to just call:
>
> ```
> size_t Target::ReadMemoryFromFileCache(const Address &addr, void *dst, size_t
> dst_len, Status &error);
> ```
> It already does what you are doing here if all that is happening here is
> reading from loaded object file section data.
>
This is exactly what I needed! The name is just not very precise =P
================
Comment at: lldb/test/API/commands/trace/TestTraceDumpInstructions.py:41-61
self.expect("thread trace dump instructions",
- substrs=['thread #1: tid = 3842849, total instructions = 1000',
- 'would print 20 instructions from position 0'])
+ substrs=['''thread #1: tid = 3842849, total instructions = 21
+ [ 0] 0x40052d
+ [ 1] 0x400529
+ [ 2] 0x400525
+ [ 3] 0x400521
+ [ 4] 0x40052d
----------------
clayborg wrote:
> What is the default count here? 19? Seems like an odd number to choose as a
> default?
The default is 20, but I'm adding the 20th-element here for clarity
================
Comment at: lldb/test/API/commands/trace/TestTraceDumpInstructions.py:142-143
+ substrs=['''thread #1: tid = 3842849, total instructions = 2
+ [0] no memory mapped at this address
+ [1] no memory mapped at this address'''])
+
----------------
clayborg wrote:
> We should be showing addresses here. It doesn't matter if they are mapped or
> not. This will happen for JIT'ed code.
I'll do this in a later diff. Currently libipt doesn't report the addresses
that it fails to decode, but I'm planning on making a patch on libipt to
support that.
================
Comment at: lldb/test/API/commands/trace/TestTraceDumpInstructions.py:163
+ [ 3] 0x400654
+ [ 4] no memory mapped at this address
+ [ 5] 0x400516
----------------
clayborg wrote:
> Why aren't we showing the address here? We will run into cases, for possibly
> JIT'ed code where we won't have a section for an address, so we should still
> show the address
>
Repeating my message from above:
> I'll do this in a later diff. Currently libipt doesn't report the addresses
> that it fails to decode, but I'm planning on making a patch on libipt to
> support that.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D89283/new/
https://reviews.llvm.org/D89283
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
