-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Description
Many instruction formats in the new x64 backend accept a RegMem parameter, implying that they may load from memory. This may cause a trap if the memory address is out-of-bounds and, in order to return the correct trap code, we should be registering the source location of the instruction, e.g.:
if let Some(srcloc) = *srcloc {
sink.add_trap(srcloc, TrapCode::HeapOutOfBounds);
}Currently, some formats do accept a RegMem parameter but do not correctly register the possible trap location. This passes the spec tests because these formats are not used in their RegMem::Mem variant, but for correctness and completeness we should register the trap locations any time it is possible that an instruction may load from memory.
The current convention for doing so appends a srcloc: Option<SourceLoc> field to the format and expects the programmer to correctly choose between None and Some(ctx.srcloc(insn)) when instatiating a new instruction format. I propose we change this: instead, attach the Option<SourceLoc>, or perhaps even a plain SourceLoc, to SyntheticAmode. This approach has advantages:
- any programmer attempting to do a load/store will be forced to decide how the trap will be reported (but not when only accessing registers)
- perhaps the helper functions in
lower.rscan automatically attach aSourceLocto anySyntheticAmodes created - the
Instformats no longer need an extra field filled withNonein many cases
If we agree that the above approach is a good one, this issue should be closed not only
- when that refactoring is complete but
- also when any instruction formats that can load/store properly registers traps for their
RegMem::Memvariants