Auto merge of #148685 - Zalathar:lldb-python, r=<try>

compiletest: Run the `lldb_batchmode.py` script in LLDB's embedded Python

try-job: aarch64-apple

Author: rust-bors[bot]

bootstrap.example.toml

@@ -320,7 +320,7 @@
 #build.npm = "npm"
 
 # Python interpreter to use for various tasks throughout the build, notably
-# rustdoc tests, the lldb python interpreter, and some dist bits and pieces.
+# rustdoc tests, and some dist bits and pieces.
 #
 # Defaults to the Python interpreter used to execute x.py.
 #build.python = "python"

src/bootstrap/src/core/build_steps/test.rs

@@ -2094,11 +2094,11 @@ Please disable assertions with `rust.debug-assertions = false`.
                 cmd.arg("--gdb").arg(gdb);
             }
 
-            if let Some(debuggers::Lldb { lldb_version, lldb_python_dir }) =
+            if let Some(debuggers::Lldb { lldb_exe, lldb_version }) =
                 debuggers::discover_lldb(builder)
             {
+                cmd.arg("--lldb").arg(lldb_exe);
                 cmd.arg("--lldb-version").arg(lldb_version);
-                cmd.arg("--lldb-python-dir").arg(lldb_python_dir);
             }
         }
 

src/bootstrap/src/core/debuggers/lldb.rs

