Skip to content

fix: windows compilation of hash maps tests and increase stack size avoiding segfaults #988

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+23 −0
Open

fix: windows compilation of hash maps tests and increase stack size avoiding segfaults #988

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f977ffc
fix build and runtime issues with hash tests usinc c/c++
jalvesz Apr 25, 2025
57bb15c
Merge branch 'fortran-lang:master' into windows
jalvesz Apr 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions test/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,14 @@ if (NOT TARGET "test-drive::test-drive")
find_package("test-drive" REQUIRED)
endif()

if(WIN32)
if(CMAKE_Fortran_COMPILER_ID MATCHES "^Intel")
add_link_options(/Qoption,link,/STACK:8388608)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've looked in the hashmap source file and I can't find reasons more stack size would be needed: there are no static (save)d variables or arrays there, only allocatables.

I'm worried that increasing the stack size may just be hiding other issues with the code that we haven't figured out yet. For example, the test cases include static arrays, maybe they could be replaced with allocatables?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably the arrays, but it would need figuring out. Keep in mind windows has a 1MB default, while Linux usually has 8MB. This brings it to 8MB.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test cases explicitly state that they may require quite absurd stack sizes. 48MB is huge. Not sure why you would ever need that. And clearly was an overestimation.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really weird. Not sure I understand the reason for why it was done like this. The first thing that is done is allocating 1MB of random data on the stack. https://github.com/fortran-lang/stdlib/blob/master/test%2Fhashmaps%2Ftest_open_maps.f90#L32

While the comment was saying 48MB that was for values of rand_power above 18. rand_size is 2**rand_power. rand_object is an integer array of rand_size elements. So this immediately overflows the stack on windows.

elseif(CMAKE_Fortran_COMPILER_ID STREQUAL GNU)
add_link_options(-Wl,--stack,8388608)
endif()
endif()

macro(ADDTEST name)
add_executable(test_${name} test_${name}.f90)
target_link_libraries(test_${name} "${PROJECT_NAME}" "test-drive::test-drive")
Expand Down
15 changes: 15 additions & 0 deletions test/hash_functions/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,21 @@ if(CMAKE_Fortran_COMPILER_ID MATCHES "^Intel")
set(CMAKE_CXX_EXTENSIONS OFF)

set_target_properties(test_hash_functions PROPERTIES LINKER_LANGUAGE Fortran)

if(WIN32)
set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreadedDLL$<$<CONFIG:Debug>:Debug>")
target_compile_options(
test_hash_functions
PRIVATE
$<$<COMPILE_LANGUAGE:Fortran>:/libs:dll> )
if (CMAKE_BUILD_TYPE STREQUAL "Debug" OR "RelWithDebInfo")
target_link_options(test_hash_functions
PRIVATE
/Qoption,link,/NODEFAULTLIB:libcmt
/Qoption,link,/NODEFAULTLIB:msvcrt.lib
/Qoption,link,/NODEFAULTLIB:libifcoremt.lib )
endif()
endif()
endif()

if(CMAKE_Fortran_COMPILER_ID STREQUAL GNU AND CMAKE_Fortran_COMPILER_VERSION VERSION_LESS 10.0)
Expand Down
Loading