Copilot commented on code in PR #18886:
URL: https://github.com/apache/nuttx/pull/18886#discussion_r3252792138
##########
arch/sim/src/sim/CMakeLists.txt:
##########
@@ -150,14 +150,17 @@ list(
sim_hosttime.c
sim_hostuart.c)
-# Note: sim_macho_init.c is picky about the place in the object list for
-# linking. Namely, its constructor should be the first one in the executable.
-# For now, we are just assuming no other files in HOSTSRCS provide
constructors.
if(CONFIG_HOST_MACOS)
- if(CONFIG_HAVE_CXXINITIALIZE)
- list(APPEND HOSTSRCS sim_macho_init.c)
- target_link_options(nuttx PRIVATE -Wl,-ld_classic,-no_fixup_chains)
- endif()
+ # Keep classic __mod_init_func format so post-link lief patching works
+ target_link_options(nuttx PRIVATE -Wl,-ld_classic,-no_fixup_chains)
+ add_custom_command(
+ TARGET nuttx
+ POST_BUILD
+ COMMAND
+ ${Python3_EXECUTABLE}
+ ${CMAKE_CURRENT_SOURCE_DIR}/../patch_macho_initsection.py
+ $<TARGET_FILE:nuttx>
+ COMMENT "Patching Mach-O init section type flags")
Review Comment:
The post-build patch is registered for every macOS CMake build, so
configurations without `CONFIG_HAVE_CXXINITIALIZE` still require Python/lief
and run the Mach-O patcher even though no deferred C++ constructor support is
needed. This regresses plain macOS simulator builds by adding an unnecessary
build-time dependency; gate the custom command the same way the make build does.
##########
arch/sim/src/Makefile:
##########
@@ -139,18 +139,8 @@ sim_hostfs.c: hostfs.h
STDLIBS += -lpthread
ifeq ($(CONFIG_HOST_MACOS),y)
-ifeq ($(CONFIG_HAVE_CXXINITIALIZE),y)
- # Note: sim_macho_init.c is not in CSRCS because it's picky about
- # the place in the object list for linking. Namely, its constructor
- # should be the first one in the executable.
- HEADSRC = sim_macho_init.c
-
- # sim_macho_init.c is not compatible with chained fixups.
- # cf. https://github.com/apache/nuttx/issues/15208
- ifeq ($(shell $(LD) -ld_classic -no_fixup_chains 2>&1 | grep "unknown
option"),)
- LDLINKFLAGS += -ld_classic -no_fixup_chains
- LDFLAGS += -Wl,-ld_classic,-no_fixup_chains
- endif
+ LDLINKFLAGS += -ld_classic,-no_fixup_chains
Review Comment:
This passes the two ld64 options as a single argument to the direct `$(LD)`
invocation used at line 480. The previous code used separate arguments
(`-ld_classic -no_fixup_chains`); with the comma form ld64 treats it as one
option and can reject it, breaking the macOS make build before the final link
step.
##########
arch/sim/src/patch_macho_initsection.py:
##########
@@ -0,0 +1,72 @@
+#!/usr/bin/env python3
+############################################################################
+# arch/sim/src/patch_macho_initsection.py
+#
+# SPDX-License-Identifier: Apache-2.0
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership. The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+# implied. See the License for the specific language governing
+# permissions and limitations under the License.
+#
+############################################################################
+
+"""Patch Mach-O init section type flags to prevent dyld from
+auto-running C++ constructors.
+
+Changes MOD_INIT_FUNC_POINTERS (0x9) and INIT_FUNC_OFFSETS (0x16)
+section types to REGULAR (0x0) so that dyld ignores them. NuttX
+will invoke the constructors explicitly from lib_cxx_initialize().
+
+Requires: pip install lief
+"""
+
+import argparse
+import sys
+
+try:
+ import lief
+except ImportError:
+ print(
+ "Error: lief is required. Install with: pip install lief",
+ file=sys.stderr,
+ )
+ sys.exit(1)
+
+
+def main():
+ parser = argparse.ArgumentParser(description="Patch Mach-O init section
type flags")
+ parser.add_argument("binary", help="Path to the Mach-O binary")
+ args = parser.parse_args()
+
+ binary = lief.MachO.parse(args.binary)
+ if binary is None:
+ print(f"Error: failed to parse {args.binary}", file=sys.stderr)
+ sys.exit(1)
+
+ fat = binary.at(0)
+ T = lief.MachO.Section.TYPE
+ patched = 0
+ for section in fat.sections:
+ if section.type in (T.MOD_INIT_FUNC_POINTERS, T.INIT_FUNC_OFFSETS):
+ section.type = T.REGULAR
Review Comment:
`INIT_FUNC_OFFSETS` sections do not contain an array of function pointers;
they contain 32-bit offsets. Changing only the section type to `REGULAR` leaves
the data in offset form, but `lib_cxx_initialize()` walks `_sinit`/`_einit` as
`initializer_t *`, so any binary that reaches this branch will later call
invalid addresses instead of constructors. The script should either reject this
section type or convert the offsets to pointers before patching it.
##########
arch/sim/src/sim/CMakeLists.txt:
##########
@@ -150,14 +150,17 @@ list(
sim_hosttime.c
sim_hostuart.c)
-# Note: sim_macho_init.c is picky about the place in the object list for
-# linking. Namely, its constructor should be the first one in the executable.
-# For now, we are just assuming no other files in HOSTSRCS provide
constructors.
if(CONFIG_HOST_MACOS)
- if(CONFIG_HAVE_CXXINITIALIZE)
- list(APPEND HOSTSRCS sim_macho_init.c)
- target_link_options(nuttx PRIVATE -Wl,-ld_classic,-no_fixup_chains)
- endif()
+ # Keep classic __mod_init_func format so post-link lief patching works
+ target_link_options(nuttx PRIVATE -Wl,-ld_classic,-no_fixup_chains)
+ add_custom_command(
+ TARGET nuttx
+ POST_BUILD
+ COMMAND
+ ${Python3_EXECUTABLE}
+ ${CMAKE_CURRENT_SOURCE_DIR}/../patch_macho_initsection.py
+ $<TARGET_FILE:nuttx>
+ COMMENT "Patching Mach-O init section type flags")
Review Comment:
This unconditionally adds ld64-specific options for all macOS CMake builds
without checking whether the selected linker supports them. The make path
previously probed for `unknown option`; without an equivalent guard,
older/different macOS toolchains can fail to link even when C++ constructor
deferral is not enabled.
##########
arch/sim/include/arch.h:
##########
@@ -27,4 +27,25 @@
#ifndef __ARCH_SIM_INCLUDE_ARCH_H
#define __ARCH_SIM_INCLUDE_ARCH_H
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* _sinit and _einit mark the beginning and end of the C++ constructor
+ * array. They are mapped to platform-specific linker symbols:
+ * macOS: section$start$__DATA_CONST$__mod_init_func /
+ * section$end$__DATA_CONST$__mod_init_func (Mach-O auto)
+ * On macOS, the section type flags are patched post-link to prevent
+ * dyld from auto-running constructors before NuttX is initialized.
+ */
+
+#ifdef CONFIG_HAVE_CXXINITIALIZE
+# if defined(CONFIG_HOST_MACOS)
+extern void (*_sinit[])(void)
+ __asm("section$start$__DATA_CONST$__mod_init_func");
+extern void (*_einit[])(void)
+ __asm("section$end$__DATA_CONST$__mod_init_func");
Review Comment:
These C symbols are declared before `include/nuttx/arch.h` opens its `extern
"C"` block, and that header later redeclares `_sinit`/`_einit` with C linkage.
In C++ translation units this creates conflicting C++-linkage vs C-linkage
declarations for the same names, so macOS C++ builds that include
`<nuttx/arch.h>` can fail to compile.
##########
arch/sim/src/Makefile:
##########
@@ -139,18 +139,8 @@ sim_hostfs.c: hostfs.h
STDLIBS += -lpthread
ifeq ($(CONFIG_HOST_MACOS),y)
-ifeq ($(CONFIG_HAVE_CXXINITIALIZE),y)
- # Note: sim_macho_init.c is not in CSRCS because it's picky about
- # the place in the object list for linking. Namely, its constructor
- # should be the first one in the executable.
- HEADSRC = sim_macho_init.c
-
- # sim_macho_init.c is not compatible with chained fixups.
- # cf. https://github.com/apache/nuttx/issues/15208
- ifeq ($(shell $(LD) -ld_classic -no_fixup_chains 2>&1 | grep "unknown
option"),)
- LDLINKFLAGS += -ld_classic -no_fixup_chains
- LDFLAGS += -Wl,-ld_classic,-no_fixup_chains
- endif
+ LDLINKFLAGS += -ld_classic,-no_fixup_chains
+ LDFLAGS += -Wl,-ld_classic,-no_fixup_chains
endif
Review Comment:
After removing the inner `ifeq ($(CONFIG_HAVE_CXXINITIALIZE),y)`, this
`endif` now closes the `CONFIG_HOST_MACOS` block before the `else` at line 150.
That leaves the later `else` unmatched, so GNU make will fail to parse this
file for simulator builds.
##########
arch/sim/src/Makefile:
##########
@@ -139,18 +139,8 @@ sim_hostfs.c: hostfs.h
STDLIBS += -lpthread
ifeq ($(CONFIG_HOST_MACOS),y)
-ifeq ($(CONFIG_HAVE_CXXINITIALIZE),y)
- # Note: sim_macho_init.c is not in CSRCS because it's picky about
- # the place in the object list for linking. Namely, its constructor
- # should be the first one in the executable.
- HEADSRC = sim_macho_init.c
-
- # sim_macho_init.c is not compatible with chained fixups.
- # cf. https://github.com/apache/nuttx/issues/15208
- ifeq ($(shell $(LD) -ld_classic -no_fixup_chains 2>&1 | grep "unknown
option"),)
- LDLINKFLAGS += -ld_classic -no_fixup_chains
- LDFLAGS += -Wl,-ld_classic,-no_fixup_chains
- endif
+ LDLINKFLAGS += -ld_classic,-no_fixup_chains
+ LDFLAGS += -Wl,-ld_classic,-no_fixup_chains
Review Comment:
This now forces `-ld_classic`/`-no_fixup_chains` on every macOS make build
and removed the previous probe that skipped them when ld64 reported `unknown
option`. On toolchains that do not provide these options, even configurations
without C++ constructor initialization will fail to link.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
