Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 1697302 - s390x toolchain/gcc9 issue
Summary: s390x toolchain/gcc9 issue
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: gcc
Version: rawhide
Hardware: s390x
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Jakub Jelinek
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: ZedoraTracker
TreeView+ depends on / blocked
 
Reported: 2019-04-08 10:31 UTC by Benjamin Kircher
Modified: 2019-05-30 08:10 UTC (History)
14 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2019-05-30 08:10:59 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)
koji logs (2.75 MB, text/plain)
2019-04-08 10:31 UTC, Benjamin Kircher
no flags Details
preprocessed source file (1.07 MB, text/plain)
2019-04-08 14:19 UTC, Dan Horák
no flags Details


Links
System ID Private Priority Status Summary Last Updated
GNU Compiler Collection 90025 0 None None None 2019-05-30 06:34:51 UTC
IBM Linux Technology Center 176781 0 None None None 2019-05-30 06:34:51 UTC

Description Benjamin Kircher 2019-04-08 10:31:29 UTC
Created attachment 1553532 [details]
koji logs

Description of problem:
Package build of botan2 fails on rawhide s390x because of (suspected) toolchain issue.

Version-Release number of selected component (if applicable): 
The tests fails for botan 2.10, but also for botan 2.9 package (this previously succeeded).

How reproducible:
Build botan2 package on s390x rawhide.


Steps to Reproduce:
1. fedpkg scratch-build --arches s390x

Actual results:
Build fails during testing skein-512. Logs attached.

Expected results:
the toolchain builds the package.

