Page MenuHomeFreeBSD

stddef.h: hide from C++
AbandonedPublic

Authored by vishwin on Wed, Dec 3, 2:23 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Dec 4, 7:48 PM
Unknown Object (File)
Thu, Dec 4, 7:17 PM
Unknown Object (File)
Thu, Dec 4, 4:45 PM
Unknown Object (File)
Thu, Dec 4, 4:43 PM
Subscribers
None

Details

Summary

Despite the __ISO_C_VISIBLE >= 2023, this is exposed in all of C++. Some C++ < 23 codebases, ie in ports, may define their own different signature unreachable() or rely on __builtin_unreachable(). For C++23, this still conflicts with its unreachable(), so hide this definition from all of C++.

Approved by: ?

Test Plan

Example of www/node25 exhibiting compile error without the additional preprocessor condition:

c++ -o /wrkdirs/overlays/overlay/www/node25/work/node-v25.2.1/out/Release/obj.target/v8_initializers/deps/v8/src/builtins/builtins-sharedarraybuffer-gen.o ../deps/v8/src/builtins/builtins-sharedarraybuffer-gen.cc '-D_GLIBCXX_USE_CXX11_ABI=1' '-D_FILE_OFFSET_BITS=64' '-DNODE_OPENSSL_CONF_NAME=nodejs_conf' '-DICU_NO_USER_DATA_OVERRIDE' '-DV8_GYP_BUILD' '-DV8_TYPED_ARRAY_MAX_SIZE_IN_HEAP=64' '-DBUILDING_V8_SHARED' '-D__STDC_FORMAT_MACROS' '-DOPENSSL_NO_PINSHARED' '-DOPENSSL_THREADS' '-DV8_TARGET_ARCH_X64' '-DV8_EMBEDDER_STRING="-node.14"' '-DENABLE_DISASSEMBLER' '-DV8_PROMISE_INTERNAL_FIELD_COUNT=1' '-DV8_SHORT_BUILTIN_CALLS' '-DOBJECT_PRINT' '-DV8_INTL_SUPPORT' '-DV8_ATOMIC_OBJECT_FIELD_WRITES' '-DV8_ENABLE_LAZY_SOURCE_POSITIONS' '-DV8_USE_SIPHASH' '-DNDEBUG' '-DV8_WIN64_UNWINDING_INFO' '-DV8_ENABLE_REGEXP_INTERPRETER_THREADED_DISPATCH' '-DV8_USE_ZLIB' '-DV8_ENABLE_LEAPTIERING' '-DV8_ENABLE_SPARKPLUG' '-DV8_ENABLE_MAGLEV' '-DV8_ENABLE_TURBOFAN' '-DV8_ENABLE_WEBASSEMBLY' '-DV8_ENABLE_JAVASCRIPT_PROMISE_HOOKS' '-DV8_ENABLE_CONTINUATION_PRESERVED_EMBEDDER_DATA' '-DV8_ALLOCATION_FOLDING' '-DV8_ALLOCATION_SITE_TRACKING' '-DV8_ADVANCED_BIGINT_ALGORITHMS' '-DV8_ENABLE_WASM_SIMD256_REVEC' -I../deps/v8 -I../deps/v8/include -I/wrkdirs/overlays/overlay/www/node25/work/node-v25.2.1/out/Release/obj/gen -I/wrkdirs/overlays/overlay/www/node25/work/node-v25.2.1/out/Release/obj/gen/generate-bytecode-output-root -I../deps/v8/third_party/fp16/src/include -I../deps/v8/third_party/abseil-cpp  -fvisibility=hidden -fvisibility-inlines-hidden -pthread -Wno-unused-parameter -m64 -O3 -fno-omit-frame-pointer -fdata-sections -ffunction-sections -O3 -fno-rtti -fno-exceptions -fno-strict-aliasing -std=gnu++20 -Wno-invalid-offsetof -Wno-nullability-completeness -MMD -MF /wrkdirs/overlays/overlay/www/node25/work/node-v25.2.1/out/Release/.deps//wrkdirs/overlays/overlay/www/node25/work/node-v25.2.1/out/Release/obj.target/v8_initializers/deps/v8/src/builtins/builtins-sharedarraybuffer-gen.o.d.raw -isystem /usr/local/include -O2 -pipe -fstack-protector-strong -isystem /usr/local/include -fno-strict-aliasing   -isystem /usr/local/include  -c
../deps/v8/src/builtins/builtins-sharedarraybuffer-gen.cc:156:52: error: too many arguments provided to function-like macro invocation
  156 |   Label done(this), range_error(this), unreachable(this);
      |                                                    ^
