diff mbox series

fix typo in attr_fnspec::verify

Message ID or5yxeucoh.fsf@lxoliva.fsfla.org
State New
Headers show
Series fix typo in attr_fnspec::verify | expand

Commit Message

Alexandre Oliva July 13, 2021, 3:15 a.m. UTC
Odd-numbered indices describing argument access sizes in the fnspec
string can only hold 't' or a digit, as tested in the beginning of the
case.  When checking that the size-supplying argument does not have
additional information associated with it, the test that excludes the
't' possibility looks for it at the even position in the fnspec
string.  Oops.

This might yield false positives and negatives if a function has a
fnspec in which an argument uses a 't' access-size, and ('t' - '1')
happens to be the index of an argument described in an fnspec string.
Assuming ASCII encoding, it would take a function with at least 68
arguments described in fnspec.  Still, probably worth fixing.

Regstrapped on x86_64-linux-gnu.  I'm checking this in as obvious unless
there are objections within some 48 hours.


for  gcc/ChangeLog

	* tree-ssa-alias.c (attr_fnspec::verify): Fix index in
	non-'t'-sized arg check.
---
 gcc/tree-ssa-alias.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Richard Biener July 13, 2021, 8:16 a.m. UTC | #1
On Tue, Jul 13, 2021 at 5:15 AM Alexandre Oliva <oliva@adacore.com> wrote:
>
>
> Odd-numbered indices describing argument access sizes in the fnspec
> string can only hold 't' or a digit, as tested in the beginning of the
> case.  When checking that the size-supplying argument does not have
> additional information associated with it, the test that excludes the
> 't' possibility looks for it at the even position in the fnspec
> string.  Oops.
>
> This might yield false positives and negatives if a function has a
> fnspec in which an argument uses a 't' access-size, and ('t' - '1')
> happens to be the index of an argument described in an fnspec string.
> Assuming ASCII encoding, it would take a function with at least 68
> arguments described in fnspec.  Still, probably worth fixing.
>
> Regstrapped on x86_64-linux-gnu.  I'm checking this in as obvious unless
> there are objections within some 48 hours.

oops - also worth backporting to affected branches.

Richard.

>
> for  gcc/ChangeLog
>
>         * tree-ssa-alias.c (attr_fnspec::verify): Fix index in
>         non-'t'-sized arg check.
> ---
>  gcc/tree-ssa-alias.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
> index 0421bfac99869..742a95a549e20 100644
> --- a/gcc/tree-ssa-alias.c
> +++ b/gcc/tree-ssa-alias.c
> @@ -3895,7 +3895,7 @@ attr_fnspec::verify ()
>                     && str[idx] != 'w' && str[idx] != 'W'
>                     && str[idx] != 'o' && str[idx] != 'O')
>                   err = true;
> -               if (str[idx] != 't'
> +               if (str[idx + 1] != 't'
>                     /* Size specified is scalar, so it should be described
>                        by ". " if specified at all.  */
>                     && (arg_specified_p (str[idx + 1] - '1')
>
>
> --
> Alexandre Oliva, happy hacker                https://FSFLA.org/blogs/lxo/
>    Free Software Activist                       GNU Toolchain Engineer
> Disinformation flourishes because many people care deeply about injustice
> but very few check the facts.  Ask me about <https://stallmansupport.org>
Alexandre Oliva July 14, 2021, 1:45 a.m. UTC | #2
On Jul 13, 2021, Richard Biener <richard.guenther@gmail.com> wrote:

> oops - also worth backporting to affected branches.

Thanks, I took that as explicit approval and put it in.

attr fnspec is new in gcc-11, not present in gcc-10, so I'm testing a
trivial backport, just to be sure...  Will install in gcc-11 when done.

>> * tree-ssa-alias.c (attr_fnspec::verify): Fix index in
>> non-'t'-sized arg check.
Richard Biener July 14, 2021, 7:03 a.m. UTC | #3
On Wed, Jul 14, 2021 at 3:45 AM Alexandre Oliva <oliva@adacore.com> wrote:
>
> On Jul 13, 2021, Richard Biener <richard.guenther@gmail.com> wrote:
>
> > oops - also worth backporting to affected branches.
>
> Thanks, I took that as explicit approval and put it in.
>
> attr fnspec is new in gcc-11, not present in gcc-10, so I'm testing a
> trivial backport, just to be sure...  Will install in gcc-11 when done.

Thanks - yes, it was meant as approval ;)

Richard.

> >> * tree-ssa-alias.c (attr_fnspec::verify): Fix index in
> >> non-'t'-sized arg check.
>
> --
> Alexandre Oliva, happy hacker                https://FSFLA.org/blogs/lxo/
>    Free Software Activist                       GNU Toolchain Engineer
> Disinformation flourishes because many people care deeply about injustice
> but very few check the facts.  Ask me about <https://stallmansupport.org>
Alexandre Oliva July 14, 2021, 4:44 p.m. UTC | #4
On Jul 13, 2021, Alexandre Oliva <oliva@adacore.com> wrote:

> On Jul 13, 2021, Richard Biener <richard.guenther@gmail.com> wrote:
>> oops - also worth backporting to affected branches.

> Thanks, I took that as explicit approval and put it in.

> attr fnspec is new in gcc-11, not present in gcc-10, so I'm testing a
> trivial backport, just to be sure...  Will install in gcc-11 when done.

Here's what I've just installed.


fix typo in attr_fnspec::verify

Odd-numbered indices describing argument access sizes in the fnspec
string can only hold 't' or a digit, as tested in the beginning of the
case.  When checking that the size-supplying argument does not have
additional information associated with it, the test that excludes the
't' possibility looks for it at the even position in the fnspec
string.  Oops.

This might yield false positives and negatives if a function has a
fnspec in which an argument uses a 't' access-size, and ('t' - '1')
happens to be the index of an argument described in an fnspec string.
Assuming ASCII encoding, it would take a function with at least 68
arguments described in fnspec.  Still, probably worth fixing.


for  gcc/ChangeLog

	* tree-ssa-alias.c (attr_fnspec::verify): Fix index in
	non-'t'-sized arg check.

(cherry picked from commit a7098d6ef4e4e799dab8ef925c62b199d707694b)
---
 gcc/tree-ssa-alias.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
index ebb3f49c86c66..3e578e5d05f49 100644
--- a/gcc/tree-ssa-alias.c
+++ b/gcc/tree-ssa-alias.c
@@ -3868,7 +3868,7 @@ attr_fnspec::verify ()
 		    && str[idx] != 'w' && str[idx] != 'W'
 		    && str[idx] != 'o' && str[idx] != 'O')
 		  err = true;
-		if (str[idx] != 't'
+		if (str[idx + 1] != 't'
 		    /* Size specified is scalar, so it should be described
 		       by ". " if specified at all.  */
 		    && (arg_specified_p (str[idx + 1] - '1')
diff mbox series

Patch

diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
index 0421bfac99869..742a95a549e20 100644
--- a/gcc/tree-ssa-alias.c
+++ b/gcc/tree-ssa-alias.c
@@ -3895,7 +3895,7 @@  attr_fnspec::verify ()
 		    && str[idx] != 'w' && str[idx] != 'W'
 		    && str[idx] != 'o' && str[idx] != 'O')
 		  err = true;
-		if (str[idx] != 't'
+		if (str[idx + 1] != 't'
 		    /* Size specified is scalar, so it should be described
 		       by ". " if specified at all.  */
 		    && (arg_specified_p (str[idx + 1] - '1')