This is an archive of the discontinued LLVM Phabricator instance.

[trace] Add LoadTraceFromFile to SBDebugger and SBTrace
ClosedPublic

Authored by jj10306 on Jun 17 2022, 4:20 PM.

Details

Summary

Before we add bindings for the TraceCursor to allow for programatic traversal of traces, we need to bindings to load a trace from a trace session file.
This diff adds trace load functionality to SBDebugger via the LoadTraceFromFile method and updates the intelpt test case class to have testTraceLoad method so we can take advantage of the testApiAndSB decorator to test both the CLI and SB without duplicating code.

Diff Detail

Event Timeline

jj10306 created this revision.Jun 17 2022, 4:20 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 17 2022, 4:21 PM
jj10306 requested review of this revision.Jun 17 2022, 4:21 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 17 2022, 4:21 PM
jj10306 edited the summary of this revision. (Show Details)Jun 17 2022, 4:23 PM
jj10306 retitled this revision from Add LoadTraceFromFile to SBDebugger and SBTrace to [trace] Add LoadTraceFromFile to SBDebugger and SBTrace.Jun 17 2022, 4:23 PM
JDevlieghere added inline comments.Jun 17 2022, 6:58 PM
lldb/include/lldb/API/SBDebugger.h
403

Can this take a filespec?

wallace requested changes to this revision.Jun 18 2022, 2:32 PM

just some minor changes, including requiring the FileSpec as input to the SB function

lldb/include/lldb/API/SBDebugger.h
394–395
403

+1

403

We need to come up with a good public name for the json file. I've been using trace session file, but I dislike. What about trace description file? Then we can have a "trace bundle" that contains "trace description file". Do you have better suggestions?

lldb/include/lldb/API/SBTrace.h
15

+1

24–41

you can also refer to the documentation of the other function if you don't want to repeat yourself

lldb/source/API/SBTrace.cpp
32

you'll need LLDB_INSTRUMENT_VA(this, error, debugger, trace_session_file_path). This LLDB_INSTRUMENT_VA macro is used for diagnosing test failures on some systems. I haven't used this diagnostics systems myself but let's adhere to the convention

33–38

if you want, add using namespace llvm so that you don't have to write llvm:: everywhere. You can also omit lldb:: already

lldb/source/Commands/CommandObjectTrace.cpp
92–105

nice refactor

102

you can drop the trace_sp check because it should be valid because otherwise you'd have gotten an Expected

lldb/source/Target/Trace.cpp
92–114

you can drop all llvm:: and lldb:: if you want

lldb/test/API/commands/trace/TestTraceLoad.py
12

thanks!

102

split into multiple lines

This revision now requires changes to proceed.Jun 18 2022, 2:32 PM
jj10306 updated this revision to Diff 438400.EditedJun 20 2022, 7:53 AM
jj10306 marked 12 inline comments as done.

Address comments
Note: I changed "trace session file" to "trace description file" in the new methods, but saving the project wide rename/adoption of "trace description file" for a separate diff as to not cloud the purpose of this diff.

jj10306 added inline comments.Jun 20 2022, 8:00 AM
lldb/include/lldb/API/SBDebugger.h
403

I think trace description file is a more appropriate name as well.

lldb/packages/Python/lldbsuite/test/tools/intelpt/intelpt_testcase.py
139

the single arg constructor is deprecated, update this to pass resolve flag

wallace accepted this revision.Jun 20 2022, 11:13 AM

nice job!

lldb/packages/Python/lldbsuite/test/tools/intelpt/intelpt_testcase.py
139

i don't think it's deprecated, but you should pass the resolve flag anyone

This revision is now accepted and ready to land.Jun 20 2022, 11:13 AM
jj10306 updated this revision to Diff 438449.Jun 20 2022, 11:28 AM
jj10306 marked 2 inline comments as done.

rebase and use SBFileSpec constructor with that requires an explicit resolve flag before landing

lldb/packages/Python/lldbsuite/test/tools/intelpt/intelpt_testcase.py
139

You can find the deprecation warning in the comments here:
https://lldb.llvm.org/cpp_reference/SBFileSpec_8cpp_source.html

wallace added inline comments.Jun 20 2022, 11:35 AM
lldb/packages/Python/lldbsuite/test/tools/intelpt/intelpt_testcase.py
139

TIL!

This revision was automatically updated to reflect the committed changes.