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

Coverity Scan #3361

Merged
merged 4 commits into from
Jun 12, 2023
Merged

Coverity Scan #3361

merged 4 commits into from
Jun 12, 2023

Conversation

WeiqunZhang
Copy link
Member

@WeiqunZhang WeiqunZhang commented Jun 10, 2023

Summary

Fix issues found by Coverity Scan, including infinite loop bugs in the kernels of MLALaplacian.

The scan will be done nightly on a local workstation.

Add coverity scan and license badges to README.md. Remove the cmake README.md because it's broken.

Checklist

The proposed changes:

  • fix a bug or incorrect behavior in AMReX
  • add new capabilities to AMReX
  • changes answers in the test suite to more than roundoff level
  • are likely to significantly affect the results of downstream AMReX users
  • include documentation in the code and/or rst files, if appropriate

@WeiqunZhang WeiqunZhang force-pushed the fix_coverity branch 3 times, most recently from 6f80b90 to 06a1527 Compare June 11, 2023 00:22
Src/Base/AMReX_VisMF.cpp Fixed Show fixed Hide fixed
@WeiqunZhang WeiqunZhang force-pushed the fix_coverity branch 7 times, most recently from 67dcc13 to 98631cd Compare June 11, 2023 05:55
Fix issues found by Coverity Scan, including infinite loop bugs in the
kernels of MLALaplacian.

The scan will be done nightly on a local workstation.

Add a badge to README.md.

Remove the cmake README.md because it's broken.
@WeiqunZhang WeiqunZhang marked this pull request as ready for review June 11, 2023 18:35
@WeiqunZhang WeiqunZhang requested a review from atmyers June 11, 2023 18:35
@ax3l ax3l self-requested a review June 12, 2023 20:40
@ax3l ax3l added the bug label Jun 12, 2023
Copy link
Member

@ax3l ax3l left a comment

Choose a reason for hiding this comment

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

Looks great 👍

Comment on lines +994 to +996
ifeq ($(USE_COVERITY),TRUE)
DEFINES += -DAMREX_USE_COVERITY
endif
Copy link
Member

Choose a reason for hiding this comment

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

We can add this also to CMake, but if your build logic on Coverity is GNUmake only then this is fine, too.

@ax3l
Copy link
Member

ax3l commented Jun 12, 2023

@WeiqunZhang just for sake of documentation, where did you implement the nightly sync-to-the-coverity-branch logic?

@ax3l ax3l self-assigned this Jun 12, 2023
@ax3l ax3l merged commit 0375e78 into AMReX-Codes:development Jun 12, 2023
64 checks passed
@WeiqunZhang WeiqunZhang deleted the fix_coverity branch June 12, 2023 23:23
@WeiqunZhang
Copy link
Member Author

I have a cron job running on this. We can only run 3 scans per day on average.

export PATH=$HOME/opt/cov-analysis/bin:$PATH

cd $HOME/coverity/amrex
git pull

rm -rf cov-int amrex.tgz

make distclean

./configure --with-mpi yes \
            --with-omp no \
            --with-fortran no \
            --enable-fortran-api no \
            --enable-eb yes \
            --enable-linear-solver yes \
            --enable-particle yes \
            --enable-tiny-profile yes

cov-build --dir cov-int make -j4 CXX=g++ USE_ASSERTION=TRUE USE_COVERITY=TRUE

tar cvfz amrex.tgz cov-int

AMREX_VERSION=$(git describe --abbrev=12 --dirty --always --tags)

curl --form token=${TOKEN} \
  --form email=weiqunzhang@lbl.gov \
  --form file=@amrex.tgz \
  --form version="AMReX-${AMREX_VERSION}" \
  --form description="AMReX" \
  https://scan.coverity.com/builds?project=AMReX-Codes%2Famrex

@ax3l
Copy link
Member

ax3l commented Jun 13, 2023

Ah I see, got it.

In the past, I merged to a branch, which kicked off the upload via a plugin. Curling works as well.

If you like, you can also define cron-jobs in GH actions instead of locally: https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#schedule

@WeiqunZhang
Copy link
Member Author

Good suggestion. We could do it.

@WeiqunZhang
Copy link
Member Author

But then we have to download the coverity tools everytime.

@WeiqunZhang
Copy link
Member Author

The tarball is quite big. More than 1 GB.

@ax3l
Copy link
Member

ax3l commented Jun 13, 2023

Oh, I see. Yeah, it's on the size of the CUDA toolkit or oneAPI then 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants