Skip to content

Add PWR080: Conditionally initialized variables can lead to undefined behavior - #103

Merged
alvrogd merged 2 commits into
mainfrom
feature/AddPWR080
Oct 23, 2025
Merged

Add PWR080: Conditionally initialized variables can lead to undefined behavior#103
alvrogd merged 2 commits into
mainfrom
feature/AddPWR080

Conversation

@alvrogd

@alvrogd alvrogd commented Oct 22, 2025

Copy link
Copy Markdown
Contributor

Add entry and benchmark for a new check. My results:

$ lscpu
  Model name:             13th Gen Intel(R) Core(TM) i7-13700H

$ gcc --version                            
gcc (Ubuntu 13.3.0-6ubuntu2~24.04) 13.3.0

$ gfortran --version                                                 
GNU Fortran (Ubuntu 13.3.0-6ubuntu2~24.04) 13.3.0

$ ./run-benchmarks.py --check PWR080
------------------------------------------------------------------
Benchmark                        Time             CPU   Iterations
------------------------------------------------------------------
PWR080 C Example               560 us          560 us         1235
PWR080 C Improved              559 us          559 us         1244
PWR080 Fortran Example         558 us          558 us         1254
PWR080 Fortran Improved        561 us          561 us         1242

@alvrogd alvrogd self-assigned this Oct 22, 2025
@alvrogd
alvrogd force-pushed the feature/AddPWR080 branch 2 times, most recently from 1ecf3d3 to 4760960 Compare October 22, 2025 11:12
@alvrogd
alvrogd marked this pull request as ready for review October 22, 2025 11:15

@daniel-otero daniel-otero left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM.

Comment thread Checks/PWR080/README.md Outdated
Comment thread Checks/PWR080/README.md Outdated
Comment thread Checks/PWR080/README.md Outdated
Comment thread Checks/PWR080/README.md
Comment thread Checks/PWR080/README.md Outdated
Comment on lines +4 to +30
pure function transform_and_sum_improved_f(n, array, option) bind(c)
use iso_c_binding, only: c_double, c_int
implicit none

integer(kind=c_int), intent(in), value :: n
real(kind=c_double), dimension(n), intent(in) :: array
integer(kind=c_int), intent(in), value :: option
real(kind=c_double) :: transform_and_sum_improved_f

real(kind=c_double) :: sum
real(kind=c_double) :: factor
integer(kind=c_int) :: i

sum = 0.0

if (option == 1) then
factor = 1.0
else if (option == 2) then
factor = 2.0
else
! Nullifies the computation
factor = 0.0
end if

do i = 1, n
sum = sum + array(i) * factor
end do

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I disagree this is a good example solution: PWR080 should be about the unitialized variable factor alone, it should not bother with another variable like option and worry if all it's possible values are properly checked, that is foreign to the real issue.

The simplest solution (and the one that would allow linting tools to produce autofixes for PWR080) would be to just initialize factor in it's declaration to avoid potential undefined behavior. Whether the value chosen is "correct" or not, is also besides the point, which is to turn undefined behavior into consistently defined behavior.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I disagree this is a good example solution: PWR080 should be about the unitialized variable factor alone, it should not bother with another variable like option and worry if all it's possible values are properly checked, that is foreign to the real issue.

That's a good point of view! I've updated the solutions to suggest the default initialization of factor.

@alvrogd
alvrogd force-pushed the feature/AddPWR080 branch 2 times, most recently from 282e3c2 to 764d64a Compare October 23, 2025 11:47
@alvrogd
alvrogd requested a review from ruifm October 23, 2025 12:04
@alvrogd
alvrogd merged commit 629d868 into main Oct 23, 2025
7 checks passed
@alvrogd
alvrogd deleted the feature/AddPWR080 branch October 23, 2025 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants