diff --git a/.jules/bolt.md b/.jules/bolt.md new file mode 100644 index 0000000000000..d3420067bbfc2 --- /dev/null +++ b/.jules/bolt.md @@ -0,0 +1,3 @@ +## 2025-05-15 - Redundant data cloning in command execution +**Learning:** In the command execution module, cache hits were causing redundant clones of the entire captured stdout/stderr. By using `Arc<[u8]>` instead of `Vec` in `CommandOutput`, we can avoid copying large outputs when retrieving from the cache or when cloning the output object. Additionally, `fingerprint()` was being called redundantly on the failure path, which involves cloning all arguments and environment variables. +**Action:** Use `Arc` for shared data in performance-critical structures that are frequently cloned. Avoid redundant calculations of complex objects like command fingerprints by passing them down the call stack. diff --git a/.jules/sentinel.md b/.jules/sentinel.md new file mode 100644 index 0000000000000..0376421574f1d --- /dev/null +++ b/.jules/sentinel.md @@ -0,0 +1,4 @@ +## 2025-05-15 - No security issues found +**Vulnerability:** None identified in this session. +**Learning:** The bootstrap build system is relatively self-contained and doesn't handle sensitive user data or external network requests in a way that exposed common vulnerabilities. +**Prevention:** Continue to monitor for insecure use of `Command` and potential path traversal in build steps. diff --git a/src/bootstrap/src/utils/exec.rs b/src/bootstrap/src/utils/exec.rs index 61b8b26dceaf4..a261726041932 100644 --- a/src/bootstrap/src/utils/exec.rs +++ b/src/bootstrap/src/utils/exec.rs @@ -445,8 +445,8 @@ pub fn command>(program: S) -> BootstrapCommand { #[derive(Clone, PartialEq)] pub struct CommandOutput { status: CommandStatus, - stdout: Option>, - stderr: Option>, + stdout: Option>, + stderr: Option>, } impl CommandOutput { @@ -456,11 +456,11 @@ impl CommandOutput { status: CommandStatus::DidNotStartOrFinish, stdout: match stdout { OutputMode::Print => None, - OutputMode::Capture => Some(vec![]), + OutputMode::Capture => Some(Arc::from(vec![])), }, stderr: match stderr { OutputMode::Print => None, - OutputMode::Capture => Some(vec![]), + OutputMode::Capture => Some(Arc::from(vec![])), }, } } @@ -471,11 +471,11 @@ impl CommandOutput { status: CommandStatus::Finished(output.status), stdout: match stdout { OutputMode::Print => None, - OutputMode::Capture => Some(output.stdout), + OutputMode::Capture => Some(Arc::from(output.stdout)), }, stderr: match stderr { OutputMode::Print => None, - OutputMode::Capture => Some(output.stderr), + OutputMode::Capture => Some(Arc::from(output.stderr)), }, } } @@ -503,14 +503,17 @@ impl CommandOutput { #[must_use] pub fn stdout(&self) -> String { String::from_utf8( - self.stdout.clone().expect("Accessing stdout of a command that did not capture stdout"), + self.stdout + .as_ref() + .expect("Accessing stdout of a command that did not capture stdout") + .to_vec(), ) .expect("Cannot parse process stdout as UTF-8") } #[must_use] pub fn stdout_if_present(&self) -> Option { - self.stdout.as_ref().and_then(|s| String::from_utf8(s.clone()).ok()) + self.stdout.as_ref().and_then(|s| String::from_utf8(s.to_vec()).ok()) } #[must_use] @@ -521,14 +524,17 @@ impl CommandOutput { #[must_use] pub fn stderr(&self) -> String { String::from_utf8( - self.stderr.clone().expect("Accessing stderr of a command that did not capture stderr"), + self.stderr + .as_ref() + .expect("Accessing stderr of a command that did not capture stderr") + .to_vec(), ) .expect("Cannot parse process stderr as UTF-8") } #[must_use] pub fn stderr_if_present(&self) -> Option { - self.stderr.as_ref().and_then(|s| String::from_utf8(s.clone()).ok()) + self.stderr.as_ref().and_then(|s| String::from_utf8(s.to_vec()).ok()) } } @@ -536,8 +542,8 @@ impl Default for CommandOutput { fn default() -> Self { Self { status: CommandStatus::Finished(ExitStatus::default()), - stdout: Some(vec![]), - stderr: Some(vec![]), + stdout: Some(Arc::from(vec![])), + stderr: Some(Arc::from(vec![])), } } } @@ -826,8 +832,15 @@ impl<'a> DeferredCommand<'a> { } => { let exec_ctx = exec_ctx.as_ref(); - let output = - Self::finish_process(process, command, stdout, stderr, executed_at, exec_ctx); + let output = Self::finish_process( + process, + command, + stdout, + stderr, + executed_at, + exec_ctx, + &fingerprint, + ); #[cfg(feature = "tracing")] drop(_span_guard); @@ -852,6 +865,7 @@ impl<'a> DeferredCommand<'a> { stderr: OutputMode, executed_at: &'a std::panic::Location<'a>, exec_ctx: &ExecutionContext, + fingerprint: &CommandFingerprint, ) -> CommandOutput { use std::fmt::Write; @@ -904,7 +918,7 @@ impl<'a> DeferredCommand<'a> { let command_str = if exec_ctx.is_verbose() { format!("{command:?}") } else { - command.fingerprint().format_short_cmd() + fingerprint.format_short_cmd() }; let action = match fail_reason { FailureReason::FailedAtRuntime(e) => {