search

conda-forge / ctng-compiler-activation-feedstock

A conda-smithy repository for ctng-compiler-activation.
BSD 3-Clause "New" or "Revised" License
13 stars 22 forks source link

Wrong return type for std::isinf/isnan(double) #108

Open ZzEeKkAa opened 3 months ago

ZzEeKkAa commented 3 months ago

Solution to issue cannot be found in the documentation.

Issue

Return type of std::isinf(double); must be bool, not int. Similar issue happens with pkgs/main. Was able to reproduce both with clang++ and g++.

#include <cmath>

#include <type_traits>

// this one works as expected
static_assert(
  std::is_same_v < decltype(std::isinf(std::declval < float > ())), bool > );

// this works, but should not
static_assert(
  std::is_same_v < decltype(std::isinf(std::declval < double > ())), int > );

// this one does not work, but should
static_assert(
  std::is_same_v < decltype(std::isinf(std::declval < double > ())), bool > );

cpp reference (https://en.cppreference.com/w/cpp/numeric/math/isinf):

bool isinf( float num );
bool isinf( double num );
bool isinf( long double num );

To create environment:

conda create -n compiler -c conda-forge --override-channels gxx_linux-64 gcc_linux-64
conda activate compiler
x86_64-conda-linux-gnu-g++ --sysroot=${CONDA_BUILD_SYSROOT} -c test.hpp

Exact error:

test.hpp:15:8: error: static assertion failed
   15 |   std::is_same_v < decltype(std::isinf(std::declval < double > ())), bool > );
      |   ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Installed packages

# packages in environment at /home/yevhenii/.miniforge3/envs/compiler:
#
# Name                    Version                   Build  Channel
_libgcc_mutex             0.1                 conda_forge    conda-forge
_openmp_mutex             4.5                       2_gnu    conda-forge
binutils_impl_linux-64    2.40                 hf600244_0    conda-forge
binutils_linux-64         2.40                 hdade7a5_3    conda-forge
gcc_impl_linux-64         13.2.0               h338b0a0_5    conda-forge
gcc_linux-64              13.2.0               h1ed452b_3    conda-forge
gxx_impl_linux-64         13.2.0               h338b0a0_5    conda-forge
gxx_linux-64              13.2.0               he8deefe_3    conda-forge
kernel-headers_linux-64   2.6.32              he073ed8_17    conda-forge
ld_impl_linux-64          2.40                 h41732ed_0    conda-forge
libgcc-devel_linux-64     13.2.0             ha9c7c90_105    conda-forge
libgcc-ng                 13.2.0               h807b86a_5    conda-forge
libgomp                   13.2.0               h807b86a_5    conda-forge
libsanitizer              13.2.0               h7e041cc_5    conda-forge
libstdcxx-devel_linux-64  13.2.0             ha9c7c90_105    conda-forge
libstdcxx-ng              13.2.0               h7e041cc_5    conda-forge
sysroot_linux-64          2.12                he073ed8_17    conda-forge

Environment info

active environment : compiler
    active env location : /home/yevhenii/.miniforge3/envs/compiler
            shell level : 1
       user config file : /home/yevhenii/.condarc
 populated config files : /home/yevhenii/.miniforge3/.condarc
                          /home/yevhenii/.condarc
          conda version : 24.1.2
    conda-build version : not installed
         python version : 3.10.14.final.0
                 solver : libmamba (default)
       virtual packages : __archspec=1=skylake
                          __conda=24.1.2=0
                          __glibc=2.35=0
                          __linux=5.15.123.1=0
                          __unix=0=0
       base environment : /home/yevhenii/.miniforge3  (writable)
      conda av data dir : /home/yevhenii/.miniforge3/etc/conda
  conda av metadata url : None
           channel URLs : https://repo.anaconda.com/pkgs/main/linux-64
                          https://repo.anaconda.com/pkgs/main/noarch
                          https://repo.anaconda.com/pkgs/r/linux-64
                          https://repo.anaconda.com/pkgs/r/noarch
                          https://conda.anaconda.org/conda-forge/linux-64
                          https://conda.anaconda.org/conda-forge/noarch
          package cache : /home/yevhenii/.miniforge3/pkgs
                          /home/yevhenii/.conda/pkgs
       envs directories : /home/yevhenii/.miniforge3/envs
                          /home/yevhenii/.conda/envs
               platform : linux-64
             user-agent : conda/24.1.2 requests/2.31.0 CPython/3.10.14 Linux/5.15.123.1-microsoft-standard-WSL2 ubuntu/22.04.4 glibc/2.35 solver/libmamba conda-libmamba-solver/24.1.0 libmambapy/1.5.7
                UID:GID : 1000:1000
             netrc file : /home/yevhenii/.netrc
           offline mode : False
