Skip to content

protect CGAL against macro free#8523

Merged
sloriot merged 7 commits intoCGAL:masterfrom
lrineau:CGAL_CGAL-protect_against_macro_free-GF
Oct 18, 2024
Merged

protect CGAL against macro free#8523
sloriot merged 7 commits intoCGAL:masterfrom
lrineau:CGAL_CGAL-protect_against_macro_free-GF

Conversation

@lrineau
Copy link
Copy Markdown
Member

@lrineau lrineau commented Oct 7, 2024 •
edited
Loading

Summary of Changes

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<CORE::ConstDoubleRep,1024>::_free_dbg': function does not take 2 arguments
D:\installed\x64-windows\include\CGAL/CORE/MemoryPool.h(76): note: see declaration of 'CORE::MemoryPool<CORE::ConstDoubleRep,1024>::_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.

Update: If the macros _DEBUG and _CRTDBG_MAP_ALLOC are defined, then the Windows runtime headers redefine free and malloc.

This PR also adds a test for _CRTDBG_MAP_ALLOC in the CGAL test suite.

Release Management

@lrineau lrineau added Bug Pkg::CGAL_Core upstream bug Bugs in third party software used by CGAL labels Oct 7, 2024
@lrineau lrineau added this to the 6.0.1 milestone Oct 7, 2024
lrineau added a commit to lrineau/vcpkg that referenced this pull request Oct 7, 2024
@lrineau
add a patch
c1be0a9 
patch from upstream PR CGAL/cgal#8523
@lrineau
CGAL_Core: protect against macro free
cab1117 
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<CORE::ConstDoubleRep,1024>::_free_dbg': function does not take 2 arguments
D:\installed\x64-windows\include\CGAL/CORE/MemoryPool.h(76): note: see declaration of 'CORE::MemoryPool<CORE::ConstDoubleRep,1024>::_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.
@lrineau lrineau force-pushed the CGAL_CGAL-protect_against_macro_free-GF branch from 590724e to cab1117 Compare October 7, 2024 13:05
lrineau added a commit to lrineau/vcpkg that referenced this pull request Oct 7, 2024
@lrineau
add a patch
12f85a8 
patch from upstream PR CGAL/cgal#8523
@lrineau lrineau changed the title CGAL_Core: protect against macro free protect CGAL against macro free Oct 8, 2024
@lrineau lrineau added Pkg::BGL Pkg::PMP Pkg::Classification and removed upstream bug Bugs in third party software used by CGAL labels Oct 8, 2024
@afabri
Copy link
Copy Markdown
Member

afabri commented Oct 9, 2024

This also concerns TBB, in I-c-343 so not in our responsability. No idea if that can be protected with an `#undef`` and if we should do that.

@afabri
Copy link
Copy Markdown
Member

afabri commented Oct 9, 2024 •
edited
Loading

We also have the error I-c-343 (see code), when initializing a data member called free.

@afabri
Copy link
Copy Markdown
Member

afabri commented Oct 9, 2024

Concering TBB I suggest to upgrade it on rothko. It is TBB 2019. Already with 2022 on my machine I cannot reproduce. @janetournois @soesau can you do the upgrade please.

@sloriot
Copy link
Copy Markdown
Member

sloriot commented Oct 14, 2024

Successfully tested in CGAL-6.0.1-Ic-345

@sloriot
Copy link
Copy Markdown
Member

sloriot commented Oct 14, 2024

@lrineau still marked as draft. If you are done with it, please mark it as "Ready for review" so that it can be merged.

@lrineau lrineau marked this pull request as ready for review October 14, 2024 07:58
@lrineau
Copy link
Copy Markdown
Member Author

lrineau commented Oct 14, 2024 •
edited
Loading

@lrineau still marked as draft. If you are done with it, please mark it as "Ready for review" so that it can be merged.

You are right. I have marked it as ready for review.

@sloriot @afabri, I am still unsure about the patch of <CGAL/config.h>:

cgal/Installation/include/CGAL/config.h

Lines 41 to 46 in 42488c7

#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 <crtdbg.h> which will redefine `malloc` and `free`.
// # define _CRTDBG_MAP_ALLOC 1
#endif

I really would have liked that we fix the issue with TBB, and keep testing with #define _CRTDBG_MAP_ALLOC 1 every day. That is the only way to make sure the issue will not resurface.

@github-actions github-actions bot removed the Tested label Oct 14, 2024
@github-actions
Copy link
Copy Markdown

github-actions bot commented Oct 14, 2024

This pull-request was previously marked with the label Tested, but has been modified with new commits. That label has been removed.

@afabri
Copy link
Copy Markdown
Member

afabri commented Oct 15, 2024

I guess the warning in [I-c-348](
10>C:\dev\SCIP\lib\cmake\scip......\include\scip/def.h(109): warning C4005: 'getcwd': macro redefinition [C:\CGAL_ROOT\CGAL-6.0.1-Ic-348\cmake\platforms\MSVC2017-Debug-64bits\test\Polygonal_surface_reconstruction_Examples\polyfit_example_user_provided_planes.vcxproj]
C:\Program Files (x86)\Windows Kits\10\Include\10.0.22621.0\ucrt\crtdbg.h(327): note: see previous definition of 'getcwd') is triggered by this PR. @soesau can you please check if the two definitions are identical.

@afabri
Copy link
Copy Markdown
Member

afabri commented Oct 17, 2024

I reported an issue in the scip project on github.

@afabri afabri mentioned this pull request Oct 17, 2024
Closed
@afabri
Copy link
Copy Markdown
Member

afabri commented Oct 18, 2024

The scip project has fixed the warning. As we installed scip with vcpkg as @soesau told me, we will have the warning for a while in the testsuite.

@sloriot
Copy link
Copy Markdown
Member

sloriot commented Oct 18, 2024

Successfully tested in CGAL-6.0.1-Ic-351

@sloriot sloriot merged commit e7b8a4e into CGAL:master Oct 18, 2024
@lrineau lrineau deleted the CGAL_CGAL-protect_against_macro_free-GF branch October 21, 2024 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Bug Merged_in_6.0.1 Merged_in_6.0.2 Merged_in_6.0.3 Merged_in_6.1 Pkg::BGL Pkg::CGAL_Core Pkg::Classification Pkg::PMP Tested

Projects

None yet

Milestone

6.0.1

Development

Successfully merging this pull request may close these issues.

3 participants

@lrineau @afabri @sloriot