Build Failure on Windows with MSYS2 MinGW due to `gettimeofday` and `alloca` Issues

Foadsf commented 3 months ago

Description

I encountered build errors while compiling ElmerFEM on Windows using MSYS2 MinGW. The errors are related to the gettimeofday function and redefinition of alloca.

Steps to Reproduce

Clone the repository:

git clone https://github.com/ElmerCSC/elmerfem
cd elmerfem
git checkout 99abe68ee8d51feb07ec3f50a5026494e33c7992

Create a build directory and configure the build:
```
mkdir build
cd build
cmake -D WITH_MPI=OFF ..
```
Build the project:
```
cmake --build .
```

Error Messages

FAILED: elmergrid/src/metis-5.1.0/libmetis/CMakeFiles/metis.dir/__/GKlib/timers.c.obj
C:\tools\msys64\mingw64\bin\cc.exe -DCONTIG="" -DHAVE_EXECUTECOMMANDLINE -DMINGW32 -DUSE_ARPACK -DUSE_ISO_C_BINDINGS -DWIN32 -IC:/dev/Elmer/20240715/elmerfem/elmergrid/src/metis-5.1.0/libmetis/. -IC:/dev/Elmer/20240715/elmerfem/elmergrid/src/metis-5.1.0/include -IC:/dev/Elmer/20240715/elmerfem/elmergrid/src/metis-5.1.0/GKlib -std=c99 -fno-strict-aliasing -Wall -pedantic -Wno-unused-but-set-variable -Wno-unused-variable -Wno-unknown-pragmas -Wno-misleading-indentation -DNDEBUG -DNDEBUG2 -DUSE_GKREGEX -O2 -g -DNDEBUG -MD -MT elmergrid/src/metis-5.1.0/libmetis/CMakeFiles/metis.dir/__/GKlib/timers.c.obj -MF elmergrid\src\metis-5.1.0\libmetis\CMakeFiles\metis.dir\__\GKlib\timers.c.obj.d -o elmergrid/src/metis-5.1.0/libmetis/CMakeFiles/metis.dir/__/GKlib/timers.c.obj -c C:/dev/Elmer/20240715/elmerfem/elmergrid/src/metis-5.1.0/GKlib/timers.c
C:/dev/Elmer/20240715/elmerfem/elmergrid/src/metis-5.1.0/GKlib/timers.c: In function 'gk_WClockSeconds':
C:/dev/Elmer/20240715/elmerfem/elmergrid/src/metis-5.1.0/GKlib/timers.c:24:3: error: implicit declaration of function 'gettimeofday'; did you mean 'mingw_gettimeofday'? [-Wimplicit-function-declaration]
   24 |   gettimeofday(&ctime, NULL);
      |   ^~~~~~~~~~~~
      |   mingw_gettimeofday
[1032/2375] Building C object elmergrid/src/metis-5.1.0/libmetis/CMakeFiles/metis.dir/__/GKlib/gkregex.c.obj
C:/dev/Elmer/20240715/elmerfem/elmergrid/src/metis-5.1.0/GKlib/gkregex.c:488:10: warning: "alloca" redefined
  488 | # define alloca(size)   __builtin_alloca (size)
      |          ^~~~~~
In file included from C:/tools/msys64/mingw64/include/stdlib.h:659,
                 from C:/dev/Elmer/20240715/elmerfem/elmergrid/src/metis-5.1.0/GKlib/GKlib.h:30,
                 from C:/dev/Elmer/20240715/elmerfem/elmergrid/src/metis-5.1.0/GKlib/gkregex.c:55:
C:/tools/msys64/mingw64/include/malloc.h:164:9: note: this is the location of the previous definition
  164 | #define alloca(x) __builtin_alloca((x))
      |         ^~~~~~
C:/dev/Elmer/20240715/elmerfem/elmergrid/src/metis-5.1.0/GKlib/gkregex.c: In function 'parse_dup_op':
C:/dev/Elmer/20240715/elmerfem/elmergrid/src/metis-5.1.0/GKlib/gkregex.c:5089:39: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
 5089 |     postorder (elem, mark_opt_subexp, (void *) (long) elem->token.opr.idx);
      |                                       ^
C:/dev/Elmer/20240715/elmerfem/elmergrid/src/metis-5.1.0/GKlib/gkregex.c: In function 'mark_opt_subexp':
C:/dev/Elmer/20240715/elmerfem/elmergrid/src/metis-5.1.0/GKlib/gkregex.c:6301:19: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
 6301 |   int idx = (int) (long) extra;
      |                   ^
[1033/2375] Building C object fem/src/CMakeFiles/elmersolver.dir/Load.c.obj
ninja: build stopped: subcommand failed.

Suggested Fixes

Fix gettimeofday Issue: Modify the source code to use mingw_gettimeofday when compiling on Windows. Add the following conditional compilation block in timers.c:

#ifdef _WIN32
#include <sys/timeb.h>
#define gettimeofday mingw_gettimeofday
int mingw_gettimeofday(struct timeval *tv, void *tz) {
   struct _timeb timebuffer;
   _ftime(&timebuffer);
   tv->tv_sec = (long)timebuffer.time;
   tv->tv_usec = timebuffer.millitm * 1000;
   return 0;
}
#else
#include <sys/time.h>
#endif

Fix alloca Redefinition: Ensure alloca is only defined if it hasn't been defined already. Modify gkregex.c:
```
#ifndef alloca
#define alloca(size) __builtin_alloca(size)
#endif
```

Fix Casting Warnings: Use intptr_t or uintptr_t for casting between pointers and integers. Modify gkregex.c:

#include <stdint.h>
...
postorder(elem, mark_opt_subexp, (void *)(intptr_t)elem->token.opr.idx);
...
int idx = (int)(intptr_t)extra;

Environment

OS: Windows 10
Compiler: MSYS2 MinGW 64-bit
CMake Version: 3.24.0
ElmerFEM Version: commit 99abe68ee8d51feb07ec3f50a5026494e33c7992

mmuetzel commented 3 months ago

I can confirm this issue when using the compilers from MSYS2.

I'm not sure if this is MinGW-specific though. Maybe, this would also fail for GCC 14 or LLVM 18 on other platforms. These versions of the compilers are stricter when it comes to implicitly declaring functions (without prior explicit declaration).

mmuetzel commented 3 months ago

Note that intptr_t and uinptr_t was added for C99. It isn't guaranteed to be available in ANSI C. Currently, the build rules don't explicitly require a minimum version of the C standard. But most C compilers should default to C99 or a later standard by now. (GCC 14 defaults to C17 for example.) Should a minimum version of the C standard be required in the CMake build rules? Should an exact version be enforced?

tzwinger commented 3 months ago

Guess, this issue then also can be closed with respect to recent code updates

ElmerCSC / elmerfem