Michael137 updated this revision to Diff 479190.
Michael137 added a comment.
- Reflow comment
- Remove redundant null-check
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
@@ -1682,6 +1682,31 @@
m_breakpoint_list.UpdateBreakpoints(module_list, false, delete_locations);
m_internal_breakpoint_list.UpdateBreakpoints(module_list, false,
delete_locations);
+
+ // If a module was torn down it will have torn down the 'TypeSystemClang'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. We only want to flush 'TypeSystem's if the module being
+ // unloaded was capable of describing a source type. JITted module unloads
+ // happen frequently for Objective-C utility functions or the REPL and rely
+ // on the persistent variables to stick around.
+ const bool should_flush_type_systems =
+ module_list.AllOf([](const lldb::ModuleSP &module_sp) {
+ auto *object_file = module_sp->GetObjectFile();
+
+ if (!object_file)
+ return true;
+
+ auto type = object_file->GetType();
+
+ return type == ObjectFile::eTypeObjectFile ||
+ type == ObjectFile::eTypeExecutable;
+ });
+
+ 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
@@ -1074,3 +1074,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
@@ -473,6 +473,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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
