Fix cppia JIT "Bad move target" converting an int subtraction to a string - #1369
Fix cppia JIT "Bad move target" converting an int subtraction to a string#1369MeguminBOT wants to merge 2 commits into
Conversation
CppiaCompiler::convert moves between two untyped registers in three places. getCommonType(jtAny, jtAny) returns jtAny, which move() rejects. The cppia test suite covers it, run with -jit.
OpSub::genCode handed convert a bare sJitTemp1 where its sibling OpMult::genCode passes sJitTemp1.as(jtInt). That is the whole bug: with OpSub fixed the test passes with CppiaCompiler.cpp untouched. The 2-byte array read in ArrayBuiltin has the same omission against its byte-sized sibling, though nothing instantiates ArrayBuiltin with a 2-byte element, so that block never compiles. The convert hardening stays as defence in depth, since convert is handed inSrcType and every other move in it already types both sides from it. The etObject to etFloat branch guarded on inSrc==sJitTemp1, but JitVal equality includes type, so only a bare untyped sJitTemp1 ever matched. A typed R1 source fell through to a path where makeAddress overwrote R1 before objToFloat read it. Nothing produces that shape today, so this is correctness on an unreachable path rather than a live fix. It now guards on inSrc.uses(SLJIT_R1) like the etObject to etString branch above it, spills once, and shares the memory and non-memory target paths. Tests cover the reported crash, both of the other OpSub destinations, and the three convert paths touched here. They cannot isolate the hardening from the OpSub fix, since with either one present nothing hands convert an untyped register. Emitted sljit code is unchanged: the whole cppia test client jits to identical LIR before and after.
d55281d to
cb07560
Compare
|
Small update.
I kept the three I also rewrote the The tests are reworked. Since this is all in the JIT I wanted to be sure it doesn't shift any generated code, so I built |
|
Also the MacOS arm64 test seems to be very random wheter it passes or not. |
Remake of #1366
The problem
With the JIT on, a cppia module containing an ordinary integer expression fails to load:
The failure is at module boot, because JIT compilation runs over the whole module there, so one
expression takes the entire module down. cppia with jit off runs the same code correctly.
Why
OpSub::genCodehandsconverta baresJitTemp1, whereOpMult::genCoderight above it passessJitTemp1.as(jtInt):Inside
convert, theetInttoetStringcase moves that untyped source into an untypedsJitArg0:Neither side has a width.
getCommonType(jtAny, jtAny)returnsjtAny, whichmove()rejects withsetError("Bad move target"). That is thrown, caught inCppiaModule, and re-raised as the loaderror above.
Only a String destination trips it.
etFloatgoes throughSLJIT_CONV_F64_FROM_S32andetObjectthrough
intToObj, and neither of those callsmove."" + (a * b)does not trip it either, sinceOpMulttypes its source.The fix
One token in
src/hx/cppia/Cppia.cpp, soOpSubmatchesOpMult:That alone fixes the crash with
CppiaCompiler.cppuntouched. Three other things ride along:ArrayBuiltinread has the same omission against its byte-sized sibling. Nothinginstantiates
ArrayBuiltinwith a 2-byte element, so that block never compiles. Consistency only.CppiaCompiler.cppstill gets the three widths, as hardening rather than as the fix.convertishanded
inSrcTypeand every other move in it already types both sides from that, so these threewere the ones that had been missed.
etObjecttoetFloatbranch guarded oninSrc==sJitTemp1, butJitVal::operator==comparestype, so only a bare untypedsJitTemp1ever matched. A typedR1source fell through to a pathwhere
makeAddressstompsR1beforeobjToFloatreads it. It guards oninSrc.uses(SLJIT_R1)now, like the
etObjecttoetStringbranch above it. Nothing produces that shape today either,so this is correctness on an unreachable path.
Test
test/cppiacovers it.ClientJitConvertinClient.hxholds the cases andcases/TestCommon.hxchecks the answers: the crash itself, both of the other
OpSubdestinations, and the threeconvertpaths touched here.
The
-jitmatters, andRunTests.hxalready runs the suite both ways. 27/27 either way.Without the fix the
-jitrun fails atsetupClass failed: Bad move targetand exits 1, since themodule never loads. The same build without
-jitreportsALL TESTS OK.Worth saying plainly: the tests cannot isolate the hardening from the
OpSubfix. With either one inplace nothing hands
convertan untyped register, so no Haxe code can tell them apart. RevertingCppiaCompiler.cppcompletely withOpSubfixed still passes everything. To check the new cases arenot vacuous I dropped a line from the branch I rewrote, and exactly one test failed.
Since this is all in the JIT, I also checked it moves no generated code. Master and this branch were
built as two hosts and run against the same
client.cppia, dumping the sljit LIR throughsljit_compiler_verbose: 2601 lines each, zero differences once the pointer immediates that shiftunder ASLR are normalised. That follows from
getDatanever readingtypeandgetTargetreading itonly to choose between
maxFTempCountandmaxTempCount, wherejtAny,jtIntandjtPointeralltake the same branch.
Reproducing by hand
Built with
haxe -m Script --cppia script.cppiaand loaded from a host built with-D scriptable --dce no, callingcpp.cppia.Host.enableJit(true)beforecpp.cppia.Module.fromData(bytes).boot().Error : Bad move targetat boot11Pre-edit description:
The problem
With the JIT on, a cppia module containing an ordinary integer expression fails to load:
The failure is at module boot, because JIT compilation runs over the whole module there, so one
expression takes the entire module down. cppia with jit off runs the same code correctly.
Why
CppiaCompiler::convertmoves between two untyped registers in three places, for example whenconverting an
Intto aString:Neither side is given a width. When both operands are untyped,
getCommonType(jtAny, jtAny)returnsjtAny, whichmove()rejects withsetError("Bad move target"). That is thrown, caught inCppiaModule, and re-raised as the load error above.It needs both conditions at once, which is why it is easy to miss: the source has to be in
R1anduntyped. An expression like
"" + (a * b)reaches the branch but with a typed source, and"" + ints[0]has an untyped source but does not reach the branch.The fix
In
src/hx/cppia/CppiaCompiler.cpp, give both sides a width:Test
test/cppiacovers it.ClientUntypedMoveinClient.hxsubtracts one array element from anotherinto a string, and
testUntypedRegisterMoveincases/TestCommon.hxchecks the answer.The
-jitmatters, andRunTests.hxalready runs the suite both ways.Without the fix, the
-jitrun fails atsetupClass failed: Bad move targetand exits 1, since themodule never loads. The same build without
-jitreportsALL TESTS OK.Reproducing by hand
Built with
haxe -m Script --cppia script.cppiaand loaded from a host built with-D scriptable --dce no, callingcpp.cppia.Host.enableJit(true)beforecpp.cppia.Module.fromData(bytes).boot().Error : Bad move targetat boot11