[Lldb-commits] [lldb] e1edcf7 - Reland "[lldb][Target] Flush the scratch TypeSystem when owning lldb_private::Module gets unloaded"
Michael Buch via lldb-commits
lldb-commits at lists.llvm.org
Mon Dec 5 08:58:10 PST 2022
Author: Michael Buch
Date: 2022-12-05T16:57:42Z
New Revision: e1edcf7d14c126b9ebd2a77fcd9041d056cce64a
URL: https://github.com/llvm/llvm-project/commit/e1edcf7d14c126b9ebd2a77fcd9041d056cce64a
DIFF: https://github.com/llvm/llvm-project/commit/e1edcf7d14c126b9ebd2a77fcd9041d056cce64a.diff
LOG: Reland "[lldb][Target] Flush the scratch TypeSystem when owning lldb_private::Module gets unloaded"
This relands commit `71f3cac7895ad516ec25438f803ed3c9916c215a`
Fixes LLDB Linux bots and improves TypeSystem flushing for shared libraries.
Differential Revision: https://reviews.llvm.org/D138724
Added:
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
lldb/test/API/functionalities/rerun_and_expr_dylib/Makefile
lldb/test/API/functionalities/rerun_and_expr_dylib/TestRerunAndExprDylib.py
lldb/test/API/functionalities/rerun_and_expr_dylib/lib.cpp
lldb/test/API/functionalities/rerun_and_expr_dylib/main.cpp
lldb/test/API/functionalities/rerun_and_expr_dylib/rebuild.cpp
Modified:
lldb/source/Target/Target.cpp
Removed:
################################################################################
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index bbb685f117ab8..c4275db2c8a73 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -1682,6 +1682,34 @@ void Target::ModulesDidUnload(ModuleList &module_list, bool delete_locations) {
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.AnyOf([](lldb_private::Module &module) {
+ auto *object_file = module.GetObjectFile();
+
+ if (!object_file)
+ return false;
+
+ auto type = object_file->GetType();
+
+ // eTypeExecutable: when debugged binary was rebuilt
+ // eTypeSharedLibrary: if dylib was re-loaded
+ return module.FileHasChanged() &&
+ (type == ObjectFile::eTypeObjectFile ||
+ type == ObjectFile::eTypeExecutable ||
+ type == ObjectFile::eTypeSharedLibrary);
+ });
+
+ if (should_flush_type_systems)
+ m_scratch_type_system_map.Clear();
}
}
diff --git a/lldb/test/API/functionalities/rerun_and_expr/Makefile b/lldb/test/API/functionalities/rerun_and_expr/Makefile
new file mode 100644
index 0000000000000..22f1051530f87
--- /dev/null
+++ b/lldb/test/API/functionalities/rerun_and_expr/Makefile
@@ -0,0 +1 @@
+include Makefile.rules
diff --git a/lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py b/lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py
new file mode 100644
index 0000000000000..ebf26e50091b3
--- /dev/null
+++ b/lldb/test/API/functionalities/rerun_and_expr/TestRerunAndExpr.py
@@ -0,0 +1,62 @@
+"""
+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 TestRerunExpr(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'})
+
+ exe = self.getBuildArtifact("a.out")
+ target = self.dbg.CreateTarget(exe)
+ target.BreakpointCreateBySourceRegex('return', lldb.SBFileSpec('rebuild.cpp'))
+ target.BreakpointCreateBySourceRegex('return', lldb.SBFileSpec('main.cpp'))
+ process = target.LaunchSimple(None, None, self.get_process_working_directory())
+
+ 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'})
+
+ # Rerun program within the same target
+ process.Destroy()
+ process = target.LaunchSimple(None, None, self.get_process_working_directory())
+
+ 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: | |-public 'Base'
+ # CHECK-NEXT: | `-FieldDecl {{.*}} m_derived_val 'int'
+ # CHECK-NEXT: `-CXXRecordDecl {{.*}} struct Base definition
+ # CHECK: `-FieldDecl {{.*}} m_base_val 'int'
+
+ # ...but the original definition of 'struct Foo' is not in the scratch AST anymore
+ # CHECK-NOT: FieldDecl {{.*}} m_val 'int'
+
diff --git a/lldb/test/API/functionalities/rerun_and_expr/main.cpp b/lldb/test/API/functionalities/rerun_and_expr/main.cpp
new file mode 100644
index 0000000000000..a19dac03ffa98
--- /dev/null
+++ b/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;
+}
diff --git a/lldb/test/API/functionalities/rerun_and_expr/rebuild.cpp b/lldb/test/API/functionalities/rerun_and_expr/rebuild.cpp
new file mode 100644
index 0000000000000..745a2ce954e89
--- /dev/null
+++ b/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;
+}
diff --git a/lldb/test/API/functionalities/rerun_and_expr_dylib/Makefile b/lldb/test/API/functionalities/rerun_and_expr_dylib/Makefile
new file mode 100644
index 0000000000000..99998b20bcb05
--- /dev/null
+++ b/lldb/test/API/functionalities/rerun_and_expr_dylib/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
diff --git a/lldb/test/API/functionalities/rerun_and_expr_dylib/TestRerunAndExprDylib.py b/lldb/test/API/functionalities/rerun_and_expr_dylib/TestRerunAndExprDylib.py
new file mode 100644
index 0000000000000..cf322215a65a1
--- /dev/null
+++ b/lldb/test/API/functionalities/rerun_and_expr_dylib/TestRerunAndExprDylib.py
@@ -0,0 +1,84 @@
+"""
+Test that re-running a process from within the same target
+after rebuilding the a dynamic library flushes the scratch
+TypeSystems tied to that process.
+"""
+
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+from lldbsuite.test.decorators import *
+
+class TestRerunExprDylib(TestBase):
+ @skipIfWindows
+ 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 dylib
+ 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.
+ """
+
+ DYLIB_NAME = 'foo'
+ FULL_DYLIB_NAME = 'libfoo.dylib' if self.platformIsDarwin() else 'libfoo.so'
+
+ # Build libfoo.dylib
+ self.build(dictionary={'DYLIB_CXX_SOURCES':'lib.cpp',
+ 'DYLIB_ONLY':'YES',
+ 'DYLIB_NAME':DYLIB_NAME,
+ 'USE_LIBDL':'1',
+ 'LD_EXTRAS':'-L.'})
+
+ # Build a.out
+ self.build(dictionary={'EXE':'a.out',
+ 'CXX_SOURCES':'main.cpp',
+ 'USE_LIBDL':'1',
+ 'CXXFLAGS_EXTRAS':f'-DLIB_NAME=\\"{FULL_DYLIB_NAME}\\"',
+ 'LD_EXTRAS':'-L.'})
+
+ exe = self.getBuildArtifact("a.out")
+ target = self.dbg.CreateTarget(exe)
+ breakpoint = target.BreakpointCreateBySourceRegex('return', lldb.SBFileSpec('main.cpp'))
+ process = target.LaunchSimple(None, None, self.get_process_working_directory())
+
+ self.expect_expr('*foo', result_type='Foo', result_children=[
+ ValueCheck(name='m_val', value='42')
+ ])
+
+ # Re-build libfoo.dylib
+ self.build(dictionary={'DYLIB_CXX_SOURCES':'rebuild.cpp',
+ 'DYLIB_ONLY':'YES',
+ 'DYLIB_NAME':DYLIB_NAME,
+ 'USE_LIBDL':'1',
+ 'LD_EXTRAS':'-L.'})
+
+ # Rerun program within the same target
+ process.Destroy()
+ process = target.LaunchSimple(None, None, self.get_process_working_directory())
+
+ 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: | |-public 'Base'
+ # CHECK-NEXT: | `-FieldDecl {{.*}} m_derived_val 'int'
+ # CHECK-NEXT: `-CXXRecordDecl {{.*}} struct Base definition
+ # CHECK: `-FieldDecl {{.*}} m_base_val 'int'
+
+ # ...but the original definition of 'struct Foo' is not in the scratch AST anymore
+ # CHECK-NOT: FieldDecl {{.*}} m_val 'int'
+
diff --git a/lldb/test/API/functionalities/rerun_and_expr_dylib/lib.cpp b/lldb/test/API/functionalities/rerun_and_expr_dylib/lib.cpp
new file mode 100644
index 0000000000000..aad9816a4be27
--- /dev/null
+++ b/lldb/test/API/functionalities/rerun_and_expr_dylib/lib.cpp
@@ -0,0 +1 @@
+LLDB_DYLIB_EXPORT struct Foo { int m_val = 42; } global_foo;
diff --git a/lldb/test/API/functionalities/rerun_and_expr_dylib/main.cpp b/lldb/test/API/functionalities/rerun_and_expr_dylib/main.cpp
new file mode 100644
index 0000000000000..bbd218bef66c2
--- /dev/null
+++ b/lldb/test/API/functionalities/rerun_and_expr_dylib/main.cpp
@@ -0,0 +1,17 @@
+#include <cassert>
+#include <dlfcn.h>
+#include <string>
+
+extern struct Foo imported;
+
+int main() {
+ // LIB_NAME defined on commandline
+ std::string libname{"./"};
+ libname += LIB_NAME;
+
+ void *handle = dlopen(libname.c_str(), RTLD_NOW);
+ struct Foo *foo = (struct Foo *)dlsym(handle, "global_foo");
+ assert(foo != nullptr);
+
+ return 0;
+}
diff --git a/lldb/test/API/functionalities/rerun_and_expr_dylib/rebuild.cpp b/lldb/test/API/functionalities/rerun_and_expr_dylib/rebuild.cpp
new file mode 100644
index 0000000000000..b9d835184dde8
--- /dev/null
+++ b/lldb/test/API/functionalities/rerun_and_expr_dylib/rebuild.cpp
@@ -0,0 +1,7 @@
+struct Base {
+ int m_base_val = 42;
+};
+
+LLDB_DYLIB_EXPORT struct Foo : public Base {
+ int m_derived_val = 137;
+} global_foo;
More information about the lldb-commits
mailing list