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

fixed 32-bit integer overflow in stdlib_io_npy #655

Merged
merged 3 commits into from
May 5, 2022

Conversation

degawa
Copy link
Contributor

@degawa degawa commented Apr 24, 2022

32-bit integer overflows in stdlib_io_npy_save.fypp and stdlib_io_npy_load.fypp are fixed from 2**32 to 2**24, to become the power of base-number (256) at the 4th digit a base-256 number.
To improve readability and understandability, the powers of base-number are refactored from 2**8, 2**16, 2**24 to 256**1, 256**2, 256**3, respectively.

The tasks done are summarized as follows:

  • fixed and refactored stdlib_io_npy_save.fypp
  • fixed and refactored stdlib_io_npy_load.fypp
  • built with gfortran 10.3, ifort 2021.1, and nagfor 7.1 on Windows 10 with cmake 3.20.3
  • executed cmake --build build --target test to run the test npy and confirmed it passed

closes #647

Note that config/DefaultFlags.cmake is locally changed to specify compiler flags for nagfor as follows:

elseif(CMAKE_Fortran_COMPILER_ID MATCHES "NAG")
  set(
    CMAKE_Fortran_FLAGS_INIT
    "-fpp"
    "-f2018"
    "-ieee=full"
  )
  set(
    CMAKE_Fortran_FLAGS_RELEASE_INIT
  )
  set(
    CMAKE_Fortran_FLAGS_DEBUG_INIT
    "-g"
    "-nan"
  )

@awvwgk awvwgk added bug Something isn't working reviewers needed This patch requires extra eyes topic: IO Common input/output related features labels Apr 24, 2022
Copy link
Contributor

@sakamoti sakamoti left a comment

Choose a reason for hiding this comment

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

I think this patch looks fine.
stdlib_io_npy_save and stdlib_io_npy_load have passed compilation phaze with nagfor7.1 (Linux version).

Copy link
Member

@awvwgk awvwgk left a comment

Choose a reason for hiding this comment

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

Thanks for sharing. Looks good to me.

Copy link
Member

@milancurcic milancurcic left a comment

Choose a reason for hiding this comment

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

Looks good, thank you!

@milancurcic milancurcic merged commit 5ad2837 into fortran-lang:master May 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working reviewers needed This patch requires extra eyes topic: IO Common input/output related features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

32bit integer overflow in stdlib_io_npy_save.fypp and load.fypp
4 participants