Skip to content

Commit 3638c44

Browse files
committed
Fix compiler backend audit findings
1 parent 8973fff commit 3638c44

9 files changed

Lines changed: 679 additions & 59 deletions

File tree

‎src/codegen/abi.rs‎

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ use crate::error::Result;
1212
use crate::ir::*;
1313
use crate::ENTRY_WITNESS_ABI_MAGIC;
1414

15+
use super::assembler::align_up;
1516
use super::{
1617
fixed_aggregate_pointer_param_width, fixed_byte_pointer_param_width, fixed_register_width, named_type_name, type_static_length,
1718
CellScriptRuntimeError, CodeGenerator,
@@ -80,6 +81,15 @@ pub(crate) fn entry_abi_arg_count(params: &[IrParam], abi: Option<&CallableAbi>)
8081
.sum()
8182
}
8283

84+
pub(crate) fn outgoing_stack_arg_bytes(abi_arg_count: usize) -> usize {
85+
let bytes = abi_arg_count.saturating_sub(8) * 8;
86+
if bytes == 0 {
87+
0
88+
} else {
89+
align_up(bytes, 16)
90+
}
91+
}
92+
8393
pub(crate) fn call_param_abi_arg_count(param: &IrParam, needs_type_hash: bool) -> usize {
8494
if named_type_name(&param.ty).is_some() {
8595
return 2 + usize::from(needs_type_hash) * 2;
@@ -211,7 +221,7 @@ impl CodeGenerator {
211221
let callable_abi = self.callable_abis.get(target).cloned();
212222
let type_hash_param_indices = callable_abi.as_ref().map(|abi| abi.type_hash_param_indices.clone()).unwrap_or_default();
213223
let runtime_bound_param_indices = callable_abi.as_ref().map(|abi| abi.runtime_bound_param_indices.clone()).unwrap_or_default();
214-
let outgoing_stack_arg_bytes = entry_abi_arg_count(params, callable_abi.as_ref()).saturating_sub(8) * 8;
224+
let outgoing_stack_arg_bytes = outgoing_stack_arg_bytes(entry_abi_arg_count(params, callable_abi.as_ref()));
215225
let payload = entry_witness_payload_layout(params, &runtime_bound_param_indices);
216226
let payload_len = payload.iter().map(|arg| arg.width).sum::<usize>();
217227
let has_witness_payload = payload.iter().any(|arg| arg.width > 0 || arg.unsupported);
@@ -388,9 +398,15 @@ impl CodeGenerator {
388398
self.emit(format!("addi t0, t0, {}", ENTRY_WITNESS_BUFFER_OFFSET));
389399
if abi_index < 8 {
390400
self.emit_entry_witness_scalar_load_from_reg(&format!("a{}", abi_index), "t0", width);
401+
if param.ty == IrType::Bool {
402+
self.emit_entry_bool_canonical_check(&format!("a{}", abi_index), &fail_label);
403+
}
391404
} else {
392405
let caller_stack_offset = (abi_index - 8) * 8;
393406
self.emit_entry_witness_scalar_load_from_reg("t3", "t0", width);
407+
if param.ty == IrType::Bool {
408+
self.emit_entry_bool_canonical_check("t3", &fail_label);
409+
}
394410
self.emit(format!(
395411
"# cellscript entry abi: scalar param {} stored to caller stack +{}",
396412
param.name, caller_stack_offset
@@ -480,9 +496,15 @@ impl CodeGenerator {
480496
let stack_offset = ENTRY_WITNESS_BUFFER_OFFSET + ENTRY_WITNESS_HEADER_SIZE + payload_cursor;
481497
if abi_index < 8 {
482498
self.emit_entry_witness_scalar_load(&format!("a{}", abi_index), stack_offset, width);
499+
if param.ty == IrType::Bool {
500+
self.emit_entry_bool_canonical_check(&format!("a{}", abi_index), &fail_label);
501+
}
483502
} else {
484503
let caller_stack_offset = (abi_index - 8) * 8;
485504
self.emit_entry_witness_scalar_load("t3", stack_offset, width);
505+
if param.ty == IrType::Bool {
506+
self.emit_entry_bool_canonical_check("t3", &fail_label);
507+
}
486508
self.emit(format!(
487509
"# cellscript entry abi: scalar param {} stored to caller stack +{}",
488510
param.name, caller_stack_offset
@@ -606,6 +628,17 @@ impl CodeGenerator {
606628
}
607629
}
608630

631+
fn emit_entry_bool_canonical_check(&mut self, register: &str, fail_label: &str) {
632+
let zero_ok = self.fresh_label("entry_bool_zero_ok");
633+
let bool_ok = self.fresh_label("entry_bool_canonical_ok");
634+
self.emit(format!("beqz {}, {}", register, zero_ok));
635+
self.emit("li t2, 1");
636+
self.emit(format!("beq {}, t2, {}", register, bool_ok));
637+
self.emit(format!("j {}", fail_label));
638+
self.emit_label(&zero_ok);
639+
self.emit_label(&bool_ok);
640+
}
641+
609642
fn emit_entry_load_u32_from_stack(&mut self, dest_reg: &str, stack_offset: usize) {
610643
self.emit(format!("li {}, 0", dest_reg));
611644
for byte_index in 0..4 {
@@ -729,9 +762,15 @@ impl CodeGenerator {
729762
*abi_index += 2;
730763
} else if *abi_index < 8 {
731764
self.emit_entry_witness_scalar_load_from_reg(&format!("a{}", *abi_index), "t0", width);
765+
if param.ty == IrType::Bool {
766+
self.emit_entry_bool_canonical_check(&format!("a{}", *abi_index), fail_label);
767+
}
732768
*abi_index += 1;
733769
} else {
734770
self.emit_entry_witness_scalar_load_from_reg("t4", "t0", width);
771+
if param.ty == IrType::Bool {
772+
self.emit_entry_bool_canonical_check("t4", fail_label);
773+
}
735774
self.emit_entry_abi_reg_arg(*abi_index, "t4", outgoing_stack_arg_bytes);
736775
*abi_index += 1;
737776
}

‎src/codegen/assembler.rs‎

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -633,6 +633,12 @@ impl ParsedAssembly {
633633
if !signed_bits_fit(relative_offset(pc, target)?, 13) {
634634
relaxed.insert(index);
635635
}
636+
} else if let Instruction::Jump { label } = inst {
637+
let pc = layout.text_user_base + offset as u64;
638+
let target = self.symbol_address(label, layout)?;
639+
if !signed_bits_fit(relative_offset(pc, target)?, 21) {
640+
relaxed.insert(index);
641+
}
636642
}
637643
}
638644
offset += op_size(op, offset, SectionKind::Text, index, BranchSizeMode::Conservative);
@@ -1417,7 +1423,11 @@ fn encode_instruction(
14171423
}
14181424
Instruction::Jump { label } => {
14191425
let target = parsed.symbol_address(label, layout)?;
1420-
out.extend_from_slice(&encode_j_type(0x6f, 0, relative_offset(pc, target)?)?.to_le_bytes());
1426+
if relaxed_branch {
1427+
encode_long_jump_sequence(out, pc, target)?;
1428+
} else {
1429+
out.extend_from_slice(&encode_j_type(0x6f, 0, relative_offset(pc, target)?)?.to_le_bytes());
1430+
}
14211431
}
14221432
Instruction::Beq { .. }
14231433
| Instruction::Bne { .. }
@@ -1430,8 +1440,8 @@ fn encode_instruction(
14301440
let (rs1, rs2, label, funct3) = conditional_branch_parts(inst).expect("conditional branch parts");
14311441
let target = parsed.symbol_address(label, layout)?;
14321442
if relaxed_branch {
1433-
out.extend_from_slice(&encode_b_type(0x63, inverse_branch_funct3(funct3), rs1, rs2, 8)?.to_le_bytes());
1434-
out.extend_from_slice(&encode_j_type(0x6f, 0, relative_offset(pc + 4, target)?)?.to_le_bytes());
1443+
out.extend_from_slice(&encode_b_type(0x63, inverse_branch_funct3(funct3), rs1, rs2, 12)?.to_le_bytes());
1444+
encode_long_jump_sequence(out, pc + 4, target)?;
14351445
} else {
14361446
out.extend_from_slice(&encode_b_type(0x63, funct3, rs1, rs2, relative_offset(pc, target)?)?.to_le_bytes());
14371447
}
@@ -1493,12 +1503,29 @@ fn encode_call_sequence(out: &mut Vec<u8>, pc: u64, target: u64) -> Result<()> {
14931503
Ok(())
14941504
}
14951505

1506+
fn encode_long_jump_sequence(out: &mut Vec<u8>, pc: u64, target: u64) -> Result<()> {
1507+
let scratch = 31;
1508+
let offset = relative_offset(pc, target)?;
1509+
if offset % 2 != 0 {
1510+
return Err(CompileError::new("jump target is not 2-byte aligned", crate::error::Span::default()));
1511+
}
1512+
let (hi, lo) = split_hi_lo(offset)?;
1513+
out.extend_from_slice(&encode_u_type(0x17, scratch, hi).to_le_bytes());
1514+
out.extend_from_slice(&encode_i_type(0x67, 0, 0b000, scratch, lo)?.to_le_bytes());
1515+
Ok(())
1516+
}
1517+
14961518
fn op_size(op: &AsmOp, current_offset: usize, section: SectionKind, op_index: usize, branch_size_mode: BranchSizeMode<'_>) -> usize {
14971519
match op {
14981520
AsmOp::Label(_) => 0,
14991521
AsmOp::Instruction(Instruction::Li { imm, .. }) => li_sequence_size(*imm),
15001522
AsmOp::Instruction(Instruction::La { .. }) => 8,
15011523
AsmOp::Instruction(Instruction::Call { .. }) => 8,
1524+
AsmOp::Instruction(Instruction::Jump { .. }) => match branch_size_mode {
1525+
BranchSizeMode::Conservative => 8,
1526+
BranchSizeMode::Exact(relaxed) if section == SectionKind::Text && relaxed.contains(&op_index) => 8,
1527+
BranchSizeMode::Exact(_) => 4,
1528+
},
15021529
AsmOp::Instruction(
15031530
Instruction::Beq { .. }
15041531
| Instruction::Bne { .. }
@@ -1509,8 +1536,8 @@ fn op_size(op: &AsmOp, current_offset: usize, section: SectionKind, op_index: us
15091536
| Instruction::Beqz { .. }
15101537
| Instruction::Bnez { .. },
15111538
) => match branch_size_mode {
1512-
BranchSizeMode::Conservative => 8,
1513-
BranchSizeMode::Exact(relaxed) if section == SectionKind::Text && relaxed.contains(&op_index) => 8,
1539+
BranchSizeMode::Conservative => 12,
1540+
BranchSizeMode::Exact(relaxed) if section == SectionKind::Text && relaxed.contains(&op_index) => 12,
15141541
BranchSizeMode::Exact(_) => 4,
15151542
},
15161543
AsmOp::Instruction(_) => 4,

‎src/codegen/calls.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use crate::error::{CompileError, Result};
99
use crate::ir::*;
1010
use crate::runtime_errors::CellScriptRuntimeError;
1111

12-
use super::abi::{abi_arg_label, call_abi_arg_count, CallLengthKind};
12+
use super::abi::{abi_arg_label, call_abi_arg_count, outgoing_stack_arg_bytes, CallLengthKind};
1313
use super::assembler::{scratch_register_avoiding, small_signed_immediate};
1414
use super::runtime::is_ckb_fixed_hash_helper;
1515
use super::schema::{fixed_aggregate_pointer_param_width, fixed_byte_pointer_param_width, named_type_name};
@@ -71,7 +71,7 @@ impl CodeGenerator {
7171
self.emit(format!("# call {}", func));
7272

7373
let abi = self.callable_abis.get(func).cloned();
74-
let outgoing_stack_arg_bytes = call_abi_arg_count(abi.as_ref(), args).saturating_sub(8) * 8;
74+
let outgoing_stack_arg_bytes = outgoing_stack_arg_bytes(call_abi_arg_count(abi.as_ref(), args));
7575
let mut abi_index = 0usize;
7676
for (arg_index, arg) in args.iter().enumerate() {
7777
if let Some(abi) = &abi {

‎src/codegen/mod.rs‎

Lines changed: 83 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@
5858
5959
use crate::ast::{BinaryOp, ParamSource, UnaryOp};
6060
use crate::codegen::cell_ops::consumed_operand_var;
61-
use crate::error::Result;
61+
use crate::error::{CompileError, Result};
6262

6363
use crate::ir::*;
6464
use crate::runtime_errors::CellScriptRuntimeError;
@@ -1807,7 +1807,27 @@ impl CodeGenerator {
18071807
}
18081808
IrTerminator::Return(Some(operand)) => {
18091809
if let IrOperand::Var(v) = operand {
1810+
if let IrType::Tuple(items) = &v.ty {
1811+
if items.len() > 8 {
1812+
return Err(CompileError::new(
1813+
format!("tuple return ABI supports at most 8 fields, but return value has {} fields", items.len()),
1814+
crate::error::Span::default(),
1815+
));
1816+
}
1817+
if !self.tuple_aggregate_fields.contains_key(&v.id) {
1818+
return Err(CompileError::new(
1819+
"tuple return ABI requires a directly materialized tuple aggregate",
1820+
crate::error::Span::default(),
1821+
));
1822+
}
1823+
}
18101824
if let Some(fields) = self.tuple_aggregate_fields.get(&v.id).cloned() {
1825+
if fields.len() > 8 {
1826+
return Err(CompileError::new(
1827+
format!("tuple return ABI supports at most 8 fields, but return value has {} fields", fields.len()),
1828+
crate::error::Span::default(),
1829+
));
1830+
}
18111831
self.emit(format!("# cellscript abi: return tuple aggregate var{} fields={}", v.id, fields.len()));
18121832
if fields.is_empty() {
18131833
self.emit("li a0, 0");
@@ -2411,6 +2431,35 @@ mod tests {
24112431
("tail", "tail target"),
24122432
];
24132433

2434+
#[test]
2435+
fn outgoing_stack_arg_area_is_16_byte_aligned_at_call_boundaries() {
2436+
let cases = [(0, 0), (1, 0), (8, 0), (9, 16), (10, 16), (11, 32), (12, 32), (13, 48), (16, 64)];
2437+
2438+
for (abi_arg_count, expected_bytes) in cases {
2439+
let bytes = super::abi::outgoing_stack_arg_bytes(abi_arg_count);
2440+
assert_eq!(bytes, expected_bytes, "unexpected outgoing stack size for {} ABI args", abi_arg_count);
2441+
if bytes > 0 {
2442+
assert_eq!(bytes % 16, 0, "outgoing stack area must preserve RISC-V psABI sp alignment");
2443+
}
2444+
}
2445+
}
2446+
2447+
#[test]
2448+
fn internal_assembler_keeps_near_unconditional_jump_compact() {
2449+
let lines = vec![
2450+
".section .text".to_string(),
2451+
".global entry".to_string(),
2452+
"entry:".to_string(),
2453+
"j done".to_string(),
2454+
"done:".to_string(),
2455+
"ret".to_string(),
2456+
];
2457+
2458+
let plan = MachineLayoutPlan::build(&lines).expect("near unconditional jump should assemble");
2459+
assert_eq!(plan.metrics.text_size, 8, "near j should remain a compact 4-byte jal x0 sequence");
2460+
assert_eq!(plan.metrics.relaxed_branch_count, 0, "near j should not be marked as a relaxed long jump");
2461+
}
2462+
24142463
#[test]
24152464
fn internal_assembler_relaxes_out_of_range_conditional_branch() {
24162465
let mut lines = vec![
@@ -2430,6 +2479,39 @@ mod tests {
24302479
assert!(elf.starts_with(b"\x7fELF"));
24312480
}
24322481

2482+
#[test]
2483+
fn internal_assembler_encodes_far_unconditional_jump() {
2484+
let lines = vec![
2485+
".section .text".to_string(),
2486+
".global entry".to_string(),
2487+
"entry:".to_string(),
2488+
"j far_target".to_string(),
2489+
format!(".ascii \"{}\"", "x".repeat(1_100_000)),
2490+
"far_target:".to_string(),
2491+
"ret".to_string(),
2492+
];
2493+
2494+
let elf = assemble_elf_internal(&lines).expect("internal assembler should encode far unconditional jumps");
2495+
assert!(elf.starts_with(b"\x7fELF"));
2496+
}
2497+
2498+
#[test]
2499+
fn internal_assembler_relaxes_far_conditional_branch_with_long_jump() {
2500+
let lines = vec![
2501+
".section .text".to_string(),
2502+
".global entry".to_string(),
2503+
"entry:".to_string(),
2504+
"li a0, 0".to_string(),
2505+
"beqz a0, far_target".to_string(),
2506+
format!(".ascii \"{}\"", "x".repeat(1_100_000)),
2507+
"far_target:".to_string(),
2508+
"ret".to_string(),
2509+
];
2510+
2511+
let elf = assemble_elf_internal(&lines).expect("internal assembler should relax far conditional branches through long jumps");
2512+
assert!(elf.starts_with(b"\x7fELF"));
2513+
}
2514+
24332515
#[test]
24342516
fn internal_assembler_encodes_register_conditional_branches() {
24352517
for mnemonic in ["beq", "bne", "blt", "bge", "bltu", "bgeu"] {

0 commit comments

Comments
 (0)