New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 598085 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug

Blocked on:
issue 600467



Sign in to add a comment

Videostream app broken by NaCl x86 validator change for rewriting non-temporals

Project Member Reported by mseaborn@chromium.org, Mar 25 2016

Issue description

Bug reported here:
https://groups.google.com/forum/#!topic/native-client-discuss/L2w3YVbxrpw
"What changed for Native Client between Chrome 49 and Chrome 50 Beta?"

This app, Videostream, was broken:
https://chrome.google.com/webstore/detail/videostream-for-google-ch/cnciopoikihiagdjbjpnocolokfelagl

Caused by this change:
https://codereview.chromium.org/1309953002
(for  issue 500026 )

I verified this by installing the app and checking its binaries:

$ objdump -dr ~/.config/google-chrome/Default/Extensions/cnciopoikihiagdjbjpnocolokfelagl/2.16.304.0_0/_platform_specific/x86-64/lib64/libx264.so.142 | grep movnt
   f766b:       43 0f e7 0c 1f          movntq %mm1,(%r15,%r11,1)
   f778b:       43 0f e7 0c 1f          movntq %mm1,(%r15,%r11,1)
...

We're currently not handling movntq on x86-64.  I will change the validator to handle this case.

We were supposed to have checked the Web Store to find all the instructions that we need to handle and rewrite, but it looks like we missed this one.  Maybe it appeared after we did the check.

 
Labels: Arch-x86
There's another case that needs fixing, for x86-32:

$ objdump -dr .../2.16.304.0_0/_platform_specific/x86-32/lib32/libx264.so.142 | grep movntps
   e6be0:	0f 2b 0c 10          	movntps %xmm1,(%eax,%edx,1)
   e7474:	0f 2b 1c 10          	movntps %xmm3,(%eax,%edx,1)

Labels: Merge-Request-50
Merge requested: I'd like to merge these two NaCl changes to Chrome 50 (Beta):

https://chromium.googlesource.com/native_client/src/native_client/+/fb2c685d12b5a13cdb5b284b0ee1ee3c2080a769 - "x86 validator: Implement rewriting "movntq" to "movq" on x86-64"
https://chromium.googlesource.com/native_client/src/native_client/+/ca352b231f2f448e90549dd5cd09901f49f97cbd - "x86 validator: Implement rewriting "movntps" to "movaps" on x86-32"

Comment 3 by tin...@google.com, Mar 30 2016

Labels: -Merge-Request-50 Merge-Approved-50 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M50 (branch: 2661)

Comment 4 by gov...@chromium.org, Mar 31 2016

Please merge your change to M50 branch 2661 by Friday 5:00 PM PST so we can take it for next week beta.
Cc: tinazh@chromium.org
Are there any instructions for how to cherry-pick a change to a Git repo that is DEPS'd into Chromium?

git-drover does not work in the native_client repo because no 2661 branch exists there yet:

$ git drover --branch 2661 --cherry-pick fb2c685d12b5a13cdb5b284b0ee1ee3c2080a769
Error: Branch refs/remotes/branch-heads/2661 not found

But I also don't have permission to create that branch:

$ git push origin d182b0a028d44ba9767ae25b30cf3986a7e72e99:refs/branch-heads/2661
Total 0 (delta 0), reused 0 (delta 0)
remote: Processing changes: refs: 1, done    
To https://chromium.googlesource.com/native_client/src/native_client.git
 ! [remote rejected] d182b0a028d44ba9767ae25b30cf3986a7e72e99 -> refs/branch-heads/2661 (prohibited by Gerrit)
error: failed to push some refs to 'https://chromium.googlesource.com/native_client/src/native_client.git'

Note that the above commit ID is the native_client rev that's currently DEPS'd into Chromium's branch 2661:

$ git cat-file blob refs/remotes/origin/branch-heads/2661:DEPS | grep "'nacl_revision':"
  'nacl_revision': 'd182b0a028d44ba9767ae25b30cf3986a7e72e99',

Comment 6 by tin...@google.com, Apr 4 2016

Hey, you can check https://sites.google.com/a/google.com/chrome-pmo/Home/how-to "How-to update DEPS file for a release branch?".
Blockedon: 600467
I've filed issue 600467 for getting this branch created.
Project Member

Comment 8 by bugdroid1@chromium.org, Apr 6 2016

Labels: -merge-approved-50 merge-merged-2661
The following revision refers to this bug:
  http://goto.ext.google.com/viewvc/chrome-internal?view=rev&revision=86213

------------------------------------------------------------------
r86213 | mseaborn@google.com | 2016-04-06T17:13:24.401381Z

-----------------------------------------------------------------
Status: Fixed (was: Started)
Fixed and merged to branch.

Sign in to add a comment