From 73a6ea5c9c881344128f26ca7e3ba74334bac4e9 Mon Sep 17 00:00:00 2001 From: Alejandro Cabeza Romero Date: Mon, 18 May 2026 18:54:24 +0200 Subject: [PATCH] wip2 --- .github/resources/witness-generator/Makefile | 23 +++++------- CONTRIBUTING.md | 38 ++++++++++---------- 2 files changed, 28 insertions(+), 33 deletions(-) diff --git a/.github/resources/witness-generator/Makefile b/.github/resources/witness-generator/Makefile index f8d1613..a2a4a59 100644 --- a/.github/resources/witness-generator/Makefile +++ b/.github/resources/witness-generator/Makefile @@ -52,15 +52,10 @@ windows: $(BIN) windows-lib: CXXFLAGS=$(CXXFLAGS_COMMON) -fPIC -I/include -Duint="unsigned int" windows-lib: $(LIB) -# Localizes circuit-specific symbols so multiple circuit libraries can coexist in the -# same binary without symbol conflicts. See CONTRIBUTING.md § "Symbol Isolation". -# Override with LOCALIZE_SYMS= to skip localization. -MERGE_OBJS := $(PROJECT).o $(PROJECT)/ffi.o circom_adapter.o -LOCALIZE_SYMS ?= loadCircuit writeBinWitness \ - get_size_of_witness get_size_of_constants get_size_of_input_hashmap \ - get_main_input_signal_no get_main_input_signal_start get_total_signal_no \ - get_number_of_components get_size_of_io_map get_size_of_bus_field_map -LOCAL_OBJ := $(PROJECT)_local.o +# Localizes all circuit-specific code to prevent conflicts when multiple circuit +# libraries are linked into the same binary. See CONTRIBUTING.md § "Symbol Isolation". +PUBLIC_SYMS := $(PROJECT)_generate_witness $(PROJECT)_generate_witness_from_files +LOCAL_OBJ := $(PROJECT)_local.o UNAME := $(shell uname -s) @@ -69,14 +64,12 @@ $(BIN): $(COMMON_OBJS) $(CXX) $(LDFLAGS) $^ $(LDLIBS) -o $@ $(LIB): $(LIB_OBJS) -ifeq ($(strip $(LOCALIZE_SYMS)),) - ar rcs $@ $^ # Localization disabled -else ifeq ($(UNAME),Darwin) +ifeq ($(UNAME),Darwin) ar rcs $@ $^ # On macOS two-level namespace, conflicts don't arise else - ld -r -o $(LOCAL_OBJ) $(MERGE_OBJS) - objcopy $(foreach s,$(LOCALIZE_SYMS),--localize-symbol=$(s)) $(LOCAL_OBJ) - ar rcs $@ $(filter-out $(MERGE_OBJS),$^) $(LOCAL_OBJ) + ld -r -o $(LOCAL_OBJ) $(filter-out fr.o,$^) + llvm-objcopy $(foreach s,$(PUBLIC_SYMS),--keep-global-symbol=$(s)) $(LOCAL_OBJ) + ar rcs $@ fr.o $(LOCAL_OBJ) rm $(LOCAL_OBJ) endif diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e5db59d..04ed50b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -57,32 +57,34 @@ page, which results in a **SIGSEGV**. ### The Fix -The Makefile's `$(LIB)` rule uses a two-step process on Linux and Windows to localize all internal symbols before +The Makefile's `$(LIB)` rule uses a two-step process on Linux and Windows to localize all circuit-specific code before archiving: -1. **Partial link** (`ld -r`): merges all `.o` files into a single relocatable object without producing a final -executable. -No symbols are resolved yet; this is consolidation only. -2. **Symbol localization** (`objcopy --keep-global-symbol`): demotes every global symbol to local *except* the circuit's -two public FFI entry points. -Local symbols are invisible to the final linker, so each archive retains a private copy of every internal symbol. This -means no conflict is possible regardless of how many circuits are linked together. +1. **Partial link** (`ld -r`): merges all circuit-specific `.o` files — everything except `fr.o` (pure field +arithmetic, no circuit-specific calls) — into a single relocatable object. No symbols are resolved yet; this is +consolidation only. +2. **Symbol localization** (`llvm-objcopy --keep-global-symbol`): demotes every global symbol to local *except* the +circuit's two public FFI entry points (`$(PROJECT)_generate_witness` and `$(PROJECT)_generate_witness_from_files`). +Local symbols are invisible to the final linker, so each archive retains a private copy of every internal symbol — no +conflict is possible regardless of how many circuits are linked together. -The public symbols are derived automatically from `PROJECT`: a circuit built with `PROJECT=poq` keeps -`poq_generate_witness` and `poq_generate_witness_from_files` global and localizes everything else. +`llvm-objcopy` is required rather than GNU `objcopy`. GNU `objcopy` only changes the binding of COMDAT signature +symbols to local, which confuses the linker's deduplication logic and causes "relocation refers to symbol in discarded +section" errors. `llvm-objcopy` additionally clears the `GRP_COMDAT` flag on affected section groups, turning them into +regular non-COMDAT sections that are simply kept as-is rather than deduplicated. The result is a slightly larger binary +(each circuit keeps its own copy of shared template instantiations), but no linker errors. -> To skip localization for a specific build (e.g. for debugging), pass `PUBLIC_SYMBOLS=` explicitly on the `make` command -> line. +`fr.o` is excluded from the merge because it contains only field arithmetic (`Fr_*`) with no circuit-specific calls. +It is safe to deduplicate across circuits — the linker picks one copy, which is correct since the code is identical. -On macOS, localization is skipped because `objcopy` is a GNU Binutils tool unavailable by default there. -This is safe: macOS uses a two-level namespace by default, meaning symbols are qualified by which library they come -from, so the conflict does not arise. +On macOS, localization is skipped. macOS uses a two-level namespace by default, meaning symbols are qualified by which +library they come from, so the conflict does not arise. ### Maintenance -`PUBLIC_SYMBOLS` defaults to `$(PROJECT)_generate_witness` and `$(PROJECT)_generate_witness_from_files`. -If the public FFI API ever changes, meaning the entrypoints are renamed or new ones added, the Makefile default must be -updated, otherwise the affected symbols will be localized and linking will fail. +`PUBLIC_SYMS` is hardcoded to `$(PROJECT)_generate_witness` and `$(PROJECT)_generate_witness_from_files` in the +Makefile. If the public FFI API ever changes — entry points renamed or new ones added — update that variable, +otherwise the affected symbols will be localized and linking will fail. ---