Fix mis-aligned access issues with s390x (#4702)

This fixes two problems: minimum symbol alignment for the LARL
instruction, and alignment requirements for LRL/LGRL etc.

The first problem is that the LARL instruction used to load a
symbol address (PC relative) requires that the target symbol
is at least 2-byte aligned.  This is always guaranteed for code
symbols (all instructions must be 2-aligned anyway), but not
necessarily for data symbols.

Other s390x compilers fix this problem by ensuring that all
global symbols are always emitted with a minimum 2-byte
alignment.  This patch introduces an equivalent mechanism
for cranelift:
- Add a symbol_alignment routine to TargetIsa, similar to the
  existing code_section_alignment routine.
- Respect symbol_alignment as minimum alignment for all symbols
  emitted in the object backend (code and data).

The second problem is that PC-relative instructions that
directly *access* data (like LRL/LGRL, STRL/STGRL etc.)
not only have the 2-byte requirement like LARL, but actually
require that their memory operand is *naturally* aligned
(i.e. alignment is at least the size of the access).

This property (natural alignment for memory accesses) is
supposed to be provided by the "aligned" flag in MemFlags;
however, this is not implemented correctly at the moment.

To fix this, this patch:
- Only emits PC-relative memory access instructions if the
  "aligned" flag is set in the associated MemFlags.
- Fixes a bug in emit_small_memory_copy and emit_small_memset
  which currently set the aligned flag unconditionally, ignoring
  the actual alignment info passed by their caller.

Tested with wasmtime and cg_clif.
This commit is contained in:
Ulrich Weigand
2022-08-16 21:39:42 +02:00
committed by GitHub
parent fbfceaec98
commit a916788ab4
14 changed files with 323 additions and 132 deletions

View File

@@ -1170,15 +1170,8 @@ impl MachInst for Inst {
//=============================================================================
// Pretty-printing of instructions.
fn mem_finalize_for_show(
mem: &MemArg,
state: &EmitState,
have_d12: bool,
have_d20: bool,
have_pcrel: bool,
have_index: bool,
) -> (String, MemArg) {
let (mem_insts, mem) = mem_finalize(mem, state, have_d12, have_d20, have_pcrel, have_index);
fn mem_finalize_for_show(mem: &MemArg, state: &EmitState, mi: MemInstType) -> (String, MemArg) {
let (mem_insts, mem) = mem_finalize(mem, state, mi);
let mut mem_str = mem_insts
.into_iter()
.map(|inst| {
@@ -1342,10 +1335,13 @@ impl Inst {
let (mem_str, mem) = mem_finalize_for_show(
&mem,
state,
opcode_rx.is_some(),
opcode_rxy.is_some(),
false,
true,
MemInstType {
have_d12: opcode_rx.is_some(),
have_d20: opcode_rxy.is_some(),
have_pcrel: false,
have_unaligned_pcrel: false,
have_index: true,
},
);
let op = match &mem {
&MemArg::BXD12 { .. } => opcode_rx,
@@ -1602,10 +1598,13 @@ impl Inst {
let (mem_str, mem) = mem_finalize_for_show(
&mem,
state,
opcode_rx.is_some(),
opcode_rxy.is_some(),
opcode_ril.is_some(),
true,
MemInstType {
have_d12: opcode_rx.is_some(),
have_d20: opcode_rxy.is_some(),
have_pcrel: opcode_ril.is_some(),
have_unaligned_pcrel: false,
have_index: true,
},
);
let op = match &mem {
&MemArg::BXD12 { .. } => opcode_rx,
@@ -1706,7 +1705,17 @@ impl Inst {
let rd = pretty_print_reg(rd.to_reg(), allocs);
let rn = pretty_print_reg(rn, allocs);
let mem = mem.with_allocs(allocs);
let (mem_str, mem) = mem_finalize_for_show(&mem, state, false, true, false, false);
let (mem_str, mem) = mem_finalize_for_show(
&mem,
state,
MemInstType {
have_d12: false,
have_d20: true,
have_pcrel: false,
have_unaligned_pcrel: false,
have_index: false,
},
);
let mem = mem.pretty_print_default();
format!("{}{} {}, {}, {}", mem_str, op, rd, rn, mem)
}
@@ -1723,10 +1732,13 @@ impl Inst {
let (mem_str, mem) = mem_finalize_for_show(
&mem,
state,
opcode_rs.is_some(),
opcode_rsy.is_some(),
false,
false,
MemInstType {
have_d12: opcode_rs.is_some(),
have_d20: opcode_rsy.is_some(),
have_pcrel: false,
have_unaligned_pcrel: false,
have_index: false,
},
);
let op = match &mem {
&MemArg::BXD12 { .. } => opcode_rs,
@@ -1777,10 +1789,13 @@ impl Inst {
let (mem_str, mem) = mem_finalize_for_show(
&mem,
state,
opcode_rx.is_some(),
opcode_rxy.is_some(),
opcode_ril.is_some(),
true,
MemInstType {
have_d12: opcode_rx.is_some(),
have_d20: opcode_rxy.is_some(),
have_pcrel: opcode_ril.is_some(),
have_unaligned_pcrel: false,
have_index: true,
},
);
let op = match &mem {
&MemArg::BXD12 { .. } => opcode_rx,
@@ -1814,10 +1829,13 @@ impl Inst {
let (mem_str, mem) = mem_finalize_for_show(
&mem,
state,
opcode_rx.is_some(),
opcode_rxy.is_some(),
opcode_ril.is_some(),
true,
MemInstType {
have_d12: opcode_rx.is_some(),
have_d20: opcode_rxy.is_some(),
have_pcrel: opcode_ril.is_some(),
have_unaligned_pcrel: false,
have_index: true,
},
);
let op = match &mem {
&MemArg::BXD12 { .. } => opcode_rx,
@@ -1831,7 +1849,17 @@ impl Inst {
}
&Inst::StoreImm8 { imm, ref mem } => {
let mem = mem.with_allocs(allocs);
let (mem_str, mem) = mem_finalize_for_show(&mem, state, true, true, false, false);
let (mem_str, mem) = mem_finalize_for_show(
&mem,
state,
MemInstType {
have_d12: true,
have_d20: true,
have_pcrel: false,
have_unaligned_pcrel: false,
have_index: false,
},
);
let op = match &mem {
&MemArg::BXD12 { .. } => "mvi",
&MemArg::BXD20 { .. } => "mviy",
@@ -1845,7 +1873,17 @@ impl Inst {
| &Inst::StoreImm32SExt16 { imm, ref mem }
| &Inst::StoreImm64SExt16 { imm, ref mem } => {
let mem = mem.with_allocs(allocs);
let (mem_str, mem) = mem_finalize_for_show(&mem, state, false, true, false, false);
let (mem_str, mem) = mem_finalize_for_show(
&mem,
state,
MemInstType {
have_d12: false,
have_d20: true,
have_pcrel: false,
have_unaligned_pcrel: false,
have_index: false,
},
);
let op = match self {
&Inst::StoreImm16 { .. } => "mvhhi",
&Inst::StoreImm32SExt16 { .. } => "mvhi",
@@ -1874,7 +1912,17 @@ impl Inst {
}
&Inst::LoadMultiple64 { rt, rt2, ref mem } => {
let mem = mem.with_allocs(allocs);
let (mem_str, mem) = mem_finalize_for_show(&mem, state, false, true, false, false);
let (mem_str, mem) = mem_finalize_for_show(
&mem,
state,
MemInstType {
have_d12: false,
have_d20: true,
have_pcrel: false,
have_unaligned_pcrel: false,
have_index: false,
},
);
let rt = pretty_print_reg(rt.to_reg(), &mut empty_allocs);
let rt2 = pretty_print_reg(rt2.to_reg(), &mut empty_allocs);
let mem = mem.pretty_print_default();
@@ -1882,7 +1930,17 @@ impl Inst {
}
&Inst::StoreMultiple64 { rt, rt2, ref mem } => {
let mem = mem.with_allocs(allocs);
let (mem_str, mem) = mem_finalize_for_show(&mem, state, false, true, false, false);
let (mem_str, mem) = mem_finalize_for_show(
&mem,
state,
MemInstType {
have_d12: false,
have_d20: true,
have_pcrel: false,
have_unaligned_pcrel: false,
have_index: false,
},
);
let rt = pretty_print_reg(rt, &mut empty_allocs);
let rt2 = pretty_print_reg(rt2, &mut empty_allocs);
let mem = mem.pretty_print_default();
@@ -2561,7 +2619,17 @@ impl Inst {
let rd = pretty_print_reg(rd.to_reg(), allocs);
let mem = mem.with_allocs(allocs);
let (mem_str, mem) = mem_finalize_for_show(&mem, state, true, false, false, true);
let (mem_str, mem) = mem_finalize_for_show(
&mem,
state,
MemInstType {
have_d12: true,
have_d20: false,
have_pcrel: false,
have_unaligned_pcrel: false,
have_index: true,
},
);
let mem = mem.pretty_print_default();
format!("{}{} {}, {}", mem_str, opcode, rd, mem)
}
@@ -2587,7 +2655,17 @@ impl Inst {
let rd = pretty_print_reg(rd, allocs);
let mem = mem.with_allocs(allocs);
let (mem_str, mem) = mem_finalize_for_show(&mem, state, true, false, false, true);
let (mem_str, mem) = mem_finalize_for_show(
&mem,
state,
MemInstType {
have_d12: true,
have_d20: false,
have_pcrel: false,
have_unaligned_pcrel: false,
have_index: true,
},
);
let mem = mem.pretty_print_default();
format!("{}{} {}, {}", mem_str, opcode, rd, mem)
}
@@ -2606,7 +2684,17 @@ impl Inst {
let rd = pretty_print_reg(rd.to_reg(), allocs);
let mem = mem.with_allocs(allocs);
let (mem_str, mem) = mem_finalize_for_show(&mem, state, true, false, false, true);
let (mem_str, mem) = mem_finalize_for_show(
&mem,
state,
MemInstType {
have_d12: true,
have_d20: false,
have_pcrel: false,
have_unaligned_pcrel: false,
have_index: true,
},
);
let mem = mem.pretty_print_default();
format!("{}{} {}, {}", mem_str, opcode, rd, mem)
}
@@ -2734,8 +2822,17 @@ impl Inst {
let (rd, rd_fpr) = pretty_print_fpr(rd.to_reg(), allocs);
let mem = mem.with_allocs(allocs);
if lane_imm == 0 && rd_fpr.is_some() && opcode_rx.is_some() {
let (mem_str, mem) =
mem_finalize_for_show(&mem, state, true, true, false, true);
let (mem_str, mem) = mem_finalize_for_show(
&mem,
state,
MemInstType {
have_d12: true,
have_d20: true,
have_pcrel: false,
have_unaligned_pcrel: false,
have_index: true,
},
);
let op = match &mem {
&MemArg::BXD12 { .. } => opcode_rx,
&MemArg::BXD20 { .. } => opcode_rxy,
@@ -2744,8 +2841,17 @@ impl Inst {
let mem = mem.pretty_print_default();
format!("{}{} {}, {}", mem_str, op.unwrap(), rd_fpr.unwrap(), mem)
} else {
let (mem_str, mem) =
mem_finalize_for_show(&mem, state, true, false, false, true);
let (mem_str, mem) = mem_finalize_for_show(
&mem,
state,
MemInstType {
have_d12: true,
have_d20: false,
have_pcrel: false,
have_unaligned_pcrel: false,
have_index: true,
},
);
let mem = mem.pretty_print_default();
format!("{}{} {}, {}, {}", mem_str, opcode_vrx, rd, mem, lane_imm)
}
@@ -2776,8 +2882,17 @@ impl Inst {
let (rd, rd_fpr) = pretty_print_fpr(rd, allocs);
let mem = mem.with_allocs(allocs);
if lane_imm == 0 && rd_fpr.is_some() && opcode_rx.is_some() {
let (mem_str, mem) =
mem_finalize_for_show(&mem, state, true, true, false, true);
let (mem_str, mem) = mem_finalize_for_show(
&mem,
state,
MemInstType {
have_d12: true,
have_d20: true,
have_pcrel: false,
have_unaligned_pcrel: false,
have_index: true,
},
);
let op = match &mem {
&MemArg::BXD12 { .. } => opcode_rx,
&MemArg::BXD20 { .. } => opcode_rxy,
@@ -2786,8 +2901,17 @@ impl Inst {
let mem = mem.pretty_print_default();
format!("{}{} {}, {}", mem_str, op.unwrap(), rd_fpr.unwrap(), mem)
} else {
let (mem_str, mem) =
mem_finalize_for_show(&mem, state, true, false, false, true);
let (mem_str, mem) = mem_finalize_for_show(
&mem,
state,
MemInstType {
have_d12: true,
have_d20: false,
have_pcrel: false,
have_unaligned_pcrel: false,
have_index: true,
},
);
let mem = mem.pretty_print_default();
format!("{}{} {}, {}, {}", mem_str, opcode_vrx, rd, mem, lane_imm,)
}
@@ -3017,7 +3141,17 @@ impl Inst {
&Inst::LoadAddr { rd, ref mem } => {
let rd = pretty_print_reg(rd.to_reg(), allocs);
let mem = mem.with_allocs(allocs);
let (mem_str, mem) = mem_finalize_for_show(&mem, state, true, true, true, true);
let (mem_str, mem) = mem_finalize_for_show(
&mem,
state,
MemInstType {
have_d12: true,
have_d20: true,
have_pcrel: true,
have_unaligned_pcrel: true,
have_index: true,
},
);
let op = match &mem {
&MemArg::BXD12 { .. } => "la",
&MemArg::BXD20 { .. } => "lay",