trilinos / Trilinos

Primary repository for the Trilinos Project
https://trilinos.org/
Other
1.19k stars 565 forks source link

Isorropia example error: format not a string literal and no format arguments #830

Closed sagitter closed 3 years ago

sagitter commented 7 years ago

Hi all.

Isorropia "order_example" does not compile with following error:

/builddir/build/BUILD/trilinos-12.8.1/buildopenmpi_dir/packages/isorropia/example/order_example.cpp: In function 'int main(int, char**)':
/builddir/build/BUILD/trilinos-12.8.1/buildopenmpi_dir/packages/isorropia/example/order_example.cpp:116:42: error: format not a string literal and no format arguments [-Werror=format-security]
     sprintf(file_name, MMFileName.c_str());
                                          ^

cmake configuration (MPI enabled):

+ /usr/bin/cmake -DCMAKE_C_FLAGS_RELEASE:STRING=-DNDEBUG -DCMAKE_CXX_FLAGS_RELEASE:STRING=-DNDEBUG -DCMAKE_Fortran_FLAGS_RELEASE:STRING=-DNDEBUG -DCMAKE_VERBOSE_MAKEFILE:BOOL=ON -DCMAKE_INSTALL_PREFIX:PATH=/usr -DINCLUDE_INSTALL_DIR:PATH=/usr/include -DLIB_INSTALL_DIR:PATH=/usr/lib64 -DSYSCONF_INSTALL_DIR:PATH=/etc -DSHARE_INSTALL_PREFIX:PATH=/usr/share -DLIB_SUFFIX=64 -DBUILD_SHARED_LIBS:BOOL=ON -DCMAKE_BUILD_TYPE:STRING=Release '-DCMAKE_C_FLAGS_RELEASE:STRING=-O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mtune=generic -Wl,-z,relro -fPIC -Wl,-z,now' '-DCMAKE_CXX_FLAGS_RELEASE:STRING=-O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mtune=generic -Wl,-z,relro -fPIC -Wl,-z,now' '-DCMAKE_Fortran_FLAGS_RELEASE:STRING=-O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -m64 -mtune=generic -Wl,-z,relro -fPIC -Wl,-z,now' '-DCMAKE_EXE_LINKER_FLAGS:STRING=-Wl,-z,relro -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -fPIC -pie -Wl,-z,now -Wl,--as-needed' -DCMAKE_INSTALL_PREFIX=/usr -DCMAKE_SKIP_RPATH:BOOL=YES -DCMAKE_SKIP_INSTALL_RPATH:BOOL=YES -DBUILD_SHARED_LIBS:BOOL=ON -DTPL_ENABLE_MPI=ON -DMPI_BASE_DIR= -DTrilinos_ENABLE_FORTRAN=ON -DTrilinos_ENABLE_TESTS:BOOL=ON -DTrilinos_ENABLE_OpenMP=ON -DTrilinos_ENABLE_PyTrilinos:BOOL=ON -DTPL_ENABLE_MPI4PY:BOOL=ON -DTrilinos_ENABLE_ALL_OPTIONAL_PACKAGES:BOOL=ON -DPyTrilinos_INSTALL_PREFIX:PATH=/usr/lib64 -DPYTHON_VERSION:STRING=2.7 -DTpetra_INST_SERIAL:BOOL=OFF -DCMAKE_VERBOSE_MAKEFILE=TRUE -DTrilinos_VERBOSE_CONFIGURE=OFF -DTrilinos_ENABLE_CXX11=ON -DTrilinos_INSTALL_INCLUDE_DIR=/usr/include/openmpi-x86_64/trilinos -DTrilinos_INSTALL_LIB_DIR=/usr/lib64/openmpi/lib -DTrilinos_INSTALL_RUNTIME_DIR=/usr/lib64/openmpi/bin -DTrilinos_INSTALL_EXAMPLE_DIR=/usr/share/trilinos/examples -DTrilinos_ENABLE_STRONG_C_COMPILE_WARNINGS=OFF -DTrilinos_ENABLE_STRONG_CXX_COMPILE_WARNINGS=OFF -DTrilinos_ENABLE_EXPLICIT_INSTANTIATION:BOOL=ON -DTPL_ENABLE_gtest:BOOL=OFF -DTrilinos_ENABLE_STK:BOOL=OFF -DTrilinos_ENABLE_ALL_PACKAGES=ON ..

mhoemmen commented 7 years ago

Alas, we tend not to use -Werror=format-security by default, though we do build with -Wall by default. (We treat most warnings as bugs.) We would have to look at the code more carefully to see if this is actually a security risk in practice. Are you required to build with this flag?

sagitter commented 7 years ago

Are you required to build with this flag?

Yes, since long time.

sagitter commented 7 years ago

Is there an option to exclude Isorropia's test?

mhoemmen commented 7 years ago

@sagitter wrote:

Yes, since long time.

