GEOS-DEV / GEOS

GEOS Simulation Framework
GNU Lesser General Public License v2.1
207 stars 83 forks source link

potential incorrect application of `NDEBUG` #2363

Closed rrsettgast closed 9 months ago

rrsettgast commented 1 year ago

Describe the bug I tried to build on Mac, and got the following error:

/Users/settgast1/Codes/geos/GEOS_develop/src/coreComponents/mesh/generators/CellBlockManager.cpp:309:43: error: no member named 'numNodes' in 'geosx::NodesAndElementOfFace'
      GEOSX_ASSERT_EQ( numNodesInFace, f0.numNodes );

This error makes sense, as geosx::NodesAndElementOfFace in fact does not have a member named numNodes.

The question is why is this passing everywhere else besides Mac? Digging a little deeper, the GEOSX_ASSERT_EQ macro ends up calling:

#if !defined(NDEBUG)
#define LVARRAY_ASSERT_MSG( EXP, MSG ) LVARRAY_ERROR_IF( !(EXP), MSG )
#else
#define LVARRAY_ASSERT_MSG( EXP, MSG ) ((void) 0)
#endif

So if NDEBUG is defined, then the ASSERT is a no-op.

So looking at the Mac compile line:

cd /Users/settgast1/Codes/geos/GEOS_develop/build-darwin-clang-debug/coreComponents/mesh && /usr/bin/clang++ -DFMT_LOCALE -I/Users/settgast1/Codes/geos/GEOS_develop/build-darwin-clang-debug/include -I/Users/settgast1/Codes/geos/thirdPartyLibs/install-darwin-clang-release/pugixml/include -I/Users/settgast1/Codes/geos/thirdPartyLibs/install-darwin-clang-release/conduit/include -I/Users/settgast1/Codes/geos/thirdPartyLibs/install-darwin-clang-release/conduit/include/conduit -I/Users/settgast1/Codes/geos/thirdPartyLibs/install-darwin-clang-release/fmt/include -I/Users/settgast1/Codes/geos/GEOS_develop/src/coreComponents -I/Users/settgast1/Codes/geos/GEOS_develop/src/coreComponents/constitutive/PVTPackage/PVTPackage/source -isystem /Users/settgast1/Codes/geos/thirdPartyLibs/install-darwin-clang-release/raja/include -isystem /Users/settgast1/Codes/geos/thirdPartyLibs/install-darwin-clang-release/chai/include -isystem /usr/local/Cellar/open-mpi/4.1.5/include -isystem /Users/settgast1/Codes/geos/GEOS_develop/src/thirdparty/fast_float/include -isystem /Users/settgast1/Codes/geos/thirdPartyLibs/install-darwin-clang-release/mathpresso/include -isystem /Users/settgast1/Codes/geos/thirdPartyLibs/install-darwin-clang-release/parmetis/include -isystem /Users/settgast1/Codes/geos/thirdPartyLibs/install-darwin-clang-release/metis/include -isystem /Users/settgast1/Codes/geos/thirdPartyLibs/install-darwin-clang-release/vtk/include/vtk-9.1 -isystem /Users/settgast1/Codes/geos/thirdPartyLibs/install-darwin-clang-release/scotch/include -Wall -Wextra       -Werror  -Wall -Wextra -Wpedantic -pedantic-errors -Wshadow -Wfloat-equal -Wno-cast-align -Wcast-qual  -g -Wno-unused-parameter -Wno-unused-variable -fstandalone-debug  -std=c++14 -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk -fPIC -MD -MT coreComponents/mesh/CMakeFiles/mesh.dir/generators/CellBlockManager.cpp.o -MF CMakeFiles/mesh.dir/generators/CellBlockManager.cpp.o.d -o CMakeFiles/mesh.dir/generators/CellBlockManager.cpp.o -c /Users/settgast1/Codes/geos/GEOS_develop/src/coreComponents/mesh/generators/CellBlockManager.cpp

Note there is no NDEBUG

Looking at the compile line for clang-debug on quartz:

