Skip to content

Commit

Permalink
[clang][headers] Including stddef.h always redefines NULL (#99727)
Browse files Browse the repository at this point in the history
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.

(cherry picked from commit 92a9d48)
  • Loading branch information
ian-twilightcoder authored and llvmbot committed Jul 23, 2024
1 parent 183e8ec commit e3ec8d5
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 8 deletions.
4 changes: 2 additions & 2 deletions clang/lib/Headers/stdarg.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -45,6 +44,7 @@
!defined(__STRICT_ANSI__)
#define __need_va_copy
#endif
#include <__stdarg_header_macro.h>
#endif

#ifdef __need___va_list
Expand Down
21 changes: 19 additions & 2 deletions clang/lib/Headers/stddef.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
* modules.
*/
#if defined(__MVS__) && __has_include_next(<stddef.h>)
#include <__stddef_header_macro.h>
#undef __need_ptrdiff_t
#undef __need_size_t
#undef __need_rsize_t
Expand All @@ -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
Expand All @@ -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_ptrdiff_t
#define __need_size_t
/* ISO9899:2011 7.20 (C11 Annex K): Define rsize_t if __STDC_WANT_LIB_EXT1__ is
Expand All @@ -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
Expand All @@ -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_ptrdiff_t)
Expand Down
15 changes: 11 additions & 4 deletions clang/test/Headers/stddefneeds.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
}
73 changes: 73 additions & 0 deletions clang/test/Modules/stddef.cpp
Original file line number Diff line number Diff line change
@@ -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 different 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>

0 comments on commit e3ec8d5

Please sign in to comment.