[Lldb-commits] [PATCH] D138060: Improve error logging when xcrun fails to execute successfully

[Adrian Prantl via Phabricator via lldb-commits]

aprantl created this revision.
aprantl added reviewers: JDevlieghere, kastiglione.
Herald added a project: All.
aprantl requested review of this revision.

Because Host::RunShellCommand runs commands through `$SHELL` there is an 
opportunity for this to fail spectacularly on systems that use custom shells 
with odd behaviors. This patch makes these situations easier to debug by at 
least logging the result of the failed xcrun invocation.

rdar://102107430


https://reviews.llvm.org/D138060

Files:
  lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
  lldb/unittests/Host/HostTest.cpp


Index: lldb/unittests/Host/HostTest.cpp
===================================================================
--- lldb/unittests/Host/HostTest.cpp
+++ lldb/unittests/Host/HostTest.cpp
@@ -25,3 +25,16 @@
   ASSERT_EQ("Host::GetEnvironment",
             Host::GetEnvironment().lookup("LLDB_TEST_ENVIRONMENT_VAR"));
 }
+
+#if defined(LLVM_ON_UNIX)
+TEST(Host, RunShellCommand) {
+  std::string shell = "SHELL=" + getenv("SHELL");
+  putenv(const_cast<char *>("SHELL=/bin/LLDB_TEST_this-file-does-not-exist"));
+  int status;
+  std::string out;
+  Status error =
+      Host::RunShellCommand("/usr/bin/true", FileSpec(), &status, &signo, 
&out);
+  ASSERT_TRUE(error.Fail());
+  putenv(shell);
+}
+#endif
Index: lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
===================================================================
--- lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
+++ lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
@@ -381,7 +381,7 @@
     args.AppendArgument("--sdk");
     args.AppendArgument(sdk);
 
-    Log *log = GetLog(LLDBLog::Host);
+    Log *log = GetLog(LLDBLog::Host | LLDBLog::Types);
     if (log) {
       std::string cmdstr;
       args.GetCommandString(cmdstr);
@@ -393,11 +393,21 @@
     std::string output_str;
     lldb_private::Status error =
         Host::RunShellCommand(args, FileSpec(), &status, &signo, &output_str,
-                              std::chrono::seconds(15));
+                              std::chrono::seconds(30));
 
-    // Check that xcrun return something useful.
-    if (status != 0 || output_str.empty())
+    // Check that xcrun returned something useful.
+    if (error.Fail()) {
+      LLDB_LOG(log, "xcrun failed to execute: %s", error.AsCString());
+      return {};
+    }
+    if (status != 0) {
+      LLDB_LOG(log, "xcrun returned exit code %d", status);
       return {};
+    }
+    if (output_str.empty()) {
+      LLDB_LOG(log, "xcrun returned no results");
+      return {};
+    }
 
     // Convert to a StringRef so we can manipulate the string without modifying
     // the underlying data.



Index: lldb/unittests/Host/HostTest.cpp
===================================================================
--- lldb/unittests/Host/HostTest.cpp
+++ lldb/unittests/Host/HostTest.cpp
@@ -25,3 +25,16 @@
   ASSERT_EQ("Host::GetEnvironment",
             Host::GetEnvironment().lookup("LLDB_TEST_ENVIRONMENT_VAR"));
 }
+
+#if defined(LLVM_ON_UNIX)
+TEST(Host, RunShellCommand) {
+  std::string shell = "SHELL=" + getenv("SHELL");
+  putenv(const_cast<char *>("SHELL=/bin/LLDB_TEST_this-file-does-not-exist"));
+  int status;
+  std::string out;
+  Status error =
+      Host::RunShellCommand("/usr/bin/true", FileSpec(), &status, &signo, &out);
+  ASSERT_TRUE(error.Fail());
+  putenv(shell);
+}
+#endif
Index: lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
===================================================================
--- lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
+++ lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm
@@ -381,7 +381,7 @@
     args.AppendArgument("--sdk");
     args.AppendArgument(sdk);
 
-    Log *log = GetLog(LLDBLog::Host);
+    Log *log = GetLog(LLDBLog::Host | LLDBLog::Types);
     if (log) {
       std::string cmdstr;
       args.GetCommandString(cmdstr);
@@ -393,11 +393,21 @@
     std::string output_str;
     lldb_private::Status error =
         Host::RunShellCommand(args, FileSpec(), &status, &signo, &output_str,
-                              std::chrono::seconds(15));
+                              std::chrono::seconds(30));
 
-    // Check that xcrun return something useful.
-    if (status != 0 || output_str.empty())
+    // Check that xcrun returned something useful.
+    if (error.Fail()) {
+      LLDB_LOG(log, "xcrun failed to execute: %s", error.AsCString());
+      return {};
+    }
+    if (status != 0) {
+      LLDB_LOG(log, "xcrun returned exit code %d", status);
       return {};
+    }
+    if (output_str.empty()) {
+      LLDB_LOG(log, "xcrun returned no results");
+      return {};
+    }
 
     // Convert to a StringRef so we can manipulate the string without modifying
     // the underlying data.

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits