refactor: isolate PTX pass execution in runner by yunwei37 · Pull Request #583 · eunomia-bpf/bpftime

yunwei37 · 2026-06-15T21:07:21Z

Summary

Split the PTX pass isolation change from the GPU runtime race fix.

add a dedicated bpftime_ptxpass_runner executable for PTX pass print_config and process_input calls
run pass libraries in a subprocess with LD_PRELOAD and LD_AUDIT cleared
reserve runner stdout for JSON by redirecting pass stdout to stderr
restore executable permissions when artifact transfer drops the executable bit

Current status

This remains a draft and is not merge-ready. The prerequisite GPU race fix was merged in #582, but this branch now conflicts with master. After rebasing, the remaining non-outdated review findings must be addressed: bounded process output allocation, inherited file descriptors in the spawned runner, and config pointer constness.

Validation recorded on the branch

git diff --check
configured and built the GPU/runner targets with CUDA 12.9 and Ninja
the earlier combined branch passed its checks before this change was split out

Copilot

Pull request overview

This PR introduces a dedicated bpftime_ptxpass_runner executable to execute PTX pass print_config/process_input outside the CUDA target process, aiming to improve isolation and reduce coupling between injected CUDA runtime behavior and PTX pass dynamic loading/execution.

Changes:

Add bpftime_ptxpass_runner binary that dlopen()s a pass library and invokes print_config / process_input, keeping stdout reserved for JSON output.
Update nv_attach_impl to invoke PTX passes via the runner subprocess (clearing LD_PRELOAD/LD_AUDIT), instead of dlmopen/dlsym in-process.
Extend build config to generate and use DEFAULT_PTX_PASS_RUNNER_EXECUTABLE.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
attach/nv_attach_impl/pass/ptxpass_runner.cpp	New runner executable that loads PTX pass libs and prints JSON results to stdout while redirecting pass stdout to stderr.
attach/nv_attach_impl/nv_attach_impl.hpp	Removes in-process PTX pass function pointer/handle storage from pass config struct.
attach/nv_attach_impl/nv_attach_impl.cpp	Adds subprocess-based PTX pass execution helper and switches config/patch paths to use it.
attach/nv_attach_impl/CMakeLists.txt	Builds the new runner and emits its path into generated config header.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+	std::vector<std::string> args = { mode, pass_library.string() };
+	boost::process::environment child_env =
+		boost::this_process::environment();
+	child_env["LD_AUDIT"] = "";
+	child_env["LD_PRELOAD"] = "";
+	if (access(runner_path.c_str(), X_OK) != 0 && errno == EACCES) {
+		std::error_code ec;
+		std::filesystem::permissions(
+			runner_path,
+			std::filesystem::perms::owner_exec |
+				std::filesystem::perms::group_exec |
+				std::filesystem::perms::others_exec,
+			std::filesystem::perm_options::add, ec);


+	try {
+		boost::process::child child(
+			runner_path.string(), boost::process::args(args),
+			boost::process::env = child_env,
+			boost::process::std_in < stdin_path.string(),
+			boost::process::std_out > stdout_path.string(),
+			boost::process::std_err > stderr_path.string());
+		child.wait();


Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

+			if (!parse_size(argv[++i], output_bytes) ||
+			    output_bytes == 0 ||
+			    output_bytes >
+				    (size_t)std::numeric_limits<int>::max()) {
+				std::cerr << "Invalid --output-bytes value\n";
+				return 64;
+			}


+	if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) {
+		std::ifstream efs(stderr_path);
+		std::string err((std::istreambuf_iterator<char>(efs)),
+				std::istreambuf_iterator<char>());
+		SPDLOG_ERROR("PTX pass runner {} failed for {} with status {}: {}",
+			     runner_path.c_str(), pass_library.c_str(), status,
+			     err);
+		return std::nullopt;
+	}


 							current_ptx));
 			}
-			std::lock_guard<std::mutex> _guard(map_mutex);
 			ptx_out["patched." + file_name] = std::make_tuple(
 				current_ptx, should_add_trampoline);
-		});
+		}();


Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

+	int status = 0;
+	if (waitpid(child_pid, &status, 0) < 0) {
+		SPDLOG_ERROR("waitpid failed for PTX pass runner {}: {}",
+			     runner_path.c_str(), strerror(errno));
+		return std::nullopt;
+	}


+		auto json = nlohmann::json::parse(*output);
 		ptxpass::pass_config::from_json(json, config);
