Skip to content

Remove crypto/aes/aes_x86core.c because it's dead code - #31424

Closed
andrewkdinh wants to merge 1 commit into
openssl:masterfrom
andrewkdinh:rm-aes_x86core
Closed

Remove crypto/aes/aes_x86core.c because it's dead code#31424
andrewkdinh wants to merge 1 commit into
openssl:masterfrom
andrewkdinh:rm-aes_x86core

Conversation

@andrewkdinh

@andrewkdinh andrewkdinh commented Jun 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor 
$ grep --recursive --exclude-dir=.git 'aes_x86core.c' .
$

Fixes #31117 (no issues with the code if there's no code at all 😅)

@openssl-machine openssl-machine added the approval: review pending This pull request needs review by a committer label Jun 9, 2026
@openssl-machine openssl-machine 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 Jun 9, 2026
@jogme jogme added branch: master Applies to master branch branch: 3.0 Applies to openssl-3.0 branch (EOL) branch: 3.4 Applies to openssl-3.4 branch: 3.5 Applies to openssl-3.5 branch: 3.6 Applies to openssl-3.6 branch: 4.0 Applies to openssl-4.0 triaged: cleanup The issue/pr deals with cleanup of comments/docs not altering code significantly labels Jun 9, 2026
bob-beck
bob-beck approved these changes Jun 9, 2026
View reviewed changes

@bob-beck bob-beck left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Image

Approved with extreme prejudice.

esyr
esyr approved these changes Jun 10, 2026
View reviewed changes

@esyr esyr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So, it was introduced in commit 9c62bca "Prepare playground for AES experimental code." exclusively in debug-linux-ia32-aes config (which hasn't received much love since, especially after 2010, and eventually got removed in commit 6e1ba47 "Move cpuid_asm_src file information to build.info files"). I guess the file can be safely removed.

As a side note, clang-format made aes_core.c significantly more ugly, all hail terrible useless tooling.

@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 Jun 10, 2026
@openssl-machine

openssl-machine commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

This pull request is ready to merge

@jogme

jogme commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

So, it was introduced in commit 9c62bca "Prepare playground for AES experimental code." exclusively in debug-linux-ia32-aes config (which hasn't received much love since, especially after 2010, and eventually got removed in commit 6e1ba47 "Move cpuid_asm_src file information to build.info files"). I guess the file can be safely removed.

As a side note, clang-format made aes_core.c significantly more ugly, all hail terrible useless tooling.

Opened #31446 to deal with the staircase issue

openssl-machine pushed a commit that referenced this pull request Jun 10, 2026
@andrewkdinh @jogme
Remove crypto/aes/aes_x86core.c because it's dead code
d1c4d14 
Reviewed-by: Milan Broz <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
Reviewed-by: Eugene Syromiatnikov <[email protected]>
Reviewed-by: Nikola Pajkovsky <[email protected]>
Reviewed-by: Norbert Pocs <[email protected]>
MergeDate: Wed Jun 10 12:43:39 2026
(Merged from #31424)
openssl-machine pushed a commit that referenced this pull request Jun 10, 2026
@andrewkdinh @jogme
Remove crypto/aes/aes_x86core.c because it's dead code
5bc1535 
Reviewed-by: Milan Broz <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
Reviewed-by: Eugene Syromiatnikov <[email protected]>
Reviewed-by: Nikola Pajkovsky <[email protected]>
Reviewed-by: Norbert Pocs <[email protected]>
MergeDate: Wed Jun 10 12:45:39 2026
(Merged from #31424)
openssl-machine pushed a commit that referenced this pull request Jun 10, 2026
@andrewkdinh @jogme
Remove crypto/aes/aes_x86core.c because it's dead code
e2b5cd3 
Reviewed-by: Milan Broz <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
Reviewed-by: Eugene Syromiatnikov <[email protected]>
Reviewed-by: Nikola Pajkovsky <[email protected]>
Reviewed-by: Norbert Pocs <[email protected]>
MergeDate: Wed Jun 10 12:46:55 2026
(Merged from #31424)
openssl-machine pushed a commit that referenced this pull request Jun 10, 2026
@andrewkdinh @jogme
Remove crypto/aes/aes_x86core.c because it's dead code
1cd8685 
Reviewed-by: Milan Broz <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
Reviewed-by: Eugene Syromiatnikov <[email protected]>
Reviewed-by: Nikola Pajkovsky <[email protected]>
Reviewed-by: Norbert Pocs <[email protected]>
MergeDate: Wed Jun 10 12:46:16 2026
(Merged from #31424)
@jogme

jogme commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Merged to all active branches. Thank you!

@jogme jogme closed this Jun 10, 2026
openssl-machine pushed a commit that referenced this pull request Jun 10, 2026
@andrewkdinh @jogme
Remove crypto/aes/aes_x86core.c because it's dead code
726619d 
Reviewed-by: Milan Broz <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
Reviewed-by: Eugene Syromiatnikov <[email protected]>
Reviewed-by: Nikola Pajkovsky <[email protected]>
Reviewed-by: Norbert Pocs <[email protected]>
MergeDate: Wed Jun 10 12:47:22 2026
(Merged from #31424)
openssl-machine pushed a commit that referenced this pull request Jun 10, 2026
@andrewkdinh @jogme
Remove crypto/aes/aes_x86core.c because it's dead code
0d35bef 
Reviewed-by: Milan Broz <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
Reviewed-by: Eugene Syromiatnikov <[email protected]>
Reviewed-by: Nikola Pajkovsky <[email protected]>
Reviewed-by: Norbert Pocs <[email protected]>
MergeDate: Wed Jun 10 12:47:50 2026
(Merged from #31424)
n13l pushed a commit to n13l/openssl that referenced this pull request Jul 25, 2026
@andrewkdinh @n13l
Remove crypto/aes/aes_x86core.c because it's dead code
e083104 
Reviewed-by: Milan Broz <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
Reviewed-by: Eugene Syromiatnikov <[email protected]>
Reviewed-by: Nikola Pajkovsky <[email protected]>
Reviewed-by: Norbert Pocs <[email protected]>
MergeDate: Wed Jun 10 12:43:39 2026
(Merged from openssl#31424)
rickyringler pushed a commit to rickyringler/openssl that referenced this pull request Aug 21, 2026
@andrewkdinh Ricky Ringler
Remove crypto/aes/aes_x86core.c because it's dead code
b6273cc 
Reviewed-by: Milan Broz <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
Reviewed-by: Eugene Syromiatnikov <[email protected]>
Reviewed-by: Nikola Pajkovsky <[email protected]>
Reviewed-by: Norbert Pocs <[email protected]>
MergeDate: Wed Jun 10 12:43:39 2026
(Merged from openssl#31424)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@esyr esyr esyr approved these changes
@mbroz mbroz mbroz approved these changes
@bob-beck bob-beck bob-beck approved these changes
@jogme jogme jogme approved these changes
@npajkovsky npajkovsky npajkovsky approved these changes
@Sashan Sashan Awaiting requested review from Sashan
@nhorman nhorman Awaiting requested review from nhorman

Assignees

No one assigned

Labels

approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Applies to master branch branch: 3.0 Applies to openssl-3.0 branch (EOL) branch: 3.4 Applies to openssl-3.4 branch: 3.5 Applies to openssl-3.5 branch: 3.6 Applies to openssl-3.6 branch: 4.0 Applies to openssl-4.0 triaged: cleanup The issue/pr deals with cleanup of comments/docs not altering code significantly

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Probably harmless UB in aes_x86core.c

7 participants

@andrewkdinh @openssl-machine @jogme @esyr @mbroz @bob-beck @npajkovsky