Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

0.36.0 incorrectly detects posix_memalign on MSYS2 (native MINGW64 and MINGW32 targets/shells/environs) #1083

Closed
achadwick opened this issue Nov 22, 2016 · 15 comments · Fixed by #1150
Labels
bug compilers gnome OS:windows Winodows OS specific issues

Comments

@achadwick
Copy link

Reporting on @ebassi's behalf, since they're busy. This is affecting downstream at ebassi/graphene#76.

Meson incorrectly detects the presence of posix_memalign() with the following in the project's meson.build:

conf.set('HAVE_POSIX_MEMALIGN', cc.has_function('posix_memalign', prefix: '#include <stdlib.h>'))

The output in meson-logs/meson-log.txt seems to indicate that it's settling on __builtin_posix_memalign compiling, and using that as a substitute for the real thing. But I have never used meson before, so I don't really understand this.

[...]
Checking for function "memalign": NO
Running compile:
Working directory:  C:/Users/TESTUS~1/AppData/Local/Temp/tmp2yaurewq
Command line:  cc C:/Users/TESTUS~1/AppData/Local/Temp/tmp2yaurewq/testfile.c -std=gnu99 -lkernel32 -luser32 -lgdi32 -lwinspool -lshell32 -lole32 -loleaut32 -luuid -lcomdlg32 -ladvapi32 -O0 -o C:/Users/TESTUS~1/AppData/Local/Temp/tmp2yaurewq/output.exe 

Code:
 #include <limits.h>
#include <stdlib.h>

        #if defined __stub_posix_memalign || defined __stub___posix_memalign
        fail fail fail this function is not going to work
        #endif
        
int main() { void *a = (void*) &posix_memalign; }
Compiler stdout:
 C:/Users/TESTUS~1/AppData/Local/Temp/tmp2yaurewq/testfile.c: In function 'main':

C:/Users/TESTUS~1/AppData/Local/Temp/tmp2yaurewq/testfile.c:8:33: error: 'posix_memalign' undeclared (first use in this function)

 int main() { void *a = (void*) &posix_memalign; }

                                 ^~~~~~~~~~~~~~

C:/Users/TESTUS~1/AppData/Local/Temp/tmp2yaurewq/testfile.c:8:33: note: each undeclared identifier is reported only once for each function it appears in


Compiler stderr:
 
Running compile:
Working directory:  C:/Users/TESTUS~1/AppData/Local/Temp/tmpk7achqy2
Command line:  cc C:/Users/TESTUS~1/AppData/Local/Temp/tmpk7achqy2/testfile.c -std=gnu99 -lkernel32 -luser32 -lgdi32 -lwinspool -lshell32 -lole32 -loleaut32 -luuid -lcomdlg32 -ladvapi32 -O0 -o C:/Users/TESTUS~1/AppData/Local/Temp/tmpk7achqy2/output.exe 

Code:
 int main() { __builtin_posix_memalign; }
Compiler stdout:
 
Compiler stderr:
 
Checking for function "posix_memalign": YES
[...]

The function is not defined in any header the MINGW64 and MINGW32 builds should be using.

Test User@win7test MINGW64 /usr/src/graphene
$ grep -R posix_memalign /mingw{64,32}/include /usr/include

Test User@win7test MINGW64 /usr/src/graphene
$ find /mingw{64,32}/include /usr/include -name 'stdlib.*'
/mingw64/include/c++/6.2.0/stdlib.h
/mingw64/include/c++/6.2.0/tr1/stdlib.h
/mingw32/include/c++/6.2.0/stdlib.h
/mingw32/include/c++/6.2.0/tr1/stdlib.h
@ignatenkobrain
Copy link
Member

I think it's very similar to #1053 where @nirbheek promised to send another PR as well.

@achadwick
Copy link
Author

I'll give that a try. I was half-expecting it to be an optimization thing tbh. Let's see if my instincts are any good...

@nirbheek
Copy link
Member

This is unrelated to #1053. What's happening is that somehow the libc you're using is defining __builtin_posix_memalign, which signifies that it's a compiler built-in that will be substituted with the implementation at compile-time. See: https://github.com/mesonbuild/meson/blob/master/mesonbuild/compilers.py#L986

You will need to check which libc is being used for linking, which gcc is being used, and why your compiler decides that this is available in your environment. The exact same check is done by Autotools.

