diff mbox series

Microblaze: Type confusion in fprintf

Message ID 002b01d5696e$b9c40cd0$2d4c2670$@uni-ulm.de
State New
Headers show
Series Microblaze: Type confusion in fprintf | expand

Commit Message

Jonas Pfeil Sept. 12, 2019, 1:33 p.m. UTC 
A type confusion leads to illegal (and nonsensical) assembly on certain host
architectures (e.g. ARM), where HOST_WIDE_INT (64 bit) and unsigned long (32
bit) have different alignments. So this has printed an uninitialized
register instead of the intended value. This fixes float constant
initialization on an ARM->Microblaze cross compiler.

Comments

Jeff Law Sept. 20, 2019, 10:50 p.m. UTC | #1 
On 9/12/19 7:33 AM, Jonas Pfeil wrote:
> A type confusion leads to illegal (and nonsensical) assembly on certain host
> architectures (e.g. ARM), where HOST_WIDE_INT (64 bit) and unsigned long (32
> bit) have different alignments. So this has printed an uninitialized
> register instead of the intended value. This fixes float constant
> initialization on an ARM->Microblaze cross compiler.
> 
> --- a/gcc/config/microblaze/microblaze.c
> +++ b/gcc/config/microblaze/microblaze.c
> @@ -2468,7 +2468,7 @@ print_operand (FILE * file, rtx op, int letter)
>           unsigned long value_long;
>           REAL_VALUE_TO_TARGET_SINGLE (*CONST_DOUBLE_REAL_VALUE (op),
>                                        value_long);
> -         fprintf (file, HOST_WIDE_INT_PRINT_HEX, value_long);
> +         fprintf (file, "%#lx", value_long);
>         }
>        else
>         {
> 
But shouldn't HOST_WIDE_INT_PRINT_HEX here result in the same thing?

From hwint.h:

#ifndef HAVE_INTTYPES_H
#if INT64_T_IS_LONG
# define GCC_PRI64 HOST_LONG_FORMAT
#else
# define GCC_PRI64 HOST_LONG_LONG_FORMAT
#endif

[ ... ]

#undef PRIx64
#define PRIx64 GCC_PRI64 "x"
[ ... ]

#define HOST_WIDE_INT_PRINT_HEX "%#" PRIx64


If that's not giving us back %#lx, then it seems to me like something is
wrong elsewhere.

jeff
Segher Boessenkool Sept. 22, 2019, 12:52 a.m. UTC | #2 
On Fri, Sep 20, 2019 at 04:50:12PM -0600, Jeff Law wrote:
> On 9/12/19 7:33 AM, Jonas Pfeil wrote:
> > A type confusion leads to illegal (and nonsensical) assembly on certain host
> > architectures (e.g. ARM), where HOST_WIDE_INT (64 bit) and unsigned long (32
> > bit) have different alignments. So this has printed an uninitialized
> > register instead of the intended value. This fixes float constant
> > initialization on an ARM->Microblaze cross compiler.
> > 
> > --- a/gcc/config/microblaze/microblaze.c
> > +++ b/gcc/config/microblaze/microblaze.c
> > @@ -2468,7 +2468,7 @@ print_operand (FILE * file, rtx op, int letter)
> >           unsigned long value_long;
> >           REAL_VALUE_TO_TARGET_SINGLE (*CONST_DOUBLE_REAL_VALUE (op),
> >                                        value_long);
> > -         fprintf (file, HOST_WIDE_INT_PRINT_HEX, value_long);
> > +         fprintf (file, "%#lx", value_long);
> >         }
> >        else
> >         {
> > 
> But shouldn't HOST_WIDE_INT_PRINT_HEX here result in the same thing?
> 
> >From hwint.h:
> 
> #ifndef HAVE_INTTYPES_H
> #if INT64_T_IS_LONG
> # define GCC_PRI64 HOST_LONG_FORMAT
> #else
> # define GCC_PRI64 HOST_LONG_LONG_FORMAT
> #endif
> 
> [ ... ]
> 
> #undef PRIx64
> #define PRIx64 GCC_PRI64 "x"
> [ ... ]
> 
> #define HOST_WIDE_INT_PRINT_HEX "%#" PRIx64
> 
> 
> If that's not giving us back %#lx, then it seems to me like something is
> wrong elsewhere.

This is a 32-bit target though, so INT64_T_IS_LONG is false.


Segher
diff mbox series

Patch

--- a/gcc/config/microblaze/microblaze.c
+++ b/gcc/config/microblaze/microblaze.c
@@ -2468,7 +2468,7 @@  print_operand (FILE * file, rtx op, int letter)
          unsigned long value_long;
          REAL_VALUE_TO_TARGET_SINGLE (*CONST_DOUBLE_REAL_VALUE (op),
                                       value_long);
-         fprintf (file, HOST_WIDE_INT_PRINT_HEX, value_long);
+         fprintf (file, "%#lx", value_long);
        }
       else
        {