timotimo | afl claims to have found 303 unique ways to crash moar using fuzzed .moarvm files | 01:05 | |
samcv | timotimo: crash as in ungracefully? like without a MVM_oops or throwing? | 01:09 | |
i could see that happening | |||
timotimo | segfaulting, i believe. though it can also mean running against the memory limit i *think* | 01:10 | |
just taking too long is counted separately as a timeout, i believe | 01:11 | ||
the compressed bunch of files is just 450 kbytes big - extracts to 100 megs | |||
hack.p6c.org/~timo/moar-fuzzing-stuff.tar.xz | |||
f0478cc5138d27aeeb618d84142b3c9cee81c79026b58e8556744d45284e5f6d is the sha256sum | 01:12 | ||
because i think hack's webserver doesn't do https perhaps? | |||
anyway, cd into the input folder and moar ../findings/crashes/blah | |||
hack.p6c.org/~timo/ - this has the plots afl makes | 01:14 | ||
i believe the speed differences clearly visible on the bottom left comes from it reaching a different input file | 01:15 | ||
a bigger input file, i expect | 01:16 | ||
the nqp input currently looks like lots and lots of 4, then a *, then lots of 4 again and a P and more 4s | |||
there's a null byte or two in there, as well | 01:17 | ||
cool, i just saw it go from one cycle to the next. the nqp side of things has done 46 cycles now | 01:19 | ||
what it identified as crashes was a whole lot less explosive than i had expected. .. like not crashy at all :( | 01:20 | ||
samcv | hmm | ||
../finding_0/crashes/id:000005,sig:11,src:000000,op:flip1,pos:88 this gets angry with asan | |||
By a read memory access, address points to the zero page | |||
timotimo | one of the early crashes is just clargs being turned into some op that tries to use its first register as an object of some kind, but in the fuzzed version it's actually a register that was just nulled out | 01:21 | |
samcv | yeah asan is very unhappy with most of these | 01:22 | |
maybe all | |||
timotimo | that's what we get afl for :) | 01:23 | |
01:48
ilbot3 left
01:56
ilbot3 joined
04:55
lizmat left
05:01
lizmat joined
|
|||
Geth | MoarVM: b836b0dfd1 | (Samantha McVey)++ | src/6model/reprs/MVMMultiCache.c Initialize pointers in MVMMultiCache.c to NULL Coverity scan stated known_type_st needs to be initialized as NULL as it's possible it can be accessed before it's initialized. I set all the other pointers in the file to NULL as an added measure. |
05:08 | |
05:46
domidumont joined
05:52
domidumont left
05:53
domidumont joined
06:19
domidumont left
06:37
domidumont joined
|
|||
Geth | MoarVM: 4470c96cc9 | (Samantha McVey)++ | src/strings/shiftjis.c Fix the shiftjis decode bug in decode as well as decodestream Also rename the variables in decodestream to be the same as te ones in the current patch. Fixes an issue where a -1 grapheme in the replacement could cause unexpected behavior. |
06:45 | |
06:50
Ven`` joined
06:54
Ven`` left,
Ven`` joined
07:26
zakharyas joined
|
|||
samcv | .ask brrt this showed up on the coverity scan. was thinking of maybe adding a comment like so cry.nu/p/qh7j | 07:29 | |
yoleaux | samcv: I'll pass your message to brrt. | ||
samcv | .tell brrt this was the coverity scan issue fyi scan4.coverity.com/reports.htm#v27...tId=287693 i think it's totally fine (probably) though youknow better | 07:30 | |
yoleaux | samcv: I'll pass your message to brrt. | ||
samcv | this one looks more interesting, we don't guard against accessing out of bounds array access in MVM_jit_get_template_for_opcode scan4.coverity.com/reports.htm#v27...tId=287729 | ||
we only guard it's less than MVM_OP_EXT_BASE, but the array is 867 at time of writing while MVM_OP_EXT_BASE is 1024 | 07:31 | ||
07:37
brrt joined
09:18
brrt left,
brrt joined
10:00
brrt left
11:11
Ven`` left
11:24
zakharyas left
11:58
brrt joined
12:31
Ven`` joined
12:46
Ven` joined,
Ven` left
12:47
Ven` joined
12:49
Ven`` left
12:57
zakharyas joined
13:21
Ven` left,
Ven`` joined
13:25
Redfoxmoon joined
|
|||
Redfoxmoon | What's the minimum version of GCC needed to build moarvm? | 13:25 | |
brrt | i have no idea | 13:48 | |
yoleaux | 07:29Z <samcv> brrt: this showed up on the coverity scan. was thinking of maybe adding a comment like so cry.nu/p/qh7j | ||
07:30Z <samcv> brrt: this was the coverity scan issue fyi scan4.coverity.com/reports.htm#v27...tId=287693 i think it's totally fine (probably) though youknow better | |||
brrt | is there a version that's broken? | ||
.ask samcv is there a public version where I can see that? | 13:50 | ||
yoleaux | brrt: I'll pass your message to samcv. | ||
brrt | i mean, that shouldn't ever ever happen | ||
.samcv: samcv: if you wrap it with a #if MVM_JIT_DEBUG or something like that, then I"m perfectly happy | 13:52 | ||
.tell samcv I'll be happy, and i'll be happier if wrapped with #if MVM_JIT_DEBUG, because in real code this basically cannot happen | 13:53 | ||
yoleaux | brrt: I'll pass your message to samcv. | ||
13:56
Ven`` left
14:01
Ven`` joined
14:15
brrt left
14:39
brrt joined
|
|||
brrt | i'm thinking of dropping the whole remove-invokish thing and getting the jit-stack-walker branch merged as-is | 14:48 | |
15:03
brrt left,
brrt joined
15:05
Ven`` left
15:09
Ven`` joined
|
|||
samcv | ah k. though we'll need a define that has the length of the actual array. i think that array is generated or is it not? | 15:19 | |
yoleaux | 13:50Z <brrt> samcv: is there a public version where I can see that? | ||
13:53Z <brrt> samcv: I'll be happy, and i'll be happier if wrapped with #if MVM_JIT_DEBUG, because in real code this basically cannot happen | |||
[Coke] | brrt: any luck with those warnings? | 15:20 | |
brrt | [Coke]: sorry, have not been sufficiently online so far | 15:21 | |
[Coke] | no worries | ||
brrt | samcv: which array? | ||
oh, the NVR_GPR_BITMAP | 15:22 | ||
samcv | no | ||
brrt | that's not even really an array | ||
samcv | MVM_jit_expr_template_info | 15:23 | |
brrt | oh, that one | ||
yeah | |||
i was going to fix that with a sizeof(MVM_jit_expr_template_info)/sizeof(MVM_jit_expr_template_info[0]); then i realized that we ought to have a macro for that, then I forgot and started doing something else | |||
that one *is* a bug, i actually agree | 15:24 | ||
samcv | ok :) | ||
brrt | having said that, i've been thinking about jit-debug flags | 15:26 | |
15:33
Ven`` left
15:56
brrt left
|
|||
timotimo | Redfoxmoon: we are developing moarvm in c89, so i assume the gcc can be a couple of years old and still work | 16:02 | |
Redfoxmoon: have you got any specific version in mind that doesn't seem to work? | 16:03 | ||
Redfoxmoon | o_O | ||
Oh, no, just wondered before bothering to try:-) | |||
timotimo | we're keeping compatibility with MSVC, which is not quite but almost the worst thing in my whole existence | ||
Redfoxmoon | heheheheh. yeah. | 16:04 | |
I've got a policy of only using GNU C myself :P | |||
out of pure spite | |||
16:04
domidumont left
16:21
zakharyas left
16:43
domidumont joined
16:44
brrt joined
|
|||
brrt | oh. I may have fixed the no-more-invokish thing after all | 16:44 | |
Geth | MoarVM/jit-stack-walker: bba6fd85e7 | (Bart Wiegmans)++ | 15 files [JIT] No more invokish Some MoarVM opcodes can change control flow for the interpreter, e.g by invoking a new frame or jumping to a new ecxeption handler. The JIT relied specially compiled 'control guards' to in order follow these control flow changes. However, now that we actually have a pointer to the return-address on stack, it is possible to change the 'current position' for the JIT-compiled code, much like for interpreted code. Thus, the control guards are no longer necessary, which should result in a nice reduction in code size and hopefully a speedup as well. |
||
17:02
zakharyas joined
|
|||
Geth | MoarVM/jit-stack-walker: 17 commits pushed by (Bart Wiegmans)++ review: github.com/MoarVM/MoarVM/compare/b...913728a9d9 |
17:04 | |
brrt | ^ rebase | 17:08 | |
samcv | this looks like a bug scan4.coverity.com/reports.htm#v27...tId=287729 | 17:09 | |
if (!ds_>bytes_head && pos == 0) return; but then below we check ds->bytes_head->length | 17:10 | ||
so if there's no ds->bytes_head but pos != 0 then we'll dereference the null poniter | |||
17:13
brrt left
|
|||
samcv | i'm thinking it should probably never happen so i may just want to guard against it with an exception | 17:15 | |
Geth | MoarVM: 1b83cc54bf | (Samantha McVey)++ | src/strings/decode_stream.c Guard against null pointer dereference in decodestream_discard I'm guessing that this in proper operation should not happen, but to be safe it's a good idea to guard it with throwing an exception so MoarVM doesn't explode by dereferencing a null pointer if ds->bytes_head == NULL and pos != 0. Detected by Coverity scan. |
17:18 | |
samcv | .ask jnthn i think this seems proper github.com/MoarVM/MoarVM/commit/1b83cc54bf trying to fix this coverity scan warning scan4.coverity.com/reports.htm#v27...tId=287729 let me know if any issues with this | 17:20 | |
yoleaux | samcv: I'll pass your message to jnthn. | ||
samcv | so 74 issues now i think i've dealt with almost 10 coverity issues so far. maybe 5 at least. idk i'm bad at estimating :) | 17:21 | |
seems like it's at least 6 | |||
and dismissed several that were false positives associated with MVM_trycas since it resolves to AO_compare_and_swap_full which uses asm to set values | 17:23 | ||
Geth | MoarVM: 212e4175fb | (Samantha McVey)++ | 2 files Annotate the switch in generate_codepoints_by_name Also add a comment about an unused value that was detected by a Coverity scan. Semi more importantly I annotated what each of the switch values denote making it more understandable what is happening. |
17:32 | |
MoarVM: a0eb65bf82 | (Samantha McVey)++ | src/gc/roots.c Fix unportable sizeof in roots.c sizeof(MVMCollectable*) is the same as sizeof(MVMCollectable**) generally, but not guarenteed by C standard. Detected by Coverity scan. |
17:42 | ||
MoarVM: 853ea27c70 | (Samantha McVey)++ | src/debug/debugserver.c Check that socket() doesn't return -1 in debugserver We checked that setsockopt(), bind() and listen() don't return -1 but didn't check that socket() didn't return -1. Detected by Coverity scan. |
17:52 | ||
MoarVM: 4fcbdce2af | (Samantha McVey)++ | src/main.c Put code in an ifdef for HAVE_TELEMEH that was dead otherwise Detected by Coverity scan. There is code above that's in an ifdef for HAVE_TELEMEH but code below was not. |
17:56 | ||
17:59
zakharyas left
|
|||
samcv | timotimo: i think this may be a valid bug? scan4.coverity.com/reports.htm#v38...tId=287691 you know more about this than me | 18:04 | |
seems to be a place we don't get a lock before setting a value? | 18:05 | ||
18:49
brrt joined
|
|||
Geth | MoarVM: bdw++ created pull request #868: [JIT] Use on-stack return address for reading and writing the current position |
18:50 | |
19:04
domidumont left
|
|||
Geth | MoarVM/jit-stack-walker: 409f68a294 | (Bart Wiegmans)++ | src/jit/stub.c [JIT] Unbreak stub lizmat++ for pointing this out |
19:17 | |
19:17
brrt left
20:05
brrt joined
20:40
brrt left
21:17
lizmat left
21:19
lizmat joined
|