[02/11] x86/ELF: fix .tfloat output

Message ID 3efd5dcf-53d9-8937-5b2d-ad57978d6a3e@suse.com
State New
Headers show
Series
  • gas: adjustments to floating point data directives handling
Related show

Commit Message

H.J. Lu via Binutils July 23, 2021, 6:52 a.m.
The ELF psABI-s are quite clear here: On 32-bit the data type is 12
bytes long (with 2 bytes of trailing padding), while on 64-bit it is 16
bytes long (with 6 bytes of padding). Make ieee_md_atof() capable of
handling such padding, and specify the needed padding for x86 (leaving
non-ELF targets alone for now). Split the existing x86 testcase.
---
No present target includes 't' or 'T' in FLT_CHARS[]. Does this case
really need handling?

The handling of 'p' and 'P' looks bogus: atof_ieee() determines
exponent_bits to be -1 in this case, passing it to atof_ieee_detail(),
which in turn uses it only to pass it to gen_to_words(). The first use
there is

  exponent_4 = exponent_3 + ((1 << (exponent_bits - 1)) - 2);

i.e. an attempt to shift 1 left by -2 bits (undefined behavior). Later
there are also uses of mask[exponent_bits]. Only tc-arm.c and tc-score.c
look to be using 'p' (for .packed directives). Neither target has a
test case for this directive, and trying to use it on Arm gives "Error:
cannot create floating point number" (but I of course may have used it
wrong).

If there wasn't this anomaly, I wouldn't see why separate X_PRECISION
and P_PRECISION are needed in the first place. But if 'p' really is
meant to stand for "packed", then surely ieee_md_atof() shouldn't use
P_PRECISION for 'x' and 'X' (and I wouldn't need to have P_PRECISION_PAD
fall back to X_PRECISION_PAD here).

Comments

H.J. Lu via Binutils July 23, 2021, 1:14 p.m. | #1
On Thu, Jul 22, 2021 at 11:52 PM Jan Beulich <jbeulich@suse.com> wrote:
>

> The ELF psABI-s are quite clear here: On 32-bit the data type is 12

> bytes long (with 2 bytes of trailing padding), while on 64-bit it is 16

> bytes long (with 6 bytes of padding). Make ieee_md_atof() capable of

> handling such padding, and specify the needed padding for x86 (leaving

> non-ELF targets alone for now). Split the existing x86 testcase.

> ---

> No present target includes 't' or 'T' in FLT_CHARS[]. Does this case

> really need handling?

>

> The handling of 'p' and 'P' looks bogus: atof_ieee() determines

> exponent_bits to be -1 in this case, passing it to atof_ieee_detail(),

> which in turn uses it only to pass it to gen_to_words(). The first use

> there is

>

>   exponent_4 = exponent_3 + ((1 << (exponent_bits - 1)) - 2);

>

> i.e. an attempt to shift 1 left by -2 bits (undefined behavior). Later

> there are also uses of mask[exponent_bits]. Only tc-arm.c and tc-score.c

> look to be using 'p' (for .packed directives). Neither target has a

> test case for this directive, and trying to use it on Arm gives "Error:

> cannot create floating point number" (but I of course may have used it

> wrong).

>

> If there wasn't this anomaly, I wouldn't see why separate X_PRECISION

> and P_PRECISION are needed in the first place. But if 'p' really is

> meant to stand for "packed", then surely ieee_md_atof() shouldn't use

> P_PRECISION for 'x' and 'X' (and I wouldn't need to have P_PRECISION_PAD

> fall back to X_PRECISION_PAD here).


The x86 part is OK.

> --- a/gas/config/atof-ieee.c

> +++ b/gas/config/atof-ieee.c

> @@ -30,7 +30,13 @@ extern FLONUM_TYPE generic_floating_poin

>  #define F_PRECISION    2

>  #define D_PRECISION    4

>  #define X_PRECISION    5

> +#ifndef X_PRECISION_PAD

> +#define X_PRECISION_PAD 0

> +#endif

>  #define P_PRECISION    5

> +#ifndef P_PRECISION_PAD

> +#define P_PRECISION_PAD X_PRECISION_PAD

> +#endif

>

>  /* Length in LittleNums of guard bits.  */

>  #define GUARD          2

