[Bug c/112615] New: gcc incorrectly assumes char *x[2]={"str1", "str2"} has 16-byte minimum alignment and generates SSE instructions (e.g. movaps) when accessing this data

[Bug c/112615] New: gcc incorrectly assumes char *x[2]={"str1", "str2"} has 16-byte minimum alignment and generates SSE instructions (e.g. movaps) when accessing this data

[gandalf at winds dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112615

            Bug ID: 112615
           Summary: gcc incorrectly assumes char *x[2]={"str1", "str2"}
                    has 16-byte minimum alignment and generates SSE
                    instructions (e.g. movaps) when accessing this data
           Product: gcc
           Version: 13.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: gandalf at winds dot org
  Target Milestone: ---

I ran into the following problem while trying to get Oracle 21c to run on
Gentoo OS under glibc-2.38 and GCC 13.2 on x86-64 with SSE instructions
enabled.

glibc-2.38's time/tzset.c file (see
https://github.com/bminor/glibc/blob/master/time/tzset.c) has the following
non-static declaration on line 31:

char *__tzname[2] = { (char *) "GMT", (char *) "GMT" };

GCC wrongly assumes that this variable __tzname has a minimum alignment of 16
bytes instead of 8. GCC thus generates the following assembly instructions for
this portion of __tzset_parse_tz() when compiling with -march=x86-64:

327       /* Get the standard time zone abbreviations.  */
328       if (parse_tzname (&tz, 0) && parse_offset (&tz, 0))
   0x0000000000000a6f <+63>:    movups %xmm0,0x0(%rip)        # 0xa76
<__tzset_parse_tz+70>
   0x0000000000000a76 <+70>:    movups %xmm0,0x0(%rip)        # 0xa7d
<__tzset_parse_tz+77>
   0x0000000000000a7d <+77>:    movups %xmm0,0x0(%rip)        # 0xa84
<__tzset_parse_tz+84>
   0x0000000000000a84 <+84>:    movups %xmm0,0x0(%rip)        # 0xa8b
<__tzset_parse_tz+91>
   0x0000000000000a8b <+91>:    call   0x440 <parse_tzname>
   0x0000000000000a90 <+96>:    test   %al,%al
   0x0000000000000a92 <+98>:    jne    0xac8 <__tzset_parse_tz+152>
   0x0000000000000a94 <+100>:   movq   0x0(%rip),%xmm0        # 0xa9c
<__tzset_parse_tz+108>

132       __tzname[1] = (char *) tz_rules[1].name;
   0x0000000000000a9c <+108>:   xor    %eax,%eax
   0x0000000000000a9e <+110>:   xor    %edx,%edx
   0x0000000000000aa0 <+112>:   pinsrq $0x1,0x0(%rip),%xmm0        # 0xaab
<__tzset_parse_tz+123>

129       __daylight = tz_rules[0].offset != tz_rules[1].offset;
   0x0000000000000aab <+123>:   mov    %edx,0x0(%rip)        # 0xab1
<__tzset_parse_tz+129>

130       __timezone = -tz_rules[0].offset;
   0x0000000000000ab1 <+129>:   mov    %rax,0x0(%rip)        # 0xab8
<__tzset_parse_tz+136>

131       __tzname[0] = (char *) tz_rules[0].name;
132       __tzname[1] = (char *) tz_rules[1].name;
   0x0000000000000ab8 <+136>:   movaps %xmm0,0x0(%rip)        # 0xabf
<__tzset_parse_tz+143>

In the above, line 131 and 132 are combined into a "movaps" instruction that
requires 16-byte alignment to work properly. However, if a C program is
compiled with a variable called __tzname that is not 16-byte aligned (due to
the fact that char* only requires 8-byte alignment), and this is then linked to
glibc (causing the locally defined __tzname to override the one declared in
glibc), and the if(parse_tzname()) check on line 328 fails due to an invalid TZ
environment variable setting (such as is the case when using Oracle 21c on
Gentoo), the movaps instruction above causes a segmentation fault. Here is an
example test.c C program:

#include <stdio.h>
#include <time.h>

/* Specifically align __tzname to a non-16-byte boundary */
__attribute__((aligned(8))) char *__tzname[2]={"GMT", "GMT"};

char *x="xx";  // This is here to take up the first 8 bytes in .data

int main()
{
  struct tm tm={};
  printf("%ld\n", mktime(&tm));
  return 0;
}

$ gcc -O3 -march=x86-64 test.c -o test -Wall -ggdb3
$ nm test | grep __tzname
0000000000404028 D __tzname
$ ./test
-2209057200
$ TZ=xx ./test
Segmentation fault (core dumped)

Removing the __attribute__((aligned(8))) from the test.c program, as follows,
causes the following change:

#include <stdio.h>
#include <time.h>

/* GCC now aligns __tzname to 16 bytes */
char *__tzname[2]={"GMT", "GMT"};
char *x="xx";

int main()
{
  struct tm tm={};
  printf("%ld\n", mktime(&tm));
  return 0;
}

$ gcc -O3 -march=x86-64 test.c -o test -Wall -ggdb3
$ nm test | grep __tzname
0000000000404030 D __tzname
$ ./test
-2209057200
$ TZ=xx ./test
-2209057200

In the examples above, the "x" variable is used to consume 8 bytes in .data, so
that the next available address for "__tzname" is 0x404028.

Assuming a minimum alignment of 16 for __tzname only makes sense when you're
either compiling the whole program or when __tzname is static, but GCC should
not do this when the variable is non-static (as is the case when tzset.o is
compiled inside the glibc source package).

I should clarify that optimizing the variable's address to use 16-byte
alignment can be ideal for data storage vs cache-line boundaries and so this
optimization should likely remain. But the instructions acting on this data
area must assume an 8-byte minimum alignment here, not 16.

[Bug target/112615] gcc incorrectly assumes char *x[2]={"str1", "str2"} has 16-byte minimum alignment and generates SSE instructions (e.g. movaps) when accessing this data

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112615

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |INVALID

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Well the x86_64 SYSV ABI says:
    An array uses the same alignment as its elements, except that a local or 
    global array variable that requires at least 16 bytes, or a C99 local or 
    global variable-length array variable, always has alignment of at least 16 
    bytes.[4]


So I think this is not a bug in GCC or glibc but rather `Oracle 21c`.
Notice how it says global array variable.

[Bug target/112615] gcc incorrectly assumes char *x[2]={"str1", "str2"} has 16-byte minimum alignment and generates SSE instructions (e.g. movaps) when accessing this data

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112615

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
>Assuming a minimum alignment of 16 for __tzname only makes sense when you're either compiling the whole program or when __tzname is static


Not if you follow the ABI which has had this in the ABI for years now (more
than 20).

You can workaround the issue inside glibc by adding the attribute but GCC is
producing correct code according to the ABI.

[Bug target/112615] gcc incorrectly assumes char *x[2]={"str1", "str2"} has 16-byte minimum alignment and generates SSE instructions (e.g. movaps) when accessing this data

[gandalf at winds dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112615

gandalf at winds dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|INVALID                     |FIXED

--- Comment #3 from gandalf at winds dot org ---
Thank you for the insight. Sounds like a bug in Oracle's compiler then.

I have just finished adding the aligned(8) attribute to glibc's __tzname on my
system and recompiled glibc. I verified this fixed the problem (no segfault).

Thanks much.

[Bug target/112615] gcc incorrectly assumes char *x[2]={"str1", "str2"} has 16-byte minimum alignment and generates SSE instructions (e.g. movaps) when accessing this data

[gandalf at winds dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112615

gandalf at winds dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|FIXED                       |INVALID

[Bug target/112615] gcc incorrectly assumes char *x[2]={"str1", "str2"} has 16-byte minimum alignment and generates SSE instructions (e.g. movaps) when accessing this data

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112615

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
.

Or their sources for Oracle DB ... 
I suspect they have an assembly file that contains that variable and didn't
realize the alignment rules.

Note GCC 4.1.2 even sets the alignment to 16 for a simple:
char *x[2]={"str1", "str2"};