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

False positive in has_function() #3740

Open
xclaesse opened this issue Jun 14, 2018 · 10 comments
Open

False positive in has_function() #3740

xclaesse opened this issue Jun 14, 2018 · 10 comments
Assignees

Comments

@xclaesse
Copy link
Member

cc.has_function('ngettext', prefix : '#include <libintl.h>') returns true when cross compiling with mingw, but it should return false because it also needs '-lintl' to be able to link.

The reason is CCompiler.has_function() first try to link a code and that fails. Then it tries to detect built-in, #ifdef __has_builtin is false, and ! defined({func}) is false too because mingw's libintl.h has this: # define ngettext libintl_ngettext. So that built-in check succeed and it shouldn't.

@xclaesse
Copy link
Member Author

xclaesse commented Jun 14, 2018

I'm not sure, but I think that code is missing a

#else
  #error "function defined but does not link"
#endif

EDIT: not, that break unit tests, 'alloca' is not found with that change.

@ePirat
Copy link
Contributor

ePirat commented Jun 14, 2018

Seems this is a duplicate of either #3672 or #3676, no?

See also PR #3675

@xclaesse
Copy link
Member Author

I don't think it's a dup, looks like more an extra way it's broken. Here it has nothing to do with builtin actually, it's for a case where a header #define the symbol to something that is implemented into a library and meson claims it works without actually linking to that lib.

@nirbheek nirbheek self-assigned this Jun 18, 2018
@nirbheek
Copy link
Member

I'll update #3675 with a test for this.

buildroot-auto-update pushed a commit to buildroot/buildroot that referenced this issue Jul 1, 2019
libglib2 uses a very crude and error-prone way to detect the intl
functions, which basically fails when the C library is not glibc.

There is a bug report about this in upstream meson [1], but it doesn't
seem to get any progress. Fixing that properly in Buildroot looks
complicated.

Now that a meson package can specify its LDFLAGS, use that to pass the
infrastructure-provided TARGET_NLS_LIBS to link with.

Fixes:
    http://autobuild.buildroot.org/results/f0d/f0d85d76786343d767fba9c7c5c01f042ecfc018/
    [...]

[1] mesonbuild/meson#3740

Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
Cc: Adam Duskett <aduskett@gmail.com>
Cc: Fabrice Fontaine <fontaine.fabrice@gmail.com>
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
fpoussin pushed a commit to lapwise/buildroot that referenced this issue Jul 3, 2019
libglib2 uses a very crude and error-prone way to detect the intl
functions, which basically fails when the C library is not glibc.

There is a bug report about this in upstream meson [1], but it doesn't
seem to get any progress. Fixing that properly in Buildroot looks
complicated.

Now that a meson package can specify its LDFLAGS, use that to pass the
infrastructure-provided TARGET_NLS_LIBS to link with.

Fixes:
    http://autobuild.buildroot.org/results/f0d/f0d85d76786343d767fba9c7c5c01f042ecfc018/
    [...]

[1] mesonbuild/meson#3740

Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
Cc: Adam Duskett <aduskett@gmail.com>
Cc: Fabrice Fontaine <fontaine.fabrice@gmail.com>
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
@etrunko
Copy link
Contributor

etrunko commented Jan 30, 2020

I'm getting this error with fedora rawhide (32), the check for hypot() function returns YES, while the link fails due to lack of -lm.

https://kojipkgs.fedoraproject.org//work/tasks/2934/41262934/build.log

Funny thing tough, it works with Fedora 31.

https://kojipkgs.fedoraproject.org//work/tasks/3519/41263519/build.log

I added a cat meson-logs/meson-log.txt to the spec file so to print the contents of that file after the configure time.

@freddy77
Copy link

freddy77 commented Jan 31, 2020

Looking at above log

 #include <math.h>
        int main() {
        #ifdef __has_builtin
            #if !__has_builtin(__builtin_hypot)
                #error "__builtin_hypot not found"
            #endif
        #elif ! defined(hypot)
            /* Check for __builtin_hypot only if no includes were added to the
             * prefix above, which means no definition of hypot can be found.
             * We would always check for this, but we get false positives on
             * MSYS2 if we do. Their toolchain is broken, but we can at least
             * give them a workaround. */
            #if 0
                __builtin_hypot;
            #else
                #error "No definition for __builtin_hypot found in the prefix"
            #endif
        #endif
        return 0;
        }

but there's no check if the code used is really able to use that built-in, in many cases if a replacement function is defined the compiler will use that function, not the built-in causing compiler/linking failures.
Note also that the code above will succeed if there's just a preprocessor definition, for instance a code like

compiler.has_function('foo', prefix : '#define foo xxx')

will succeed.

Autoconf code uses something like

/* 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 hypot ();
int
main ()
{
return hypot ();
  ;
  return 0;
}

that although produces a warning does a full check of compile + link.

@eli-schwartz
Copy link
Member

Is there a good reason to use prefix: ... here? I assume it will fail to be detected as available in the stdlib if you don't add the header... this should be and cc.has_header('libintl.h') if the goal is actually to find out if the development header is installed too.

Aside: meson recently grew the dependency('intl') custom builtin dependency specifically for this example.

@ePirat
Copy link
Contributor

ePirat commented Aug 31, 2021

For cross-platform proper results it is essential to include the right header when checking for the presence of the function, as the header might contain essential annotations for the function declaration used by the compiler and linker that decide if compiling and linking will succeed.

One example of this is macOS, which has special availability annotations, so when targeting older macOS versions that lack specific functions, those will become weakly linked (and with the right flags, compilation and/or linking will fail completely) which avoid accidental use of such functions when compiling for older macOS versions with code that is not prepared to deal with it (which probably applies to most projects using meson and explicitly checking if a function exists at build time).

Another big can of worms is introduced by the fact that mesons cc.has_function checks for builtins too. IMHO meson should have a new check_function where the header(s) should be a mandatory argument that only works for functions and a separate has_builtin for the small number of cases where a function might be implemented by a builtin only (like IIRC alloc on some systems). This would reduce implementation complexity, make testing easier and prevent unexpected outcomes (like current with has_function on macOS/iOS when not specifying the header in prefix:).

@eli-schwartz
Copy link
Member

Meanwhile I forgot how broken this is, rediscovered it in #9642, and then afterward came across this issue again.

Use case there was for libiconv/iconv_open, not libintl/ngettext, but the same basic issue... I ended up just writing a custom test file snippet for the check, because I can't trust the one has_function provides.

This is tolerable for meson's builtin dependencies, it is write-once-solve-everywhere. Less so for cc.has_function when used directly in project meson.build files...

@eli-schwartz
Copy link
Member

As I mentioned when originally rediscovering the problem here:
#9632 (comment)

As far as I can tell, on compilers without __has_builtin this goes straight to "is the function defined in the header? Okay, immediately return success". So if you include a header in a has_function check it always falls back to successfully answering that it's a valid builtin.

And as I was told on IRC, mingw toolchains don't have a version of GCC that is recent enough to support __has_builtin, TIL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants