This is an archive of the discontinued LLVM Phabricator instance.

[lldb] fix --source-quietly
ClosedPublic

Authored by lawrence_danna on Nov 2 2021, 12:32 AM.

Details

Summary

Jim says:

lldb has a -Q or --source-quietly option, which supposedly does:

--source-quietly     Tells the debugger to execute this one-line lldb command before any file has been loaded.

That seems like a weird description, since we don't generally use source for one line entries, but anyway, let's try it:

> $LLDB_LLVM/clean-mono/build/Debug/bin/lldb -Q "script print('I should be quiet')" a.out -O "script print('I should be before')" -o "script print('I should be after')"
(lldb) script print('I should be before')
I should be before
(lldb) target create "script print('I should be quiet')"
error: unable to find executable for 'script print('I should be quiet')'

That was weird. The first real -O gets sourced but not quietly, then the argument to the -Q gets treated as the target.

> $LLDB_LLVM/clean-mono/build/Debug/bin/lldb -Q a.out -O "script print('I should be before')" -o "script print('I should be after')"
(lldb) script print('I should be before')
I should be before
(lldb) target create "a.out"
Current executable set to '/tmp/a.out' (x86_64).
(lldb) script print('I should be after')
I should be after

Well, that's a little better, but the -Q option seems to have done nothing.


This fixes the description of --source-quietly, as well as causing it
to actually suppress echoing while executing the initialization
commands.

Diff Detail

Event Timeline

lawrence_danna created this revision.Nov 2 2021, 12:32 AM
lawrence_danna requested review of this revision.Nov 2 2021, 12:32 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 2 2021, 12:32 AM

This is great, thanks!

There was one typo I pointed out, and it would be good to add a test. I'm sure this worked at some point, but wasn't tested so it was broken without anybody noticing. We want to make sure that doesn't happen again... It should be easy to write a Shell test case that just runs an lldb with the -Q option, sources a file and checks that you don't see output from it.

lldb/tools/driver/Options.td
117

filles -> files

lawrence_danna marked an inline comment as done.Nov 2 2021, 10:31 AM

There was one typo I pointed out, and it would be good to add a test. I'm sure this worked at some point, but wasn't tested so it was broken without anybody noticing. We want to make sure that doesn't happen again... It should be easy to write a Shell test case that just runs an lldb with the -Q option, sources a file and checks that you don't see output from it.

I did, it's TestQuiet.test

jingham accepted this revision.Nov 2 2021, 10:36 AM

Ha! It was apparently so easy to add the test that it wasn't noticeable (at least by me!).

This isn't directly relevant to your patch, since this wasn't behavior you changed, but it seems weird to me to suppress the commands but print their output? Is that really the behavior we want for -Q?

Anyway, that's more fuel for thought than something you need to address in this patch, where you're just making existing behavior work.

This revision is now accepted and ready to land.Nov 2 2021, 10:36 AM

Ha! It was apparently so easy to add the test that it wasn't noticeable (at least by me!).

This isn't directly relevant to your patch, since this wasn't behavior you changed, but it seems weird to me to suppress the commands but print their output? Is that really the behavior we want for -Q?

I hadn't actually thought about it, as what i care about is getting lldb -Q -o 'script print(...)' to work without any extra output, but now that you mention it the behavior here is inconsistent. For sourcing files, all output is suppressed, but for onelines only echo is suppressed.

This revision was automatically updated to reflect the committed changes.
JDevlieghere added inline comments.Nov 2 2021, 11:36 AM
lldb/test/Shell/Driver/TestQuiet.test
8

No newline at end of file

It looks like some of the tests need to be updated after this change (at least on Windows, the Windows LLDB bot is broken now):

https://lab.llvm.org/buildbot/#/builders/83/builds/11623/steps/7/logs/stdio

It looks like some of the tests need to be updated after this change (at least on Windows, the Windows LLDB bot is broken now):

https://lab.llvm.org/buildbot/#/builders/83/builds/11623/steps/7/logs/stdio

sorry! Fix is here https://reviews.llvm.org/D113047