[Lldb-commits] [PATCH] D27459: Straw-man proposal for new logging syntax

2016-12-08 Thread Pavel Labath via Phabricator via lldb-commits
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

2016-12-08 Thread Greg Clayton via Phabricator via lldb-commits
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

2016-12-08 Thread Zachary Turner via lldb-commits
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.

2016-12-08 Thread Greg Clayton via lldb-commits
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.

2016-12-08 Thread Jason Molenda via lldb-commits
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.

2016-12-08 Thread Zachary Turner via lldb-commits
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.

2016-12-08 Thread Zachary Turner via lldb-commits
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.

2016-12-08 Thread Greg Clayton via lldb-commits
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.

2016-12-08 Thread Zachary Turner via lldb-commits
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