h-vetinari commented 3 months ago

It's easy to confirm in godbolt that this should work as claimed in the OP.

The first suspicion I have is that this is somehow due to the underlying glibc, because std::isinf is in <cmath>, which wraps the C function isinf, and it's possible that the implementation in glib 2.12 (current baseline in conda-forge, though we're planning to move to 2.17 this summer) is deficient in that regard.

ZzEeKkAa commented 3 months ago

@h-vetinari thank you! Is there any way now to check if glib is the problem for the current issue?

h-vetinari commented 3 months ago

You can add a compilation test to the recipe here, plus - {{ stdlib("c") }} under the build requirements, as well as

c_stdlib_version:    # [linux]
  - 2.17             # [linux]

under recipe/conda_build_config.yaml and then let the bot rerender the PR (or do it locally).

ZzEeKkAa commented 3 months ago

I did some investigation with current build and that's what I've found:

from /home/yevhenii/.miniforge3/envs/compiler/x86_64-conda-linux-gnu/include/c++/13.2.0/cmath:

#ifndef __CORRECT_ISO_CPP11_MATH_H_PROTO_FP
  constexpr bool
  isnan(float __x)
  { return __builtin_isnan(__x); }

#if _GLIBCXX_HAVE_OBSOLETE_ISNAN \
  && !_GLIBCXX_NO_OBSOLETE_ISINF_ISNAN_DYNAMIC
  using ::isnan;
#else
  constexpr bool
  isnan(double __x)
  { return __builtin_isnan(__x); }
#endif

  constexpr bool
  isnan(long double __x)
  { return __builtin_isnan(__x); }
#endif

Then I've created test file:

#include <cmath>
#include <cstdio>

int main(){
#ifndef __CORRECT_ISO_CPP11_MATH_H_PROTO_FP
  printf("define float\n");

#if _GLIBCXX_HAVE_OBSOLETE_ISNAN \
  && !_GLIBCXX_NO_OBSOLETE_ISINF_ISNAN_DYNAMIC
  printf("not define double\n");
#else
  printf("define double\n");
#endif
  printf("define long double\n");
#endif

#if _GLIBCXX_HAVE_OBSOLETE_ISNAN 
  printf("_GLIBCXX_HAVE_OBSOLETE_ISNAN\n");
#endif

#if _GLIBCXX_NO_OBSOLETE_ISINF_ISNAN_DYNAMIC
  printf("_GLIBCXX_NO_OBSOLETE_ISINF_ISNAN_DYNAMIC\n");
#endif

  return 0;
}

compile + run:

define float
not define double
define long double
_GLIBCXX_HAVE_OBSOLETE_ISNAN

So, for some reason cmath jumps into branch using ::isnan instead of defining isnan(double __x).

@h-vetinari do you know why is it happening?

ZzEeKkAa commented 3 months ago

P.S.: output for the system g++:

define float
define double
define long double
_GLIBCXX_NO_OBSOLETE_ISINF_ISNAN_DYNAMIC
h-vetinari commented 3 months ago

So it's not super-obvious how _GLIBCXX_HAVE_OBSOLETE_ISNAN ends up being set, but AFAICT it comes through: libstdc++-v3/acinclude.m4 --> libstdc++-v3/config.h.in --> ... --> bits/c++config.h, which, in our packaging (configured against very old glibc), ends up producing

/* Define if <math.h> defines obsolete isinf function. */
#define _GLIBCXX_HAVE_OBSOLETE_ISINF 1

