diff mbox series

Fix a few incorrect accesses.

Message ID cbc8f41c-fe12-b7af-c906-e19f1ce1224e@redhat.com
State Accepted
Headers
Series Fix a few incorrect accesses. |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Andrew MacLeod Dec. 2, 2022, 2:12 p.m. UTC 
  This consists of 3 changes which stronger type checking has indicated 
are non-compliant with the type field.

I doubt they are super important because there has not been a trap 
triggered by them, and they have been in the source base since sometime 
before 2017.  However, we should probably fix them.

I also notice that those are all uses of VOID_TYPE_P, which 
coincidentally does not check if its a type node being checked:

    /* Nonzero if this type is the (possibly qualified) void type.  */
    #define VOID_TYPE_P(NODE) (TREE_CODE (NODE) == VOID_TYPE)

So I guess it wouldn't trap anyway, just silently never trigger.

Bootstraps on x86_64-pc-linux-gnu with no regressions.  OK for trunk?

Andrew

Comments

Richard Biener Dec. 2, 2022, 2:52 p.m. UTC | #1 
On Fri, Dec 2, 2022 at 3:13 PM Andrew MacLeod via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> This consists of 3 changes which stronger type checking has indicated
> are non-compliant with the type field.
>
> I doubt they are super important because there has not been a trap
> triggered by them, and they have been in the source base since sometime
> before 2017.  However, we should probably fix them.
>
> I also notice that those are all uses of VOID_TYPE_P, which
> coincidentally does not check if its a type node being checked:
>
>     /* Nonzero if this type is the (possibly qualified) void type.  */
>     #define VOID_TYPE_P(NODE) (TREE_CODE (NODE) == VOID_TYPE)
>
> So I guess it wouldn't trap anyway, just silently never trigger.
>
> Bootstraps on x86_64-pc-linux-gnu with no regressions.  OK for trunk?

LGTM.

> Andrew
Andrew MacLeod Dec. 2, 2022, 5:02 p.m. UTC | #2 
On 12/2/22 09:52, Richard Biener wrote:
> On Fri, Dec 2, 2022 at 3:13 PM Andrew MacLeod via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>> This consists of 3 changes which stronger type checking has indicated
>> are non-compliant with the type field.
>>
>> I doubt they are super important because there has not been a trap
>> triggered by them, and they have been in the source base since sometime
>> before 2017.  However, we should probably fix them.
>>
>> I also notice that those are all uses of VOID_TYPE_P, which
>> coincidentally does not check if its a type node being checked:
>>
>>      /* Nonzero if this type is the (possibly qualified) void type.  */
>>      #define VOID_TYPE_P(NODE) (TREE_CODE (NODE) == VOID_TYPE)
>>
>> So I guess it wouldn't trap anyway, just silently never trigger.
>>
>> Bootstraps on x86_64-pc-linux-gnu with no regressions.  OK for trunk?
> LGTM.

Pushed as commit 76dd48f8956b5e17adf0ae1cd1ed3d804a005470

Andrew
Thomas Schwinge Dec. 7, 2022, 10:08 a.m. UTC | #3 
Hi Andrew!

On 2022-12-02T09:12:23-0500, Andrew MacLeod via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> This consists of 3 changes which stronger type checking has indicated
> are non-compliant with the type field.

I'm curious what that "stronger type checking" is?


Grüße
 Thomas


> I doubt they are super important because there has not been a trap
> triggered by them, and they have been in the source base since sometime
> before 2017.  However, we should probably fix them.
>
> I also notice that those are all uses of VOID_TYPE_P, which
> coincidentally does not check if its a type node being checked:
>
>     /* Nonzero if this type is the (possibly qualified) void type.  */
>     #define VOID_TYPE_P(NODE) (TREE_CODE (NODE) == VOID_TYPE)
>
> So I guess it wouldn't trap anyway, just silently never trigger.
>
> Bootstraps on x86_64-pc-linux-gnu with no regressions.  OK for trunk?
>
> Andrew


