has_function: Improve builtin detection #3675

nirbheek · 2018-06-03T09:53:50Z

has_function: Improve built-in detection with headers
- __has_builtin(foo) is more correct and more reliable than __has_builtin(__builtin_foo)
- If detection of built-ins with headers fails in the first check because we can't take the address of a built-in, we can do an ordinary check.

We can't always do this ordinary check because clang on macOS ld removes the symbol when using -Wl,-no_weak_imports, which is needed for being able to target the correct platform with XCode 8 and newer.

Closes #3672

has_function: Fix checking for __builtin_foo functions

Some functions are meant to be used only as compiler builtins, such as __builtin_smul_overflow and friends.

Detect this case and don't look for __builtin_foo because such a check will never include a header.

Fixes #3676

textshell · 2018-06-03T11:45:38Z

I think this wrongly attributes the problem to MinGW when this just reveals less than universally true a assumptions in meson. The current code also fails on linux where gcc is one of the two native compilers in #3676.

Meson should not assume a fixed relationship between foo and __builtin_foo. If there is a relationship is has to come from a system header.

This of course is breaking for projects where the build file failed to specify the needed header. We have to solve that in some way before we can do the proper fix.

nirbheek · 2018-06-03T11:55:59Z

I think this wrongly attributes the problem to MinGW

The current commits do not do this.

The current code also fails on linux where gcc is one of the two native compilers in #3676.

Now contains a fix for that issue + a test with a note about gcc being broken.

This of course is breaking for projects where the build file failed to specify the needed header. We have to solve that in some way before we can do the proper fix.

Do tell us if you find a way to fix this. We have no choice but to do this for now.

textshell · 2018-06-03T12:04:11Z

I think for compatibility we need to deprecate this implicit assumption. Ideally meson would print a warning, but work as previously for a few releases. One way to do this would be to collect a list of affected functions and their needed includes and just implicitly add that include for the deprecation period.

From a grep of my include directory it seems like these functions might be affected.
(Using egrep -r '#.*__builtin_' /usr/include/ on a debian stretch, but with newlib and diet libc headers also installed)

fpclassify isfinite isnormal isgreater isgreaterequal isless islessequal islessgreater isunordered alloca isnan va_start va_end va_arg offsetof strncpy strncat strcspn strspn strpbrk

It also reveals that the naming relation is often a bit different so the old code already does not work reliable, e.g.:

#define    __bswap16(_x)   __builtin_bswap16(_x)
#define isinf(__x) (__builtin_isinf_sign (__x))
#define __unreachable() __builtin_unreachable()

That list seems manageable although it is a bit long.

textshell · 2018-06-03T12:05:49Z

mesonbuild/compilers/c.py