cd /usr/WS2/settgast/Codes/geosx/GEOSX_develop/build-quartz-clang@10.0.0-debug/coreComponents/mesh && /usr/tce/packages/clang/clang-10.0.0/bin/clang++  -DDIY_NO_THREADS -DFMT_LOCALE -DMPICH_SKIP_MPICXX -DMPI_NO_CPPBIND -DNDEBUG -DOMPI_SKIP_MPICXX -D_MPICC_H -Dkiss_fft_scalar=double -I/usr/WS2/settgast/Codes/geosx/GEOSX_develop/src/coreComponents -I/usr/WS2/settgast/Codes/geosx/GEOSX_develop/build-quartz-clang@10.0.0-debug/include -I/usr/WS2/settgast/Codes/geosx/GEOSX_develop/src/coreComponents/denseLinearAlgebra -I/usr/WS2/settgast/Codes/geosx/GEOSX_develop/src/coreComponents/constitutive/PVTPackage/PVTPackage/source -isystem /usr/gapps/GEOSX/thirdPartyLibs/2023-03-15/install-quartz-clang@10.0.0-release/raja/include -isystem /usr/gapps/GEOSX/thirdPartyLibs/2023-03-15/install-quartz-clang@10.0.0-release/chai/include -isystem /usr/WS1/GEOS/GEOSX/TPLs_2023-03-15/install-quartz-clang@10.0.0-release/chai/include -isystem /usr/WS1/GEOS/GEOSX/TPLs_2023-03-15/install-quartz-clang@10.0.0-release/caliper/include -isystem /usr/gapps/GEOSX/thirdPartyLibs/2023-03-15/install-quartz-clang@10.0.0-release/pugixml/include -isystem /usr/gapps/GEOSX/thirdPartyLibs/2023-03-15/install-quartz-clang@10.0.0-release/conduit/include -isystem /usr/gapps/GEOSX/thirdPartyLibs/2023-03-15/install-quartz-clang@10.0.0-release/conduit/include/conduit -isystem /usr/WS1/GEOS/GEOSX/TPLs_2023-03-15/install-quartz-clang@10.0.0-release/hdf5/include -isystem /usr/gapps/GEOSX/thirdPartyLibs/2023-03-15/install-quartz-clang@10.0.0-release/fmt/include -isystem /usr/tce/packages/mkl/mkl-2019.0/include -isystem /usr/tce/packages/mvapich2/mvapich2-2.3-gcc-4.9.3/include -isystem /usr/gapps/GEOSX/thirdPartyLibs/2023-03-15/install-quartz-clang@10.0.0-release/adiak/include -isystem /usr/WS2/settgast/Codes/geosx/GEOSX_develop/src/thirdparty/fast_float/include -isystem /usr/gapps/GEOSX/thirdPartyLibs/2023-03-15/install-quartz-clang@10.0.0-release/mathpresso/include -isystem /usr/gapps/GEOSX/thirdPartyLibs/2023-03-15/install-quartz-clang@10.0.0-release/parmetis/include -isystem /usr/gapps/GEOSX/thirdPartyLibs/2023-03-15/install-quartz-clang@10.0.0-release/metis/include -isystem /usr/gapps/GEOSX/thirdPartyLibs/2023-03-15/install-quartz-clang@10.0.0-release/vtk/include/vtk-9.1 -isystem /usr/gapps/GEOSX/thirdPartyLibs/2023-03-15/install-quartz-clang@10.0.0-release/scotch/include  -Wall -Wextra  --gcc-toolchain=/usr/tce/packages/gcc/gcc-8.1.0     -Werror  -fopenmp=libomp  -Wall -Wextra -Wpedantic -pedantic-errors -Wshadow -Wfloat-equal -Wno-cast-align -Wcast-qual  -g -Wno-unused-parameter -Wno-unused-variable -fstandalone-debug  -fPIC   -fopenmp=libomp -pthread -std=c++14 -o CMakeFiles/mesh.dir/generators/CellBlockManager.cpp.o -c /usr/WS2/settgast/Codes/geosx/GEOSX_develop/src/coreComponents/mesh/generators/CellBlockManager.cpp

There is a -DNDEBUG in there.

So this is incorrect behavior. This is a debug run, so -DNDEBUG should not be on the compile line.

untereiner commented 1 year ago

What is the host config you are using ? Are the CMAKE_CXX_FLAGS_RELEASE overridden ?

TotoGaz commented 9 months ago

@tbeltzun Would your PR close this issue?

t-bltg commented 9 months ago

https://github.com/GEOS-DEV/thirdPartyLibs/pull/248 in conjunction with https://github.com/GEOS-DEV/GEOS/pull/2721 would close it, yes.

TotoGaz commented 9 months ago

OK thanks! I've linked the PR and assigned this issue to your so you get the credit 😉