/usr/include/sys/cdefs.h:258:9: note: macro '__unreachable' defined here
  258 | #define __unreachable() __builtin_unreachable()
      |         ^
../deps/v8/src/builtins/builtins-sharedarraybuffer-gen.cc:156:40: error: no matching constructor for initialization of 'Label' (aka 'v8::internal::compiler::CodeAssemblerLabel')
  156 |   Label done(this), range_error(this), unreachable(this);
      |                                        ^
/usr/include/stddef.h:65:24: note: expanded from macro 'unreachable'
   65 | #define unreachable(x) __unreachable(x)
      |                        ^
../deps/v8/src/compiler/code-assembler.h:1793:3: note: candidate constructor not viable: requires 1 argument, but 0 were provided
 1793 |   CodeAssemblerLabel(const CodeAssemblerLabel&) = delete;
      |   ^                  ~~~~~~~~~~~~~~~~~~~~~~~~~
../deps/v8/src/compiler/code-assembler.h:1766:12: note: candidate constructor not viable: requires at least argument 'assembler', but no arguments were provided
 1766 |   explicit CodeAssemblerLabel(
      |            ^
 1767 |       CodeAssembler* assembler,
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~
 1768 |       CodeAssemblerLabel::Type type = CodeAssemblerLabel::kNonDeferred)
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../deps/v8/src/compiler/code-assembler.h:1770:3: note: candidate constructor not viable: requires at least 2 arguments, but 0 were provided
 1770 |   CodeAssemblerLabel(
      |   ^
 1771 |       CodeAssembler* assembler,
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~
 1772 |       const CodeAssemblerVariableList& merged_variables,
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 1773 |       CodeAssemblerLabel::Type type = CodeAssemblerLabel::kNonDeferred)
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../deps/v8/src/compiler/code-assembler.h:1780:3: note: candidate constructor not viable: requires at least 2 arguments, but 0 were provided
 1780 |   CodeAssemblerLabel(
      |   ^
 1781 |       CodeAssembler* assembler,
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~
 1782 |       std::initializer_list<CodeAssemblerVariable*> vars,
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 1783 |       CodeAssemblerLabel::Type type = CodeAssemblerLabel::kNonDeferred)
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../deps/v8/src/compiler/code-assembler.h:1785:3: note: candidate constructor not viable: requires at least 2 arguments, but 0 were provided
 1785 |   CodeAssemblerLabel(
      |   ^
 1786 |       CodeAssembler* assembler, CodeAssemblerVariable* merged_variable,
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 1787 |       CodeAssemblerLabel::Type type = CodeAssemblerLabel::kNonDeferred)
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../deps/v8/src/compiler/code-assembler.h:1776:3: note: candidate constructor not viable: requires at least 3 arguments, but 0 were provided
 1776 |   CodeAssemblerLabel(
      |   ^
 1777 |       CodeAssembler* assembler, size_t count,
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 1778 |       CodeAssemblerVariable* const* vars,
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 1779 |       CodeAssemblerLabel::Type type = CodeAssemblerLabel::kNonDeferred);
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../deps/v8/src/builtins/builtins-sharedarraybuffer-gen.cc:162:54: error: use of undeclared identifier 'unreachable'; did you mean '__unreachable'?
  162 |       LoadJSTypedArrayLengthAndCheckDetached(array, &unreachable);
      |                                                      ^~~~~~~~~~~
      |                                                      __unreachable
../deps/v8/src/builtins/builtins-sharedarraybuffer-gen.cc:156:40: note: '__unreachable' declared here
  156 |   Label done(this), range_error(this), unreachable(this);
      |                                        ^
