--- Log opened Sun Aug 09 00:00:46 2020 01:34 -!- Kiminuo [~mix@141.98.103.100] has joined #rust-bitcoin 01:44 -!- Kiminuo [~mix@141.98.103.100] has quit [Quit: Leaving] 02:49 -!- belcher_ [~belcher@unaffiliated/belcher] has joined #rust-bitcoin 02:52 -!- belcher [~belcher@unaffiliated/belcher] has quit [Ping timeout: 260 seconds] 03:03 -!- Beverly8Stokes [~Beverly8S@static.57.1.216.95.clients.your-server.de] has joined #rust-bitcoin 07:49 < stevenroose> elichai2: https://github.com/rust-bitcoin/rust-bitcoin/pull/436 07:57 < stevenroose> elichai2: https://github.com/rust-bitcoin/rust-bitcoin/pull/403 07:57 < elichai2> I'm honestly not 100% sure this is correct, because `Uint128` is little endian, and the loop loads it from the start, shouldn't it be from the end for big endian? 07:58 < elichai2> see https://play.rust-lang.org/?gist=47f7c6e10693fe25faa57d4110b3f1c8 and https://github.com/rust-bitcoin/rust-bitcoin/blob/06b301f3c11e42309cf26b284153cc0b4c1564d4/src/util/uint.rs#L469 07:58 < elichai2> the first one is the low bits (https://github.com/rust-bitcoin/rust-bitcoin/blob/06b301f3c11e42309cf26b284153cc0b4c1564d4/src/util/uint.rs#L40) so it should be the same as `a_u128 as u64` 08:02 < dr-orlovsky> Aren't https://github.com/rust-bitcoin/rust-bitcoin/pull/436/files#diff-1e5d3d4beff6bc1cfc7a67c9fb13c85cR108-R109 and https://github.com/rust-bitcoin/rust-bitcoin/pull/436/files#diff-144d3ba9789e2ea5edd53ca2bba38baeR468-R475 that this works the same way like for other smaller int types? 08:02 < elichai2> not sure I understand 08:04 < dr-orlovsky> neither I am... Could it be that other `define_slice_to_be` functions are broken as well? 08:05 -!- Beverly8Stokes [~Beverly8S@static.57.1.216.95.clients.your-server.de] has quit [Ping timeout: 256 seconds] 08:06 < elichai2> See: https://pastebin.com/raw/6HcAi0Ew 08:06 < elichai2> dr-orlovsky: no, I think you just need to insert into the slice in reverse 08:07 < elichai2> because the Uint* is LE and you're inserting BE 08:08 < dr-orlovsky> you mean here? https://github.com/rust-bitcoin/rust-bitcoin/pull/436/files#diff-144d3ba9789e2ea5edd53ca2bba38baeR95-R97 08:09 < elichai2> yeah 08:09 < dr-orlovsky> I think we also need some other test that will be able to detect situations like that 08:10 < elichai2> I think the fuzzing against `u128` is a great way to test this, we should add more fuzzing functions here 08:11 < elichai2> easiest solution would be to replace `slice[n]` with `slice[$n_words-n-1]` 08:13 < elichai2> dr-orlovsky: so if you could please fix that and add `from_be_bytes` to https://github.com/rust-bitcoin/rust-bitcoin/blob/master/fuzz/fuzz_targets/uint128_fuzz.rs 08:16 < dr-orlovsky> doing that 08:16 < elichai2> Thanks :) 09:00 < dr-orlovsky> why hfuzz_input is a part of the git history? 09:03 < dr-orlovsky> elichai2: rebased and force-pushed new version 09:19 < BlueMatt> maybe an elichai2 question - so if I have some rust-c-bindings - which is closer to a C pointer *mut or *const? AFAIU, the right answer is *const, not *mut, because *mut is a C-restrict-pointer, and *const is a C-const-pointer. neither is fully correct, but casting away const is valid in C if there exists a non-const version, whereas casting away restrict, I dont know what the thinking is there, but I'd think compiler could optimize away 09:19 < BlueMatt> your cast. 09:22 < BlueMatt> of course its all nonsense cause if you cast a C pointer (either way) to &mut, you invoke undefined behavior 09:22 < BlueMatt> at least the best I understand it 09:30 < BlueMatt> well, I'mma ask the cbindgen folks https://github.com/eqrion/cbindgen/issues/561 09:55 < elichai2> BlueMatt: regular C pointer(ie unsgiend char*) is *mut (ie *mut u8) 09:56 < elichai2> As far rust is concerned they're equivilant memory/alias wise, so you can cast back and forth without thinking too much 09:56 < BlueMatt> right, but doesnt rustc happily add a noalias (aka restrict) to that 09:56 < elichai2> No, only to references 09:56 < elichai2> Raw pointers have no meaning in rust 09:56 < BlueMatt> ah, ok, so when you cast *mut to &mut you're fucked? 09:57 < elichai2> You could be, yeah, for example if you end up with two &mut 09:57 < BlueMatt> but, eg, how does llvm propagate noalias? 09:58 < BlueMatt> like, if I have a pointer and cast it to &mut, then return, noalias is then only applied for the duration of the &mut existing? 09:58 < elichai2> I think yeah, but llvm semantics are sketchy 09:59 < elichai2> I think as long as you don't violate rust abstract machine rules you should be fine, but exact llvm details try to ask RalfJung in unsafe-code-guidelines github or Zulip 09:59 < BlueMatt> ugh, man, makes me feel like calling Rust from C is really impossible. 09:59 < elichai2> Why? 09:59 < elichai2> It should be fine if you're careful 09:59 < BlueMatt> i mean sudden spontaneous noalias..... 09:59 < BlueMatt> heh, right, i mean sure, if you're writing something super small, I'd agree thats practical 10:00 < BlueMatt> but, ummm, mapping rust-lightning to C is +22k LoC 10:00 < BlueMatt> plz2review :( 10:00 < BlueMatt> with a ton of ((&asdf as *const) as *mut) 10:01 < BlueMatt> cause, like, once you hand it over the wall, you dont know wtf its gonna do. 10:04 < elichai2> Yeah, you need to review the C and rust together, you can't really look only at one side 10:04 < BlueMatt> like, in general, if you want to have one type for things, you either get ((&asdf as *const) as *mut) or &mut *((&mut asdf as *const) as *mut) 10:04 < BlueMatt> both seem terrible 10:05 < BlueMatt> right, well our C bindings arent intended for direct use, only as a base for building other languages on top, but by the time you've done that, you've probably lost any concept of noalias, really 10:06 < BlueMatt> hence my comment that it seems to me nontrivial anything is rather impossible to expose to non-rust :/ 10:07 < BlueMatt> more concretely, we want a struct CRustObj { pointer: *mut RustObj, _its_really_a_ref_dont_free_it: bool } 10:07 < BlueMatt> should the pointer be *mut (and then we do (&asdf as *const) as *mut) or *const (and then we do &mut *((&mut asdf) as *const) as *mut) 10:09 < elichai2> generally you shouldn't free pointers that you didn't get from malloc, and rust supports arbitrary allocators 10:09 < BlueMatt> hmm, no, the freeing isnt really the issue, I've got that under control :) 10:09 < BlueMatt> (the free bool just indicates to the Rust Drop fn whether we should Box::from_raw() or just ignore) 10:10 < elichai2> anyway, the `noalias` is only emitted on rust pointers. generally as long as you make sure that any `&mut T` is unique, and that you don 10:11 < elichai2> *and that you don't write into pointers obtained from `&T` you should be fine 10:11 < BlueMatt> right, thats vaguely what I'm going based on, it would just be nice to have that be, like, a guarantee. 10:11 < BlueMatt> thanks, though 10:11 < BlueMatt> basically you're saying *const and *mut are the same compilation-wise :) 10:11 < elichai2> ie `c_func(&a as *const u8 as *mut u8)` `void c_func(uint_8t* a) {*a = 5;}` is UB (you probably know that but saying anyway :) ) 10:12 < elichai2> yes 10:12 < BlueMatt> right, sure. 10:12 < elichai2> also, it's really important in rust how did you obtain the pointer, ie you can't use`&a[0]` to access any other members of the array 10:13 < BlueMatt> you can if its slice.as_mut_ptr(), no? 10:13 < elichai2> well that does `&mut a as *mut [u8] as *mut u8` it obtains a pointer to the whole array/slice not to the first element 10:14 < elichai2> although they currently don't use this for optimizations yet(e.g. https://godbolt.org/z/q3Y5T8) 10:16 < elichai2> something interesting i've done in the past, is used `c2rust` to translate secp to rust, and then ran miri on rust-secp256k1 10:16 < BlueMatt> hmm, right. 10:16 < BlueMatt> what *is* the correct way to get a pointer to the slice region? 10:16 < BlueMatt> ie https://github.com/rust-bitcoin/rust-lightning/pull/618/files#diff-672a8bbc5b731a958a1d6b0e3a3ec51fR268 10:17 < elichai2> `as*_ptr()` is always best if you want the whole slice, and you can do `let ptr = slice[0..3].as_ptr()` 10:18 < elichai2> as for that example. A. you can directly construct Vecs https://doc.rust-lang.org/std/vec/struct.Vec.html#method.from_raw_parts 10:18 < BlueMatt> right, but how do I do vec to_raw_parts 10:18 < BlueMatt> without hitting a nightly-only api :p 10:19 < elichai2> B. you can directly destruct vecs on nightly if you're using https://doc.rust-lang.org/std/vec/struct.Vec.html#method.into_raw_parts otherwise call `((me.as_mut_ptr(), me.len(), me.capacity()))` 10:19 < elichai2> and then forget the vec 10:19 < elichai2> altough now you're stuck also with capacity hmm 10:19 < BlueMatt> right 10:19 < BlueMatt> which sucks 10:19 < BlueMatt> i mean in theory you could shrink_to_fit() and then do it? 10:19 < BlueMatt> of course that should be the same as into_boxed_slice() 10:19 < elichai2> yep, seems like that's what into_boxed_slice does https://doc.rust-lang.org/src/alloc/vec.rs.html#678 10:20 < BlueMatt> which is what the above code relies on 10:20 < elichai2> so do something like that: 10:21 < elichai2> oh wait, just replace `CVecTempl { datalen, data: unsafe { (*data).as_mut_ptr() } }` with `CVecTempl { datalen, data: data as *mut u8 }` 10:21 < BlueMatt> https://github.com/rust-bitcoin/rust-lightning/blob/643c1b4d783fe319fc7b841b7f2fb4ddf856f713/lightning-c-bindings/src/c_types/mod.rs#L270-L295 10:21 < elichai2> no need for that deref 10:21 < BlueMatt> I think you do need it cause you have to release the pointer without Drop 10:21 < BlueMatt> hence the Box::into_raw 10:21 < elichai2> or `as *mut T` for that matter 10:21 < elichai2> BlueMatt: yeah I meant after into_raw 10:21 < BlueMatt> in practice I think its the same, but the point is data should point at the data, not at the Box itself 10:22 < BlueMatt> so C can use it as a pointer/array 10:22 < elichai2> well `into_raw` gives you a pointer to the data AFAIK 10:22 < BlueMatt> it does in practice, I think but I dont know that thats guaranteed? 10:22 < elichai2> `The pointer will be properly aligned and non-null.` 10:23 < BlueMatt> but Box::into_raw(Box<[]>) returns *mut [] 10:23 < BlueMatt> not *mut T 10:24 < elichai2> yeah, because it's a rust pointer, and rust pointers to slice contain ptr to the first element + length, by casting to *mut T you remove the length metadata 10:24 < elichai2> I'll try to find it written somewhere 10:24 < BlueMatt> right. 10:24 < BlueMatt> which is what I want :) 10:25 < BlueMatt> hence the deref + as_mut_ptr(). 10:25 < BlueMatt> the len is stored, too, so its not lost 10:25 < elichai2> FWIW we do that rust-secp 10:26 < elichai2> https://github.com/rust-bitcoin/rust-secp256k1/blob/master/src/context.rs#L147 10:26 < BlueMatt> huh, it doesnt compile when I drop it, but ok :) 10:28 < BlueMatt> (also note that the comment there that drop will short-curcuit itself with datalen == 0 seems to be wrong, I get all kinds of fun strange heisenbugs when I dont check it, but I'm actually somewhat sure its a miscompilation kinda sorta...) 10:28 < elichai2> when you drop what? https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=f817c6e2985e1841ffd1c96748862d81 10:29 < elichai2> yeah you can't have null references 10:29 < elichai2> period. even ZST aren't null 10:29 < elichai2> also, maybe replace `std::slice::from_raw_parts_mut` with `std::ptr::slice_from_raw_parts_mut` 10:29 < BlueMatt> namely, when I do a conversion from CVecTempl to a CVec, the last step before the drop is set data to null and datelen to 0, which should short-curcuit the drop, cause thats how slice.to_raw_parts() works when the slice is 0-length 10:29 < elichai2> no need for references when they're not needed 10:30 < BlueMatt> errr, slice.as_mut_ptr() 10:30 < BlueMatt> it returns null when you have 0-length 10:30 < BlueMatt> so, presumably, from_raw_parts is also ok on null with 0-length 10:30 < BlueMatt> at least according to docs 10:30 < BlueMatt> (even though in theory thats not allowed) 10:31 < elichai2> it should never return null I think 10:31 < BlueMatt> it has to Vec::new() is guaranteed to not allocate, so its data-ptr must be null 10:31 < BlueMatt> so &vec[..] is always going to be (null, 0) 10:32 < BlueMatt> I do agree with you that this is invalid bogus garbage (and it crashes sometimes!), but its also what rust does 10:32 < elichai2> I don't think you're right 10:32 < elichai2> it should *always* return aligned pointer 10:32 < elichai2> if it's empty it will return a dangling aligned pointer 10:32 < BlueMatt> sometimes it returns 0x8 10:32 < BlueMatt> which is even worse 10:32 < elichai2> but never null afaik 10:32 < BlueMatt> 0x8 is effectively-null 10:33 < BlueMatt> i mean when I run things I see {:p} print 0x8 all the time, so I'm pretty sure it does that sometimes 10:33 < elichai2> but it's not null :) because you should be able to construct a slice reference from it. and slices are llvm `nonnull` 10:33 < elichai2> why is 0x8==null? 10:33 < BlueMatt> its in the null page 10:33 < elichai2> oh lol 10:34 < elichai2> it's because of how they construct dangling pointer 10:34 < BlueMatt> and its also definitely no derefable 10:34 < BlueMatt> I thought the point of nonnull is that you can deref it 10:34 < elichai2> `align_of::() as usize as *const T` 10:34 < BlueMatt> yeeshwtf 10:34 < elichai2> BlueMatt: no, nonnull and `dereferenceable` aren't the same 10:35 < BlueMatt> huh. llvm is insane 10:35 < BlueMatt> wait, so what *does* nonnull allow optimization-wise? 10:35 < BlueMatt> where its in the null page/non-deref'able but also not-0 10:35 < elichai2> evalr: std::mem::size_of::<&usize>() == std::mem::size_of::>() 10:35 -evalr:#rust-bitcoin- true 10:35 < elichai2> this is what it allows :) 10:36 < BlueMatt> right, no, i meant optimization-wise in terms of llvm compilation 10:36 < BlueMatt> not at the rust layer 10:36 < elichai2> good question 10:37 < BlueMatt> this confuses me greatly - free(0) is guaranteed to do nothing, free(8) is not 10:37 < BlueMatt> and rust now uses malloc/free directly for these things :( 10:37 < BlueMatt> or does rust's alloc wrapper check for 8 or other null-page values? 10:37 < elichai2> rust doesn't free ZST pointers 10:37 < elichai2> or allocates them 10:37 < BlueMatt> ah, ok, so its alloc checks 10:37 < BlueMatt> lol 10:37 < elichai2> as they are ZST :) 10:39 < BlueMatt> ok, so you definitely just solved the issue i was banging my head against for hours yesterday, which is nice 10:39 < BlueMatt> but i still hate rust <-> c stuff 10:40 < BlueMatt> (of course it doesnt help when the issue shows up as "Illegal Instruction" and nothing can get a useful backtrace and it dissapears if you put an eprintln!() in the offending function....) 10:41 < elichai2> lol yeah these are the hardest to debug 10:42 < elichai2> yeah the whole setting to null isn't the right thing IMO 10:43 < elichai2> you should `mem::forget(self)` 10:43 < elichai2> or use ManuallyDrop 10:43 < BlueMatt> nah, the setting to null this is used also cause we need something that C++ can do to set the object to when std::move() kicks in 10:43 < BlueMatt> and memset(self, 0, sizeof(self)) is the easiest by far 10:44 < elichai2> so I guess you need a condition when dropping (and when calling `into_rust` so you won't create null references) 10:45 < BlueMatt> right, which I added and it fixes the issue 10:45 < BlueMatt> (in this case datalen == 0) 10:45 < elichai2> :) 10:45 < BlueMatt> but I wasnt sure that this *was* the issue, and didnt want to "hey it fixed the heisenbug"-"fix" it 10:45 < BlueMatt> cause thats an easy way to leave heisenbugs in 10:45 < elichai2> on the into_rust side maybe check for null self.data instead, because it might be valid ZST 10:46 < BlueMatt> anyway, if you get a chance, I pushed a new version of the C bindings for rust-lighting, I'd greatly appreciate at least a careful look at c_types/mod.rs and c_types/derived.rs 10:46 < BlueMatt> they arent big files, even if the overall auto-generated stuff is quite large 10:46 * BlueMatt -> family time 10:46 < elichai2> which files aren't autogenerated? c_types/mod.rs and c_types/derived.rs? 10:47 < elichai2> Enjoy :) 10:47 < BlueMatt> c_types/mod.rs isnt autogenerated 10:47 < BlueMatt> derived *is*, but also could use careful looking 10:47 < BlueMatt> and its easier to review the generated stuff than the generator lol 10:47 < BlueMatt> the generator is a mess 10:48 < BlueMatt> if you want to 5-second glance through some of the gerated garbage that would be cool, but its all just miles of stuff following the same pattern 15:00 -!- DeanWeen [~dean@gateway/tor-sasl/deanguss] has joined #rust-bitcoin 15:01 -!- Dean_Guss [~dean@gateway/tor-sasl/deanguss] has quit [Remote host closed the connection] 22:48 -!- willcl_ark [~quassel@cpc123762-trow7-2-0-cust7.18-1.cable.virginm.net] has quit [Ping timeout: 256 seconds] 22:56 -!- willcl_ark [~quassel@cpc123762-trow7-2-0-cust7.18-1.cable.virginm.net] has joined #rust-bitcoin --- Log closed Mon Aug 10 00:00:45 2020