github.com/moarvm/moarvm | IRC logs at colabti.org/irclogger/irclogger_logs/moarvm
Set by AlexDaniel on 12 June 2018.
[Coke] yup, wasn't really expecting to find anything. no worries. 00:15
nwc10 good *, #moarvm 06:52
japhb good *! :-) 06:56
MasterDuke hm, my other commits in github.com/MoarVM/MoarVM/pull/689 are all fine (nqp and rakudo build and pass tests), but github.com/MoarVM/MoarVM/pull/689/...afce887545 dies in the nqp build almost immediately 17:14
any thoughts? 17:15
timotimo did you see that the bytes value gets changed between where there was malloc and where there is fixed_size_alloc now? 18:02
MasterDuke well yeah, i think that change should happen regardless. now i'm allocated the correct amount, before it could have been allocating more than it needs 18:04
timotimo right 18:05
what does the crash look like? 18:06
did you try to run it with valgrind support configured and then run it through valgrind? 18:07
it should give details like "used wrong size / allocator" and such
timotimo MasterDuke:i don't know what calls read_bytes, but perhaps there's a spot missing where it sets the flag on the object that it uses the fsa? 18:12
oh it's literally right there
are there other read_bytes impls than just syncfile? 18:13
oh 18:14
i might have seen it
MasterDuke: your buf is **, but you realloc it as if it were *buf
same for the free actually perhaps? 18:15
i mean, before the change even?!
no, i'm dumb, buf and buf_out aren't the same 18:16
there's also syncsocket reading that also offers a sync_readable, but doesn't FSA yet, but it would be slightly surprising to have that happen early in the nqp build at all 18:18
i'll build it locally and see if i can find out anything 18:25
MasterDuke timotimo: gist.github.com/MasterDuke17/07db0...beb90f017d 18:28
thought you had something at first with the **, but not quite that simple
timotimo ooh 18:30
MasterDuke Decoder?
timotimo you're not using the return value of MVM_fixed_size_realloc
to update the buf pointer
MasterDuke doh 18:31
timotimo i didn't see it either :)
can we annotate the function so we get an extra nasty warning if we discard the return value?
MasterDuke there is a way, libtommath does that 18:32
nqp just built, on to rakudo... 18:33
rakudo built, on to a spectest 18:35
spectest passed. timotimo++ 18:38
timotimo cool
MasterDuke now come the even more complicated changes. normalization, decoding, etc 18:41
oh, so close. next changes die during the rakudo install 18:45
timotimo the next changes that aren't on github yet you mean? 18:50
MasterDuke yeah. i can push them if you're willing to take a look 18:54
timotimo i may not have terribly much time to look into it, but maybe i can help 19:17
MasterDuke pushed 19:20
discoD To get line coverage reports, do I just build with --coverage then set MVM_COVERAGE_LOG & MVM_COVERAGE_CONTROL? I've been using a pintool to test coverage in a function, but it's a royal pain. 19:24
MasterDuke pretty sure you don't need MVM_COVERAGE_CONTROL if you want coverage of everything 19:25
it's only so you can turn it on/off later if you want
timotimo -i think --coverage in moarvm is for C-level coverage, the kind that gcc and clang can give you
you'll only need the coverage log env var, the other one is for extra control 19:26
discoD c-level coverage would be wonderful compared to what i've got
can you limit it to certain functions by any chance? 19:27
i'm looking into the utf8_c8 decoder, so that's all i really care about at the moment 19:28
timotimo you can check the .travis.yml for how we generate C coverage on the CI
discoD ok, thanks
timotimo github.com/samcv/moarvm-cover 19:30
discoD that's great. thanks again 19:34
timotimo MasterDuke:i wonder if "multiplied with sizeof codepoint" and "not multiplied" are getting mixed up somehow? 19:38
in normalize.c at the bottom, i.e. line 200 and around 19:39
could maybe_grow_result realloc the buffer but orig_alloc is not being updated?
MasterDuke oh, hm... 19:41
timotimo you could potentially output the pointers that come out of alloc with their sizes and the pointer and old size and new size coming in and out of realloc and then into free 19:42
or set the fsa debug mode
which checks the values automatically
nine So....can MVMCompUnits ever be allocated in the nursery? Part of the code thinks so as they are sometimes MVMROOTed 19:59
lizmat nine: I assume a BEGIN block is a compunit, but one of which the codegen should not be kept? 20:00
so maybe allocated in the nursery ? 20:01
ah, and constant definitions as well ?
nine The only place I find that allocates an MVMCompUnit is MVM_cu_from_bytes and it does so in gen2 20:02
I think most of check-root.py's value actually comes from it using the list of allocating functions I generated with cflow. It's really surprising what functions can enter the GC. And who can keep a list of 507 functions in their head when reviewing code? 20:08
MasterDuke nine: sounds like a good list to compare against the oplist and see if anything still needs to be added to the profiler (or if anything should be removed) 20:09
lizmat nine: feels like some introspection on such functions would be nice, like "can you enter GC" ? 20:10
nine lizmat: yes, that's what I want to have at some point. Possible via a custom attribute 20:11
Now who sees the GC violation in this statement? MVM_ASSIGN_REF(tc, &(sf->common.header), sf->body.static_env[lex_idx].o, MVM_sc_get_object(tc, sc, read_int32(pos, 8)));
MasterDuke timotimo: are those extra reallocs i added even needed (though i was getting the same errors before i added them)? freeing just cares about the current size, which it can get from body.elems 20:15
nine Spoiler alert! The compiler turns this call into: 20:17
_233 = read_int32(pos, 8); _234 = (long int) _233; _r = MVM_sc_get_object(tc, sc, _234); sf.80_235 = sf; _236 = &sf.80_235->common.header; MVM_gc_write_barrier(tc, _236, _r); sf.81_237 = sf; _238 = sf.81_237->body.static_env; _239 = (long unsigned int) lex_idx; _240 = _239 * 8; _241 = _238 + _240; _241->o = _r;
So the GC triggering MVM_sc_get_object function is called, before the moveable sf variable is accessed. 20:18
Turns out, a C compiler is much better at reading C than your average developer. Who'd have thought?
Geth MoarVM/gcc_root_checker_plugin: 5 commits pushed by (Stefan Seifert)++ 20:24
jnthn nine++ 20:25
Impressed by how much the plugin finds. 20:26
nine Well for those who are curious, this is the full list of still existing candidates: gist.github.com/niner/f389fe6ebf43...924da4b544 20:28
Deserialization and the profiler allocate in gen2 directly, so those are false positives. The rest turns out to be very much worth looking at 20:29
And the list of 507 GC triggering functions may be outdated. I generated it in September and am not quite sure anymore how I did that... 20:30
Btw. would a JITed version of getlexouter ever have to deal with the situation that the lexical was not found? 20:38
If not, I could fix that but without having to MVMROOT 20:39