/usr/include/stddef.h:65:24: note: expanded from macro 'unreachable'
   65 | #define unreachable(x) __unreachable(x)
      |                        ^
../deps/v8/src/builtins/builtins-sharedarraybuffer-gen.cc:171:9: error: use of undeclared identifier 'unreachable'; did you mean '__unreachable'?
  171 |   BIND(&unreachable);
      |         ^~~~~~~~~~~
      |         __unreachable
../deps/v8/src/codegen/define-code-stub-assembler-macros.inc:73:26: note: expanded from macro 'BIND'
   73 | #define BIND(label) Bind(label)
      |                          ^
../deps/v8/src/builtins/builtins-sharedarraybuffer-gen.cc:156:40: note: '__unreachable' declared here
  156 |   Label done(this), range_error(this), unreachable(this);
      |                                        ^
/usr/include/stddef.h:65:24: note: expanded from macro 'unreachable'
   65 | #define unreachable(x) __unreachable(x)
      |                        ^

Build continues with the additional preprocessor condition applied.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 69054
Build 65937: arc lint + arc unit

Event Timeline

vishwin created this revision.
This revision is now accepted and ready to land.Wed, Dec 3, 4:19 AM

I recommend that instead of !defined(__cplusplus), maybe we do !defined(unreachable). This should also cover C codebases with their own unreachable macros and matches what we do for offsetof.

There shouldn't be C23-compliant codebases that define their own unreachable(). Based on my understanding of sys/sys/_visible.h, it looks like __ISO_C_VISIBLE is getting defined as 2023 almost unconditionally, at least in base, so it leaked into jemalloc. It may leak into other C codebases if a standard is not explicitly passed or other macros defined during build steps.

Probably better using __STDC_VERSION__ >= 202311L both since this is not defined for C++ and to not have to worry about existing codebases, but I won't be able to test for another day or so.

ISO_C_VISIBLE: Interfaces available. It's set unconditionally based on the dialect. It's an undocumented interface.
STDC_VERSION: the exact dialect being compiled for. This is standards defined.

use __STDC_VERSION__ to ensure visibility only on C23 and later

This has not been tested on ports yet, but world builds successfully still.

This revision now requires review to proceed.Thu, Dec 4, 12:25 AM
fuz requested changes to this revision.Thu, Dec 4, 12:39 AM
fuz added inline comments.
include/stddef.h
64–66

Sorry, no, this is not the design pattern we use anywhere to decide whether to expose interfaces or not. Please don't do this.

In particular, the policy is to expose all interfaces if no compliance to a specific standard has been requested. We don't do “expose interface foo only if adherence to standard bar has been requested” unless we have to (e.g. because the interface requires some new language feature to be provided). 15.1 is a long time away from being released and I'm confident we'll be able to fix all breakage until then.

In particular, with this suggested change, passing -Dunreachable=unreachable would do the trick in most cases.

This revision now requires changes to proceed.Thu, Dec 4, 12:39 AM
include/stddef.h
64–66

This should not be leaking into C++, which is what is happening in node, webkit, etc. As for C, I thought we specified a "default" standard if one is not requested. Regardless I don't think fixing or passing flags to ports for this is reasonable.

include/stddef.h
64–66

Sure, feel free to add in !defined(__cplusplus). The way it works is that our include file machinery checks the various feature test flags,including __STDC_VERSION__ and from those derives __ISO_C_VISIBLE and friends, which are then tested by libc headers to expose features. If no feature test macro is defined, all interfaces are exposed.

revert to initial diff

Checking for the presence of an unreachable() doesn't seem right either. A pre-C23 codebase defining their own should be valid, but not for a C23 and later codebase. For instance, afaict jemalloc, even with our changes, is not a C23 codebase and should not be subject to that compilation mode or exposure to C23 elements, so it should not have that #undef unreachable hack before it defines its own.

In D54041#1234795, @fuz wrote:

I recommend that instead of !defined(__cplusplus), maybe we do !defined(unreachable). This should also cover C codebases with their own unreachable macros and matches what we do for offsetof.

This would work too and probably preferable.

No longer immediately relevant after the revert. Worth discussing the venn diagram regarding __ISO_C_VISIBLE et al since there are gotchas in C as well.