[clang] 92a9d48 - [clang][headers] Including stddef.h always redefines NULL (#99727)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Jul 23 13:03:03 PDT 2024
Author: Ian Anderson
Date: 2024-07-23T13:02:59-07:00
New Revision: 92a9d4831d5e40c286247c30fcd794563adbef6e
URL: https://github.com/llvm/llvm-project/commit/92a9d4831d5e40c286247c30fcd794563adbef6e
DIFF: https://github.com/llvm/llvm-project/commit/92a9d4831d5e40c286247c30fcd794563adbef6e.diff
LOG: [clang][headers] Including stddef.h always redefines NULL (#99727)
stddef.h always includes __stddef_null.h. This is fine in modules
because it's not possible to re-include the pcm, and it's necessary to
export the _Builtin_stddef.null submodule. However, without modules it
causes NULL to always get redefined which disrupts some C++ code. Rework
the inclusion of __stddef_null.h so that with not building with modules
it's only included if __need_NULL is set by the includer, or it's the
first time stddef.h is being included.
Added:
clang/test/Modules/stddef.cpp
Modified:
clang/lib/Headers/stdarg.h
clang/lib/Headers/stddef.h
clang/test/Headers/stddefneeds.cpp
Removed:
################################################################################
diff --git a/clang/lib/Headers/stdarg.h b/clang/lib/Headers/stdarg.h
index 8292ab907becf..6203d7a600a23 100644
--- a/clang/lib/Headers/stdarg.h
+++ b/clang/lib/Headers/stdarg.h
@@ -20,19 +20,18 @@
* modules.
*/
#if defined(__MVS__) && __has_include_next(<stdarg.h>)
-#include <__stdarg_header_macro.h>
#undef __need___va_list
#undef __need_va_list
#undef __need_va_arg
#undef __need___va_copy
#undef __need_va_copy
+#include <__stdarg_header_macro.h>
#include_next <stdarg.h>
#else
#if !defined(__need___va_list) && !defined(__need_va_list) && \
!defined(__need_va_arg) && !defined(__need___va_copy) && \
!defined(__need_va_copy)
-#include <__stdarg_header_macro.h>
#define __need___va_list
#define __need_va_list
#define __need_va_arg
@@ -45,6 +44,7 @@
!defined(__STRICT_ANSI__)
#define __need_va_copy
#endif
+#include <__stdarg_header_macro.h>
#endif
#ifdef __need___va_list
diff --git a/clang/lib/Headers/stddef.h b/clang/lib/Headers/stddef.h
index 8985c526e8fc5..99b275aebf5aa 100644
--- a/clang/lib/Headers/stddef.h
+++ b/clang/lib/Headers/stddef.h
@@ -20,7 +20,6 @@
* modules.
*/
#if defined(__MVS__) && __has_include_next(<stddef.h>)
-#include <__stddef_header_macro.h>
#undef __need_ptr
diff _t
#undef __need_size_t
#undef __need_rsize_t
@@ -31,6 +30,7 @@
#undef __need_max_align_t
#undef __need_offsetof
#undef __need_wint_t
+#include <__stddef_header_macro.h>
#include_next <stddef.h>
#else
@@ -40,7 +40,6 @@
!defined(__need_NULL) && !defined(__need_nullptr_t) && \
!defined(__need_unreachable) && !defined(__need_max_align_t) && \
!defined(__need_offsetof) && !defined(__need_wint_t)
-#include <__stddef_header_macro.h>
#define __need_ptr
diff _t
#define __need_size_t
/* ISO9899:2011 7.20 (C11 Annex K): Define rsize_t if __STDC_WANT_LIB_EXT1__ is
@@ -49,7 +48,24 @@
#define __need_rsize_t
#endif
#define __need_wchar_t
+#if !defined(__STDDEF_H) || __has_feature(modules)
+/*
+ * __stddef_null.h is special when building without modules: if __need_NULL is
+ * set, then it will unconditionally redefine NULL. To avoid stepping on client
+ * definitions of NULL, __need_NULL should only be set the first time this
+ * header is included, that is when __STDDEF_H is not defined. However, when
+ * building with modules, this header is a textual header and needs to
+ * unconditionally include __stdef_null.h to support multiple submodules
+ * exporting _Builtin_stddef.null. Take module SM with submodules A and B, whose
+ * headers both include stddef.h When SM.A builds, __STDDEF_H will be defined.
+ * When SM.B builds, the definition from SM.A will leak when building without
+ * local submodule visibility. stddef.h wouldn't include __stddef_null.h, and
+ * SM.B wouldn't import _Builtin_stddef.null, and SM.B's `export *` wouldn't
+ * export NULL as expected. When building with modules, always include
+ * __stddef_null.h so that everything works as expected.
+ */
#define __need_NULL
+#endif
#if (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 202311L) || \
defined(__cplusplus)
#define __need_nullptr_t
@@ -65,6 +81,7 @@
/* wint_t is provided by <wchar.h> and not <stddef.h>. It's here
* for compatibility, but must be explicitly requested. Therefore
* __need_wint_t is intentionally not defined here. */
+#include <__stddef_header_macro.h>
#endif
#if defined(__need_ptr
diff _t)
diff --git a/clang/test/Headers/stddefneeds.cpp b/clang/test/Headers/stddefneeds.cpp
index 0763bbdee13ae..0282e8afa600d 100644
--- a/clang/test/Headers/stddefneeds.cpp
+++ b/clang/test/Headers/stddefneeds.cpp
@@ -56,14 +56,21 @@ max_align_t m5;
#undef NULL
#define NULL 0
-// glibc (and other) headers then define __need_NULL and rely on stddef.h
-// to redefine NULL to the correct value again.
-#define __need_NULL
+// Including stddef.h again shouldn't redefine NULL
#include <stddef.h>
// gtk headers then use __attribute__((sentinel)), which doesn't work if NULL
// is 0.
-void f(const char* c, ...) __attribute__((sentinel));
+void f(const char* c, ...) __attribute__((sentinel)); // expected-note{{function has been explicitly marked sentinel here}}
void g() {
+ f("", NULL); // expected-warning{{missing sentinel in function call}}
+}
+
+// glibc (and other) headers then define __need_NULL and rely on stddef.h
+// to redefine NULL to the correct value again.
+#define __need_NULL
+#include <stddef.h>
+
+void h() {
f("", NULL); // Shouldn't warn.
}
diff --git a/clang/test/Modules/stddef.cpp b/clang/test/Modules/stddef.cpp
new file mode 100644
index 0000000000000..c53bfa3485194
--- /dev/null
+++ b/clang/test/Modules/stddef.cpp
@@ -0,0 +1,73 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/no-lsv -I%t %t/stddef.cpp -verify
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-local-submodule-visibility -fmodules-cache-path=%t/lsv -I%t %t/stddef.cpp -verify
+
+//--- stddef.cpp
+#include <b.h>
+
+void *pointer = NULL;
+size_t size = 0;
+
+// When building with modules, a pcm is never re-imported, so re-including
+// stddef.h will not re-import _Builtin_stddef.null to restore the definition of
+// NULL, even though stddef.h will unconditionally include __stddef_null.h when
+// building with modules.
+#undef NULL
+#include <stddef.h>
+
+void *anotherPointer = NULL; // expected-error{{use of undeclared identifier 'NULL'}}
+
+// stddef.h needs to be a `textual` header to support clients doing things like
+// this.
+//
+// #define __need_NULL
+// #include <stddef.h>
+//
+// As a textual header designed to be included multiple times, it can't directly
+// declare anything, or those declarations would go into every module that
+// included it. e.g. if stddef.h contained all of its declarations, and modules
+// A and B included stddef.h, they would both have the declaration for size_t.
+// That breaks Swift, which uses the module name as part of the type name, i.e.
+// A.size_t and B.size_t are treated as completely
diff erent types in Swift and
+// cannot be interchanged. To fix that, stddef.h (and stdarg.h) are split out
+// into a separate file per __need macro that can be normal headers in explicit
+// submodules. That runs into yet another wrinkle though. When modules build,
+// declarations from previous submodules leak into subsequent ones when not
+// using local submodule visibility. Consider if stddef.h did the normal thing.
+//
+// #ifndef __STDDEF_H
+// #define __STDDEF_H
+// // include all of the sub-headers
+// #endif
+//
+// When SM builds without local submodule visibility, it will precompile a.h
+// first. When it gets to b.h, the __STDDEF_H declaration from precompiling a.h
+// will leak, and so when b.h includes stddef.h, it won't include any of its
+// sub-headers, and SM.B will thus not import _Builtin_stddef or make any of its
+// submodules visible. Precompiling b.h will be fine since it sees all of the
+// declarations from a.h including stddef.h, but clients that only include b.h
+// will not see any of the stddef.h types. stddef.h thus has to make sure to
+// always include the necessary sub-headers, even if they've been included
+// already. They all have their own header guards to allow this.
+// __stddef_null.h is extra special, so this test makes sure to cover NULL plus
+// one of the normal stddef.h types.
+
+//--- module.modulemap
+module SM {
+ module A {
+ header "a.h"
+ export *
+ }
+
+ module B {
+ header "b.h"
+ export *
+ }
+}
+
+//--- a.h
+#include <stddef.h>
+
+//--- b.h
+#include <stddef.h>
More information about the cfe-commits
mailing list