I also found the following commit: https://github.com/gcc-mirror/gcc/commit/350fe2829e2012d0e768602102fac31b474b1d2c, which means that _GLIBCXX_NO_OBSOLETE_ISINF_ISNAN_DYNAMIC should be defined as true on systems with glibc >=2.23 through bits/os_defines.h (which I checked is #included in a local bits/c++config.h as packaged in our libstdcxx-devel_linux-64).

Given that your conda info shows __glibc=2.35=0, you should definitely satisfy the conditions for that, so I'm confused why _GLIBCXX_NO_OBSOLETE_ISINF_ISNAN_DYNAMIC isn't true. Could you distinguish in your little test script between #ifdef <var> and #if <var>?

isuruf commented 3 months ago

I can reproduce the same issue on Ubuntu 22.04 with gcc 11.4.0. So this is not unique to conda. Probably a gcc bug.

h-vetinari commented 3 months ago

How did you reproduce it exactly? According to godbolt, it works back all the way to GCC + libstdcxx v7

isuruf commented 3 months ago

Ubuntu 22.04 with stock gcc 11.4.0

h-vetinari commented 3 months ago

Probably a gcc bug.

Or both ubuntu and conda-forge have the same kind of packaging bug...

h-vetinari commented 3 months ago

Ubuntu 22.04 with stock gcc 11.4.0

What's the output of the test script above? What are the values of _GLIBCXX_HAVE_OBSOLETE_ISNAN / _GLIBCXX_NO_OBSOLETE_ISINF_ISNAN_DYNAMIC?

ZzEeKkAa commented 3 months ago

Could you distinguish in your little test script between #ifdef <var> and #if <var>?

test.cpp:

#include <cmath>
#include <cstdio>
#include <gnu/libc-version.h>

int main(){
  printf("GNU libc version: %s\n", gnu_get_libc_version());
  printf("GNU libc release: %s\n", gnu_get_libc_release());

#ifndef __CORRECT_ISO_CPP11_MATH_H_PROTO_FP
  printf("define float\n");

#if _GLIBCXX_HAVE_OBSOLETE_ISNAN \
  && !_GLIBCXX_NO_OBSOLETE_ISINF_ISNAN_DYNAMIC
  printf("not define double\n");
#else
  printf("define double\n");
#endif
  printf("define long double\n");
#endif

#if _GLIBCXX_HAVE_OBSOLETE_ISNAN 
  printf("_GLIBCXX_HAVE_OBSOLETE_ISNAN\n");
#endif

#if _GLIBCXX_NO_OBSOLETE_ISINF_ISNAN_DYNAMIC
  printf("_GLIBCXX_NO_OBSOLETE_ISINF_ISNAN_DYNAMIC\n");
#endif

#ifdef _GLIBCXX_HAVE_OBSOLETE_ISNAN 
  printf("def _GLIBCXX_HAVE_OBSOLETE_ISNAN\n");
#endif

#ifdef _GLIBCXX_NO_OBSOLETE_ISINF_ISNAN_DYNAMIC
  printf("def _GLIBCXX_NO_OBSOLETE_ISINF_ISNAN_DYNAMIC\n");
#endif

  return 0;
}

Ubuntu 22.04 output:

GNU libc version: 2.35
GNU libc release: stable
define float
define double
define long double
_GLIBCXX_NO_OBSOLETE_ISINF_ISNAN_DYNAMIC
def _GLIBCXX_NO_OBSOLETE_ISINF_ISNAN_DYNAMIC

Conda output:

GNU libc version: 2.35
GNU libc release: stable
define float
not define double
define long double
_GLIBCXX_HAVE_OBSOLETE_ISNAN
def _GLIBCXX_HAVE_OBSOLETE_ISNAN
def _GLIBCXX_NO_OBSOLETE_ISINF_ISNAN_DYNAMIC
h-vetinari commented 3 months ago

Thanks! So the macro is defined but false.

I'm absolutely mystified how

#define _GLIBCXX_NO_OBSOLETE_ISINF_ISNAN_DYNAMIC __GLIBC_PREREQ(2,23)

can evaluate to false on your system.

Ubuntu 22.04 output:

Can you provide a few more details on how you ran this? @isuruf said he can reproduce the problem on 22.04 with stock GCC, yet your 22.04 looks fine?

ZzEeKkAa commented 3 months ago

@h-vetinari it is actually strange, because I've just tried stock ubuntu in docker container:

