Merge pull request #6048 from adrian-prantl/deadlock

Work around an infinite recursion with deadlock that only exists on

Author: adrian-prantl

lldb/include/lldb/Expression/IRExecutionUnit.h

@@ -391,6 +391,9 @@ class IRExecutionUnit : public std::enable_shared_from_this<IRExecutionUnit>,
   std::vector<ConstString> m_failed_lookups;
 
   std::atomic<bool> m_did_jit;
+  // BEGIN SWIFT
+  std::atomic<bool> m_in_populate_symtab = false;
+  // END SWIFT
 
   lldb::addr_t m_function_load_addr;
   lldb::addr_t m_function_end_load_addr;

lldb/packages/Python/lldbsuite/test/lldbplaygroundrepl.py

@@ -77,7 +77,6 @@ def execute_code(self, inputFile):
 
         with open(inputFile, 'r') as contents_file:
             contents = contents_file.read()
-        self.expect("log enable lldb types expr -f /tmp/types.log")
         result = self.frame.EvaluateExpression(contents, self.options)
         output = self.frame.EvaluateExpression("get_output()")
         with recording(self, self.TraceOn()) as sbuf:

lldb/source/Expression/IRExecutionUnit.cpp

@@ -839,17 +839,20 @@ IRExecutionUnit::FindInSymbols(const std::vector<ConstString> &names,
     }
 
     if (sc.target_sp) {
+      ModuleList images = sc.target_sp->GetImages();
+      // BEGIN SWIFT
+      if (m_in_populate_symtab)
+        if (lldb::ModuleSP module_sp = m_jit_module_wp.lock())
+        images.Remove(module_sp);
+      // END SWIFT
+
       SymbolContextList sc_list;
-      sc.target_sp->GetImages().FindFunctions(name, lldb::eFunctionNameTypeFull,
-                                              function_options, sc_list);
+      images.FindFunctions(name, lldb::eFunctionNameTypeFull, function_options,
+                           sc_list);
       if (auto load_addr = resolver.Resolve(sc_list))
         return *load_addr;
-    }
 
-    if (sc.target_sp) {
-      SymbolContextList sc_list;
-      sc.target_sp->GetImages().FindSymbolsWithNameAndType(
-          name, lldb::eSymbolTypeAny, sc_list);
+      images.FindSymbolsWithNameAndType(name, lldb::eSymbolTypeAny, sc_list);
       if (auto load_addr = resolver.Resolve(sc_list))
         return *load_addr;
     }
@@ -1194,6 +1197,9 @@ uint32_t IRExecutionUnit::GetAddressByteSize() const {
 
 void IRExecutionUnit::PopulateSymtab(lldb_private::ObjectFile *obj_file,
                                      lldb_private::Symtab &symtab) {
+  // BEGIN SWIFT
+  m_in_populate_symtab = true;
+  auto _ = llvm::make_scope_exit([this]() { m_in_populate_symtab = false; });
   if (m_execution_engine_up) {
     uint32_t symbol_id = 0;
     lldb_private::SectionList *section_list = obj_file->GetSectionList();
@@ -1271,6 +1277,7 @@ void IRExecutionUnit::PopulateSymtab(lldb_private::ObjectFile *obj_file,
       }
     }
   }
+  // END SWIFT
 }
 
 void IRExecutionUnit::PopulateSectionList(

lldb/test/API/lang/swift/playgrounds/Import.swift

@@ -0,0 +1,2 @@
+import Dylib
+f()

lldb/test/API/lang/swift/playgrounds/TestSwiftPlaygrounds.py

@@ -88,6 +88,7 @@ def test_no_force_target(self):
     @swiftTest
     @skipIf(setting=('symbols.use-swift-clangimporter', 'false'))
     @skipIf(debug_info=decorators.no_match("dsym"))
+    @skipIf(macos_version=["<", "12"])
     def test_concurrency(self):
         """Test that concurrency is available in playgrounds"""
         self.launch(True)
@@ -100,8 +101,7 @@ def test_concurrency(self):
     def test_import(self):
         """Test that a dylib can be imported in playgrounds"""
         self.launch(True)
-        self.do_concurrency_test()
-
+        self.do_import_test()
 
     def launch(self, force_target):
         """Test that playgrounds work"""
@@ -145,47 +145,56 @@ def launch(self, force_target):
         self.options.SetTryAllThreads(True)
 
 
-    def do_basic_test(self, force_target):
-        contents = ""
-
-        with open('Contents.swift', 'r') as contents_file:
+    def execute_code(self, input_file, expect_error=False):
+        contents = "syntax error"
+        with open(input_file, 'r') as contents_file:
             contents = contents_file.read()
-
-        self.frame().EvaluateExpression(contents, self.options)
+        res = self.frame().EvaluateExpression(contents, self.options)
         ret = self.frame().EvaluateExpression("get_output()")
+        is_error = res.GetError().Fail() and not (
+                     res.GetError().GetType() == 1 and
+                     res.GetError().GetError() == 0x1001)
         playground_output = ret.GetSummary()
+        with recording(self, self.TraceOn()) as sbuf:
+            print("playground result: ", file=sbuf)
+            print(str(res), file=sbuf)
+            if is_error:
+                print("error:", file=sbuf)
+                print(str(res.GetError()), file=sbuf)
+            else:
+                print("playground output:", file=sbuf)
+                print(str(ret), file=sbuf)
+
+        if expect_error:
+            self.assertTrue(is_error)
+            return playground_output
+        self.assertFalse(is_error)
+        self.assertIsNotNone(playground_output)
+        return playground_output
+        
+    def do_basic_test(self, force_target):
+        playground_output = self.execute_code('Contents.swift', not force_target)
         if not force_target:
             # This is expected to fail because the deployment target
             # is less than the availability of the function being
             # called.
             self.assertEqual(playground_output, '""')
             return
 
-        self.assertTrue(playground_output is not None)
-        self.assertTrue("a=\\'3\\'" in playground_output)
-        self.assertTrue("b=\\'5\\'" in playground_output)
-        self.assertTrue("=\\'8\\'" in playground_output)
-        self.assertTrue("=\\'11\\'" in playground_output)
+        self.assertIn("a=\\'3\\'", playground_output)
+        self.assertIn("b=\\'5\\'", playground_output)
+        self.assertIn("=\\'8\\'", playground_output)
+        self.assertIn("=\\'11\\'", playground_output)
 
     def do_concurrency_test(self):
-        contents = "error"
-        with open('Concurrency.swift', 'r') as contents_file:
-            contents = contents_file.read()
-        res = self.frame().EvaluateExpression(contents, self.options)
-        ret = self.frame().EvaluateExpression("get_output()")
-        playground_output = ret.GetSummary()
-        self.assertTrue(playground_output is not None)
+        playground_output = self.execute_code('Concurrency.swift')
         self.assertIn("=\\'23\\'", playground_output)
 
     def do_import_test(self):
         # Test importing a library that adds new Clang options.
         log = self.getBuildArtifact('types.log')
         self.expect('log enable lldb types -f ' + log)
-        contents = "import Dylib\nf()\n"
-        res = self.frame().EvaluateExpression(contents, self.options)
-        ret = self.frame().EvaluateExpression("get_output()")
-        playground_output = ret.GetSummary()
-        self.assertTrue(playground_output is not None)
+        playground_output = self.execute_code('Import.swift')
         self.assertIn("Hello from the Dylib", playground_output)
 
         # Scan through the types log to make sure the SwiftASTContext was poisoned.