search

fortran-lang / stdlib

Fortran Standard Library
https://stdlib.fortran-lang.org
MIT License
1.09k stars 168 forks source link

gfortran: Arithmetic overflow converting INTEGER(16) to INTEGER(4) in stdlib_hash_32bit.f90 #635

Open Beliavsky opened 2 years ago

Beliavsky commented 2 years ago

I ran git checkout stdlib-fpm. My gfortran version on WSL2 is

(fpm) /mnt/c/fortran/public_domain/github/stdlib$ gfortran --version
GNU Fortran (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0

When I build with fpm I get Error: Arithmetic overflow converting INTEGER(16) to INTEGER(4) at (1)

in stdlib_hash_32bit.f90

(fpm) /mnt/c/fortran/public_domain/github/stdlib$ fpm build --profile release
 + mkdir -p build/dependencies
Initialized empty Git repository in /mnt/c/fortran/public_domain/github/stdlib/build/dependencies/test-drive/.git/
remote: Enumerating objects: 31, done.        
remote: Counting objects: 100% (31/31), done.        
remote: Compressing objects: 100% (29/29), done.        
remote: Total 31 (delta 5), reused 10 (delta 0), pack-reused 0        
Unpacking objects: 100% (31/31), 32.30 KiB | 118.00 KiB/s, done.
From https://github.com/fortran-lang/test-drive
 * tag               v0.4.0     -> FETCH_HEAD
 + mkdir -p build/gfortran_5F75F7C92365B9B9/stdlib
 + gfortran -c ././src/stdlib_array.f90 -O3 -Wimplicit-interface -fPIC -fmax-errors=1 -funroll-loops -fcoarray=single -J build/gfortran_5F75F7C92365B9B9/stdlib -I build/gfortran_5F75F7C92365B9B9/stdlib  -o build/gfortran_5F75F7C92365B9B9/stdlib/src_stdlib_array.f90.o
 + gfortran -c ././src/stdlib_kinds.f90 -O3 -Wimplicit-interface -fPIC -fmax-errors=1 -funroll-loops -fcoarray=single -J build/gfortran_5F75F7C92365B9B9/stdlib -I build/gfortran_5F75F7C92365B9B9/stdlib  -o build/gfortran_5F75F7C92365B9B9/stdlib/src_stdlib_kinds.f90.o
 + gfortran -c ././src/stdlib_system.F90 -O3 -Wimplicit-interface -fPIC -fmax-errors=1 -funroll-loops -fcoarray=single -J build/gfortran_5F75F7C92365B9B9/stdlib -I build/gfortran_5F75F7C92365B9B9/stdlib  -o build/gfortran_5F75F7C92365B9B9/stdlib/src_stdlib_system.F90.o
 + gfortran -c ././src/stdlib_version.f90 -O3 -Wimplicit-interface -fPIC -fmax-errors=1 -funroll-loops -fcoarray=single -J build/gfortran_5F75F7C92365B9B9/stdlib -I build/gfortran_5F75F7C92365B9B9/stdlib  -o build/gfortran_5F75F7C92365B9B9/stdlib/src_stdlib_version.f90.o
 + gfortran -c build/dependencies/test-drive/src/testdrive.F90 -O3 -Wimplicit-interface -fPIC -fmax-errors=1 -funroll-loops -fcoarray=single -J build/gfortran_5F75F7C92365B9B9/stdlib -I build/gfortran_5F75F7C92365B9B9/stdlib  -o build/gfortran_5F75F7C92365B9B9/stdlib/build_dependencies_test-drive_src_testdrive.F90.o
 + gfortran -c build/dependencies/test-drive/src/testdrive_version.f90 -O3 -Wimplicit-interface -fPIC -fmax-errors=1 -funroll-loops -fcoarray=single -J build/gfortran_5F75F7C92365B9B9/stdlib -I build/gfortran_5F75F7C92365B9B9/stdlib  -o build/gfortran_5F75F7C92365B9B9/stdlib/build_dependencies_test-drive_src_testdrive_version.f90.o
 + gfortran -c ././src/stdlib_ascii.f90 -O3 -Wimplicit-interface -fPIC -fmax-errors=1 -funroll-loops -fcoarray=single -J build/gfortran_5F75F7C92365B9B9/stdlib -I build/gfortran_5F75F7C92365B9B9/stdlib  -o build/gfortran_5F75F7C92365B9B9/stdlib/src_stdlib_ascii.f90.o
 + gfortran -c ././src/stdlib_hash_64bit.f90 -O3 -Wimplicit-interface -fPIC -fmax-errors=1 -funroll-loops -fcoarray=single -J build/gfortran_5F75F7C92365B9B9/stdlib -I build/gfortran_5F75F7C92365B9B9/stdlib  -o build/gfortran_5F75F7C92365B9B9/stdlib/src_stdlib_hash_64bit.f90.o
 + gfortran -c test/test_sleep.f90 -O3 -Wimplicit-interface -fPIC -fmax-errors=1 -funroll-loops -fcoarray=single -J build/gfortran_5F75F7C92365B9B9/stdlib -I build/gfortran_5F75F7C92365B9B9/stdlib  -o build/gfortran_5F75F7C92365B9B9/stdlib/test_test_sleep.f90.o
 + gfortran -c ././src/stdlib_hash_32bit.f90 -O3 -Wimplicit-interface -fPIC -fmax-errors=1 -funroll-loops -fcoarray=single -J build/gfortran_5F75F7C92365B9B9/stdlib -I build/gfortran_5F75F7C92365B9B9/stdlib  -o build/gfortran_5F75F7C92365B9B9/stdlib/src_stdlib_hash_32bit.f90.o
././src/stdlib_hash_64bit.f90:66:29:

   66 |         pow64_over_phi = int(z'9E3779B97F4A7C15', int64)
      |                             1
Error: Arithmetic overflow converting INTEGER(16) to INTEGER(8) at (1). This check can be disabled with the option ‘-fno-range-check’
compilation terminated due to -fmax-errors=1.
././src/stdlib_hash_32bit.f90:25:30:

   25 |         pow32_over_phi = int( z'9E3779B9', int32 )
      |                              1
Error: Arithmetic overflow converting INTEGER(16) to INTEGER(4) at (1). This check can be disabled with the option ‘-fno-range-check’
compilation terminated due to -fmax-errors=1.
 <ERROR> Compilation failed for object "src_stdlib_hash_32bit.f90.o"
 <ERROR> Compilation failed for object "src_stdlib_hash_64bit.f90.o"
STOP 1
jvdp1 commented 2 years ago

The following option -fno-range-check seems to be missing. Could you check if it is present?

Beliavsky commented 2 years ago

Thanks. When I run

fpm --flag -fno-range-check build --profile release

it works. The source files are compiled with the options

-O3 -Wimplicit-interface -fPIC -fmax-errors=1 -funroll-loops -fcoarray=single -fno-range-check -J

Please excuse a naive question, which is more about fpm, but where are those options coming from?

Euler-37 commented 2 years ago

It seems like a gfortran 9's bug , and gfortran 10 and after fixed it.

godbolt

jvdp1 commented 2 years ago

fpm --flag -fno-range-check build --profile release

it works. The source files are compiled with the options

-O3 -Wimplicit-interface -fPIC -fmax-errors=1 -funroll-loops -fcoarray=single -fno-range-check -J

Please excuse a naive question, which is more about fpm, but where are those options coming from?

These options are triggerred by --profile release. The option --profile debug should result in other options.

wclodius2 commented 2 years ago

The hash codes emulate unsigned arithmetic using signed arithmetic assuming the underlying arithmetic is two's complement for integer over or under flows. While there have been processors that use one's complement or signed magnitude implementations, such processors do not currently have a hosted Fortran 90+ compiler and the only processors with hosted Fortran 90 compilers have two's complement signed integers. Such compilers find it easiest to implement signed arithmetic using two's complement arithmetic. However over and underflowing in many contexts represent an error in the code and compilers often have flags to turn on or off over and underflow detection. As there is a performance cost for such detection, detection is typically turned off for runtime processing with significant optimization. However there is no such cost for compile time processing, and gfortran 9.x and earlier by default reported an error when a hexadecimal literal (which is defined to be an unsigned integer) was larger than the largest positive integer of that kind. The default for those versions of the compiler could be overridden by setting the flag -fno-range-check. As some of the integer constants use in the hash algorithms had the highest bit set the flag had to be set for those versions of the compiler. Later version and Ifort, by default treat a set his bit as setting the sign bit as would be expected for two's complement integers.

kjelljorner commented 2 years ago

This effectively breaks the use of stdlib as a dependency of conda-forge builds on MacOS, where GFortran 9.3.0 is the current default.

wclodius2 commented 2 years ago

What prevents conda-forge from using the -fno-range-check flag for GFortran 9.3.0?

kjelljorner commented 2 years ago

@wclodius2 nothing it turns out. I was fetching stdlib with the CMake example here and apparently the standard compiler flags for stdlib are not retained in this procedure. I could now add -fno-range-check back in again though and it works fine. Maybe @awvwgk would have an idea if this can be facilitated.

awvwgk commented 2 years ago

I pushed a fix for keeping the -fno-range-check option to #642, which already fixes another CMake issue.

14NGiestas commented 2 years ago

@awvwgk @Beliavsky was this issue solved by #642? Should it be closed?

awvwgk commented 2 years ago

This is only fixed for the CMake build, it is still an issue for fpm.

ivan-pi commented 1 month ago

The hash module (or any module relying on two-complement modulo arithmetic) should be compiled with -fwrapv flag when using gfortran >=v13. Details given in the release notes:

GCC 13 includes new optimizations which may change behavior on integer overflow. Traditional code, like linear congruential pseudo-random number generators in old programs and relying on a specific, non-standard behavior may now generate unexpected results. The option -fsanitize=undefined can be used to detect such code at runtime. It is recommended to use the intrinsic subroutine RANDOM_NUMBER for random number generators or, if the old behavior is desired, to use the -fwrapv option. Note that this option can impact performance.

jvdp1 commented 1 month ago

Thank you Ivan for the heads up. Could this flag -fwrapv be used with gfortran <13?

Le lun. 21 oct. 2024 à 00:15, Ivan Pribec @.***> a écrit :

The hash module (or any module relying on two-complement modulo arithmetic) should be compiled with -fwrapv flag when using gfortran

=v13. Details given in the release notes https://gcc.gnu.org/gcc-13/porting_to.html:

GCC 13 includes new optimizations which may change behavior on integer overflow. Traditional code, like linear congruential pseudo-random number generators in old programs and relying on a specific, non-standard behavior may now generate unexpected results. The option -fsanitize=undefined can be used to detect such code at runtime. It is recommended to use the intrinsic subroutine RANDOM_NUMBER for random number generators or, if the old behavior is desired, to use the -fwrapv option. Note that this option can impact performance.

— Reply to this email directly, view it on GitHub https://github.com/fortran-lang/stdlib/issues/635#issuecomment-2425253242, or unsubscribe https://github.com/notifications/unsubscribe-auth/AD5RO7EBZFA4SDVRNSH2SZTZ4QTP7AVCNFSM6AAAAABQI5JMLGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMRVGI2TGMRUGI . You are receiving this because you commented.Message ID: @.***>

ivan-pi commented 1 month ago

It can indeed. That is more explicit.

I think the correct way to add file-specific flags in CMake is something like:

set_source_files_properties(stdlib_hash_64bit.f90 
    PROPERTIES COMPILE_FLAGS $<$<Fortran_COMPILER_ID:GNU>:-fwrapv>)
set_source_files_properties(stdlib_hash_32bit.f90 
    PROPERTIES COMPILE_FLAGS $<$<Fortran_COMPILER_ID:GNU>:-fwrapv>)