From e6459a0af4ac1b0bfd69e41c9b11d3f6703d5233 Mon Sep 17 00:00:00 2001 From: Alcaro Date: Sun, 25 Dec 2016 11:46:18 +0100 Subject: [PATCH] Modernize BPS patcher --- patch/bps.cpp | 211 ++++++++++++++++++------------------------------- patch/patch.h | 111 +++++++++++++------------- patch/test.cpp | 38 +++++---- patch/ups.cpp | 56 +++++-------- 4 files changed, 173 insertions(+), 243 deletions(-) diff --git a/patch/bps.cpp b/patch/bps.cpp index 54f733c..c8c5947 100644 --- a/patch/bps.cpp +++ b/patch/bps.cpp @@ -1,94 +1,51 @@ #include "patch.h" -namespace patch { namespace bps { -//TODO: HEAVY cleanups needed here -static uint32_t read32(uint8_t * ptr) -{ - uint32_t out; - out =ptr[0]; - out|=ptr[1]<<8; - out|=ptr[2]<<16; - out|=ptr[3]<<24; - return out; -} +//Things I would've done differently if I had a chance to redesign BPS: +//- Don't allow encoding -0 in Source/TargetCopy +//- Ditch metadata, it goes in a separate file +//- Reconsider SourceRead; maybe patches would be smaller if of the three others was one bit rather than two, or maybe a new command +// or maybe only Read/Copy commands? Read is TargetRead, Copy treats target as concatenated to source +//- Invert 0x80 bit in the encoded numbers, set means continue; it would simplify the decoder +//- Replace BPS1 signature with something not containing an 1 +// while DWORD alignment sounds nice, it's useless for a byte-oriented format like this; even the checksums aren't aligned +// four-byte signatures are nicer than three, but '1' is the wrong choice for the last byte; PNG's \x89 would work +//- Make the checksums mandatory +// (1) Ignoring them allows all files of that size, including ones that are clearly not the proper source +// (2) Even if a ROM hacker is careful to only change a few bytes, BPS likes copying stuff around, +// so the patch could break due to changes to unrelated and unpredictable locations +// (3) Patches designed to cooperate are most likely written in assembly, not binary, and should be shared in that format. +// Closed source patches help nobody. +namespace patch { namespace bps { enum { SourceRead, TargetRead, SourceCopy, TargetCopy }; -static bool try_add(size_t& a, size_t b) -{ - if (SIZE_MAX-a < b) return false; - a+=b; - return true; -} - -static bool try_shift(size_t& a, size_t b) -{ - if (SIZE_MAX>>b < a) return false; - a<<=b; - return true; -} - -static bool decodenum(const uint8_t*& ptr, size_t& out) -{ - out=0; - unsigned int shift=0; - while (true) - { - uint8_t next=*ptr++; - size_t addthis=(next&0x7F); - if (shift) addthis++; - if (!try_shift(addthis, shift)) return false; - // unchecked because if it was shifted, the lowest bit is zero, and if not, it's <=0x7F. - if (!try_add(out, addthis)) return false; - if (next&0x80) return true; - shift+=7; - } -} - #define error(which) do { error=which; goto exit; } while(0) -#define assert_sum(a,b) do { if (SIZE_MAX-(a)<(b)) error(bps_too_big); } while(0) -#define assert_shift(a,b) do { if (SIZE_MAX>>(b)<(a)) error(bps_too_big); } while(0) -result apply(const file& patch_, const file& source_, file& target_, bool accept_wrong_input) +result apply(arrayview patchmem, arrayview in, array& out, bool accept_wrong_input) { - struct mem patch = patch_.mmap(); - struct mem in = source_.mmap(); - struct mem out_; - struct mem * out = &out_; - struct mem * metadata = NULL; + if (patchmem.size()<4+3+12) return e_broken; result error = e_ok; - out->len=0; - out->ptr=NULL; - if (metadata) - { - metadata->len=0; - metadata->ptr=NULL; - } - if (patch.len<4+3+12) return e_broken; + memstream patch = patchmem.slice(0, patchmem.size()-12); if (true) { -#define read8() (*(patchat++)) #define decodeto(var) \ do { \ - if (!decodenum(patchat, var)) error(e_too_big); \ + if (!patch.bpsnum(var)) error(e_too_big); \ } while(false) -#define write8(byte) (*(outat++)=byte) - const uint8_t * patchat=patch.ptr; - const uint8_t * patchend=patch.ptr+patch.len-12; + if (patch.u8()!='B') error(e_broken); + if (patch.u8()!='P') error(e_broken); + if (patch.u8()!='S') error(e_broken); + if (patch.u8()!='1') error(e_broken); - if (read8()!='B') error(e_broken); - if (read8()!='P') error(e_broken); - if (read8()!='S') error(e_broken); - if (read8()!='1') error(e_broken); + memstream checks = patchmem.slice(patchmem.size()-12, 12); + uint32_t crc_in_e = checks.u32(); + uint32_t crc_out_e = checks.u32(); + uint32_t crc_patch_e = checks.u32(); - uint32_t crc_in_e = read32(patch.ptr+patch.len-12); - uint32_t crc_out_e = read32(patch.ptr+patch.len-8); - uint32_t crc_patch_e = read32(patch.ptr+patch.len-4); - - uint32_t crc_in_a = crc32(in.v()); - uint32_t crc_patch_a = crc32(patch.v().slice(0, patch.len-4)); + uint32_t crc_in_a = crc32(in); + uint32_t crc_patch_a = crc32(patchmem.slice(0, patchmem.size()-4)); if (crc_patch_a != crc_patch_e) error(e_broken); @@ -98,64 +55,44 @@ result apply(const file& patch_, const file& source_, file& target_, bool accept size_t outlen; decodeto(outlen); - if (inlen!=in.len || crc_in_a!=crc_in_e) + if (inlen!=in.size() || (crc_in_a!=crc_in_e && !accept_wrong_input)) { - if (in.len==outlen && crc_in_a==crc_out_e) error=e_to_output; + if (in.size()==outlen && crc_in_a==crc_out_e) error=e_to_output; else error=e_not_this; - if (!accept_wrong_input) goto exit; + if (inlen==in.size() && !accept_wrong_input) goto exit; } - out->len=outlen; - out->ptr=(uint8_t*)malloc(outlen); + array outmem; + outmem.reserve_noinit(outlen); + membufwriter out = outmem; - const uint8_t * instart=in.ptr; - const uint8_t * inreadat=in.ptr; - const uint8_t * inend=in.ptr+in.len; - - uint8_t * outstart=out->ptr; - uint8_t * outreadat=out->ptr; - uint8_t * outat=out->ptr; - uint8_t * outend=out->ptr+out->len; + size_t inreadat = 0; + size_t outreadat = 0; size_t metadatalen; decodeto(metadatalen); + patch.bytes(metadatalen); // discard this, grab it from info::parse - if (metadata && metadatalen) - { - metadata->len=metadatalen; - metadata->ptr=(uint8_t*)malloc(metadatalen+1); - for (size_t i=0;iptr[i]=read8(); - metadata->ptr[metadatalen]='\0';//just to be on the safe side - that metadata is assumed to be text, might as well terminate it - } - else - { - for (size_t i=0;i>2)+1; int action=(thisinstr&3); - if (outat+length>outend) error(e_broken); + if (length > out.remaining()) error(e_broken); switch (action) { case SourceRead: { - if (outat-outstart+length > in.len) error(e_broken); - for (size_t i=0;i in.size()) error(e_broken); + out.write(in.slice(out.pos(), length)); } break; case TargetRead: { - if (patchat+length>patchend) error(e_broken); - for (size_t i=0;i patch.remaining()) error(e_broken); + out.write(patch.bytes(length)); } break; case SourceCopy: @@ -163,11 +100,19 @@ result apply(const file& patch_, const file& source_, file& target_, bool accept size_t encodeddistance; decodeto(encodeddistance); size_t distance=encodeddistance>>1; - if ((encodeddistance&1)==0) inreadat+=distance; - else inreadat-=distance; + if ((encodeddistance&1)==0) + { + if (inreadat+length > in.size()) error(e_broken); + inreadat+=distance; + } + else + { + if (distance > inreadat) error(e_broken); + inreadat-=distance; + } - if (inreadatinend) error(e_broken); - for (size_t i=0;i>1; - if ((encodeddistance&1)==0) outreadat+=distance; - else outreadat-=distance; + if ((encodeddistance&1)==0) + { + if (distance+outreadat > out.pos()) error(e_broken); + if (outreadat+length > out.size()) error(e_broken); + outreadat+=distance; + } + else + { + if (distance > outreadat) error(e_broken); + outreadat-=distance; + } - if (outreadat=outat || outreadat+length>outend) error(e_broken); - for (size_t i=0;iv()); + uint32_t crc_out_a = crc32(outmem); if (crc_out_a!=crc_out_e) { @@ -195,28 +148,12 @@ result apply(const file& patch_, const file& source_, file& target_, bool accept if (!accept_wrong_input) goto exit; } - target_.write(out->v()); - free(out->ptr); - patch_.unmap(patch.v()); - source_.unmap(in.v()); return error; -#undef read8 #undef decodeto -#undef write8 } exit: - free(out->ptr); - patch_.unmap(patch.v()); - source_.unmap(in.v()); - out->len=0; - out->ptr=NULL; - if (metadata) - { - free(metadata->ptr); - metadata->len=0; - metadata->ptr=NULL; - } + out.resize(0); return error; } #undef error @@ -224,6 +161,7 @@ exit: +/* result info::parse(const file& patch, bool changefrac) { size_t len = patch.size(); @@ -305,6 +243,7 @@ result info::parse(const file& patch, bool changefrac) return e_ok; } +*/ diff --git a/patch/patch.h b/patch/patch.h index ad14f8d..207a3b0 100644 --- a/patch/patch.h +++ b/patch/patch.h @@ -34,18 +34,20 @@ static inline result create(const file& source, const file& target, file&& patch } namespace ups { -result apply(const file& patch, const file& source, file& target); -static inline result apply(const file& patch, const file& source, file&& target) { return apply(patch, source, (file&)target); } +result apply(arrayview patch, const file& in, array& out); +static inline result apply(arrayview patch, arrayview in, array& out) +{ + file inf = file::mem(in); + return apply(patch, inf, out); +} //no need to implement this //result create(const file& source, const file& target, file& patch); } namespace bps { -result apply(const file& patch, const file& source, file& target, bool accept_wrong_input = false); -static inline result apply(const file& patch, const file& source, file&& target, bool accept_wrong_input = false) -{ - return apply(patch, source, (file&)target, accept_wrong_input); -} +//metadata is extracted through info::parse +result apply(arrayview patch, arrayview source, array& target, bool accept_wrong_input = false); + //Because this one can take quite a long time, a progress meter is supplied. total is guaranteed to // be constant between every call until this function returns, done is guaranteed to increase // between each call, and done/total is an approximate percentage counter. Anything else is @@ -72,7 +74,7 @@ static inline result create(const file& source, const file& target, file&& patch } struct info { - result parse(const file& patch, bool changefrac = false); + result parse(arrayview data, bool changefrac = false); size_t size_in; size_t size_out; @@ -80,7 +82,7 @@ struct info { uint32_t crc_in; uint32_t crc_out; - array metadata; + arrayview metadata; //Tells approximately how much of the input ROM is changed compared to the output ROM. //It's quite heuristic. The algorithm may change with or without notice. @@ -129,68 +131,54 @@ public: arrayview b = bytes(4); return b[0] | b[1]<<8 | b[2]<<16 | b[3]<<24; } + uint32_t u32at(size_t pos) + { + const byte* b = start+pos; + return b[0] | b[1]<<8 | b[2]<<16 | b[3]<<24; + } size_t size() { return end-start; } size_t remaining() { return end-at; } //if the bpsnum is too big, number of read bytes is undefined //does not do bounds checks, there must be at least 10 unread bytes in the buffer - safeint bpsnum() + bool bpsnum(size_t& out) { - //similar to uleb128, but uleb lacks the +1 that ensures there's only one way to encode an integer - uint8_t first = u8(); - if (LIKELY(first&0x80)) return first&0x7F; + //similar to uleb128, but bpsnum adds another 1< ret = 0; - safeint shift = 0; + //really heavily optimized, so it looks a bit weird + uint8_t b = *(at++); + if (LIKELY(b&0x80)) + { + out = b ^ (1<<7); + return true; + } + size_t tmp = b; + b = *(at++); + tmp |= b<<7; + if (LIKELY(b&0x80)) + { + out = tmp + (1<<7) - (1<<7<<7); + return true; + } + + //these weird subtractions and additions wouldn't be needed if the 0x80 bits were inverted + //but I can't change the BPS spec, so they'll have to stay + size_t ret = tmp + (1<<7) + (1<<7<<7); + size_t shift = 7+7; while (true) { + uint8_t next = *(at++); + if (safeint::lslov(next^0x80, shift, &tmp)) return false; + if (safeint::addov(ret, tmp, &ret)) return false; + if (next&0x80) break; shift+=7; - ret+=1< shifted = (next&0x7F)< buf; - size_t totalbytes; - - uint32_t crc; - - void flush() - { - crc = crc32_update(buf, crc); - f.write(buf, fpos); - fpos += buf.size(); - buf.reset(); - } - -public: - filebufwriter(file& f) : f(f), fpos(0), totalbytes(0), crc(0) {} - void write(arrayview bytes) - { - buf += bytes; - totalbytes += bytes.size(); - if (buf.size() > 65536) flush(); - } - void write(byte b) - { - buf.append(b); - totalbytes++; - if (buf.size() > 65536) flush(); - } - size_t size() { return totalbytes; } - uint32_t crc32() { flush(); return crc; } - void cancel() { f.resize(0); } -}; class membufwriter { arrayvieww buf; size_t bufpos; @@ -209,7 +197,14 @@ public: { buf[bufpos++] = b; } - size_t size() { return bufpos; } + void write_xor(byte b) + { + buf[bufpos++] ^= b; + } + void write_skip(size_t bytes) { bufpos += bytes; } + size_t pos() { return bufpos; } + size_t size() { return buf.size(); } + size_t remaining() { return buf.size()-bufpos; } uint32_t crc32() { crc = crc32_update(buf.slice(crcpos, bufpos-crcpos), crc); diff --git a/patch/test.cpp b/patch/test.cpp index b8ffbcd..d72defd 100644 --- a/patch/test.cpp +++ b/patch/test.cpp @@ -1,5 +1,9 @@ #include "patch.h" +/* +make clean; rm callgrind.out.*; make test -j8 TESTRUNNER='time valgrind --tool=callgrind' CFLAGS='-Os -g' && kcachegrind callgrind.out.* +*/ + namespace patch { //test("filebufreader") //{ @@ -63,17 +67,16 @@ static void createtest(arrayview a, arrayview b, size_t ipssize, siz array b2; r = ips::apply(file::mem(patch), file::mem(a), file::mem(b2)); if (r!=e_to_output) assert_eq(r, e_ok); - assert_eq(b2.size(), b.size()); - for (size_t i=0;i b2; - r = bps::apply(file::mem(patch), file::mem(a), file::mem(b2)); + r = bps::apply(patch, a, b2); if (r!=e_to_output) assert_eq(r, e_ok); - assert_eq(b2.size(), b.size()); - for (size_t i=0;i sm64 = file::read("patch/test/sm64.z64"); array sm64_bps = file::read("patch/test/star.bps"); if (!smw || !smw_bps || !dl || !dl_ups || !sm64 || !sm64_bps) test_skip("test files not present; see patch/test/readme.txt"); + result r; array smwhack; - bps::apply(file::mem(smw_bps), file::mem(smw), file::mem(smwhack)); + r = bps::apply(smw_bps, smw, smwhack); + assert_eq(r, e_ok); //testcall(createtest(smw, smwhack, 3302980, 2077386)); //array sm64hack; - //bps::apply(file::mem(sm64_bps), file::mem(sm64), file::mem(sm64hack)); + //r = bps::apply(file::mem(sm64_bps), file::mem(sm64), file::mem(sm64hack)); + //assert_eq(r, e_ok); //testcall(createtest(sm64, sm64hack, -1, 6788133)); //this is the only UPS test, UPS is pretty much an easter egg in Flips //array dlhack; - //ups::apply(file::mem(dl_ups), file::mem(dl), file::mem(dlhack)); + //r = ups::apply(dl_ups, dl, dlhack); + //assert_eq(r, e_ok); + //array dl2; + //r = ups::apply(dl_ups, dlhack, dl2); + //assert_eq(r, e_ok); + //assert(dl == dl2); //testcall(createtest(dl, dlhack, 852134, 817190)); } } diff --git a/patch/ups.cpp b/patch/ups.cpp index ca8b07b..11d3b81 100644 --- a/patch/ups.cpp +++ b/patch/ups.cpp @@ -4,24 +4,18 @@ namespace patch { namespace ups { //TODO: HEAVY cleanups needed here #define error(which) do { error=which; goto exit; } while(0) -result apply(const file& patch_, const file& source_, file& target_) +result apply(arrayview patchmem, const file& in, array& outmem) { - if (patch_.size()<4+2+12) return e_broken; - - arrayview patchmem = patch_.mmap(); - memstream patch = patchmem; - arrayview inmem = source_.mmap(); - memstream in = inmem; + if (patchmem.size()<4+2+12) return e_broken; + memstream patch = patchmem.slice(0, patchmem.size()-12); result error; if (true) { #define decodeto(var) \ do { \ - safeint ret = patch.bpsnum(); \ - if (!ret.valid()) error(e_too_big); \ - var = ret.val(); \ + if (!patch.bpsnum(var)) error(e_too_big); \ } while(false) bool backwards=false; @@ -44,42 +38,36 @@ result apply(const file& patch_, const file& source_, file& target_) } if (inlen!=in.size()) error(e_not_this); - array outmem; + outmem = in.read(); + uint32_t crc_in = crc32(outmem); outmem.resize(outlen); membufwriter out = outmem; - while (patch.remaining() > 12) + while (patch.remaining()) { size_t skip; decodeto(skip); - size_t skip_fast = min(skip, outlen-out.size(), in.remaining()); - out.write(in.bytes(skip_fast)); - skip -= skip_fast; - while (skip>0) - { - uint8_t outb = in.u8_or(0); - if (out.size()