diff --git a/Benchmark/src/CMakeLists.txt b/Benchmark/src/CMakeLists.txt index 6863b83..7e1f266 100644 --- a/Benchmark/src/CMakeLists.txt +++ b/Benchmark/src/CMakeLists.txt @@ -21,3 +21,4 @@ endif() add_benchmark(PWR079) add_benchmark(PWR080) add_benchmark(PWR081) +add_benchmark(PWR083) diff --git a/Benchmark/src/PWR083.cpp b/Benchmark/src/PWR083.cpp new file mode 100644 index 0000000..1d2f9a7 --- /dev/null +++ b/Benchmark/src/PWR083.cpp @@ -0,0 +1,37 @@ +#include "Benchmark.h" + +// Forward-declare the functions to benchmark +extern "C" { +void update_simulation_state_by_fixed_amount(const int *n, double *state); +void update_simulation_state_by_fixed_amount_improved(const int *n, + double *state); +} + +constexpr int N = 1024 * 1024; + +#if OCB_ENABLE_Fortran + +static void FortranExampleBench(benchmark::State &state) { + auto array = OpenCatalog::CreateRandomVector(N); + + for (auto _ : state) { + update_simulation_state_by_fixed_amount(&N, array.data()); + benchmark::DoNotOptimize(array); + } +} + +static void FortranImprovedBench(benchmark::State &state) { + auto array = OpenCatalog::CreateRandomVector(N); + + for (auto _ : state) { + update_simulation_state_by_fixed_amount_improved(&N, array.data()); + benchmark::DoNotOptimize(array); + } +} + +// The improved benchmark prevents the runtime bug, while not incurring in any +// performance penalty +OC_BENCHMARK("PWR083 Fortran Example", FortranExampleBench); +OC_BENCHMARK("PWR083 Fortran Improved", FortranImprovedBench); + +#endif diff --git a/Checks/PWR083/README.md b/Checks/PWR083/README.md new file mode 100644 index 0000000..c7f421b --- /dev/null +++ b/Checks/PWR083/README.md @@ -0,0 +1,161 @@ +# PWR083: Match dummy and actual argument types in procedure calls + +### Issue + +Calling a procedure with actual arguments that are incompatible with the +characteristics of the procedure's dummy arguments (e.g., different `type`, +`kind`, `rank`, etc.) can lead to compilation failures or, if not caught, +undefined behavior at runtime. + +### Actions + +Ensure the arguments supplied at the call site match the characteristics of the +procedure's dummy arguments. Depending on the situation, this may involve +actions such as: + +- Changing the `type` and/or `kind` of the actual argument declarations. +- Applying explicit type conversions (e.g., `int(...)`, `real(...)`). +- Providing alternative procedure implementations for different argument types. + +Also verify that arguments are passed in the intended order; a type mismatch +can be caused by accidentally swapping argument positions. + +### Relevance + +When calling a Fortran procedure, actual and dummy arguments are expected to +have compatible characteristics (`type`, `kind`, `rank`, etc.). A common +mistake is to inadvertently mix numeric types; for example: + +- Passing a `real` where an `integer` is expected. +- Using the wrong `kind` (e.g., `real32` instead of `real64`). + +When an explicit procedure interface is available at the call site, compilers +can typically catch and report type mismatch issues during compilation. + +However, Fortran also allows calling procedures without information about the +expected arguments; in such cases, an _implicit interface_ is used. The caller +effectively passes a list of memory addresses, which the called procedure +interprets according to its dummy argument declarations. If the actual +arguments do not match the dummy arguments, the result is undefined behavior +and may manifest as incorrect results, "random" behavior, or even crashes. + +> [!TIP] +> To enhance code safety and reliability, ensure procedures provide +> explicit interfaces to their callers. Check the [PWR068 entry](../PWR068/) +> for more details! + +### Code examples + +The following example advances a simulation time using a step count +`numberSteps`. The procedure expects an `integer` step count, but the caller +accidentally provides a `real` value: + +```fortran {6} showLineNumbers +! simulation.f90 +pure subroutine updateSimulationTime(numberSteps, prevTime, newTime) + use iso_fortran_env, only: real32 + implicit none + + integer, intent(in) :: numberSteps + real(kind=real32), intent(in) :: prevTime + real(kind=real32), intent(out) :: newTime + + ! Each step is 0.1 seconds + newTime = prevTime + numberSteps * 0.1_real32 +end subroutine updateSimulationTime +``` + +```fortran {6,7,12} showLineNumbers +! example.f90 +program call_with_type_mismatch + use iso_fortran_env, only: real32 + implicit none + + external :: updateSimulationTime + real(kind=real32) :: numberSteps, prevTime, newTime + + numberSteps = 10 + prevTime = 0.0 + + call updateSimulationTime(numberSteps, prevTime, newTime) + print *, "New time = ", newTime +end program call_with_type_mismatch +``` + +Because `updateSimulationTime()` is an `external` procedure defined in another +source file, the call uses an implicit interface. Compilers will typically +allow this program to compile despite the type mismatch, producing an incorrect +result at runtime: + +```txt +$ gfortran --version +GNU Fortran (Ubuntu 13.3.0-6ubuntu2~24.04) 13.3.0 +$ gfortran simulation.f90 example.f90 +$ ./a.out + New time = 109261624. +``` + +A simple solution is to declare `numberSteps` using the type expected by the +procedure: + +```fortran {7} showLineNumbers +! solution.f90 +program correct_call + use iso_fortran_env, only: real32 + implicit none + + external :: updateSimulationTime + integer :: numberSteps + real(kind=real32) :: prevTime, newTime + + numberSteps = 10 + prevTime = 0.0 + + call updateSimulationTime(numberSteps, prevTime, newTime) + print *, "New time = ", newTime +end program correct_call +``` + +Now, the program will produce the correct result: + +```txt +$ gfortran simulation.f90 solution.f90 +$ ./a.out + New time = 1.00000000 +``` + +Ultimately, it is recommended to encapsulate all procedures within modules so +that callers have explicit interfaces. This enables the compiler to verify the +provided arguments against the actual dummy arguments, preventing +difficult-to-diagnose runtime bugs. + +> [!TIP] +> Check the [PWR068 entry](../PWR068/) for more details on implicit and +> explicit interfaces! + +### Related resources + +- [PWR083 + examples](https://github.com/codee-com/open-catalog/tree/main/Checks/PWR083/) + +### References + +- ["Implicit +Interfaces"](https://people.cs.vt.edu/~asandu/Courses/MTU/CS2911/fortran_notes/node44.html), +Adrian Sandu. [last checked January 2026] + +- ["More on Implicit +Interfaces"](https://people.cs.vt.edu/~asandu/Courses/MTU/CS2911/fortran_notes/node181.html), +Adrian Sandu. [last checked January 2026] + +- ["Explicit +Interfaces"](https://people.cs.vt.edu/~asandu/Courses/MTU/CS2911/fortran_notes/node182.html), +Adrian Sandu. [last checked January 2026] + +- ["Doctor Fortran Gets +Explicit!"](https://web.archive.org/web/20130803094211/http://software.intel.com/en-us/forums/topic/275071), +Steve Lionel. [last checked January 2026] + +- ["Doctor Fortran Gets Explicit - Again! +"](https://web.archive.org/web/20130113070703/http://software.intel.com/en-us/blogs/2012/01/05/doctor-fortran-gets-explicit-again), +Steve Lionel. [last checked January 2026] diff --git a/Checks/PWR083/benchmark/example.f90 b/Checks/PWR083/benchmark/example.f90 new file mode 100644 index 0000000..68bf0a9 --- /dev/null +++ b/Checks/PWR083/benchmark/example.f90 @@ -0,0 +1,20 @@ +! PWR083: Match dummy and actual argument types in procedure calls + +! NOT-PWR068: External procedure used to demonstrate how compilers can't catch +! the type mismatch +! NOT-PWR070: Explicit-shape arrays used for C-interoperability +subroutine update_simulation_state_by_fixed_amount(n, state) bind(c) + use iso_c_binding, only: c_double, c_int + use iso_fortran_env, only: real32 + implicit none + + external :: update_simulation_state + + integer(kind=c_int), intent(in) :: n + real(kind=c_double), dimension(n), intent(inout) :: state + + real(kind=real32) :: numberSteps + + numberSteps = 100 + call update_simulation_state(numberSteps, n, state) +end subroutine update_simulation_state_by_fixed_amount diff --git a/Checks/PWR083/benchmark/simulation.f90 b/Checks/PWR083/benchmark/simulation.f90 new file mode 100644 index 0000000..2ea4301 --- /dev/null +++ b/Checks/PWR083/benchmark/simulation.f90 @@ -0,0 +1,17 @@ +! PWR083: Match dummy and actual argument types in procedure calls + +! NOT-PWR070: Explicit-shape arrays used for C-interoperability +pure subroutine update_simulation_state(numberSteps, n, state) + use iso_c_binding, only: c_double, c_int + implicit none + + integer, intent(in) :: numberSteps + integer(kind=c_int), intent(in) :: n + real(kind=c_double), dimension(n), intent(inout) :: state + + integer(kind=c_int) :: i + + do i = 1, n + state(i) = state(i) + numberSteps * 9.81_c_double + end do +end subroutine update_simulation_state diff --git a/Checks/PWR083/benchmark/solution.f90 b/Checks/PWR083/benchmark/solution.f90 new file mode 100644 index 0000000..481a65c --- /dev/null +++ b/Checks/PWR083/benchmark/solution.f90 @@ -0,0 +1,19 @@ +! PWR083: Match dummy and actual argument types in procedure calls + +! NOT-PWR068: External procedure used to demonstrate how compilers can't catch +! the type mismatch +! NOT-PWR070: Explicit-shape arrays used for C-interoperability +subroutine update_simulation_state_by_fixed_amount_improved(n, state) bind(c) + use iso_c_binding, only: c_double, c_int + implicit none + + external :: update_simulation_state + + integer(kind=c_int), intent(in) :: n + real(kind=c_double), dimension(n), intent(inout) :: state + + integer :: numberSteps + + numberSteps = 100 + call update_simulation_state(numberSteps, n, state) +end subroutine update_simulation_state_by_fixed_amount_improved diff --git a/Checks/PWR083/example.f90 b/Checks/PWR083/example.f90 new file mode 100644 index 0000000..9815b7d --- /dev/null +++ b/Checks/PWR083/example.f90 @@ -0,0 +1,17 @@ +! PWR083: Match dummy and actual argument types in procedure calls + +! NOT-PWR068: External procedure used to demonstrate how compilers can't catch +! the type mismatch +program call_with_type_mismatch + use iso_fortran_env, only: real32 + implicit none + + external :: updateSimulationTime + real(kind=real32) :: numberSteps, prevTime, newTime + + numberSteps = 10 + prevTime = 0.0 + + call updateSimulationTime(numberSteps, prevTime, newTime) + print *, "New time = ", newTime +end program call_with_type_mismatch diff --git a/Checks/PWR083/simulation.f90 b/Checks/PWR083/simulation.f90 new file mode 100644 index 0000000..c8199b3 --- /dev/null +++ b/Checks/PWR083/simulation.f90 @@ -0,0 +1,13 @@ +! PWR083: Match dummy and actual argument types in procedure calls + +pure subroutine updateSimulationTime(numberSteps, prevTime, newTime) + use iso_fortran_env, only: real32 + implicit none + + integer, intent(in) :: numberSteps + real(kind=real32), intent(in) :: prevTime + real(kind=real32), intent(out) :: newTime + + ! Each step is 0.1 seconds + newTime = prevTime + numberSteps * 0.1_real32 +end subroutine updateSimulationTime diff --git a/Checks/PWR083/solution.f90 b/Checks/PWR083/solution.f90 new file mode 100644 index 0000000..a2691d7 --- /dev/null +++ b/Checks/PWR083/solution.f90 @@ -0,0 +1,18 @@ +! PWR083: Match dummy and actual argument types in procedure calls + +! NOT-PWR068: External procedure used to demonstrate how compilers can't catch +! the type mismatch +program correct_call + use iso_fortran_env, only: real32 + implicit none + + external :: updateSimulationTime + integer :: numberSteps + real(kind=real32) :: prevTime, newTime + + numberSteps = 10 + prevTime = 0.0 + + call updateSimulationTime(numberSteps, prevTime, newTime) + print *, "New time = ", newTime +end program correct_call diff --git a/README.md b/README.md index 82082de..d39ddaf 100644 --- a/README.md +++ b/README.md @@ -97,6 +97,7 @@ designed to demonstrate: | [PWR079](Checks/PWR079/) | Avoid undefined behavior due to uninitialized variables | correctness, portability, security | [CWE-758](https://cwe.mitre.org/data/definitions/758.html), [CWE-908](https://cwe.mitre.org/data/definitions/908.html), [CWE-909](https://cwe.mitre.org/data/definitions/909.html) | [6.13](https://j3-fortran.org/doc/year/23/23-241.pdf), [6.22](https://j3-fortran.org/doc/year/23/23-241.pdf), [6.56](https://j3-fortran.org/doc/year/23/23-241.pdf) | [EXP33-C](https://wiki.sei.cmu.edu/confluence/display/c/EXP33-C.+Do+not+read+uninitialized+memory), [EXP34-C](https://wiki.sei.cmu.edu/confluence/display/c/EXP34-C.+Do+not+dereference+null+pointers), [MSC15-C](https://wiki.sei.cmu.edu/confluence/display/c/MSC15-C.+Do+not+depend+on+undefined+behavior) | [EXP53-CPP](https://wiki.sei.cmu.edu/confluence/spaces/cplusplus/pages/88046609/EXP53-CPP.+Do+not+read+uninitialized+memory) | ✓ | ✓ | ✓ | | | [PWR080](Checks/PWR080/) | Conditionally initialized variables can lead to undefined behavior | correctness, portability, security | [CWE-758](https://cwe.mitre.org/data/definitions/758.html), [CWE-908](https://cwe.mitre.org/data/definitions/908.html), [CWE-909](https://cwe.mitre.org/data/definitions/909.html) | [6.13](https://j3-fortran.org/doc/year/23/23-241.pdf), [6.22](https://j3-fortran.org/doc/year/23/23-241.pdf), [6.56](https://j3-fortran.org/doc/year/23/23-241.pdf) | [EXP33-C](https://wiki.sei.cmu.edu/confluence/display/c/EXP33-C.+Do+not+read+uninitialized+memory), [EXP34-C](https://wiki.sei.cmu.edu/confluence/display/c/EXP34-C.+Do+not+dereference+null+pointers), [MSC15-C](https://wiki.sei.cmu.edu/confluence/display/c/MSC15-C.+Do+not+depend+on+undefined+behavior) | [EXP53-CPP](https://wiki.sei.cmu.edu/confluence/spaces/cplusplus/pages/88046609/EXP53-CPP.+Do+not+read+uninitialized+memory) | ✓ | ✓ | ✓ | | | [PWR081](Checks/PWR081/) | Uninitialized output arguments can lead to undefined behavior | correctness, portability, security | [CWE-758](https://cwe.mitre.org/data/definitions/758.html), [CWE-908](https://cwe.mitre.org/data/definitions/908.html), [CWE-909](https://cwe.mitre.org/data/definitions/909.html) | [6.13](https://j3-fortran.org/doc/year/23/23-241.pdf), [6.22](https://j3-fortran.org/doc/year/23/23-241.pdf), [6.56](https://j3-fortran.org/doc/year/23/23-241.pdf) | [EXP33-C](https://wiki.sei.cmu.edu/confluence/display/c/EXP33-C.+Do+not+read+uninitialized+memory), [EXP34-C](https://wiki.sei.cmu.edu/confluence/display/c/EXP34-C.+Do+not+dereference+null+pointers), [MSC15-C](https://wiki.sei.cmu.edu/confluence/display/c/MSC15-C.+Do+not+depend+on+undefined+behavior) | | | ✓ | | | +| [PWR083](Checks/PWR083/) | Match dummy and actual argument types in procedure calls | correctness, security | [CWE-628](https://cwe.mitre.org/data/definitions/628.html) | [6.11](https://j3-fortran.org/doc/year/23/23-241.pdf), [6.32](https://j3-fortran.org/doc/year/23/23-241.pdf), [6.53](https://j3-fortran.org/doc/year/23/23-241.pdf) | [EXP37-C](https://wiki.sei.cmu.edu/confluence/display/c/EXP37-C.+Call+functions+with+the+correct+number+and+type+of+arguments) | | | ✓ | | | | [PWD002](Checks/PWD002/) | Unprotected multithreading reduction operation | correctness, security | [CWE-366](https://cwe.mitre.org/data/definitions/366.html), [CWE-820](https://cwe.mitre.org/data/definitions/820.html) | [6.61](https://j3-fortran.org/doc/year/23/23-241.pdf) | [CON07-C](https://wiki.sei.cmu.edu/confluence/display/c/CON07-C.+Ensure+that+compound+operations+on+shared+variables+are+atomic), [CON43-C](https://wiki.sei.cmu.edu/confluence/display/c/CON43-C.+Do+not+allow+data+races+in+multithreaded+code) | | ✓ | ✓ | ✓ | | | [PWD003](Checks/PWD003/) | Missing array range in data copy to the GPU | correctness, security | [CWE-131](https://cwe.mitre.org/data/definitions/131.html), [CWE-758](https://cwe.mitre.org/data/definitions/758.html) | [6.56](https://j3-fortran.org/doc/year/23/23-241.pdf) | [MSC15-C](https://wiki.sei.cmu.edu/confluence/display/c/MSC15-C.+Do+not+depend+on+undefined+behavior) | | ✓ | ✓ | ✓ | | | [PWD004](Checks/PWD004/) | Out-of-memory-bounds array access | correctness, security | [CWE-125](https://cwe.mitre.org/data/definitions/125.html), [CWE-787](https://cwe.mitre.org/data/definitions/787.html) | [6.8](https://j3-fortran.org/doc/year/23/23-241.pdf), [6.9](https://j3-fortran.org/doc/year/23/23-241.pdf), [6.10](https://j3-fortran.org/doc/year/23/23-241.pdf) | [ARR30-C](https://wiki.sei.cmu.edu/confluence/display/c/ARR30-C.+Do+not+form+or+use+out-of-bounds+pointers+or+array+subscripts) | | ✓ | ✓ | ✓ | |