@@ -678,7 +678,8 @@ def has_function(self, funcname, prefix, env, extra_args=None, dependencies=None
            head, main = self._no_prototype_templ()
        templ = head + stubs_fail + main

-        if self.links(templ.format(**fargs), env, extra_args, dependencies):
+        if not funcname.startswith('__builtin_') and \


I do not think meson should have this assumption, why is this needed? Optimization?

Yes, that check will always fail with builtins because it tries to take the address of the function.

Which is fine, it just fails and meson goes on to the builtin test.
But what happens if there is a function that matches that pattern that is not a builtin? I think hardcoding that naming convention is just assuming things that might not be true. So i would always let that test fail. And if in some odd environment a function with such an name is an actual external function it will not break just because of naming.

textshell · 2018-06-03T12:06:33Z

mesonbuild/compilers/c.py

        t = '''{prefix}
        int main() {{
        #ifdef __has_builtin
-            #if !__has_builtin(__builtin_{func})
+            #if !__has_builtin({func})


👍

But does this work without also adding implicit includes for the cases where users currently got away with not giving the includes?

Yes, the alloca test checks for this already (on clang).

Yes, it seems to work with alloca, but with isgreaterequal it does not. Now it might be my clang install (it's just for compiling qtcreator). But a simple manual build test with clang seems to work. But even has_function('isgreaterequal', prefix: '#include <math.h>') does not work for me at the moment. It works with a random older meson.

textshell · 2018-06-03T12:10:38Z

mesonbuild/compilers/c.py

                __builtin_{func};
+            #elif defined(__stub_{func}) || defined(_stub__{func})


Do we need a comment similar to the one near stubs_fail to make it easier to understand.
Do we need to make sure that limits.h is included? Because the comment above implies it is needed for this to work.

textshell · 2018-06-03T12:14:42Z

test cases/common/43 has function/meson.build

             prefix : '#include <alloca.h>\n#undef alloca',
             args : unit_test_args),
-             'built-in alloca must not be found with #include and #undef')
+             'built-in alloca must be found with #include and #undef')


How does this work? It seems like it should not?

The test was added to ensure that the __builtin_foo codepath is not hit when there is a #include in prefix because there was a #error in the other path. That other path is gone now, and we have a fallback check for foo there now.

alloca may not be a #define now, but it is still available as a prototype. See: alloca.h.

textshell · 2018-06-03T12:15:51Z

test cases/common/43 has function/meson.build

+  if ['gcc', 'clang'].contains(cc.get_id())
+    assert(cc.has_function('__builtin_smul_overflow'), '__builtin_smul_overflow must be found')
+    # FIXME: this fails on gcc, and we can't fix it without breaking builtin
+    # detection without headers for things like alloca()


I think with the implicit include workaround we can enable this test for gcc as well.

textshell · 2018-06-03T12:18:05Z

mesonbuild/compilers/c.py

@@ -704,10 +706,12 @@ def has_function(self, funcname, prefix, env, extra_args=None, dependencies=None
             * 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 {no_includes:d}
+            #if {no_includes:d} && !{func_is_builtin:d}


I think we should try if the idea with implicit includes allows us to avoid this.

I think we should do that in a separate PR since it has a large potential for a yak-shave.

If this is the only place where we would need that, i'm ok with splitting that part of into a separate PR.

* __has_builtin(foo) is more correct and more reliable than __has_builtin(__builtin_foo) * If detection of built-ins with headers fails in the first check because we can't take the address of a built-in, we can do an ordinary check. We can't always do this ordinary check because clang on macOS ld removes the symbol when using -Wl,-no_weak_imports, which is needed for being able to target the correct platform with XCode 8 and newer. Closes #3672

Some functions are meant to be used only as compiler builtins, such as __builtin_smul_overflow and friends. Detect this case and don't look for __builtin_foo because such a check will never include a header. Fixes #3676

Who knows maybe people are dumb and use __builtin_ as a prefix to their own functions

@textshell

This allows us to also accept functions that are simple pre-processor defines in the header pointing to an inline definition, built-in implementation, etc. Also add a test for all the builtins listed by @textshell here: #3675 (comment)

textshell · 2018-06-09T15:02:21Z

Sorry, not yet gotten to review the current version.

But from discussing with @ePirat and from @ePirat's testing i stumbled over this "nice" loophole with builtins:

https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html says:

With the exception of built-ins that have library equivalents such as the standard C library functions discussed below, or that expand to library calls, GCC built-in functions are always expanded inline and thus do not have corresponding entry points and their address cannot be obtained. Attempting to use them in an expression other than a function call results in a compile-time error.

GCC includes built-in versions of many of the functions in the standard C library. These functions come in two forms: one whose names start with the _builtin prefix, and the other without. Both forms have the same type (including prototype), the same address (when their address is taken), and the same meaning as the C library functions even if you specify the -fno-builtin option see C Dialect Options). Many of these functions are only optimized in certain cases; if they are not optimized in a particular case, a call to the library function is emitted.

And then goes on to list a ton of functions that are special in this way. So all functions from the list are builtins and still need their implementation to be available for linking. So this basically makes this a lot harder to do right.

But i think passing -fno-builtin to the test compilation would help. Btw, that option name is a bit misleading, as it's documented to only disable builtins that do not start with __builtin. But i would expect it to also not disable the "mandatory" builtins that don't have a non builtin version, but that would need checking.

Testing by @ePirat suggests that this would for nstrdup as well as the cases covered by the existing tests.

nirbheek · 2018-07-02T13:55:56Z

I didn't get enough time to look into this, and it's too dangerous to change this now because it might regress existing users. Will work on this after the 0.47 release which will happen today.

jpakkane · 2018-08-29T19:24:08Z

⚔️ conflicts.

nirbheek · 2018-08-29T22:54:03Z

This is WIP, not ready to merge yet. I have to add the 'loophole' that @textshell pointed out above.

ePirat · 2018-09-16T10:42:08Z

#3902 is somewhat related

nirbheek · 2020-07-12T08:53:23Z

Fixed by #7116.

nirbheek force-pushed the nirbheek/mingw-has_function-quirks branch from fa2a067 to 5bebeaa Compare June 3, 2018 11:40

textshell reviewed Jun 3, 2018

View reviewed changes

nirbheek added 3 commits June 3, 2018 18:40

has_function: Fix checking for __builtin_foo functions

9457947

Some functions are meant to be used only as compiler builtins, such as __builtin_smul_overflow and friends. Detect this case and don't look for __builtin_foo because such a check will never include a header. Fixes #3676

has_function: Add tests for the previous two commits

ce3121b

nirbheek force-pushed the nirbheek/mingw-has_function-quirks branch from 5bebeaa to ce3121b Compare June 3, 2018 13:10

nirbheek changed the title ~~has_function: Don't use builtin detection at all on MinGW~~ has_function: Improve builtin detection Jun 3, 2018

nirbheek added 2 commits June 3, 2018 18:43

has_function: Don't short-circuit __builtin_

ca1558a

Who knows maybe people are dumb and use __builtin_ as a prefix to their own functions

nirbheek self-assigned this Jun 3, 2018

ePirat mentioned this pull request Jun 14, 2018

False positive in has_function() #3740

Open

nirbheek added this to the meson-next milestone Jun 18, 2018

nirbheek mentioned this pull request Jun 18, 2018

has_function fails to find strcpy_s on mingw #2605

Open

nirbheek modified the milestones: meson-next, 0.48.0 Jul 7, 2018

nirbheek added the WIP label Aug 29, 2018

nirbheek modified the milestones: 0.48.0, 0.49.0 Sep 20, 2018

nirbheek closed this Jul 12, 2020

nirbheek deleted the nirbheek/mingw-has_function-quirks branch July 12, 2020 08:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

has_function: Improve builtin detection #3675

has_function: Improve builtin detection #3675

nirbheek commented Jun 3, 2018 •

edited

textshell commented Jun 3, 2018

nirbheek commented Jun 3, 2018

textshell commented Jun 3, 2018 •

edited by nirbheek

textshell Jun 3, 2018 •

edited

nirbheek Jun 3, 2018

textshell Jun 3, 2018

textshell Jun 3, 2018

nirbheek Jun 3, 2018

textshell Jun 3, 2018

textshell Jun 3, 2018

nirbheek Jun 3, 2018

textshell Jun 3, 2018

nirbheek Jun 3, 2018 •

edited

textshell Jun 3, 2018

textshell Jun 3, 2018

nirbheek Jun 3, 2018

textshell Jun 3, 2018

textshell commented Jun 9, 2018

nirbheek commented Jul 2, 2018

jpakkane commented Aug 29, 2018

nirbheek commented Aug 29, 2018

ePirat commented Sep 16, 2018

nirbheek commented Jul 12, 2020

		__builtin_{func};
		#elif defined(__stub_{func}) \|\| defined(_stub__{func})

has_function: Improve builtin detection #3675

has_function: Improve builtin detection #3675

Conversation

nirbheek commented Jun 3, 2018 • edited

textshell commented Jun 3, 2018

nirbheek commented Jun 3, 2018

textshell commented Jun 3, 2018 • edited by nirbheek

textshell Jun 3, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nirbheek Jun 3, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

textshell commented Jun 9, 2018

nirbheek commented Jul 2, 2018

jpakkane commented Aug 29, 2018

nirbheek commented Aug 29, 2018

ePirat commented Sep 16, 2018

nirbheek commented Jul 12, 2020

nirbheek commented Jun 3, 2018 •

edited

textshell commented Jun 3, 2018 •

edited by nirbheek

textshell Jun 3, 2018 •

edited

nirbheek Jun 3, 2018 •

edited