@achadwick
Copy link
Author

Confirmed not a dup. I'm completely lost now, other than to confirm the version I'm using:

Test User@win7test MINGW64 /usr/src/graphene/_build
$ gcc --version
gcc.exe (Rev2, Built by MSYS2 project) 6.2.0
Copyright (C) 2016 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.


Test User@win7test MINGW64 /usr/src/graphene/_build
$ pacman -Qo `which gcc.exe`
/mingw64/bin/gcc.exe is owned by mingw-w64-x86_64-gcc 6.2.0-2

And the fact that #1066 does not fix it like @nirbheek said:

Compiler stdout:
 C:/Users/TESTUS~1/AppData/Local/Temp/tmp29u5wr84/testfile.c: In function 'main'
:
C:/Users/TESTUS~1/AppData/Local/Temp/tmp29u5wr84/testfile.c:8:33: error: 'posix_
memalign' undeclared (first use in this function)
 int main() { void *a = (void*) &posix_memalign; long b = (long) a; return (int)
 b; }
                                 ^~~~~~~~~~~~~~
C:/Users/TESTUS~1/AppData/Local/Temp/tmp29u5wr84/testfile.c:8:33: note: each und
eclared identifier is reported only once for each function it appears in
C:/Users/TESTUS~1/AppData/Local/Temp/tmp29u5wr84/testfile.c:8:58: warning: cast
from pointer to integer of different size [-Wpointer-to-int-cast]
 int main() { void *a = (void*) &posix_memalign; long b = (long) a; return (int)
 b; }
                                                          ^

Compiler stderr:

Running compile:
Working directory:  C:/Users/TESTUS~1/AppData/Local/Temp/tmp8zkcy2jf
Command line:  cc C:/Users/TESTUS~1/AppData/Local/Temp/tmp8zkcy2jf/testfile.c -std=gnu99 -lkernel32 -luser32 -lgdi32 -lwinspool -lshell32 -lole32 -loleaut32 -luuid -lcomdlg32 -ladvapi32 -O0 -o C:/Users/TESTUS~1/AppData/Local/Temp/tmp8zkcy2jf/output.exe

Code:
 int main() { __builtin_posix_memalign; }
Compiler stdout:

Compiler stderr:

Checking for function "posix_memalign": YES

@achadwick
Copy link
Author

I think in this case, meson is not doing its job properly. It was asked to confirm the presence of a specific function, and it failed. It then went looking for a similarly-named builtin, found that, and then declared that the original function was present. This feels wrong, but I'm no expert (not in meson, and not really in autotools).

Why bother with the fallback?

@nirbheek
Copy link
Member

Meson might be doing something wrong, but till the questions I asked are answered, we can't know.

@nirbheek
Copy link
Member

To clarify, cc.has_function checks whether a symbol is provided by the libc. It does not require headers for doing this check (same as AC_CHECK_FUNC ). For most functions, this is fine.

However, in the case of builtins this does not work because for those somefunction will not be exported by the libc. This is the case for several POSIX functions. With Clang and GCC built-in functions are exported as __builtin_somefunction instead, which is why we check for that if the original function is not found.

If the libc exports __builtin_posix_memalign (signalling that it provides an inline implementaion of posix_memalign) but doesn't provide the #define posix_memalign __builtin_posix_memalign that allows applications to use it, then that is almost certainly a MinGW libc bug and this will fail with both Meson and Autotools in the same way.

Hence, we require more investigation before we can know what the appropriate solution is.

@achadwick
Copy link
Author

You will need to check which libc is being used for linking, which gcc is being used,

MSYS2 uses GCC 6.2.0 obtained from ftp.gnu.org/gnu/gcc, with the patches you can see at https://github.com/Alexpux/MINGW-packages/tree/master/mingw-w64-gcc.

