Wasmtime GitHub notifications bot (Aug 30 2022 at 19:14): Wasmtime GitHub notifications bot (Aug 30 2022 at 19:14):alexcrichton commented on issue #4824:
For testing this, would you be up for running this fuzzer for 15-30 minutes locally to ensure that there's no low-hanging fruit the fuzzer picks out?
Wasmtime GitHub notifications bot (Aug 30 2022 at 20:32):afonso360 commented on issue #4824:
I've ran this in the weekend on an aarch64 server for a few hours, and while it did turn up some bugs none of them were related to this.
Additionally I've ran this for the past hour on my local setup and so far it hasn't crashed. But I wouldn't count that too much since I now only get about 13 exec/s. This was probably fine when fuzzgen was smaller but I guess It's not anymore.
Wasmtime GitHub notifications bot (Aug 30 2022 at 20:44):jameysharp commented on issue #4824:
I suspect the fuzzer is silently rejecting a lot of inputs too. I started refactoring it a couple weeks ago but haven't had time to finish it. @afonso360, maybe you could take a look at the direction I was going and see what you think?
https://github.com/bytecodealliance/wasmtime/compare/main...jameysharp:wasmtime:cranelift-fuzzgen
Wasmtime GitHub notifications bot (Sep 01 2022 at 00:22):jameysharp commented on issue #4824:
I found a failure when fuzzing using this branch. After
cargo fuzz tmin, here's the input (when run at commit 4c12aa7982e7211a1c3142842533644b393bf270):<details>
<summary>Base64-encoded input</summary>QoEBAscK+wAAAAj4AADk3uTk5OQC5QPHARoAS0tLS0tLS/sARAAGAAEAAAABS0tLS0tLS0tLS0tL S0tLS0tLS0tLS0tL/////////whLS0tLS0tLS0tL7v9BVe4SJRJBEiX//1cqCUtLI0tLS0tLS0tL S0tLS0tLSwAAAABLS0tLS0tLS9PT09PT9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0 9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT+/v7+/v7+/gP///8AABISFf8J+7cAAQEA AAQA1wAAANYAAP8J+3W3AAEBAAAAAABsJWzqAgD09PT09PT09PT09PT09PT09PT09PQN9PT09PT0 9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09AAI+AAA 5N7k5OTkAuUDxwEaAEtLS0tLS0v7AEQABgABAAAAAUtLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS//0 9PT09PT09PT09PT09PT09PT09PT09PT09NNERERE09PT09PT09PT09PT09PT0wAAADjT09PT00tL S0tLS0tLS0sASwAASwAAACQqAAAAAQAAAAAAXQEAAAAA/wAAFQnk5ORC5AKBxAHHCh0AAAD///// //////TkAuTH5OR85APlAsfT09PT09PT09PT09PT09PT09PT00tLS0tLS0tLS0sASwAASwAAACQq AAAAAQAAAAAAXQEAxMTExMTExMTExMTExAEkAAAAAQQAAEEAAF309PT09PT09PT09PT09PT09PT0 9PT09PT09PT09PT09PT09PT09NNERERE09PT09PT09PT09PT09PT09PT09PT09PT00tLS0tLS0tL S0sASwAASwAAACQqAAAAAQAAAAAAXQEAAAAA/wAAFQnk5ORC5AKBxAHHCh0AAAD///////////Tk AuTH5OR85APlAsfExMTExMTExMTExMTExMTExMTExMTExMQBJAAAAAEEAABBAABd</details>
<details>
<summary>Output ofcargo fuzz fmt</summary>test interpret test run set enable_llvm_abi_extensions target aarch64 target s390x target x86_64 function u1:0(i8 uext, i16 uext, i16 sext, b1, b1 sext, b1, b1, f32, i64, i64 sext, i128, f64 uext, i16, i32, i32) -> i32, i32, i64, f32, i8, b1, i8 system_v { ss0 = explicit_slot 75 ss1 = explicit_slot 75 ss2 = explicit_slot 75 block0(v0: i8, v1: i16, v2: i16, v3: b1, v4: b1, v5: b1, v6: b1, v7: f32, v8: i64, v9: i64, v10: i128, v11: f64, v12: i16, v13: i32, v14: i32): v64 = iconst.i32 0x4b4b_4b4b v65 = bconst.b1 false v66 = bconst.b1 false v67 = iconst.i32 0x4b4b_4b4b v68 = iconst.i32 0xffff_ffff_d3d3_4b4b v69 = iconst.i32 0xffff_ffff_f4f4_d3d3 v70 = iconst.i64 0xf4f4_f4f4_f4f4_f4f4 v71 = iconst.i128 0 v72 = iconst.i64 0 v73 = iconst.i32 0 v74 = iconst.i16 0 v75 = iconst.i8 0 stack_store v71, ss0 ; v71 = 0 stack_store v71, ss0+16 ; v71 = 0 stack_store v71, ss0+32 ; v71 = 0 stack_store v71, ss0+48 ; v71 = 0 stack_store v72, ss0+64 ; v72 = 0 stack_store v74, ss0+72 ; v74 = 0 stack_store v75, ss0+74 ; v75 = 0 stack_store v71, ss1 ; v71 = 0 stack_store v71, ss1+16 ; v71 = 0 stack_store v71, ss1+32 ; v71 = 0 stack_store v71, ss1+48 ; v71 = 0 stack_store v72, ss1+64 ; v72 = 0 stack_store v74, ss1+72 ; v74 = 0 stack_store v75, ss1+74 ; v75 = 0 stack_store v71, ss2 ; v71 = 0 stack_store v71, ss2+16 ; v71 = 0 stack_store v71, ss2+32 ; v71 = 0 stack_store v71, ss2+48 ; v71 = 0 stack_store v72, ss2+64 ; v72 = 0 stack_store v74, ss2+72 ; v74 = 0 stack_store v75, ss2+74 ; v75 = 0 v76 = rotl v9, v10 v77 = rotl v76, v10 v78 = rotl v77, v10 v79 = rotl v78, v10 v80 = rotl v79, v10 v81 = rotl v80, v10 v82 = rotl v81, v10 v83 = rotl v82, v10 v84 = rotl v83, v10 v85 = rotl v84, v10 v86 = rotl v85, v10 v87 = rotl v86, v10 v88 = rotl v87, v10 v89 = ishl v12, v0 v245 = floor v11 v246 = fcmp ne v245, v245 v247 = f64const +NaN v90 = select v246, v247, v245 ; v247 = +NaN v248 = floor v90 v249 = fcmp ne v248, v248 v250 = f64const +NaN v91 = select v249, v250, v248 ; v250 = +NaN v92 = stack_load.i64 ss0+51 nop v93 = rotr v67, v67 ; v67 = 0x4b4b_4b4b, v67 = 0x4b4b_4b4b v251 = fadd v7, v7 v252 = fcmp ne v251, v251 v253 = f32const +NaN v94 = select v252, v253, v251 ; v253 = +NaN v95 = iadd v0, v0 v96 = stack_addr.i64 ss0+4 istore8 v69, v96 ; v69 = 0xffff_ffff_f4f4_d3d3 nop v97 = fcmp ord v91, v91 stack_store v95, ss2+42 v98 = ushr v88, v2 nop nop v99 = ushr v10, v98 v100 = stack_addr.i64 ss0+22 v101 = uload8.i16 v100+36 v102 = rotl v88, v99 v103 = rotl v102, v99 v104 = rotl v103, v99 v105 = rotl v104, v99 v106 = rotl v105, v99 v107 = rotl v106, v99 v108 = rotl v107, v99 v109 = rotl v108, v99 v110 = rotl v109, v99 v111 = rotl v110, v99 v112 = rotl v111, v99 v113 = rotl v112, v99 v114 = rotl v113, v99 v115 = rotl v114, v99 v116 = rotl v115, v99 v117 = rotl v116, v99 v118 = rotl v117, v99 v119 = rotl v118, v99 v120 = rotl v119, v99 return v13, v64, v70, v94, v95, v97, v95 ; v64 = 0x4b4b_4b4b, v70 = 0xf4f4_f4f4_f4f4_f4f4 block1(v15: i32, v16: i32, v17: i32, v18: i32, v19: i32, v20: i32, v21: i32, v121: i32, v122: i64, v124: i8, v125: i16, v127: f32, v128: f64, v129: i128, v220: i64, v224: i16, v228: i32, v232: b1) cold: v123 = rotr v121, v122 v126 = ishl v124, v125 jump block6(v127, v128, v126, v129, v127, v125, v129, v220, v224, v228, v122, v232) block2(v22: i32, v23: i32, v24: i32, v25: i32, v26: i32, v27: i32, v28: i32) cold: brz.i64 v131, block1(v28, v28, v130, v23, v23, v23, v23, v130, v197, v134, v136, v132, v133, v135, v131, v225, v229, v233) ; v130 = 0, v130 = 0, v132 = 0.0, v229 = 0, v233 = false jump block6(v132, v133, v134, v135, v132, v136, v135, v131, v225, v229, v197, v233) ; v132 = 0.0, v132 = 0.0, v229 = 0, v233 = false block3(v29: i32, v30: i32, v31: f64, v32: f64, v33: f64, v34: f64, v35: f64) cold: v133 -> v35 v139 = sshr.i16 v137, v138 v140 = sshr v139, v138 v136 -> v140 v142 = sshr v140, v141 ; v141 = 0 v225 -> v142 v144 = rotl.i64 v141, v143 ; v141 = 0 v145 = rotl v144, v143 v146 = rotl v145, v143 v147 = rotl v146, v143 v148 = rotl v147, v143 v149 = rotl v148, v143 v197 -> v149 v151 = ishl.i32 v150, v150 ; v150 = 0, v150 = 0 brz v142, block2(v152, v152, v152, v152, v152, v152, v152) ; v152 = 0, v152 = 0, v152 = 0, v152 = 0, v152 = 0, v152 = 0, v152 = 0 jump block2(v152, v152, v152, v153, v153, v153, v151) ; v152 = 0, v152 = 0, v152 = 0, v153 = 0, v153 = 0, v153 = 0 block4 cold: v235 = bconst.b1 false v233 -> v235 v234 -> v235 v231 = iconst.i32 0 v229 -> v231 v230 -> v231 v214 = f32const 0.0 v200 -> v214 v202 -> v200 v132 -> v202 v213 = iconst.i32 0 v174 -> v213 v152 -> v174 v130 -> v152 v212 = iconst.i64 0 v173 -> v212 v141 -> v173 v211 = iconst.i32 0 v172 -> v211 v210 = iconst.i32 0 v171 -> v210 v153 -> v171 v209 = iconst.i32 0 v168 -> v209 v150 -> v168 v208 = f64const 0.0 v165 -> v208 v207 = iconst.i16 0 v163 -> v207 v206 = iconst.i128 0 v162 -> v206 v205 = iconst.i8 0 v159 -> v205 v204 = iconst.i16 0 v155 -> v204 v154 = stack_load.i64 ss1+7 v156 = sshr v155, v154 ; v155 = 0 v157 = sshr v156, v154 v158 = rotr v157, v157 nop v160 = ishl v158, v159 ; v159 = 0 v161 = iadd v159, v159 ; v159 = 0, v159 = 0 v203 -> v161 v134 -> v203 nop nop v164 = ishl v162, v163 ; v162 = 0, v163 = 0 v143 -> v164 v135 -> v143 nop nop v254 = trunc v165 ; v165 = 0.0 v255 = fcmp ne v254, v254 v256 = f64const +NaN v166 = select v255, v256, v254 ; v256 = +NaN v167 = sdiv v160, v160 v137 -> v167 v169 = sshr v154, v168 ; v168 = 0 v138 -> v169 v131 -> v138 v170 = stack_load.i64 ss1+10 br_icmp sge v173, v169, block1(v171, v172, v172, v172, v172, v172, v172, v174, v173, v161, v167, v200, v166, v164, v169, v163, v230, v234) ; v173 = 0, v171 = 0, v172 = 0, v172 = 0, v172 = 0, v172 = 0, v172 = 0, v172 = 0, v174 = 0, v173 = 0, v200 = 0.0, v163 = 0, v230 = 0, v234 = false jump block3(v171, v174, v166, v166, v166, v166, v166) ; v171 = 0, v174 = 0 block5(v36: i32, v37: i32, v38: i32, v39: i32, v40: i32, v41: i32, v42: i32): v238 = bconst.b1 false v237 -> v238 v227 = iconst.i16 0 v226 -> v227 v223 = iconst.i64 0 v222 -> v223 v219 = iconst.i16 0 v183 -> v219 v218 = iconst.i128 0 v182 -> v218 v217 = iconst.i8 0 v181 -> v217 v216 = f64const 0.0 v180 -> v216 v215 = f32const 0.0 v179 -> v215 v175 = stack_load.i64 ss1+7 v176 = stack_load.i64 ss1+7 v177 = stack_load.i64 ss1+7 v178 = stack_load.i64 ss1+7 jump block6(v179, v180, v181, v182, v179, v183, v182, v222, v226, v38, v178, v237) ; v179 = 0.0, v180 = 0.0, v181 = 0, v182 = 0, v179 = 0.0, v183 = 0, v182 = 0, v222 = 0, v226 = 0, v237 = false block6(v43: f32, v44: f64, v45: i8, v46: i128, v47: f32, v48: i16, v49: i128, v184: i64, v190: i16, v193: i32, v194: i64, v195: b1) cold: v185 = sshr v48, v184 v186 = ishl v45, v185 v187 = ishl v186, v185 nop v188 = ishl v185, v187 v189 = iadd v187, v187 nop nop v191 = ishl v49, v190 v192 = fcopysign v44, v44 return v193, v193, v194, v47, v189, v195, v189 block7(v50: i16, v51: i128, v52: f64, v53: f64, v54: f64, v55: i16, v56: i8, v57: i32, v58: i32, v59: i32, v60: i32, v61: i32, v62: i32, v63: i32): v244 = bconst.b1 false v236 -> v244 v243 = iconst.i64 0 v221 -> v243 v242 = f32const 0.0 v201 -> v242 v241 = iconst.i16 0 v199 -> v241 v240 = iconst.i64 0 v198 -> v240 v239 = iconst.i32 0 v196 -> v239 br_icmp ule v56, v56, block1(v62 [message truncated]
Wasmtime GitHub notifications bot (Sep 01 2022 at 00:42):jameysharp commented on issue #4824:
...nope, it's just #4828 again. Here's the part that's actually producing different answers:
test interpret test run set enable_llvm_abi_extensions target aarch64 target s390x target x86_64 function %fn(f64 uext) -> b1 system_v { block0(v11: f64): v245 = floor v11 v246 = fcmp ne v245, v245 v247 = f64const +NaN v90 = select v246, v247, v245 ; v247 = +NaN v248 = floor v90 v249 = fcmp ne v248, v248 v250 = f64const +NaN v91 = select v249, v250, v248 ; v250 = +NaN v97 = fcmp ord v91, v91 return v97 } ; run: %fn(-NaN:0x7ffffff000000) == false
Wasmtime GitHub notifications bot (Sep 01 2022 at 00:54):jameysharp commented on issue #4824:
On further reflection, I guess I don't want to merge cranelift-fuzzgen input format changes whenever there are open bugs from OSS-Fuzz, because I think it'll close the existing reports as "fixed" and then open new ones when it rediscovers them.
So I've run this branch long enough locally that I feel good about merging it, but I'm going to hold off until #4828 is fixed.
Wasmtime GitHub notifications bot (Sep 01 2022 at 09:43):afonso360 commented on issue #4824:
I started refactoring it a couple weeks ago but haven't had time to finish it. @afonso360, maybe you could take a look at the direction I was going and see what you think?
Those refactors look really nice! I do slightly like our current huge list of legal opcodes table since while its quite verbose its really easy to understand how it works, but I'm not opposed to changing it.
It is also not the final design! One of the ideas we had originally in #3050 was to update
cranelift-metato generate us a list of valid opcode/type combos that we can use. That to me seems like the best design to ensure that we don't accidentally not fuzz some opcode. Although we are still a bit far from being able to use it.I was planning on finishing up all the scalar opcodes (small task :big_smile: ) and then switch to that and ignore all SIMD types and iterate from there.
Wasmtime GitHub notifications bot (Sep 01 2022 at 09:44):afonso360 edited a comment on issue #4824:
I started refactoring it a couple weeks ago but haven't had time to finish it. @afonso360, maybe you could take a look at the direction I was going and see what you think?
Those refactors look really nice! I do slightly like our current huge list of legal opcodes table since while its quite verbose its really easy to understand how it works, but I'm not opposed to changing it.
It is also not the final design! One of the ideas we had originally in #3050 was to update
cranelift-metato generate us a list of valid opcode/type combos that we can use. That to me seems like the best design to ensure that we don't accidentally not fuzz some opcode. Although we are still a bit far from being able to use it.I was planning on finishing up all the scalar opcodes (small task :big_smile: ) and then switch to that and ignore all SIMD types and iterate on SIMD from there.
Wasmtime GitHub notifications bot (Sep 01 2022 at 09:44):afonso360 edited a comment on issue #4824:
I started refactoring it a couple weeks ago but haven't had time to finish it. @afonso360, maybe you could take a look at the direction I was going and see what you think?
Those refactors look really nice! I do slightly like our current huge list of legal opcodes table since while its quite verbose its really easy to understand how it works, but I'm not opposed to changing it.
It is also not the final design! One of the ideas we had originally in #3050 was to update
cranelift-metato generate us a list of valid opcode/type combos that we can use. That to me seems like the best design to ensure that we don't accidentally not fuzz some opcode.I was planning on finishing up all the scalar opcodes (small task :big_smile: ) and then switch to that and ignore all SIMD types and iterate on SIMD from there.
Wasmtime GitHub notifications bot (Sep 01 2022 at 09:53):afonso360 edited a comment on issue #4824:
I started refactoring it a couple weeks ago but haven't had time to finish it. @afonso360, maybe you could take a look at the direction I was going and see what you think?
Those refactors look really nice! I do slightly like our current huge list of legal opcodes table since while its quite verbose its really easy to understand how it works, but I'm not opposed to changing it.
It is also not the final design! One of the ideas we had originally in #3050 was to update
cranelift-metato generate us a list of valid opcode/type combos that we can use. That to me seems like the best design to ensure that we don't accidentally not fuzz some opcode.I was planning on finishing up all the scalar opcodes (small task :big_smile: ), switch to that and then iterate on SIMD from there.
Wasmtime GitHub notifications bot (Sep 01 2022 at 09:53):afonso360 edited a comment on issue #4824:
I started refactoring it a couple weeks ago but haven't had time to finish it. @afonso360, maybe you could take a look at the direction I was going and see what you think?
Those refactors look really nice! I do slightly like our current huge list of legal opcodes table since while its quite verbose its really easy to understand how it works, but I'm not opposed to changing it.
It is also not the final design! One of the ideas we had originally in #3050 was to update
cranelift-metato generate us a list of valid opcode/type combos that we can use. That to me seems like the best design to ensure that we don't accidentally not fuzz some opcode.I was planning on finishing up all the scalar opcodes (small task :big_smile:), switch to that and then iterate on SIMD from there.
Wasmtime GitHub notifications bot (Sep 01 2022 at 09:55):afonso360 edited a comment on issue #4824:
I started refactoring it a couple weeks ago but haven't had time to finish it. @afonso360, maybe you could take a look at the direction I was going and see what you think?
Those refactors look really nice! I do like our current huge list of legal opcodes table since while its quite verbose its really easy to understand how it works, but I'm not opposed to changing it.
It is also not the final design! One of the ideas we had originally in #3050 was to update
cranelift-metato generate us a list of valid opcode/type combos that we can use. That to me seems like the best design to ensure that we don't accidentally not fuzz some opcode.I was planning on finishing up all the scalar opcodes (small task :big_smile:), switch to that and then iterate on SIMD from there.
Wasmtime GitHub notifications bot (Sep 01 2022 at 09:59):afonso360 edited a comment on issue #4824:
I started refactoring it a couple weeks ago but haven't had time to finish it. @afonso360, maybe you could take a look at the direction I was going and see what you think?
Those refactors look really nice! I do like our current huge list of legal opcodes table since while its quite verbose its really easy to understand how it works, but I'm not opposed to changing it.
It is also not the final design! One of the ideas we had originally in #3050 was to update
cranelift-metato generate us a list of valid opcode/type combos that we can use. That to me seems like the best design to ensure that we don't accidentally not fuzz some opcode.I was planning on finishing up all the scalar opcodes (small task :big_smile:), switch to that and then iterate on SIMD from there. Although we can do that at any point, since we just have to blacklist the unimplemented opcodes/types.
Wasmtime GitHub notifications bot (Sep 02 2022 at 18:40):jameysharp commented on issue #4824:
With #4828 fixed by #4849, now I just want to wait for OSS-Fuzz to confirm the fix and then I'll merge this.
Although we'll see if it catches the new fuzzgen bug introduced in #4849, in which case I'll wait for it to see our fix for _that_ before merging this. :sweat_smile:
Meanwhile I'm running cranelift-fuzzgen locally on this PR (plus the other fixes) until it's time to merge this.
Wasmtime GitHub notifications bot (Sep 03 2022 at 02:07):jameysharp commented on issue #4824:
Those refactors look really nice! I do like our current huge list of legal opcodes table since while its quite verbose its really easy to understand how it works, but I'm not opposed to changing it.
The problem I set out to solve was that currently when we select an opcode from the list, we might then fail to generate anything because we don't have any variables of the right type, or any stack slots of sufficient size, or any funcrefs that we can actually call. So I wanted to do a single pass over the opcodes to identify which ones we're able to generate, given the resources we've generated up front. Then selecting from that filtered list produces choices that we're guaranteed to be able to add to the current function.
You could still use something like the current opcodes table, as long as you have a way of filtering it to ensure that all the resources needed are available for each opcode. I don't think just input/output type signatures are sufficient for that.
I think this is important because I suspect that we're currently discarding a lot of inputs due to, say, not generating any I128 variables up front but then trying to insert an I128 "add", or things like that. I'm betting that we'll find more bugs with the same number of test executions if we fix this. But I haven't done any measurements to check this guess.
Either way, I think it's nice to apply the patches that allow just borrowing slices everywhere. So I've opened #4863 with those. The remaining work-in-progress part is no longer on that branch but is still accessible as 0c9689ba978276bd925b47db0d20623a118699fb.
It is also not the final design! One of the ideas we had originally in #3050 was to update
cranelift-metato generate us a list of valid opcode/type combos that we can use. That to me seems like the best design to ensure that we don't accidentally not fuzz some opcode.I was planning on finishing up all the scalar opcodes (small task smile), switch to that and then iterate on SIMD from there. Although we can do that at any point, since we just have to blacklist the unimplemented opcodes/types.
That sounds great! Right now the blocklist feels like it's going to be kinda long, but maybe it's better to get it all at once. Besides, cranelift-meta's instruction formats tell us what resources each opcode needs available, which should help with my concern above...
Wasmtime GitHub notifications bot (Sep 04 2022 at 20:21):afonso360 commented on issue #4824:
The problem I set out to solve was that currently when we select an opcode from the list, we might then fail to generate anything because we don't have any variables of the right type, or any stack slots of sufficient size, or any funcrefs that we can actually call. So I wanted to do a single pass over the opcodes to identify which ones we're able to generate, given the resources we've generated up front. Then selecting from that filtered list produces choices that we're guaranteed to be able to add to the current function.
I sort of missed this when I first read that commit :sweat_smile: , I thought we were just generating the opcode table programmatically.
And you are right! It is a big issue with fuzzgen I did some measurements and we lose about 60% of our inputs due to invalid input formats.
You could still use something like the current opcodes table, as long as you have a way of filtering it to ensure that all the resources needed are available for each opcode. I don't think just input/output type signatures are sufficient for that.
Is there any opcode that you think could be problematic? I'm not seeing why we couldn't do that based on signature.
I think this is important because I suspect that we're currently discarding a lot of inputs due to, say, not generating any I128 variables up front but then trying to insert an I128 "add", or things like that. I'm betting that we'll find more bugs with the same number of test executions if we fix this. But I haven't done any measurements to check this guess.
Either way, I think it's nice to apply the patches that allow just borrowing slices everywhere. So I've opened https://github.com/bytecodealliance/wasmtime/pull/4863 with those. The remaining work-in-progress part is no longer on that branch but is still accessible as https://github.com/bytecodealliance/wasmtime/commit/0c9689ba978276bd925b47db0d20623a118699fb.
I did some measurements on these! With #4863 and the WIP commit we get a 3-4% improvement in the number of valid inputs that we generate (vs total inputs). While these are not earth shattering numbers, I think we definitely should either commit this or something like it.
Wasmtime GitHub notifications bot (Sep 04 2022 at 20:29):afonso360 edited a comment on issue #4824:
The problem I set out to solve was that currently when we select an opcode from the list, we might then fail to generate anything because we don't have any variables of the right type, or any stack slots of sufficient size, or any funcrefs that we can actually call. So I wanted to do a single pass over the opcodes to identify which ones we're able to generate, given the resources we've generated up front. Then selecting from that filtered list produces choices that we're guaranteed to be able to add to the current function.
I sort of missed this when I first read that commit :sweat_smile: , I thought we were just generating the opcode table programmatically.
And you are right! It is a big issue with fuzzgen I did some measurements and we lose about 60% of our inputs due to invalid input formats.
You could still use something like the current opcodes table, as long as you have a way of filtering it to ensure that all the resources needed are available for each opcode. I don't think just input/output type signatures are sufficient for that.
Is there any opcode that you think could be problematic? I'm not seeing why we couldn't do that based on signature.
I think this is important because I suspect that we're currently discarding a lot of inputs due to, say, not generating any I128 variables up front but then trying to insert an I128 "add", or things like that. I'm betting that we'll find more bugs with the same number of test executions if we fix this. But I haven't done any measurements to check this guess.
Either way, I think it's nice to apply the patches that allow just borrowing slices everywhere. So I've opened https://github.com/bytecodealliance/wasmtime/pull/4863 with those. The remaining work-in-progress part is no longer on that branch but is still accessible as https://github.com/bytecodealliance/wasmtime/commit/0c9689ba978276bd925b47db0d20623a118699fb.
I did some measurements on these! With #4863 and the WIP commit we get a 3-4% improvement in the number of valid inputs that we generate (vs total inputs). While these are not earth shattering numbers, I think we definitely should either commit this or something like it.
See the "Better Instruction Selection" section on this comment with the full measurements that I did on these patches.
Wasmtime GitHub notifications bot (Sep 07 2022 at 15:59):jameysharp commented on issue #4824:
I'm going to give this another hour of fuzzing when merged with the latest changes on main and then I hope to merge it today!
Wasmtime GitHub notifications bot (Sep 07 2022 at 18:02):jameysharp commented on issue #4824:
My meeting ran long, so I got close to half a million fuzz target executions. I didn't find any bugs in that time so I'm definitely calling that good enough.
Last updated: Nov 22 2024 at 17:03 UTC