diff mbox series

Fix handling of non-integral bit-fields in native_encode_initializer

Message ID 13270277.uLZWGnKmhe@fomalhaut
State Accepted
Headers
Series Fix handling of non-integral bit-fields in native_encode_initializer |

Checks

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

Commit Message

Eric Botcazou May 22, 2023, 8:08 a.m. UTC 
  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.

Comments

Richard Biener May 22, 2023, 12:15 p.m. UTC | #1 
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
Eric Botcazou May 23, 2023, 8:09 a.m. UTC | #2 
> 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.
diff mbox series

Patch

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)