Author: royitaqi Date: 2024-06-03T13:52:03-07:00 New Revision: c2d061da7e17e61d4a0efad261e5280793c1b7ce
URL: https://github.com/llvm/llvm-project/commit/c2d061da7e17e61d4a0efad261e5280793c1b7ce DIFF: https://github.com/llvm/llvm-project/commit/c2d061da7e17e61d4a0efad261e5280793c1b7ce.diff LOG: Re-merge `A few updates around "transcript"` (#92843) (#94067) Problematic PR: https://github.com/llvm/llvm-project/pull/92843 Reverted by: https://github.com/llvm/llvm-project/pull/94088 The first PR added a test which fails in Linux builds (see the last few comments there). This PR contains all the changes in the first PR, plus the fix to the said test. --------- Co-authored-by: Roy Shi <roy...@meta.com> Added: Modified: lldb/include/lldb/API/SBCommandInterpreter.h lldb/include/lldb/Interpreter/CommandInterpreter.h lldb/include/lldb/Target/Statistics.h lldb/source/Commands/CommandObjectStats.cpp lldb/source/Commands/Options.td lldb/source/Interpreter/CommandInterpreter.cpp lldb/source/Target/Statistics.cpp lldb/test/API/commands/statistics/basic/TestStats.py lldb/test/API/python_api/interpreter/TestCommandInterpreterAPI.py Removed: ################################################################################ diff --git a/lldb/include/lldb/API/SBCommandInterpreter.h b/lldb/include/lldb/API/SBCommandInterpreter.h index 8ac36344b3a79..639309aa32bfc 100644 --- a/lldb/include/lldb/API/SBCommandInterpreter.h +++ b/lldb/include/lldb/API/SBCommandInterpreter.h @@ -320,10 +320,17 @@ class SBCommandInterpreter { /// Returns a list of handled commands, output and error. Each element in /// the list is a dictionary with the following keys/values: - /// - "command" (string): The command that was executed. + /// - "command" (string): The command that was given by the user. + /// - "commandName" (string): The name of the executed command. + /// - "commandArguments" (string): The arguments of the executed command. /// - "output" (string): The output of the command. Empty ("") if no output. /// - "error" (string): The error of the command. Empty ("") if no error. - /// - "seconds" (float): The time it took to execute the command. + /// - "durationInSeconds" (float): The time it took to execute the command. + /// - "timestampInEpochSeconds" (int): The timestamp when the command is + /// executed. + /// + /// Turn on settings `interpreter.save-transcript` for LLDB to populate + /// this list. Otherwise this list is empty. SBStructuredData GetTranscript(); protected: diff --git a/lldb/include/lldb/Interpreter/CommandInterpreter.h b/lldb/include/lldb/Interpreter/CommandInterpreter.h index ccc30cf4f1a82..8863523b2e31f 100644 --- a/lldb/include/lldb/Interpreter/CommandInterpreter.h +++ b/lldb/include/lldb/Interpreter/CommandInterpreter.h @@ -776,10 +776,14 @@ class CommandInterpreter : public Broadcaster, /// Contains a list of handled commands and their details. Each element in /// the list is a dictionary with the following keys/values: - /// - "command" (string): The command that was executed. + /// - "command" (string): The command that was given by the user. + /// - "commandName" (string): The name of the executed command. + /// - "commandArguments" (string): The arguments of the executed command. /// - "output" (string): The output of the command. Empty ("") if no output. /// - "error" (string): The error of the command. Empty ("") if no error. - /// - "seconds" (float): The time it took to execute the command. + /// - "durationInSeconds" (float): The time it took to execute the command. + /// - "timestampInEpochSeconds" (int): The timestamp when the command is + /// executed. /// /// Turn on settings `interpreter.save-transcript` for LLDB to populate /// this list. Otherwise this list is empty. diff --git a/lldb/include/lldb/Target/Statistics.h b/lldb/include/lldb/Target/Statistics.h index c4f17b503a1f9..c04d529290fff 100644 --- a/lldb/include/lldb/Target/Statistics.h +++ b/lldb/include/lldb/Target/Statistics.h @@ -133,6 +133,7 @@ struct ConstStringStats { struct StatisticsOptions { bool summary_only = false; bool load_all_debug_info = false; + bool include_transcript = false; }; /// A class that represents statistics for a since lldb_private::Target. diff --git a/lldb/source/Commands/CommandObjectStats.cpp b/lldb/source/Commands/CommandObjectStats.cpp index a92bb5d1165ee..1935b0fdfadfb 100644 --- a/lldb/source/Commands/CommandObjectStats.cpp +++ b/lldb/source/Commands/CommandObjectStats.cpp @@ -81,6 +81,9 @@ class CommandObjectStatsDump : public CommandObjectParsed { case 'f': m_stats_options.load_all_debug_info = true; break; + case 't': + m_stats_options.include_transcript = true; + break; default: llvm_unreachable("Unimplemented option"); } diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td index 62bbfdc117834..194902abdce49 100644 --- a/lldb/source/Commands/Options.td +++ b/lldb/source/Commands/Options.td @@ -1425,4 +1425,8 @@ let Command = "statistics dump" in { Desc<"Dump the total possible debug info statistics. " "Force loading all the debug information if not yet loaded, and collect " "statistics with those.">; + def statistics_dump_transcript: Option<"transcript", "t">, Group<1>, + Desc<"If the setting interpreter.save-transcript is enabled and this " + "option is specified, include a JSON array with all commands the user and/" + "or scripts executed during a debug session.">; } diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp index 6a61882df093d..acd6294cb3f42 100644 --- a/lldb/source/Interpreter/CommandInterpreter.cpp +++ b/lldb/source/Interpreter/CommandInterpreter.cpp @@ -6,6 +6,7 @@ // //===----------------------------------------------------------------------===// +#include <chrono> #include <cstdlib> #include <limits> #include <memory> @@ -1909,6 +1910,11 @@ bool CommandInterpreter::HandleCommand(const char *command_line, transcript_item = std::make_shared<StructuredData::Dictionary>(); transcript_item->AddStringItem("command", command_line); + transcript_item->AddIntegerItem( + "timestampInEpochSeconds", + std::chrono::duration_cast<std::chrono::seconds>( + std::chrono::system_clock::now().time_since_epoch()) + .count()); m_transcript.AddItem(transcript_item); } @@ -2056,6 +2062,14 @@ bool CommandInterpreter::HandleCommand(const char *command_line, log, "HandleCommand, command line after removing command name(s): '%s'", remainder.c_str()); + // To test whether or not transcript should be saved, `transcript_item` is + // used instead of `GetSaveTrasncript()`. This is because the latter will + // fail when the command is "settings set interpreter.save-transcript true". + if (transcript_item) { + transcript_item->AddStringItem("commandName", cmd_obj->GetCommandName()); + transcript_item->AddStringItem("commandArguments", remainder); + } + ElapsedTime elapsed(execute_time); cmd_obj->Execute(remainder.c_str(), result); } @@ -2072,7 +2086,8 @@ bool CommandInterpreter::HandleCommand(const char *command_line, transcript_item->AddStringItem("output", result.GetOutputData()); transcript_item->AddStringItem("error", result.GetErrorData()); - transcript_item->AddFloatItem("seconds", execute_time.get().count()); + transcript_item->AddFloatItem("durationInSeconds", + execute_time.get().count()); } return result.Succeeded(); diff --git a/lldb/source/Target/Statistics.cpp b/lldb/source/Target/Statistics.cpp index 7f866ae0ef324..be0848573f812 100644 --- a/lldb/source/Target/Statistics.cpp +++ b/lldb/source/Target/Statistics.cpp @@ -16,6 +16,7 @@ #include "lldb/Target/Process.h" #include "lldb/Target/Target.h" #include "lldb/Target/UnixSignals.h" +#include "lldb/Utility/StructuredData.h" using namespace lldb; using namespace lldb_private; @@ -225,6 +226,7 @@ llvm::json::Value DebuggerStats::ReportStatistics( const bool summary_only = options.summary_only; const bool load_all_debug_info = options.load_all_debug_info; + const bool include_transcript = options.include_transcript; json::Array json_targets; json::Array json_modules; @@ -364,5 +366,36 @@ llvm::json::Value DebuggerStats::ReportStatistics( global_stats.try_emplace("commands", std::move(cmd_stats)); } + if (include_transcript) { + // When transcript is available, add it to the to-be-returned statistics. + // + // NOTE: + // When the statistics is polled by an LLDB command: + // - The transcript in the returned statistics *will NOT* contain the + // returned statistics itself (otherwise infinite recursion). + // - The returned statistics *will* be written to the internal transcript + // buffer. It *will* appear in the next statistcs or transcript poll. + // + // For example, let's say the following commands are run in order: + // - "version" + // - "statistics dump" <- call it "A" + // - "statistics dump" <- call it "B" + // The output of "A" will contain the transcript of "version" and + // "statistics dump" (A), with the latter having empty output. The output + // of B will contain the trascnript of "version", "statistics dump" (A), + // "statistics dump" (B), with A's output populated and B's output empty. + const StructuredData::Array &transcript = + debugger.GetCommandInterpreter().GetTranscript(); + if (transcript.GetSize() != 0) { + std::string buffer; + llvm::raw_string_ostream ss(buffer); + json::OStream json_os(ss); + transcript.Serialize(json_os); + if (auto json_transcript = llvm::json::parse(ss.str())) + global_stats.try_emplace("transcript", + std::move(json_transcript.get())); + } + } + return std::move(global_stats); } diff --git a/lldb/test/API/commands/statistics/basic/TestStats.py b/lldb/test/API/commands/statistics/basic/TestStats.py index fb6fc07d2d443..ebe6166eb45e5 100644 --- a/lldb/test/API/commands/statistics/basic/TestStats.py +++ b/lldb/test/API/commands/statistics/basic/TestStats.py @@ -623,3 +623,50 @@ def test_had_frame_variable_errors(self): # Verify that the top level statistic that aggregates the number of # modules with debugInfoHadVariableErrors is greater than zero self.assertGreater(stats["totalModuleCountWithVariableErrors"], 0) + + def test_transcript_happy_path(self): + """ + Test "statistics dump" and the transcript information. + """ + self.build() + exe = self.getBuildArtifact("a.out") + target = self.createTestTarget(file_path=exe) + self.runCmd("settings set interpreter.save-transcript true") + self.runCmd("version") + + # Verify the output of a first "statistics dump" + debug_stats = self.get_stats("--transcript") + self.assertIn("transcript", debug_stats) + transcript = debug_stats["transcript"] + self.assertEqual(len(transcript), 2) + self.assertEqual(transcript[0]["commandName"], "version") + self.assertEqual(transcript[1]["commandName"], "statistics dump") + # The first "statistics dump" in the transcript should have no output + self.assertNotIn("output", transcript[1]) + + # Verify the output of a second "statistics dump" + debug_stats = self.get_stats("--transcript") + self.assertIn("transcript", debug_stats) + transcript = debug_stats["transcript"] + self.assertEqual(len(transcript), 3) + self.assertEqual(transcript[0]["commandName"], "version") + self.assertEqual(transcript[1]["commandName"], "statistics dump") + # The first "statistics dump" in the transcript should have output now + self.assertIn("output", transcript[1]) + self.assertEqual(transcript[2]["commandName"], "statistics dump") + # The second "statistics dump" in the transcript should have no output + self.assertNotIn("output", transcript[2]) + + def test_transcript_should_not_exist_when_not_asked_for(self): + """ + Test "statistics dump" and the transcript information. + """ + self.build() + exe = self.getBuildArtifact("a.out") + target = self.createTestTarget(file_path=exe) + self.runCmd("settings set interpreter.save-transcript true") + self.runCmd("version") + + # Verify the output of a first "statistics dump" + debug_stats = self.get_stats() # Not with "--transcript" + self.assertNotIn("transcript", debug_stats) diff --git a/lldb/test/API/python_api/interpreter/TestCommandInterpreterAPI.py b/lldb/test/API/python_api/interpreter/TestCommandInterpreterAPI.py index 95643eef0d344..1029bdc3096d0 100644 --- a/lldb/test/API/python_api/interpreter/TestCommandInterpreterAPI.py +++ b/lldb/test/API/python_api/interpreter/TestCommandInterpreterAPI.py @@ -104,9 +104,10 @@ def getTranscriptAsPythonObject(self, ci): return json.loads(stream.GetData()) - def test_structured_transcript(self): + def test_get_transcript(self): """Test structured transcript generation and retrieval.""" ci = self.buildAndCreateTarget() + self.assertTrue(ci, VALID_COMMAND_INTERPRETER) # Make sure the "save-transcript" setting is on self.runCmd("settings set interpreter.save-transcript true") @@ -118,8 +119,7 @@ def test_structured_transcript(self): res = lldb.SBCommandReturnObject() ci.HandleCommand("version", res) ci.HandleCommand("an-unknown-command", res) - ci.HandleCommand("breakpoint set -f main.c -l %d" % self.line, res) - ci.HandleCommand("r", res) + ci.HandleCommand("br s -f main.c -l %d" % self.line, res) ci.HandleCommand("p a", res) ci.HandleCommand("statistics dump", res) total_number_of_commands = 6 @@ -130,22 +130,28 @@ def test_structured_transcript(self): # All commands should have expected fields. for command in transcript: self.assertIn("command", command) + # Unresolved commands don't have "commandName"/"commandArguments". + # We will validate these fields below, instead of here. self.assertIn("output", command) self.assertIn("error", command) - self.assertIn("seconds", command) + self.assertIn("durationInSeconds", command) + self.assertIn("timestampInEpochSeconds", command) # The following validates individual commands in the transcript. # # Notes: # 1. Some of the asserts rely on the exact output format of the # commands. Hopefully we are not changing them any time soon. - # 2. We are removing the "seconds" field from each command, so that - # some of the validations below can be easier / more readable. + # 2. We are removing the time-related fields from each command, so + # that some of the validations below can be easier / more readable. for command in transcript: - del(command["seconds"]) + del command["durationInSeconds"] + del command["timestampInEpochSeconds"] # (lldb) version self.assertEqual(transcript[0]["command"], "version") + self.assertEqual(transcript[0]["commandName"], "version") + self.assertEqual(transcript[0]["commandArguments"], "") self.assertIn("lldb version", transcript[0]["output"]) self.assertEqual(transcript[0]["error"], "") @@ -153,33 +159,37 @@ def test_structured_transcript(self): self.assertEqual(transcript[1], { "command": "an-unknown-command", + # Unresolved commands don't have "commandName"/"commandArguments" "output": "", "error": "error: 'an-unknown-command' is not a valid command.\n", }) - # (lldb) breakpoint set -f main.c -l <line> - self.assertEqual(transcript[2]["command"], "breakpoint set -f main.c -l %d" % self.line) + # (lldb) br s -f main.c -l <line> + self.assertEqual(transcript[2]["command"], "br s -f main.c -l %d" % self.line) + self.assertEqual(transcript[2]["commandName"], "breakpoint set") + self.assertEqual( + transcript[2]["commandArguments"], "-f main.c -l %d" % self.line + ) # Breakpoint 1: where = a.out`main + 29 at main.c:5:3, address = 0x0000000100000f7d self.assertIn("Breakpoint 1: where = a.out`main ", transcript[2]["output"]) self.assertEqual(transcript[2]["error"], "") - # (lldb) r - self.assertEqual(transcript[3]["command"], "r") - # Process 25494 launched: '<path>/TestCommandInterpreterAPI.test_structured_transcript/a.out' (x86_64) - self.assertIn("Process", transcript[3]["output"]) - self.assertIn("launched", transcript[3]["output"]) - self.assertEqual(transcript[3]["error"], "") - # (lldb) p a - self.assertEqual(transcript[4], + self.assertEqual(transcript[3], { "command": "p a", - "output": "(int) 123\n", - "error": "", + "commandName": "dwim-print", + "commandArguments": "-- a", + "output": "", + "error": "error: <user expression 0>:1:1: use of undeclared identifier 'a'\n 1 | a\n | ^\n", }) # (lldb) statistics dump - statistics_dump = json.loads(transcript[5]["output"]) + self.assertEqual(transcript[4]["command"], "statistics dump") + self.assertEqual(transcript[4]["commandName"], "statistics dump") + self.assertEqual(transcript[4]["commandArguments"], "") + self.assertEqual(transcript[4]["error"], "") + statistics_dump = json.loads(transcript[4]["output"]) # Dump result should be valid JSON self.assertTrue(statistics_dump is not json.JSONDecodeError) # Dump result should contain expected fields @@ -189,15 +199,15 @@ def test_structured_transcript(self): self.assertIn("targets", statistics_dump) def test_save_transcript_setting_default(self): - ci = self.buildAndCreateTarget() - res = lldb.SBCommandReturnObject() + ci = self.dbg.GetCommandInterpreter() + self.assertTrue(ci, VALID_COMMAND_INTERPRETER) # The setting's default value should be "false" self.runCmd("settings show interpreter.save-transcript", "interpreter.save-transcript (boolean) = false\n") - # self.assertEqual(res.GetOutput(), ) def test_save_transcript_setting_off(self): - ci = self.buildAndCreateTarget() + ci = self.dbg.GetCommandInterpreter() + self.assertTrue(ci, VALID_COMMAND_INTERPRETER) # Make sure the setting is off self.runCmd("settings set interpreter.save-transcript false") @@ -208,8 +218,8 @@ def test_save_transcript_setting_off(self): self.assertEqual(transcript, []) def test_save_transcript_setting_on(self): - ci = self.buildAndCreateTarget() - res = lldb.SBCommandReturnObject() + ci = self.dbg.GetCommandInterpreter() + self.assertTrue(ci, VALID_COMMAND_INTERPRETER) # Make sure the setting is on self.runCmd("settings set interpreter.save-transcript true") @@ -220,7 +230,7 @@ def test_save_transcript_setting_on(self): self.assertEqual(len(transcript), 1) self.assertEqual(transcript[0]["command"], "version") - def test_save_transcript_returns_copy(self): + def test_get_transcript_returns_copy(self): """ Test that the returned structured data is *at least* a shallow copy. @@ -229,7 +239,8 @@ def test_save_transcript_returns_copy(self): because there is no logic in the command interpreter to modify a transcript item (representing a command) after it has been returned. """ - ci = self.buildAndCreateTarget() + ci = self.dbg.GetCommandInterpreter() + self.assertTrue(ci, VALID_COMMAND_INTERPRETER) # Make sure the setting is on self.runCmd("settings set interpreter.save-transcript true") _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
