github.com/moarvm/moarvm | IRC logs at colabti.org/irclogger/irclogger_logs/moarvm
Set by AlexDaniel on 12 June 2018.
00:34 Kaiepi left, Kaypie joined 00:53 Kaypie left, Kaypie joined 01:19 ggoebel joined
ggoebel (delurk) lizmat: my initial reaction was surprise that lists can contain containers... I would expect them to contain immutable values. Wasn't that part of the design decision behind the Great List Refactor (perl6advent.wordpress.com/2015/12/...efactor/)? 01:24
01:35 ggoebel left 02:14 pamplemousse left 02:21 klapperl_ joined, klapperl left 02:29 Kaypie left, Kaypie joined 02:33 Kaypie left 02:34 Kaypie joined 06:30 patrickb joined
lizmat ggoebel: that was my initial reaction as well but observe: 06:50
m: my ($a,$b) = 3,4; dd $a, $b
camelia Int $a = 3
Int $b = 4
lizmat how would that work if a List would *not* be able to have containers ?
06:51 domidumont joined 07:06 robertle joined 07:53 zakharyas joined 08:33 robertle left
jnthn Well, yes, since list assignment relies on that, and it'd be weird if the data structure involved in list assignment was not, well, List, then I don't think List can play that role. We're putting too much expectation on one type. 09:26
yoleaux 08:02Z <lizmat> jnthn: would it make sense to fix thread safety of state variables / once blocks by using an atomic int increment to signal initialization on the MoarVM backend?
09:26 patrickb left
jnthn .tell lizmat It wouldn't help much, since the issue is ultimately that one thread sees it's the first into the state block and runs the state initializer (which is what a `once` block is treated as, effectively), and then the second thread would (correctly) see it's not the first thing, so just sail on through and read the (so far uninitialized) state variable. 09:35
yoleaux jnthn: I'll pass your message to lizmat.
09:38 patrickb joined
timotimo we could have the state variable go from 0 to 1 if a thread is initializing the value and from 1 to 2 if the thread is done, and if the value is 1, we busyloop or something 10:29
jnthn busylooping is horrible, though :) 10:37
Sort of related, I often want something that's like `start`, but that doesn't actually run the code block until the first `await`. 10:38
timotimo so like liz' "slack"? 10:39
jnthn Link?
I think I'd like a different name, though. "slack" makes me think of excessive memory use :P 10:40
timotimo modules.perl6.org/dist/Object::Del...:ELIZABETH 10:41
that's the one
Guest37021 what does the following message mean? 'Invalid owner in item added to GC worklist' 10:56
timotimo the field "owner" in the header doesn't have a valid value
i.e. potentially memory corruption; i'm not sure if the owner spot is shared with the forwarder or something 10:57
jnthn No, it's not; it just means some kind of memory corruption
timotimo OK
hm. the owner shouldn't change over the lifetime of an object, right?
could try to put in a valgrind client request to mark those bytes as invalid after they've been initialized 10:58
Guest37021 I was thinking of continuing to look for GC related probs. I don't think there are many left (famous last words) 11:04
this message shows up when running t/spec/S32-io/out-buffering.t with as small nursery 11:05
the other problem, with t/spec/S32-io/socket-recv-vs-read.t, was fixed by timotimo++ yesterday
11:41 MasterDuke joined 12:06 MasterDuke left, zakharyas left
Guest37021 timotimo: this is what I have atm - gist.github.com/dogbert17/c0523756...1e1cf5888f 13:02
timotimo Guest37021: can you output the i and num_roots locals in the gc_root_add_temps_to_worklist frame? 13:12
Guest37021 sure, gimme a couple of minutes, I managed to close gdb 13:13
13:19 brrt joined
Guest37021 reproducing this error is a slow process, I shouldn't have closed gdb, grrr 13:24
13:26 Voldenet left
timotimo oh well :( 13:26
of course rr wouldn't run on your computer :< 13:27
13:31 lucasb joined 13:32 Voldenet joined, Voldenet left, Voldenet joined
brrt good * 13:33
13:36 robertle joined
Guest37021 hello brrt 13:37
brrt hi Guest37021 13:38
isn't it time for you to get a nick, or *is* that your nick ;-)
Guest37021 aka Dogbert17 :-)
brrt ah, I see
13:38 ggoebel joined
Guest37021 are you don with your fp work? 13:39
*done
13:48 AlexDaniel left, AlexDaniel joined, ggoebel left
brrt yeah, I think FP is done. We should get it merged still, though 13:53
timotimo has asked me to look at adding devirt support
and I've basically been looking into a way to do that elegantly
Guest37021 have you come up with a clever solution? 13:54
brrt I think so 13:56
timotimo cool 13:57
it ought to allow more optimizations than just devirt
14:01 brrt left 14:04 brrt joined 14:20 zakharyas joined 14:27 patrickb left
Guest37021 timotimo: finally, num_roots = 2 and i = 0 14:27
timotimo ok, so the earliest root is the wrong one, eh? 14:28
could you go up the stack and look what was the first thing pushed onto it?
14:31 brrt left, brrt joined
Guest37021 which frame do you suggest - gist.github.com/dogbert17/c0523756...1e1cf5888f 14:31
timotimo hm, could it be in MVM_platform_uname, the result variable? 14:36
you should be able to compare "&result" with the first entry in the temp roots array
ah
that needs to be nulled at the start of the function
please try that as a patch; in src/platform/sys.c have "MVMObject *result = NULL" 14:37
otherwise an uninitialized pointer (which may fall onto a pointer from an earlier function) will be fed to the GC
um, also, pretty sure the variables sysname, release, version, and machine shouldn't be unrooted like that either 14:38
Guest37021 interesting
timotimo putting the corresponding MVM_repr_bind_pos_s right after the MVM_string_utf8_decode would probably be enough
but of course that's mixing declaration and code
Guest37021 the compiler has a tendency to dislike that i think 14:39
timotimo yeah
can just put empty declarations at the start of the block
then have assignment, bindpos, assignment, bindpos, ...
14:39 Kaypie left 14:41 Kaiepi joined
Guest37021 timotimo: like this then - MVMObject *result = NULL; 14:41
ah you wrote that above :) 14:42
timotimo that's the first step 14:43
Guest37021 I can give it a go
ugexe impossible. i wrote that code therefore it must be perfect. 14:44
Guest37021 :-)
timotimo the code is perfect, it's just the rest of moarvm that's wrong
is there a gcc annotation that we can put onto temp_root_push that warns if an undefined value is pointed at? 14:48
actually, not all instances of that pattern are dangerous
only if there's a function that can allocate that is called before the variable is actually assigned to 14:49
so MVMObject *result; MVMROOT(result, { result = blah }); would be fine
Guest37021 does that mean that the code is perfect :) 14:50
or is a MVMroot missing 14:51
ugexe result does not need to be rooted. the original PR passed in *result as a 2nd parameter, but I created a new PR that removed that parameter and created the result inside the function.
github.com/ugexe/MoarVM/blob/05e2d....c#L15-L31 (the original PR code) 14:52
Guest37021 timotimo, ugexe: now I got this - gist.github.com/dogbert17/09a85e83...5bfa8bb902
timotimo how do you mean result doesn't have to be rooted? 14:53
Guest37021: what does your patch look like?
Guest37021 I only did the NULL assignment
of *result 14:54
timotimo yeah, that's not enough
after each of the utf8_decode calls, all variables before that get potentially invalidated
except for result
the original code that ugexe pasted also didn't have a problem, because it didn't assign the results of utf8_decode into variables 14:55
ugexe that had other problems
Guest37021 uh oh
timotimo what were those? 14:56
ugexe github.com/MoarVM/MoarVM/pull/1037...r254714185
timotimo ah
ugexe well, maybe that doesn't apply because result isn't a parameter anymore?
timotimo correct
14:57 pamplemousse joined
timotimo hm 14:59
i think i might have misunderstood what jnthn said
we can take the very big hammer that is "volatile" for this purpose perhaps
hm
is this about sequence points... 15:00
well, splitting it into two lines is fine
just needs to make sure the bind_pos comes right after the decode
rather than all the decodes followed by all the bind_pos calls
third option is to stack MVMROOTs four levels deep 15:02
Guest37021 will you make those changes? 15:06
timotimo i can do that
15:06 domidumont left
Guest37021 jnthn has been silent so he probably agrees with you :) 15:07
ugexe stacking MVMROOTs just like the good ol days 15:08
timotimo whoa, a game i remember playing as a child (released 1997) was open-sourced and i just stumbled upon it again 15:09
Geth MoarVM: 2911ec7e40 | (Timo Paulssen)++ | src/platform/sys.c
Fix GC rooting in MVM_platform_uname

  dogbert17++ for finding the crashes,
  ugexe++ for pointing out the reason behind
splitting decode and bind_pos calls
15:11
Guest37021 ugexe: wrt R#3080, FWIW the deadlock vanishes if the golf is run with MVM_SPESH_INLINE_DISABLE=1 15:12
synopsebot R#3080 [open]: github.com/rakudo/rakudo/issues/3080 Deadlock when using multiple CATCH + rethrow
Guest37021 timotimo++
timotimo needs a very unlucky user to reproduce this crash in the wild, but of course it's best not to have it in there at all 15:14
Guest37021 we also have the get_lex crash to look into 15:19
Guest37021 relocates & 15:20
15:31 chloekek joined 15:43 robertle left 16:36 pamplemousse left 16:45 robertle joined 17:01 brrt left 17:15 sena_kun joined
AlexDaniel samcv: everything looks fine to me, let's release 17:16
17:29 pamplemousse joined 17:38 pamplemousse_ joined 17:40 pamplemousse left 17:41 pamplemousse_ is now known as pamplemousse 17:44 chloekek left 18:20 AlexDaniel left 18:22 pamplemousse_ joined 18:24 pamplemousse left, pamplemousse_ is now known as pamplemousse 18:32 zakharyas left 18:36 AlexDaniel joined
lucasb timotimo: ooc, which game? :) 18:58
timotimo www.blupi.org/ 18:59
lucasb cool, I didn't know it 19:00
19:01 pamplemousse_ joined 19:03 pamplemousse left
samcv AlexDaniel, is everything that needs to be in the MVM release repo cherry-picked? 19:18
timotimo the one for uname up there could also go in
AlexDaniel timotimo: don't know anything about blupi, but I enjoyed CoLoBoT at the time 19:22
samcv: this one was mentioned but is not cherry-picked github.com/MoarVM/MoarVM/commit/ab7bac5ad1 19:31
Geth MoarVM/release-2019.07.1: cb5787aa99 | (Stefan Seifert)++ (committed by Samantha McVey) | src/core/interp.c
Fix possible memory corruption in bindkey_*

