diff options
| author | 2014-09-17 22:35:07 +0700 | |
|---|---|---|
| committer | 2014-09-30 07:33:16 -0700 | |
| commit | db7239ccce7748f2b494fb3b91c128b37019a093 (patch) | |
| tree | e4237afbb576588ee6be0003de2ea6f10cd469b5 /compiler | |
| parent | 6d031046eeb5e9f62b657df0695b752e1d0aa583 (diff) | |
ART: Overflow of bound check in ArrayCopy intrinsic
System.arraycopy method is implemented as intrinsic on x86. It has
bound check which has a bug to overflow in certain conditions
when summ of array offset and number elements to be copied are
more than MAX_INT. For the methods like CarArrayBuffer.get it
means no OutOfBound exception to be thrown. The proposed
solution fixed that.
b/17711775
Signed-off-by: avignate <aleksey.v.ignatenko@intel.com>
(cherry picked from commit f9f0ed401f7fe4138a71b36719423b908a3b7bfb)
Change-Id: I1d4ca900df262d483a94ebea8fa686ea361772c8
Diffstat (limited to 'compiler')
| -rwxr-xr-x | compiler/dex/quick/x86/target_x86.cc | 34 |
1 files changed, 26 insertions, 8 deletions
diff --git a/compiler/dex/quick/x86/target_x86.cc b/compiler/dex/quick/x86/target_x86.cc index 75279d7130..0c11bac07a 100755 --- a/compiler/dex/quick/x86/target_x86.cc +++ b/compiler/dex/quick/x86/target_x86.cc @@ -1127,37 +1127,51 @@ bool X86Mir2Lir::GenInlinedArrayCopyCharArray(CallInfo* info) { LoadValueDirectFixed(rl_src, rs_rAX); LoadWordDisp(rs_rAX, mirror::Array::LengthOffset().Int32Value(), rs_rAX); LIR* src_bad_len = nullptr; + LIR* src_bad_off = nullptr; LIR* srcPos_negative = nullptr; if (!rl_srcPos.is_const) { LoadValueDirectFixed(rl_srcPos, tmp_reg); srcPos_negative = OpCmpImmBranch(kCondLt, tmp_reg, 0, nullptr); - OpRegReg(kOpAdd, tmp_reg, rs_rDX); - src_bad_len = OpCmpBranch(kCondLt, rs_rAX, tmp_reg, nullptr); + // src_pos < src_len + src_bad_off = OpCmpBranch(kCondLt, rs_rAX, tmp_reg, nullptr); + // src_len - src_pos < copy_len + OpRegRegReg(kOpSub, tmp_reg, rs_rAX, tmp_reg); + src_bad_len = OpCmpBranch(kCondLt, tmp_reg, rs_rDX, nullptr); } else { int32_t pos_val = mir_graph_->ConstantValue(rl_srcPos.orig_sreg); if (pos_val == 0) { src_bad_len = OpCmpBranch(kCondLt, rs_rAX, rs_rDX, nullptr); } else { - OpRegRegImm(kOpAdd, tmp_reg, rs_rDX, pos_val); - src_bad_len = OpCmpBranch(kCondLt, rs_rAX, tmp_reg, nullptr); + // src_pos < src_len + src_bad_off = OpCmpImmBranch(kCondLt, rs_rAX, pos_val, nullptr); + // src_len - src_pos < copy_len + OpRegRegImm(kOpSub, tmp_reg, rs_rAX, pos_val); + src_bad_len = OpCmpBranch(kCondLt, tmp_reg, rs_rDX, nullptr); } } LIR* dstPos_negative = nullptr; LIR* dst_bad_len = nullptr; + LIR* dst_bad_off = nullptr; LoadValueDirectFixed(rl_dst, rs_rAX); LoadWordDisp(rs_rAX, mirror::Array::LengthOffset().Int32Value(), rs_rAX); if (!rl_dstPos.is_const) { LoadValueDirectFixed(rl_dstPos, tmp_reg); dstPos_negative = OpCmpImmBranch(kCondLt, tmp_reg, 0, nullptr); - OpRegRegReg(kOpAdd, tmp_reg, tmp_reg, rs_rDX); - dst_bad_len = OpCmpBranch(kCondLt, rs_rAX, tmp_reg, nullptr); + // dst_pos < dst_len + dst_bad_off = OpCmpBranch(kCondLt, rs_rAX, tmp_reg, nullptr); + // dst_len - dst_pos < copy_len + OpRegRegReg(kOpSub, tmp_reg, rs_rAX, tmp_reg); + dst_bad_len = OpCmpBranch(kCondLt, tmp_reg, rs_rDX, nullptr); } else { int32_t pos_val = mir_graph_->ConstantValue(rl_dstPos.orig_sreg); if (pos_val == 0) { dst_bad_len = OpCmpBranch(kCondLt, rs_rAX, rs_rDX, nullptr); } else { - OpRegRegImm(kOpAdd, tmp_reg, rs_rDX, pos_val); - dst_bad_len = OpCmpBranch(kCondLt, rs_rAX, tmp_reg, nullptr); + // dst_pos < dst_len + dst_bad_off = OpCmpImmBranch(kCondLt, rs_rAX, pos_val, nullptr); + // dst_len - dst_pos < copy_len + OpRegRegImm(kOpSub, tmp_reg, rs_rAX, pos_val); + dst_bad_len = OpCmpBranch(kCondLt, tmp_reg, rs_rDX, nullptr); } } // Everything is checked now. @@ -1201,11 +1215,15 @@ bool X86Mir2Lir::GenInlinedArrayCopyCharArray(CallInfo* info) { src_null_branch->target = check_failed; if (srcPos_negative != nullptr) srcPos_negative ->target = check_failed; + if (src_bad_off != nullptr) + src_bad_off->target = check_failed; if (src_bad_len != nullptr) src_bad_len->target = check_failed; dst_null_branch->target = check_failed; if (dstPos_negative != nullptr) dstPos_negative->target = check_failed; + if (dst_bad_off != nullptr) + dst_bad_off->target = check_failed; if (dst_bad_len != nullptr) dst_bad_len->target = check_failed; AddIntrinsicSlowPath(info, launchpad_branch, return_point); |