wallace added inline comments.
================
Comment at: lldb/include/lldb/Target/Trace.h:31-32
+///
+/// This class assumes that errors will be extremely rare compared to the
number
+/// of correct instructions and storing them as \a ConstString should be fine.
+class TraceInstruction {
----------------
wallace wrote:
> clayborg wrote:
> > Can't we just avoid including any errors in this TraceInstruction class?
> > And have the function that generates these return a:
> >
> > ```
> > llvm::Expected<TraceInstruction>
> > ```
> > ?
> not really, because these errors don't mean that the entire decoding failed,
> they instead mean that some specific chunks of the trace were not able to be
> decoded.
> For example, you can have this decoded trace:
>
> ```
> a.out`(none)
> [ 4] 0x0000000000400510 pushq 0x200af2(%rip) ;
> _GLOBAL_OFFSET_TABLE_ + 8
> [ 5] 0x0000000000400516 jmpq *0x200af4(%rip) ;
> _GLOBAL_OFFSET_TABLE_ + 16
> [ 6] 0x00007ffff7df1950 error: no memory mapped at this address
> ...missing instructions
> a.out`main + 20 at main.cpp:10
> [ 7] 0x0000000000400674 movl %eax, -0xc(%rbp)
> ```
>
> Instruction 6 is an error. We have an associated address but the decoder
> couldn't decode it and then it skipped the trace until the next
> synchronization point, which is instruction 7 in this case.
>
> Not all decoder errors have an associated address, though, only some. So I
> prefer just to have any errors be represented as a strings for now.
>
> Another option is to create a new class that contains a sequential list of
> valid instructions terminated by 0 or more contiguous errors. But that seems
> a little bit too much, as there's not a lot that we can do when we see
> errors, we either ignore them or we stop the current iteration.
It should be possible. I wonder about memory consumption.
llvm::Expected has these members:
```
union {
AlignedCharArrayUnion<storage_type> TStorage;
AlignedCharArrayUnion<error_type> ErrorStorage;
};
bool HasError : 1;
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
bool Unchecked : 1;
#endif
```
that means that it would end up using 3 * 8 bytes in total, right?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103588/new/
https://reviews.llvm.org/D103588
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