@@ -4,8 +4,8 @@ use crate::core::builder::Builder;
 use crate::utils::exec::command;
 
 pub(crate) struct Lldb {
+    pub(crate) lldb_exe: PathBuf,
     pub(crate) lldb_version: String,
-    pub(crate) lldb_python_dir: String,
 }
 
 pub(crate) fn discover_lldb(builder: &Builder<'_>) -> Option<Lldb> {
@@ -21,12 +21,5 @@ pub(crate) fn discover_lldb(builder: &Builder<'_>) -> Option<Lldb> {
         .stdout_if_ok()
         .and_then(|v| if v.trim().is_empty() { None } else { Some(v) })?;
 
-    let lldb_python_dir = command(&lldb_exe)
-        .allow_failure()
-        .arg("-P")
-        .run_capture_stdout(builder)
-        .stdout_if_ok()
-        .map(|p| p.lines().next().expect("lldb Python dir not found").to_string())?;
-
-    Some(Lldb { lldb_version, lldb_python_dir })
+    Some(Lldb { lldb_exe, lldb_version })
 }

src/etc/lldb_batchmode.py

@@ -176,77 +176,85 @@ def watchdog():
     watchdog_thread.start()
 
 
+def get_env_arg(name):
+    value = os.environ.get(name)
+    if value is None:
+        print("must set %s" % name)
+        sys.exit(1)
+    return value
+
+
 ####################################################################################################
 # ~main
 ####################################################################################################
 
 
-if len(sys.argv) != 3:
-    print("usage: python lldb_batchmode.py target-path script-path")
-    sys.exit(1)
+def main():
+    target_path = get_env_arg("LLDB_BATCHMODE_TARGET_PATH")
+    script_path = get_env_arg("LLDB_BATCHMODE_SCRIPT_PATH")
 
-target_path = sys.argv[1]
-script_path = sys.argv[2]
+    print("LLDB batch-mode script")
+    print("----------------------")
+    print("Debugger commands script is '%s'." % script_path)
+    print("Target executable is '%s'." % target_path)
+    print("Current working directory is '%s'" % os.getcwd())
 
-print("LLDB batch-mode script")
-print("----------------------")
-print("Debugger commands script is '%s'." % script_path)
-print("Target executable is '%s'." % target_path)
-print("Current working directory is '%s'" % os.getcwd())
+    # Start the timeout watchdog
+    start_watchdog()
 
-# Start the timeout watchdog
-start_watchdog()
+    # Create a new debugger instance
+    debugger = lldb.SBDebugger.Create()
 
-# Create a new debugger instance
-debugger = lldb.SBDebugger.Create()
+    # When we step or continue, don't return from the function until the process
+    # stops. We do this by setting the async mode to false.
+    debugger.SetAsync(False)
 
-# When we step or continue, don't return from the function until the process
-# stops. We do this by setting the async mode to false.
-debugger.SetAsync(False)
+    # Create a target from a file and arch
+    print("Creating a target for '%s'" % target_path)
+    target_error = lldb.SBError()
+    target = debugger.CreateTarget(target_path, None, None, True, target_error)
 
-# Create a target from a file and arch
-print("Creating a target for '%s'" % target_path)
-target_error = lldb.SBError()
-target = debugger.CreateTarget(target_path, None, None, True, target_error)
-
-if not target:
-    print(
-        "Could not create debugging target '"
-        + target_path
-        + "': "
-        + str(target_error)
-        + ". Aborting.",
-        file=sys.stderr,
-    )
-    sys.exit(1)
+    if not target:
+        print(
+            "Could not create debugging target '"
+            + target_path
+            + "': "
+            + str(target_error)
+            + ". Aborting.",
+            file=sys.stderr,
+        )
+        sys.exit(1)
 
+    # Register the breakpoint callback for every breakpoint
+    start_breakpoint_listener(target)
 
-# Register the breakpoint callback for every breakpoint
-start_breakpoint_listener(target)
+    command_interpreter = debugger.GetCommandInterpreter()
 
-command_interpreter = debugger.GetCommandInterpreter()
-
-try:
-    script_file = open(script_path, "r")
-
-    for line in script_file:
-        command = line.strip()
-        if (
-            command == "run"
-            or command == "r"
-            or re.match(r"^process\s+launch.*", command)
-        ):
-            # Before starting to run the program, let the thread sleep a bit, so all
-            # breakpoint added events can be processed
-            time.sleep(0.5)
-        if command != "":
-            execute_command(command_interpreter, command)
-
-except IOError as e:
-    print("Could not read debugging script '%s'." % script_path, file=sys.stderr)
-    print(e, file=sys.stderr)
-    print("Aborting.", file=sys.stderr)
-    sys.exit(1)
-finally:
-    debugger.Terminate()
-    script_file.close()
+    try:
+        script_file = open(script_path, "r")
+
+        for line in script_file:
+            command = line.strip()
+            if (
+                command == "run"
+                or command == "r"
+                or re.match(r"^process\s+launch.*", command)
+            ):
+                # Before starting to run the program, let the thread sleep a bit, so all
+                # breakpoint added events can be processed
+                time.sleep(0.5)
+            if command != "":
+                execute_command(command_interpreter, command)
+
+    except IOError as e:
+        print("Could not read debugging script '%s'." % script_path, file=sys.stderr)
+        print(e, file=sys.stderr)
+        print("Aborting.", file=sys.stderr)
+        sys.exit(1)
+    finally:
+        debugger.Terminate()
+        script_file.close()
+
+
+if __name__ == "__main__":
+    main()

src/tools/compiletest/src/common.rs

@@ -311,10 +311,7 @@ pub struct Config {
     /// Path to the `src/tools/coverage-dump/` bootstrap tool executable.
     pub coverage_dump_path: Option<Utf8PathBuf>,
 
-    /// Path to the Python 3 executable to use for LLDB and htmldocck.
-    ///
-    /// FIXME: the `lldb` setup currently requires I believe Python 3.10 **exactly**, it can't even
-    /// be Python 3.11 or 3.9...
+    /// Path to the Python 3 executable to use for htmldocck and some run-make tests.
     pub python: String,
 
     /// Path to the `src/tools/jsondocck/` bootstrap tool executable.
@@ -540,6 +537,9 @@ pub struct Config {
     /// FIXME: `gdb_version` is *derived* from gdb, but it's *not* technically a config!
     pub gdb_version: Option<u32>,
 
+    /// Path to or name of the LLDB executable to use for debuginfo tests.
+    pub lldb: Option<Utf8PathBuf>,
+
     /// Version of LLDB.
     ///
     /// FIXME: `lldb_version` is *derived* from lldb, but it's *not* technically a config!
@@ -586,11 +586,6 @@ pub struct Config {
     /// FIXME: take a look at this; this also influences adb in gdb code paths in a strange way.
     pub adb_device_status: bool,
 
-    /// Path containing LLDB's Python module.
-    ///
-    /// FIXME: `PYTHONPATH` takes precedence over this flag...? See `runtest::run_lldb`.
-    pub lldb_python_dir: Option<String>,
-
     /// Verbose dump a lot of info.
     ///
     /// FIXME: this is *way* too coarse; the user can't select *which* info to verbosely dump.

src/tools/compiletest/src/debuggers.rs

@@ -49,7 +49,7 @@ pub(crate) fn configure_gdb(config: &Config) -> Option<Arc<Config>> {
 }
 
 pub(crate) fn configure_lldb(config: &Config) -> Option<Arc<Config>> {
-    config.lldb_python_dir.as_ref()?;
+    config.lldb.as_ref()?;
 
     Some(Arc::new(Config { debugger: Some(Debugger::Lldb), ..config.clone() }))
 }

src/tools/compiletest/src/lib.rs

@@ -141,13 +141,13 @@ fn parse_config(args: Vec<String>) -> Config {
         .optopt("", "host", "the host to build for", "HOST")
         .optopt("", "cdb", "path to CDB to use for CDB debuginfo tests", "PATH")
         .optopt("", "gdb", "path to GDB to use for GDB debuginfo tests", "PATH")
+        .optopt("", "lldb", "path to LLDB to use for LLDB debuginfo tests", "PATH")
         .optopt("", "lldb-version", "the version of LLDB used", "VERSION STRING")
         .optopt("", "llvm-version", "the version of LLVM used", "VERSION STRING")
         .optflag("", "system-llvm", "is LLVM the system LLVM")
         .optopt("", "android-cross-path", "Android NDK standalone path", "PATH")
         .optopt("", "adb-path", "path to the android debugger", "PATH")
         .optopt("", "adb-test-dir", "path to tests for the android debugger", "PATH")
-        .optopt("", "lldb-python-dir", "directory containing LLDB's python module", "PATH")
         .reqopt("", "cc", "path to a C compiler", "PATH")
         .reqopt("", "cxx", "path to a C++ compiler", "PATH")
         .reqopt("", "cflags", "flags for the C compiler", "FLAGS")
@@ -264,6 +264,7 @@ fn parse_config(args: Vec<String>) -> Config {
     let gdb = debuggers::discover_gdb(matches.opt_str("gdb"), &target, &android_cross_path);
     let gdb_version = gdb.as_deref().and_then(debuggers::query_gdb_version);
     // FIXME: `lldb_version` is *derived* from lldb, but it's *not* technically a config!
+    let lldb = matches.opt_str("lldb").map(Utf8PathBuf::from);
     let lldb_version =
         matches.opt_str("lldb-version").as_deref().and_then(debuggers::extract_lldb_version);
     let color = match matches.opt_str("color").as_deref() {
@@ -435,6 +436,7 @@ fn parse_config(args: Vec<String>) -> Config {
         cdb_version,
         gdb,
         gdb_version,
+        lldb,
         lldb_version,
         llvm_version,
         system_llvm: matches.opt_present("system-llvm"),
@@ -444,7 +446,6 @@ fn parse_config(args: Vec<String>) -> Config {
         adb_device_status: opt_str2(matches.opt_str("target")).contains("android")
             && "(none)" != opt_str2(matches.opt_str("adb-test-dir"))
             && !opt_str2(matches.opt_str("adb-test-dir")).is_empty(),
-        lldb_python_dir: matches.opt_str("lldb-python-dir"),
         verbose: matches.opt_present("verbose"),
         only_modified: matches.opt_present("only-modified"),
         color,

src/tools/compiletest/src/runtest.rs

@@ -200,10 +200,11 @@ pub fn compute_stamp_hash(config: &Config) -> String {
         }
 
         Some(Debugger::Lldb) => {
-            config.python.hash(&mut hash);
-            config.lldb_python_dir.hash(&mut hash);
+            // LLDB debuginfo tests now use LLDB's embedded Python, with an
+            // explicit PYTHONPATH, so they don't depend on `--python` or
+            // the ambient PYTHONPATH.
+            config.lldb.hash(&mut hash);
             env::var_os("PATH").hash(&mut hash);
-            env::var_os("PYTHONPATH").hash(&mut hash);
         }
 
         None => {}

src/tools/compiletest/src/runtest/debuginfo.rs

@@ -326,9 +326,9 @@ impl TestCx<'_> {
     }
 
     fn run_debuginfo_lldb_test(&self) {
-        if self.config.lldb_python_dir.is_none() {
-            self.fatal("Can't run LLDB test because LLDB's python path is not set.");
-        }
+        let Some(ref lldb) = self.config.lldb else {
+            self.fatal("Can't run LLDB test because LLDB's path is not set.");
+        };
 
         // compile test file (it should have 'compile-flags:-g' in the directive)
         let should_run = self.run_if_enabled();
@@ -434,7 +434,7 @@ impl TestCx<'_> {
         let debugger_script = self.make_out_name("debugger.script");
 
         // Let LLDB execute the script via lldb_batchmode.py
-        let debugger_run_result = self.run_lldb(&exe_file, &debugger_script);
+        let debugger_run_result = self.run_lldb(lldb, &exe_file, &debugger_script);
 
         if !debugger_run_result.status.success() {
             self.fatal_proc_rec("Error while running LLDB", &debugger_run_result);
@@ -445,23 +445,23 @@ impl TestCx<'_> {
         }
     }
 
-    fn run_lldb(&self, test_executable: &Utf8Path, debugger_script: &Utf8Path) -> ProcRes {
-        // Prepare the lldb_batchmode which executes the debugger script
-        let lldb_script_path = self.config.src_root.join("src/etc/lldb_batchmode.py");
-
-        // FIXME: `PYTHONPATH` takes precedence over the flag...?
-        let pythonpath = if let Ok(pp) = std::env::var("PYTHONPATH") {
-            format!("{pp}:{}", self.config.lldb_python_dir.as_ref().unwrap())
-        } else {
-            self.config.lldb_python_dir.clone().unwrap()
-        };
-        self.run_command_to_procres(
-            Command::new(&self.config.python)
-                .arg(&lldb_script_path)
-                .arg(test_executable)
-                .arg(debugger_script)
-                .env("PYTHONUNBUFFERED", "1") // Help debugging #78665
-                .env("PYTHONPATH", pythonpath),
-        )
+    fn run_lldb(
+        &self,
+        lldb: &Utf8Path,
+        test_executable: &Utf8Path,
+        debugger_script: &Utf8Path,
+    ) -> ProcRes {
+        // Path containing `lldb_batchmode.py`, so that the `script` command can import it.
+        let pythonpath = self.config.src_root.join("src/etc");
+
+        let mut cmd = Command::new(lldb);
+        cmd.arg("--one-line")
+            .arg("script -- import lldb_batchmode; lldb_batchmode.main()")
+            .env("LLDB_BATCHMODE_TARGET_PATH", test_executable)
+            .env("LLDB_BATCHMODE_SCRIPT_PATH", debugger_script)
+            .env("PYTHONUNBUFFERED", "1") // Help debugging #78665
+            .env("PYTHONPATH", pythonpath);
+
+        self.run_command_to_procres(&mut cmd)
     }
 }

src/tools/compiletest/src/rustdoc_gui_test.rs

@@ -99,14 +99,14 @@ fn incomplete_config_for_rustdoc_gui_test() -> Config {
         cdb_version: Default::default(),
         gdb: Default::default(),
         gdb_version: Default::default(),
+        lldb: Default::default(),
         lldb_version: Default::default(),
         llvm_version: Default::default(),
         system_llvm: Default::default(),
         android_cross_path: Default::default(),
         adb_path: Default::default(),
         adb_test_dir: Default::default(),
         adb_device_status: Default::default(),
-        lldb_python_dir: Default::default(),
         verbose: Default::default(),
         color: Default::default(),
         remote_test_client: Default::default(),