On Mon, May 6, 2024 at 10:29 AM Peter0x44 <peter0...@disroot.org> wrote:
>
> On Mon May 6, 2024 at 8:14 AM BST, Richard Biener wrote:
> > On Sat, May 4, 2024 at 9:36 PM Peter Damianov <peter0...@disroot.org> wrote:
> > >
> > > Currently, commands like:
> > > gcc -o file.c -lm
> > > will delete the user's code.
> >
> > Since there's an error from the linker in the end (missing 'main'), I 
> > wonder if
> > the linker can avoid truncating/opening the output file instead?  A trivial
> > solution might be to open a temporary file first and only atomically replace
> > the output file with the temporary file when there were no errors?
> I think this is a great idea! The only concern I have is that I think
> for mingw targets it would be necessary to be careful to append .exe if
> the file has no suffix when moving the temporary file to the output
> file. Maybe some other targets have similar concerns.
> >
> > > This patch checks the suffix of the output, and errors if the output ends 
> > > in
> > > any of the suffixes listed in default_compilers.
> > >
> > > Unfortunately, I couldn't come up with a better heuristic to diagnose 
> > > this case
> > > more specifically, so it is now not possible to directly make executables 
> > > with
> > > said suffixes. I am unsure if any users are depending on this.
> >
> > A way to provide a workaround would be to require the file not existing.  So
> > change the heuristic to only trigger if the output file exists (and is
> > non-empty?).
> I guess this could work, and has a lower chance of breaking anyone
> depending on this behavior, but I think it would still be confusing to
> anyone who did rely on this behavior, since then it wouldn't be allowed
> to overwrite an executable with the ".c" name. If anyone did rely on
> this behavior, their build would succeed once, and then error for every
> subsequent invokation, which would be confusing. It seems to me it is
> not a meaningful improvement.

That's true and the behavior would be confusing.

> With your previous suggestion, this whole heuristic becomes unnecessary
> anyway, so I think I will just forego it.

It of course wouldn't handle the case if there isn't a link error like

gcc -o file -lm -r

but it should still be an improvement.  And yes, I typoed a wrong -o myself
a few times ...

Richard.

> >
> > Richard.
> >
> > >         PR driver/80182
> > >         * gcc.cc (process_command): fatal_error if the output has the 
> > > suffix of
> > >           a source file.
> > >         (have_c): Change type to bool.
> > >         (have_O): Change type to bool.
> > >         (have_E): Change type to bool.
> > >         (have_S): New global variable.
> > >         (driver_handle_option): Assign have_S
> > >
> > > Signed-off-by: Peter Damianov <peter0...@disroot.org>
> > > ---
> > >  gcc/gcc.cc | 29 ++++++++++++++++++++++++++---
> > >  1 file changed, 26 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/gcc/gcc.cc b/gcc/gcc.cc
> > > index 830a4700a87..53169c16460 100644
> > > --- a/gcc/gcc.cc
> > > +++ b/gcc/gcc.cc
> > > @@ -2127,13 +2127,16 @@ static vec<const_char_p> at_file_argbuf;
> > >  static bool in_at_file = false;
> > >
> > >  /* Were the options -c, -S or -E passed.  */
> > > -static int have_c = 0;
> > > +static bool have_c = false;
> > >
> > >  /* Was the option -o passed.  */
> > > -static int have_o = 0;
> > > +static bool have_o = false;
> > >
> > >  /* Was the option -E passed.  */
> > > -static int have_E = 0;
> > > +static bool have_E = false;
> > > +
> > > +/* Was the option -S passed.  */
> > > +static bool have_S = false;
> > >
> > >  /* Pointer to output file name passed in with -o. */
> > >  static const char *output_file = 0;
> > > @@ -4593,6 +4596,10 @@ driver_handle_option (struct gcc_options *opts,
> > >        have_E = true;
> > >        break;
> > >
> > > +    case OPT_S:
> > > +      have_S = true;
> > > +      break;
> > > +
> > >      case OPT_x:
> > >        spec_lang = arg;
> > >        if (!strcmp (spec_lang, "none"))
> > > @@ -5058,6 +5065,22 @@ process_command (unsigned int 
> > > decoded_options_count,
> > >                        output_file);
> > >      }
> > >
> > > +  /* Reject output file names that have the same suffix as a source
> > > +     file. This is to catch mistakes like: gcc -o file.c -lm
> > > +     that could delete the user's code. */
> > > +  if (have_o && output_file != NULL && !have_E && !have_S)
> > > +    {
> > > +      const char* filename = lbasename(output_file);
> > > +      const char* suffix = strchr(filename, '.');
> > > +      if (suffix != NULL)
> > > +       for (int i = 0; i < n_default_compilers; ++i)
> > > +         if (!strcmp(suffix, default_compilers[i].suffix))
> > > +           fatal_error (input_location,
> > > +                        "output file suffix %qs could be a source file",
> > > +                        suffix);
> > > +    }
> > > +
> > > +
> > >    if (output_file != NULL && output_file[0] == '\0')
> > >      fatal_error (input_location, "output filename may not be empty");
> > >
> > > --
> > > 2.39.2
> > >
>
> Thanks for the feedback,
> Peter D.

Reply via email to