P2AffinesAdd crashes when executing in alphine docker image #142
Comments
|
Is it 100% reproducible or does it fail occasionally? Since you mention it explicitly, is it actually alpine-specific, or is it so you simply happen to use it? Since you mention it explicitly, were previous go versions working, or is 1.18 simply the first go version you compile blst with? It's not impossible to imagine that the problem is due to an uncertainty introduced by Is caller's code publicly available? I've found go-photon on github, but there is no VerifyBlocks there... |
|
It is 100% reproducible and always fails. I have tried the same docker image in both EC2 host and my macbook, they both fail. I have also tried docker image based on "golang:1.18.8-bullseye", it is fine. The crashing docker image is based on "golang:1.18-alpine" with "apk --no-cache --update add gcc musl-dev build-base bash git" |
|
Our repo is not ready to open to public yet as it is still very rough and WIP. I paste the function below. Let me know if you need any further information. // VerifyBlocks used to batch verify POR proofs of multiple blocks
func (v *Verifier) VerifyBlocks(sigs, blks [][]byte, indexes []uint32) error {
blkSize := len(blks)
if len(sigs) != blkSize || len(indexes) != blkSize {
return ErrBlockSizeMismatch
}
mus := make([]*big.Int, v.spb)
for j := range mus {
mus[j] = big.NewInt(0)
}
for i := range blks {
for j := uint32(0); j < v.spb; j++ {
s := BlockSector(blks[i], j)
mus[j] = new(big.Int).Add(mus[j], s)
}
}
sigmas := make([]*blst.P2Affine, blkSize)
for i, sig := range sigs {
sigmas[i] = new(blst.P2Affine).Uncompress(sig)
}
sgm := blst.P2AffinesAdd(sigmas)
var aggs []*blst.P2Affine
for _, index := range indexes {
aggs = append(aggs, BlockTag(v.oid, index).ToAffine())
}
for i, u := range v.us {
aggs = append(aggs, blst.P2Mult(u, ReduceToScalar(mus[i])))
}
p1, err := v.pk.GetRaw()
if err != nil {
return err
}
a := blst.Fp12MillerLoop(sgm.ToAffine(), blst.P1Generator().ToAffine())
b := blst.Fp12MillerLoop(blst.P2AffinesAdd(aggs).ToAffine(), p1)
if !blst.Fp12FinalVerify(a, b) {
return ErrVerificationFailure
}
return nil
} |
So it sounds like a musl vs glibc thing... Not that it totally explains the failure on alpine, but at least there is some hint... However, I see no reason for why you wouldn't be able to restructure your code to not rely on an array of pointers. And it would be more efficient too. So instead of |
|
How many points are you adding? When going over 1024 G2 points or 2048 G1 points I have a crash as well (mratsim/nim-blscurve#1 (comment)) It appears here: https://github.com/supranational/blst/blob/6382d67/src/bulk_addition.c#L148-L149 The C-API seems to have a double pointer/array indirection, could it be that the code assumes the points are in batches of 1024/2048? |
|
I'm not convinced that this is the same problem. The [original] problem is believed to have everything to do with how Go pointers are handled on the Cgo interface. Or more specifically Go array of Go pointers. And suggestion was to restructure the application code to avoid it. As for the 2nd question. No, C-API doesn't expect specific batch sizes. But it does claim temporary storage on the stack to accommodate the internal batching, [up to] 288KB. In Go it's not a problem, because C subroutine is executed on own stack, one with "normal" limit, where 288KB are readily available. But can you be hitting your stack ceiling, @mratsim? |
|
Just in case for reference, what's up with internal batching. The algorithm in question requires temporary storage, by where to get is from? blst doesn't do heap allocations, so we take it from stack. But it's not safe to assume that arbitrary amount of stack is available, so we iterate in smaller steps. On the other hand we want to amortize inversion costs across larger amount of points. So where to draw the line? Somewhere within 1-2-3% performance penalty... |
Though I think that the evaluation was done with slower inversion subroutine... In other words it might turn out that reducing the internal batch size could be acceptable... |
|
Long reply, a bit off-topic regarding OP problem then.
It's unlikely, I'm not using MSVC which limits stack to 1MB, my ulimit -a shows 8MB
I don't think anyone has issue with internal batching, in particular quoting myself in my own implementation: https://github.com/mratsim/constantine/blob/53a5729/constantine/math/elliptic/ec_shortweierstrass_batch_ops.nim#L269-L286
The part that is confusing to me is the function signature / double pointer indirection, why void blst_p2s_add(blst_p2 *ret, const blst_p2_affine *const points[], size_t npoints);over void blst_p2s_add(blst_p2 *ret, const blst_p2_affine const points[], size_t npoints); |
For flexibility. Note that it traverses the array of pointers till NULL, after which it treats the last pointer as an array of points. So that if you have an array of points you can simply |
The remark was more for the OP :-) As already said "just in case" :-) |
Yes, but the main thread can start threads with smaller stacks... I'm not saying that it does, but it's the only thing I can think of give the circumstances. Can you collect the failing address and compare it to the current stack pointer? Just in case, I can pass more points to the subroutines in question... |
I am getting crash on Cgo execution of P2AffinesAdd. Wonder if you guys have any advice?
Signature and verification are fine so I guess it only affects certain APIs.
Docker base: golang:1.18-alpine
Crash stacktrace:
The text was updated successfully, but these errors were encountered: