Skip to content

Comments

bn_nist: fix strict aliasing problem (1.1.1 backport)#18948

Closed
mattcaswell wants to merge 1 commit intoopenssl:OpenSSL_1_1_1-stablefrom
mattcaswell:fix-strict-aliasing-111
Closed

bn_nist: fix strict aliasing problem (1.1.1 backport)#18948
mattcaswell wants to merge 1 commit intoopenssl:OpenSSL_1_1_1-stablefrom
mattcaswell:fix-strict-aliasing-111

Conversation

@mattcaswell
Copy link
Member

@mattcaswell mattcaswell commented Aug 3, 2022

As of clang-14 the strict aliasing is causing code to magically disappear.
By explicitly inlining the code, the aliasing problem evaporates.

Fixes #18225

Backport of #18258 to 1.1.1.

@mattcaswell mattcaswell 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) approval: otc review pending labels Aug 3, 2022
As of clang-14 the strict aliasing is causing code to magically disappear.
By explicitly inlining the code, the aliasing problem evaporates.

Fixes openssl#18225

Backport of openssl#18258 to 1.1.1.
int ii; \
const BN_ULONG *src = src_in; \
\
for (ii = 0; ii < top; ii++) \
Copy link
Member

Choose a reason for hiding this comment

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

I suppose if dst is in parens src_in, top and max should be as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm....yes. But this is a backport of code that is already merged to master/3.0. I'm not sure we should have a different version of the code in 1.1.1.

Copy link
Member

Choose a reason for hiding this comment

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

Up to you I guess. It's safe in all the places it's currently used, so...

int ii; \
const BN_ULONG *src = src_in; \
\
for (ii = 0; ii < top; ii++) \
Copy link
Member

Choose a reason for hiding this comment

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

Up to you I guess. It's safe in all the places it's currently used, so...

@hlandau hlandau removed the approval: review pending This pull request needs review by a committer label Aug 4, 2022
@paulidale paulidale added approval: done This pull request has the required number of approvals and removed approval: otc review pending labels Aug 15, 2022
@openssl-machine openssl-machine added approval: ready to merge The 24 hour grace period has passed, ready to merge and removed approval: done This pull request has the required number of approvals labels Aug 16, 2022
@openssl-machine
Copy link
Collaborator

This pull request is ready to merge

@hlandau
Copy link
Member

hlandau commented Aug 17, 2022

Merged to 1.1.1. Thank you.

@hlandau hlandau closed this Aug 17, 2022
openssl-machine pushed a commit that referenced this pull request Aug 17, 2022
As of clang-14 the strict aliasing is causing code to magically disappear.
By explicitly inlining the code, the aliasing problem evaporates.

Fixes #18225

Backport of #18258 to 1.1.1.

Reviewed-by: Hugo Landau <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from #18948)
bernd-edlinger pushed a commit to bernd-edlinger/openssl that referenced this pull request Sep 9, 2022
As of clang-14 the strict aliasing is causing code to magically disappear.
By explicitly inlining the code, the aliasing problem evaporates.

Fixes openssl#18225

Backport of openssl#18258 to 1.1.1.

Reviewed-by: Hugo Landau <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#18948)

(cherry picked from commit 6246649)
bernd-edlinger pushed a commit to bernd-edlinger/openssl that referenced this pull request Sep 9, 2022
As of clang-14 the strict aliasing is causing code to magically disappear.
By explicitly inlining the code, the aliasing problem evaporates.

Fixes openssl#18225

Backport of openssl#18258 to 1.1.1.

Reviewed-by: Hugo Landau <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#18948)

(cherry picked from commit 6246649)
jeremy added a commit to basecamp/homebrew-dev that referenced this pull request Mar 21, 2024
jeremy added a commit to basecamp/homebrew-dev that referenced this pull request Mar 21, 2024
a-kromm-rogii pushed a commit to a-kromm-rogii/openssl that referenced this pull request Mar 14, 2025
As of clang-14 the strict aliasing is causing code to magically disappear.
By explicitly inlining the code, the aliasing problem evaporates.

Fixes openssl#18225

Backport of openssl#18258 to 1.1.1.

Reviewed-by: Hugo Landau <[email protected]>
Reviewed-by: Tomas Mraz <[email protected]>
(Merged from openssl#18948)
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: 1.1.1 Applies to OpenSSL_1_1_1-stable branch (EOL)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants