[libcxx] Undefine all supported C math functions

[petrhosek]

The C standard allows these to be defined as macros which is problem when providing the C++ versions and so we need to undefine the C macros first. Rather than undefining these selectively, we should undefine all of them since we have any guarantees which are going to be defined as macros and it depeneds on the target platform.

We don't need to guard these with #ifdef since undefining a symbol that's not defined is not an error.

This is related to issue #84879.

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Petr Hosek (petrhosek)

Changes

The C standard allows these to be defined as macros which is problem when providing the C++ versions and so we need to undefine the C macros first. Rather than undefining these selectively, we should undefine all of them since we have any guarantees which are going to be defined as macros and it depeneds on the target platform.

We don't need to guard these with #ifdef since undefining a symbol that's not defined is not an error.

This is related to issue #84879.

---

Full diff: https://github.com/llvm/llvm-project/pull/94533.diff

2 Files Affected:

• (modified) libcxx/include/math.h (+68-47)
• (modified) libcxx/include/stdlib.h (+6-18)

diff --git a/libcxx/include/math.h b/libcxx/include/math.h
index 4e6304a753984..893dca8dc1c17 100644
--- a/libcxx/include/math.h
+++ b/libcxx/include/math.h
@@ -307,53 +307,74 @@ long double    truncl(long double x);
 // back to C++ linkage before including these C++ headers.
 extern "C++" {
 
-#    ifdef fpclassify
-#      undef fpclassify
-#    endif
-
-#    ifdef signbit
-#      undef signbit
-#    endif
-
-#    ifdef isfinite
-#      undef isfinite
-#    endif
-
-#    ifdef isinf
-#      undef isinf
-#    endif
-
-#    ifdef isnan
-#      undef isnan
-#    endif
-
-#    ifdef isnormal
-#      undef isnormal
-#    endif
-
-#    ifdef isgreater
-#      undef isgreater
-#    endif
-
-#    ifdef isgreaterequal
-#      undef isgreaterequal
-#    endif
-
-#    ifdef isless
-#      undef isless
-#    endif
-
-#    ifdef islessequal
-#      undef islessequal
-#    endif
-
-#    ifdef islessgreater
-#      undef islessgreater
-#    endif
-
-#    ifdef isunordered
-#      undef isunordered
-#    endif
+#    undef acos
+#    undef acosh
+#    undef asin
+#    undef asinh
+#    undef atan
+#    undef atan2
+#    undef atanh
+#    undef cbrt
+#    undef ceil
+#    undef copysign
+#    undef cos
+#    undef cosh
+#    undef erf
+#    undef erfc
+#    undef exp
+#    undef exp2
+#    undef expm1
+#    undef fabs
+#    undef fdim
+#    undef floor
+#    undef fma
+#    undef fmax
+#    undef fmin
+#    undef fmod
+#    undef fpclassify
+#    undef frexp
+#    undef hypot
+#    undef ilogb
+#    undef isfinite
+#    undef isgreater
+#    undef isgreaterequal
+#    undef isinf
+#    undef isless
+#    undef islessequal
+#    undef islessgreater
+#    undef isnan
+#    undef isnormal
+#    undef isunordered
+#    undef ldexp
+#    undef lgamma
+#    undef llrint
+#    undef llround
+#    undef log
+#    undef log10
+#    undef log1p
+#    undef log2
+#    undef logb
+#    undef lrint
+#    undef lround
+#    undef modf
+#    undef nearbyint
+#    undef nextafter
+#    undef nexttoward
+#    undef pow
+#    undef remainder
+#    undef remquo
+#    undef rint
+#    undef round
+#    undef scalbln
+#    undef scalbn
+#    undef signbit
+#    undef sin
+#    undef sinh
+#    undef sqrt
+#    undef tan
+#    undef tanh
+#    undef tgamma
+#    undef trunc
 
 #    include <__math/abs.h>
 #    include <__math/copysign.h>
diff --git a/libcxx/include/stdlib.h b/libcxx/include/stdlib.h
index a74344d49150c..b1151b58a356f 100644
--- a/libcxx/include/stdlib.h
+++ b/libcxx/include/stdlib.h
@@ -98,15 +98,9 @@ void *aligned_alloc(size_t alignment, size_t size);                       // C11
 extern "C++" {
 // abs
 
-#    ifdef abs
-#      undef abs
-#    endif
-#    ifdef labs
-#      undef labs
-#    endif
-#    ifdef llabs
-#      undef llabs
-#    endif
+#    undef abs
+#    undef labs
+#    undef llabs
 
 // MSVCRT already has the correct prototype in <stdlib.h> if __cplusplus is defined
 #    if !defined(_LIBCPP_MSVCRT)
@@ -128,15 +122,9 @@ _LIBCPP_NODISCARD inline _LIBCPP_HIDE_FROM_ABI long double abs(long double __lcp
 
 // div
 
-#    ifdef div
-#      undef div
-#    endif
-#    ifdef ldiv
-#      undef ldiv
-#    endif
-#    ifdef lldiv
-#      undef lldiv
-#    endif
+#    undef div
+#    undef ldiv
+#    undef lldiv
 
 // MSVCRT already has the correct prototype in <stdlib.h> if __cplusplus is defined
 #    if !defined(_LIBCPP_MSVCRT)

[PiJoules]

Looking at isnan as an example, it looks like isnan was a macro that resolved to just __builtin_isnan from /home/runner/_work/llvm-project/llvm-project/build/generic-cxx03/include/c++/v1/__math/traits.h, hence

/home/runner/_work/llvm-project/llvm-project/build/generic-cxx03/include/c++/v1/cmath:582:10: error: no member named '__builtin_isnan' in namespace 'std'; did you mean simply '__builtin_isnan'?
  582 |   return std::isnan(__lcpp_x);
      |          ^~~~~
/home/runner/_work/llvm-project/llvm-project/build/generic-cxx03/include/c++/v1/__math/traits.h:97:10: note: '__builtin_isnan' declared here
   97 |   return __builtin_isnan(__x);
      |          ^

Locally, mine is defined as

#  define isnan(x) \
     (sizeof (x) == sizeof (float) ? __isnanf (x) : __isnan (x))

So the macros are still being used it seems.

[PiJoules]

So the #undef isnan path in libcxx/include/__math/traits.h is definitely taken. I think perhaps the macro is redefined elsewhere by including the system math.h somewhere.

[philnik777]

This breaks if one of the __math/ headers is included before math.h is included. I also don't understand what the failure mode is here. These macros should always be #undefed right after they are defined.

[petrhosek]

We trying to use libc++ with FreeRTOS which defines some of the math.h standard C library functions as macros which is allowed by the C standard but it breaks libc++ headers. libc++ should undefine the macro before providing the symbol of the same name (just like it already does for some of them).

[philnik777]

We trying to use libc++ with FreeRTOS which defines some of the math.h standard C library functions as macros which is allowed by the C standard but it breaks libc++ headers. libc++ should undefine the macro before providing the symbol of the same name (just like it already does for some of them).

Are you sure that the C standard allows this? The ones we #undefine (fpclassify, signbit, isfinite and friends) are specified as macros in the standard, while acos, atan etc. are specified as functions.

[petrhosek]

We trying to use libc++ with FreeRTOS which defines some of the math.h standard C library functions as macros which is allowed by the C standard but it breaks libc++ headers. libc++ should undefine the macro before providing the symbol of the same name (just like it already does for some of them).

Are you sure that the C standard allows this? The ones we #undefine (fpclassify, signbit, isfinite and friends) are specified as macros in the standard, while acos, atan etc. are specified as functions.

Yes, this is explicitly mentioned in the section 7.1.4 Use of library functions of the C standard:

Any function declared in a header may be additionally implemented as a function-like macro defined in the header, so if a library function is declared explicitly when its header is included, one of the techniques shown below can be used to ensure the declaration is not affected by such a macro. Any macro definition of a function can be suppressed locally by enclosing the name of the function in parentheses, because the name is then not followed by the left parenthesis that indicates expansion of a macro function name. For the same syntactic reason, it is permitted to take the address of a library function even if it is also defined as a macro.190). The use of #undef to remove any macro definition will also ensure that an actual function is referred to.

[philnik777]

Interesting. But wouldn't that mean that we'd technically also have to #undef memcpy and every other C function that exists? That seems a bit excessive to me.

[PiJoules]

Just as philnik777 points out, it looks like that's what's happening here. For isnan, it's first undefd via an include of __math/traits.h here

In file included from /usr/local/google/home/leonardchan/llvm-projects/my-llvm-project-2/libcxx/src/bind.cpp:9:
In file included from /usr/local/google/home/leonardchan/llvm-projects/my-llvm-project-2-build/include/c++/v1/functional:535:
In file included from /usr/local/google/home/leonardchan/llvm-projects/my-llvm-project-2-build/include/c++/v1/__functional/bind.h:21:
In file included from /usr/local/google/home/leonardchan/llvm-projects/my-llvm-project-2-build/include/c++/v1/tuple:261:
In file included from /usr/local/google/home/leonardchan/llvm-projects/my-llvm-project-2-build/include/c++/v1/compare:147:
In file included from /usr/local/google/home/leonardchan/llvm-projects/my-llvm-project-2-build/include/c++/v1/__compare/compare_partial_order_fallback.h:13:
In file included from /usr/local/google/home/leonardchan/llvm-projects/my-llvm-project-2-build/include/c++/v1/__compare/partial_order.h:14:
In file included from /usr/local/google/home/leonardchan/llvm-projects/my-llvm-project-2-build/include/c++/v1/__compare/weak_order.h:14:
In file included from /usr/local/google/home/leonardchan/llvm-projects/my-llvm-project-2-build/include/c++/v1/__compare/strong_order.h:17:
/usr/local/google/home/leonardchan/llvm-projects/my-llvm-project-2-build/include/c++/v1/__math/traits.h:95:2: warning: "undef isnan" [-W#warnings]

then we include math.h afterwards which would redefine it:

In file included from /usr/local/google/home/leonardchan/llvm-projects/my-llvm-project-2/libcxx/src/bind.cpp:9:
In file included from /usr/local/google/home/leonardchan/llvm-projects/my-llvm-project-2-build/include/c++/v1/functional:540:
In file included from /usr/local/google/home/leonardchan/llvm-projects/my-llvm-project-2-build/include/c++/v1/__functional/boyer_moore_searcher.h:26:
In file included from /usr/local/google/home/leonardchan/llvm-projects/my-llvm-project-2-build/include/c++/v1/unordered_map:591:
In file included from /usr/local/google/home/leonardchan/llvm-projects/my-llvm-project-2-build/include/c++/v1/__hash_table:43:
In file included from /usr/local/google/home/leonardchan/llvm-projects/my-llvm-project-2-build/include/c++/v1/cmath:318:
In file included from /usr/local/google/home/leonardchan/llvm-projects/my-llvm-project-2-build/include/c++/v1/math.h:301:
/usr/local/google/home/leonardchan/misc/linux-sysroot/usr/include/math.h:259:2: warning: "included math.h" [-W#warnings]
  259 | #warning "included math.h"
      |  ^

[petrhosek]

I have reverted to the earlier implementation which shouldn't have this issue and include a comment with explanation for why this is needed.

Interesting. But wouldn't that mean that we'd technically also have to #undef memcpy and every other C function that exists? That seems a bit excessive to me.

I believe we only need to do this for functions where we provide our own definition in libc++.

[philnik777]

I have reverted to the earlier implementation which shouldn't have this issue and include a comment with explanation for why this is needed.

Interesting. But wouldn't that mean that we'd technically also have to #undef memcpy and every other C function that exists? That seems a bit excessive to me.

I believe we only need to do this for functions where we provide our own definition in libc++.

In C++ users are free to write

namespace my_ns {
  void memcpy() {}
}

though, which would break if memcpy was something like #define memcpy(dest, src, size) __memcpy(dest, src, size).

[ldionne]

I think I agree with @philnik777 's comments about this being needed for every C library function.

Basically, we have to choose whether libc++ works on top of a conforming C Standard Library per the C Standard, or whether we require C++ friendliness in a C Standard Library for it to be usable with libc++.

If we support an arbitrary conforming C Standard Library, then per 7.1.4 it does seem like they are allowed to define functions as macros, which is extremely unfriendly to C++ due to namespaces amongst other things. I've come across this issue (with memcpy for example) several times when porting libc++ to new platforms.

So far, what I've always done was to ask the underlying C library to stop using macros because that is C++-unfriendly. However, it might be reasonable for libc++ to support that.

Doing this correctly in the general case is going to be non-trivial, though, since it basically means that we can't blindly reuse names from the C Standard Library anymore. For example, strxfrm isn't defined anywhere by libc++, we just reuse it from the C stdlib:

// <cstring>
namespace std {
  using ::strxfrm;
}

If we wanted to work in spite of §7.1.4, we'd basically have to #undef strxfrm and then define it ourselves as a function. I believe the logical conclusion here is that we basically can't reuse anything from the C Standard Library, we'd have to implement everything ourselves.

This makes me wonder whether that is a viable approach. Thoughts?

[ldionne]

IMO it would make more sense to undefine each macro where the corresponding libc++ function is implemented. For example in

#undef cosh
inline _LIBCPP_HIDE_FROM_ABI float cosh(float __x) _NOEXCEPT { return __builtin_coshf(__x); }
// etc...

in libcxx/include/__math/hyperbolic_functions.h.

[petrhosek]

I tried that in 745745c but that lead to build errors, see #94533 (comment). I noticed that the particular <cmath> include that triggered that build error was removed by @philnik777 in 0dd8c0d so it might worth trying it again and see if there are any remaining issues.

[ldionne]

I read that comment, but I still don't understand what's happening that would prevent the #undef from being located right before the definition of the libc++ function with the same name. It seems like we need exactly one canonical place where we #undef foo and then define namespace std { int foo(); }, and I don't see how this can lead to problems?

[philnik777]

@ldionne Consider

#include <__math/traits.h>
#include <math.h>

The __math/traits.h would try to #undef a macro that will only be defined when including math.h. Does that make it clearer?

[philnik777]

@petrhosek Do you plan to pursue this?

[frobtech]

I think I agree with @philnik777 's comments about this being needed for every C library function.

Basically, we have to choose whether libc++ works on top of a conforming C Standard Library per the C Standard, or whether we require C++ friendliness in a C Standard Library for it to be usable with libc++.

If we support an arbitrary conforming C Standard Library, then per 7.1.4 it does seem like they are allowed to define functions as macros, which is extremely unfriendly to C++ due to namespaces amongst other things. I've come across this issue (with memcpy for example) several times when porting libc++ to new platforms.

So far, what I've always done was to ask the underlying C library to stop using macros because that is C++-unfriendly. However, it might be reasonable for libc++ to support that.

Doing this correctly in the general case is going to be non-trivial, though, since it basically means that we can't blindly reuse names from the C Standard Library anymore. For example, strxfrm isn't defined anywhere by libc++, we just reuse it from the C stdlib:

// <cstring>
namespace std {
  using ::strxfrm;
}

If we wanted to work in spite of §7.1.4, we'd basically have to #undef strxfrm and then define it ourselves as a function. I believe the logical conclusion here is that we basically can't reuse anything from the C Standard Library, we'd have to implement everything ourselves.

This makes me wonder whether that is a viable approach. Thoughts?

You're right that you need #undef strxfrm. You're not right that this means you need to define it yourself. The requirement of the C standard is that any standard library function may also be defined as a macro. It's also valid to #undef strxfrm and then call strxfrm. There has to be a proper extern function declaration before the #define in the standard C header. The C standard allows any function to have a macro defined. It specifies precisely which APIs are macros per se and thus both must be macros (users can use #ifdef as a conformance check on the implementation) and must be used as macros (#undef by a user is a violation of the standard). The "may be defined as macro" rule applies to all standard library functions that are specified as functions: it must be the case that using that macro has the semantics the standard specifies for that function, and it must also be the case that #undef foo (or (foo)(args) or other means of avoiding using the macro) does not change the semantics of using that function.

So it's entirely sufficient and entirely conforms with the C standard to write #undef foo before each and every use of foo in a libc++ header. That might not be the most fun way to maintain things, but it would be perfectly fine semantically and avoids error-prone complexities of getting a single #undef foo in exactly after #include <foo.h> (for some standard C function foo specified to be declared in standard C header <foo.h>).

[ldionne]

You're right that you need #undef strxfrm. You're not right that this means you need to define it yourself. The requirement of the C standard is that any standard library function may also be defined as a macro. It's also valid to #undef strxfrm and then call strxfrm.

Ah ah! Thanks a lot for providing this insight, that was missing from my understanding. A lot of things make more sense now. In that case, I believe the right path forward would be to #undef foo before we introduce a function named foo, and to do that systematically. I don't think that should be too controversial given this new understanding.

[philnik777]

You're right that you need #undef strxfrm. You're not right that this means you need to define it yourself. The requirement of the C standard is that any standard library function may also be defined as a macro. It's also valid to #undef strxfrm and then call strxfrm.

Ah ah! Thanks a lot for providing this insight, that was missing from my understanding. A lot of things make more sense now. In that case, I believe the right path forward would be to #undef foo before we introduce a function named foo, and to do that systematically. I don't think that should be too controversial given this new understanding.

We'd also have to #undef every other libc function, since these names are allowed to be used in namespaces other than the global one.

[philnik777]

@petrhosek Do you plan to pursue this again?