$ docker run -it --rm --platform linux/x86_64 ubuntu:22.04 bash
root@58295fbcf5e2:/# ldd --version
ldd (Ubuntu GLIBC 2.35-0ubuntu3.6) 2.35
Copyright (C) 2022 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
Written by Roland McGrath and Ulrich Drepper.
root@58295fbcf5e2:/# apt update && apt install g++ vim
root@58295fbcf5e2:/# vim test.cpp
root@58295fbcf5e2:/# g++ test.cpp
root@58295fbcf5e2:/# ./a.out
GNU libc version: 2.35
GNU libc release: stable
define float
define double
define long double
_GLIBCXX_NO_OBSOLETE_ISINF_ISNAN_DYNAMIC
def _GLIBCXX_NO_OBSOLETE_ISINF_ISNAN_DYNAMIC
ZzEeKkAa commented 3 months ago

And 20.04 works as well (g++ (Ubuntu 9.4.0-1ubuntu1~20.04.2) 9.4.0):

GNU libc version: 2.31
GNU libc release: stable
define float
define double
define long double
_GLIBCXX_NO_OBSOLETE_ISINF_ISNAN_DYNAMIC
def _GLIBCXX_NO_OBSOLETE_ISINF_ISNAN_DYNAMIC

Ubuntu 18.04 (g++ (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0):

GNU libc version: 2.27
GNU libc release: stable
define float
define double
define long double
_GLIBCXX_NO_OBSOLETE_ISINF_ISNAN_DYNAMIC
def _GLIBCXX_NO_OBSOLETE_ISINF_ISNAN_DYNAMIC

Ubuntu 16.04 (g++ (Ubuntu 5.4.0-6ubuntu1~16.04.12) 5.4.0):


GNU libc version: 2.23
GNU libc release: stable
define float
define double
define long double

Ubuntu 14.04 (g++ (Ubuntu 4.8.4-2ubuntu1~14.04.4) 4.8.4):

GNU libc version: 2.19
GNU libc release: stable
define float
define double
define long double
h-vetinari commented 3 months ago

Can you add __GLIBC_PREREQ(2,23) to the printed outputs? Something like

#include <features.h>
#define STR(x)   #x
#define SHOW_DEFINE(x) printf("%s=%s\n", #x, STR(x))

int main() {
  ...
  SHOW_DEFINE(__GLIBC_PREREQ(2,23));
  SHOW_DEFINE(_GLIBCXX_NO_OBSOLETE_ISINF_ISNAN_DYNAMIC);
}

In godbolt this gets me

__GLIBC_PREREQ(2,23)=((2 << 16) + 35 >= ((2) << 16) + (23))
_GLIBCXX_NO_OBSOLETE_ISINF_ISNAN_DYNAMIC=((2 << 16) + 35 >= ((2) << 16) + (23))
ZzEeKkAa commented 3 months ago

@h-vetinari sorry for long response, here is the output of conda:

GNU libc version: 2.35
GNU libc release: stable
define float
not define double
define long double
_GLIBCXX_HAVE_OBSOLETE_ISNAN
def _GLIBCXX_HAVE_OBSOLETE_ISNAN
def _GLIBCXX_NO_OBSOLETE_ISINF_ISNAN_DYNAMIC
__GLIBC_PREREQ(2,23)=((2 << 16) + 12 >= ((2) << 16) + (23))
_GLIBCXX_NO_OBSOLETE_ISINF_ISNAN_DYNAMIC=((2 << 16) + 12 >= ((2) << 16) + (23))

And ubuntu output:

GNU libc version: 2.35
GNU libc release: stable
define float
define double
define long double
_GLIBCXX_NO_OBSOLETE_ISINF_ISNAN_DYNAMIC
def _GLIBCXX_NO_OBSOLETE_ISINF_ISNAN_DYNAMIC
__GLIBC_PREREQ(2,23)=((2 << 16) + 35 >= ((2) << 16) + (23))
_GLIBCXX_NO_OBSOLETE_ISINF_ISNAN_DYNAMIC=((2 << 16) + 35 >= ((2) << 16) + (23))

See the difference + 12 and + 35

ZzEeKkAa commented 3 months ago

From sources:

#define __GLIBC_PREREQ(maj, min) \
    ((__GLIBC__ << 16) + __GLIBC_MINOR__ >= ((maj) << 16) + (min))

That means, that __GLIBC_MINOR__ is defined as 12

So, having that, even upgrade to glibc 2.17 won't solve the issue.

ZzEeKkAa commented 3 months ago

Having that __GLIBC_MINOR__ and __GLIBC_PREREQ can't be changed, we need to focus on how _GLIBCXX_HAVE_OBSOLETE_ISNAN and _GLIBCXX_NO_OBSOLETE_ISINF_ISNAN_DYNAMIC are defined:

/home/yevhenii/.miniforge3/envs/compiler/x86_64-conda-linux-gnu/include/c++/13.2.0/x86_64-conda-linux-gnu/bits/os_defines.h:
#define _GLIBCXX_NO_OBSOLETE_ISINF_ISNAN_DYNAMIC __GLIBC_PREREQ(2,23)
/home/yevhenii/.miniforge3/envs/compiler/x86_64-conda-linux-gnu/include/c++/13.2.0/x86_64-conda-linux-gnu/bits/c++config.h:
#define _GLIBCXX_HAVE_OBSOLETE_ISNAN 1

Both those files are maintained by libstdcxx-devel.

My bet, is if conda is restricted to 2.12 glibc, compiler must not introduce environment variables set from later releases. But it's just a blind guess

h-vetinari commented 3 months ago

Having that __GLIBC_MINOR__ and __GLIBC_PREREQ can't be changed,

__GLIBC_MINOR__ should not be 12 if you're on a glibc 2.35 system! Something somewhere seems to be overriding that macro.

h-vetinari commented 3 months ago

__GLIBC_MINOR__ should not be 12 if you're on a glibc 2.35 system! Something somewhere seems to be overriding that macro.

Wait, this is probably coming in due to the presence of

sysroot_linux-64          2.12                he073ed8_17    conda-forge

Can you try installing sysroot_linux-64 =2.28?

If I do that, I get the following output with our compilers:

GNU libc version: 2.35
GNU libc release: stable
define float
define double
define long double
_GLIBCXX_NO_OBSOLETE_ISINF_ISNAN_DYNAMIC
def _GLIBCXX_NO_OBSOLETE_ISINF_ISNAN_DYNAMIC
ZzEeKkAa commented 3 months ago

@h-vetinari yes! Sorry, I've looked at sysroot_linux-64 and for some reason thought it is glibc version. Indeed Setting sysroot_linux-64==2.28 gives the expected output:

GNU libc version: 2.35
GNU libc release: stable
define float
define double
define long double
_GLIBCXX_HAVE_OBSOLETE_ISNAN
_GLIBCXX_NO_OBSOLETE_ISINF_ISNAN_DYNAMIC
def _GLIBCXX_HAVE_OBSOLETE_ISNAN
def _GLIBCXX_NO_OBSOLETE_ISINF_ISNAN_DYNAMIC
__GLIBC_PREREQ(2,23)=((2 << 16) + 28 >= ((2) << 16) + (23))
_GLIBCXX_NO_OBSOLETE_ISINF_ISNAN_DYNAMIC=((2 << 16) + 28 >= ((2) << 16) + (23))

And indeed:

/home/yevhenii/.miniforge3/envs/compiler/x86_64-conda-linux-gnu/sysroot/usr/include/features.h:#define  __GLIBC_MINOR__ 28

sysroot is overriding it. Do you know how to use real glibc version?

h-vetinari commented 3 months ago

Do you know how to use real glibc version?

Pointing to our own sysroot is kind of built-into our infrastructure, in the sense that we want to control the glibc baseline that packages build against, so that packages are widely installable, and don't end up depending on glibc features not available on older systems.

Since this is a runtime dependence here in the recipe https://github.com/conda-forge/ctng-compiler-activation-feedstock/blob/bc40bf36410363e2f87cffdada4e33d302c9f7f6/recipe/meta.yaml#L141 you also cannot override this with the currently-published artefacts (and we can't remove it here because it's required to keep our infrastructure working).

Possible answers (we'll definitely need input from @conda-forge/linux-sysroot here):