Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release build for MinGW CI; Fix GCC 12/13 warnings #1536

Merged
merged 2 commits into from
Apr 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/windows-alt.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ jobs:
CMAKE_FIND_ROOT_PATH_MODE_PROGRAM=NEVER \
CMAKE_FIND_ROOT_PATH_MODE_LIBRARY=ONLY \
CMAKE_FIND_ROOT_PATH_MODE_INCLUDE=ONLY \
CMAKE_BUILD_TYPE=Release \
- name: Build Project
run: cmake --build ./build --target all
- name: Run tests
Expand Down Expand Up @@ -63,6 +64,7 @@ jobs:
CMAKE_SYSTEM_NAME=Windows \
CMAKE_SYSTEM_PROCESSOR=x86_64 \
CMAKE_BUILD_TOOL=ninja.exe \
CMAKE_BUILD_TYPE=Release \
- name: Build Project
run: cmake --build ./build --target all
- name: Run tests
Expand Down
4 changes: 3 additions & 1 deletion crypto/bytestring/cbs.c
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,9 @@ int CBS_get_asn1_int64(CBS *cbs, int64_t *out) {
}
uint8_t sign_extend[sizeof(int64_t)];
memset(sign_extend, is_negative ? 0xff : 0, sizeof(sign_extend));
for (size_t i = 0; i < len; i++) {
// GCC 12/13 report `stringop-overflow` on the following line
// without additional condition: `i < sizeof(int64_t)`
for (size_t i = 0; i < len && i < sizeof(int64_t); i++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

huh, that can already be deduced from (len > sizeof(int64_t)....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I believe so. But gcc-13 seems to think otherwise:

/Users/justsmth/repos/aws-lc/crypto/bytestring/cbs.c: In function 'CBS_get_asn1_int64':
/Users/justsmth/repos/aws-lc/crypto/bytestring/cbs.c:534:20: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=]
  534 |     sign_extend[i] = data[len - i - 1];
      |     ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~
/Users/justsmth/repos/aws-lc/crypto/bytestring/cbs.c:521:11: note: at offset 8 into destination object 'sign_extend' of size 8
  521 |   uint8_t sign_extend[sizeof(int64_t)];
      |           ^~~~~~~~~~~

¯\_(ツ)_/¯

// `data` is big-endian.
// Values are always shifted toward the "little" end.
#ifdef OPENSSL_BIG_ENDIAN
Expand Down
4 changes: 2 additions & 2 deletions crypto/fipsmodule/ecdsa/ecdsa.c
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ ECDSA_SIG *ecdsa_digestsign_no_self_test(const EVP_MD *md, const uint8_t *input,
const uint8_t *nonce,
size_t nonce_len) {
uint8_t digest[EVP_MAX_MD_SIZE];
unsigned int digest_len;
unsigned int digest_len = EVP_MAX_MD_SIZE;
Copy link
Contributor

Choose a reason for hiding this comment

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

should be zero, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe EVP_MAX_MD_SIZE is correct. This value gets dereferenced as the len parameter in the call to EVP_DigestFinalXOF.

if (!EVP_Digest(input, in_len, digest, &digest_len, md, NULL)) {
return 0;
}
Expand All @@ -455,7 +455,7 @@ int ecdsa_digestverify_no_self_test(const EVP_MD *md, const uint8_t *input,
size_t in_len, const ECDSA_SIG *sig,
const EC_KEY *eckey){
uint8_t digest[EVP_MAX_MD_SIZE];
unsigned int digest_len;
unsigned int digest_len = EVP_MAX_MD_SIZE;
if (!EVP_Digest(input, in_len, digest, &digest_len, md, NULL)) {
return 0;
}
Expand Down
4 changes: 2 additions & 2 deletions crypto/fipsmodule/rsa/rsa.c
Original file line number Diff line number Diff line change
Expand Up @@ -692,7 +692,7 @@ int rsa_digestsign_no_self_test(const EVP_MD *md, const uint8_t *input,
size_t in_len, uint8_t *out, unsigned *out_len,
RSA *rsa) {
uint8_t digest[EVP_MAX_MD_SIZE];
unsigned int digest_len;
unsigned int digest_len = EVP_MAX_MD_SIZE;
if (!EVP_Digest(input, in_len, digest, &digest_len, md, NULL)) {
return 0;
}
Expand Down Expand Up @@ -760,7 +760,7 @@ int rsa_digestverify_no_self_test(const EVP_MD *md, const uint8_t *input,
size_t in_len, const uint8_t *sig,
size_t sig_len, RSA *rsa) {
uint8_t digest[EVP_MAX_MD_SIZE];
unsigned int digest_len;
unsigned int digest_len = EVP_MAX_MD_SIZE;
if (!EVP_Digest(input, in_len, digest, &digest_len, md, NULL)) {
return 0;
}
Expand Down
Loading