libdrgn: always create all modules in load_debug_info()

For the Linux kernel at least, it's important to have modules populated
even if there is no debuginfo available, because the modules are
necessary to load the built-in ORC data for unwinding.

The break statement here intentionally prevents exhausting the module
iterator, so that the Linux kernel modules don't get created. It doesn't
impact functionality afterwards: only modules that we want to find debug
info for are added to the module vector. So as far as I can see, this is
only a performance optimization.

The documentation for load_debug_info() only states "Try to load all
debugging information for all loaded modules", which doesn't really
specify whether the drgn Modules are actually created. By dropping this
break statement, we may slightly increase runtime for the --main-symbols
case, but we also improve correctness for those same cases, by ensuring
that stack tracing (via ORC) can work without DWARF debug info.

Closes #505

Signed-off-by: Stephen Brennan <[email protected]>

Author: brenns10

libdrgn/debug_info.c

@@ -5362,17 +5362,6 @@ drgn_program_load_debug_info(struct drgn_program *prog, const char **paths,
 			    && drgn_module_kind(module) == DRGN_MODULE_MAIN))
 		    && !drgn_module_vector_append(&modules, &module))
 			return &drgn_enomem;
-
-		// If we are only trying files for the main module (i.e., if
-		// we're not loading all default debug info and any provided
-		// files were all for the main module), then we only want to
-		// create the main module.
-		if (!load_default
-		    && drgn_module_kind(module) == DRGN_MODULE_MAIN
-		    && state.unmatched_provided == 0) {
-			err = NULL;
-			break;
-		}
 	}
 	if (err)
 		return err;

tests/test_debug_info.py

@@ -971,8 +971,8 @@ def test_only_main_path(self):
 
         self.prog.load_debug_info([crashme_path])
 
-        # Only the main module should be created.
-        self.assertEqual(list(self.prog.modules()), [self.prog.main_module()])
+        # The main module should be created.
+        self.assertIn(self.prog.main_module(), list(self.prog.modules()))
         # The provided path should be used for the main module.
         self.assertEqual(
             self.prog.main_module().loaded_file_path,
@@ -1027,8 +1027,8 @@ def test_main_by_path(self):
 
         self.prog.load_debug_info([crashme_path], main=True)
 
-        # Only the main module should be created.
-        self.assertEqual(list(self.prog.modules()), [self.prog.main_module()])
+        # The main module should be created.
+        self.assertIn(self.prog.main_module(), list(self.prog.modules()))
         # The provided path should be used for the main module.
         self.assertEqual(
             self.prog.main_module().loaded_file_path,
@@ -1053,8 +1053,8 @@ def finder(modules):
 
         self.prog.load_debug_info(main=True)
 
-        # Only the main module should be created.
-        self.assertEqual(list(self.prog.modules()), [self.prog.main_module()])
+        # The main module should be created.
+        self.assertIn(self.prog.main_module(), list(self.prog.modules()))
         # The finder should be called and set the file for the main module.
         self.assertEqual(
             self.prog.main_module().loaded_file_path,
@@ -1168,8 +1168,8 @@ def test_main_gnu_debugaltlink_by_path(self):
 
         self.prog.load_debug_info([crashme_dwz_path, crashme_alt_path], main=True)
 
-        # Only the main module should be created.
-        self.assertEqual(list(self.prog.modules()), [self.prog.main_module()])
+        # The main module should be created.
+        self.assertIn(self.prog.main_module(), list(self.prog.modules()))
         # The provided paths should be used for the main module.
         self.assertEqual(
             self.prog.main_module().loaded_file_path,
@@ -1200,8 +1200,8 @@ def finder(modules):
 
         self.prog.load_debug_info(main=True)
 
-        # Only the main module should be created.
-        self.assertEqual(list(self.prog.modules()), [self.prog.main_module()])
+        # The main module should be created.
+        self.assertIn(self.prog.main_module(), list(self.prog.modules()))
         # The finder should be called and set the files for the main module.
         self.assertEqual(
             self.prog.main_module().loaded_file_path,
@@ -1236,8 +1236,8 @@ def finder(modules):
             main=True,
         )
 
-        # Only the main module should be created.
-        self.assertEqual(list(self.prog.modules()), [self.prog.main_module()])
+        # The main module should be created.
+        self.assertIn(self.prog.main_module(), list(self.prog.modules()))
         # The provided path should be used for the loaded file.
         self.assertEqual(
             self.prog.main_module().loaded_file_path,
@@ -1275,8 +1275,8 @@ def finder(modules):
 
         self.assertRaises(MissingDebugInfoError, self.prog.load_debug_info, main=True)
 
-        # Only the main module should be created.
-        self.assertEqual(list(self.prog.modules()), [self.prog.main_module()])
+        # The main module should be created.
+        self.assertIn(self.prog.main_module(), list(self.prog.modules()))
         # The finder should be called and set the loaded file for the main
         # module but fail to find the supplementary file.
         self.assertEqual(
@@ -1313,8 +1313,8 @@ def finder(modules):
 
         self.prog.load_debug_info([crashme_dwz_path], main=True)
 
-        # Only the main module should be created.
-        self.assertEqual(list(self.prog.modules()), [self.prog.main_module()])
+        # The main module should be created.
+        self.assertIn(self.prog.main_module(), list(self.prog.modules()))
         # The provided path should be used for the main module.
         self.assertEqual(
             self.prog.main_module().loaded_file_path,
@@ -1381,8 +1381,8 @@ def test_main_wants_gnu_debugaltlink_by_path(self):
 
         self.prog.load_debug_info([crashme_alt_path], main=True)
 
-        # Only the main module should be created.
-        self.assertEqual(list(self.prog.modules()), [self.prog.main_module()])
+        # The main module should be created.
+        self.assertIn(self.prog.main_module(), list(self.prog.modules()))
         # The provided path should be used for the supplementary file.
         self.assertEqual(
             self.prog.main_module().supplementary_debug_file_path,
@@ -1420,8 +1420,8 @@ def finder(modules):
 
         self.prog.load_debug_info(main=True)
 
-        # Only the main module should be created.
-        self.assertEqual(list(self.prog.modules()), [self.prog.main_module()])
+        # The main module should be created.
+        self.assertIn(self.prog.main_module(), list(self.prog.modules()))
         # The finder should be called and set the supplementary file for the
         # main module.
         self.assertEqual(
@@ -1448,8 +1448,8 @@ def test_main_wants_gnu_debugaltlink_not_found(self):
 
         self.assertRaises(MissingDebugInfoError, self.prog.load_debug_info, main=True)
 
-        # Only the main module should be created.
-        self.assertEqual(list(self.prog.modules()), [self.prog.main_module()])
+        # The main module should be created.
+        self.assertIn(self.prog.main_module(), list(self.prog.modules()))
         # The finder should be called and fail to find the supplementary file
         # for the main module, but the supplementary file should still be
         # wanted.
@@ -1570,8 +1570,8 @@ def finder(modules):
 
         self.prog.load_debug_info(main=True)
 
-        # Only the main module should be created.
-        self.assertEqual(list(self.prog.modules()), [self.prog.main_module()])
+        # The main module should be created.
+        self.assertIn(self.prog.main_module(), list(self.prog.modules()))
         # The finder should be called and set the files, address range, and
         # build ID for the main module.
         self.assertEqual(