Re: Bug fix for glibc broke freebsd build in REL_11_STABLE - Mailing list pgsql-hackers

On September 4, 2018 6:16:24 AM PDT, Victor Wagner <vitus@wagner.pp.ru> wrote:
>
>Collegues,
>
>Few days ago commit
>
> 1f349aa7d9a6633e87db071390c73a39ac279ba4
>
>Fix 8a934d67 for libc++ and make more include order resistant.
>
>was introduced into branch REL_11_STABLE.
>It seems that it deals with GLIBC-based systems (i.e Linux and Hurd)
>only, and shouldn't affect systems with non-GNU libc (such as BSD
>family).

What libc is this using?


>It changes code  which has a comment:
>
>/*
> * Glibc doesn't use the builtin for clang due to a *gcc* bug in a
>version
> * newer than the gcc compatibility clang claims to have. This would
>cause a
> * *lot* of superfluous function calls, therefore revert when using
>clang. In
> * C++ there's issues with libc++ (not libstdc++), so disable as well.
> */
>
>
>However, this commit broke float8 test on 32-bit FreeBSD 11 with clang
>3.8.0 compiler. Regressions.diff follows:

Does this happen with a newer clang version too?

>================== pgsql.build/src/test/regress/regression.diffs
>===================
>***
>/usr/home/buildfarm/build-farm/REL_11_STABLE/pgsql.build/src/test/regress/expected/float8.out
>Tue Sep  4 11:57:47 2018
>---
>/usr/home/buildfarm/build-farm/REL_11_STABLE/pgsql.build/src/test/regress/results/float8.out
>Tue Sep  4 12:03:28 2018 *************** *** 418,424 **** SET f1 =
>FLOAT8_TBL.f1 * '-1' WHERE FLOAT8_TBL.f1 > '0.0';
>  SELECT '' AS bad, f.f1 * '1e200' from FLOAT8_TBL f;
>! ERROR:  value out of range: overflow
>  SELECT '' AS bad, f.f1 ^ '1e200' from FLOAT8_TBL f;
>  ERROR:  value out of range: overflow
>  SELECT 0 ^ 0 + 0 ^ 1 + 0 ^ 0.0 + 0 ^ 0.5;
>--- 418,432 ----
>     SET f1 = FLOAT8_TBL.f1 * '-1'
>     WHERE FLOAT8_TBL.f1 > '0.0';
>  SELECT '' AS bad, f.f1 * '1e200' from FLOAT8_TBL f;
>!  bad |     ?column?
>! -----+------------------
>!      |                0
>!      |      -3.484e+201
>!      |     -1.0043e+203
>!      |        -Infinity
>!      | -1.2345678901234
>! (5 rows)
>!
>  SELECT '' AS bad, f.f1 ^ '1e200' from FLOAT8_TBL f;
>  ERROR:  value out of range: overflow
>  SELECT 0 ^ 0 + 0 ^ 1 + 0 ^ 0.0 + 0 ^ 0.5;
>================================================================
>Problem doesn't arise on 64-bit FreeBSD and MacOS. (we have no 32-bit
>MacOs at hand).

This seems like something that needs to be understood.  If a compiler intrinsic doesn't work, it's serious.


>Following patch fixes this problem:
>
>diff --git a/src/include/port.h b/src/include/port.h
>index 0ce72e50e5..7daaebf568 100644
>--- a/src/include/port.h
>+++ b/src/include/port.h
>@@ -350,7 +350,7 @@ extern int  isinf(double x);
>  * *lot* of superfluous function calls, therefore revert when using
>clang. In
>  * C++ there's issues with libc++ (not libstdc++), so disable as well.
>  */
>-#if defined(__clang__) && !defined(__cplusplus)
>+#if defined(__clang__) && !defined(__cplusplus) && defined(__linux__)
> /* needs to be separate to not confuse other compilers */
> #if __has_builtin(__builtin_isinf)
> /* need to include before, to avoid getting overwritten */
>
>Idea of the patch is that because patch is only GLIBC-related, we
>should apply this logic only if we have linux (more precisely linux or
>hurd, but adding hurd would make condition overcomplicated and no one
>seems to use it) system.

This seems too restrictive. There's nothing Linux specific in the issue - causes slowdowns on other platforms too.

Thanks!

Andres
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.