From cab1117b50a322f8d99f10b11b9654d36e889548 Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Mon, 7 Oct 2024 10:52:17 +0200 Subject: [PATCH 1/7] CGAL_Core: protect against macro `free` From vcpkg CI, there is this error: ``` 123456789101112131415161718192021222324252627C:\PROGRA~1\MICROS~1\2022\ENTERP~1\VC\Tools\MSVC\1441~1.341\bin\Hostx64\x64\cl.exe /TP -DBOOST_ATOMIC_DYN_LINK -DBOOST_ATOMIC_NO_LIB -DBOOST_CHRONO_DYN_LINK -DBOOST_CHRONO_NO_LIB -DBOOST_CONTAINER_DYN_LINK -DBOOST_CONTAINER_NO_LIB -DBOOST_DATE_TIME_DYN_LINK -DBOOST_DATE_TIME_NO_LIB -DBOOST_IOSTREAMS_DYN_LINK -DBOOST_IOSTREAMS_NO_LIB -DBOOST_PROGRAM_OPTIONS_DYN_LINK -DBOOST_PROGRAM_OPTIONS_NO_LIB -DBOOST_RANDOM_DYN_LINK -DBOOST_RANDOM_NO_LIB -DBOOST_SERIALIZATION_DYN_LINK -DBOOST_SERIALIZATION_NO_LIB -DBOOST_THREAD_DYN_LINK -DBOOST_THREAD_NO_LIB -DBOOST_THREAD_USE_DLL -DCGAL_USE_GMPXX=1 -DGFLAGS_IS_A_DLL=1 -DGLOG_NO_ABBREVIATED_SEVERITIES -DGLOG_USE_GFLAGS -DGLOG_USE_GLOG_EXPORT -DH5_BUILT_AS_DYNAMIC_LIB -D_LIB -D_USE_BOOST -D_USE_EIGEN -D_USE_FAST_CBRT -D_USE_FAST_FLOAT2INT -D_USE_NONFREE -D_USE_OPENGL -D_USE_SSE -ID:\b\openmvs\x64-windows-dbg\libs\MVS\MVS_autogen\include -ID:\b\openmvs\src\v2.1.0-1e694de437.clean -ID:\b\openmvs\x64-windows-dbg -external:ID:\installed\x64-windows\include -external:ID:\installed\x64-windows\include\eigen3 -external:W0 /nologo /DWIN32 /D_WINDOWS /utf-8 /GR /EHsc /MP /D _CRT_SECURE_NO_DEPRECATE /D _CRT_NONSTDC_NO_DEPRECATE /D _SCL_SECURE_NO_WARNINGS /Gy /bigobj /Oi /Zm170 /Zc:__cplusplus /wd4231 /wd4251 /wd4308 /wd4396 /wd4503 /wd4661 /wd4996 /D_DEBUG /MDd /Z7 /Ob0 /Od /RTC1 -std:c++17 -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS /fp:strict /fp:except- /bigobj /Zc:__cplusplus /YuD:/b/openmvs/x64-windows-dbg/libs/MVS/CMakeFiles/MVS.dir/cmake_pch.hxx /FpD:/b/openmvs/x64-windows-dbg/libs/MVS/CMakeFiles/MVS.dir/./cmake_pch.cxx.pch /FID:/b/openmvs/x64-windows-dbg/libs/MVS/CMakeFiles/MVS.dir/cmake_pch.hxx /showIncludes /Folibs\MVS\CMakeFiles\MVS.dir\SceneReconstruct.cpp.obj /Fdlibs\MVS\CMakeFiles\MVS.dir\MVS.pdb /FS -c D:\b\openmvs\src\v2.1.0-1e694de437.clean\libs\MVS\SceneReconstruct.cpp D:\installed\x64-windows\include\CGAL/CORE/MemoryPool.h(76): error C2059: syntax error: 'constant' D:\installed\x64-windows\include\CGAL/CORE/MemoryPool.h(76): note: the template instantiation context (the oldest one first) is D:\installed\x64-windows\include\CGAL/CORE/MemoryPool.h(39): note: while compiling class template 'CORE::MemoryPool' D:\installed\x64-windows\include\CGAL/CORE/MemoryPool.h(119): error C2988: unrecognizable template declaration/definition D:\installed\x64-windows\include\CGAL/CORE/MemoryPool.h(119): error C2059: syntax error: 'constant' D:\installed\x64-windows\include\CGAL/CORE/Expr_impl.h(1217): error C2660: 'CORE::MemoryPool::_free_dbg': function does not take 2 arguments D:\installed\x64-windows\include\CGAL/CORE/MemoryPool.h(76): note: see declaration of 'CORE::MemoryPool::_free_dbg' D:\installed\x64-windows\include\CGAL/CORE/Expr_impl.h(1217): note: while trying to match the argument list '(void *, int)' ``` There is probably a macro named `free` somewhere. --- CGAL_Core/include/CGAL/CORE/ExprRep.h | 4 ++-- CGAL_Core/include/CGAL/CORE/Impl.h | 2 +- CGAL_Core/include/CGAL/CORE/MemoryPool.h | 2 +- CGAL_Core/include/CGAL/CORE/RealRep.h | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/CGAL_Core/include/CGAL/CORE/ExprRep.h b/CGAL_Core/include/CGAL/CORE/ExprRep.h index 0e721dda16a..a325930a2ab 100644 --- a/CGAL_Core/include/CGAL/CORE/ExprRep.h +++ b/CGAL_Core/include/CGAL/CORE/ExprRep.h @@ -595,7 +595,7 @@ public: } void operator delete( void *p, size_t ){ - MemoryPool::global_allocator().free(p); + (MemoryPool::global_allocator().free)(p); } private: @@ -1248,7 +1248,7 @@ void * AddSubRep::operator new( size_t size) template void AddSubRep::operator delete( void *p, size_t ) -{ MemoryPool >::global_allocator().free(p); } +{ (MemoryPool >::global_allocator().free)(p); } /// \typedef AddRep diff --git a/CGAL_Core/include/CGAL/CORE/Impl.h b/CGAL_Core/include/CGAL/CORE/Impl.h index 4ff8b4fa3d4..8ae4c53fe7d 100644 --- a/CGAL_Core/include/CGAL/CORE/Impl.h +++ b/CGAL_Core/include/CGAL/CORE/Impl.h @@ -58,7 +58,7 @@ { return MemoryPool >::global_allocator().allocate(size); } \ template \ CGAL_INLINE_FUNCTION void C::operator delete( void *p, size_t ) \ - { MemoryPool >::global_allocator().free(p); } + { (MemoryPool >::global_allocator().free)(p); } #endif // include some common header files diff --git a/CGAL_Core/include/CGAL/CORE/MemoryPool.h b/CGAL_Core/include/CGAL/CORE/MemoryPool.h index 2db3de8736e..d218a871bec 100644 --- a/CGAL_Core/include/CGAL/CORE/MemoryPool.h +++ b/CGAL_Core/include/CGAL/CORE/MemoryPool.h @@ -73,7 +73,7 @@ public: void* allocate(std::size_t size); - void free(void* p); + void free BOOST_PREVENT_MACRO_SUBSTITUTION (void* p); // Access the corresponding static global allocator. static MemoryPool& global_allocator() { diff --git a/CGAL_Core/include/CGAL/CORE/RealRep.h b/CGAL_Core/include/CGAL/CORE/RealRep.h index 1c5d0f13a40..f2ec1e90cb3 100644 --- a/CGAL_Core/include/CGAL/CORE/RealRep.h +++ b/CGAL_Core/include/CGAL/CORE/RealRep.h @@ -154,7 +154,7 @@ void * Realbase_for::operator new( size_t size) template void Realbase_for::operator delete( void *p, size_t ) -{ MemoryPool >::global_allocator().free(p); } +{ (MemoryPool >::global_allocator().free)(p); } typedef Realbase_for RealLong; typedef Realbase_for RealDouble; From 546d1301da401eb01b73b67dc1b498bd52de144d Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Tue, 8 Oct 2024 10:55:27 +0200 Subject: [PATCH 2/7] the patch was still incomplete --- CGAL_Core/include/CGAL/CORE/MemoryPool.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CGAL_Core/include/CGAL/CORE/MemoryPool.h b/CGAL_Core/include/CGAL/CORE/MemoryPool.h index d218a871bec..1cfa96fa93d 100644 --- a/CGAL_Core/include/CGAL/CORE/MemoryPool.h +++ b/CGAL_Core/include/CGAL/CORE/MemoryPool.h @@ -116,7 +116,7 @@ void* MemoryPool< T, nObjects >::allocate(std::size_t) { } template< class T, int nObjects > -void MemoryPool< T, nObjects >::free(void* t) { +void MemoryPool< T, nObjects >::free BOOST_PREVENT_MACRO_SUBSTITUTION (void* t) { CGAL_assertion(t != 0); if (t == 0) return; // for safety if(blocks.empty()){ From d907b362bdb8679fb450d762f10ff5cff40d3298 Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Tue, 8 Oct 2024 12:45:21 +0200 Subject: [PATCH 3/7] the patch was still not complete --- BGL/include/CGAL/boost/graph/METIS/partition_dual_graph.h | 4 ++-- BGL/include/CGAL/boost/graph/METIS/partition_graph.h | 4 ++-- CGAL_Core/include/CGAL/CORE/Impl.h | 2 +- .../include/CGAL/Classification/Feature/Elevation.h | 4 ++-- .../include/CGAL/Classification/Feature/Height_above.h | 2 +- .../include/CGAL/Classification/Feature/Height_below.h | 2 +- .../include/CGAL/Classification/Feature/Vertical_range.h | 2 +- Classification/include/CGAL/Classification/Image.h | 2 +- 8 files changed, 11 insertions(+), 11 deletions(-) diff --git a/BGL/include/CGAL/boost/graph/METIS/partition_dual_graph.h b/BGL/include/CGAL/boost/graph/METIS/partition_dual_graph.h index 53f8968f86f..9a217389d82 100644 --- a/BGL/include/CGAL/boost/graph/METIS/partition_dual_graph.h +++ b/BGL/include/CGAL/boost/graph/METIS/partition_dual_graph.h @@ -116,8 +116,8 @@ void partition_dual_graph(const TriangleMesh& tm, delete[] eptr; delete[] eind; - std::free(npart); - std::free(epart); + (std::free)(npart); + (std::free)(epart); } template diff --git a/BGL/include/CGAL/boost/graph/METIS/partition_graph.h b/BGL/include/CGAL/boost/graph/METIS/partition_graph.h index 08926a64116..42f8c240f01 100644 --- a/BGL/include/CGAL/boost/graph/METIS/partition_graph.h +++ b/BGL/include/CGAL/boost/graph/METIS/partition_graph.h @@ -151,8 +151,8 @@ void partition_graph(const TriangleMesh& tm, delete[] eptr; delete[] eind; - std::free(npart); - std::free(epart); + (std::free)(npart); + (std::free)(epart); } template diff --git a/CGAL_Core/include/CGAL/CORE/Impl.h b/CGAL_Core/include/CGAL/CORE/Impl.h index 8ae4c53fe7d..2e21aab5ac0 100644 --- a/CGAL_Core/include/CGAL/CORE/Impl.h +++ b/CGAL_Core/include/CGAL/CORE/Impl.h @@ -51,7 +51,7 @@ CGAL_INLINE_FUNCTION void *T::operator new( size_t size) \ { return MemoryPool::global_allocator().allocate(size); } \ CGAL_INLINE_FUNCTION void T::operator delete( void *p, size_t ) \ - { MemoryPool::global_allocator().free(p); } + { (MemoryPool::global_allocator().free)(p); } #define CORE_MEMORY_IMPL_TEMPLATE_WITH_ONE_ARG(C) \ template \ CGAL_INLINE_FUNCTION void *C::operator new( size_t size) \ diff --git a/Classification/include/CGAL/Classification/Feature/Elevation.h b/Classification/include/CGAL/Classification/Feature/Elevation.h index 175b20b6a44..9ea9f267cc4 100644 --- a/Classification/include/CGAL/Classification/Feature/Elevation.h +++ b/Classification/include/CGAL/Classification/Feature/Elevation.h @@ -130,7 +130,7 @@ public: std::nth_element (z.begin(), z.begin() + (z.size() / 10), z.end()); dtm_x(i,j) = z[z.size() / 10]; } - dem.free(); + (dem.free)(); if (grid.width() * grid.height() > input.size()) values.resize (input.size(), compressed_float(0)); @@ -162,7 +162,7 @@ public: values[*it] = v; } } - dtm_x.free(); + (dtm_x.free)(); } diff --git a/Classification/include/CGAL/Classification/Feature/Height_above.h b/Classification/include/CGAL/Classification/Feature/Height_above.h index b59b108c1ac..3c85d27f91e 100644 --- a/Classification/include/CGAL/Classification/Feature/Height_above.h +++ b/Classification/include/CGAL/Classification/Feature/Height_above.h @@ -100,7 +100,7 @@ public: std::size_t J = grid.y(i); values[i] = float(dtm(I,J) - get (point_map, *(input.begin() + i)).z()); } - dtm.free(); + (dtm.free)(); } } diff --git a/Classification/include/CGAL/Classification/Feature/Height_below.h b/Classification/include/CGAL/Classification/Feature/Height_below.h index 22371934155..f71195dd348 100644 --- a/Classification/include/CGAL/Classification/Feature/Height_below.h +++ b/Classification/include/CGAL/Classification/Feature/Height_below.h @@ -100,7 +100,7 @@ public: std::size_t J = grid.y(i); values[i] = float(get (point_map, *(input.begin() + i)).z() - dtm(I,J)); } - dtm.free(); + (dtm.free)(); } } diff --git a/Classification/include/CGAL/Classification/Feature/Vertical_range.h b/Classification/include/CGAL/Classification/Feature/Vertical_range.h index 45b9c98d3ee..a4df1591c13 100644 --- a/Classification/include/CGAL/Classification/Feature/Vertical_range.h +++ b/Classification/include/CGAL/Classification/Feature/Vertical_range.h @@ -102,7 +102,7 @@ public: std::size_t J = grid.y(i); values[i] = dtm(I,J); } - dtm.free(); + (dtm.free)(); } } diff --git a/Classification/include/CGAL/Classification/Image.h b/Classification/include/CGAL/Classification/Image.h index 084e9572764..3bd915f0b5d 100644 --- a/Classification/include/CGAL/Classification/Image.h +++ b/Classification/include/CGAL/Classification/Image.h @@ -71,7 +71,7 @@ public: { } - void free() + void free BOOST_PREVENT_MACRO_SUBSTITUTION () { m_raw.reset(); m_sparse.reset(); From 1f2f6c8e93701153d208e179a8fc4aab864884d0 Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Tue, 8 Oct 2024 12:49:14 +0200 Subject: [PATCH 4/7] add testing with _CRTDBG_MAP_ALLOC --- Installation/include/CGAL/config.h | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Installation/include/CGAL/config.h b/Installation/include/CGAL/config.h index bd5dafbf017..92a4ac768f5 100644 --- a/Installation/include/CGAL/config.h +++ b/Installation/include/CGAL/config.h @@ -37,6 +37,13 @@ #endif #ifdef CGAL_INCLUDE_WINDOWS_DOT_H + +#if defined(_MSC_VER) && defined(_DEBUG) +// Include support for memory leak detection +// This is only available in debug mode and when _CRTDBG_MAP_ALLOC is defined. +// It will include which will redefine `malloc` and `free`. +# define _CRTDBG_MAP_ALLOC 1 +#endif // Mimic users including this file which defines min max macros // and other names leading to name clashes #include From 98d4690da7ace7349a056b121914a1ddfb6da060 Mon Sep 17 00:00:00 2001 From: Andreas Fabri Date: Wed, 9 Oct 2024 13:45:26 +0100 Subject: [PATCH 5/7] Fix in Unique_hash_map --- Hash_map/include/CGAL/Hash_map/internal/chained_map.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Hash_map/include/CGAL/Hash_map/internal/chained_map.h b/Hash_map/include/CGAL/Hash_map/internal/chained_map.h index bea9b0993f0..bc94a3d3b0b 100644 --- a/Hash_map/include/CGAL/Hash_map/internal/chained_map.h +++ b/Hash_map/include/CGAL/Hash_map/internal/chained_map.h @@ -16,6 +16,7 @@ #ifndef CGAL_HASH_MAP_INTERNAL_CHAINED_MAP_H #define CGAL_HASH_MAP_INTERNAL_CHAINED_MAP_H +#include #include #include #include @@ -257,7 +258,7 @@ chained_map::chained_map(chained_map&& D) noexcept(std::is_nothrow_move_constructible_v && std::is_nothrow_move_constructible_v) : table(std::exchange(D.table, nullptr)) , table_end(std::exchange(D.table_end, nullptr)) - , free(std::exchange(D.free, nullptr)) + , free BOOST_PREVENT_MACRO_SUBSTITUTION (std::exchange(D.free, nullptr)) , table_size(std::exchange(D.table_size, 0)) , table_size_1(std::exchange(D.table_size_1, 0)) , alloc(std::move(D.alloc)) From 42488c787697ad1e004671dc7abafc4e09e8a2f6 Mon Sep 17 00:00:00 2001 From: Andreas Fabri Date: Wed, 9 Oct 2024 14:44:32 +0100 Subject: [PATCH 6/7] free -> freelist; Comment define of _CRTDBG_MAP_ALLOC --- .../CGAL/Hash_map/internal/chained_map.h | 29 +++++++++---------- Installation/include/CGAL/config.h | 2 +- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/Hash_map/include/CGAL/Hash_map/internal/chained_map.h b/Hash_map/include/CGAL/Hash_map/internal/chained_map.h index bc94a3d3b0b..0451685d133 100644 --- a/Hash_map/include/CGAL/Hash_map/internal/chained_map.h +++ b/Hash_map/include/CGAL/Hash_map/internal/chained_map.h @@ -16,7 +16,6 @@ #ifndef CGAL_HASH_MAP_INTERNAL_CHAINED_MAP_H #define CGAL_HASH_MAP_INTERNAL_CHAINED_MAP_H -#include #include #include #include @@ -45,7 +44,7 @@ class chained_map chained_map_elem* table; chained_map_elem* table_end; - chained_map_elem* free; + chained_map_elem* freelist; std::size_t table_size; std::size_t table_size_1; @@ -144,10 +143,10 @@ void chained_map::init_table(std::size_t n) std::allocator_traits::construct(alloc,table + i); } - free = table + t; + freelist = table + t; table_end = table + t + t/2; - for (Item p = table; p < free; ++p) + for (Item p = table; p < freelist; ++p) { p->succ = nullptr; p->k = nullkey; } @@ -161,10 +160,10 @@ inline void chained_map::insert(std::size_t x, T y) q->k = x; q->i = y; } else { - free->k = x; - free->i = y; - free->succ = q->succ; - q->succ = free++; + freelist->k = x; + freelist->i = y; + freelist->succ = q->succ; + q->succ = freelist++; } } @@ -214,7 +213,7 @@ T& chained_map::access(Item p, std::size_t x) // index x not present, insert it - if (free == table_end) // table full: rehash + if (freelist == table_end) // table full: rehash { rehash(); p = HASH(x); } @@ -225,7 +224,7 @@ T& chained_map::access(Item p, std::size_t x) return p->i; } - q = free++; + q = freelist++; q->k = x; init_inf(q->i); // initializes q->i to xdef q->succ = p->succ; @@ -246,7 +245,7 @@ chained_map::chained_map(const chained_map& D) { init_table(D.table_size); - for(Item p = D.table; p < D.free; ++p) + for(Item p = D.table; p < D.freelist; ++p) { if (p->k != nullkey || p >= D.table + D.table_size) { insert(p->k,p->i); //D.copy_inf(p->i); // see chapter Implementation @@ -258,7 +257,7 @@ chained_map::chained_map(chained_map&& D) noexcept(std::is_nothrow_move_constructible_v && std::is_nothrow_move_constructible_v) : table(std::exchange(D.table, nullptr)) , table_end(std::exchange(D.table_end, nullptr)) - , free BOOST_PREVENT_MACRO_SUBSTITUTION (std::exchange(D.free, nullptr)) + , freelist(std::exchange(D.freelist, nullptr)) , table_size(std::exchange(D.table_size, 0)) , table_size_1(std::exchange(D.table_size_1, 0)) , alloc(std::move(D.alloc)) @@ -273,7 +272,7 @@ chained_map& chained_map::operator=(const chained_ma init_table(D.table_size); - for(Item p = D.table; p < D.free; ++p) + for(Item p = D.table; p < D.freelist; ++p) { if (p->k != nullkey || p >= D.table + D.table_size) { insert(p->k,p->i); //copy_inf(p->i); // see chapter Implementation @@ -290,7 +289,7 @@ chained_map& chained_map::operator=(chained_map::statistics() const std::size_t n = 0; for (Item p = table; p < table + table_size; ++p) if (p ->k != nullkey) ++n; - std::size_t used_in_overflow = free - (table + table_size ); + std::size_t used_in_overflow = freelist - (table + table_size ); n += used_in_overflow; std::cout << "number of entries: " << n << "\n"; std::cout << "fraction of entries in first position: " << diff --git a/Installation/include/CGAL/config.h b/Installation/include/CGAL/config.h index 92a4ac768f5..64e68c53500 100644 --- a/Installation/include/CGAL/config.h +++ b/Installation/include/CGAL/config.h @@ -42,7 +42,7 @@ // Include support for memory leak detection // This is only available in debug mode and when _CRTDBG_MAP_ALLOC is defined. // It will include which will redefine `malloc` and `free`. -# define _CRTDBG_MAP_ALLOC 1 +// # define _CRTDBG_MAP_ALLOC 1 #endif // Mimic users including this file which defines min max macros // and other names leading to name clashes From 9cdce85fbddc66cdae9753e52eb2d1aa209ed74c Mon Sep 17 00:00:00 2001 From: Laurent Rineau Date: Mon, 14 Oct 2024 14:10:49 +0200 Subject: [PATCH 7/7] Re-add the test for `free`, since TBB has been updated on our Windows test machines --- Installation/include/CGAL/config.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Installation/include/CGAL/config.h b/Installation/include/CGAL/config.h index 64e68c53500..92a4ac768f5 100644 --- a/Installation/include/CGAL/config.h +++ b/Installation/include/CGAL/config.h @@ -42,7 +42,7 @@ // Include support for memory leak detection // This is only available in debug mode and when _CRTDBG_MAP_ALLOC is defined. // It will include which will redefine `malloc` and `free`. -// # define _CRTDBG_MAP_ALLOC 1 +# define _CRTDBG_MAP_ALLOC 1 #endif // Mimic users including this file which defines min max macros // and other names leading to name clashes