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

table.hpp:106 error: redefinition of ‘size_t* TablePrefixSum on 32-bit architectures #168

Open
drew-parsons opened this issue Nov 11, 2023 · 4 comments

Comments

@drew-parsons
Copy link

netgen 6.2.2305 fails to build on 32-bit architectures. Older versions previously built successfully.
Build logs can be found at https://buildd.debian.org/status/package.php?p=netgen&suite=experimental
(past builds can be found under the "old" links at https://buildd.debian.org/status/package.php?p=netgen)

The error message from i386 is
(https://buildd.debian.org/status/fetch.php?pkg=netgen&arch=i386&ver=6.2.2305%2Bdfsg1-1exp1&stamp=1699663029&raw=0)

[  6%] Building CXX object libsrc/core/CMakeFiles/ngcore.dir/taskmanager.cpp.o
cd /<<PKGBUILDDIR>>/obj-i686-linux-gnu/libsrc/core && /usr/bin/c++ -DFFMPEG -DHAVE_DLFCN_H -DHAVE_FREEIMAGE -DHAVE_FREETYPE -DHAVE_OPENGL_EXT -DHAVE_RAPIDJSON -DHAVE_TBB -DHAVE_TK -DHAVE_XLIB -DJPEGLIB -DNETGEN_PYTHON -DNGCORE_EXPORTS -DNG_PYTHON -DOCCGEOMETRY -DOCC_CONVERT_SIGNALS -DPARALLEL -DPYBIND11_SIMPLE_GIL_MANAGEMENT -D__STDC_CONSTANT_MACROS -Dngcore_EXPORTS -I/<<PKGBUILDDIR>>/obj-i686-linux-gnu/libsrc/core -I/<<PKGBUILDDIR>>/libsrc/core -I/<<PKGBUILDDIR>>/obj-i686-linux-gnu -I/<<PKGBUILDDIR>>/include -I/<<PKGBUILDDIR>>/libsrc -I/<<PKGBUILDDIR>>/libsrc/include -I/usr/include/opencascade -I/usr/lib/i386-linux-gnu/openmpi/include -I/usr/lib/i386-linux-gnu/openmpi/include/openmpi -I/usr/include/python3.11 -Wdate-time -D_FORTIFY_SOURCE=2 -g -O2 -ffile-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -O2 -g -DNDEBUG -std=gnu++17 -fPIC -fvisibility=hidden -MD -MT libsrc/core/CMakeFiles/ngcore.dir/taskmanager.cpp.o -MF CMakeFiles/ngcore.dir/taskmanager.cpp.o.d -o CMakeFiles/ngcore.dir/taskmanager.cpp.o -c /<<PKGBUILDDIR>>/libsrc/core/taskmanager.cpp
In file included from /<<PKGBUILDDIR>>/libsrc/core/table.cpp:11:
/<<PKGBUILDDIR>>/libsrc/core/table.hpp:106:26: error: redefinition of ‘size_t* ngcore::TablePrefixSum(FlatArray<unsigned int, unsigned int>)’
  106 |   NETGEN_INLINE size_t * TablePrefixSum (FlatArray<size_t> entrysize)
      |                          ^~~~~~~~~~~~~~
/<<PKGBUILDDIR>>/libsrc/core/table.hpp:100:26: note: ‘size_t* ngcore::TablePrefixSum(FlatArray<unsigned int, unsigned int>)’ previously defined here
  100 |   NETGEN_INLINE size_t * TablePrefixSum (FlatArray<unsigned int> entrysize)
      |                          ^~~~~~~~~~~~~~
make[3]: *** [libsrc/core/CMakeFiles/ngcore.dir/build.make:191: libsrc/core/CMakeFiles/ngcore.dir/table.cpp.o] Error 1
make[3]: *** Waiting for unfinished jobs....
In file included from /<<PKGBUILDDIR>>/libsrc/core/mpi_wrapper.hpp:12,
                 from /<<PKGBUILDDIR>>/libsrc/core/taskmanager.cpp:13:
/<<PKGBUILDDIR>>/libsrc/core/table.hpp:106:26: error: redefinition of ‘size_t* ngcore::TablePrefixSum(FlatArray<unsigned int, unsigned int>)’
  106 |   NETGEN_INLINE size_t * TablePrefixSum (FlatArray<size_t> entrysize)
      |                          ^~~~~~~~~~~~~~
/<<PKGBUILDDIR>>/libsrc/core/table.hpp:100:26: note: ‘size_t* ngcore::TablePrefixSum(FlatArray<unsigned int, unsigned int>)’ previously defined here
  100 |   NETGEN_INLINE size_t * TablePrefixSum (FlatArray<unsigned int> entrysize)
      |                          ^~~~~~~~~~~~~~
In file included from /<<PKGBUILDDIR>>/libsrc/core/mpi_wrapper.hpp:12,
                 from /<<PKGBUILDDIR>>/libsrc/core/paje_trace.cpp:11:
/<<PKGBUILDDIR>>/libsrc/core/table.hpp:106:26: error: redefinition of ‘size_t* ngcore::TablePrefixSum(FlatArray<unsigned int, unsigned int>)’
  106 |   NETGEN_INLINE size_t * TablePrefixSum (FlatArray<size_t> entrysize)
      |                          ^~~~~~~~~~~~~~
/<<PKGBUILDDIR>>/libsrc/core/table.hpp:100:26: note: ‘size_t* ngcore::TablePrefixSum(FlatArray<unsigned int, unsigned int>)’ previously defined here
  100 |   NETGEN_INLINE size_t * TablePrefixSum (FlatArray<unsigned int> entrysize)
      |                          ^~~~~~~~~~~~~~
/<<PKGBUILDDIR>>/libsrc/core/paje_trace.cpp: In member function ‘int ngcore::PajeFile::DefineEntityValue(int, const std::string&, double)’:
/<<PKGBUILDDIR>>/libsrc/core/paje_trace.cpp:322:21: warning: right shift count >= width of type [-Wshift-count-overflow]
  322 |               h ^= h>>32U;
      |                    ~^~~~~
make[3]: *** [libsrc/core/CMakeFiles/ngcore.dir/build.make:205: libsrc/core/CMakeFiles/ngcore.dir/taskmanager.cpp.o] Error 1
make[3]: *** [libsrc/core/CMakeFiles/ngcore.dir/build.make:163: libsrc/core/CMakeFiles/ngcore.dir/paje_trace.cpp.o] Error 1
make[3]: Leaving directory '/<<PKGBUILDDIR>>/obj-i686-linux-gnu'
make[2]: *** [CMakeFiles/Makefile2:1415: libsrc/core/CMakeFiles/ngcore.dir/all] Error 2
@drew-parsons
Copy link
Author

NETGEN_INLINE size_t * TablePrefixSum (FlatArray<unsigned int> entrysize)

has

NETGEN_INLINE size_t * TablePrefixSum (FlatArray<unsigned int> entrysize)
  { return TablePrefixSum32 (entrysize); }

while

NETGEN_INLINE size_t * TablePrefixSum (FlatArray<size_t> entrysize)

has

NETGEN_INLINE size_t * TablePrefixSum (FlatArray<size_t> entrysize)
  { return TablePrefixSum64 (entrysize); }

So size_t is 32-bit and the same as unsigned int on 32-bit machines. In that context I guess size_t should not be returning TablePrefixSum64. What's the best way to unscramble it? Make the size_t definitions conditional on not being a 32-bit system?

@drew-parsons
Copy link
Author

The same issue applies elsewhere where size_t is used, not just TablePrefixSum.
For instance patching around TablePrefixSum, there's another instance in python_ngcore.cpp.
https://buildd.debian.org/status/fetch.php?pkg=netgen&arch=i386&ver=6.2.2305%2Bdfsg1-1exp2&stamp=1699709448&raw=0

[  7%] Building CXX object libsrc/core/CMakeFiles/ngcore.dir/python_ngcore.cpp.o
cd /<<PKGBUILDDIR>>/obj-i686-linux-gnu/libsrc/core && /usr/bin/c++ -DFFMPEG -DHAVE_DLFCN_H -DHAVE_FREEIMAGE -DHAVE_FREETYPE -DHAVE_OPENGL_EXT -DHAVE_RAPIDJSON -DHAVE_TBB -DHAVE_TK -DHAVE_XLIB -DJPEGLIB -DNETGEN_PYTHON -DNGCORE_EXPORTS -DNG_PYTHON -DOCCGEOMETRY -DOCC_CONVERT_SIGNALS -DPARALLEL -DPYBIND11_SIMPLE_GIL_MANAGEMENT -D__STDC_CONSTANT_MACROS -Dngcore_EXPORTS -I/<<PKGBUILDDIR>>/obj-i686-linux-gnu/libsrc/core -I/<<PKGBUILDDIR>>/libsrc/core -I/<<PKGBUILDDIR>>/obj-i686-linux-gnu -I/<<PKGBUILDDIR>>/include -I/<<PKGBUILDDIR>>/libsrc -I/<<PKGBUILDDIR>>/libsrc/include -I/usr/include/opencascade -I/usr/lib/i386-linux-gnu/openmpi/include -I/usr/lib/i386-linux-gnu/openmpi/include/openmpi -I/usr/include/python3.11 -Wdate-time -D_FORTIFY_SOURCE=2 -g -O2 -ffile-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -O2 -g -DNDEBUG -std=gnu++17 -fPIC -fvisibility=hidden -MD -MT libsrc/core/CMakeFiles/ngcore.dir/python_ngcore.cpp.o -MF CMakeFiles/ngcore.dir/python_ngcore.cpp.o.d -o CMakeFiles/ngcore.dir/python_ngcore.cpp.o -c /<<PKGBUILDDIR>>/libsrc/core/python_ngcore.cpp
In file included from /<<PKGBUILDDIR>>/libsrc/core/python_ngcore.cpp:3:
/<<PKGBUILDDIR>>/libsrc/core/python_ngcore.hpp:149:10: error: redefinition of ‘struct ngcore::PyNameTraits<unsigned int>’
  149 |   struct PyNameTraits<size_t> {
      |          ^~~~~~~~~~~~~~~~~~~~
/<<PKGBUILDDIR>>/libsrc/core/python_ngcore.hpp:134:10: note: previous definition of ‘struct ngcore::PyNameTraits<unsigned int>’
  134 |   struct PyNameTraits<unsigned> {
      |          ^~~~~~~~~~~~~~~~~~~~~~
make[3]: *** [libsrc/core/CMakeFiles/ngcore.dir/build.make:247: libsrc/core/CMakeFiles/ngcore.dir/python_ngcore.cpp.o] Error 1

@drew-parsons
Copy link
Author

This patch seems to be sufficient

Index: netgen/libsrc/core/table.hpp
===================================================================
--- netgen.orig/libsrc/core/table.hpp
+++ netgen/libsrc/core/table.hpp
@@ -8,6 +8,7 @@
 /**************************************************************************/
 
 #include <atomic>
+#include <cstdint>
 #include <iostream>
 #include <optional>
 
@@ -103,8 +104,10 @@ namespace ngcore
   { return TablePrefixSum32 (FlatArray<unsigned> (entrysize.Size(), (unsigned int*)(int*)(entrysize.Addr(0)))); }
   NETGEN_INLINE size_t * TablePrefixSum (FlatArray<std::atomic<int>> entrysize)
   { return TablePrefixSum32 (FlatArray<unsigned> (entrysize.Size(), (unsigned int*)(std::atomic<int>*)entrysize.Addr(0))); }
+#if INTPTR_MAX != INT32_MAX
   NETGEN_INLINE size_t * TablePrefixSum (FlatArray<size_t> entrysize)
   { return TablePrefixSum64 (entrysize); }
+#endif
 
 
   /**
Index: netgen/libsrc/core/python_ngcore.hpp
===================================================================
--- netgen.orig/libsrc/core/python_ngcore.hpp
+++ netgen/libsrc/core/python_ngcore.hpp
@@ -2,6 +2,7 @@
 #define NETGEN_CORE_PYTHON_NGCORE_HPP
 
 #include "ngcore_api.hpp" // for operator new
+#include <cstdint>
 #include <pybind11/pybind11.h>
 #include <pybind11/operators.h>
 #include <pybind11/numpy.h>
@@ -145,10 +146,12 @@ namespace ngcore
     static std::string GetName() { return "D"; }
   };
 
+#if INTPTR_MAX != INT32_MAX
   template<>
   struct PyNameTraits<size_t> {
     static std::string GetName() { return "S"; }
   };
+#endif
 
   template<typename T>
   struct PyNameTraits<std::shared_ptr<T>> {
Index: netgen/libsrc/core/python_ngcore_export.cpp
===================================================================
--- netgen.orig/libsrc/core/python_ngcore_export.cpp
+++ netgen/libsrc/core/python_ngcore_export.cpp
@@ -1,3 +1,5 @@
+#include <cstdint>
+
 #include "python_ngcore.hpp"
 #include "bitarray.hpp"
 #include "taskmanager.hpp"
@@ -16,7 +18,9 @@ PYBIND11_MODULE(pyngcore, m) // NOLINT
   catch(...) {}
   ExportArray<int>(m);
   ExportArray<unsigned>(m);
+#if INTPTR_MAX != INT32_MAX
   ExportArray<size_t>(m);
+#endif  
   ExportArray<double>(m);
   ExportArray<float>(m);
   ExportArray<signed short>(m);
Index: netgen/tests/pytest/test_array.py
===================================================================
--- netgen.orig/tests/pytest/test_array.py
+++ netgen/tests/pytest/test_array.py
@@ -2,7 +2,12 @@ from pyngcore import *
 from numpy import sort, array
 
 def test_array_numpy():
-    a = Array_I_S(5)
+    try:
+        a = Array_I_S(5)
+    except NameError:
+        # 32-bit systems have restricted size_t definitions
+        # so test with unsigned int if size_t is not available
+        a = Array_I_U(5)
     a[:] = 0
     a[3:] = 2
     assert(sum(a) == 4)

The 32 bit test was adapted from https://stackoverflow.com/a/1505839/12401525
There may be other ways to test for it.

This patch simply skips definitions with <size_t> on 32 bit systems since they were identified as identical to the preceding <unsigned int> definition.

A consequence is that Array_I_S does not get defined in the python bindings (in python_ngcore_export.cpp). If it is important to have Array_I_S available, it could be verified in pyngcore/__init__.py, e.g.

try:
    Array_I_S
except NameError:
    Array_I_S = Array_I_U

@drew-parsons
Copy link
Author

Some 32 bit systems (armel, m68k, powerpc, sh4) don't link to atomic symbols. The problem is under discussion in gcc at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81358

This patch works around it for netgen, linking libngcore to libatomic where needed

Index: netgen/libsrc/core/CMakeLists.txt
===================================================================
--- netgen.orig/libsrc/core/CMakeLists.txt      2023-11-12 16:18:12.039784202 +0100
+++ netgen/libsrc/core/CMakeLists.txt   2023-11-12 16:20:53.737110429 +0100
@@ -60,6 +60,24 @@
     target_link_libraries(ngcore PRIVATE ${NUMA_LIBRARY})
 endif(USE_NUMA)

+# some 32-bit arches do not automatically link to atomic symbols (util.hpp)
+# cf. https://github.com/google/highway/pull/1008
+check_cxx_source_compiles(
+"#include <atomic>
+#include <cstdint>
+std::atomic<uint8_t> n8 (0);   // basic (should not fail)
+std::atomic<uint64_t> n64 (0); // expected to fail on armel, mipsel, powerpc (undefined reference to `__atomic_fetch_add_8')
+int main() {
+  ++n8;
+  ++n64;
+  return 0;
+}"
+ HAS_INTERNAL_ATOMIC
+)
+if(NOT HAS_INTERNAL_ATOMIC)
+  target_link_libraries(ngcore INTERFACE atomic)
+endif()
+
 install(TARGETS ngcore DESTINATION ${NG_INSTALL_DIR} COMPONENT netgen)

 target_link_libraries(ngcore PUBLIC netgen_mpi PRIVATE "$<BUILD_INTERFACE:netgen_python>" ${CMAKE_THREAD_LIBS_INIT})

atomic needs to be linked to ngcore as INTERFACE not PRIVATE (or PUBLIC) since libngcore.so itself does not use atomic symbols (they are used inline in core/utils.hpp).

The cmake test for atomic linking was adapted from google/highway#1008

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

No branches or pull requests

1 participant