Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions libwild/src/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,8 +175,8 @@ pub(crate) enum FileWriteMode {

/// The existing output file, if any, will be edited in-place. Any hard links to the file will
/// update accordingly. If the file is locked due to currently being executed, then our write
/// will fail.
UpdateInPlace,
/// will fail. Specified indicates whether the mode was explicitly specified by the user or not.
UpdateInPlace { specified: bool },
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the code, I wonder if adding a third variant Auto might look better. But entirely up to you. I'm happy with either.

}

#[derive(Debug, Eq, PartialEq, Clone, Copy)]
Expand Down Expand Up @@ -2191,7 +2191,7 @@ fn setup_argument_parser() -> ArgumentParser {
.long("update-in-place")
.help("Update file in place")
.execute(|args, _modifier_stack| {
args.file_write_mode = Some(FileWriteMode::UpdateInPlace);
args.file_write_mode = Some(FileWriteMode::UpdateInPlace { specified: true });
Ok(())
});

Expand Down
54 changes: 29 additions & 25 deletions libwild/src/file_writer.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::args::Args;
use crate::args::FileWriteMode;
use crate::args::OutputKind;
use crate::args::WRITE_VERIFY_ALLOCATIONS_ENV;
use crate::error;
use crate::error::Context as _;
Expand All @@ -11,6 +12,7 @@ use crate::output_section_map::OutputSectionMap;
use crate::output_section_part_map::OutputSectionPartMap;
use crate::output_trace::TraceOutput;
use memmap2::MmapOptions;
use std::io::ErrorKind;
use std::io::Write;
use std::ops::Deref;
use std::ops::DerefMut;
Expand Down Expand Up @@ -108,7 +110,7 @@ impl Output {
pub(crate) fn new(args: &Args) -> Output {
let file_write_mode = args
.file_write_mode
.unwrap_or_else(|| default_file_write_mode(&args.output));
.unwrap_or_else(|| default_file_write_mode(args));

let creator = if args.available_threads.get() > 1 {
let (sized_output_sender, sized_output_recv) = std::sync::mpsc::channel();
Expand Down Expand Up @@ -222,27 +224,16 @@ impl Output {
}

/// Returns the file write mode that we should use to write to the specified path.
fn default_file_write_mode(path: &Path) -> FileWriteMode {
use std::os::unix::fs::FileTypeExt as _;
fn default_file_write_mode(args: &Args) -> FileWriteMode {
if matches!(args.output_kind(), OutputKind::SharedObject) {
return FileWriteMode::UnlinkAndReplace;
}

let Ok(metadata) = std::fs::metadata(path) else {
if std::fs::metadata(&args.output).is_err() {
return FileWriteMode::UnlinkAndReplace;
};

let file_type = metadata.file_type();

// If we've been asked to write to a path that currently holds some exotic kind of file, then we
// don't want to delete it, even if we have permission to. For example, we don't want to delete
// `/dev/null` if we're running in a container as root.
if file_type.is_char_device()
|| file_type.is_block_device()
|| file_type.is_socket()
|| file_type.is_fifo()
{
return FileWriteMode::UpdateInPlace;
}

FileWriteMode::UnlinkAndReplace
FileWriteMode::UpdateInPlace { specified: false }
}

/// Delete the old output file. Note, this is only used when running from a single thread.
Expand Down Expand Up @@ -271,17 +262,30 @@ impl SizedOutput {
FileWriteMode::UnlinkAndReplace => {
open_options.truncate(true);
}
FileWriteMode::UpdateInPlace => {
FileWriteMode::UpdateInPlace { .. } => {
open_options.truncate(false);
}
}

let file = open_options
.read(true)
.write(true)
.create(true)
.open(&path)
.with_context(|| format!("Failed to open `{}`", path.display()))?;
let file = match open_options.read(true).write(true).create(true).open(&path) {
Ok(file) => file,
Err(error) => {
// Retry open operation with UnlinkAndReplace if it's an
// ETXTBSY error and UpdateInPlace was not explicitly specified
// by the user
if error.kind() == ErrorKind::ExecutableFileBusy
&& matches!(
output_config.file_write_mode,
FileWriteMode::UpdateInPlace { specified: false }
)
{
open_options.truncate(true).open(&path)?
} else {
return Err(error)
.with_context(|| format!("Failed to open `{}`", path.display()));
}
}
};

let out = OutputBuffer::new(&file, file_size, output_config);

Expand Down