mirror of
https://github.com/logos-blockchain/logos-blockchain-circuits.git
synced 2026-05-21 00:39:26 +00:00
fix(build): Fix symbol resolution conflicts between circuits (#22)
This commit is contained in:
parent
653c9295e4
commit
104acb8f47
24
.github/resources/witness-generator/Makefile
vendored
24
.github/resources/witness-generator/Makefile
vendored
@ -52,15 +52,35 @@ windows: $(BIN)
|
||||
windows-lib: CXXFLAGS=$(CXXFLAGS_COMMON) -fPIC -I/include -Duint="unsigned int"
|
||||
windows-lib: $(LIB)
|
||||
|
||||
# Localizes all circuit-specific code to prevent conflicts when multiple circuit
|
||||
# libraries are linked into the same binary. See CONTRIBUTING.md § "Symbol Isolation".
|
||||
# On Linux/ELF: llvm-objcopy is required — it clears GRP_COMDAT when localizing
|
||||
# COMDAT signature symbols, preventing "relocation refers to symbol in discarded
|
||||
# section" errors that GNU objcopy causes. On Windows/COFF: GNU objcopy suffices
|
||||
# because COFF COMDAT is per-section (not group-based) and is already deduplicated
|
||||
# automatically by the linker — the ELF GRP_COMDAT problem does not apply.
|
||||
PUBLIC_SYMS := $(PROJECT)_generate_witness $(PROJECT)_generate_witness_from_files
|
||||
LOCAL_OBJ := $(PROJECT)_local.o
|
||||
OBJCOPY := $(if $(filter windows,$(OS)),objcopy,llvm-objcopy)
|
||||
|
||||
UNAME := $(shell uname -s)
|
||||
|
||||
# ---- Rules ----
|
||||
$(BIN): $(COMMON_OBJS)
|
||||
$(CXX) $(LDFLAGS) $^ $(LDLIBS) -o $@
|
||||
|
||||
$(LIB): $(LIB_OBJS)
|
||||
ar rcs $@ $^
|
||||
ifeq ($(UNAME),Darwin)
|
||||
ar rcs $@ $^ # macOS: two-level namespace, conflicts don't arise
|
||||
else
|
||||
ld -r -o $(LOCAL_OBJ) $(filter-out fr.o,$^)
|
||||
$(OBJCOPY) $(foreach s,$(PUBLIC_SYMS),--keep-global-symbol=$(s)) $(LOCAL_OBJ)
|
||||
ar rcs $@ fr.o $(LOCAL_OBJ)
|
||||
rm $(LOCAL_OBJ)
|
||||
endif
|
||||
|
||||
%.o: %.cpp $(DEPS_HPP)
|
||||
$(CXX) $(CXXFLAGS) -c $< -o $@
|
||||
|
||||
clean:
|
||||
rm -f $(COMMON_OBJS) $(LIB_ONLY_OBJS) $(BIN) $(LIB)
|
||||
rm -f $(COMMON_OBJS) $(LIB_ONLY_OBJS) $(BIN) $(LIB) $(LOCAL_OBJ)
|
||||
|
||||
4
.github/workflows/ci.yml
vendored
4
.github/workflows/ci.yml
vendored
@ -197,7 +197,7 @@ jobs:
|
||||
run: sudo apt install -y build-essential cmake libgmp-dev libsodium-dev nasm curl m4
|
||||
|
||||
- name: Install Dependencies [Witness Generator]
|
||||
run: sudo apt install nlohmann-json3-dev
|
||||
run: sudo apt install -y nlohmann-json3-dev llvm
|
||||
|
||||
- name: Replace Prover Makefile # TODO: Make a fork generate the appropriate Linux Makefile
|
||||
run: cp .github/resources/prover/Makefile rapidsnark/Makefile
|
||||
@ -434,7 +434,7 @@ jobs:
|
||||
run: sudo apt install -y build-essential cmake libgmp-dev libsodium-dev nasm curl m4
|
||||
|
||||
- name: Install Dependencies [Witness Generator]
|
||||
run: sudo apt install -y nlohmann-json3-dev
|
||||
run: sudo apt install -y nlohmann-json3-dev llvm
|
||||
|
||||
- name: Replace Prover Makefile
|
||||
run: cp .github/resources/prover/Makefile rapidsnark/Makefile
|
||||
|
||||
@ -6,6 +6,7 @@
|
||||
|
||||
- [Rust](https://rustup.rs/) — the pinned toolchain version is in `rust-toolchain.toml` and will be installed automatically by `rustup`.
|
||||
- [pre-commit](https://pre-commit.com/) — used to run formatting, linting, and audit checks before each commit.
|
||||
- `llvm-objcopy` — required to build circuit static libraries (symbol isolation) on Linux. Install via `sudo apt install -y llvm`. On Windows (MSYS2), GNU `objcopy` from `mingw-w64-x86_64-toolchain` is used instead (no extra install needed). On macOS, symbol isolation is skipped entirely, so no objcopy tool is required.
|
||||
|
||||
### Installing the Pre-Commit Hooks
|
||||
|
||||
@ -37,6 +38,63 @@ When bumping the stable toolchain, update `channel` in `rust-toolchain.toml`. Th
|
||||
|
||||
---
|
||||
|
||||
## Symbol Isolation in Circuit Libraries
|
||||
|
||||
Each circuit (PoQ, PoL, PoC, Signature) is compiled into a static archive (`libpoq.a`, `libpol.a`, etc.).
|
||||
All four archives share the same internal C++ runtime — `loadCircuit`, `get_size_of_witness`, the `fr_*` field
|
||||
arithmetic functions, `calcwit_*` functions, and others. They are compiled from the same source files but with
|
||||
**different constant values per circuit** (e.g. `get_size_of_witness()` returns 18149 for PoQ and 20531 for PoL).
|
||||
|
||||
### The Problem
|
||||
|
||||
When two or more circuit libraries are linked into the same binary, the GNU linker silently picks the first definition
|
||||
it encounters for each symbol and discards the rest.
|
||||
No error, no warning.
|
||||
The result is that one circuit's constants end up hardwired into functions shared by both circuits, corrupting witness
|
||||
parsing.
|
||||
In practice: the wrong `get_size_of_witness()` value causes `loadCircuit` to compute an incorrect buffer size, `pu32`
|
||||
walks off the end of the buffer, reads garbage as a length field, and the subsequent `memcpy` reads past the stack guard
|
||||
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 circuit-specific code before
|
||||
archiving:
|
||||
|
||||
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` on Linux, `objcopy --keep-global-symbol` on Windows): 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.
|
||||
|
||||
`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.
|
||||
|
||||
`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. 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 Windows, GNU `objcopy` (from MinGW binutils) is used instead of `llvm-objcopy`. `llvm-objcopy --keep-global-symbol`
|
||||
is not supported for COFF objects, but GNU `objcopy --keep-global-symbol` works correctly on COFF — it maps the local
|
||||
binding to COFF storage class `C_STAT`. The ELF `GRP_COMDAT` problem that required `llvm-objcopy` on Linux does not
|
||||
apply on Windows: COFF COMDAT is per-section rather than group-based, and the linker already deduplicates it
|
||||
automatically.
|
||||
|
||||
### Maintenance
|
||||
|
||||
`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.
|
||||
|
||||
---
|
||||
|
||||
## Triggering a New Release for Logos Blockchain Circuits
|
||||
|
||||
To trigger a release build:
|
||||
|
||||
8
rust/Cargo.lock
generated
8
rust/Cargo.lock
generated
@ -237,6 +237,14 @@ dependencies = [
|
||||
"logos-blockchain-circuits-types",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "logos-blockchain-circuits-tests"
|
||||
version = "0.5.0"
|
||||
dependencies = [
|
||||
"logos-blockchain-circuits-pol-sys",
|
||||
"logos-blockchain-circuits-poq-sys",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "logos-blockchain-circuits-types"
|
||||
version = "0.5.0"
|
||||
|
||||
@ -15,6 +15,7 @@ members = [
|
||||
"logos-blockchain-circuits-pol-sys",
|
||||
"logos-blockchain-circuits-poq-sys",
|
||||
"logos-blockchain-circuits-signature-sys",
|
||||
"logos-blockchain-circuits-tests",
|
||||
"logos-blockchain-circuits-types",
|
||||
"logos-blockchain-circuits-common",
|
||||
]
|
||||
|
||||
10
rust/logos-blockchain-circuits-tests/Cargo.toml
Normal file
10
rust/logos-blockchain-circuits-tests/Cargo.toml
Normal file
@ -0,0 +1,10 @@
|
||||
[package]
|
||||
name = "logos-blockchain-circuits-tests"
|
||||
edition.workspace = true
|
||||
license.workspace = true
|
||||
version.workspace = true
|
||||
publish = false
|
||||
|
||||
[dev-dependencies]
|
||||
lbc-pol-sys = { workspace = true, features = ["prebuilt"] }
|
||||
lbc-poq-sys = { workspace = true, features = ["prebuilt"] }
|
||||
22
rust/logos-blockchain-circuits-tests/src/lib.rs
Normal file
22
rust/logos-blockchain-circuits-tests/src/lib.rs
Normal file
@ -0,0 +1,22 @@
|
||||
pub mod roots {
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::sync::LazyLock;
|
||||
|
||||
pub static TESTS: LazyLock<PathBuf> =
|
||||
LazyLock::new(|| PathBuf::from(env!("CARGO_MANIFEST_DIR")));
|
||||
pub static REPOSITORY: LazyLock<&Path> =
|
||||
LazyLock::new(|| TESTS.parent().expect("Failed to find the repository root."));
|
||||
pub static POL: LazyLock<PathBuf> =
|
||||
LazyLock::new(|| REPOSITORY.join("logos-blockchain-circuits-pol-sys"));
|
||||
pub static POQ: LazyLock<PathBuf> =
|
||||
LazyLock::new(|| REPOSITORY.join("logos-blockchain-circuits-poq-sys"));
|
||||
}
|
||||
|
||||
pub mod inputs {
|
||||
use super::roots;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::LazyLock;
|
||||
|
||||
pub static POL: LazyLock<PathBuf> = LazyLock::new(|| roots::POL.join("sample.input.json"));
|
||||
pub static POQ: LazyLock<PathBuf> = LazyLock::new(|| roots::POQ.join("sample.input.json"));
|
||||
}
|
||||
24
rust/logos-blockchain-circuits-tests/tests/conflicts.rs
Normal file
24
rust/logos-blockchain-circuits-tests/tests/conflicts.rs
Normal file
@ -0,0 +1,24 @@
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use lbc_poq_sys::PoqWitnessInput;
|
||||
use logos_blockchain_circuits_tests::inputs;
|
||||
|
||||
#[test]
|
||||
fn test_both_circuits_generate_witness() {
|
||||
let pol_inputs_raw = std::fs::read_to_string(inputs::POL.as_path()).unwrap();
|
||||
let pol_witness_input = lbc_pol_sys::PolWitnessInput::new(pol_inputs_raw).unwrap();
|
||||
|
||||
// Each sys crate compiles a copy of the same C++ runtime (loadCircuit, get_size_of_witness,
|
||||
// ...) under identical symbol names. When two crates are linked into the same binary, the
|
||||
// linker silently keeps one definition of each symbol, so one circuit ends up using the
|
||||
// other's size constants — corrupting dat parsing and causing a SIGSEGV.
|
||||
// This test reproduces the conflict by calling generate_witness on both circuits in the
|
||||
// same binary.
|
||||
let _pol_witness = lbc_pol_sys::generate_witness(&pol_witness_input);
|
||||
|
||||
let inputs_json_raw = std::fs::read_to_string(inputs::POQ.as_path()).unwrap();
|
||||
let inputs_json = PoqWitnessInput::new(inputs_json_raw).unwrap();
|
||||
let poq_result = lbc_poq_sys::generate_witness(&inputs_json);
|
||||
assert!(poq_result.is_ok());
|
||||
}
|
||||
}
|
||||
Loading…
x
Reference in New Issue
Block a user