Skip to content

Commit

Permalink
read/line: check for overflow when advancing the address (#731)
Browse files Browse the repository at this point in the history 
Addresses in line sequences should only increase.
The `addr2line` crate relies on this for its binary search.
  • Loading branch information
@philipc
philipc committed Jul 4, 2024
1 parent 9257192 commit 450cb69
Show file tree
Hide file tree
Showing 2 changed files with 99 additions and 43 deletions.
137 changes: 96 additions & 41 deletions src/read/line.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ where
Err(err) => return Err(err),
Ok(None) => return Ok(None),
Ok(Some(instruction)) => {
if self.row.execute(instruction, &mut self.program) {
if self.row.execute(instruction, &mut self.program)? {
if self.row.tombstone {
// Perform any reset that was required for the tombstone row.
// Normally this is done when `next_row` is called again, but for
Expand Down Expand Up @@ -631,7 +631,7 @@ pub type LineNumberRow = LineRow;
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub struct LineRow {
tombstone: bool,
address: Wrapping<u64>,
address: u64,
op_index: Wrapping<u64>,
file: u64,
line: Wrapping<u64>,
Expand All @@ -652,7 +652,7 @@ impl LineRow {
// "At the beginning of each sequence within a line number program, the
// state of the registers is:" -- Section 6.2.2
tombstone: false,
address: Wrapping(0),
address: 0,
op_index: Wrapping(0),
file: 1,
line: Wrapping(1),
Expand All @@ -676,7 +676,7 @@ impl LineRow {
/// generated by the compiler."
#[inline]
pub fn address(&self) -> u64 {
self.address.0
self.address
}

/// > An unsigned integer representing the index of an operation within a VLIW
Expand Down Expand Up @@ -800,21 +800,21 @@ impl LineRow {
&mut self,
instruction: LineInstruction<R>,
program: &mut Program,
) -> bool
) -> Result<bool>
where
Program: LineProgram<R>,
R: Reader,
{
match instruction {
Ok(match instruction {
LineInstruction::Special(opcode) => {
self.exec_special_opcode(opcode, program.header());
self.exec_special_opcode(opcode, program.header())?;
true
}

LineInstruction::Copy => true,

LineInstruction::AdvancePc(operation_advance) => {
self.apply_operation_advance(operation_advance, program.header());
self.apply_operation_advance(operation_advance, program.header())?;
false
}

Expand Down Expand Up @@ -846,13 +846,18 @@ impl LineRow {
LineInstruction::ConstAddPc => {
let adjusted = self.adjust_opcode(255, program.header());
let operation_advance = adjusted / program.header().line_encoding.line_range;
self.apply_operation_advance(u64::from(operation_advance), program.header());
self.apply_operation_advance(u64::from(operation_advance), program.header())?;
false
}

LineInstruction::FixedAddPc(operand) => {
self.address += Wrapping(u64::from(operand));
self.op_index.0 = 0;
if !self.tombstone {
self.address = self
.address
.checked_add(u64::from(operand))
.ok_or(Error::AddressOverflow)?;
self.op_index.0 = 0;
}
false
}

Expand All @@ -879,8 +884,13 @@ impl LineRow {
LineInstruction::SetAddress(address) => {
let tombstone_address = !0 >> (64 - program.header().encoding.address_size * 8);
self.tombstone = address == tombstone_address;
self.address.0 = address;
self.op_index.0 = 0;
if !self.tombstone {
if address < self.address {
return Err(Error::InvalidAddressRange);
}
self.address = address;
self.op_index.0 = 0;
}
false
}

Expand All @@ -899,7 +909,7 @@ impl LineRow {
| LineInstruction::UnknownStandard1(_, _)
| LineInstruction::UnknownStandardN(_, _)
| LineInstruction::UnknownExtended(_, _) => false,
}
})
}

/// Perform any reset that was required after copying the previous row.
Expand Down Expand Up @@ -940,7 +950,11 @@ impl LineRow {
&mut self,
operation_advance: u64,
header: &LineProgramHeader<R>,
) {
) -> Result<()> {
if self.tombstone {
return Ok(());
}

let operation_advance = Wrapping(operation_advance);

let minimum_instruction_length = u64::from(header.line_encoding.minimum_instruction_length);
Expand All @@ -950,15 +964,20 @@ impl LineRow {
u64::from(header.line_encoding.maximum_operations_per_instruction);
let maximum_operations_per_instruction = Wrapping(maximum_operations_per_instruction);

if maximum_operations_per_instruction.0 == 1 {
self.address += minimum_instruction_length * operation_advance;
let address_advance = if maximum_operations_per_instruction.0 == 1 {
self.op_index.0 = 0;
minimum_instruction_length * operation_advance
} else {
let op_index_with_advance = self.op_index + operation_advance;
self.address += minimum_instruction_length
* (op_index_with_advance / maximum_operations_per_instruction);
self.op_index = op_index_with_advance % maximum_operations_per_instruction;
}
minimum_instruction_length
* (op_index_with_advance / maximum_operations_per_instruction)
};
self.address = self
.address
.checked_add(address_advance.0)
.ok_or(Error::AddressOverflow)?;
Ok(())
}

#[inline]
Expand All @@ -967,7 +986,11 @@ impl LineRow {
}

/// Section 6.2.5.1
fn exec_special_opcode<R: Reader>(&mut self, opcode: u8, header: &LineProgramHeader<R>) {
fn exec_special_opcode<R: Reader>(
&mut self,
opcode: u8,
header: &LineProgramHeader<R>,
) -> Result<()> {
let adjusted_opcode = self.adjust_opcode(opcode, header);

let line_range = header.line_encoding.line_range;
Expand All @@ -979,7 +1002,8 @@ impl LineRow {
self.apply_line_advance(line_base + i64::from(line_advance));

// Step 2
self.apply_operation_advance(u64::from(operation_advance), header);
self.apply_operation_advance(u64::from(operation_advance), header)?;
Ok(())
}
}

Expand Down Expand Up @@ -2480,7 +2504,7 @@ mod tests {
let mut program = IncompleteLineProgram { header };
let is_new_row = registers.execute(opcode, &mut program);

assert_eq!(is_new_row, expect_new_row);
assert_eq!(is_new_row, Ok(expect_new_row));
assert_eq!(registers, expected_registers);
}

Expand Down Expand Up @@ -2533,7 +2557,7 @@ mod tests {
let opcode = LineInstruction::Special(52);

let mut expected_registers = initial_registers;
expected_registers.address.0 += 3;
expected_registers.address += 3;

assert_exec_opcode(header, initial_registers, opcode, expected_registers, true);
}
Expand All @@ -2547,7 +2571,7 @@ mod tests {
let opcode = LineInstruction::Special(55);

let mut expected_registers = initial_registers;
expected_registers.address.0 += 3;
expected_registers.address += 3;
expected_registers.line.0 += 3;

assert_exec_opcode(header, initial_registers, opcode, expected_registers, true);
Expand All @@ -2563,7 +2587,7 @@ mod tests {
let opcode = LineInstruction::Special(49);

let mut expected_registers = initial_registers;
expected_registers.address.0 += 3;
expected_registers.address += 3;
expected_registers.line.0 -= 3;

assert_exec_opcode(header, initial_registers, opcode, expected_registers, true);
Expand Down Expand Up @@ -2592,7 +2616,7 @@ mod tests {
let header = make_test_header(EndianSlice::new(&[], LittleEndian));

let mut initial_registers = LineRow::new(&header);
initial_registers.address.0 = 1337;
initial_registers.address = 1337;
initial_registers.line.0 = 42;

let opcode = LineInstruction::Copy;
Expand All @@ -2609,23 +2633,20 @@ mod tests {
let opcode = LineInstruction::AdvancePc(42);

let mut expected_registers = initial_registers;
expected_registers.address.0 += 42;
expected_registers.address += 42;

assert_exec_opcode(header, initial_registers, opcode, expected_registers, false);
}

#[test]
fn test_exec_advance_pc_overflow() {
let header = make_test_header(EndianSlice::new(&[], LittleEndian));
let mut registers = LineRow::new(&header);
registers.address = u64::MAX;
let opcode = LineInstruction::AdvancePc(42);

let mut initial_registers = LineRow::new(&header);
initial_registers.address.0 = u64::MAX;

let mut expected_registers = initial_registers;
expected_registers.address.0 = 41;

assert_exec_opcode(header, initial_registers, opcode, expected_registers, false);
let mut program = IncompleteLineProgram { header };
let result = registers.execute(opcode, &mut program);
assert_eq!(result, Err(Error::AddressOverflow));
}

#[test]
Expand Down Expand Up @@ -2758,11 +2779,22 @@ mod tests {
let opcode = LineInstruction::ConstAddPc;

let mut expected_registers = initial_registers;
expected_registers.address.0 += 20;
expected_registers.address += 20;

assert_exec_opcode(header, initial_registers, opcode, expected_registers, false);
}

#[test]
fn test_exec_const_add_pc_overflow() {
let header = make_test_header(EndianSlice::new(&[], LittleEndian));
let mut registers = LineRow::new(&header);
registers.address = u64::MAX;
let opcode = LineInstruction::ConstAddPc;
let mut program = IncompleteLineProgram { header };
let result = registers.execute(opcode, &mut program);
assert_eq!(result, Err(Error::AddressOverflow));
}

#[test]
fn test_exec_fixed_add_pc() {
let header = make_test_header(EndianSlice::new(&[], LittleEndian));
Expand All @@ -2773,12 +2805,24 @@ mod tests {
let opcode = LineInstruction::FixedAddPc(10);

let mut expected_registers = initial_registers;
expected_registers.address.0 += 10;
expected_registers.address += 10;
expected_registers.op_index.0 = 0;

assert_exec_opcode(header, initial_registers, opcode, expected_registers, false);
}

#[test]
fn test_exec_fixed_add_pc_overflow() {
let header = make_test_header(EndianSlice::new(&[], LittleEndian));
let mut registers = LineRow::new(&header);
registers.address = u64::MAX;
registers.op_index.0 = 1;
let opcode = LineInstruction::FixedAddPc(10);
let mut program = IncompleteLineProgram { header };
let result = registers.execute(opcode, &mut program);
assert_eq!(result, Err(Error::AddressOverflow));
}

#[test]
fn test_exec_set_prologue_end() {
let header = make_test_header(EndianSlice::new(&[], LittleEndian));
Expand Down Expand Up @@ -2855,7 +2899,7 @@ mod tests {
let opcode = LineInstruction::SetAddress(3030);

let mut expected_registers = initial_registers;
expected_registers.address.0 = 3030;
expected_registers.address = 3030;

assert_exec_opcode(header, initial_registers, opcode, expected_registers, false);
}
Expand All @@ -2868,11 +2912,22 @@ mod tests {

let mut expected_registers = initial_registers;
expected_registers.tombstone = true;
expected_registers.address.0 = !0;

assert_exec_opcode(header, initial_registers, opcode, expected_registers, false);
}

#[test]
fn test_exec_set_address_backwards() {
let header = make_test_header(EndianSlice::new(&[], LittleEndian));
let mut registers = LineRow::new(&header);
registers.address = 1;
let opcode = LineInstruction::SetAddress(0);

let mut program = IncompleteLineProgram { header };
let result = registers.execute(opcode, &mut program);
assert_eq!(result, Err(Error::InvalidAddressRange));
}

#[test]
fn test_exec_define_file() {
let mut program = make_test_program(EndianSlice::new(&[], LittleEndian));
Expand All @@ -2888,7 +2943,7 @@ mod tests {
};

let opcode = LineInstruction::DefineFile(file);
let is_new_row = row.execute(opcode, &mut program);
let is_new_row = row.execute(opcode, &mut program).unwrap();

assert!(!is_new_row);
assert_eq!(Some(&file), program.header().file_names.last());
Expand Down
5 changes: 3 additions & 2 deletions src/write/line.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1126,13 +1126,14 @@ mod convert {
Some(val) => address = Some(val),
None => return Err(ConvertError::InvalidAddress),
}
from_row.execute(read::LineInstruction::SetAddress(0), &mut from_program);
from_row
.execute(read::LineInstruction::SetAddress(0), &mut from_program)?;
}
read::LineInstruction::DefineFile(_) => {
return Err(ConvertError::UnsupportedLineInstruction);
}
_ => {
if from_row.execute(instruction, &mut from_program) {
if from_row.execute(instruction, &mut from_program)? {
if !program.in_sequence() {
program.begin_sequence(address);
address = None;
Expand Down

0 comments on commit 450cb69

Please sign in to comment.