Skip to content

Commit

Permalink
allow spell comments in TH08 to have no null
Browse files Browse the repository at this point in the history
Some files were failing to recompile at all because
apparently some of the spell comments use all 64 bytes.
This is safe because they are memcpied (not strcpied)
into a larger buffer that is zero initialized, but... ffs.
  • Loading branch information
ExpHP committed Dec 19, 2021
1 parent ff16155 commit 2b65a58
Show file tree
Hide file tree
Showing 8 changed files with 87 additions and 8 deletions.
2 changes: 1 addition & 1 deletion src/core_mapfiles/ecl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -609,7 +609,7 @@ static ECL_08_09: &'static CoreSignatures = &CoreSignatures {
(Th08, 119, Some("Sfff")),
(Th08, 120, Some("S")),
(Th08, 121, Some("S")),
(Th08, 122, Some("ssSm(len=48;mask=0xaa,0,0)m(len=48;mask=0xbb,0,0)m(len=64;mask=0xdd,0,0)m(len=64;mask=0xee,0,0)")),
(Th08, 122, Some("ssSm(len=48;mask=0xaa,0,0)m(len=48;mask=0xbb,0,0)m(len=64;nulless;mask=0xdd,0,0)m(len=64;nulless;mask=0xee,0,0)")),
(Th08, 123, Some("")),
(Th08, 124, Some("S")),
(Th08, 125, Some("S")),
Expand Down
8 changes: 5 additions & 3 deletions src/llir/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,11 @@ pub enum ArgEncoding {
pub enum StringArgSize {
/// A string arg that uses `len=`.
///
/// A fixed length string buffer. A trailing null is required to be present INSIDE the buffer
/// as in some cases it may be copied to another buffer of identical length.
Fixed { len: usize },
/// A fixed length string buffer. Typically a trailing null is required to be present INSIDE the
/// buffer as in some cases it may be copied to another buffer of identical length.
/// `;nulless` may be added for specific cases where it is known that the lack of a trailing
/// null is not an issue.
Fixed { len: usize, nulless: bool },
/// A string arg that uses `bs=`.
///
/// A null-terminated string argument which **can only be the final argument**, and
Expand Down
13 changes: 11 additions & 2 deletions src/llir/lower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,16 @@ fn encode_args(

// convert to Shift-JIS or whatever
let mut encoded = Encoded::encode(&sp!(arg.span => string), DEFAULT_ENCODING).map_err(|e| emitter.emit(e))?;
encoded.0.push(b'\0'); // have to do this eagerly to correctly reproduce TH17 Extra files

// have to append null eagerly to correctly reproduce TH17 Extra files
match size_spec {
| StringArgSize::Block { .. }
| StringArgSize::Fixed { nulless: false, .. }
=> encoded.0.push(b'\0'),

| StringArgSize::Fixed { nulless: true, .. }
=> {}
}

// the furigana bug appends a copy of the masked bytes from a previous furigana string
if furibug {
Expand All @@ -557,7 +566,7 @@ fn encode_args(
encoded.null_pad(block_size);
}
},
StringArgSize::Fixed { len } => {
StringArgSize::Fixed { len, nulless: _ } => {
if encoded.len() > len {
return Err(emitter.emit(error!(
message("string argument too large for buffer"),
Expand Down
9 changes: 8 additions & 1 deletion src/llir/raise/early.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,13 +254,20 @@ fn decode_args_with_abi(
=> {
let read_len = match size_spec {
StringArgSize::Block { .. } => remaining_len, // read to end
StringArgSize::Fixed { len } => len,
StringArgSize::Fixed { len, nulless: _ } => len,
};
decrease_len(emitter, &mut remaining_len, read_len)?;

let mut encoded = Encoded(args_blob.read_byte_vec(read_len).expect("already checked len"));
encoded.apply_xor_mask(mask);

if let StringArgSize::Fixed { nulless: true, .. } = size_spec {
// suppress the warning about missing nulls by adding one now if missing
if !encoded.0.contains(&b'\0') {
encoded.0.push(b'\0');
}
};

let warn_on_trimmed_data = !furibug; // furibug DOES leave garbage after the null
encoded.trim_first_nul(emitter, warn_on_trimmed_data);

Expand Down
6 changes: 5 additions & 1 deletion src/parse/abi.lalrpop
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,12 @@ MString: ArgEncoding = {
},
};

#[inline]
StringArgSize: StringArgSize = {
"len" "=" <x:UnsignedInt> => StringArgSize::Fixed { len: x as _ },
"len" "=" <len:UnsignedInt> <nulless:(";" "nulless")?> => StringArgSize::Fixed {
len: len as _,
nulless: nulless.is_some(),
},
"bs" "=" <x:UnsignedInt> => StringArgSize::Block { block_size: x as _ },
};

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
source: tests/integration/strings.rs
expression: stderr
---
error: string argument too large for buffer
┌─ <input>:10:13
10 │ nulless("abcdefghi");
│ ^^^^^^^^^^^ requires 9 bytes
= this argument is written to a 8-byte buffer


Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
source: tests/integration/strings.rs
expression: stderr
---
warning: <compiled-file>: in sub0: instr 0 (opcode 666, offset 0x0): argument 1: string will be truncated at first null


37 changes: 37 additions & 0 deletions tests/integration/strings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,11 @@ const STRING_ABI_TEST_SIGNATURES: &'static str = r#"!eclmap
!ins_names
444 block
555 fixed
666 nulless
!ins_signatures
444 z(bs=4)
555 z(len=8)
666 z(len=8;nulless)
"#;

source_test!(
Expand Down Expand Up @@ -151,6 +153,14 @@ source_test!(
require_roundtrip: false,
);

source_test!(
ECL_06, decompile_string_arg_with_null_4,
mapfile: STRING_ABI_TEST_SIGNATURES,
main_body: r#" nulless("abc\0def"); "#,
expect_decompile_warning: "first null",
require_roundtrip: false,
);

source_test!(
ECL_06, decompile_string_arg_without_null_bs,
mapfile: STRING_ABI_TEST_SIGNATURES,
Expand All @@ -167,6 +177,13 @@ source_test!(
recompile: false,
);

source_test!(
ECL_06, decompile_string_arg_without_null_nulless,
mapfile: STRING_ABI_TEST_SIGNATURES,
main_body: r#" nulless(@blob="40404040 40404040"); "#,
check_decompiled: |text| assert!(text.contains("\"@@@@@@@@\"")),
);

source_test!(
ECL_06, compile_string_arg_too_big_eq,
mapfile: STRING_ABI_TEST_SIGNATURES,
Expand All @@ -182,3 +199,23 @@ source_test!(
fixed("abcdefghi"); //~ ERROR too large
"#,
);

source_test!(
ECL_06, compile_string_arg_too_big_eq_nulless,
mapfile: STRING_ABI_TEST_SIGNATURES,
main_body: r#"
nulless("abcdefgh");
"#,
check_compiled: |output, format| {
let ecl = output.read_ecl(format);
assert_eq!(ecl.subs[0][0].args_blob, b"abcdefgh");
}
);

source_test!(
ECL_06, compile_string_arg_too_big_gt_nulless,
mapfile: STRING_ABI_TEST_SIGNATURES,
main_body: r#"
nulless("abcdefghi"); //~ ERROR too large
"#,
);

0 comments on commit 2b65a58

Please sign in to comment.