On Sun, May 12, 2024 at 3:40 PM Peter Damianov <peter0...@disroot.org> wrote:
>
> Currently, commands like:
> gcc -o file.c -lm
> will delete the user's code.
>
> This patch makes the linker write executables to a temp file, and then renames
> the temp file if successful. This fixes the case above, but has limitations.
> The source file will still get overwritten if the link "succeeds", such as the
> case of: gcc -o file.c -lm -r
>
> It's not perfect, but it should hopefully stop some people from ruining their
> day.
Hmm. When suggesting this I was originally hoping for this to be implemented
in the linker so that it delays opening (and truncating) of the output
file as much as possible.
If we want to do something in the compiler driver then I like the filename based
heuristics more. v3 seems to only address the case of -o specifying the linker
output file but of course
gcc -c t.c -o t2.c
or
gcc -S t.c -o t2.c
happily overwrite a source file as well. For these cases
heuristically rejecting
source file patterns would be better. As we've shown the rename trick when
the link was successful doesn't fully solve the issue. And I bet some people
will claim it isn't an issue at all ...
That is, I do think the linker itself, as a quality of implementation issue,
should avoid truncating the output early. In fact the BFD linker seems to
unlink the output very early:
24937 stat("t.c", {st_mode=S_IFREG|0644, st_size=13, ...}) = 0
24937 lstat("t.c", {st_mode=S_IFREG|0644, st_size=13, ...}) = 0
24937 unlink("t.c") = 0
24937 openat(AT_FDCWD, "t.c", O_RDWR|O_CREAT|O_TRUNC, 0666) = 3
before even opening other inputs or the default linker script.
Richard.
> gcc/ChangeLog:
> PR driver/80182
> * gcc.cc (output_file_temp): New global variable
> (driver_handle_option): Create temp file for executable output
> (driver::maybe_run_linker): Rename output_file_temp to output_file if
> the linker ran successfully
>
> Signed-off-by: Peter Damianov <peter0...@disroot.org>
> ---
>
> v3: don't attempt to create temp files -> rename for -o /dev/null
>
> gcc/gcc.cc | 53 +++++++++++++++++++++++++++++++++++++----------------
> 1 file changed, 37 insertions(+), 16 deletions(-)
>
> diff --git a/gcc/gcc.cc b/gcc/gcc.cc
> index 830a4700a87..5e38c6e578a 100644
> --- a/gcc/gcc.cc
> +++ b/gcc/gcc.cc
> @@ -2138,6 +2138,11 @@ static int have_E = 0;
> /* Pointer to output file name passed in with -o. */
> static const char *output_file = 0;
>
> +/* We write the output file to a temp file, and rename it if linking
> + is successful. This is to prevent mistakes like: gcc -o file.c -lm from
> + deleting the user's code. */
> +static const char *output_file_temp = 0;
> +
> /* Pointer to input file name passed in with -truncate.
> This file should be truncated after linking. */
> static const char *totruncate_file = 0;
> @@ -4610,10 +4615,18 @@ driver_handle_option (struct gcc_options *opts,
> #if defined(HAVE_TARGET_EXECUTABLE_SUFFIX) ||
> defined(HAVE_TARGET_OBJECT_SUFFIX)
> arg = convert_filename (arg, ! have_c, 0);
> #endif
> - output_file = arg;
> + output_file_temp = output_file = arg;
> + /* If creating an executable, create a temp file for the output, unless
> + -o /dev/null was requested. This will later get renamed, if the
> linker
> + succeeds. */
> + if (!have_c && strcmp (output_file, HOST_BIT_BUCKET) != 0)
> + {
> + output_file_temp = make_temp_file ("");
> + record_temp_file (output_file_temp, false, true);
> + }
> /* On some systems, ld cannot handle "-o" without a space. So
> split the option from its argument. */
> - save_switch ("-o", 1, &arg, validated, true);
> + save_switch ("-o", 1, &output_file_temp, validated, true);
> return true;
>
> case OPT_pie:
> @@ -9266,22 +9279,30 @@ driver::maybe_run_linker (const char *argv0) const
> linker_was_run = (tmp != execution_count);
> }
>
> - /* If options said don't run linker,
> - complain about input files to be given to the linker. */
> -
> - if (! linker_was_run && !seen_error ())
> - for (i = 0; (int) i < n_infiles; i++)
> - if (explicit_link_files[i]
> - && !(infiles[i].language && infiles[i].language[0] == '*'))
> + if (!seen_error ())
> + {
> + if (linker_was_run)
> + /* If the linker finished without errors, rename the output from the
> + temporary file to the real output name. */
> + rename (output_file_temp, output_file);
> + else
> {
> - warning (0, "%s: linker input file unused because linking not done",
> - outfiles[i]);
> - if (access (outfiles[i], F_OK) < 0)
> - /* This is can be an indication the user specifed an errorneous
> - separated option value, (or used the wrong prefix for an
> - option). */
> - error ("%s: linker input file not found: %m", outfiles[i]);
> + /* If options said don't run linker,
> + complain about input files to be given to the linker. */
> + for (i = 0; (int) i < n_infiles; i++)
> + if (explicit_link_files[i]
> + && !(infiles[i].language && infiles[i].language[0] == '*'))
> + {
> + warning (0, "%s: linker input file unused because linking not
> done",
> + outfiles[i]);
> + if (access (outfiles[i], F_OK) < 0)
> + /* This is can be an indication the user specifed an
> errorneous
> + separated option value, (or used the wrong prefix for an
> + option). */
> + error ("%s: linker input file not found: %m", outfiles[i]);
> + }
> }
> + }
> }
>
> /* The end of "main". */
> --
> 2.39.2
>
