From 5cb7468791489ae8517021b446b785ca1963b177 Mon Sep 17 00:00:00 2001 From: hans Date: Tue, 3 Mar 2015 19:13:30 -0800 Subject: Cherry-pick "ARM assembler: fix undefined behaviour in fits_shifter" It appears that some change to the toolchain and/or compiler flags in AOSP master has caused this UB to be a problem for webview, causing crashes in V8 on ARM (it wasn't an issue when using the L MR1 build config/toolchain). Cherrypick the trivial UB fix, which avoids the crashes. > Bit-shifts have undefined behaviour if the shift amount is greater > or equal to the width of the type. > > In this case the code would do imm32 >> 32 when rot == 0. > > A newer version of Clang unrolled the loop, optimized the first > iteration away, causing the test suite to fail with: > > # > # Fatal error in ../src/arm/assembler-arm.cc, line 1212 > # Check failed: !rn.is(ip). > # > > as well as crashing when running Chromium tests on Android (at least > we think this was the cause, see the bug). > > BUG=463436, 444089 > LOG=Y > > Review URL: https://codereview.chromium.org/979633002 > > Cr-Commit-Position: refs/heads/master@{#26974} (cherry picked from commit 721fdb56e0fc92c662c7d8b42be8a1d689c3b535) Bug: 20064008 --- src/arm/assembler-arm.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/arm/assembler-arm.cc b/src/arm/assembler-arm.cc index 17bf4f93a..364ccfc13 100644 --- a/src/arm/assembler-arm.cc +++ b/src/arm/assembler-arm.cc @@ -991,7 +991,8 @@ static bool fits_shifter(uint32_t imm32, Instr* instr) { // imm32 must be unsigned. for (int rot = 0; rot < 16; rot++) { - uint32_t imm8 = (imm32 << 2*rot) | (imm32 >> (32 - 2*rot)); + uint32_t imm8 = + rot == 0 ? imm32 : (imm32 << 2 * rot) | (imm32 >> (32 - 2 * rot)); if ((imm8 <= 0xff)) { *rotate_imm = rot; *immed_8 = imm8; -- cgit v1.2.3