Skip to content

Commit b8858da

Browse files
authored
Merge pull request #9003 from ian-twilightcoder/null_overwrite
[cherry-pick swift/release/6.0][clang][headers] Including stddef.h always redefines NULL
2 parents 723ba90 + d5e0d9c commit b8858da

File tree

4 files changed

+103
-6
lines changed

4 files changed

+103
-6
lines changed

clang/lib/Headers/stdarg.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
#if !defined(__need___va_list) && !defined(__need_va_list) && \
2323
!defined(__need_va_arg) && !defined(__need___va_copy) && \
2424
!defined(__need_va_copy)
25-
#include <__stdarg_header_macro.h>
2625
#define __need___va_list
2726
#define __need_va_list
2827
#define __need_va_arg
@@ -35,6 +34,7 @@
3534
!defined(__STRICT_ANSI__)
3635
#define __need_va_copy
3736
#endif
37+
#include <__stdarg_header_macro.h>
3838
#endif
3939

4040
#ifdef __need___va_list

clang/lib/Headers/stddef.h

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232
!defined(__need_NULL) && !defined(__need_nullptr_t) && \
3333
!defined(__need_unreachable) && !defined(__need_max_align_t) && \
3434
!defined(__need_offsetof) && !defined(__need_wint_t)
35-
#include <__stddef_header_macro.h>
3635
#define __need_ptrdiff_t
3736
#define __need_size_t
3837
/* ISO9899:2011 7.20 (C11 Annex K): Define rsize_t if __STDC_WANT_LIB_EXT1__ is
@@ -41,7 +40,24 @@
4140
#define __need_rsize_t
4241
#endif
4342
#define __need_wchar_t
43+
#if !defined(__STDDEF_H) || __has_feature(modules)
44+
/*
45+
* __stddef_null.h is special when building without modules: if __need_NULL is
46+
* set, then it will unconditionally redefine NULL. To avoid stepping on client
47+
* definitions of NULL, __need_NULL should only be set the first time this
48+
* header is included, that is when __STDDEF_H is not defined. However, when
49+
* building with modules, this header is a textual header and needs to
50+
* unconditionally include __stdef_null.h to support multiple submodules
51+
* exporting _Builtin_stddef.null. Take module SM with submodules A and B, whose
52+
* headers both include stddef.h When SM.A builds, __STDDEF_H will be defined.
53+
* When SM.B builds, the definition from SM.A will leak when building without
54+
* local submodule visibility. stddef.h wouldn't include __stddef_null.h, and
55+
* SM.B wouldn't import _Builtin_stddef.null, and SM.B's `export *` wouldn't
56+
* export NULL as expected. When building with modules, always include
57+
* __stddef_null.h so that everything works as expected.
58+
*/
4459
#define __need_NULL
60+
#endif
4561
/* FIXME: This is using the placeholder dates Clang produces for these macros
4662
in C2x mode; switch to the correct values once they've been published. */
4763
#if (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 202000L) || \
@@ -59,6 +75,7 @@
5975
/* wint_t is provided by <wchar.h> and not <stddef.h>. It's here
6076
* for compatibility, but must be explicitly requested. Therefore
6177
* __need_wint_t is intentionally not defined here. */
78+
#include <__stddef_header_macro.h>
6279
#endif
6380

6481
#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)