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:
> 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?
disregard this comment, I understood your remark incorrectly
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
