github.com/moarvm/moarvm | IRC logs at colabti.org/irclogger/irclogger_logs/moarvm
Set by AlexDaniel on 12 June 2018.
01:17 lizmat left 01:30 lizmat joined, p6bannerbot sets mode: +v lizmat 02:36 lucasb left 06:13 robertle left 06:42 Kaiepi left 06:46 Kaiepi joined 06:47 p6bannerbot sets mode: +v Kaiepi 07:03 domidumont joined, p6bannerbot sets mode: +v domidumont 07:18 lizmat left
Geth MoarVM: 6455d1f6ac | (Ben Davies)++ | src/platform/sys.c
Fix double free in platform/sys.c
07:18
MoarVM: d7a4c4c56b | niner++ (committed using GitHub Web editor) | src/platform/sys.c
Merge pull request #1008 from Kaiepi/cpus

Fix double free in platform/sys.c
07:29 lizmat joined, p6bannerbot sets mode: +v lizmat 07:35 travis-ci joined, p6bannerbot sets mode: +v travis-ci
travis-ci MoarVM build failed. niner 'Merge pull request #1008 from Kaiepi/cpus 07:35
travis-ci.org/MoarVM/MoarVM/builds/464205207 github.com/MoarVM/MoarVM/compare/f...a4c4c56b7f
07:35 travis-ci left
nine The failure is apparently due to a label key conflict. It tries to reference a label that hasn't been added to the frame yet. This sounds suspiciously like the 32 bit problem reported by dogbert++ 07:55
Since the label key is just the nqp::objectid($label), I wonder: shouldn't that id be unique? 07:56
timotimo when you request the objid of something, it gets "pinned" 07:57
by getting a permanent slot in the gen2 allocated for it
which, i now realize, could perhaps be avoided for mild memory savings
nine And the id is essentially the new permanent memory address.
timotimo aye 07:58
nine So how can that not be unique?
timotimo good question!
nine This build failure shows that clearly the issue also exists on 64 bit architectures. The smaller address space on 32 bit just makes it much more probable.
Oh 07:59
I looked at the wrong end. I audited the code multiple times to make sure the label object is still referenced at the point of the error and it's address hasn't been re-used yet.
But it's entirely possible that a different label in the same frame has lived at the same address before. 08:00
And has been collected in the meantime.
timotimo could try verifying the assumption by putting the objid it got at the start in an attribute and later check if it's still right
hm, re-use of the slot is possible i guess, though i don't know if we perhaps have something to prevent that 08:01
which of course would be a good way to DOS a program i guess
nine So, either I add a destructor to MAST::Label that removes the label from the frame's %!labels lookup hash, or I collect the labels in the frame itself to prevent them from being freed until the frame is done compiling. 08:02
timotimo the latter doesn't seem too hard? 08:03
how many labels would get their lifetime extended by that?
and for how long?
nine Probably all of them until the comp unit is finished. 08:06
timotimo oh, hmm.
nine But labels are also tiny objects consisting of just an int
timotimo fwiw, the object header makes them not quite as tiny :P 08:14
08:20 robertle joined 08:21 p6bannerbot sets mode: +v robertle 10:06 lucasb joined, p6bannerbot sets mode: +v lucasb 10:50 zakharyas joined 10:51 p6bannerbot sets mode: +v zakharyas 10:58 dogbert2_ joined 10:59 p6bannerbot sets mode: +v dogbert2_ 11:06 domidumont left 13:00 domidumont joined, p6bannerbot sets mode: +v domidumont 13:03 robertle left 13:05 robertle joined 13:06 p6bannerbot sets mode: +v robertle 13:14 zakharyas left 13:52 zakharyas joined, p6bannerbot sets mode: +v zakharyas 13:56 robertle left 14:04 robertle joined 14:05 p6bannerbot sets mode: +v robertle 15:05 lizmat left 15:30 lizmat joined, p6bannerbot sets mode: +v lizmat 15:35 robertle left 15:37 robertle joined 15:38 p6bannerbot sets mode: +v robertle 16:18 robertle left 16:26 Geth left 16:31 domidumont1 joined 16:32 p6bannerbot sets mode: +v domidumont1 16:34 domidumont left 16:46 zakharyas left 16:52 domidumont1 left
AlexDaniel please some attention to M#1009 17:01
synopsebot_: help 17:02
anyway: github.com/MoarVM/MoarVM/issues/1009
nine Looks like an endianness issue in the bytecode writer 17:43
timotimo my first thought, too
who made versions a signed integer? :D 17:44
lucasb check if the first bytes are ARMOVM or whatever is the representation of the signature in the other endianness platform 17:55
j/k, dunno if this check would work :) 17:56
github.com/MoarVM/MoarVM/blob/mast...#L145-L146 18:00
ah, the MOARVM test is done before the version test
Would be informative to dump version variable (and maybe MIN_BYTECODE_VERSION and MAX_BYTECODE_VERSION) in those messages 18:01
nine Nah, MoarVM's bytecode has a well defined byte order
I think, writeint is missing support for the endian flag 18:04
18:15 robertle joined, p6bannerbot sets mode: +v robertle
nine Darn....I just stumbled over the sentence "Operations not configured with one of these options will assume native endian." which means that I left too little space for the endian-flags. I'd need 2 bits instead of just 1 20:30
Fixing that requires another coordinated change in nqp and MoarVM 20:32
Unless I work around that by using some high bit, i.e. 0x100000000 20:33
for nqp::const::BINARY_ENDIAN_LITTLE and 0x100000001 for nqp::const::BINARY_ENDIAN_BIG
But then, I'd still need to add nqp::const::BINARY_ENDIAN_LITTLE to all the (many) callers for it to actually have an effect on big endian machines. But at least we'd stay backwards compatible on little endian ones 20:35
timotimo i have a suggestion for you 20:39
the qastcompiler can introspect how many arguments an op was called with
if you just check if there was an "extra" argument, interpret that as the big/little endian marker, otherwise just spit out a call with the default endianness flag 20:40
there ought to already be some ops that work like that
20:41 brrt joined, p6bannerbot sets mode: +v brrt
nine But all existing callers actually need to set the little endian flag 20:50
timotimo in that case make that the default? 20:53
nine The existing callers need it because MoarVM's bytecode is explicitly in little endian. In other cases defaulting to the native endianness makes most sense. 21:03
brrt ohai 21:05
timotimo yo brrt 21:11
brrt \o timotimo 21:19
can i just say that dynasm is an evil piece of code
timotimo well, it has to do a pretty evil job
brrt hmmm 21:20
I've pondered rewriting it in perl
timotimo a friend of mine is going to give a public little talk on x86 assembly in a few days
brrt but. maybe let's not
hehe
yeah, x86 encoding is a bit hairy
anyway, /me -> mario kart
:-)
timotimo have fun!
21:20 brrt left 23:47 MasterDuke joined 23:48 p6bannerbot sets mode: +v MasterDuke 23:51 MasterDuke left 23:55 MasterDuke joined 23:56 p6bannerbot sets mode: +v MasterDuke