Issue metadata
Sign in to add a comment
|
[S7 SM-G930F] Crash on search or on loading any URL |
||||||||||||||||||||||
Issue descriptionApplication Version (from "Chrome Settings > About Chrome"): 50.0.2661.32 (64-bit build) Android Build Number (from "Android Settings > About Phone/Tablet"): MRA58K Device: Samsung galaxy S7 (SM_G930F) Is Merge Tabs and Apps enabled: OFF Steps to reproduce: 1. Launch Chrome 2. DO some search from omnibox or navigate to some URL (Ex. cnn.com) Observed behavior: Renderer crash. Report id: https://crash.corp.google.com/browse?stbtiq=ee01dea800000000 Expected behavior: No crash Frequency: 100% Bisect range: Good build: 50.0.2638.3 Bad Build: 50.0.2639.0 Additional comments: * Doesn't crash with 32-bit apk. * Doesn't crash on S7 verizon
,
Mar 14 2016
I also see this issue while testing webview. The trigger seems to be different since browsing a webpage will work fine for a few seconds then crash will happen. Have attached the logs. I'm using webview shell browser for testing.
,
Mar 14 2016
I'm swapping this from an M50 blocker to an M51 blocker. We've already regressed the dev channel with M50, and our last planned M50 dev release is tomorrow, where we have no chance of getting a fix in time. We also don't see the build in 32-bit builds, and we don't ship 64-bit builds to beta and stable, so a fix for 51 should be fine. M51 dev is planned for Thursday, so we should ensure we have a fix landed on trunk by Wednesday.
,
Mar 15 2016
That is the same range as https://bugs.chromium.org/p/chromium/issues/detail?id=593867 and it also looks the same. Please verify that this does not work with an officially build 50.0.2661.32 (v8:5.0.71.16). If you really can reproduce it with that specific V8 version (5.0.71.16) can you please bisect the V8 range https://chromium.googlesource.com/v8/v8/+log/4.10.127..4.10.139? We don't have an S7 device so we cannot have a look ourselve. How to bisect into V8: Preparation -Checkout Chromium branch -Verify that it builds -Verify that the error shows -Modify ./DEPS and set it to the first GOOD V8 revision (4.10.127) -gclient sync -verify that ./v8/ points to 4.10.127 -Verify that the error does not show Than repeat until culprit CL found: 1.) Set V8 revision in ./DEPS to V8 revision used for bisect 2.) gclient sync 3.) Test 4.) Note result
,
Mar 15 2016
[Bulk edit] This issue is marked as a dev blocker for Android M51. We will be cutting the candidate build for our first M51 dev tomorrow evening, which means a CL to fix this issue must be landed on trunk by tomorrow @ 3-4 PM PT. Please prioritize fixing this issue ASAP. If you don't feel this issue should block the release, please remove the ReleaseBlock-Dev label. If you have other questions or concerns, please let me know by looping me in to the bug.
,
Mar 15 2016
Since this is not a regression, there's no point in letting it block releases. As #4 noted, the regression range and crash pattern look identical to the bug we just fixed. Which is weird, because over the course of finding that fix I looked at crash reports, and didn't see any coming from SM-G930F and exhibiting the same pattern (SIGBUS with pc=0x439da9017b9d). Further, I was under the impression that local testing confirmed the fix for G930V/G930P. It will be very interesting to see how 50.0.2661.32 behaves in the field when it rolls out. In the meantime, the log in #2 provided an interesting clue: The value in register x16 == ip is 0x9100439da9017b9d, which is almost the same as the crashing pc except for the leading "91", so clearly we did a jump to the value in "ip" (which is not uncommon). When interpreted as instructions, this value disassembles to: 9D 7B 01 A9 -- stp x29, x30, [x28,#16] 9D 43 00 91 -- add x29, x28, #0x10 which is generated by MacroAssembler::EmitFrameSetupForCodeAgePatching: __ stp(fp, lr, MemOperand(jssp, 2 * kXRegSize)); __ add(fp, jssp, StandardFrameConstants::kFixedFrameSizeFromFp); These two instructions are in the place where aged code puts a jump target, so this bug is quite clearly related to code aging and i-cache flushing. The fact that we have been able to fix issue 593867 indicates that V8 is generally successful at flushing the i-cache when needed. Maybe V8's i-cache flushing sequence for some reason isn't working correctly on the SM-G930F's Exynos8 CPU. Maybe that CPU reports its cache line sizes (either incorrectly or) in a format that V8 misinterprets. (This guess is derived from the similarity to issue 593867 where cache line size detection was the cause.) Neither of those two theories would explain the regression range though.
,
Mar 15 2016
The difference I noticed in the crash on 50.0.2661.26 and 50.0.2661.32 was that, on the .26 build I would see an 'Aw, snap' as soon as I try to navigate to an URL. Where as on the .32 build, the URL tries to load (atleast some content on the page loads and then I see the 'Aw, snap' after couple seconds.
,
Mar 16 2016
Re: removing RB label, I'd generally agree, but this impacts a flagship device for the most popular Android manufacturer out there - so we need to fix it ASAP. 50.0.2661.35 has been pushed to dev and contains the first fix, so please keep an eye on crash statistics today. If we see any more occurrences of this crash, please order a device (go out and buy one from a local shop preferably, otherwise order) so that we don't have to cycle debug / fix rounds on a 24h cycle.
,
Mar 16 2016
I was able to bisect. This CL causes the crash. https://chromium.googlesource.com/v8/v8/+/c3ff68b6b76ff1ffa1f6cd7c82181366020a0fe2
,
Mar 16 2016
Updates: (1) Revert is in flight: https://codereview.chromium.org/1806853002/ . Please test with that as soon as you get a chance. (2) We're expecting to get an SM-G930F in the MUC office tomorrow. (3) At first I didn't like the idea of reverting c3ff68b6b76ff1ffa1f6cd7c82181366020a0fe2 because as far as I could see that CL just moved code around, and I couldn't see a reason why that would change behavior (so I was skeptical that a revert would help). However yangguo@ provided a possible explanation: if the different cores in a big.LITTLE architecture report different cache line sizes, then that CL will indeed do the wrong thing. That makes sense, so now I'm hopeful that the revert actually will solve the problem.
,
Mar 16 2016
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/5d62db7430d48e112ddda7178419dc63adb32620 commit 5d62db7430d48e112ddda7178419dc63adb32620 Author: jkummerow <jkummerow@chromium.org> Date: Wed Mar 16 13:49:39 2016 Revert "Detect cache line size on Linux for PPC hosts." along with "[arm64] Fix i/d cache line size confusion typo" and "Fix a warning about inline asm source/destination mismatches..." which were building on it. This reverts the following commits: 8d7399f9f8688a93137ced9e084bccb8cc1b075c 474e6a3d6d3482b97c8307e45bb314f6e50bfc3c c3ff68b6b76ff1ffa1f6cd7c82181366020a0fe2 Reason for revert: We're getting a large number of crash reports from arm64 devices that are obviously related to cache flushing after code patching. Bisection results say that the problems started at revision c3ff68b. Since I can't find a bug in that CL except for the typo that I've fixed in 474e6a3 (which made some of the crashes go away but not all of them), we have no choice but to revert the changes in order to get stability under control while we investigate. BUG= chromium:594646 LOG=n Review URL: https://codereview.chromium.org/1806853002 Cr-Commit-Position: refs/heads/master@{#34816} [modify] https://crrev.com/5d62db7430d48e112ddda7178419dc63adb32620/src/arm/assembler-arm.cc [modify] https://crrev.com/5d62db7430d48e112ddda7178419dc63adb32620/src/arm/codegen-arm.cc [modify] https://crrev.com/5d62db7430d48e112ddda7178419dc63adb32620/src/arm64/assembler-arm64.cc [modify] https://crrev.com/5d62db7430d48e112ddda7178419dc63adb32620/src/arm64/cpu-arm64.cc [modify] https://crrev.com/5d62db7430d48e112ddda7178419dc63adb32620/src/assembler.cc [modify] https://crrev.com/5d62db7430d48e112ddda7178419dc63adb32620/src/assembler.h [modify] https://crrev.com/5d62db7430d48e112ddda7178419dc63adb32620/src/base/cpu.cc [modify] https://crrev.com/5d62db7430d48e112ddda7178419dc63adb32620/src/base/cpu.h [modify] https://crrev.com/5d62db7430d48e112ddda7178419dc63adb32620/src/ppc/assembler-ppc.cc [modify] https://crrev.com/5d62db7430d48e112ddda7178419dc63adb32620/src/ppc/cpu-ppc.cc [modify] https://crrev.com/5d62db7430d48e112ddda7178419dc63adb32620/src/ppc/macro-assembler-ppc.cc
,
Mar 16 2016
,
Mar 17 2016
Friendly ping, please verify that the revert fixes the problem (for now).
,
Mar 17 2016
Issue 595745 has been merged into this issue.
,
Mar 17 2016
+mlliu for manual verification
,
Mar 17 2016
Shipped M50 beta webview with this crash. Nothing private here anymore I think so -RVG
,
Mar 17 2016
Verified fix on 51.0.2681.0 . I don't see any crash on browsing
,
Mar 17 2016
Ok, let's merge to 5.0.
,
Mar 17 2016
Verified fix on 51.0.2681.0 using Galaxy S7 SM=G930FD MMB29K for WebView.
,
Mar 21 2016
Hello, I'm seeing (what looks like) same crash with 50.0.2661.35 "beta", S7 Edge: https://bugs.chromium.org/p/chromium/issues/detail?id=595745 Seeing that this was verified in 51.0.2681.0 (comments #17, #19): Will the fix also be merged into the next "beta" 50.* WebView and then the "stable" 50.* WebView?
,
Mar 21 2016
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/f396d7d65fc540daedef7d08632c0c3d7629eab2 commit f396d7d65fc540daedef7d08632c0c3d7629eab2 Author: Jakob Kummerow <jkummerow@chromium.org> Date: Mon Mar 21 16:07:00 2016 Version 5.0.71.21: Merge arm64 CacheLineSizes revert This reverts: d627337e9 - 5.0.71.16 / Fix i/d cache line size confusion typo c3ff68b6b - Detect cache line size on Linux for PPC hosts. Which is the branch's equivalent of backmerging: 5d62db743 - Revert "Detect cache line size on Linux for PPC hosts." And cherry-picks: 042f09a95 - Reland PPC portion of "Detect cache line size on Linux for PPC hosts." BUG= chromium:594646 ,chromium:593867 LOG=n R=cbruni@chromium.org Review URL: https://codereview.chromium.org/1819913002 . Cr-Commit-Position: refs/branch-heads/5.0@{#28} Cr-Branched-From: ad16e6c2cbd2c6b0f2e8ff944ac245561c682ac2-refs/heads/5.0.71@{#1} Cr-Branched-From: bd9df50d75125ee2ad37b3d92c8f50f0a8b5f030-refs/heads/master@{#34215} [modify] https://crrev.com/f396d7d65fc540daedef7d08632c0c3d7629eab2/include/v8-version.h [modify] https://crrev.com/f396d7d65fc540daedef7d08632c0c3d7629eab2/src/arm64/assembler-arm64.cc [modify] https://crrev.com/f396d7d65fc540daedef7d08632c0c3d7629eab2/src/arm64/cpu-arm64.cc [modify] https://crrev.com/f396d7d65fc540daedef7d08632c0c3d7629eab2/src/base/cpu.cc
,
Mar 21 2016
Fixed and merged, we're all done here. #20: yes, see #18/#21. The next builds that will be cut will contain the fix.
,
Mar 23 2016
Verified the issue not reproducible in the latest M 50 build. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by aska...@chromium.org
, Mar 14 2016