search

DUNE-DAQ / readoutlibs

readoutlibs
0 stars 2 forks source link

Compiler warnings #61

Open bieryAtFnal opened 2 years ago

bieryAtFnal commented 2 years ago

[34/126] Building CXX object readoutlibs/CMakeFiles/readoutlibs_test_composite_key.dir/test/apps/test_composite_key_app.cxx.o In file included from /cvmfs/dunedaq.opensciencegrid.org/spack/externals/ext-v1.0/spack-0.18.1-gcc-12.1.0/spack-0.18.1/opt/spack/gcc-12.1.0/folly-2021.12.13.00-acobv4yxnw7pfvnjabizdyahmmb7fmd4/include/folly/ConcurrentSkipList.h:130, from /home/nfs/dunedaq/daqsw/25SepDevTest/sourcecode/readoutlibs/test/apps/test_composite_key_app.cxx:15: In member function 'size_t folly::detail::SkipListRandomHeight::getSizeLimit(int) const', inlined from 'std::pair<folly::detail::SkipListNode, long unsigned int> folly::ConcurrentSkipList<T, Comp, NodeAlloc, MAX_HEIGHT>::addOrGetData(U&&) [with U = dunedaq::readoutlibs::types::DUMMY_FRAME_STRUCT; T = dunedaq::readoutlibs::types::DUMMY_FRAME_STRUCT; Comp = std::less; NodeAlloc = folly::SysAllocator; int MAX_HEIGHT = 24]' at /cvmfs/dunedaq.opensciencegrid.org/spack/externals/ext-v1.0/spack-0.18.1-gcc-12.1.0/spack-0.18.1/opt/spack/gcc-12.1.0/folly-2021.12.13.00-acobv4yxnw7pfvnjabizdyahmmb7fmd4/include/folly/ConcurrentSkipList.h:352:63: /cvmfs/dunedaq.opensciencegrid.org/spack/externals/ext-v1.0/spack-0.18.1-gcc-12.1.0/spack-0.18.1/opt/spack/gcc-12.1.0/folly-2021.12.13.00-acobv4yxnw7pfvnjabizdyahmmb7fmd4/include/folly/ConcurrentSkipList-inl.h:199:34: warning: array subscript 64 is above array bounds of 'const sizet [64]' {aka 'const long unsigned int [64]'} [-Warray-bounds] 199 | return sizeLimitTable[height]; | ~~~~~~^ /cvmfs/dunedaq.opensciencegrid.org/spack/externals/ext-v1.0/spack-0.18.1-gcc-12.1.0/spack-0.18.1/opt/spack/gcc-12.1.0/folly-2021.12.13.00-acobv4yxnw7pfvnjabizdyahmmb7fmd4/include/folly/ConcurrentSkipList-inl.h: In member function 'std::pair<folly::detail::SkipListNode, long unsigned int> folly::ConcurrentSkipList<T, Comp, NodeAlloc, MAX_HEIGHT>::addOrGetData(U&&) [with U = dunedaq::readoutlibs::types::DUMMY_FRAME_STRUCT; T = dunedaq::readoutlibs::types::DUMMY_FRAME_STRUCT; Comp = std::less; NodeAlloc = folly::SysAllocator; int MAXHEIGHT = 24]': /cvmfs/dunedaq.opensciencegrid.org/spack/externals/ext-v1.0/spack-0.18.1-gcc-12.1.0/spack-0.18.1/opt/spack/gcc-12.1.0/folly-2021.12.13.00-acobv4yxnw7pfvnjabizdyahmmb7fmd4/include/folly/ConcurrentSkipList-inl.h:232:10: note: while referencing 'folly::detail::SkipListRandomHeight::sizeLimitTable' 232 | sizet sizeLimitTable[kMaxHeight]; | ^~~~~~~ [34/126] Building CXX object hsilibs/CMakeFiles/hsilibs_HSIController_duneDAQModule.dir/plugins/HSIController.cpp.o [35/126] Building CXX object readoutlibs/CMakeFiles/readoutlibs_test_lb_allocation.dir/test/apps/test_lb_allocation_app.cxx.o [35/126] Generating codegen/include/dfmodules/datawriter/Structs.hpp [36/126] Building CXX object readoutlibs/CMakeFiles/readoutlibs_test_skiplist.dir/test/apps/test_skiplist_app.cxx.o In file included from /cvmfs/dunedaq.opensciencegrid.org/spack/externals/ext-v1.0/spack-0.18.1-gcc-12.1.0/spack-0.18.1/opt/spack/gcc-12.1.0/folly-2021.12.13.00-acobv4yxnw7pfvnjabizdyahmmb7fmd4/include/folly/ConcurrentSkipList.h:130, from /home/nfs/dunedaq/daqsw/25SepDevTest/sourcecode/readoutlibs/test/apps/test_skiplist_app.cxx:15: In member function 'size_t folly::detail::SkipListRandomHeight::getSizeLimit(int) const', inlined from 'std::pair<folly::detail::SkipListNode, long unsigned int> folly::ConcurrentSkipList<T, Comp, NodeAlloc, MAX_HEIGHT>::addOrGetData(U&&) [with U = dunedaq::readoutlibs::types::DUMMY_FRAME_STRUCT; T = dunedaq::readoutlibs::types::DUMMY_FRAME_STRUCT; Comp = std::less; NodeAlloc = folly::SysAllocator; int MAX_HEIGHT = 24]' at /cvmfs/dunedaq.opensciencegrid.org/spack/externals/ext-v1.0/spack-0.18.1-gcc-12.1.0/spack-0.18.1/opt/spack/gcc-12.1.0/folly-2021.12.13.00-acobv4yxnw7pfvnjabizdyahmmb7fmd4/include/folly/ConcurrentSkipList.h:352:63: /cvmfs/dunedaq.opensciencegrid.org/spack/externals/ext-v1.0/spack-0.18.1-gcc-12.1.0/spack-0.18.1/opt/spack/gcc-12.1.0/folly-2021.12.13.00-acobv4yxnw7pfvnjabizdyahmmb7fmd4/include/folly/ConcurrentSkipList-inl.h:199:34: warning: array subscript 64 is above array bounds of 'const sizet [64]' {aka 'const long unsigned int [64]'} [-Warray-bounds] 199 | return sizeLimitTable[height]; | ~~~~~~^ /cvmfs/dunedaq.opensciencegrid.org/spack/externals/ext-v1.0/spack-0.18.1-gcc-12.1.0/spack-0.18.1/opt/spack/gcc-12.1.0/folly-2021.12.13.00-acobv4yxnw7pfvnjabizdyahmmb7fmd4/include/folly/ConcurrentSkipList-inl.h: In member function 'std::pair<folly::detail::SkipListNode, long unsigned int> folly::ConcurrentSkipList<T, Comp, NodeAlloc, MAX_HEIGHT>::addOrGetData(U&&) [with U = dunedaq::readoutlibs::types::DUMMY_FRAME_STRUCT; T = dunedaq::readoutlibs::types::DUMMY_FRAME_STRUCT; Comp = std::less; NodeAlloc = folly::SysAllocator; int MAXHEIGHT = 24]': /cvmfs/dunedaq.opensciencegrid.org/spack/externals/ext-v1.0/spack-0.18.1-gcc-12.1.0/spack-0.18.1/opt/spack/gcc-12.1.0/folly-2021.12.13.00-acobv4yxnw7pfvnjabizdyahmmb7fmd4/include/folly/ConcurrentSkipList-inl.h:232:10: note: while referencing 'folly::detail::SkipListRandomHeight::sizeLimitTable' 232 | sizet sizeLimitTable[kMaxHeight]; | ^~~~~~~

