Skip to content

Commit 99e3639

Browse files
Merge pull request #9013 from ian-twilightcoder/null_overwrite
[cherry-pick stable/20240625][clang][headers] Including stddef.h always redefines NULL
2 parents 8ea33e8 + 8dab9cb commit 99e3639

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
#elif defined(__musl__)
@@ -45,7 +45,6 @@
4545
!defined(__need_NULL) && !defined(__need_nullptr_t) && \
4646
!defined(__need_unreachable) && !defined(__need_max_align_t) && \
4747
!defined(__need_offsetof) && !defined(__need_wint_t)
48-
#include <__stddef_header_macro.h>
4948
#define __need_ptrdiff_t
5049
#define __need_size_t
5150
/* ISO9899:2011 7.20 (C11 Annex K): Define rsize_t if __STDC_WANT_LIB_EXT1__ is
@@ -54,7 +53,24 @@
5453
#define __need_rsize_t
5554
#endif
5655
#define __need_wchar_t
56+
#if !defined(__STDDEF_H) || __has_feature(modules)
57+
/*
58+
* __stddef_null.h is special when building without modules: if __need_NULL is
59+
* set, then it will unconditionally redefine NULL. To avoid stepping on client
60+
* definitions of NULL, __need_NULL should only be set the first time this
61+
* header is included, that is when __STDDEF_H is not defined. However, when
62+
* building with modules, this header is a textual header and needs to
63+
* unconditionally include __stdef_null.h to support multiple submodules
64+
* exporting _Builtin_stddef.null. Take module SM with submodules A and B, whose
65+
* headers both include stddef.h When SM.A builds, __STDDEF_H will be defined.
66+
* When SM.B builds, the definition from SM.A will leak when building without
67+
* local submodule visibility. stddef.h wouldn't include __stddef_null.h, and
68+
* SM.B wouldn't import _Builtin_stddef.null, and SM.B's `export *` wouldn't
69+
* export NULL as expected. When building with modules, always include
70+
* __stddef_null.h so that everything works as expected.
71+
*/
5772
#define __need_NULL
73+
#endif
5874
#if (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 202311L) || \
5975
defined(__cplusplus)
6076
#define __need_nullptr_t
@@ -70,6 +86,7 @@
7086
/* wint_t is provided by <wchar.h> and not <stddef.h>. It's here
7187
* for compatibility, but must be explicitly requested. Therefore
7288
* __need_wint_t is intentionally not defined here. */
89+
#include <__stddef_header_macro.h>
7390
#endif
7491

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