search

Consensys / gnark-crypto

gnark-crypto provides elliptic curve and pairing-based cryptography on BN, BLS12, BLS24 and BW6 curves. It also provides various algorithms (algebra, crypto) of particular interest to zero knowledge proof systems.
Apache License 2.0
495 stars 160 forks source link

bug: When dynamic linking, R15 may be clobbered by a global variable access #468

Closed ale-linux closed 8 months ago

ale-linux commented 10 months ago

Compilation fails with dynamic linking. The issue is linked to https://github.com/Consensys/gnark-crypto/pull/112 and https://github.com/Consensys/gnark-crypto/pull/113

Description

When dynamic linking, R15 is used to access global variables so an asm compilation unit that makes use of both global variables and R15 will fail to compile as is the case.

Expected Behavior

Compilation should succeed when building with linkshared

Actual Behavior

Compilation fails.

Possible Fix

See https://github.com/Consensys/gnark-crypto/pull/114

Steps to Reproduce

  1. checkout main
  2. run
$ go build -linkshared ./...
# github.com/consensys/gnark-crypto/ecc/bn254/fr/mimc/test_vectors
/home/aso/local/go/pkg/tool/linux_amd64/link: cannot implicitly include runtime/cgo in a shared library
# github.com/consensys/gnark-crypto/ecc/bls12-377/fp
asm: element_mul_amd64.s:93: when dynamic linking, R15 is clobbered by a global variable access and is used here: 00012 (/home/aso/w/Projects/OBC/Workspace/src/github.com/Consensys/gnark-crypto/ecc/bls12-377/fp/element_mul_amd64.s:93)      MULXQ   R10, R14, R15
asm: assembly failed
# github.com/consensys/gnark-crypto/ecc/bls12-378/fp
asm: element_mul_amd64.s:93: when dynamic linking, R15 is clobbered by a global variable access and is used here: 00012 (/home/aso/w/Projects/OBC/Workspace/src/github.com/Consensys/gnark-crypto/ecc/bls12-378/fp/element_mul_amd64.s:93)      MULXQ   R10, R14, R15
asm: assembly failed
# github.com/consensys/gnark-crypto/ecc/bls12-381/fp
asm: element_mul_amd64.s:93: when dynamic linking, R15 is clobbered by a global variable access and is used here: 00012 (/home/aso/w/Projects/OBC/Workspace/src/github.com/Consensys/gnark-crypto/ecc/bls12-381/fp/element_mul_amd64.s:93)      MULXQ   R10, R14, R15
asm: assembly failed
# github.com/consensys/gnark-crypto/ecc/bw6-633/fp
asm: element_mul_amd64.s:106: when dynamic linking, R15 is clobbered by a global variable access and is used here: 00009 (/home/aso/w/Projects/OBC/Workspace/src/github.com/Consensys/gnark-crypto/ecc/bw6-633/fp/element_mul_amd64.s:106)      MULXQ   (R12), R14, R15
asm: assembly failed

Context

Linking gnark-crypto as a go plugin or shared library

Your Environment

gbotrel commented 10 months ago

Hi -- I think the best workaround for now if you are not in a perf critical workflow, is to compile with -tags=purego .

Could also be that we unnecessarily force users to import more packages than they need; do you need to link with all gnark-crypto or some specific packages only?

ale-linux commented 10 months ago

Thanks @gbotrel . For now I think it's acceptable to take this route but it would be good to find a long-term solution. As you can see, we're pulling gnark-crypto from half of Hyperledger's projects now :grin:

gbotrel commented 10 months ago

Are you? :) Well that's where I'm curious -- do you need to import github.com/consensys/gnark-crypto/ecc/bls12-378/fp and github.com/consensys/gnark-crypto/ecc/bw6-633/fp for example, or are they being "vendored" or are you, as a user being forced to import all of it because in some place we accidentally imported more than we should have??

ale-linux commented 10 months ago

we use 3 curves (bls12-377, bls12-381, bn254) directly in mathlib which is in turn used in quite a few places (idemix, fabric, aries-framework-go). See the latest issue in fabric.

ale-linux commented 10 months ago

Just a clarification: in the comments to the issue I linked, you'll see the compilation failure text relating to https://github.com/kilic/bls12-381/issues/42, but https://github.com/Consensys/gnark-crypto/issues/468 equally affects that build.

gbotrel commented 10 months ago

@ale-linux I could NOT reproduce with Go 1.21.4 on Ubuntu, but could reproduce with go1.20.8.linux-amd64. Seems this part of Go 1.21 releases notes helps:

The verifier that checks for incorrect uses of R15 when dynamic linking on amd64 has been improved.
ale-linux commented 10 months ago

a-ha... yeah in many cases the clobbering actually did not take place because for instance the access to the global variable took place before R15 was actually used to store temp data. I would say we're good to go then - we're building with purego till we upgrade to go1.21. Thanks!