Avoid using glibc-specific strerror_r variant

[rcombs]

IGC currently calls strerror_r and expects it to return a const char*:

intel-graphics-compiler/visa/iga/IGALibrary/system.cpp

Line 263 in 9307c01

errMsg = strerror_r(errCode, buf, sizeof(buf));

This is glibc-specific behavior. The POSIX behavior is for strerror_r to always write its output to the passed-in buffer, and to return an integer error code. This behavior is the only variant available in musl, the default on android bionic, and is available in glibc by defining the _POSIX_C_SOURCE macro to a value ≥ 200112L while not setting _GNU_SOURCE.

Could IGC set _POSIX_C_SOURCE before including headers in system.cpp and switch to expecting the POSIX behavior for this, to improve portability?

[trbauer]

Excellent. (At a glace) I agree. We desire portability here.

[jbeich]

Obsolete after f2918ee ?

[rcombs]

f2918ee looks potentially incorrect to me; the glibc version of strerror_r doesn't necessarily write its output to the passed-in buffer (it may instead just return a static string).

[mnaczk]

A new commit with the fix proposition was merged with the repo. 7f14252 @rcombs Could you please check if this solves the issue?

[rcombs]

Looking at the new code:

#if (_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600) && ! _GNU_SOURCE

This checks the glibc behavior, but doesn't check that glibc is in use. You need to add a check for __GLIBC__ :

#if !defined(__GLIBC__) || ((_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600) && ! _GNU_SOURCE)

This way, other libc implementations will get the standard behavior regardless.

[mnaczk]

The next fix proposition was merged with the repo. b1adbfe @rcombs and @jbeich Could you please check if this solves the issue?

[jbeich]

Still fails on FreeBSD:

visa/iga/IGALibrary/system.cpp:267:18: error: incompatible integer to pointer conversion assigning to 'char *' from 'int'
        errMsg = strerror_r_return_value;
                 ^~~~~~~~~~~~~~~~~~~~~~~

[rcombs]

You need if constexpr to avoid that.

[mnaczk]

Thanks @rcombs for the suggestion, @jbeich Did commit 3759098 fix the building crash on FreeBSD?

[jbeich]

No, if constexpr didn't help. igc-1.0.11378-160-gec864ca53 still fails.

Clang/libc++ 14.0.5 reports:

visa/iga/IGALibrary/system.cpp:267:18: error: incompatible integer to pointer conversion assigning to 'char *' from 'int'
        errMsg = strerror_r_return_value;
                 ^~~~~~~~~~~~~~~~~~~~~~~

GCC/libstdc++ 11.3.0 reports:

visa/iga/IGALibrary/system.cpp: In function 'std::string iga::FormatLastError(unsigned int)':
visa/iga/IGALibrary/system.cpp:267:18: error: invalid conversion from 'int' to 'char*' [-fpermissive]
  267 |         errMsg = strerror_r_return_value;
      |                  ^~~~~~~~~~~~~~~~~~~~~~~
      |                  |
      |                  int

[rcombs]

Ah, I see, constexpr-if only works within templates. You'd need something like this: https://godbolt.org/z/ojfceaEKj

[mnaczk]

Next fix proposal d5231d0 @jbeich Did this commit fix build break on FreeBSD?

[jbeich]

Thanks. igc-1.0.11485-74-gd5231d0db built fine on FreeBSD.