Skip to content

fix: enable -Wcast-qual flag for libcrypto=awslc#4735

Merged
boquan-fang merged 4 commits intoaws:mainfrom
boquan-fang:fix-cast-qual
Nov 20, 2025
Merged

fix: enable -Wcast-qual flag for libcrypto=awslc#4735
boquan-fang merged 4 commits intoaws:mainfrom
boquan-fang:fix-cast-qual

Conversation

@boquan-fang
Copy link
Copy Markdown
Contributor

@boquan-fang boquan-fang commented Aug 26, 2024 •
edited
Loading

Resolved issues:

resolves #4710

Description of changes:

  • Remove the condition which only add -Wcast-qual back for non AWS-lc Libcrypto.

Call-outs:

Testing:

  • Local testing without specifying S2N_LIBCRYPTO=openssl.
  • CI should test it as well with AWSLC and AWSLC-FIPS.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

fix: enable -Wcast-qual flag for libcrypto=awslc
34ecf2c 
* Remove the condition which only add -Wcast-qual back for non AWS-lc
  Libcrypto.
@github-actions github-actions bot added the s2n-core team label Aug 26, 2024
goatgoose
goatgoose reviewed Aug 27, 2024
View reviewed changes
@lrstewart lrstewart requested review from lrstewart and removed request for dougch August 27, 2024 18:14
@boquan-fang boquan-fang mentioned this pull request Aug 27, 2024
Closed
@github-actions
Copy link
Copy Markdown

github-actions bot commented Oct 27, 2024

This PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

boquan-fang and others added 2 commits April 8, 2025 11:59
@boquan-fang
Merge branch 'main' into fix-cast-qual
356c721
address PR comments:
0203732 
* add -Wcast-qual to the general compilation options
@boquan-fang boquan-fang requested a review from goatgoose April 8, 2025 19:16
@boquan-fang
Copy link
Copy Markdown
Contributor Author

boquan-fang commented Apr 8, 2025

This PR should be unblocked. The original problem is caused by AWSLC having cast-qual issue, and it is fixed now.

This was the original problem with compilation:

/codebuild/output/src2687327939/src/github.com/aws/s2n-tls/crypto/s2n_kyber_evp.c: In function 's2n_kyber_evp_decapsulate':
/codebuild/output/src2687327939/src/github.com/aws/s2n-tls/crypto/s2n_kyber_evp.c:83:30: error: cast discards 'const' qualifier from pointer target type [-Werror=cast-qual]
   83 |                              (uint8_t *) ciphertext, kem->ciphertext_length),
      |                              ^
/codebuild/output/src2687327939/src/github.com/aws/s2n-tls/utils/s2n_ensure.h:35:15: note: in definition of macro '__S2N_ENSURE'
   35 |         if (!(cond)) {             \
      |               ^~~~
/codebuild/output/src2687327939/src/github.com/aws/s2n-tls/crypto/s2n_kyber_evp.c:82:5: note: in expansion of macro 'POSIX_GUARD_OSSL'
   82 |     POSIX_GUARD_OSSL(EVP_PKEY_decapsulate(kyber_pkey_ctx, shared_secret, &shared_secret_size,
      |     ^~~~~~~~~~~~~~~~

We have removed that s2n_kyber_evp.c, and compilation with AWSLC and AWSLC-FIPS are succeeding. We should generalize our check for -Wcast-qual in CMakeList.txt.

goatgoose
goatgoose reviewed Apr 8, 2025
View reviewed changes
CMakeLists.txt
-std=gnu99
-Wall
-Wcast-align
-Wcast-qual
Copy link
Copy Markdown
Contributor

@goatgoose goatgoose Apr 8, 2025

Choose a reason for hiding this comment

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

I think this change conflicts with #5198?

If we want to continue to support older versions of AWS-LC, I'm not sure we can add this flag. Unless there's a way to check the AWS-LC version first.

@boquan-fang boquan-fang mentioned this pull request Apr 8, 2025
Open
@boquan-fang boquan-fang requested review from jmayclin and jouho and removed request for lrstewart November 20, 2025 18:31
@boquan-fang
Copy link
Copy Markdown
Contributor Author

boquan-fang commented Nov 20, 2025

We decide to continue with this change. This PR should be reviewed if the CI passes.

@boquan-fang boquan-fang added this pull request to the merge queue Nov 20, 2025
Merged via the queue into aws:main with commit bcb3f28 Nov 20, 2025
68 of 70 checks passed
@boquan-fang boquan-fang deleted the fix-cast-qual branch November 20, 2025 21:33
@BrewTestBot BrewTestBot mentioned this pull request Nov 20, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jouho jouho jouho approved these changes

@kaukabrizvi kaukabrizvi kaukabrizvi approved these changes

@jmayclin jmayclin Awaiting requested review from jmayclin

+1 more reviewer

@goatgoose goatgoose goatgoose left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

s2n-core team

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

-Wcast-qual not enabled in cmake file

5 participants

@boquan-fang @goatgoose @jouho @kaukabrizvi @jmayclin