[8u] RFR: Delete incorrect NativeTrampolineCall::instruction_size
Anton Kozlov
akozlov at azul.com
Thu Dec 1 15:42:56 UTC 2016
Hi!
Could you review a patch, that deletes NativeTrampolineCall::instruction_size?
It's redundant and never used, because shared code in some sitations assumes
NativeTrampolineCall to be same size as NativeCall. However, they are different
sizes (3 and 5 words on ARMv6), so first one is padded with nops up to second.
Presence of NativeTrampolineCall::instruction_size encourage it's use and thus
introducing bugs. Example is nativeCall_before:
inline NativeCall* nativeCall_before(address return_address) {
if (NativeTrampolineCall::is_at(return_address - NativeCall::instruction_size)) {
...
}
...
}
It will be incorrect if `NativeTrampolineCall::instruction_size` used here, but
looking good.
NativeCall::instruction_size is not C++ constant and computed at VM start after
determing weither it's running on ARMv6 or ARMv7.
Making NativeTrampolineCall::instruction_size to be always equal
NativeCall::instruction_size implies making one not constant too and adding
relevant initializing code.
I suggest just to remove NativeTrampolineCall::instruction_size and use
NativeCall::instruction_size instead.
Thanks,
Anton
diff --git a/src/cpu/aarch32/vm/macroAssembler_aarch32.cpp b/src/cpu/aarch32/vm/macroAssembler_aarch32.cpp
--- a/src/cpu/aarch32/vm/macroAssembler_aarch32.cpp
+++ b/src/cpu/aarch32/vm/macroAssembler_aarch32.cpp
@@ -625,12 +625,15 @@
// Have make trampline such way: destination address should be raw 4 byte value,
// so it's patching could be done atomically.
relocate(entry.rspec());
+ address start = pc();
add(lr, r15_pc, NativeCall::instruction_size - 2 * NativeInstruction::arm_insn_sz);
ldr(r15_pc, Address(r15_pc, 4));
emit_int32((uintptr_t) entry.target());
// possibly pad the call to the NativeCall size to make patching happy
- for (int i = NativeCall::instruction_size; i > 3 * NativeInstruction::arm_insn_sz; i -= NativeInstruction::arm_insn_sz)
+ while (pc() - start < NativeCall::instruction_size) {
nop();
+ }
+ assert(pc() - start == NativeCall::instruction_size, "fix NativeTrampolineCall::instruction_size!");
} else {
bl(entry);
}
diff --git a/src/cpu/aarch32/vm/nativeInst_aarch32.cpp b/src/cpu/aarch32/vm/nativeInst_aarch32.cpp
--- a/src/cpu/aarch32/vm/nativeInst_aarch32.cpp
+++ b/src/cpu/aarch32/vm/nativeInst_aarch32.cpp
@@ -177,7 +177,7 @@
}
bool NativeTrampolineCall::is_at(address addr) {
- return (as_uint(addr ) & ~0xffu) == 0xe28fe000 // add lr, pc, #disp
+ return (as_uint(addr ) & ~0xffu) == 0xe28fe000 // add lr, pc, #disp
&& as_uint(addr + 4) == 0xe51ff004; // ldr pc, [pc, -4]
}
diff --git a/src/cpu/aarch32/vm/nativeInst_aarch32.hpp b/src/cpu/aarch32/vm/nativeInst_aarch32.hpp
--- a/src/cpu/aarch32/vm/nativeInst_aarch32.hpp
+++ b/src/cpu/aarch32/vm/nativeInst_aarch32.hpp
@@ -211,11 +211,9 @@
return NativeMovConstReg::from(address);
}
-class NativeTrampolineCall: public NativeBranchType {
+class NativeTrampolineCall: public NativeInstruction {
public:
- enum {
- instruction_size = 3 * arm_insn_sz
- };
+ // NativeTrampolineCall size is always equal to NativeCall::instruction_size
address destination() const;
void set_destination(address dest);
void set_destination_mt_safe(address dest, bool assert_lock = true);
@@ -252,7 +250,6 @@
static int instruction_size;
#ifdef ASSERT
- StaticAssert<(int) NativeTrampolineCall::instruction_size <= (int) max_instruction_size> dummy1;
StaticAssert<NativeMovConstReg::movw_movt_pair_sz
+ NativeRegCall::instruction_size <= (int) max_instruction_size> dummy2;
StaticAssert<NativeMovConstReg::mov_n_three_orr_sz
More information about the aarch32-port-dev
mailing list