Scientific and engineering computing have a few instances of inherently unsafe sparse matrix storage formats. Harwell-Boeing format comes immediately to mind. The file itself stores the Fortran FORMAT string. When I wrote a Harwell-Boeing reader in Lisp, whose FORMAT function is not much different than Fortran's, I wrote a parser for the storage format that took a "white-list" approach, by only accepting sensible and safe formats. If I were to write the same "white-list" code in C or C++, the compiler might still issue the above warning, even though the code would exclude unsafe format strings. Thus, you may encounter similar troubles with other scientific and engineering libraries. I'm not saying that's good ;-) I'm just saying that scientific and engineering codes in general may not spend so much effort on sanitizing input.

Is there an option to exclude Isorropia's test?

If this is an example and not a test, it should suffice just to disable examples:

-D Isorropia_ENABLE_EXAMPLES:BOOL=OFF

However, if you need to disable Isorropia's tests too, this is how you do it:

-D Isorropia_ENABLE_TESTS:BOOL=OFF

In general, you can do this for any Trilinos package by replacing Isorropia with the package (or subpackage) name.

sagitter commented 7 years ago

Scientific and engineering computing have a few instances of inherently unsafe sparse matrix storage formats. Harwell-Boeing format comes immediately to mind. The file itself stores the Fortran FORMAT string. When I wrote a Harwell-Boeing reader in Lisp, whose FORMAT function is not much different than Fortran's, I wrote a parser for the storage format that took a "white-list" approach, by only accepting sensible and safe formats. If I were to write the same "white-list" code in C or C++, the compiler might still issue the above warning, even though the code would exclude unsafe format strings. Thus, you may encounter similar troubles with other scientific and engineering libraries. I'm not saying that's good ;-) I'm just saying that scientific and engineering codes in general may not spend so much effort on sanitizing input.

I realize that the same security rules sometimes cannot be imposed on every code, in these cases it's hard set different compiler flags depending on strings used. In this specific case it's just an example file, so i guess that shorter way is skip the issue.

mhoemmen commented 7 years ago

@sagitter wrote:

In this specific case it's just an example file, so i guess that shorter way is skip the issue.

I agree. Nevertheless, please feel welcome to file more issues if you get warnings like this in other packages :-)

sagitter commented 7 years ago

Is there an option to exclude Isorropia's test?

If this is an example and not a test, it should suffice just to disable examples:

-D Isorropia_ENABLE_EXAMPLES:BOOL=OFF

However, if you need to disable Isorropia's tests too, this is how you do it:

-D Isorropia_ENABLE_TESTS:BOOL=OFF

In general, you can do this for any Trilinos package by replacing Isorropia with the package (or subpackage) name.

Example files are not used for testing? That is, can i avoid to compile examples and perform tests anyway?

mhoemmen commented 7 years ago

Example files are not used for testing? That is, can i avoid to compile examples and perform tests anyway?

The answer to both questions is "yes." "Tests" are tests; they verify correctness of code. "Examples" are for users and meant to be read, as well as built and executed.

When we test a package, we normally enable both tests and examples. This ensures that examples are actually correct (that they build and run to completion). In some cases, examples do some correctness tests, but this is not meant to replace the actual tests. In order to test a package, it should suffice to build and run the tests, without the examples.

sagitter commented 7 years ago

Compiling all examples extends a lot the build time; maybe distinguishing the builds of examples and tests can be useful.

mhoemmen commented 7 years ago

Compiling all examples extends a lot the build time; maybe distinguishing the builds of examples and tests can be useful.

We do. You can disable one or the other. -D ${PACKAGE_NAME}_ENABLE_EXAMPLES:BOOL=OFF disables building examples in package ${PACKAGE_NAME}; -D ${PACKAGE_NAME}_ENABLE_TESTS:BOOL=OFF disables building tests in package ${PACKAGE_NAME}. Tests normally take me longer to build than examples, so it surprises me to hear that.

sagitter commented 7 years ago

We do. You can disable one or the other. -D ${PACKAGE_NAME}_ENABLE_EXAMPLES:BOOL=OFF disables building examples in package ${PACKAGE_NAME}

All examples at once i meant.

sagitter commented 7 years ago

I've just discovered the options

-DTrilinos_ENABLE_EXAMPLES:BOOL=OFF
 -DForTrilinos_ENABLE_EXAMPLES:BOOL=OFF
 -DCTrilinos_ENABLE_EXAMPLES:BOOL=OFF
 -DPyTrilinos_ENABLE_EXAMPLES:BOOL=OFF
mhoemmen commented 7 years ago

All examples at once i meant.

Ah, I see. -D Trilinos_ENABLE_EXAMPLES:BOOL=OFF should suffice. If it doesn't suffice, that's a bug :-)

github-actions[bot] commented 3 years ago

This issue has had no activity for 365 days and is marked for closure. It will be closed after an additional 30 days of inactivity. If you would like to keep this issue open please add a comment and remove the MARKED_FOR_CLOSURE label. If this issue should be kept open even with no activity beyond the time limits you can add the label DO_NOT_AUTOCLOSE.

github-actions[bot] commented 3 years ago

This issue was closed due to inactivity for 395 days.