-		SPDLOG_INFO("Retrived config of {}", path.c_str());
+		SPDLOG_INFO("Retrieved config of {}", path.c_str());


Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

+	auto restore_stdout = [&]() -> bool {
+		fflush(stdout);
+		if (dup2(saved_stdout, STDOUT_FILENO) == -1) {
+			std::cerr << "Unable to restore stdout: "
+				  << std::strerror(errno) << "\n";
+			close(saved_stdout);
+			saved_stdout = -1;
+			return false;
+		}
+		close(saved_stdout);
+		saved_stdout = -1;
+		return true;
+	};


Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

+	const std::string mode = argv[1];
+	const char *library_path = argv[2];
+	size_t output_bytes = kDefaultOutputBytes;
+	for (int i = 3; i < argc; i++) {
+		if (std::strcmp(argv[i], "--output-bytes") == 0 &&
+		    i + 1 < argc) {
+			if (!parse_size(argv[++i], output_bytes) ||
+			    output_bytes == 0 || output_bytes > kMaxOutputBytes ||
+			    output_bytes >
+				    (size_t)std::numeric_limits<int>::max()) {
+				std::cerr << "Invalid --output-bytes value\n";
+				return 64;
+			}
+			continue;
+		}
+		usage(argv[0]);
+		return 64;
+	}
+


Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

 struct pass_cfg_with_exec_path {
 	std::filesystem::path executable_path;
 	ptxpass::pass_config::PassConfig pass_config;
-	print_config_fn print_config;
-	process_input_fn process_input;
-
-	void *handle;

 	pass_cfg_with_exec_path(std::filesystem::path path,
-				ptxpass::pass_config::PassConfig config,
-				print_config_fn print_config,
-				process_input_fn process_input, void *handle)
-		: executable_path(path), pass_config(config),
-		  print_config(print_config), process_input(process_input),
-		  handle(handle)
+				ptxpass::pass_config::PassConfig config)
+		: executable_path(path), pass_config(config)
 	{


Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

+constexpr size_t kDefaultConfigOutputBytes = 256U << 10;
+constexpr size_t kDefaultProcessOutputBytes = 64U << 20;
+constexpr size_t kMaxOutputBytes = 64U << 20;


Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

+	pid_t child_pid = -1;
+	spawn_rc = posix_spawnp(&child_pid, runner_path.c_str(), &file_actions,
+				nullptr, argv.data(), envp.data());
+	if (spawn_rc != 0) {
+		SPDLOG_ERROR("Unable to spawn PTX pass runner {} for {}: {}",
+			     runner_path.c_str(), pass_library.c_str(),
+			     strerror(spawn_rc));
+		return std::nullopt;
+	}


+		std::cout.write(output.data(),
+				strnlen(output.data(), output.size()));


+		std::cout.write(output.data(),
+				strnlen(output.data(), output.size()));


 	std::optional<std::string> extras;
-	struct pass_cfg_with_exec_path *config;
+	struct pass_cfg_with_library_path *config;
 };


Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

+		if (stdin_payload != nullptr)
+			ofs << *stdin_payload;


Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

+		SPDLOG_INFO("Found path: {}, loading config..", path.c_str());
+		ptxpass::pass_config::PassConfig config;
+		auto output = run_ptxpass_runner("--config", path, nullptr);
+		if (!output)
 			continue;


+							if (!output)
+								return;


Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

+		return 64;
+	}
+
+	fflush(stdout);


Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

yunwei37 · 2026-07-19T09:14:34Z

The PR title/body now match the current scope, and the review threads have been rechecked. This draft is currently blocked by a conflict with master plus non-outdated findings on the 64 MiB per-run output allocation, inherited file descriptors in the runner subprocess, and config pointer constness. Because conflict resolution can change the affected code, these should be fixed and re-reviewed after rebasing; the current head is not merge-ready.

AI-generated response; a maintainer will review and follow up later

fix gpu runtime attach races

a4d5e2c

Copilot AI review requested due to automatic review settings June 15, 2026 21:07

pull-request-size Bot added the size/L label Jun 15, 2026

Copilot started reviewing on behalf of yunwei37 June 15, 2026 21:07 View session

Copilot AI reviewed Jun 15, 2026

View reviewed changes

yunwei37 force-pushed the codex/ptxpass-runner-isolation branch from 2b2ec29 to c9d205a Compare June 16, 2026 00:33

yunwei37 requested a review from Copilot June 16, 2026 00:36

Copilot started reviewing on behalf of yunwei37 June 16, 2026 00:36 View session