This is an archive of the discontinued LLVM Phabricator instance.

[trace] Add a new call graph reconstructor
Needs ReviewPublic

Authored by wallace on Nov 7 2022, 11:44 PM.

Details

Summary

The previous reconstructor was flawed and served well as prototype, but
this new one is meant to stay for a while. Some features

  • Strictly typing of the different variants of calls (functions, inlined functions, symbols, etc).
  • Special handling of inline function, including inlined functions that have no instructions of their own
  • Better formatting for the dumper
  • Extensible code structure. It's easy to create new call types and special handlers based on instruction types and symbol information
  • Added better tests

As a note, the latest version of libipt included a new instruction kind "indirect" with is either an indirect jump or a call that libipt can't distinguish better. It used to be signaled as an error instruction but not anymore (see https://github.com/intel/libipt/commit/27513274c897851e15a46bb19a3861219d87c6cb). Besides that, the LLVM disassembler fails to disassembly some functions that libipt doesn't. I imagine Intel is doing a better job than LLVM at disassembling x86 instructions. So I decided to use the instruction kinds from libipt until we improve the disassembler.

In a following diff I'll add a JSON dumper for the graph.

Diff Detail

Event Timeline

wallace created this revision.Nov 7 2022, 11:44 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 7 2022, 11:44 PM
Herald added a subscriber: pengfei. · View Herald Transcript
wallace requested review of this revision.Nov 7 2022, 11:44 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 7 2022, 11:44 PM
jj10306 added inline comments.Nov 8 2022, 10:09 AM
lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
296

do we need to store this or can this information be calculated on demand? just thinking about massive traces and want to make sure we're intentional about only storing what we absolutely need to for each instruction

lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
317

what's the reason for this change?

wallace added inline comments.Nov 8 2022, 10:38 AM
lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
296

I thought about it and it will help us have more correctness for now and a little bit more speed with little memory cost. The total size per instruction went from 9 bytes to 11, which is very decent.

Now some details: the Disassembler fails with some instructions. I was tracing dynolog and found many instances of the disassembler failing, but libipt doesn't fail. In other words, we can't get the control flow kind from LLDB but we can from libipt. Same for the byte size. That means that we need to fix the disassembler, but that will require a lot of work that I can hardly prioritize, so I'd rather use libipt for now.

lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
317

wtf, i think this is from a previous commit i never intended to publish. I was playing with some stuff. I need to revert this