test-ibm128-llround fails on ppc64el when built with gcc-12 and -O2 or higher

[Frank Heimes]

root@kinetic:~/build# ./elf/ld.so --library-path .:math ./math/test-ibm128-llround
testing long double (without inline functions)
Failure: llround (0x7fffffffffffffff.8p0): Exception "Invalid operation" not set
Failure: llround_downward (0x7fffffffffffffff.8p0): Exception "Invalid operation" not set
Failure: llround_towardzero (0x7fffffffffffffff.8p0): Exception "Invalid operation" not set
Failure: llround_upward (0x7fffffffffffffff.8p0): Exception "Invalid operation" not set

Test suite completed:
  1156 test cases plus 656 tests for exception flags and
    656 tests for errno executed.
  4 errors occurred.

Recompiling math/s_llroundl.os with gcc-11 or with gcc-12 -O1 is enough to fix this. Looking at the disassembly though I'm completely confused, feraiseexcept (FE_INVALID); is still getting called in the failing case and the code that runs after that looks the same (I've spent far too long trying to debug this). I've uploaded the .o files to https://people.canonical.com/~mwh/good.o and https://people.canonical.com/~mwh/bad.o -- good luck to the next person!

A potential fix is here:
https://sourceware.org/pipermail/libc-alpha/2022-August/141547.html

For more details and discussions have a look at the attached upstream bug.

[Michael Hudson-Doyle (mwhudson)]

root@kinetic:~/build# ./elf/ld.so --library-path .:math ./math/test-ibm128-llround
testing long double (without inline functions)
Failure: llround (0x7fffffffffffffff.8p0): Exception "Invalid operation" not set
Failure: llround_downward (0x7fffffffffffffff.8p0): Exception "Invalid operation" not set
Failure: llround_towardzero (0x7fffffffffffffff.8p0): Exception "Invalid operation" not set
Failure: llround_upward (0x7fffffffffffffff.8p0): Exception "Invalid operation" not set

Test suite completed:
  1156 test cases plus 656 tests for exception flags and
    656 tests for errno executed.
  4 errors occurred.

Recompiling math/s_llroundl.os with gcc-11 or with gcc-12 -O1 is enough to fix this. Looking at the disassembly though I'm completely confused, feraiseexcept (FE_INVALID); is still getting called in the failing case and the code that runs after that looks the same (I've spent far too long trying to debug this). I've uploaded the .o files to https://people.canonical.com/~mwh/good.o and https://people.canonical.com/~mwh/bad.o -- good luck to the next person!

[Michael Hudson-Doyle (mwhudson)]

Ah my previous debugging was confused, feraiseexcept is not being reached and the issue is that the algorithm depends on signed overflow at https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/ieee754/ldbl-128ibm/s_llroundl.c;h=d85154e73ac062e6c9ed403fe3cfc3889726ff72;hb=2b274fd8c9c776cf70fcdb8356e678ada522a7b0#l86 which is of course undefined behaviour. I don't completely understand the algorithm so don't really know how to fix this (or, honestly, how much I care about edge cases in ldbl-128ibm arithmetic...)

[Joseph-codesourcery (joseph-codesourcery)]

Making res unsigned, and casting hi and lo to unsigned long long in the
"res = hi + lo;" statement, and casting res back to long long in the two
places it gets compared against 0, would be the obvious approach to avoid
signed overflow.

[Michael Hudson-Doyle (mwhudson)]

I thought casting out-of-range unsigned to signed was undefined as well?

[Andreas Schwab (schwab-linux-m68k)]

It is implementation defined.

[Michael Hudson-Doyle (mwhudson)]

(In reply to Andreas Schwab from comment #4)
> It is implementation defined.

Oh right. Anyway the suggestion helps so I'll mail a patch.

[Dimitri John Ledkov (xnox)]

I wonder if this is fixed in:

glibc (2.36-0ubuntu2) kinetic; urgency=medium

  * Add patches to fix build with GCC 12:
    - d/patches/0001-Avoid-undefined-behaviour-in-ibm128-implementation-o.patch
    - d/patches/0001-Ensure-calculations-happen-with-desired-rounding-mod.patch
    - d/patches/0001-Fix-BZ-29463-in-the-ibm128-implementation-of-y1l-too.patch
  * Switch back to building with the default GCC (i.e. 12)
  * Add patches to fix incompatibility between kernel and glibc mount.h
    headers (LP: #1985956):
    - d/patches/0001-glibcextract.py-Add-compile_c_snippet.patch
    - d/patches/0003-linux-Mimic-kernel-defition-for-BLOCK_SIZE.patch
    - d/patches/0004-linux-Use-compile_c_snippet-to-check-linux-mount.h-a.patch
    - d/patches/0005-linux-Fix-sys-mount.h-usage-with-kernel-headers.patch
    - d/patches/0006-Linux-Fix-enum-fsconfig_command-detection-in-sys-mou.patch
  * Add patch to restore DT_HASH tag/SHT_HASH section (see
    https://sourceware.org/bugzilla/show_bug.cgi?id=29456):
    - d/patches/restore-libc-DT_HASH.patch
  * Add nss/tst-reload2 to xfails as it fails in autopkgtests in check_prof
    run.

[Aurelien Jarno (aurelien-aurel32)]

Fixed in 2.37 by the following commit:

commit 2b5478569e72ee4820a6e163d306690c9c0eaf5e
Author: Aurelien Jarno <email address hidden>
Date: Mon Oct 10 00:39:33 2022 +0200

    Avoid undefined behaviour in ibm128 implementation of llroundl (BZ #29488)

    Detecting an overflow edge case depended on signed overflow of a long
    long. Replace the additions and the overflow checks by
    __builtin_add_overflow().

    Reviewed-by: Tulio Magno Quites Machado Filho <email address hidden>

[Simon Chopin (schopin)]

AFAIK this is fixed in Ubuntu Kinetic as well with a similar patch from mwhudson, thus marking as such.

[bugproxy (bugproxy)]

------- Comment From <email address hidden> 2023-10-12 09:46 EDT-------
Machine : ltcden12-lp6
GCC : 11.4
I didn't see the issue with above system.
root@ltcden12-lp6:~/glibc_check/Build# ./elf/ld.so --library-path .:math ./math/test-ibm128-llround
testing long double (without inline functions)

Test suite completed:
1156 test cases plus 656 tests for exception flags and
656 tests for errno executed.
All tests passed successfully.

------- Comment From <email address hidden> 2023-10-13 04:14 EDT-------
This is not reproducible with glibc master.
Machine : ltcd97-lp3
ppc64le and gcc 12.2
[maheshbodapati@ltcd97-lp3 Build]$ ./elf/ld.so --library-path .:math ./math/test-ibm128-llround
testing long double (without inline functions)

Test suite completed:
1156 test cases plus 656 tests for exception flags and
656 tests for errno executed.
All tests passed successfully.
[maheshbodapati@ltcd97-lp3 Build]$ gcc --version
gcc (GCC) 12.2.1 20221121 (Red Hat 12.2.1-4)