> From d1003e853d1813105eef6e441578e5bea9de8d03 Mon Sep 17 00:00:00 2001
> From: Andrew MacLeod <amacleod@redhat.com>
> Date: Tue, 29 Nov 2022 13:07:28 -0500
> Subject: [PATCH] Fix a few incorrect accesses.
>
> This consists of 3 changes which stronger type checking has indicated
> are incorrect.
>
>       gcc/
>       * fold-const.cc (fold_unary_loc): Check TREE_TYPE of node.
>       (tree_invalid_nonnegative_warnv_p): Likewise.
>
>       gcc/c-family/
>       * c-attribs.cc (handle_deprecated_attribute): Use type when
>       using TYPE_NAME.
> ---
>  gcc/c-family/c-attribs.cc | 2 +-
>  gcc/fold-const.cc         | 6 +++---
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc
> index 07bca68e9b9..b36dd97802b 100644
> --- a/gcc/c-family/c-attribs.cc
> +++ b/gcc/c-family/c-attribs.cc
> @@ -4240,7 +4240,7 @@ handle_deprecated_attribute (tree *node, tree name,
>        if (type && TYPE_NAME (type))
>       {
>         if (TREE_CODE (TYPE_NAME (type)) == IDENTIFIER_NODE)
> -         what = TYPE_NAME (*node);
> +         what = TYPE_NAME (type);
>         else if (TREE_CODE (TYPE_NAME (type)) == TYPE_DECL
>                  && DECL_NAME (TYPE_NAME (type)))
>           what = DECL_NAME (TYPE_NAME (type));
> diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc
> index 114258fa182..e80be8049e1 100644
> --- a/gcc/fold-const.cc
> +++ b/gcc/fold-const.cc
> @@ -9369,8 +9369,8 @@ fold_unary_loc (location_t loc, enum tree_code code, tree type, tree op0)
>             && TREE_CODE (tem) == COND_EXPR
>             && TREE_CODE (TREE_OPERAND (tem, 1)) == code
>             && TREE_CODE (TREE_OPERAND (tem, 2)) == code
> -           && ! VOID_TYPE_P (TREE_OPERAND (tem, 1))
> -           && ! VOID_TYPE_P (TREE_OPERAND (tem, 2))
> +           && ! VOID_TYPE_P (TREE_TYPE (TREE_OPERAND (tem, 1)))
> +           && ! VOID_TYPE_P (TREE_TYPE (TREE_OPERAND (tem, 2)))
>             && (TREE_TYPE (TREE_OPERAND (TREE_OPERAND (tem, 1), 0))
>                 == TREE_TYPE (TREE_OPERAND (TREE_OPERAND (tem, 2), 0)))
>             && (! (INTEGRAL_TYPE_P (TREE_TYPE (tem))
> @@ -15002,7 +15002,7 @@ tree_invalid_nonnegative_warnv_p (tree t, bool *strict_overflow_p, int depth)
>
>       /* If the initializer is non-void, then it's a normal expression
>          that will be assigned to the slot.  */
> -     if (!VOID_TYPE_P (t))
> +     if (!VOID_TYPE_P (TREE_TYPE (t)))
>         return RECURSE (t);
>
>       /* Otherwise, the initializer sets the slot in some way.  One common
> --
> 2.38.1
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
Andrew MacLeod Dec. 7, 2022, 2:58 p.m. UTC | #4 
On 12/7/22 05:08, Thomas Schwinge wrote:
> Hi Andrew!
>
> On 2022-12-02T09:12:23-0500, Andrew MacLeod via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>> This consists of 3 changes which stronger type checking has indicated
>> are non-compliant with the type field.
> I'm curious what that "stronger type checking" is?
>

Remnants of an old project which replaces the uses of trees to represent 
types everywhere in GCC with a new type pointer. This provided much 
stronger compile time type checking.   These few places in the patch 
caused compile time errors because the accesses were not type nodes.

They were hanging around in an old branch from 2017 I was looking at, so 
I figured it was time to get them into trunk :-)


Anddrew
diff mbox series

Patch

From d1003e853d1813105eef6e441578e5bea9de8d03 Mon Sep 17 00:00:00 2001
From: Andrew MacLeod <amacleod@redhat.com>
Date: Tue, 29 Nov 2022 13:07:28 -0500
Subject: [PATCH] Fix a few incorrect accesses.

This consists of 3 changes which stronger type checking has indicated
are incorrect.

	gcc/
	* fold-const.cc (fold_unary_loc): Check TREE_TYPE of node.
	(tree_invalid_nonnegative_warnv_p): Likewise.

	gcc/c-family/
	* c-attribs.cc (handle_deprecated_attribute): Use type when
	using TYPE_NAME.
---
 gcc/c-family/c-attribs.cc | 2 +-
 gcc/fold-const.cc         | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc
index 07bca68e9b9..b36dd97802b 100644
--- a/gcc/c-family/c-attribs.cc
+++ b/gcc/c-family/c-attribs.cc
@@ -4240,7 +4240,7 @@  handle_deprecated_attribute (tree *node, tree name,
       if (type && TYPE_NAME (type))
 	{
 	  if (TREE_CODE (TYPE_NAME (type)) == IDENTIFIER_NODE)
-	    what = TYPE_NAME (*node);
+	    what = TYPE_NAME (type);
 	  else if (TREE_CODE (TYPE_NAME (type)) == TYPE_DECL
 		   && DECL_NAME (TYPE_NAME (type)))
 	    what = DECL_NAME (TYPE_NAME (type));
diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc
index 114258fa182..e80be8049e1 100644
--- a/gcc/fold-const.cc
+++ b/gcc/fold-const.cc
@@ -9369,8 +9369,8 @@  fold_unary_loc (location_t loc, enum tree_code code, tree type, tree op0)
 	      && TREE_CODE (tem) == COND_EXPR
 	      && TREE_CODE (TREE_OPERAND (tem, 1)) == code
 	      && TREE_CODE (TREE_OPERAND (tem, 2)) == code
-	      && ! VOID_TYPE_P (TREE_OPERAND (tem, 1))
-	      && ! VOID_TYPE_P (TREE_OPERAND (tem, 2))
+	      && ! VOID_TYPE_P (TREE_TYPE (TREE_OPERAND (tem, 1)))
+	      && ! VOID_TYPE_P (TREE_TYPE (TREE_OPERAND (tem, 2)))
 	      && (TREE_TYPE (TREE_OPERAND (TREE_OPERAND (tem, 1), 0))
 		  == TREE_TYPE (TREE_OPERAND (TREE_OPERAND (tem, 2), 0)))
 	      && (! (INTEGRAL_TYPE_P (TREE_TYPE (tem))
@@ -15002,7 +15002,7 @@  tree_invalid_nonnegative_warnv_p (tree t, bool *strict_overflow_p, int depth)
 
 	/* If the initializer is non-void, then it's a normal expression
 	   that will be assigned to the slot.  */
-	if (!VOID_TYPE_P (t))
+	if (!VOID_TYPE_P (TREE_TYPE (t)))
 	  return RECURSE (t);
 
 	/* Otherwise, the initializer sets the slot in some way.  One common
-- 
2.38.1