On Wed, Feb 15, 2017 at 11:01:16AM +0000, Richard Earnshaw (lists) wrote:
> On 13/01/17 12:19, Torsten Duwe wrote:
>
> > +++ b/gcc/doc/invoke.texi
> > @@ -11341,6 +11341,27 @@ of the function name, it is considered to be a
> > match. For C99 and C++
> > extended identifiers, the function name must be given in UTF-8, not
> > using universal character names.
> >
> > +@item -fprolog-pad=@var{N},@var{M}
> This needs to make it clear that M is optional. Then below state that
> if omitted, M defaults to zero.
It was mentioned, further down in the paragraph. I moved it up.
> > --- a/gcc/opts.c
> > +++ b/gcc/opts.c
> > @@ -2157,6 +2157,26 @@ common_handle_option (struct gcc_options *opts,
> > opts->x_flag_ipa_reference = false;
> > break;
> >
> > + case OPT_fprolog_pad_:
> > + {
> > + const char *comma = strchr (arg, ',');
> > + if (comma)
> > + {
> > + prolog_nop_pad_size = atoi (arg);
> > + prolog_nop_pad_entry = atoi (comma + 1);
> > + }
> > + else
> > + {
> > + prolog_nop_pad_size = atoi (arg);
> > + prolog_nop_pad_entry = 0;
> > + }
>
> Where's the error checking? If I write gibberish after the option name
> then atoi will silently fail and return zero. I'm not overly familiar
> with the option handling code, but I'm sure we have routines to do the
> heavy lifting here.
Yes, I had already found integral_argument, but that's unsuitable for a
comma separated list, and arg is const so I could' punch a \0 there.
Using atoi was just lazy, admittedly.
> > +default_print_prolog_pad (FILE *file, unsigned HOST_WIDE_INT pad_size,
> > + bool record_p)
> > +{
> > + if (record_p)
> > + fprintf (file, "1:");
> > +
> > + unsigned i;
> > + for (i = 0; i < pad_size; ++i)
> > + fprintf (file, "\tnop\n");
> > +
> > + if (record_p)
> > + {
> > + fprintf (file, "\t.section __prolog_pads_loc, \"a\",@progbits\n");
> > + fprintf (file, "\t.quad 1b\n");
> > + fprintf (file, "\t.previous\n");
> > + }
> > +}
>
> NO! Almost everything in this function is wrong, it needs to be done
> through suitable hooks that call into the machine back-ends that
> understand assembly flavours supported.
That was already mentioned in a previous version. That code assumes GAS+ELF.
It was the quick and dirty solution to get a working prototype.
Torsten
