00:15 evalable6 joined 00:32 P6Bot joined
MasterDuke any idea why the conditional here github.com/MoarVM/MoarVM/blame/mas...#L265-L266 is `>=`, not `>`? 00:51
samcv MasterDuke, becuase we want to resize before we run out 00:58
if it was > then we'd already be overrunning the buffer
MasterDuke but if it's equal, we're just resizing to the same size 00:59
samcv hmm i see your point 01:00
geekosaur ^
MasterDuke geekosaur: ?
01:01 P6Bot joined
geekosaur echoing your point 01:02
(I did look at the paste_
MasterDuke ah, ok
01:29 P6Bot joined 01:34 committable6 joined 01:36 committable6 joined 01:37 bisectable6 joined 01:40 bisectable6 joined 01:54 ilbot3 joined 01:56 synopsebot joined
MasterDuke right, Geth is down: github.com/MoarVM/MoarVM/pull/710 02:01
02:04 synopsebot joined
samcv MasterDuke, merged 02:05
MasterDuke cool, thanks 02:06
02:08 unicodable6 joined 02:28 Ven`` joined 02:34 vendethiel- joined 03:12 evalable6 joined 03:48 synopsebot joined 03:52 synopsebot joined 04:04 lizmat joined 04:14 Geth joined
samcv i'm working on trying to fix the windows MSVC 2017 build on appveyor 04:54
06:26 domidumont joined 06:30 domidumont joined 09:10 leont joined 09:17 Ven`` joined
timotimo vmprof.readthedocs.io/en/latest/ - maybe i should read all of this carefully 09:37
09:39 evalable6 joined
timotimo there's not much there :| 09:53
10:08 zakharyas joined 10:27 evalable6 joined 10:49 evalable6 joined 11:06 TimToady joined 11:19 synopsebot joined
timotimo jnthn: do you think i should introduce a new piece of MVMIOOps table for file descriptor duping? 11:20
jnthn timotimo: Mebbe...what will it apply to?
timotimo probably only syncfile tbh
jnthn Hm, then maybe not worth putting on the vtable 11:21
timotimo yeah, that was my worry
i'm not sure what it'd mean to dup our async handles, like sockets for example
async sockets, that is
and since we don't turn our sockets into fhs, duping them makes little sense, too
11:29 leont joined
timotimo oh no! 11:34
i didn't realize dup would share file offsets between the old and new file descriptors
that makes it useless for my use case
MasterDuke asan is saying i'm `attempting free on address which was not malloc()-ed` here github.com/MoarVM/MoarVM/blob/mast...oc.c#L220, caused by `buffer = MVM_fixed_size_realloc(tc, tc->instance->fsa, buffer, old_bufsize * sizeof(MVMGrapheme32), count * sizeof(MVMGrapheme32));` in utf8.c's MVM_string_utf8_decode 11:39
timotimo aha
so somehow the flag for FSA usage gets lost
BBL 11:40
MasterDuke i added `result->common.header.flags |= MVM_CF_USES_FSA;` right here: github.com/MoarVM/MoarVM/blob/mast...tf8.c#L191 11:41
timotimo what i'm saying is that something happens that causes the flag to be lost 11:45
MasterDuke hm. how could that happen?
timotimo shrug, i advise you to record the execution with rr, replay it to that point, print the address of the headers, i.e. have it display something like (MVMuint64 *) 0x1212413 and then have gdb "watch *((MVMUint64 *) 0x12312412)" and "reverse-cont" until it displays "changed" 11:46
and see where that leads
gotta go afk 11:47
MasterDuke never actually used rr, but i'll give it a try, thanks
timotimo good luck! 11:48
might be able to join irc on the road, so write if you're in trouble and i'll try to help 11:50
MasterDuke will od
*do
11:55 evalable6 joined
MasterDuke hm. at the time of the realloc, the flags are 4112, which is MVM_CF_USES_FSA + MVM_CF_SECOND_GEN 12:16
the previous change seems to be the `MVMString *result = (MVMString *)REPR(result_type)->allocate(tc, STABLE(result_type));`, the first line of MVM_string_utf8_decode 12:17
hm, but the flags on result shouldn't matter. buffer, which is what's getting realloced, hasn't been given to result yet 12:25
weird. for debugging purposes, i changed to get a new buffer at the realloc and explicitly free the old one. i now get "Fixed size allocator: wrong size in free" 13:04
but i'm printing the size when it originally gets created, it doesn't get changed anywhere else, and it matches when i'm calling the free 13:05
13:09 evalable6 joined
MasterDuke hm, it always seems to be when the fixed_size_realloc goes to this case: github.com/MoarVM/MoarVM/blob/mast...loc.c#L220 13:31
14:15 zakharyas joined
timotimo MasterDuke: that doesn't have a case for the fsa size debug at all? 14:20
MasterDuke timotimo: right, haven't added that yet 14:22
guess i can look at what the other functions do any try to copy it
timotimo i believe you'll just have to subtract to get the "real" pointer, realloc that, and write the new size 14:23
MasterDuke "real" pointer? 14:28
timotimo yeah, the pointer you get back from the FSA in FSA_SIZE_DEBUG mode is actually a 64bit integer offset from what malloc gave you 14:30
#if FSA_SIZE_DEBUG 14:31
MVMFixedSizeAllocDebug *dbg = (MVMFixedSizeAllocDebug *)((char *)to_free - 8);
that's why fixed_size_free is implemented like that
MasterDuke ah 14:32
oh! 14:33
now it work, i.e., print "hi" and doesn't segv, panic, etc 14:34
i was just doing `perl6 -e 'say "hi"'` 14:35
timotimo good!
that's a test case i also use often :)
MasterDuke ha. it would have worked before if i had turned off FSA_SIZE_DEBUG! 14:37
timotimo yes 14:38
anyway, we can just implement dup based on nativecall and "create io handle from fd" if need be 14:47
MasterDuke looks to have taken 400k instructions off of `perl6 -e ''` 15:01
15:03 leont joined
MasterDuke but trying to build nqp does not go well: *** Error in `/home/dan/Source/perl6/install/bin/moar': munmap_chunk(): invalid pointer: 0x0000555ad9515420 *** 15:04
timotimo 400k instructions out of how many? 15:06
MasterDuke 600million
timotimo hah.
still not bad 15:07
i should have checked what difference it made when i gave the NFA FSA alloc
MasterDuke timotimo: know anything about that error above? 15:17
ugexe i think the lta stuff may have fixed segfaults from `while 1 { my $a := gather { take 1 }; for 0 { await start { $a[0] }, start { $a[0] } } }`
er FSA
timotimo i can only imagine out of bounds write or passing a non-malloced pointer to free or something 15:18
15:32 synopsebot joined
MasterDuke is this just a quick way to create an empty MVMString? github.com/MoarVM/MoarVM/blob/mast...ump.c#L332 15:49
jnthn There's a constant one handing off the MVMInstance
tc->instance->str_consts.empty or so
*hanging
MasterDuke "name" (created at the line i linked) is only used here: github.com/MoarVM/MoarVM/blob/mast...#L398-L400 15:50
if i understand what's going on, name is created as just "", then gets some other stuff stuck in it, just so it can be MVM_string_utf8_encode_C_string()ed 15:52
16:12 domidumont joined 16:15 timo joined
timotimo ah dang, i'm using buffered input so i'm not going to get good results for tell, right? 16:16
not only that, but it also apparently reads in a whole megabyte at a time 16:17
jnthn Yeah but method tell in Rakudo compensates for that by subtracting off the number of bytes in the decoder 16:23
timotimo oh, nice 16:25
the reason i wasn't seeing anything but 0 for tell was i was using $*IN rather than $*ARGFILES 16:29
17:01 evalable6 joined 17:30 timo joined
Zoffix "Upgrade done (0.58 seconds)" that was too fast :/ 17:31
Guest59997 upgrade of what?
Zoffix wc
Geth MoarVM: MasterDuke17++ created pull request #711:
Support FSA_SIZE_DEBUG in MVM_fixed_size_realloc
timotimo MasterDuke: i believe fixed_size_realloc_at_safepoint needs a bit of code, too 17:32
MasterDuke: right now it won't update the size, so when the realloced thing gets freed later on it'll still think it's the same size as before ... only when it's the same bin, of course 17:33
MasterDuke what i added to MVM_fixed_size_realloc won't update the size? 17:35
timotimo fixed_size_realloc_at_safepoint doesn't use fixed_size_realloc 17:37
MasterDuke right, i just added the same code to it 17:38
timotimo i think the code for fixed_size_realloc looks correct
under what circumstances do we still use malloc for string memory? 17:39
MasterDuke on master? or the branch i'm working on? 17:40
timotimo your branch 17:41
oh, hm, i wouldn't do realloc_at_safepoint like that
MasterDuke lots of things i think
oh? 17:42
timotimo i think it ought to only do the code you just added if old_bin == new_bin
otherwise it'll behave differently
and then i'd really just call into realloc from the old_bin == new_bin branch
because that'll be the same code then
MasterDuke move the #if into the old_bin == new_bin branch? 17:44
17:44 releasable6 joined
timotimo aye 17:45
er, wait, i misspoke
oh, no, it was correct after all 17:46
i was about to say "no need to actually MVM_realloc if the bin is the same", but with FSA_SIZE_DEBUG the allocation is actually exact, so we can't re-use the pointer unless the sizes exactly match
MasterDuke so both commits in the PR are good as they stand now? 17:49
timotimo nah, i'd still want the safepoint code moved into the old_bin == new_bin 17:50
MasterDuke i don't understand why 17:51
timotimo because realloc_at_safepoint is there so if the pointer changes, code that's still got the old pointer around doesn't read from or write to just-freed memory 17:53
and the FSA_SIZE_DEBUG version of realloc_at_safepoint will now invalidate existing pointers every time 17:54
hm, wait, that means it can't do the shortcut at all and would have to go through the full alloc + memcpy + free dance
unless the sizes exactly match 17:55
OK, i gotta go afk for a bit, but i think the way i've described it now should be correct
MasterDuke so i should move the at_safepoint debug code inside the new == old branch? 17:56
18:01 quotable6 joined 18:25 evalable6 joined 18:34 Zoffix joined 18:47 Zoffix joined
timotimo nah, i think it needs to always do the old_bin != new_bin branch if debug is on 18:56
MasterDuke with a fixed_size_alloc, or regular MVM_malloc? 19:00
timotimo if you call into fixed_size_alloc and free_at_safepoint it'll do the pointer calculations for you 19:01
MasterDuke timotimo: gist.github.com/MasterDuke17/e66c3...4647801f49 ? 19:08
19:42 geekosaur joined
timotimo i think that looks good 19:42
MasterDuke k, i'll correct the second commit in the PR to that 19:43
updated 19:48
ugh. now i'm getting `MoarVM panic: Fixed size allocator: wrong size in free, expected 256, got 232`, but it looks right at the calling site 19:52
19:53 synopsebot joined
MasterDuke i may need to back out a whole bunch of changes and start nearly over 19:53
21:02 geekosaur joined
timotimo damn 21:06
MasterDuke: what do i have to do to make it break on my end? 21:14
are you running it with asan or with valgrind?
samcv MasterDuke, why are there so many issues you're having with freeing a different amount than you allocated? any specific things going on? 21:15
timotimo src/core/fixedsizealloc.c:234:27: error: pointer of type ???void *??? used in arithmetic [-Werror=pointer-arith] 21:17
i wonder why it only blows up on my end
21:19 Ven`` joined
timotimo omg could asan please stop reporting leaks unless i tell it not to 21:24
samcv there is a compile option iirc 21:30
MasterDuke timotimo: make it break? 21:31
timotimo yeah, why do you get that panic?
MasterDuke i turned FSA_SIZE_DEBUG on
samcv: just making a bunch of changes kind of all over 21:32
timotimo i just built all of rakudo with FSA_SIZE_DEBUG on and also asan 21:33
MasterDuke heh. i guess that's good confirmation that it must be something i've done wrong
fwiw, this is my branch with current changes, it's mostly a bunch of random commits to save progress. github.com/MasterDuke17/MoarVM/tre...ng_storage 21:34
timotimo i merge that into the fsa_size_debug support branch? 21:35
MasterDuke i think i cherry picked changes out for the fsa_size_debug branch
so should just merge into master 21:36
gist.github.com/MasterDuke17/f5d17...cf0c5f543, the panic i get when trying to build nqp
timotimo ah, so very early into building nqp?
MasterDuke yep, first thing maybe 21:37
timotimo aye
i shall have a little look
MasterDuke part of the overall problem is that i didn't have much of a plan for how to go about this 21:38
i think i start while on a flight and have been working on it piecemeal
timotimo oh wow. i wondered why my arm's kinda itchy
turn it around, see rather large scratch marks from the cat 21:39
MasterDuke heh. i was wondering if my changes were so bad as to cause an allergic reaction on sight
can't blame the cat on me
timotimo i have alloc_size of 256 but free size of 50106 21:41
that could be interesting
MasterDuke oh, this is the specific line that's a problem right now: MVM_fixed_size_free(tc, tc->instance->fsa, cur_chars->length * sizeof(MVMGrapheme32), cur_chars->chars);
timotimo right 21:42
except it's length without cur_chars locally
MasterDuke right, but then you get a gigantic value 21:43
timotimo i don't think this is the right pointer to be freeing at all 21:44
i mean up above you're allocating result->body.storage.blob_32 21:45
but then the same length was used to free cur_chars->chars
which i'm not sure is fsa allocated?
MasterDuke it is/was/might be
timotimo er 21:47
doesn't free_chars actually already to that exact free action?
oh, no, free_chars frees the "outer" thing instead 21:48
but why does that free the chars pointer with the chars->length? that doesn't seem right to me
i think that must instead have a length of, like, sizeof(MVMDecodeStreamChars) to free 21:49
MasterDuke i've definitely confused myself many times with this bunch of code
the one at 470?
timotimo no, the one in free_chars 21:50
MasterDuke hm, i currently have the free in free_chars unchanged 21:51
timotimo ah, ok, htat's some local changes you have
MasterDuke i did push it 21:52
timotimo other places MVM_free(cur_chars->chars)
so that's what i tried. it's not correct though
oh, ah, utf8_decodestream does fsa allocate that 21:54
MasterDuke this would be so much easier if a normal free required the size also...
timotimo hah. 21:55
MasterDuke i'm not really sure why what i have now is wrong. if i'm freeing cur_chars->chars, cur_chars->length * sizeof(MVMGrapheme32) should be its size 21:56
timotimo hm. it should be possible with a little macro to tell valgrind to spit out all it knows about a given pointer 21:59
MasterDuke oh. maybe need to add ds->chars_head_pos 22:07
timotimo isn't that the distance from the cur_chars->chars to the actual position we'd be inserting to/reading from? 22:08
MasterDuke damn you and your unassailable logic! 22:10
timotimo not sure about it, i haven't looked closely at all :|
but i found a thing i can tell valgrind to give me info about a pointer
MasterDuke oh? 22:11
timotimo the valgrind docs are hair-pulling :| 22:12
i KNOW i saw the thing i needed a few seconds ago 22:13
perfect.
22:24 evalable6 joined
timotimo let's see if it did anything at all 22:25
now to screw up some size somewhere to see if it works 22:27
oh my gosh i think it worked 22:31
gist.github.com/timo/f6f84c4e6e48e...9241233d96 - MasterDuke how do you like this? 22:32
MasterDuke that's a very large len? 22:34
timotimo only 2x as much as was intended i think
whoops 22:35
i gave the pointer instead of the size there
good catch
MasterDuke hope you figure something out, probably gotta afk for a bit 22:38
timotimo .o( what exactly was i doing ) 22:41
fantastic, now i get not only the backtrace of where the thing was allocated, but also the backtrace of where the wrong free size was passed 22:46
i don't really understand why the cur_chars->length isn't the right size to free the cur_chars->chars with 22:53
MasterDuke i know! 22:54
bug? 22:55
timotimo it goes via decodestream_add_chars, which takes the length that was used to set up the buffer, too
oh here's another spot where the size isn't multiplied by the grapheme size
inside utf8.c, in the UTF8_REJECT branch 22:56
bufsize doesn't get mutated anywhere i'd see? :\
MasterDuke in my changes?
timotimo well, i have two of your branches merged :) 22:57
anyway, it doesn't crash any more now that i made it VALGRIND_PRINTF_BACKTRACE instead of MVM_panic ... and it finishes running 22:58
somehow it only trips over this once during that first compile
MasterDuke i fixed that, so you've got some older commits 22:59
timotimo ah
MasterDuke huh, so it build nqp?
timotimo oh, no
only the very first file
i have to put valgrind in front of the build lines every time ;_;
MasterDuke ha 23:00
timotimo MoarVM panic: Fixed size allocator: wrong size in free: expected 256, got 156 23:01
that's one of the later files
i'm going to look through what each file has to see what these buffer sizes do 23:02
like, expected 256 is the same number as the very first file
i want to see if it's just a coincidence
MasterDuke yep 23:03
timotimo ho-hum. 23:05
oh, well, what if it saves the actual number of characters that got into the buffer as length
not the size of the buffer
which makes a lot of sense
except it passes bufsize to add_chars, and not count 23:06
yeah, no, i don't know 23:08
but i'll get this valgrind stuff upstream
MasterDuke cool
timotimo aha! 23:10
after the "done:" label in utf8
there it uses count instead of bufsize
so we either put a bufsize into the struct so we can properly free, or we stick with malloc and free 23:11
MasterDuke hm 23:13
23:26 Ven`` joined

Powered by Cro and the Raku® Programming Language.

Please report any issues / comments / feature requests as an issue on App::Raku::Log.

Thank you!