$ cc -v
Using built-in specs.
COLLECT_GCC=C:\msys64\mingw64\bin\cc.exe
COLLECT_LTO_WRAPPER=C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/6.2.0/lto-wrapper.exe
Target: x86_64-w64-mingw32
Configured with: ../gcc-6.2.0/configure --prefix=/mingw64 --with-local-prefix=/mingw64/local --build=x86_64-w64-mingw32 --host=x86_64-w64-mingw32 --target=x86_64-w64-mingw32 --with-native-system-header-dir=/mingw64/x86_64-w64-mingw32/include --libexecdir=/mingw64/lib --enable-bootstrap --with-arch=x86-64 --with-tune=generic --enable-languages=c,lto,c++,objc,obj-c++,fortran,ada --enable-shared --enable-static --enable-libatomic --enable-threads=posix --enable-graphite --enable-fully-dynamic-string --enable-libstdcxx-time=yes --disable-libstdcxx-pch --disable-libstdcxx-debug --disable-isl-version-check --enable-lto --enable-libgomp --disable-multilib --enable-checking=release --disable-rpath --disable-win32-registry --disable-nls --disable-werror --disable-symvers --with-libiconv --with-system-zlib --with-gmp=/mingw64 --with-mpfr=/mingw64 --with-mpc=/mingw64 --with-isl=/mingw64 --with-pkgversion='Rev2, Built by MSYS2 project' --with-bugurl=https://sourceforge.net/projects/msys2 --with-gnu-as --with-gnu-ld
Thread model: posix
gcc version 6.2.0 (Rev2, Built by MSYS2 project)

And yeah,

$ grep -r posix_memalign /mingw{64/x86_64,32/i686}-w64-mingw32/include/
$ pacman -Qo /mingw{64/x86_64,32/i686}-w64-mingw32/include/stdlib.h
/mingw64/x86_64-w64-mingw32/include/stdlib.h is owned by mingw-w64-x86_64-headers-git 5.0.0.4747.0f8f626-1
/mingw32/i686-w64-mingw32/include/stdlib.h is owned by mingw-w64-i686-headers-git 5.0.0.4747.0f8f626-1
$ pacman -Ss mingw-w64-i686-headers-git
mingw32/mingw-w64-i686-headers-git 5.0.0.4747.0f8f626-1 (mingw-w64-i686-toolchain) [installed]
    MinGW-w64 headers for Windows
$ pacman -Ss mingw-w64-x86_64-headers-git
mingw64/mingw-w64-x86_64-headers-git 5.0.0.4747.0f8f626-1 (mingw-w64-x86_64-toolchain) [installed]
    MinGW-w64 headers for Windows

These are part of the standard toolchain packages for this platform, and it's a fresh install (not that this is the cause of anything, I hope).

The headers are built from the following sources: https://github.com/Alexpux/MINGW-packages/tree/master/mingw-w64-headers-git on top of https://sourceforge.net/p/mingw-w64/mingw-w64/ci/master/tree/.

As for the libc:

$ ldd junk.exe
        ntdll.dll => /c/Windows/SYSTEM32/ntdll.dll (0x77990000)
        kernel32.dll => /c/Windows/system32/kernel32.dll (0x77870000)
        KERNELBASE.dll => /c/Windows/system32/KERNELBASE.dll (0x7fefda10000)
        msvcrt.dll => /c/Windows/system32/msvcrt.dll (0x7feff2b0000)

and why your compiler decides that this is available in your environment.

Sorry, I don't know enough to help you diagnose that.

@nirbheek
Copy link
Member

So now what's left to check is why the mingw gcc provided by msys2 is exporting __builtin_posix_memalign, and what it inlines to. You might have to look at the compiled object and/or the source code of GCC to figure this out.

For instance, the compiler might be inlining a valid implementation and the bug might just be that MinGW neglects to add a proper header definition to use it in your application. Or, it might be replacing it with an equivalent invocation, or it's substituting a broken implementation, etc.

You will also want to try with AC_CHECK_FUNC and see if it is incorrectly detected there too. If it is not, then we should just do what Autoconf does there.

@nirbheek nirbheek added bug compilers OS:windows Winodows OS specific issues gnome labels Nov 28, 2016
@achadwick
Copy link
Author

It's correctly detected as absent by the MSYS2 autoconf 2.69-3 (pkgbuild) that would have been used for the earlier graphene builds.

## Tests for @nirbheek ##
AC_CHECK_FUNC([aligned_alloc])
AC_CHECK_FUNC([posix_memalign])
AC_CHECK_FUNC([memalign])
checking for aligned_alloc... no
checking for posix_memalign... no
checking for memalign... no

Here's the shell code that gets written into ./configure:

# [...]

