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

New warnings on the version 2.28.2 on ecp.c and bignum.c #6924

Closed
Kasimashi opened this issue Jan 13, 2023 · 6 comments · Fixed by #6929
Closed

New warnings on the version 2.28.2 on ecp.c and bignum.c #6924

Kasimashi opened this issue Jan 13, 2023 · 6 comments · Fixed by #6929
Assignees
Labels
bug size-s Estimated task size: small (~2d)

Comments

@Kasimashi
Copy link

Summary

Using an EWARM compiler (IAR for Embedded Workbench ARM) , I've new warnings on MBEDTLS_MPI_CHK macro.

System information

Mbed TLS version (number or commit id): 2.28.2
Operating system and version: Windows
Configuration (if not default, please attach mbedtls_config.h):
Compiler and options (if you used a pre-built binary, please indicate how you obtained it): EWARM
Additional environment information:

Expected behavior

We should not have any warning on compilation.

Actual behavior

Issue spotted on bignum.c file.

image

Warning[Pe546]: transfer of control bypasses initialization of: mbedtls\library\bignum.c 2174 variable "exponent_bits_in_window" (declared at line 2183)

Same issue on ecp.c file

image

Steps to reproduce

Compile MbedTLS using EWARM 9.20.2

Additional information

@Kasimashi Kasimashi changed the title New warnings on the version 2.28.2 New warnings on the version 2.28.2 on ecp.c and bignum.c Jan 13, 2023
@paul-elliott-arm
Copy link
Member

Hi,

I have had a look at this, and I presume the complaint is from the MBEDTLS_MPI_CHK() potentially failing and jumping to the cleanup label. This seems spurious to me, as not only is the initialisation of exponent_bits_in_window skipped, its also unused in this case. The same goes for have_rng

@paul-elliott-arm paul-elliott-arm self-assigned this Jan 13, 2023
@paul-elliott-arm paul-elliott-arm added bug size-s Estimated task size: small (~2d) labels Jan 13, 2023
@paul-elliott-arm
Copy link
Member

Hi Kasimashi,

I have pushed a PR to solve these (#6929) but I cannot test this. If you could test this and report back on that PR, that would be great, as I will need to forward port this back to development as well.

@Kasimashi
Copy link
Author

Kasimashi commented Jan 20, 2023

Hello @paul-elliott-arm

Thanks you very much for the feedback and for the correction : its remain some warning on ecp.c but on bignum.c it's ok.

On ecp.c you have the same issue on the function ecp_mul_comb_after_precomp

MBEDTLS_MPI_CHK is called before have_rng could you please modify that ?

image

Last as you can spot on the screen above there is a last warning in the function : ecp_drbg_seed, if you can also fix it (It's an old warning from previous version) maybe you have to cast it to : (mbedtls_md_type_t)

Warning[Pe188]: enumerated type mixed with another type	library\ecp.c	165	

image

@paul-elliott-arm
Copy link
Member

Hi @Kasimashi - I have updated the PR, please re-review. If there are a lot more issues, and in future I invite you to submit a PR fixing the warnings, as otherwise this is very slow going!

@Kasimashi
Copy link
Author

Thanks you !! :)

I would do that next time.

I'm not used to the process yet but it will come :)

Thanks !

All warnings are resolved with this PR.

@Kasimashi
Copy link
Author

@paul-elliott-arm Pull Request merged this ticket can be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug size-s Estimated task size: small (~2d)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants