On Fri, 19 Apr 2019 at 11:57, Richard Sandiford
<richard.sandif...@arm.com> wrote:
>
> Christophe Lyon <christophe.l...@linaro.org> writes:
> > On Fri, 19 Apr 2019 at 11:23, Richard Sandiford
> > <richard.sandif...@arm.com> wrote:
> >>
> >> Christophe Lyon <christophe.l...@linaro.org> writes:
> >> > On Thu, 18 Apr 2019 at 18:25, Richard Sandiford
> >> > <richard.sandif...@arm.com> wrote:
> >> >>
> >> >> Christophe Lyon <christophe.l...@linaro.org> writes:
> >> >> > Hi,
> >> >> >
> >> >> > This patch adds the missing space before '%<' in
> >> >> > config/aarch64/aarch64.c and gcc/cp/call.c. It also updates the
> >> >> > check-internal-format-escaping.py checker to warn about such cases.
> >> >> >
> >> >> > OK?
> >> >> >
> >> >> > Christophe
> >> >> >
> >> >> > diff --git a/contrib/check-internal-format-escaping.py
> >> >> > b/contrib/check-internal-format-escaping.py
> >> >> > index aac4f9e..9c62586 100755
> >> >> > --- a/contrib/check-internal-format-escaping.py
> >> >> > +++ b/contrib/check-internal-format-escaping.py
> >> >> > @@ -58,6 +58,10 @@ for i, l in enumerate(lines):
> >> >> > print('%s: %s' % (origin, text))
> >> >> > if re.search("[^%]'", p):
> >> >> > print('%s: %s' % (origin, text))
> >> >> > + # %< should not be preceded by a non-punctuation
> >> >> > + # %character.
> >> >> > + if re.search("[a-zA-Z0-9]%<", p):
> >> >> > + print('%s: %s' % (origin, text))
> >> >> > j += 1
> >> >> >
> >> >> > origin = None
> >> >> > diff --git a/gcc/config/aarch64/aarch64.c
> >> >> > b/gcc/config/aarch64/aarch64.c
> >> >> > index 9be7548..b66071f 100644
> >> >> > --- a/gcc/config/aarch64/aarch64.c
> >> >> > +++ b/gcc/config/aarch64/aarch64.c
> >> >> > @@ -11483,7 +11483,7 @@ aarch64_override_options_internal (struct
> >> >> > gcc_options *opts)
> >> >> > if (aarch64_stack_protector_guard == SSP_GLOBAL
> >> >> > && opts->x_aarch64_stack_protector_guard_offset_str)
> >> >> > {
> >> >> > - error ("incompatible options
> >> >> > %<-mstack-protector-guard=global%> and"
> >> >> > + error ("incompatible options
> >> >> > %<-mstack-protector-guard=global%> and "
> >> >> > "%<-mstack-protector-guard-offset=%s%>",
> >> >> > aarch64_stack_protector_guard_offset_str);
> >> >> > }
> >> >> > diff --git a/gcc/cp/call.c b/gcc/cp/call.c
> >> >> > index 9582345..8f3d019 100644
> >> >> > --- a/gcc/cp/call.c
> >> >> > +++ b/gcc/cp/call.c
> >> >> > @@ -3614,16 +3614,16 @@ print_z_candidate (location_t loc, const char
> >> >> > *msgstr,
> >> >> > {
> >> >> > cloc = loc;
> >> >> > if (candidate->num_convs == 3)
> >> >> > - inform (cloc, "%s%<%D(%T, %T, %T)%> <built-in>", msg, fn,
> >> >> > + inform (cloc, "%s %<%D(%T, %T, %T)%> <built-in>", msg, fn,
> >> >> > candidate->convs[0]->type,
> >> >> > candidate->convs[1]->type,
> >> >> > candidate->convs[2]->type);
> >> >> > else if (candidate->num_convs == 2)
> >> >> > - inform (cloc, "%s%<%D(%T, %T)%> <built-in>", msg, fn,
> >> >> > + inform (cloc, "%s %<%D(%T, %T)%> <built-in>", msg, fn,
> >> >> > candidate->convs[0]->type,
> >> >> > candidate->convs[1]->type);
> >> >> > else
> >> >> > - inform (cloc, "%s%<%D(%T)%> <built-in>", msg, fn,
> >> >> > + inform (cloc, "%s %<%D(%T)%> <built-in>", msg, fn,
> >> >> > candidate->convs[0]->type);
> >> >> > }
> >> >> > else if (TYPE_P (fn))
> >> >>
> >> >> I don't think this is right, since msg already has a space where
> >> >> necessary:
> >> >>
> >> >> const char *msg = (msgstr == NULL
> >> >> ? ""
> >> >> : ACONCAT ((msgstr, " ", NULL)));
> >> >>
> >> >> Adding something like "(^| )[^% ]*" to the start of the regexp might
> >> >> avoid that, at the risk of letting through real problems.
> >> >>
> >> >
> >> > Yes, that would miss the problem in aarch64.c.
> >>
> >> Are you sure? It works for me.
> >>
> > It didn't work for me, with that change the line didn't report any error.
>
> Hmm, OK. With:
>
> if re.search("(^| )[^% ]*[a-zA-Z0-9]%<", p):
> print('%s: %s' % (origin, text))
>
> and a tree without your aarch64.c fix, I get:
>
> #: config/aarch64/aarch64.c:11486: incompatible options
> %<-mstack-protector-guard=global%> and%<-mstack-"
>
> but no warning about the cp/call.c stuff.
>
This works for me too. I don't know what I got wrong in my previous check.
So... what should I do? Apply you patch, or revert mine according to
Jakub's comments?
Improving the checker now would help fixing these annoying things
without waiting for gcc-10
Christophe
> Thanks,
> Richard