# ac_fn_c_try_link LINENO
# -----------------------
# Try to link conftest.$ac_ext, and return whether this succeeded.
ac_fn_c_try_link ()
{
  as_lineno=${as_lineno-"$1"} as_lineno_stack=as_lineno_stack=$as_lineno_stack
  rm -f conftest.$ac_objext conftest$ac_exeext
  if { { ac_try="$ac_link"
case "(($ac_try" in
  *\"* | *\`* | *\\*) ac_try_echo=\$ac_try;;
  *) ac_try_echo=$ac_try;;
esac
eval ac_try_echo="\"\$as_me:${as_lineno-$LINENO}: $ac_try_echo\""
$as_echo "$ac_try_echo"; } >&5
  (eval "$ac_link") 2>conftest.err
  ac_status=$?
  if test -s conftest.err; then
    grep -v '^ *+' conftest.err >conftest.er1
    cat conftest.er1 >&5
    mv -f conftest.er1 conftest.err
  fi
  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
  test $ac_status = 0; } && {
         test -z "$ac_c_werror_flag" ||
         test ! -s conftest.err
       } && test -s conftest$ac_exeext && {
         test "$cross_compiling" = yes ||
         test -x conftest$ac_exeext
       }; then :
  ac_retval=0
else
  $as_echo "$as_me: failed program was:" >&5
sed 's/^/| /' conftest.$ac_ext >&5

        ac_retval=1
fi
  # Delete the IPA/IPO (Inter Procedural Analysis/Optimization) information
  # created by the PGI compiler (conftest_ipa8_conftest.oo), as it would
  # interfere with the next link command; also delete a directory that is
  # left behind by Apple's compiler.  We do this before executing the actions.
  rm -rf conftest.dSYM conftest_ipa8_conftest.oo
  eval $as_lineno_stack; ${as_lineno_stack:+:} unset as_lineno
  as_fn_set_status $ac_retval

} # ac_fn_c_try_link

# [...]

# ac_fn_c_check_func LINENO FUNC VAR
# ----------------------------------
# Tests whether FUNC exists, setting the cache variable VAR accordingly
ac_fn_c_check_func ()
{
  as_lineno=${as_lineno-"$1"} as_lineno_stack=as_lineno_stack=$as_lineno_stack
  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for $2" >&5
$as_echo_n "checking for $2... " >&6; }
if eval \${$3+:} false; then :
  $as_echo_n "(cached) " >&6
else
  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
/* end confdefs.h.  */
/* Define $2 to an innocuous variant, in case <limits.h> declares $2.
   For example, HP-UX 11i <limits.h> declares gettimeofday.  */
#define $2 innocuous_$2

/* System header to define __stub macros and hopefully few prototypes,
    which can conflict with char $2 (); below.
    Prefer <limits.h> to <assert.h> if __STDC__ is defined, since
    <limits.h> exists even on freestanding compilers.  */

#ifdef __STDC__
# include <limits.h>
#else
# include <assert.h>
#endif

#undef $2

/* Override any GCC internal prototype to avoid an error.
   Use char because int might match the return type of a GCC
   builtin and then its argument prototype would still apply.  */
#ifdef __cplusplus
extern "C"
#endif
char $2 ();
/* The GNU C library defines this for functions which it implements
    to always fail with ENOSYS.  Some functions are actually named
    something starting with __ and the normal name is an alias.  */
#if defined __stub_$2 || defined __stub___$2
choke me
#endif

int
main ()
{
return $2 ();
  ;
  return 0;
}
_ACEOF
if ac_fn_c_try_link "$LINENO"; then :
  eval "$3=yes"
else
  eval "$3=no"
fi
rm -f core conftest.err conftest.$ac_objext \
    conftest$ac_exeext conftest.$ac_ext
fi
eval ac_res=\$$3
               { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_res" >&5
$as_echo "$ac_res" >&6; }
  eval $as_lineno_stack; ${as_lineno_stack:+:} unset as_lineno

} # ac_fn_c_check_func

# [...]

## Tests for @nirbheek ##
ac_fn_c_check_func "$LINENO" "aligned_alloc" "ac_cv_func_aligned_alloc"
if test "x$ac_cv_func_aligned_alloc" = xyes; then :

fi

ac_fn_c_check_func "$LINENO" "posix_memalign" "ac_cv_func_posix_memalign"
if test "x$ac_cv_func_posix_memalign" = xyes; then :

