Re: [PATCH] Fix PR22371, enable type-checking for all stages

["Richard Guenther" <richard dot guenther at gmail dot com>]

On Wed, Mar 26, 2008 at 7:47 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
> > This patch also enables type-checking for all stages and not only
>  > stage1, so all of the runtimes and testsuites get coverage.
>
>  This introduced the following failure for Ada:
>
>  FAIL: gnat.dg/packed_subtype.adb (test for excess errors)
>  WARNING: gnat.dg/packed_subtype.adb compilation failed to produce executable
>
>  packed_subtype.adb: In function 'Packed_Subtype':
>  packed_subtype.adb:3: error: type mismatch in component reference
>  UNSIGNED_8
>
>  packed_subtype__ubyte___XDLU_0__255
>
>  D.180 = mp.a
>
>
>    subtype Ubyte is Integer range 0 .. 255;
>    type Packet (Id : Ubyte) is record
>       A, B : Ubyte;
>    end record;
>    pragma Pack (Packet);
>
>    subtype My_Packet is Packet (Id => 1);
>
>    MP : My_Packet;
>  begin
>    MP.A := 1;
>  [...]
>
>
>  Ubyte has got precision 32 because it's a subtype of Integer:
>
>   <integer_type 0x55701c98 packed_subtype__ubyte___XDLU_0__255
>     type <integer_type 0x556ea2d8 integer sizes-gimplified public visited SI
>         size <integer_cst 0x556dd620 constant invariant visited 32>
>         unit size <integer_cst 0x556dd40c constant invariant visited 4>
>         align 32 symtab 0 alias set -1 canonical type 0x556ea2d8 precision 32
>  min <integer_cst 0x556dd5cc -2147483648> max <integer_cst 0x556dd5e8
>  2147483647>
>         pointer_to_this <pointer_type 0x556eabc8>>
>     public unsigned SI size <integer_cst 0x556dd620 32> unit size <integer_cst
>  0x556dd40c 4>
>     align 32 symtab 0 alias set 0 canonical type 0x55701c98 precision 32 min
>  <integer_cst 0x556dd8f8 0> max <integer_cst 0x556ff2d8 255>
>     RM size <integer_cst 0x556dd498 type <integer_type 0x556ea068
>  bit_size_type> constant invariant visited 8>>
>
>
>  When passed MP.A, get_unwidened rewrites the COMPONENT_REF with the UNSIGNED_8
>  type because, since Packet is packed, the size of the A field is 8:
>
>   if (TREE_CODE (op) == COMPONENT_REF
>       /* Since type_for_size always gives an integer type.  */
>       && TREE_CODE (type) != REAL_TYPE
>       && TREE_CODE (type) != FIXED_POINT_TYPE
>       /* Don't crash if field not laid out yet.  */
>       && DECL_SIZE (TREE_OPERAND (op, 1)) != 0
>       && host_integerp (DECL_SIZE (TREE_OPERAND (op, 1)), 1))
>     {
>       unsigned int innerprec
>         = tree_low_cst (DECL_SIZE (TREE_OPERAND (op, 1)), 1);
>       int unsignedp = (DECL_UNSIGNED (TREE_OPERAND (op, 1))
>                        || TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (op, 1))));
>       type = lang_hooks.types.type_for_size (innerprec, unsignedp);
>
>       /* We can get this structure field in the narrowest type it fits in.
>          If FOR_TYPE is 0, do this only for a field that matches the
>          narrower type exactly and is aligned for it
>          The resulting extension to its nominal type (a fullword type)
>          must fit the same conditions as for other extensions.  */
>
>       if (type != 0
>           && INT_CST_LT_UNSIGNED (TYPE_SIZE (type), TYPE_SIZE (TREE_TYPE (op)))
>           && (for_type || ! DECL_BIT_FIELD (TREE_OPERAND (op, 1)))
>           && (! uns || final_prec <= innerprec || unsignedp))
>         {
>           win = build3 (COMPONENT_REF, type, TREE_OPERAND (op, 0),
>                         TREE_OPERAND (op, 1), NULL_TREE);
>           TREE_SIDE_EFFECTS (win) = TREE_SIDE_EFFECTS (op);
>           TREE_THIS_VOLATILE (win) = TREE_THIS_VOLATILE (op);
>         }
>     }
>
>  and this creates the type mismatch.  So I think that the verification code
>  should let it go through.
>
>  Lightly tested on i586-suse-linux, OK for mainline?

I think get_unwidened should not do this as I do not see any reason why it
should.  So no, please do not weaken the verifier this way.  Where is
get_unwidened called from?

Thanks,
Richard.

>  2008-03-26  Eric Botcazou  <ebotcazou@adacore.com>
>
>         * tree-cfg.c (verify_gimple_reference): Cope with get_unwidened and
>         its rewriting of COMPONENT_REFs.
>
>
>  --
>  Eric Botcazou
>