Additional info:
using gcc9 with -O0 makes the problem go away (reported by sharkcz on #fedora-devel)

Also see upstream issue https://github.com/randombit/botan/issues/1875.

thm made an observation that on s390x hashing of an empty file and reading from /dev/null yield different results (both wrong):

<snip>
$ ./botan hash --algo=Skein-512 empty
3C51E78406D8A980EEF05A56F58980126257BB8CFD1469A4D31F6A264D2AF1BC22379EFE87960F916CB17C6146ECDC84C3BBAB22BF7C20C40DD8B8335AEF448D empty
$ ./botan hash --algo=Skein-512 </dev/null
437AAE4BA75F92BA0913DED7E499C9B61644DFEDD1D6FC45AD3DA5788624781B7099685A99975EAA6F421C2E9A41517371769A7E0E061F4D5B53C4409A666AF9 -

(Expected result is BC5B4C50925519C290CC634277AE3D6257212395CBA733BBAD37A4AF0FA06AF41FCA7903D06564FEA7A2D3730DBDB80C1F85562DFCC070334EA4D1D9E72CBA7A, I think)
<snip/>

Comment 1 Dan Horák 2019-04-08 11:19:57 UTC
setting __attribute__((optimize(("O0")))) for Skein_512::initial_block() (https://github.com/randombit/botan/blob/master/src/lib/hash/skein/skein_512.cpp#L70) makes the tests pass again, I'm looking further ...

Comment 2 Dan Horák 2019-04-08 11:43:48 UTC
void Skein_512::initial_block()
   {
   const uint8_t zeros[64] = { 0 };

   m_threefish->set_key(zeros, sizeof(zeros));

   // ASCII("SHA3") followed by version (0x0001) code
   uint8_t config_str[32] = { 0x53, 0x48, 0x41, 0x33, 0x01, 0x00, 0 };

==>       the problem is the array doesn't contain zeros after ^^^ on s390x

   store_le(uint32_t(m_output_bits), config_str + 8);

   reset_tweak(SKEIN_CONFIG, true);
   ubi_512(config_str, sizeof(config_str));


for upstream code see https://github.com/randombit/botan/blob/master/src/lib/hash/skein/skein_512.cpp#L77

from a gdb session on rawhide/s390x
(gdb) where
#0  Botan::Skein_512::initial_block (this=this@entry=0x10b76d0) at src/lib/hash/skein/skein_512.cpp:72
#1  0x000003fffdb697e0 in Botan::Skein_512::Skein_512 (this=0x10b76d0, arg_output_bits=<optimized out>, arg_personalization=...) at src/lib/hash/skein/skein_512.cpp:25
#2  0x000003fffdb5278a in Botan::HashFunction::create (algo_spec="Skein-512", provider="") at /usr/include/c++/9/ext/new_allocator.h:80
#3  0x000000000102d4e4 in Botan_CLI::Hash::go (this=0x10b6b70) at /usr/include/c++/9/bits/basic_string.h:263
#4  0x0000000001022808 in Botan_CLI::Command::run (this=0x10b6b70, params=std::vector of length 1, capacity 1 = {...}) at src/cli/cli.cpp:75
#5  0x00000000010191d0 in main (argc=<optimized out>, argv=0x3fffffff008) at /usr/include/c++/9/bits/unique_ptr.h:357
(gdb) n
73	   const uint8_t zeros[64] = { 0 };
(gdb) p zeros
$1 = "\000\000\000\000\001\vv\320\000\000\003\377\375\372_@\000\000\000\000\001\vv\350\000\000\003\377\375\371a\\\000\000\003\377\377\377\346H\000\000\000\000\000\000\004-\000\000\003\377\377\377\362\332\000\000\000\000\000\000\000"
(gdb) n
357	      get() const noexcept
(gdb) 
78	   uint8_t config_str[32] = { 0x53, 0x48, 0x41, 0x33, 0x01, 0x00, 0 };
(gdb) l
73	   const uint8_t zeros[64] = { 0 };
74	
75	   m_threefish->set_key(zeros, sizeof(zeros));
76	
77	   // ASCII("SHA3") followed by version (0x0001) code
78	   uint8_t config_str[32] = { 0x53, 0x48, 0x41, 0x33, 0x01, 0x00, 0 };
79	   store_le(uint32_t(m_output_bits), config_str + 8);
80	
81	   reset_tweak(SKEIN_CONFIG, true);
82	   ubi_512(config_str, sizeof(config_str));
(gdb) p zeros
$2 = '\000' <repeats 63 times>
(gdb) n
79	   store_le(uint32_t(m_output_bits), config_str + 8);
(gdb) p config_str
$3 = "SHA3\001\000\000\000\000\000\000\000\000\000\002\000\000\000\000\000\001\vv\320\000\000\003\377\375\364", <incomplete sequence \310>

Comment 3 Dan Horák 2019-04-08 14:19:33 UTC
Created attachment 1553647 [details]
preprocessed source file

Comment 4 Jakub Jelinek 2019-04-09 09:32:19 UTC
What g++ options are used to compile it when it is miscompiled?

Comment 5 Dan Horák 2019-04-09 09:41:41 UTC
g++ -fPIC -fvisibility=hidden -fstack-protector -m64 -pthread -std=c++11 -D_REENTRANT -O3 -Wall -Wextra -Wpedantic -Wstrict-aliasing -Wcast-align -Wmissing-declarations -Wpointer-arith -Wcast-qual -Wzero-as-null-pointer-constant -Wnon-virtual-dtor  -Ibuild/include -Ibuild/include/external -c src/lib/hash/skein/skein_512.cpp -o build/obj/lib/hash_skein_512.o

^^^ this is from an upstream build, using Fedora flags gives the same (miscompiled) result

Comment 6 Jakub Jelinek 2019-04-09 09:53:29 UTC
I think I can reproduce it with C -O2 -march=zEC12 -mtune=z13:
__attribute__((noipa)) void
bar (char *p)
{
  int i;
  for (i = 0; i < 6; i++)
    if (p[i] != "foobar"[i])
      __builtin_abort ();
  for (; i < 32; i++)
    if (p[i] != '\0')
      __builtin_abort ();
}

__attribute__((noipa)) void
foo (unsigned int x)
{
  char s[32] = { 'f', 'o', 'o', 'b', 'a', 'r', 0 };
  ((unsigned int *) s)[2] = __builtin_bswap32 (x);
  bar (s);
}

int
main ()
{
  foo (0);
  return 0;
}

(though, haven't tried to actually ssh somewhere and run it, just eyeballed dumps).
Will now bisect and fix.  In any case, looks like the bug is that the MEM used to clear the remainder of the array (last 24 bytes) claims to have size 1 instead of either 24 (exact size) or no size (conservative):
(insn 7 6 0 (parallel [
            (set (mem/c:BLK (plus:DI (reg/f:DI 55 virtual-stack-vars)
                        (const_int -24 [0xffffffffffffffe8])) [0 s+8 S1 A64])
                (const_int 0 [0]))
            (use (const_int 23 [0x17]))
            (use (const:BLK (unspec:BLK [
                            (const_int 0 [0])
                        ] UNSPEC_INSN)))
            (clobber (scratch:DI))
            (clobber (reg:CC 33 %cc))
        ]) "rh1697302.c":16:8 -1
     (nil))
from expansion or
(insn 19 6 15 2 (parallel [
            (set (mem/c:BLK (plus:DI (reg/f:DI 15 %r15)
                        (const_int 168 [0xa8])) [0 s+8 S1 A64])
                (const_int 0 [0]))
            (use (const_int 24 [0x18]))
            (clobber (reg:CC 33 %cc))
        ]) "rh1697302.c":16:8 1752 {*xc_zero}
     (nil))
after splitting.  It should use S24 or no S...

Comment 7 Dan Horák 2019-04-09 10:09:35 UTC
(In reply to Jakub Jelinek from comment #6)
> I think I can reproduce it with C -O2 -march=zEC12 -mtune=z13:
> __attribute__((noipa)) void
> bar (char *p)
> {
>   int i;
>   for (i = 0; i < 6; i++)
>     if (p[i] != "foobar"[i])
>       __builtin_abort ();
>   for (; i < 32; i++)
>     if (p[i] != '\0')
>       __builtin_abort ();
> }
> 
> __attribute__((noipa)) void
> foo (unsigned int x)
> {
>   char s[32] = { 'f', 'o', 'o', 'b', 'a', 'r', 0 };
>   ((unsigned int *) s)[2] = __builtin_bswap32 (x);
>   bar (s);
> }
> 
> int
> main ()
> {
>   foo (0);
>   return 0;
> }
> 
> (though, haven't tried to actually ssh somewhere and run it, just eyeballed
> dumps).

just checked and it really aborts

Comment 8 Jakub Jelinek 2019-04-09 10:14:17 UTC
Started with my r268957 aka PR66152 change, though I bet it is a backend latent bug before that.

Comment 9 Jakub Jelinek 2019-04-09 10:58:40 UTC
Actually no, the bug was in my patch.

Comment 10 Benjamin Kircher 2019-05-30 08:10:59 UTC
Thanks Jakub, builds fine now with gcc 9.1


Note You need to log in before you can comment on or make changes to this bug.