github.com/moarvm/moarvm | IRC logs at colabti.org/irclogger/irclogger_logs/moarvm
Set by AlexDaniel on 12 June 2018.
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
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
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
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
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
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!