Skip to content

Comments

Fix: ecp_nistz256-armv4.S bad arguments#12854

Closed
HenryNe wants to merge 1 commit intoopenssl:masterfrom
HenryNe:ecp_nistz256-armv4.S-bad-arguments
Closed

Fix: ecp_nistz256-armv4.S bad arguments#12854
HenryNe wants to merge 1 commit intoopenssl:masterfrom
HenryNe:ecp_nistz256-armv4.S-bad-arguments

Conversation

@HenryNe
Copy link
Contributor

@HenryNe HenryNe commented Sep 10, 2020

Fix this error:

crypto/ec/ecp_nistz256-armv4.S:3853: Error: bad arguments to instruction -- orr r11,r10
crypto/ec/ecp_nistz256-armv4.S:3854: Error: bad arguments to instruction -- orr r11,r12
crypto/ec/ecp_nistz256-armv4.S:3855: Error: bad arguments to instruction -- orrs r11,r14

Always use 3-operand form instructions

CLA: trivial

Fixes #12848

Fix this error:

crypto/ec/ecp_nistz256-armv4.S:3853: Error: bad arguments to instruction -- `orr r11,r10'
crypto/ec/ecp_nistz256-armv4.S:3854: Error: bad arguments to instruction -- `orr r11,r12'
crypto/ec/ecp_nistz256-armv4.S:3855: Error: bad arguments to instruction -- `orrs r11,r14'

CLA: trivial

Fixes #12848
Copy link
Member

@t8m t8m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with CLA: trivial.

@t8m t8m added approval: review pending This pull request needs review by a committer branch: 1.1.1 Applies to OpenSSL_1_1_1-stable branch (EOL) branch: master Applies to master branch labels Sep 11, 2020
@romen
Copy link
Member

romen commented Sep 11, 2020

The only thing preventing me from approving is that I have no access to an armv4 platform to test this on, and this is going to 1.1.1 as well.

I'd really like if @bernd-edlinger that wrote and tested the new code originally could chime in.

That aside, I agree this would fit the "CLA: trivial" requirements.

@t8m
Copy link
Member

t8m commented Sep 11, 2020

I am fairly sure this is OK, although I did not test it either.

@kroeckx kroeckx added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Sep 13, 2020
@kroeckx
Copy link
Member

kroeckx commented Sep 13, 2020

I agree this is trivial

@openssl-machine
Copy link
Collaborator

24 hours has passed since 'approval: done' was set, but as this PR has been updated in that time the label 'approval: ready to merge' is not being automatically set. Please review the updates and set the label manually.

@kroeckx kroeckx added approval: ready to merge The 24 hour grace period has passed, ready to merge cla: trivial One of the commits is marked as 'CLA: trivial' and removed approval: done This pull request has the required number of approvals labels Sep 14, 2020
openssl-machine pushed a commit that referenced this pull request Sep 20, 2020
Fix this error:

crypto/ec/ecp_nistz256-armv4.S:3853: Error: bad arguments to instruction -- `orr r11,r10'
crypto/ec/ecp_nistz256-armv4.S:3854: Error: bad arguments to instruction -- `orr r11,r12'
crypto/ec/ecp_nistz256-armv4.S:3855: Error: bad arguments to instruction -- `orrs r11,r14'

CLA: trivial

Fixes #12848

Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Kurt Roeckx <[email protected]>
GH: #12854
openssl-machine pushed a commit that referenced this pull request Sep 20, 2020
Fix this error:

crypto/ec/ecp_nistz256-armv4.S:3853: Error: bad arguments to instruction -- `orr r11,r10'
crypto/ec/ecp_nistz256-armv4.S:3854: Error: bad arguments to instruction -- `orr r11,r12'
crypto/ec/ecp_nistz256-armv4.S:3855: Error: bad arguments to instruction -- `orrs r11,r14'

CLA: trivial

Fixes #12848

Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Kurt Roeckx <[email protected]>
GH: #12854
(cherry picked from commit b5f8256)
@kroeckx kroeckx closed this Sep 20, 2020
@HenryNe HenryNe deleted the ecp_nistz256-armv4.S-bad-arguments branch September 21, 2020 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Applies to master branch branch: 1.1.1 Applies to OpenSSL_1_1_1-stable branch (EOL) cla: trivial One of the commits is marked as 'CLA: trivial'

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ecp_nistz256-armv4.S bad arguments to instruction

5 participants