bindkey reads the target object from a register, calls the bind_key repr function and then calls MVM_SC_WB_OBJ with the object. The repr function however may allocate and thus trigger a GC run which may move the target object. In that case we'd end up calling MVM_SC_WB_OBJ on the outdated copy of the object. Fix by reading it fresh from the register as those get updated automatically by the GC.
MoarVM/release-2019.07.1: 7360ee5dbd | (Timo Paulssen)++ (committed by Samantha McVey) | src/platform/sys.c
Fix GC rooting in MVM_platform_uname

  dogbert17++ for finding the crashes,
  ugexe++ for pointing out the reason behind
splitting decode and bind_pos calls
AlexDaniel samcv: also github.com/MoarVM/MoarVM/commit/c3c9d7dfac 19:32
samcv: see the list here github.com/rakudo/rakudo/issues/3067
Geth MoarVM/release-2019.07.1: 36cb624e5c | (Jonathan Worthington)++ (committed by Samantha McVey) | src/core/frame.c
Avoid various races in profile/coverage//debug

We could attempt to duplicately instrument code when multiple threads started to execute it around the same time, which could lead to crashes, since it was assumed that would only happen once. Do it under lock, so as to prevent such issues.
AlexDaniel samcv: I think currently c3c9d7d and e07c0f2 are missing 19:36
Geth MoarVM/release-2019.07.1: 79d211893f | (Jonathan Worthington)++ (committed by Samantha McVey) | src/core/frame.c
Bump level until after instrumenting

Rather than before, which can lead to various races. This fixes a whole range of different problems in both debugging and profiling of apps that are multi-threaded.
19:38
MoarVM/release-2019.07.1: 1c2747724d | (Timo Paulssen)++ (committed by Samantha McVey) | src/profiler/configuration.c
confprog: don't return null from filename()
19:50 pamplemousse_ is now known as pamplemousse 20:10 MasterDuke joined
MasterDuke ordered my new parts. ryzen 7 3700x, 32gb pc3200 ddr4, msi x570-a pro. motherboard was on backorder though, so probably won't get them for a little while 20:17
20:17 TimToady left, TimToady joined
dogbert17 MasterDuke: very cool 20:22
setting the nursery to 8k actually manages to crash a few tests 20:23
MasterDuke new crashes? 20:31
dogbert17 yes, at least I haven't seen them before 20:36
e.g. t/spec/S17-channel/subscription-drain-in-react.t
here's what it looks like - gist.github.com/dogbert17/13643f76...289ef8df4b 20:42
MasterDuke don't think i've seen that one before either 20:46
dogbert17 owner is set to 44058952, that feels a bit high 20:49
21:04 Kaiepi left 21:17 Kaiepi joined 21:30 MasterDuke left
timotimo in an rr replay you could hardware watchpoint that value and reverse-continue until it gets overwritten 21:41
dogbert17 haha 21:42
timotimo www.myfonts.com/fonts/tabular-type...omic-code/ 21:51
22:08 sena_kun left 23:01 pamplemousse left 23:11 lucasb left