[PATCH] Fix handling of non-integral bit-fields in native_encode_initializer

[PATCH] Fix handling of non-integral bit-fields in native_encode_initializer

[Eric Botcazou]

[-- Attachment #1: Type: text/plain, Size: 862 bytes --]

Hi,

the encoder for CONSTRUCTORs assumes that all bit-fields (DECL_BIT_FIELD) have
integral types, but that's not the case in Ada where they may have pretty much
any type, resulting in a wrong encoding for them.

The attached fix filters out non-integral bit-fields, except if they start and
end on a byte boundary because they are correctly handled in this case.

Bootstrapped/regtested on x86-64/Linux, OK for mainline and 13 branch?


2023-05-22  Eric Botcazou  <ebotcazou@adacore.com>

	* fold-const.cc (native_encode_initializer) <CONSTRUCTOR>: Apply the
	specific treatment for bit-fields only if they have an integral type
	and filter out non-integral bit-fields that do not start and end on
	a byte boundary.


2023-05-22  Eric Botcazou  <ebotcazou@adacore.com>

	* gnat.dg/opt101.adb: New test.
	* gnat.dg/opt101_pkg.ads: New helper.

-- 
Eric Botcazou

[-- Attachment #2: p.diff --]
[-- Type: text/x-patch, Size: 1737 bytes --]

diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc
index 25466e97220..57521501fff 100644
--- a/gcc/fold-const.cc
+++ b/gcc/fold-const.cc
@@ -8360,20 +8360,26 @@ native_encode_initializer (tree init, unsigned char *ptr, int len,
 	      if (fieldsize == 0)
 		continue;
 
+	      /* Prepare to deal with integral bit-fields and filter out other
+		 bit-fields that do not start and end on a byte boundary.  */
 	      if (DECL_BIT_FIELD (field))
 		{
 		  if (!tree_fits_uhwi_p (DECL_FIELD_BIT_OFFSET (field)))
 		    return 0;
-		  fieldsize = TYPE_PRECISION (TREE_TYPE (field));
 		  bpos = tree_to_uhwi (DECL_FIELD_BIT_OFFSET (field));
-		  if (bpos % BITS_PER_UNIT)
-		    bpos %= BITS_PER_UNIT;
-		  else
-		    bpos = 0;
-		  fieldsize += bpos;
-		  epos = fieldsize % BITS_PER_UNIT;
-		  fieldsize += BITS_PER_UNIT - 1;
-		  fieldsize /= BITS_PER_UNIT;
+		  if (INTEGRAL_TYPE_P (TREE_TYPE (field)))
+		    {
+		      bpos %= BITS_PER_UNIT;
+		      fieldsize = TYPE_PRECISION (TREE_TYPE (field)) + bpos;
+		      epos = fieldsize % BITS_PER_UNIT;
+		      fieldsize += BITS_PER_UNIT - 1;
+		      fieldsize /= BITS_PER_UNIT;
+		    }
+		  else if (bpos % BITS_PER_UNIT
+			   || DECL_SIZE (field) == NULL_TREE
+			   || !tree_fits_shwi_p (DECL_SIZE (field))
+			   || tree_to_shwi (DECL_SIZE (field)) % BITS_PER_UNIT)
+		    return 0;
 		}
 
 	      if (off != -1 && pos + fieldsize <= off)
@@ -8382,7 +8388,8 @@ native_encode_initializer (tree init, unsigned char *ptr, int len,
 	      if (val == NULL_TREE)
 		continue;
 
-	      if (DECL_BIT_FIELD (field))
+	      if (DECL_BIT_FIELD (field)
+		  && INTEGRAL_TYPE_P (TREE_TYPE (field)))
 		{
 		  /* FIXME: Handle PDP endian.  */
 		  if (BYTES_BIG_ENDIAN != WORDS_BIG_ENDIAN)

[-- Attachment #3: opt101.adb --]
[-- Type: text/x-adasrc, Size: 377 bytes --]

-- { dg-do run }
-- { dg-options "-O" }

pragma Optimize_Alignment (Space);

with Opt101_Pkg; use Opt101_Pkg;

procedure Opt101 is

  C1 : Cont1;
  C2 : Cont2;

begin
  C1 := ((1234, 1, 2), 1, 2);
  if C1.R.I1 /= 1 or C1.I2 /= 2 then
    raise Program_Error;
  end if;

  C2 := (1, (1234, 1, 2), 2);
  if C2.R.I1 /= 1 or C2.I2 /= 2 then
    raise Program_Error;
  end if;
end;

[-- Attachment #4: opt101_pkg.ads --]
[-- Type: text/x-adasrc, Size: 424 bytes --]

package Opt101_Pkg is

  type Int is mod 16;

  type Rec is record
    S : Short_Integer;
    I1, I2 : Int;
  end record;
  pragma Pack (Rec);
  for Rec'Alignment use 4;

  type Cont1 is record
    R : Rec;
    I1, I2 : Int;
  end record;
  pragma Pack (Cont1);

  type Cont2 is record
    I1 : Int;
    R  : Rec;
    I2 : Int;
  end record;
  pragma Pack (Cont2);
  pragma No_Component_Reordering (Cont2);

end Opt101_Pkg;

Re: [PATCH] Fix handling of non-integral bit-fields in native_encode_initializer

[Richard Biener]

On Mon, May 22, 2023 at 10:10 AM Eric Botcazou via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Hi,
>
> the encoder for CONSTRUCTORs assumes that all bit-fields (DECL_BIT_FIELD) have
> integral types, but that's not the case in Ada where they may have pretty much
> any type, resulting in a wrong encoding for them.
>
> The attached fix filters out non-integral bit-fields, except if they start and
> end on a byte boundary because they are correctly handled in this case.
>
> Bootstrapped/regtested on x86-64/Linux, OK for mainline and 13 branch?

OK.

Can we handle non-integer bitfields by recursing with a temporary buffer to
encode it byte-aligned and then apply shifting and masking to get it in place?
Or is that not worth it?

Thanks,
Richard.

>
>
> 2023-05-22  Eric Botcazou  <ebotcazou@adacore.com>
>
>         * fold-const.cc (native_encode_initializer) <CONSTRUCTOR>: Apply the
>         specific treatment for bit-fields only if they have an integral type
>         and filter out non-integral bit-fields that do not start and end on
>         a byte boundary.
>
>
> 2023-05-22  Eric Botcazou  <ebotcazou@adacore.com>
>
>         * gnat.dg/opt101.adb: New test.
>         * gnat.dg/opt101_pkg.ads: New helper.
>
> --
> Eric Botcazou

Re: [PATCH] Fix handling of non-integral bit-fields in native_encode_initializer

[Eric Botcazou]

> OK.

Thanks!

> Can we handle non-integer bitfields by recursing with a temporary buffer to
> encode it byte-aligned and then apply shifting and masking to get it in
> place? Or is that not worth it?

Certainly doable, something along these lines is implemented in varasm.c to 
output these non-integral bit-fields (output_constructor et al) so we could 
even try to share some code.  However, in practice, these cases turn out to be 
rare because the tree_output_constant_def path in gimplify_init_constructor is 
well guarded.

-- 
Eric Botcazou