Michael137 updated this revision to Diff 479052.
Michael137 added a comment.

- Clear TypeSystem in notification


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138724/new/

https://reviews.llvm.org/D138724

Files:
  lldb/include/lldb/Core/ModuleList.h
  lldb/source/Core/ModuleList.cpp
  lldb/source/Target/Target.cpp
  lldb/test/API/functionalities/rerun_and_expr/Makefile
  lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py
  lldb/test/API/functionalities/rerun_and_expr/main.cpp
  lldb/test/API/functionalities/rerun_and_expr/rebuild.cpp

Index: lldb/test/API/functionalities/rerun_and_expr/rebuild.cpp
===================================================================
--- /dev/null
+++ lldb/test/API/functionalities/rerun_and_expr/rebuild.cpp
@@ -0,0 +1,12 @@
+struct Base {
+  int m_base_val = 42;
+};
+
+struct Foo : public Base {
+  int m_derived_val = 137;
+};
+
+int main() {
+  Foo foo;
+  return 0;
+}
Index: lldb/test/API/functionalities/rerun_and_expr/main.cpp
===================================================================
--- /dev/null
+++ lldb/test/API/functionalities/rerun_and_expr/main.cpp
@@ -0,0 +1,8 @@
+struct Foo {
+  int m_val = 42;
+};
+
+int main() {
+  Foo foo;
+  return 0;
+}
Index: lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py
===================================================================
--- /dev/null
+++ lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py
@@ -0,0 +1,71 @@
+"""
+Test that re-running a process from within the same target
+after rebuilding the executable flushes the scratch TypeSystems
+tied to that process.
+"""
+
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestRerun(TestBase):
+    def test(self):
+        """
+        Tests whether re-launching a process without destroying
+        the owning target keeps invalid ASTContexts in the
+        scratch AST's importer.
+
+        We test this by:
+        1. Evaluating an expression to import 'struct Foo' into
+           the scratch AST
+        2. Change the definition of 'struct Foo' and rebuild the executable
+        3. Re-launch the process
+        4. Evaluate the same expression in (1). We expect to have only
+           the latest definition of 'struct Foo' in the scratch AST.
+        """
+        self.build(dictionary={'CXX_SOURCES':'main.cpp', 'EXE':'a.out'})
+        (target, _, _, bkpt) = \
+                lldbutil.run_to_source_breakpoint(self, 'return', lldb.SBFileSpec('main.cpp'))
+
+        target.BreakpointCreateBySourceRegex('return', lldb.SBFileSpec('rebuild.cpp', False))
+
+        self.expect_expr('foo', result_type='Foo', result_children=[
+                ValueCheck(name='m_val', value='42')
+            ])
+
+        self.build(dictionary={'CXX_SOURCES':'rebuild.cpp', 'EXE':'a.out'})
+
+        self.runCmd('process launch')
+
+        self.expect_expr('foo', result_type='Foo', result_children=[
+            ValueCheck(name='Base', children=[
+                ValueCheck(name='m_base_val', value='42')
+            ]),
+            ValueCheck(name='m_derived_val', value='137')
+        ])
+
+        self.filecheck("target module dump ast", __file__)
+
+        # The new definition 'struct Foo' is in the scratch AST
+        # CHECK:      |-CXXRecordDecl {{.*}} struct Foo definition
+        # CHECK-NEXT: | |-DefinitionData pass_in_registers standard_layout trivially_copyable trivial literal
+        # CHECK-NEXT: | | |-DefaultConstructor exists trivial needs_implicit
+        # CHECK-NEXT: | | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
+        # CHECK-NEXT: | | |-MoveConstructor exists simple trivial needs_implicit
+        # CHECK-NEXT: | | |-CopyAssignment simple trivial has_const_param needs_implicit implicit_has_const_param
+        # CHECK-NEXT: | | |-MoveAssignment exists simple trivial needs_implicit
+        # CHECK-NEXT: | | `-Destructor simple irrelevant trivial needs_implicit
+        # CHECK-NEXT: | |-public 'Base'
+        # CHECK-NEXT: | `-FieldDecl {{.*}} m_derived_val 'int'
+        # CHECK-NEXT: `-CXXRecordDecl {{.*}} struct Base definition
+        # CHECK-NEXT:   |-DefinitionData pass_in_registers aggregate standard_layout trivially_copyable pod trivial literal
+        # CHECK-NEXT:   | |-DefaultConstructor exists trivial needs_implicit
+        # CHECK-NEXT:   | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
+        # CHECK-NEXT:   | |-MoveConstructor exists simple trivial needs_implicit
+        # CHECK-NEXT:   | |-CopyAssignment simple trivial has_const_param needs_implicit implicit_has_const_param
+        # CHECK-NEXT:   | |-MoveAssignment exists simple trivial needs_implicit
+        # CHECK-NEXT:   | `-Destructor simple irrelevant trivial needs_implicit
+
+        # ...but the original definition of 'struct Foo' is not in the scratch AST anymore
+        # CHECK-NOT: FieldDecl {{.*}} m_val 'int'
+
Index: lldb/test/API/functionalities/rerun_and_expr/Makefile
===================================================================
--- /dev/null
+++ lldb/test/API/functionalities/rerun_and_expr/Makefile
@@ -0,0 +1 @@
+include Makefile.rules
Index: lldb/source/Target/Target.cpp
===================================================================
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -199,7 +199,6 @@
     m_process_sp->Finalize();
 
     CleanupProcess();
