This is an archive of the discontinued LLVM Phabricator instance.

[lldb/crashlog] Add '-t|--target' option to interactive mode
ClosedPublic

Authored by mib on Jul 12 2022, 6:15 PM.

Details

Summary

This patch introduces a new flag for the interactive crashlog mode, that
allow the user to specify, which target to use to create the scripted
process.

This can be very useful when lldb already have few targets created:
Instead of taking the first one (zeroth index), we will use that flag to
create a new target. If the user didn't provide a target path, we will rely
on the symbolicator to create a targer.If that fails and there are already
some targets loaded in lldb, we use the first one.

rdar://94682869

Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>

Diff Detail

Event Timeline

mib created this revision.Jul 12 2022, 6:15 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 12 2022, 6:15 PM
mib requested review of this revision.Jul 12 2022, 6:15 PM
JDevlieghere added inline comments.Jul 13 2022, 10:01 AM
lldb/examples/python/crashlog.py
1022
1024

I think if the user provided a target path and that didn't work, we should report and error and give up.

1025
1201

Please make it clear that this is optional and describe the fallback behavior.

lldb/examples/python/scripted_process/crashlog_scripted_process.py
46–47

Maybe leave a comment why the executable matters here rather than just the target.

47–48

The comment says return error but we're not returning anything. Should this raise an exception?

mib marked 5 inline comments as done.Jul 28 2022, 2:02 PM
mib added inline comments.
lldb/examples/python/scripted_process/crashlog_scripted_process.py
47–48

I was thinking of adding a SBCommandObjectReturn to every script process init function so we print errors in lldb instead of raising a python exception. That way we could propage that into the IDEs.
I'm doing that in a follow-up patch.

mib updated this revision to Diff 448434.Jul 28 2022, 2:02 PM
mib edited the summary of this revision. (Show Details)

Address @JDevlieghere feedbacks.

mib updated this revision to Diff 449495.Aug 2 2022, 5:30 PM

Make lit run command more readable

JDevlieghere added inline comments.Aug 3 2022, 11:09 AM
lldb/examples/python/crashlog.py
1025

This is not an f-string so this will actually print {option.target_path} rather than the actual value. That said, we don't use those anywhere else, so let's use .format() to be consistent.

lldb/test/Shell/ScriptInterpreter/Python/Crashlog/scripted_crashlog_json.test
7

Can we test some of the error cases here? Should be easy enough to provide a target that doesn't exist (and would've caught the f-string issue).

mib marked an inline comment as done.Aug 3 2022, 11:43 AM
mib added inline comments.
lldb/examples/python/crashlog.py
1025

.format() was too long compared to using an f-string :p that why I went with that ... I'll update it

lldb/test/Shell/ScriptInterpreter/Python/Crashlog/scripted_crashlog_json.test
7

I tried having a first crashlog command with an invalid target fail before the one that works but lit would just stop at the error. Instead, I test it in D129614.

mib marked 3 inline comments as done.Aug 3 2022, 2:08 PM
mib updated this revision to Diff 449782.Aug 3 2022, 2:12 PM

Change f-string to string.format

JDevlieghere accepted this revision.Aug 8 2022, 11:52 AM

LGTM modulo the # Return error but that's addressed in a follow up patch.

This revision is now accepted and ready to land.Aug 8 2022, 11:52 AM
This revision was automatically updated to reflect the committed changes.