fi

ac_fn_c_check_func "$LINENO" "memalign" "ac_cv_func_memalign"
if test "x$ac_cv_func_memalign" = xyes; then :

fi

achadwick added a commit to achadwick/graphene that referenced this issue Dec 5, 2016
As recommended by @nirbheek, only search for a single aligned memory
allocation function: ebassi#86 (comment)

The cascade of tests means we can exclude the ones for which MSYS2's
native mingw-w64 builtins and headers conspire to provide meson with a
dud test. Although it is still not clear that meson is doing the right
thing in this case (mesonbuild/meson#1083), we can work around the
failed build in graphene by excluding certain POSIXy cases on Windows.

Closes ebassi#76, leaving ebassi#88 unsolved.
@nirbheek
Copy link
Member

nirbheek commented Dec 6, 2016

Does #1150 fix this for you?

jpakkane pushed a commit that referenced this issue Dec 10, 2016
We were checking for builtins explicitly like this because the ordinary
checks don't work for builtins at all. We do exactly the same check as
Autoconf and it doesn't work with Autoconf either (Autoconf is broken!)

So now we check for it in two ways: if there's no #include in prefix, we
check if `__builtin_symbol` exists (has_function allows checking for
functions without providing includes). If there's a #include, we check
if `symbol` exists.

The old method was causing problems with some buggy toolchains such as
MSYS2 which define some builtins in the C library but don't expose them
via headers which meant that `__builtin_symbol` would be found even
though `symbol` is not available.

Doing this allows people to always get the correct answer as long as
they specify the includes that are required to find a function while
also not forcing people to always specify includes to find a function
which is cumbersome.

Closes #1083
achadwick added a commit to achadwick/graphene that referenced this issue Jan 3, 2017
Closes: ebassi#76

This commit makes the existing flat set of macro tests into a big if-else
block, and selects only one aligned alloc function. If no such function is
available, meson now terminates with an error and the build will fail.

This is as recommended by @nirbheek, who suggests that meson should only
search for a single aligned memory allocation function during configuration
of graphene.

Ref: ebassi#86 (comment)

On MSYS2, a buggy mismatch between in the native mingw-w64 builtins and
headers causes meson < 0.37.0 to report that the function exists because
the underlying builtin exists. Cascading the tests like this works around
breaking configuration on the MSYS2 platform for meson<0.37 by excluding
certain POSIX-only cases on Windows.

Ref: mesonbuild/meson#1083
achadwick added a commit to achadwick/graphene that referenced this issue Jan 3, 2017
Closes: ebassi#76

This commit makes the existing flat set of macro tests into a big if-else
block, and selects only one aligned alloc function. If no such function is
available, meson now terminates with an error and the build will fail.

This is as recommended by @nirbheek, who suggests that meson should only
search for a single aligned memory allocation function during configuration
of graphene.

Ref: ebassi#86 (comment)

On MSYS2, a buggy mismatch between the native mingw-w64 builtins and the
standard headers causes meson < 0.37.0 to report that the function exists
because the underlying builtin exists. Cascading the tests like this works
around breaking configuration on the MSYS2 platform for meson<0.37 by
excluding certain POSIX-only cases on Windows.

Ref: mesonbuild/meson#1083
@lazka
Copy link
Contributor

lazka commented Jul 14, 2017

Still the same with 0.41.1. I've proposed a patch in MSYS2 for now: https://github.com/Alexpux/MINGW-packages/pull/2698 including stpcpy which fails the same way.

@nirbheek
Copy link
Member

This bug is fundamentally unfixable in Meson without a blacklist since this is an MSYS2 toolchain bug. The libc is defining these symbols as available as built-ins (__builtin_posix_memalign, __builtin_stpcpy) but they are not. The workaround is to also specify the header which will tell Meson exactly what type of symbol it is, and it doesn't have to guess.

The actual fix would be in MSYS2's libc. It should not export builtin symbols that it does not support.

@nirbheek
Copy link
Member

The other option is for GCC to start supporting __has_builtin like clang so we can actually reliably detect builtin functions.

@lazka
Copy link
Contributor

lazka commented Jul 15, 2017

OK, thanks for the additional info. I don't have any experience with the msys2 toolchain internals, so the downstream patch will have to do for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug compilers gnome OS:windows Winodows OS specific issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants