[Lldb-commits] [PATCH] D103349: [lldb] Don't print script output twice in HandleCommand

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: teemperor, labath, mib, jingham.
Herald added a subscriber: pengfei.
JDevlieghere requested review of this revision.

When executing a `script` command in `HandleCommand(s)` we currently print its output twice: once directly to the debugger's output stream, and once as part of `HandleCommand`'s result. You can see this issue in action when adding a breakpoint command:

  (lldb) b main
  Breakpoint 1: where = main.out`main + 13 at main.cpp:2:3, address = 0x0000000100003fad
  (lldb) break command add 1 -o "script print(\"Hey!\")"
  (lldb) r
  Process 76041 launched: '/tmp/main.out' (x86_64)
  Hey!
  (lldb)  script print("Hey!")
  Hey!
  
  Process 76041 stopped
  * thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
      frame #0: 0x0000000100003fad main.out`main at main.cpp:2:3

The issue is that `HandleCommands` uses a temporary `CommandReturnObject`. When running a one-liner with `script`, the script interpreter redirects the I/O from to the command object through `ScriptInterpreterIORedirect` which sets the immediate output on the temporary result and causes the result to be printed the first time. HandleCommand will then copy the result's output into its own result and print it a second time, not knowing that it has already been printed.

I'm not entirely sure why `ScriptInterpreterIORedirect` relies on an immediate output file, but there are several parts of LLDB that rely on it, so I'm hesitant to change that. We could check in `HandleCommands` if someone set immediate mode on our temporary result and not copy it into the final result when that's the case. A better solution in my opinion, which I implemented in this patch, is a "hermetic" mode for the `CommandReturnObject` which prevents anyone from setting and immediate stream on the temporary result by the command interpreter. This should prevent similar bugs if there are other places that try to do this.

I added a new test using the breakpoint command and fixed a Lua test that already suffered from this issue.


https://reviews.llvm.org/D103349

Files:
  lldb/include/lldb/Interpreter/CommandReturnObject.h
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Interpreter/CommandReturnObject.cpp
  lldb/test/Shell/Breakpoint/breakpoint-command.test
  lldb/test/Shell/ScriptInterpreter/Lua/nested_sessions.test

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D103349.348603.patch
Type: text/x-patch
Size: 4209 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20210529/ca2a6b5a/attachment.bin>