[lldb] Keep the existing behavior for untrusted dSYMs

[JDevlieghere]

This patch does two thing:

• It reverts to the previous behavior of warning for untrusted dSYMs.
• It includes whether a dSYM is trusted or untrusted in the warning output.

My reasoning is that there's no tooling for automatically signing dSYMs and therefore we shouldn't change the behavior until this is more common. The inclusion of whether the dSYM is signed or not is the first step towards advertising the existence of the feature.

This now also means the release note I added in #189444 is correct (again).

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

This patch does two thing:

• It reverts to the previous behavior of warning for untrusted dSYMs.
• It includes whether a dSYM is trusted or untrusted in the warning output.

My reasoning is that there's no tooling for automatically signing dSYMs and therefore we shouldn't change the behavior until this is more common. The inclusion of whether the dSYM is signed or not is the first step towards advertising the existence of the feature.

This now also means the release note I added in #189444 is correct (again).

---

Full diff: https://github.com/llvm/llvm-project/pull/190407.diff

2 Files Affected:

• (modified) lldb/source/Core/ModuleList.cpp (+12-6)
• (modified) lldb/test/Shell/Platform/AutoLoad/Darwin/dsym-python-script-name-warnings.test (+1-1)

diff --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp
index fae42cb90a7fb..fd374221cbeca 100644
--- a/lldb/source/Core/ModuleList.cpp
+++ b/lldb/source/Core/ModuleList.cpp
@@ -1374,25 +1374,30 @@ bool ModuleList::LoadScriptingResourceInTargetForModule(Module &module,
     debugger.ReportWarning(feedback_stream.GetString().str(), debugger.GetID());
 
   for (const auto &[scripting_fspec, load_style] : file_specs) {
+    if (load_style == eLoadScriptFromSymFileFalse)
+      continue;
+
     if (!FileSystem::Instance().Exists(scripting_fspec))
       continue;
 
+    const bool trusted = platform_sp->IsSymbolFileTrusted(module);
+
     switch (load_style) {
     case eLoadScriptFromSymFileFalse:
-      continue;
+      llvm_unreachable("case already handled");
     case eLoadScriptFromSymFileTrue:
       break;
     case eLoadScriptFromSymFileTrusted:
-      if (!platform_sp->IsSymbolFileTrusted(module))
-        continue;
-      break;
+      if (trusted)
+        break;
+      LLVM_FALLTHROUGH;
     case eLoadScriptFromSymFileWarn:
       debugger.ReportWarning(
           llvm::formatv(
               // clang-format off
-R"('{0}' contains a debug script. To run this script in this debug session:
+R"('{0}' contains {1} debug script. To run this script in this debug session:
 
-    command script import "{1}"
+    command script import "{2}"
 
 To run all discovered debug scripts in this session:
 
@@ -1400,6 +1405,7 @@ To run all discovered debug scripts in this session:
 )",
               // clang-format on
               module.GetFileSpec().GetFileNameStrippingExtension(),
+              trusted ? "a trusted" : "an untrusted",
               scripting_fspec.GetPath()),
           debugger.GetID());
 
diff --git a/lldb/test/Shell/Platform/AutoLoad/Darwin/dsym-python-script-name-warnings.test b/lldb/test/Shell/Platform/AutoLoad/Darwin/dsym-python-script-name-warnings.test
index 9c84045d75932..07fa3c0d6e679 100644
--- a/lldb/test/Shell/Platform/AutoLoad/Darwin/dsym-python-script-name-warnings.test
+++ b/lldb/test/Shell/Platform/AutoLoad/Darwin/dsym-python-script-name-warnings.test
@@ -33,7 +33,7 @@
 
 ## Also confirm that the warning message about auto-loading scripts is printed afterwards.
 
-# CHECK-REMOVE: warning: 'Test-Module2' contains a debug script. To run this script in this
+# CHECK-REMOVE: warning: 'Test-Module2' contains an untrusted debug script. To run this script in this
 
 #--- main.c
 int main() { return 0; }

[JDevlieghere]

I don't like that we now potentially print the warning more than once. I don't want to further complicate this PR, so I'll address that together with a test in a follow-up later today.

[jimingham]

LGTM to get the bots unstuck.

[Michael137]

I don't like that we now potentially print the warning more than once. I don't want to further complicate this PR, so I'll address that together with a test in a follow-up later today.

FWIW i think that #189943 will address this