bieryAtFnal commented 2 years ago

I'm not sure if these are also relevant for the readoutlibs package. Maybe there should instead be reported against hsilibs?

[108/126] Building CXX object hsilibs/CMakeFiles/hsilibs_HSIDataLinkHandler_duneDAQModule.dir/plugins/HSIDataLinkHandler.cpp.o In file included from /home/nfs/dunedaq/daqsw/25SepDevTest/sourcecode/readoutlibs/include/readoutlibs/models/IterableQueueModel.hpp:335, from /home/nfs/dunedaq/daqsw/25SepDevTest/sourcecode/readoutlibs/include/readoutlibs/models/BinarySearchQueueModel.hpp:16, from /home/nfs/dunedaq/daqsw/25SepDevTest/sourcecode/hsilibs/plugins/HSIDataLinkHandler.cpp:16: /home/nfs/dunedaq/daqsw/25SepDevTest/sourcecode/readoutlibs/include/readoutlibs/models/detail/IterableQueueModel.hxx: In instantiation of 'void dunedaq::readoutlibs::IterableQueueModel::allocate_memory(std::size_t, bool, uint8_t, bool, std::size_t) [with T = dunedaq::hsilibs::TIMING_HSI_FRAME_STRUCT; std::size_t = long unsigned int; uint8_t = unsigned char]': /home/nfs/dunedaq/daqsw/25SepDevTest/sourcecode/readoutlibs/include/readoutlibs/models/detail/IterableQueueModel.hxx:228:3: required from 'void dunedaq::readoutlibs::IterableQueueModel::conf(const nlohmann::json&) [with T = dunedaq::hsilibs::TIMING_HSI_FRAME_STRUCT; nlohmann::json = nlohmann::basic_json<>]' /home/nfs/dunedaq/daqsw/25SepDevTest/sourcecode/readoutlibs/include/readoutlibs/models/detail/IterableQueueModel.hxx:222:1: required from here /home/nfs/dunedaq/daqsw/25SepDevTest/sourcecode/readoutlibs/include/readoutlibs/models/detail/IterableQueueModel.hxx:54:38: warning:comparison is always true due to limited range of data type [-Wtype-limits] 54 | } else if (numa_aware && numa_node >= 0 && numanode < 8) { // numa allocator from libnuma | ~~^~~~ In member function 'bool dunedaq::readoutlibs::IterableQueueModel::write(Args&& ...) [with Args = {dunedaq::hsilibs::TIMING_HSI_FRAME_STRUCT}; T = dunedaq::hsilibs::TIMING_HSI_FRAME_STRUCT]', inlined from 'void dunedaq::readoutlibs::IterableQueueModel::conf(const nlohmann::json&) [with T = dunedaq::hsilibs::TIMING_HSI_FRAMESTRUCT]' at /home/nfs/dunedaq/daqsw/25SepDevTest/sourcecode/readoutlibs/include/readoutlibs/models/detail/IterableQueueModel.hxx:243:13: /home/nfs/dunedaq/daqsw/25SepDevTest/sourcecode/readoutlibs/include/readoutlibs/models/detail/IterableQueueModel.hxx:287:5: warning:'element' may be used uninitialized [-Wmaybe-uninitialized] 287 | new (&records[currentWrite]) T(std::forward(recordArgs)...); | ^~~~~~~~~~~~~~ /home/nfs/dunedaq/daqsw/25SepDevTest/sourcecode/readoutlibs/include/readoutlibs/models/detail/IterableQueueModel.hxx: In member function 'void dunedaq::readoutlibs::IterableQueueModel::conf(const nlohmann::json&) [with T = dunedaq::hsilibs::TIMING_HSI_FRAME_STRUCT]': /home/nfs/dunedaq/daqsw/25SepDevTest/sourcecode/readoutlibs/include/readoutlibs/models/detail/IterableQueueModel.hxx:242:9: note: 'element' declared here 242 | T element; | ^~~

jcfreeman2 commented 2 years ago

Concerning the first post, in which there's a warning about folly's ConcurrentSkipList class:

gcc 12.1.0 seems to be warning merely of the possibility of an out-of-bounds array error, rather than identifying a concrete violation. Even creating a one-liner test program where the body of main() looks like "std::shared_ptr<SkipListT> skl(SkipListT::createInstance(2));" and where the application immediately returns after, the warning still appears. This is basically a heavily-truncated version of test_composite_key_app.cxx. It looks like to this day the classic C-style array is still used for ConcurrentSkipList in folly, so upgrading's not going to quiet this warning; we may want to consider doing so manually using #pragmas, as this may actually be an appropriate use of them.

Concerning the second post, discussing readoutlibs-based code causing a warning when hsilibs is built:

For starters, it's of course "interesting" that the warnings appear when hsilibs is built, but not when the actual source of the warning, readoutlibs, is built. The reason for this is that there isn't any place in the readoutlibs codebase where an IterableQueueModel instance is actually created. If, say, a unit test for IterableQueueModel were written, then the warning would appear when readoutlibs was built and not discovered when developing other packages. In the case of hsilibs, we do get a warning since a derived class of IterableQueueModel gets instantiated in a plugin.

As far as the warnings themselves: the first one will go away if we make numa_node an int rather than a uint8_t, which has the added advantage of adhering more closely to our coding guideline "Don't declare a variable unsigned unless it's really needed, like with a bit pattern". This will likely also require us to make the change in the corresponding jsonnet file. The second one will go away with a change as simple as replacing T element; with T element = T(); since it seems gcc 12.1.0 wants to be sure the default construction is a conscious decision on the part of the developer.