wallace marked 6 inline comments as done.
wallace added inline comments.
================
Comment at: lldb/source/Target/Trace.cpp:173
+ // instruction's disassembler when possible.
+ auto calculate_disass = [&](const Address &address, const SymbolContext &sc)
{
+ if (prev_insn.disassembler) {
----------------
clayborg wrote:
> Can or should we get the disassembler in calculate_symbol_context above after
> calling address.CalculateSymbolContext(...)?
i think it's fine as it is, as we don't always need a disassembly. I want to
use this TraverseInstructionsWithSymbolInfo method for creating the call graph,
and I don't need the disassembly for that
================
Comment at: lldb/source/Target/Trace.cpp:203
+ thread, position, direction,
+ [&](size_t index, Expected<lldb::addr_t> load_address) -> bool {
+ if (!load_address)
----------------
clayborg wrote:
> Can this function really be called without a valid load address?
yes, whenever there are gaps in the trace
================
Comment at: lldb/source/Target/Trace.cpp:310
size_t end_position, bool raw) {
- if (!IsTraced(thread)) {
+ Optional<size_t> instructions_count = GetInstructionCount(thread);
+ if (!instructions_count) {
----------------
clayborg wrote:
> Should this return a Expected<size_t>? In case there is an error we want to
> show? Like maybe the trace buffer was truncated, or missing when it was
> expected? Can we have a process that is traced, but by the time we fetch the
> instructions, the circular buffer was filled with other data and even though
> the thread was traced there is no data?
Any kind of errors are represented as failed instructions, which means that if
the entire operation failed, there's one single instruction with an error
message. If the thread is not traced at all, then this method return None.
There can be errors while getting the data or errors while decoding individual
instructions. That was a suggestion by Labath and I think this makes the code
simpler, as there is only one way to handle errors.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D100740/new/
https://reviews.llvm.org/D100740
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
