[Lldb-commits] [PATCH] D27459: Straw-man proposal for new logging syntax
labath updated this revision to Diff 80744. labath added a comment. Update the example to the formatv-based API https://reviews.llvm.org/D27459 Files: include/lldb/Core/Error.h include/lldb/Core/Log.h include/lldb/Host/FileSpec.h source/Plugins/Process/Linux/NativeProcessLinux.cpp Index: source/Plugins/Process/Linux/NativeProcessLinux.cpp === --- source/Plugins/Process/Linux/NativeProcessLinux.cpp +++ source/Plugins/Process/Linux/NativeProcessLinux.cpp @@ -41,6 +41,7 @@ #include "lldb/Utility/LLDBAssert.h" #include "lldb/Utility/PseudoTerminal.h" #include "lldb/Utility/StringExtractor.h" +#include "llvm/Support/FormatVariadic.h" #include "NativeThreadLinux.h" #include "Plugins/Process/POSIX/ProcessPOSIXLog.h" @@ -72,6 +73,11 @@ using namespace lldb_private::process_linux; using namespace llvm; +// Obviously, these would not live in this file. +llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, Error &error) { + return OS << error.AsCString(); +} + // Private bits we only need internally. static bool ProcessVmReadvSupported() { @@ -93,16 +99,13 @@ // value from our own process. ssize_t res = process_vm_readv(getpid(), &local, 1, &remote, 1, 0); is_supported = (res == sizeof(source) && source == dest); -if (log) { - if (is_supported) -log->Printf("%s: Detected kernel support for process_vm_readv syscall. " -"Fast memory reads enabled.", -__FUNCTION__); - else -log->Printf("%s: syscall process_vm_readv failed (error: %s). Fast " -"memory reads disabled.", -__FUNCTION__, strerror(errno)); -} +if (is_supported) + LLDB_LOG(log, "Detected kernel support for process_vm_readv syscall. " +"Fast memory reads enabled."); +else + LLDB_LOG(log, "syscall process_vm_readv failed (error: {0}). Fast memory " +"reads disabled.", + strerror(errno)); }); return is_supported; @@ -115,16 +118,14 @@ return; if (const FileAction *action = info.GetFileActionForFD(STDIN_FILENO)) -log->Printf("%s: setting STDIN to '%s'", __FUNCTION__, -action->GetFileSpec().GetCString()); +LLDB_LOG(log, "setting STDIN to {0}", action->GetFileSpec()); else -log->Printf("%s leaving STDIN as is", __FUNCTION__); +LLDB_LOG(log, "leaving STDIN as is"); if (const FileAction *action = info.GetFileActionForFD(STDOUT_FILENO)) -log->Printf("%s setting STDOUT to '%s'", __FUNCTION__, -action->GetFileSpec().GetCString()); +LLDB_LOG(log, "setting STDOUT to '{0}'", action->GetFileSpec()); else -log->Printf("%s leaving STDOUT as is", __FUNCTION__); +LLDB_LOG(log, "leaving STDOUT as is"); if (const FileAction *action = info.GetFileActionForFD(STDERR_FILENO)) log->Printf("%s setting STDERR to '%s'", __FUNCTION__, @@ -135,8 +136,7 @@ int i = 0; for (const char **args = info.GetArguments().GetConstArgumentVector(); *args; ++args, ++i) -log->Printf("%s arg %d: \"%s\"", __FUNCTION__, i, -*args ? *args : "nullptr"); +LLDB_LOG(log, "arg {0}: \"{1}\"", i, *args); } void DisplayBytes(StreamString &s, void *bytes, uint32_t count) { @@ -256,9 +256,7 @@ if (error.Fail()) { native_process_sp.reset(); -if (log) - log->Printf("NativeProcessLinux::%s failed to launch process: %s", - __FUNCTION__, error.AsCString()); +LLDB_LOG(log, "failed to launch process: ", error); return error; } @@ -308,9 +306,7 @@ void NativeProcessLinux::AttachToInferior(MainLoop &mainloop, lldb::pid_t pid, Error &error) { Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS)); - if (log) -log->Printf("NativeProcessLinux::%s (pid = %" PRIi64 ")", __FUNCTION__, -pid); + LLDB_LOG(log, "(pid = {0})", pid); m_sigchld_handle = mainloop.RegisterSignal( SIGCHLD, [this](MainLoopBase &) { SigchldHandler(); }, error); @@ -322,10 +318,8 @@ return; // Set the architecture to the exe architecture. - if (log) -log->Printf("NativeProcessLinux::%s (pid = %" PRIi64 -") detected architecture %s", -__FUNCTION__, pid, m_arch.GetArchitectureName()); + LLDB_LOG(log, "(pid = {0}) detected architecture {1}", pid, + m_arch.GetArchitectureName()); m_pid = pid; SetState(eStateAttaching); @@ -357,9 +351,7 @@ int status; if ((wpid = waitpid(pid, &status, 0)) < 0) { error.SetErrorToErrno(); -if (log) - log->Printf("NativeProcessLinux::%s waitpid for inferior failed with %s", - __FUNCTION__, error.AsCString()); +LLDB_LOG(log, "waitpid for inferior failed with: ", error); // Mark the inferior as invalid. // FIXME this could really use a new state - eStateLaunchFa
[Lldb-commits] [PATCH] D27459: Straw-man proposal for new logging syntax
clayborg added a comment. Seems weird that we are a C++ codebase and we fall back to C macros. We currently need the macro only for file + line and to also not have to worry about doing "if (log) log->". I am not a big fan of macros, but I am open to it if everyone else wants it. Comment at: include/lldb/Core/Log.h:218-219 + << llvm::formatv( \ + "{0,-60}: ", \ + (llvm::sys::path::filename(__FILE__) + "::" + __FUNCTION__) \ + .str()) \ This is hard coding in the file + line all the time. I would like this to be an option. We also need to be able to optionally enable the pid/tid, thread name, stack backtrace and all the other options that are currently supported by "log enable". Seems like we need a log function that exists in Log.cpp to take a log mutex, add the extra log prefix stuff (file + line, pid/tid, thread name, timestamp, delta since last log time, stack etc) and then call the llvm::formatv() one or more times and then release the mutex. Comment at: include/lldb/Host/FileSpec.h:675-677 + void format(llvm::raw_ostream &Stream, llvm::StringRef Options) const { +Stream << this->GetCString(); + } For a good example of what objects can do to help sell this proposal we should handle different values for the Options string. Maybe this to support the following: Options is empty -> print out full path Options == "filename" -> print out file name only with extension Options == "basename" -> print out file name only without extension Options == "directory" -> print out file directory only Options == "extension" -> print out file extension only Options == "volume" -> print out file volume only which will probably only work on windows paths currently Options == "fullname" -> print out volume + directory + filename, same as default empty string Then we can format like: ``` FileSpec file("/tmp/foo.txt"); llvm::formatv("{0; filename}", file); ``` https://reviews.llvm.org/D27459 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D27459: Straw-man proposal for new logging syntax
Personally I'm not really a fan of source/line information at all. There is only a very small probability that anyone using a released LLDB (e.g. through Xcode) is going to have their log lines match up with ToT, because we're changing files all the time. You change one thing on line 10, and now every other log line that comes after it, potentially tens of thousands of lines in the same file, are going to have their line information out of sync. On Thu, Dec 8, 2016 at 9:50 AM Greg Clayton via Phabricator < revi...@reviews.llvm.org> wrote: > clayborg added a comment. > > Seems weird that we are a C++ codebase and we fall back to C macros. We > currently need the macro only for file + line and to also not have to worry > about doing "if (log) log->". I am not a big fan of macros, but I am open > to it if everyone else wants it. > > > > > Comment at: include/lldb/Core/Log.h:218-219 > + << llvm::formatv( > \ > + "{0,-60}: ", >\ > + (llvm::sys::path::filename(__FILE__) + "::" + > __FUNCTION__) \ > + .str()) > \ > > This is hard coding in the file + line all the time. I would like this to > be an option. We also need to be able to optionally enable the pid/tid, > thread name, stack backtrace and all the other options that are currently > supported by "log enable". Seems like we need a log function that exists in > Log.cpp to take a log mutex, add the extra log prefix stuff (file + line, > pid/tid, thread name, timestamp, delta since last log time, stack etc) and > then call the llvm::formatv() one or more times and then release the mutex. > > > > Comment at: include/lldb/Host/FileSpec.h:675-677 > + void format(llvm::raw_ostream &Stream, llvm::StringRef Options) const { > +Stream << this->GetCString(); > + } > > For a good example of what objects can do to help sell this proposal we > should handle different values for the Options string. Maybe this to > support the following: > > Options is empty -> print out full path > Options == "filename" -> print out file name only with extension > Options == "basename" -> print out file name only without extension > Options == "directory" -> print out file directory only > Options == "extension" -> print out file extension only > Options == "volume" -> print out file volume only which will probably only > work on windows paths currently > Options == "fullname" -> print out volume + directory + filename, same as > default empty string > > Then we can format like: > > > ``` > FileSpec file("/tmp/foo.txt"); > llvm::formatv("{0; filename}", file); > ``` > > > https://reviews.llvm.org/D27459 > > > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r289100 - Fixed a crasher that has been borking out heap for a long time.
Author: gclayton Date: Thu Dec 8 14:38:19 2016 New Revision: 289100 URL: http://llvm.org/viewvc/llvm-project?rev=289100&view=rev Log: Fixed a crasher that has been borking out heap for a long time. ThreadList had an assignment operator that didn't lock the "rhs" thread list object. This means a thread list can be mutated while it is being copied. The copy constructor calls the assignment operator as well. So this fixes the unsafe threaded access to ThreadList which we believe is responsible for a lot of crashes. Modified: lldb/trunk/include/lldb/Target/ThreadCollection.h lldb/trunk/include/lldb/Target/ThreadList.h lldb/trunk/source/Target/ThreadList.cpp Modified: lldb/trunk/include/lldb/Target/ThreadCollection.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/ThreadCollection.h?rev=289100&r1=289099&r2=289100&view=diff == --- lldb/trunk/include/lldb/Target/ThreadCollection.h (original) +++ lldb/trunk/include/lldb/Target/ThreadCollection.h Thu Dec 8 14:38:19 2016 @@ -48,11 +48,11 @@ public: return ThreadIterable(m_threads, GetMutex()); } - virtual std::recursive_mutex &GetMutex() { return m_mutex; } + virtual std::recursive_mutex &GetMutex() const { return m_mutex; } protected: collection m_threads; - std::recursive_mutex m_mutex; + mutable std::recursive_mutex m_mutex; }; } // namespace lldb_private Modified: lldb/trunk/include/lldb/Target/ThreadList.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/ThreadList.h?rev=289100&r1=289099&r2=289100&view=diff == --- lldb/trunk/include/lldb/Target/ThreadList.h (original) +++ lldb/trunk/include/lldb/Target/ThreadList.h Thu Dec 8 14:38:19 2016 @@ -135,7 +135,7 @@ public: void SetStopID(uint32_t stop_id); - std::recursive_mutex &GetMutex() override; + std::recursive_mutex &GetMutex() const override; void Update(ThreadList &rhs); Modified: lldb/trunk/source/Target/ThreadList.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/ThreadList.cpp?rev=289100&r1=289099&r2=289100&view=diff == --- lldb/trunk/source/Target/ThreadList.cpp (original) +++ lldb/trunk/source/Target/ThreadList.cpp Thu Dec 8 14:38:19 2016 @@ -44,6 +44,7 @@ const ThreadList &ThreadList::operator=( // Lock both mutexes to make sure neither side changes anyone on us // while the assignment occurs std::lock_guard guard(GetMutex()); +std::lock_guard rhs_guard(rhs.GetMutex()); m_process = rhs.m_process; m_stop_id = rhs.m_stop_id; @@ -749,7 +750,7 @@ void ThreadList::Flush() { (*pos)->Flush(); } -std::recursive_mutex &ThreadList::GetMutex() { +std::recursive_mutex &ThreadList::GetMutex() const { return m_process->m_thread_mutex; } ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r289155 - Clean up the new TestInterruptThreadNames test a bit.
Author: jmolenda Date: Thu Dec 8 17:34:56 2016 New Revision: 289155 URL: http://llvm.org/viewvc/llvm-project?rev=289155&view=rev Log: Clean up the new TestInterruptThreadNames test a bit. Modified: lldb/trunk/packages/Python/lldbsuite/test/macosx/thread-names/TestInterruptThreadNames.py Modified: lldb/trunk/packages/Python/lldbsuite/test/macosx/thread-names/TestInterruptThreadNames.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/macosx/thread-names/TestInterruptThreadNames.py?rev=289155&r1=289154&r2=289155&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/macosx/thread-names/TestInterruptThreadNames.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/macosx/thread-names/TestInterruptThreadNames.py Thu Dec 8 17:34:56 2016 @@ -40,19 +40,7 @@ class TestInterruptThreadNames(TestBase) self.assertTrue(rc != 0, "Unable to add listener to process") self.assertTrue(self.wait_for_running(process, listener), "Check that process is up and running") - -inferior_set_up = lldb.SBValue() -retry = 5 -while retry > 0: -time.sleep(1) -process.SendAsyncInterrupt() -self.assertTrue(self.wait_for_stop(process, listener), "Check that process is paused") -inferior_set_up = target.CreateValueFromExpression("threads_up_and_running", "threads_up_and_running") -if inferior_set_up.IsValid() and inferior_set_up.GetValueAsSigned() == 1: -retry = 0 -else: -process.Continue() -retry = retry - 1 +inferior_set_up = self.wait_until_program_setup_complete(process, listener) self.assertTrue(inferior_set_up.IsValid() and inferior_set_up.GetValueAsSigned() == 1, "Check that the program was able to create its threads within the allotted time") @@ -70,12 +58,32 @@ class TestInterruptThreadNames(TestBase) if t.GetName() == "third thread": third_thread = t -self.assertTrue( -main_thread.IsValid() and second_thread.IsValid() and third_thread.IsValid(), -"Got all three expected threads") +self.check_expected_threads_present(main_thread, second_thread, third_thread) process.Kill() + +# The process will set a global variable 'threads_up_and_running' to 1 when +# it has has completed its setup. Sleep for one second, pause the program, +# check to see if the global has that value, and continue if it does not. +def wait_until_program_setup_complete(self, process, listener): +inferior_set_up = lldb.SBValue() +retry = 5 +while retry > 0: +time.sleep(1) +process.SendAsyncInterrupt() +self.assertTrue(self.wait_for_stop(process, listener), "Check that process is paused") +inferior_set_up = process.GetTarget().CreateValueFromExpression("threads_up_and_running", "threads_up_and_running") +if inferior_set_up.IsValid() and inferior_set_up.GetValueAsSigned() == 1: +retry = 0 +else: +process.Continue() +retry = retry - 1 +return inferior_set_up + +# Listen to the process events until we get an event saying that the process is +# running. Retry up to five times in case we get other events that are not +# what we're looking for. def wait_for_running(self, process, listener): retry_count = 5 if process.GetState() == lldb.eStateRunning: @@ -91,13 +99,9 @@ class TestInterruptThreadNames(TestBase) return False -def check_number_of_threads(self, process): -self.assertTrue( -process.GetNumThreads() == 3, -"Check that the process has three threads when sitting at the stopper() breakpoint") - - - +# Listen to the process events until we get an event saying the process is +# stopped. Retry up to five times in case we get other events that we are +# not looking for. def wait_for_stop(self, process, listener): retry_count = 5 if process.GetState() == lldb.eStateStopped or process.GetState() == lldb.eStateCrashed or process.GetState() == lldb.eStateDetached or process.GetState() == lldb.eStateExited: @@ -109,8 +113,20 @@ class TestInterruptThreadNames(TestBase) if event.GetType() == lldb.SBProcess.eBroadcastBitStateChanged: if process.GetState() == lldb.eStateStopped or process.GetState() == lldb.eStateCrashed or process.GetState() == lldb.eStateDetached or process.GetState() == lldb.eStateExited: return True +if process.GetState() == lldb.eStateCrashed or process.GetState() == lldb.eStateDetached or process.GetState() == lldb.eStateExited: +return False retry_count = retr
[Lldb-commits] [lldb] r289164 - Modernize the Args access pattern in a few more commands.
Author: zturner Date: Thu Dec 8 19:08:29 2016 New Revision: 289164 URL: http://llvm.org/viewvc/llvm-project?rev=289164&view=rev Log: Modernize the Args access pattern in a few more commands. Modified: lldb/trunk/include/lldb/Interpreter/CommandObject.h lldb/trunk/source/Commands/CommandObjectHelp.cpp lldb/trunk/source/Commands/CommandObjectLog.cpp lldb/trunk/source/Commands/CommandObjectMemory.cpp lldb/trunk/source/Interpreter/CommandObject.cpp Modified: lldb/trunk/include/lldb/Interpreter/CommandObject.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/CommandObject.h?rev=289164&r1=289163&r2=289164&view=diff == --- lldb/trunk/include/lldb/Interpreter/CommandObject.h (original) +++ lldb/trunk/include/lldb/Interpreter/CommandObject.h Thu Dec 8 19:08:29 2016 @@ -193,7 +193,7 @@ public: static const ArgumentTableEntry *GetArgumentTable(); - static lldb::CommandArgumentType LookupArgumentName(const char *arg_name); + static lldb::CommandArgumentType LookupArgumentName(llvm::StringRef arg_name); static const ArgumentTableEntry * FindArgumentDataByType(lldb::CommandArgumentType arg_type); Modified: lldb/trunk/source/Commands/CommandObjectHelp.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectHelp.cpp?rev=289164&r1=289163&r2=289164&view=diff == --- lldb/trunk/source/Commands/CommandObjectHelp.cpp (original) +++ lldb/trunk/source/Commands/CommandObjectHelp.cpp Thu Dec 8 19:08:29 2016 @@ -108,11 +108,8 @@ bool CommandObjectHelp::DoExecute(Args & // Get command object for the first command argument. Only search built-in // command dictionary. StringList matches; -cmd_obj = -m_interpreter.GetCommandObject(command.GetArgumentAtIndex(0), &matches); -bool is_alias_command = -m_interpreter.AliasExists(command.GetArgumentAtIndex(0)); -std::string alias_name = command.GetArgumentAtIndex(0); +auto command_name = command[0].ref; +cmd_obj = m_interpreter.GetCommandObject(command_name, &matches); if (cmd_obj != nullptr) { StringList matches; @@ -176,11 +173,11 @@ bool CommandObjectHelp::DoExecute(Args & sub_cmd_obj->GenerateHelpText(result); - if (is_alias_command) { + if (m_interpreter.AliasExists(command_name)) { StreamString sstr; -m_interpreter.GetAlias(alias_name)->GetAliasExpansion(sstr); +m_interpreter.GetAlias(command_name)->GetAliasExpansion(sstr); result.GetOutputStream().Printf("\n'%s' is an abbreviation for %s\n", -alias_name.c_str(), sstr.GetData()); +command[0].c_str(), sstr.GetData()); } } else if (matches.GetSize() > 0) { Stream &output_strm = result.GetOutputStream(); @@ -194,16 +191,16 @@ bool CommandObjectHelp::DoExecute(Args & // Maybe the user is asking for help about a command argument rather than // a command. const CommandArgumentType arg_type = - CommandObject::LookupArgumentName(command.GetArgumentAtIndex(0)); + CommandObject::LookupArgumentName(command_name); if (arg_type != eArgTypeLastArg) { Stream &output_strm = result.GetOutputStream(); CommandObject::GetArgumentHelp(output_strm, arg_type, m_interpreter); result.SetStatus(eReturnStatusSuccessFinishNoResult); } else { StreamString error_msg_stream; -GenerateAdditionalHelpAvenuesMessage(&error_msg_stream, - command.GetArgumentAtIndex(0), - m_interpreter.GetCommandPrefix(), ""); +GenerateAdditionalHelpAvenuesMessage(&error_msg_stream, command_name, + m_interpreter.GetCommandPrefix(), + ""); result.AppendError(error_msg_stream.GetString()); result.SetStatus(eReturnStatusFailed); } @@ -225,8 +222,7 @@ int CommandObjectHelp::HandleCompletion( input, cursor_index, cursor_char_position, match_start_point, max_return_elements, word_complete, matches); } else { -CommandObject *cmd_obj = -m_interpreter.GetCommandObject(input.GetArgumentAtIndex(0)); +CommandObject *cmd_obj = m_interpreter.GetCommandObject(input[0].ref); // The command that they are getting help on might be ambiguous, in which // case we should complete that, Modified: lldb/trunk/source/Commands/CommandObjectLog.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectLog.cpp?rev=289164&r1=289163&r2=289164&view=diff == --- lldb/trunk/source/Commands/CommandObjectLog.
[Lldb-commits] [lldb] r289168 - Fix some occurrences of passing StringRef to Printf.
Author: zturner Date: Thu Dec 8 19:20:58 2016 New Revision: 289168 URL: http://llvm.org/viewvc/llvm-project?rev=289168&view=rev Log: Fix some occurrences of passing StringRef to Printf. Hopefully these will all disappear in the future once we move to formatv. Modified: lldb/trunk/source/Commands/CommandObjectApropos.cpp lldb/trunk/source/Commands/CommandObjectCommands.cpp lldb/trunk/source/Commands/CommandObjectFrame.cpp Modified: lldb/trunk/source/Commands/CommandObjectApropos.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectApropos.cpp?rev=289168&r1=289167&r2=289168&view=diff == --- lldb/trunk/source/Commands/CommandObjectApropos.cpp (original) +++ lldb/trunk/source/Commands/CommandObjectApropos.cpp Thu Dec 8 19:20:58 2016 @@ -51,11 +51,10 @@ bool CommandObjectApropos::DoExecute(Arg const size_t argc = args.GetArgumentCount(); if (argc == 1) { -const char *search_word = args.GetArgumentAtIndex(0); -if ((search_word != nullptr) && (strlen(search_word) > 0)) { +auto search_word = args[0].ref; +if (!search_word.empty()) { // The bulk of the work must be done inside the Command Interpreter, since - // the command dictionary - // is private. + // the command dictionary is private. StringList commands_found; StringList commands_help; @@ -66,11 +65,11 @@ bool CommandObjectApropos::DoExecute(Arg result.AppendMessageWithFormat("No commands found pertaining to '%s'. " "Try 'help' to see a complete list of " "debugger commands.\n", - search_word); + args[0].c_str()); } else { if (commands_found.GetSize() > 0) { result.AppendMessageWithFormat( - "The following commands may relate to '%s':\n", search_word); + "The following commands may relate to '%s':\n", args[0].c_str()); size_t max_len = 0; for (size_t i = 0; i < commands_found.GetSize(); ++i) { @@ -93,7 +92,7 @@ bool CommandObjectApropos::DoExecute(Arg const bool dump_qualified_name = true; result.AppendMessageWithFormat( "\nThe following settings variables may relate to '%s': \n\n", -search_word); +args[0].c_str()); for (size_t i = 0; i < num_properties; ++i) properties[i]->DumpDescription( m_interpreter, result.GetOutputStream(), 0, dump_qualified_name); Modified: lldb/trunk/source/Commands/CommandObjectCommands.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectCommands.cpp?rev=289168&r1=289167&r2=289168&view=diff == --- lldb/trunk/source/Commands/CommandObjectCommands.cpp (original) +++ lldb/trunk/source/Commands/CommandObjectCommands.cpp Thu Dec 8 19:20:58 2016 @@ -840,7 +840,7 @@ protected: result.AppendErrorWithFormat( "'%s' is not a known command.\nTry 'help' to see a " "current list of commands.\n", - command_name); + args[0].c_str()); result.SetStatus(eReturnStatusFailed); return false; } @@ -850,11 +850,11 @@ protected: result.AppendErrorWithFormat( "'%s' is not an alias, it is a debugger command which can be " "removed using the 'command delete' command.\n", -command_name); +args[0].c_str()); } else { result.AppendErrorWithFormat( "'%s' is a permanent debugger command and cannot be removed.\n", -command_name); +args[0].c_str()); } result.SetStatus(eReturnStatusFailed); return false; @@ -863,10 +863,11 @@ protected: if (!m_interpreter.RemoveAlias(command_name)) { if (m_interpreter.AliasExists(command_name)) result.AppendErrorWithFormat( -"Error occurred while attempting to unalias '%s'.\n", command_name); +"Error occurred while attempting to unalias '%s'.\n", +args[0].c_str()); else result.AppendErrorWithFormat("'%s' is not an existing alias.\n", - command_name); + args[0].c_str()); result.SetStatus(eReturnStatusFailed); return false; } @@ -932,7 +933,7 @@ protected: if (!m_interpreter.RemoveCommand(command_name)) { result.AppendErrorWithFormat( "'%s' is a permanent debugger command and cannot be removed.\n", - command_name); + args[0].c_str()); result.SetStatus(eReturnStatusFailed); return false; } @@ -1882,7 +1883,7 @@ protected: if (cmd_name.empty() || !m_interpreter.HasUserCommands() || !m
[Lldb-commits] [lldb] r289169 - Calling SBDebugger::CeeateTarget being called on multiple threads was crashing LLDB.
Author: gclayton Date: Thu Dec 8 19:21:14 2016 New Revision: 289169 URL: http://llvm.org/viewvc/llvm-project?rev=289169&view=rev Log: Calling SBDebugger::CeeateTarget being called on multiple threads was crashing LLDB. I found the race condition in: ScriptInterpreter *CommandInterpreter::GetScriptInterpreter(bool can_create); More than one "ScriptInterpreter *" was being returned due to the race which caused any clients with the first one to now be pointing to freed memory and we would quickly crash. Added a test to catch this so we don't regress. Added: lldb/trunk/packages/Python/lldbsuite/test/api/multiple-targets/ lldb/trunk/packages/Python/lldbsuite/test/api/multiple-targets/Makefile lldb/trunk/packages/Python/lldbsuite/test/api/multiple-targets/TestMultipleTargets.py lldb/trunk/packages/Python/lldbsuite/test/api/multiple-targets/main.cpp Modified: lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h lldb/trunk/source/Interpreter/CommandInterpreter.cpp Modified: lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h?rev=289169&r1=289168&r2=289169&view=diff == --- lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h (original) +++ lldb/trunk/include/lldb/Interpreter/CommandInterpreter.h Thu Dec 8 19:21:14 2016 @@ -12,6 +12,7 @@ // C Includes // C++ Includes +#include // Other libraries and framework includes // Project includes #include "lldb/Core/Broadcaster.h" @@ -538,6 +539,7 @@ private: std::string m_repeat_command; // Stores the command that will be executed for // an empty command string. lldb::ScriptInterpreterSP m_script_interpreter_sp; + std::mutex m_script_interpreter_mutex; lldb::IOHandlerSP m_command_io_handler_sp; char m_comment_char; bool m_batch_command_mode; Added: lldb/trunk/packages/Python/lldbsuite/test/api/multiple-targets/Makefile URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/api/multiple-targets/Makefile?rev=289169&view=auto == --- lldb/trunk/packages/Python/lldbsuite/test/api/multiple-targets/Makefile (added) +++ lldb/trunk/packages/Python/lldbsuite/test/api/multiple-targets/Makefile Thu Dec 8 19:21:14 2016 @@ -0,0 +1,8 @@ +LEVEL = ../../make + +MAKE_DSYM := NO + +ENABLE_THREADS := YES +CXX_SOURCES := main.cpp + +include $(LEVEL)/Makefile.rules Added: lldb/trunk/packages/Python/lldbsuite/test/api/multiple-targets/TestMultipleTargets.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/api/multiple-targets/TestMultipleTargets.py?rev=289169&view=auto == --- lldb/trunk/packages/Python/lldbsuite/test/api/multiple-targets/TestMultipleTargets.py (added) +++ lldb/trunk/packages/Python/lldbsuite/test/api/multiple-targets/TestMultipleTargets.py Thu Dec 8 19:21:14 2016 @@ -0,0 +1,39 @@ +"""Test the lldb public C++ api when creating multiple targets simultaneously.""" + +from __future__ import print_function + + +import os +import re +import subprocess + +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class TestMultipleSimultaneousDebuggers(TestBase): + +mydir = TestBase.compute_mydir(__file__) + +@skipIfNoSBHeaders +def test_multiple_debuggers(self): +env = {self.dylibPath: self.getLLDBLibraryEnvVal()} + +self.driver_exe = os.path.join(os.getcwd(), "multi-target") +self.buildDriver('main.cpp', self.driver_exe) +self.addTearDownHook(lambda: os.remove(self.driver_exe)) +self.signBinary(self.driver_exe) + +# check_call will raise a CalledProcessError if multi-process-driver doesn't return +# exit code 0 to indicate success. We can let this exception go - the test harness +# will recognize it as a test failure. + +if self.TraceOn(): +print("Running test %s" % self.driver_exe) +check_call([self.driver_exe, self.driver_exe], env=env) +else: +with open(os.devnull, 'w') as fnull: +check_call([self.driver_exe, self.driver_exe], + env=env, stdout=fnull, stderr=fnull) Added: lldb/trunk/packages/Python/lldbsuite/test/api/multiple-targets/main.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/api/multiple-targets/main.cpp?rev=289169&view=auto == --- lldb/trunk/packages/Python/lldbsuite/test/api/multiple-targets/main.cpp (added) +++ lldb/trunk/packages/Python/lldbsuite/test/api/multiple-targets/main.cpp Thu Dec 8 19:21:14 2016 @@ -0,0 +1,31 @@ +#i
[Lldb-commits] [lldb] r289188 - Remove some more uses of Args::GetArgumentAtIndex.
Author: zturner Date: Thu Dec 8 23:46:41 2016 New Revision: 289188 URL: http://llvm.org/viewvc/llvm-project?rev=289188&view=rev Log: Remove some more uses of Args::GetArgumentAtIndex. Modified: lldb/trunk/source/Commands/CommandObjectMultiword.cpp lldb/trunk/source/Commands/CommandObjectPlugin.cpp lldb/trunk/source/Commands/CommandObjectRegister.cpp lldb/trunk/source/Commands/CommandObjectSource.cpp lldb/trunk/source/Commands/CommandObjectSyntax.cpp lldb/trunk/source/Interpreter/Args.cpp Modified: lldb/trunk/source/Commands/CommandObjectMultiword.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectMultiword.cpp?rev=289188&r1=289187&r2=289188&view=diff == --- lldb/trunk/source/Commands/CommandObjectMultiword.cpp (original) +++ lldb/trunk/source/Commands/CommandObjectMultiword.cpp Thu Dec 8 23:46:41 2016 @@ -98,58 +98,61 @@ bool CommandObjectMultiword::Execute(con const size_t argc = args.GetArgumentCount(); if (argc == 0) { this->CommandObject::GenerateHelpText(result); - } else { -const char *sub_command = args.GetArgumentAtIndex(0); +return result.Succeeded(); + } -if (sub_command) { - if (::strcasecmp(sub_command, "help") == 0) { -this->CommandObject::GenerateHelpText(result); - } else if (!m_subcommand_dict.empty()) { -StringList matches; -CommandObject *sub_cmd_obj = GetSubcommandObject(sub_command, &matches); -if (sub_cmd_obj != nullptr) { - // Now call CommandObject::Execute to process and options in - // 'rest_of_line'. From there - // the command-specific version of Execute will be called, with the - // processed arguments. + auto sub_command = args[0].ref; + if (sub_command.empty()) +return result.Succeeded(); - args.Shift(); + if (sub_command.equals_lower("help")) { +this->CommandObject::GenerateHelpText(result); +return result.Succeeded(); + } - sub_cmd_obj->Execute(args_string, result); -} else { - std::string error_msg; - const size_t num_subcmd_matches = matches.GetSize(); - if (num_subcmd_matches > 0) -error_msg.assign("ambiguous command "); - else -error_msg.assign("invalid command "); - - error_msg.append("'"); - error_msg.append(GetCommandName()); - error_msg.append(" "); - error_msg.append(sub_command); - error_msg.append("'."); - - if (num_subcmd_matches > 0) { -error_msg.append(" Possible completions:"); -for (size_t i = 0; i < num_subcmd_matches; i++) { - error_msg.append("\n\t"); - error_msg.append(matches.GetStringAtIndex(i)); -} - } - error_msg.append("\n"); - result.AppendRawError(error_msg.c_str()); - result.SetStatus(eReturnStatusFailed); -} - } else { -result.AppendErrorWithFormat("'%s' does not have any subcommands.\n", - GetCommandName().str().c_str()); -result.SetStatus(eReturnStatusFailed); - } -} + if (m_subcommand_dict.empty()) { +result.AppendErrorWithFormat("'%s' does not have any subcommands.\n", + GetCommandName().str().c_str()); +result.SetStatus(eReturnStatusFailed); +return false; + } + + StringList matches; + CommandObject *sub_cmd_obj = GetSubcommandObject(sub_command, &matches); + if (sub_cmd_obj != nullptr) { +// Now call CommandObject::Execute to process options in `rest_of_line`. +// From there the command-specific version of Execute will be called, +// with the processed arguments. + +args.Shift(); +sub_cmd_obj->Execute(args_string, result); +return result.Succeeded(); } - return result.Succeeded(); + std::string error_msg; + const size_t num_subcmd_matches = matches.GetSize(); + if (num_subcmd_matches > 0) +error_msg.assign("ambiguous command "); + else +error_msg.assign("invalid command "); + + error_msg.append("'"); + error_msg.append(GetCommandName()); + error_msg.append(" "); + error_msg.append(sub_command); + error_msg.append("'."); + + if (num_subcmd_matches > 0) { +error_msg.append(" Possible completions:"); +for (size_t i = 0; i < num_subcmd_matches; i++) { + error_msg.append("\n\t"); + error_msg.append(matches.GetStringAtIndex(i)); +} + } + error_msg.append("\n"); + result.AppendRawError(error_msg.c_str()); + result.SetStatus(eReturnStatusFailed); + return false; } void CommandObjectMultiword::GenerateHelpText(Stream &output_stream) { @@ -191,16 +194,15 @@ int CommandObjectMultiword::HandleComple bool &word_complete, StringList &matches) { // Any of the command