> @@ -760,7 +766,7 @@ ieee_md_atof (int type,

>    LITTLENUM_TYPE words[MAX_LITTLENUMS];

>    LITTLENUM_TYPE *wordP;

>    char *t;

> -  int prec = 0;

> +  int prec = 0, pad = 0;

>

>    if (strchr (FLT_CHARS, type) != NULL)

>      {

> @@ -788,6 +794,7 @@ ieee_md_atof (int type,

>         case 't':

>         case 'T':

>           prec = X_PRECISION;

> +         pad = X_PRECISION_PAD;

>           type = 'x';           /* This is what atof_ieee() understands.  */

>           break;

>

> @@ -803,6 +810,7 @@ ieee_md_atof (int type,

>  #else

>           prec = P_PRECISION;

>  #endif

> +         pad = P_PRECISION_PAD;

>           break;

>

>         default:

> @@ -835,7 +843,7 @@ ieee_md_atof (int type,

>    if (t)

>      input_line_pointer = t;

>

> -  *sizeP = prec * sizeof (LITTLENUM_TYPE);

> +  *sizeP = (prec + pad) * sizeof (LITTLENUM_TYPE);

>

>    if (big_wordian)

>      {

> @@ -854,5 +862,8 @@ ieee_md_atof (int type,

>         }

>      }

>

> +  memset (litP, 0, pad * sizeof (LITTLENUM_TYPE));

> +  litP += pad * sizeof (LITTLENUM_TYPE);

> +

>    return NULL;

>  }

> --- a/gas/config/tc-i386.c

> +++ b/gas/config/tc-i386.c

> @@ -10201,6 +10201,19 @@ x86_cons_fix_new (fragS *frag, unsigned

>    fix_new_exp (frag, off, len, exp, 0, r);

>  }

>

> +/* Return the number of padding LITTLENUMs following a tbyte floating

> +   point value.  */

> +

> +int

> +x86_tfloat_pad (void)

> +{

> +#if defined (OBJ_ELF) || defined (OBJ_MAYBE_ELF)

> +   if (IS_ELF)

> +     return object_64bit ? 3 : 1;

> +#endif

> +   return 0;

> +}

> +

>  /* Export the ABI address size for use by TC_ADDRESS_BYTES for the

>     purpose of the `.dc.a' internal pseudo-op.  */

>

> --- a/gas/config/tc-i386.h

> +++ b/gas/config/tc-i386.h

> @@ -134,6 +134,9 @@ extern bfd_reloc_code_real_type x86_cons

>  extern void x86_cons_fix_new

>  (fragS *, unsigned int, unsigned int, expressionS *, bfd_reloc_code_real_type);

>

> +#define X_PRECISION_PAD x86_tfloat_pad ()

> +extern int x86_tfloat_pad (void);

> +

>  #define TC_ADDRESS_BYTES x86_address_bytes

>  extern int x86_address_bytes (void);

>

> --- /dev/null

> +++ b/gas/testsuite/gas/i386/fp-elf32.d

> @@ -0,0 +1,12 @@

> +#objdump: -s -j .data

> +#name: i386 fp (ELF)

> +#source: fp.s

> +

> +.*:     file format .*

> +

> +Contents of section .data:

> + 0000 00881bcd 4b789ad4 00400000 71a37909  .*

> + 0010 4f930a40 789a5440 789a5440 00000000  .*

> + 0020 e65e1710 20395e3b e65e1710 20395e3b  .*

> + 0030 00000000 0000a044 01000000 0000a044  .*

> + 0040 00000000 0000f03f .*

> --- /dev/null

> +++ b/gas/testsuite/gas/i386/fp-elf64.d

> @@ -0,0 +1,12 @@

> +#objdump: -s -j .data

> +#name: x86-64 fp (ELF)

> +#source: fp.s

> +

> +.*:     file format .*

> +

> +Contents of section .data:

> + 0000 00881bcd 4b789ad4 00400000 00000000  .*

> + 0010 71a37909 4f930a40 789a5440 789a5440  .*

> + 0020 e65e1710 20395e3b e65e1710 20395e3b  .*

> + 0030 00000000 0000a044 01000000 0000a044  .*

> + 0040 00000000 0000f03f .*

> --- a/gas/testsuite/gas/i386/fp.s

> +++ b/gas/testsuite/gas/i386/fp.s

> @@ -7,10 +7,10 @@

>  #      .byte 0x71, 0xa3, 0x79, 0x09, 0x4f, 0x93, 0x0a, 0x40

>  # The next two are 32-bit floating point format.

>         .float 3.32192809488736218171e0

> -#      .byte 0x78, 0x9a, 0x54, 0x40, 0, 0, 0, 0

> +#      .byte 0x78, 0x9a, 0x54, 0x40

>         .single 3.32192809488736218171e0

> -#      .byte 0x78, 0x9a, 0x54, 0x40, 0, 0, 0, 0

> -       .byte 0, 0, 0, 0, 0, 0

> +#      .byte 0x78, 0x9a, 0x54, 0x40

> +       .p2align 4,0

>

>  # The assembler used to treat the next value as zero instead of 1e-22.

>          .double .0000000000000000000001

> --- a/gas/testsuite/gas/i386/i386.exp

> +++ b/gas/testsuite/gas/i386/i386.exp

> @@ -118,7 +118,6 @@ if [gas_32_check] then {

>      run_list_test "lockbad-1" "-al"

>      run_dump_test "long-1"

>      run_dump_test "long-1-intel"

> -    run_dump_test "fp"

>      run_dump_test "nops"

>      run_dump_test "nops16-1"

>      run_dump_test "nops-1"

> @@ -618,6 +617,7 @@ if [gas_32_check] then {

>         run_dump_test "intel-movs16"

>         run_dump_test "intel-cmps32"

>         run_dump_test "intel-cmps16"

> +       run_dump_test "fp-elf32"

>         run_list_test "inval-equ-1" "-al"

>         run_list_test "inval-equ-2" "-al"

>         run_dump_test "ifunc"

> @@ -691,6 +691,8 @@ if [gas_32_check] then {

>             run_dump_test "iamcu-5"

>             run_list_test "iamcu-inval-1" "-march=iamcu -al"

>         }

> +    } else {

> +       run_dump_test "fp"

>      }

>

>      # This is a PE specific test.

> @@ -1260,6 +1262,7 @@ if [gas_64_check] then {

>         run_list_test "reloc64" "--defsym _bad_=1"

>         run_dump_test "mixed-mode-reloc64"

>         run_dump_test "rela"

> +       run_dump_test "fp-elf64"

>         run_dump_test "x86-64-ifunc"

>         run_dump_test "x86-64-opcode-inval"

>         run_dump_test "x86-64-opcode-inval-intel"

>



-- 
H.J.

Patch

--- a/gas/config/atof-ieee.c
+++ b/gas/config/atof-ieee.c
@@ -30,7 +30,13 @@  extern FLONUM_TYPE generic_floating_poin
 #define F_PRECISION    2
 #define D_PRECISION    4
 #define X_PRECISION    5
+#ifndef X_PRECISION_PAD
+#define X_PRECISION_PAD 0
+#endif
 #define P_PRECISION    5
+#ifndef P_PRECISION_PAD
+#define P_PRECISION_PAD X_PRECISION_PAD
+#endif
 
 /* Length in LittleNums of guard bits.  */
 #define GUARD          2
@@ -760,7 +766,7 @@  ieee_md_atof (int type,
   LITTLENUM_TYPE words[MAX_LITTLENUMS];
   LITTLENUM_TYPE *wordP;
   char *t;
-  int prec = 0;
+  int prec = 0, pad = 0;
 
   if (strchr (FLT_CHARS, type) != NULL)
     {
@@ -788,6 +794,7 @@  ieee_md_atof (int type,
 	case 't':
 	case 'T':
 	  prec = X_PRECISION;
+	  pad = X_PRECISION_PAD;
 	  type = 'x';		/* This is what atof_ieee() understands.  */
 	  break;
 
@@ -803,6 +810,7 @@  ieee_md_atof (int type,
 #else
 	  prec = P_PRECISION;
 #endif
+	  pad = P_PRECISION_PAD;
 	  break;
 
 	default:
@@ -835,7 +843,7 @@  ieee_md_atof (int type,
   if (t)
     input_line_pointer = t;
 
-  *sizeP = prec * sizeof (LITTLENUM_TYPE);
+  *sizeP = (prec + pad) * sizeof (LITTLENUM_TYPE);
 
   if (big_wordian)
     {
@@ -854,5 +862,8 @@  ieee_md_atof (int type,
 	}
     }
 
+  memset (litP, 0, pad * sizeof (LITTLENUM_TYPE));
+  litP += pad * sizeof (LITTLENUM_TYPE);
+
   return NULL;
 }
--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -10201,6 +10201,19 @@  x86_cons_fix_new (fragS *frag, unsigned
   fix_new_exp (frag, off, len, exp, 0, r);
 }
 
+/* Return the number of padding LITTLENUMs following a tbyte floating
+   point value.  */
+
+int
+x86_tfloat_pad (void)
+{
+#if defined (OBJ_ELF) || defined (OBJ_MAYBE_ELF)
+   if (IS_ELF)
+     return object_64bit ? 3 : 1;
+#endif
+   return 0;
+}
+
 /* Export the ABI address size for use by TC_ADDRESS_BYTES for the
    purpose of the `.dc.a' internal pseudo-op.  */
 
--- a/gas/config/tc-i386.h
+++ b/gas/config/tc-i386.h
@@ -134,6 +134,9 @@  extern bfd_reloc_code_real_type x86_cons
 extern void x86_cons_fix_new
 (fragS *, unsigned int, unsigned int, expressionS *, bfd_reloc_code_real_type);
 
+#define X_PRECISION_PAD x86_tfloat_pad ()
+extern int x86_tfloat_pad (void);
+
 #define TC_ADDRESS_BYTES x86_address_bytes
 extern int x86_address_bytes (void);
 
--- /dev/null
+++ b/gas/testsuite/gas/i386/fp-elf32.d
@@ -0,0 +1,12 @@ 
+#objdump: -s -j .data
+#name: i386 fp (ELF)
+#source: fp.s
+
+.*:     file format .*
+
+Contents of section .data:
+ 0000 00881bcd 4b789ad4 00400000 71a37909  .*
+ 0010 4f930a40 789a5440 789a5440 00000000  .*
+ 0020 e65e1710 20395e3b e65e1710 20395e3b  .*
+ 0030 00000000 0000a044 01000000 0000a044  .*
+ 0040 00000000 0000f03f .*
--- /dev/null
+++ b/gas/testsuite/gas/i386/fp-elf64.d
@@ -0,0 +1,12 @@ 
+#objdump: -s -j .data
+#name: x86-64 fp (ELF)
+#source: fp.s
+
+.*:     file format .*
+
+Contents of section .data:
+ 0000 00881bcd 4b789ad4 00400000 00000000  .*
+ 0010 71a37909 4f930a40 789a5440 789a5440  .*
+ 0020 e65e1710 20395e3b e65e1710 20395e3b  .*
+ 0030 00000000 0000a044 01000000 0000a044  .*
+ 0040 00000000 0000f03f .*
--- a/gas/testsuite/gas/i386/fp.s
+++ b/gas/testsuite/gas/i386/fp.s
@@ -7,10 +7,10 @@ 
 #	.byte 0x71, 0xa3, 0x79, 0x09, 0x4f, 0x93, 0x0a, 0x40
 # The next two are 32-bit floating point format.
 	.float 3.32192809488736218171e0
-#	.byte 0x78, 0x9a, 0x54, 0x40, 0, 0, 0, 0
+#	.byte 0x78, 0x9a, 0x54, 0x40
 	.single 3.32192809488736218171e0
-#	.byte 0x78, 0x9a, 0x54, 0x40, 0, 0, 0, 0
-	.byte 0, 0, 0, 0, 0, 0
+#	.byte 0x78, 0x9a, 0x54, 0x40
+	.p2align 4,0
 
 # The assembler used to treat the next value as zero instead of 1e-22.
         .double .0000000000000000000001
--- a/gas/testsuite/gas/i386/i386.exp
+++ b/gas/testsuite/gas/i386/i386.exp
@@ -118,7 +118,6 @@  if [gas_32_check] then {
     run_list_test "lockbad-1" "-al"
     run_dump_test "long-1"
     run_dump_test "long-1-intel"
-    run_dump_test "fp"
     run_dump_test "nops"
     run_dump_test "nops16-1"
     run_dump_test "nops-1"
@@ -618,6 +617,7 @@  if [gas_32_check] then {
 	run_dump_test "intel-movs16"
 	run_dump_test "intel-cmps32"
 	run_dump_test "intel-cmps16"
+	run_dump_test "fp-elf32"
 	run_list_test "inval-equ-1" "-al"
 	run_list_test "inval-equ-2" "-al"
 	run_dump_test "ifunc"
@@ -691,6 +691,8 @@  if [gas_32_check] then {
 	    run_dump_test "iamcu-5"
 	    run_list_test "iamcu-inval-1" "-march=iamcu -al"
 	}
+    } else {
+	run_dump_test "fp"
     }
 
     # This is a PE specific test.
@@ -1260,6 +1262,7 @@  if [gas_64_check] then {
 	run_list_test "reloc64" "--defsym _bad_=1"
 	run_dump_test "mixed-mode-reloc64"
 	run_dump_test "rela"
+	run_dump_test "fp-elf64"
 	run_dump_test "x86-64-ifunc"
 	run_dump_test "x86-64-opcode-inval"
 	run_dump_test "x86-64-opcode-inval-intel"