--- Log opened Sat Mar 30 00:00:31 2019
00:46 -!- sipa [~pw@gateway/tor-sasl/sipa1024] has quit [Ping timeout: 256 seconds]
00:52 -!- sipa [~pw@gateway/tor-sasl/sipa1024] has joined #secp256k1
04:16 -!- Netsplit *.net <-> *.split quits: luke-jr, jonasschnelli, gmaxwell, niftynei, roconnor
04:16 -!- gmaxwell [gmaxwell@mf4-xiph.osuosl.org] has joined #secp256k1
04:17 -!- Netsplit over, joins: roconnor
04:17 -!- gmaxwell is now known as Guest90059
04:17 -!- Netsplit over, joins: luke-jr
04:17 -!- jonasschnelli [~jonasschn@2a01:4f8:172:10da::2] has joined #secp256k1
04:18 -!- Netsplit over, joins: niftynei
04:19 -!- instagibbs [~instagibb@pool-100-15-135-248.washdc.fios.verizon.net] has quit [Ping timeout: 246 seconds]
04:23 -!- instagibbs [~instagibb@pool-100-15-135-248.washdc.fios.verizon.net] has joined #secp256k1
09:21 -!- Guest90059 [gmaxwell@mf4-xiph.osuosl.org] has quit [Changing host]
09:21 -!- Guest90059 [gmaxwell@wikimedia/KatWalsh/x-0001] has joined #secp256k1
09:21 -!- Guest90059 is now known as gmaxwell
12:59 -!- jonasschnelli [~jonasschn@2a01:4f8:172:10da::2] has quit [Changing host]
12:59 -!- jonasschnelli [~jonasschn@unaffiliated/jonasschnelli] has joined #secp256k1
13:02 < sipa> https://github.com/bitcoin-core/secp256k1/pull/607
13:28 < gmaxwell> sipa: why multiply instead shifting the sizeof?
13:29 < sipa> that's even better
13:48 < roconnor> BTW, when doing math like: https://github.com/bitcoin-core/secp256k1/blob/1b8e50570c7227d958989d16fafa40313073a462/src/ecmult_impl.h#L962-L963 are we sure that the alignment of all these various kinds of structures will allow the maximum needed allocations to fit in the computed number of chars?
13:51 < sipa> sizeof includes alignment padding, so i think yes?
13:54 < gmaxwell> sipa: I think roconnor was asking, e.g. sizeof(int)=4 sizeof(int*)=8  but a struct of int,int* has size 16 due to padding. so if you try to compute the size of a struct yourself you'll mess it up.
13:56 < gmaxwell> I audited that code for that before, but then again I also didn't catch (1<<foo) * bar.
13:59 < roconnor> ya.  I'm not certain how scratch space works but you you alloc the size given by secp256k1_pippenger_scratch_size, for example, and then try to do pointer arithmetic and casting to pull out various arrays and structures from that allocation, you may accidentally create unaligned pointers.
14:00 < roconnor> but I'm kinda speculating on how the scratch space works.
14:00 < sipa> roconnor: the scratch allocation code actually rounds everything up to alignment size
14:00 < sipa> (which is guessed when unavailable, but no practical systems need alignment over 16 bytes afaik)
14:01 < roconnor> In that case, the issue would be that you haven't allocated enough room.
14:01 < roconnor> because you haven't rounded during the scratch size allocation computation.
14:02 < sipa> roconnor: the allocation code includes a per-object overhead to account for that
14:02 < roconnor> (e.g. https://github.com/bitcoin-core/secp256k1/blob/1b8e50570c7227d958989d16fafa40313073a462/src/ecmult_impl.h#L998-L999 I see int arrays and pointer both being allocted form scratch.
14:02 < sipa> and you tell it how many objects you'll need
14:03 < roconnor> I mean https://github.com/bitcoin-core/secp256k1/blob/1b8e50570c7227d958989d16fafa40313073a462/src/ecmult_impl.h#L962 doesn't round up the sizeof(int)
14:04 < roconnor> and it is an odd-sized int array.
14:04 < roconnor> anyhow if you are fine with all this, that's okay with me.
14:04 < sipa> why does it nees to round up?
14:05 < sipa> the rounding is done when allocating the blob, and when allocating space inside of ot
14:06 < roconnor> because if ints are 32-bit alligned and pointers are 64-bit aligned, then the allocator will round((WNAF_SIZE(bucket_window+1)+1)*sizeof(int)) when allocating an int[WNAF_SIZE(bucket_window+1)+1] array.  I.e. the allocator may give out more memory than you've computed here for the maximum memory needed.
14:06 < roconnor> and then later run out of space.
14:06 < sipa> i think you're missing something, but i don't know what
14:06 < roconnor> okay.
14:06 < roconnor> np.
14:07 < sipa> but you also have a tendency to find issues nobody else sees, so i'd like to understand it
14:07 < roconnor> well let me try to give a simplified example.
14:08 < roconnor> we are doing computation that needs memory for 1 int pointer and an array of 3 ints.
14:08 < sipa> okay
14:08 < roconnor> We are on a system with 64-bit pointers with 64-bit alignment and 32-bit integers with 32-bit alignment.
14:09 < sipa> okay
14:09 < roconnor> We compute our needed scratch space as (sizeof(*int) + 3*sizeof(int)) and we get 8 + 3*4 = 20.
14:09 < sipa> correct
14:09 < sipa> but that will be rounded up by 2*16 for alignment
14:10 < sipa> (if the assumed max alignment is 16)
14:10 < gmaxwell> sipa: roconnor is squaking I think because there is a line adding the size requirements of hetrogenious types and it contains nothing about alignment in it.
14:10 < roconnor> let me ammend my example then.
14:11 < roconnor> we are doing computation that needs memory for 1 int pointer and two array of 3 ints.
14:11 < roconnor> We compute our needed scratch space as (sizeof(*int) + 3*sizeof(int) + 3*sizeof(int)) and we get 8 + 3*4 + 3*4 = 32.
14:11 < sipa> and it'll be incremented by 3*16
14:11 < roconnor> which is a perfectly round value.
14:11 < roconnor> oh it will?
14:11 < roconnor> okay
14:12 < sipa> https://github.com/bitcoin-core/secp256k1/blob/master/src/scratch_impl.h#L44
14:12 < sipa> https://github.com/bitcoin-core/secp256k1/blob/master/src/scratch_impl.h#L51
14:15 < roconnor> so at https://github.com/bitcoin-core/secp256k1/blob/1b8e50570c7227d958989d16fafa40313073a462/src/ecmult_impl.h#L991  the PIPPENGER_SCRATCH_OBJECTS value is 6 because there are 6 allocations below.
14:15 < sipa> yup
14:16 < roconnor> Got it.  Sorry for the trouble.
14:16 < sipa> no worries! happy someone looks at this
16:06 < gmaxwell> sipa: shift has weaker binding than addition.
16:06 < gmaxwell> (this is just one of those things where it's not clear what the language designers were thinking, and you just need to remember it)
16:07 < sipa> gah, now i look like a fool :)
16:08 < gmaxwell> sorry I didn't review it faster.
16:08 < gmaxwell> :)
16:09 < gmaxwell> don't feel too silly, if someone that gave me nitty paren changes told me to fix a precidence I'd probably also disagree without thinking. :P
16:11 < gmaxwell> I wonder if any of our tests would have caught that error?
16:12 < sipa> yes. https://travis-ci.org/bitcoin-core/secp256k1/builds/513590794
21:24 -!- luke-jr [~luke-jr@unaffiliated/luke-jr] has quit [Ping timeout: 250 seconds]
21:29 -!- luke-jr [~luke-jr@unaffiliated/luke-jr] has joined #secp256k1
23:36 -!- sipa [~pw@gateway/tor-sasl/sipa1024] has quit [Remote host closed the connection]
23:43 -!- sipa [~pw@gateway/tor-sasl/sipa1024] has joined #secp256k1
23:53 < midnightmagic> oh, hry roconnor  :-)
--- Log closed Sun Mar 31 00:00:31 2019