Skip to content

Commit 92a9d48

Browse files
[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.
1 parent d19e71d commit 92a9d48

File tree

4 files changed

+105
-8
lines changed

4 files changed

+105
-8
lines changed

clang/lib/Headers/stdarg.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,19 +20,18 @@
2020
* modules.
2121
*/
2222
#if defined(__MVS__) && __has_include_next(<stdarg.h>)
23-
#include <__stdarg_header_macro.h>
2423
#undef __need___va_list
2524
#undef __need_va_list
2625
#undef __need_va_arg
2726
#undef __need___va_copy
2827
#undef __need_va_copy
28+
#include <__stdarg_header_macro.h>
2929
#include_next <stdarg.h>
3030

3131
#else
3232
#if !defined(__need___va_list) && !defined(__need_va_list) && \
3333
!defined(__need_va_arg) && !defined(__need___va_copy) && \
3434
!defined(__need_va_copy)
35-
#include <__stdarg_header_macro.h>
3635
#define __need___va_list
3736
#define __need_va_list
3837
#define __need_va_arg
@@ -45,6 +44,7 @@
4544
!defined(__STRICT_ANSI__)
4645
#define __need_va_copy
4746
#endif
47+
#include <__stdarg_header_macro.h>
4848
#endif
4949

5050
#ifdef __need___va_list

clang/lib/Headers/stddef.h

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
* modules.
2121
*/
2222
#if defined(__MVS__) && __has_include_next(<stddef.h>)
23-
#include <__stddef_header_macro.h>
2423
#undef __need_ptrdiff_t
2524
#undef __need_size_t
2625
#undef __need_rsize_t
@@ -31,6 +30,7 @@
3130
#undef __need_max_align_t
3231
#undef __need_offsetof
3332
#undef __need_wint_t
33+
#include <__stddef_header_macro.h>
3434
#include_next <stddef.h>
3535

3636
#else
@@ -40,7 +40,6 @@
4040
!defined(__need_NULL) && !defined(__need_nullptr_t) && \
4141
!defined(__need_unreachable) && !defined(__need_max_align_t) && \
4242
!defined(__need_offsetof) && !defined(__need_wint_t)
43-
#include <__stddef_header_macro.h>
4443
#define __need_ptrdiff_t
4544
#define __need_size_t
4645
/* ISO9899:2011 7.20 (C11 Annex K): Define rsize_t if __STDC_WANT_LIB_EXT1__ is
@@ -49,7 +48,24 @@
4948
#define __need_rsize_t
5049
#endif
5150
#define __need_wchar_t
51+
#if !defined(__STDDEF_H) || __has_feature(modules)
52+
/*
53+
* __stddef_null.h is special when building without modules: if __need_NULL is
54+
* set, then it will unconditionally redefine NULL. To avoid stepping on client
55+
* definitions of NULL, __need_NULL should only be set the first time this
56+
* header is included, that is when __STDDEF_H is not defined. However, when
57+
* building with modules, this header is a textual header and needs to
58+
* unconditionally include __stdef_null.h to support multiple submodules
59+
* exporting _Builtin_stddef.null. Take module SM with submodules A and B, whose
60+
* headers both include stddef.h When SM.A builds, __STDDEF_H will be defined.
61+
* When SM.B builds, the definition from SM.A will leak when building without
62+
* local submodule visibility. stddef.h wouldn't include __stddef_null.h, and
63+
* SM.B wouldn't import _Builtin_stddef.null, and SM.B's `export *` wouldn't
64+
* export NULL as expected. When building with modules, always include
65+
* __stddef_null.h so that everything works as expected.
66+
*/
5267
#define __need_NULL
68+
#endif
5369
#if (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 202311L) || \
5470
defined(__cplusplus)
5571
#define __need_nullptr_t
@@ -65,6 +81,7 @@
6581
/* wint_t is provided by <wchar.h> and not <stddef.h>. It's here
6682
* for compatibility, but must be explicitly requested. Therefore
6783
* __need_wint_t is intentionally not defined here. */
84+
#include <__stddef_header_macro.h>
6885
#endif
6986

7087
#if defined(__need_ptrdiff_t)

clang/test/Headers/stddefneeds.cpp

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,14 +56,21 @@ max_align_t m5;
5656
#undef NULL
5757
#define NULL 0
5858

59-
// glibc (and other) headers then define __need_NULL and rely on stddef.h
60-
// to redefine NULL to the correct value again.
61-
#define __need_NULL
59+
// Including stddef.h again shouldn't redefine NULL
6260
#include <stddef.h>
6361

6462
// gtk headers then use __attribute__((sentinel)), which doesn't work if NULL
6563
// is 0.
66-
void f(const char* c, ...) __attribute__((sentinel));
64+
void f(const char* c, ...) __attribute__((sentinel)); // expected-note{{function has been explicitly marked sentinel here}}
6765
void g() {
66+
f("", NULL); // expected-warning{{missing sentinel in function call}}
67+
}
68+
69+
// glibc (and other) headers then define __need_NULL and rely on stddef.h
70+
// to redefine NULL to the correct value again.
71+
#define __need_NULL
72+
#include <stddef.h>
73+
74+
void h() {
6875
f("", NULL); // Shouldn't warn.
6976
}

clang/test/Modules/stddef.cpp

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
// RUN: rm -rf %t
2+
// RUN: split-file %s %t
3+
// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/no-lsv -I%t %t/stddef.cpp -verify
4+
// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-local-submodule-visibility -fmodules-cache-path=%t/lsv -I%t %t/stddef.cpp -verify
5+
6+
//--- stddef.cpp
7+
#include <b.h>
8+
9+
void *pointer = NULL;
10+
size_t size = 0;
11+
12+
// When building with modules, a pcm is never re-imported, so re-including
13+
// stddef.h will not re-import _Builtin_stddef.null to restore the definition of
14+
// NULL, even though stddef.h will unconditionally include __stddef_null.h when
15+
// building with modules.
16+
#undef NULL
17+
#include <stddef.h>
18+
19+
void *anotherPointer = NULL; // expected-error{{use of undeclared identifier 'NULL'}}
20+
21+
// stddef.h needs to be a `textual` header to support clients doing things like
22+
// this.
23+
//
24+
// #define __need_NULL
25+
// #include <stddef.h>
26+
//
27+
// As a textual header designed to be included multiple times, it can't directly
28+
// declare anything, or those declarations would go into every module that
29+
// included it. e.g. if stddef.h contained all of its declarations, and modules
30+
// A and B included stddef.h, they would both have the declaration for size_t.
31+
// That breaks Swift, which uses the module name as part of the type name, i.e.
32+
// A.size_t and B.size_t are treated as completely different types in Swift and
33+
// cannot be interchanged. To fix that, stddef.h (and stdarg.h) are split out
34+
// into a separate file per __need macro that can be normal headers in explicit
35+
// submodules. That runs into yet another wrinkle though. When modules build,
36+
// declarations from previous submodules leak into subsequent ones when not
37+
// using local submodule visibility. Consider if stddef.h did the normal thing.
38+
//
39+
// #ifndef __STDDEF_H
40+
// #define __STDDEF_H
41+
// // include all of the sub-headers
42+
// #endif
43+
//
44+
// When SM builds without local submodule visibility, it will precompile a.h
45+
// first. When it gets to b.h, the __STDDEF_H declaration from precompiling a.h
46+
// will leak, and so when b.h includes stddef.h, it won't include any of its
47+
// sub-headers, and SM.B will thus not import _Builtin_stddef or make any of its
48+
// submodules visible. Precompiling b.h will be fine since it sees all of the
49+
// declarations from a.h including stddef.h, but clients that only include b.h
50+
// will not see any of the stddef.h types. stddef.h thus has to make sure to
51+
// always include the necessary sub-headers, even if they've been included
52+
// already. They all have their own header guards to allow this.
53+
// __stddef_null.h is extra special, so this test makes sure to cover NULL plus
54+
// one of the normal stddef.h types.
55+
56+
//--- module.modulemap
57+
module SM {
58+
module A {
59+
header "a.h"
60+
export *
61+
}
62+
63+
module B {
64+
header "b.h"
65+
export *
66+
}
67+
}
68+
69+
//--- a.h
70+
#include <stddef.h>
71+
72+
//--- b.h
73+
#include <stddef.h>

0 commit comments

Comments
 (0)