-
     m_process_sp.reset();
   }
 }
@@ -1682,6 +1681,36 @@
     m_breakpoint_list.UpdateBreakpoints(module_list, false, delete_locations);
     m_internal_breakpoint_list.UpdateBreakpoints(module_list, false,
                                                  delete_locations);
+
+    const bool should_flush_type_systems =
+        module_list.AllOf([](const lldb::ModuleSP &module_sp) {
+          if (!module_sp)
+            return true;
+
+          auto *object_file = module_sp->GetObjectFile();
+
+          if (!object_file)
+            return true;
+
+          auto type = object_file->GetType();
+
+          // We only want to flush persistent variables
+          // if the module was capable of describing a
+          // source type.
+          return type == ObjectFile::eTypeObjectFile ||
+                 type == ObjectFile::eTypeExecutable;
+        });
+
+    // If a module was torn down it will have torn
+    // down the 'TypeSystem's that we used as source
+    // 'ASTContext's for the persistent variables
+    // in the current target. Those would now be
+    // unsafe to access because the 'DeclOrigin'
+    // are now possibly stale. Thus clear all
+    // persistent variables.
+    if (should_flush_type_systems) {
+      m_scratch_type_system_map.Clear();
+    }
   }
 }
 
Index: lldb/source/Core/ModuleList.cpp
===================================================================
--- lldb/source/Core/ModuleList.cpp
+++ lldb/source/Core/ModuleList.cpp
@@ -1073,3 +1073,14 @@
       break;
   }
 }
+
+bool ModuleList::AllOf(
+    std::function<bool(const ModuleSP &module_sp)> const &callback) const {
+  bool ret = true;
+  ForEach([&](const ModuleSP &module_sp) {
+    ret &= callback(module_sp);
+    return ret;
+  });
+
+  return ret;
+}
Index: lldb/include/lldb/Core/ModuleList.h
===================================================================
--- lldb/include/lldb/Core/ModuleList.h
+++ lldb/include/lldb/Core/ModuleList.h
@@ -467,6 +467,13 @@
   void ForEach(std::function<bool(const lldb::ModuleSP &module_sp)> const
                    &callback) const;
 
+  /// Returns true if 'callback' returns true for every module
+  /// in this ModuleList.
+  ///
+  /// This function is thread-safe.
+  bool AllOf(std::function<bool(const lldb::ModuleSP &module_sp)> const
+                 &callback) const;
+
 protected:
   // Class typedefs.
   typedef std::vector<lldb::ModuleSP>
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to