Omr: Many compilation units contain nonresolvable WINDOWS macrodefinition

Created on 29 Mar 2018  路  5Comments  路  Source: eclipse/omr

When I try building OMR with enabled JitBuilder on Windows using MSVC or clang, the following error appears:

omr\compiler\cs2/timer.h(43): fatal error C1083: Cannot open include file: 'sys/time.h': No such file or directory

If we open the compiler/cs2/timer.h file, the following macrodefinition will be found:

#if defined(WINDOWS)
#include <time.h>
#include "windows_api.h"
#else
#include <sys/time.h>
#endif

So, #include <sys/time.h> is going to the compiled file regardless the build runs on Windows!

As described in the documentation, doc/BuildingWithCMake.cmake, on Windows we use the MSVC and clang toolchains to build OMR. I've checked, both toolchains define the following symbols on Windows (x64):

  • WIN32
  • _WIN32
  • _WIN64

No WINDOWS or WIN64 are defined there!

My suggestion is to add the using of WIN32 or _WIN32 (but WIN32 is much common for OMR, see later) in the coding standard and use this symbol where possible. I'm not sure whether the construction #if defined(WIN32) || defined(WIN64) does matter because WIN64 is defined neither by clang nor by MSVC even on Windows x64. If we must be sure the code is compiled on (for) Windows x32 only, the construction #if defined(WIN32) && !defined(OMR_ENV_DATA64) looks like a good solution.

Also, a number of compiler specific checks are in the project, I have no idea what to do with that (MinGW-related especially), so your opinion is needed!

The analysis shows the WINDOWS word occurrences in the following files:

  • compiler/compile/OMRCompilation.cpp
  • compiler/control/OMROptions.cpp
  • compiler/cs2/timer.h
  • compiler/cs2/windows_api.h
  • compiler/infra/Annotations.hpp
  • compiler/infra/Bit.hpp
  • compiler/infra/OMRMonitor.cpp
  • compiler/infra/ThreadLocal.h
  • compiler/ras/CallStack.cpp
  • compiler/ras/CallStackIterator.hpp
  • compiler/ras/Debug.cpp
  • compiler/ras/Debug.hpp
  • compiler/ras/IgnoreLocale.hpp
  • compiler/x/runtime/X86Runtime.hpp

The WIN32 word occurrences may be found in the following files:

  • ddr/include/ddr/scaner/pdb/PdbScanner.hpp
  • ddr/lib/ddr-scanner/pdb/PdbScanner.cpp
  • example/glue/UtilGlue.c
  • example/glue/VerboseManagerImpl.cpp
  • fvtest/gctest/gcTestHelpers.cpp
  • fvtest/omrGtestGlue/argmain.cpp
  • fvtest/porttest/fileTest.cpp
  • fvtest/porttest/main.cpp
  • fvtest/porttest/omrdumpTest.cpp
  • fvtest/porttest/omrfilestreamTest.cpp
  • fvtest/porttest/omrfileTest.cpp
  • fvtest/porttest/omrsignalExtendedTest.cpp
  • fvtest/porttest/omrsignalTest.cpp
  • fvtest/porttest/omrslTest.cpp
  • fvtest/porttest/omrstrTest.cpp
  • fvtest/porttest/omrtimeTest.cpp
  • fvtest/porttest/omrttyExtendedTest.cpp
  • fvtest/porttest/omrvmemTest.cpp
  • fvtest/porttest/si.cpp
  • fvtest/porttest/testHelpers.cpp
  • fvtest/porttest/testProcessHelpers.cpp
  • fvtest/porttest/vmemTest.cpp
  • fvtest/rastest/agentNegativeTest.cpp
  • fvtest/rastest/bindthreadagent.c
  • fvtest/rastest/cpuLoadAgent.c

    • fvtest/rastest/traceagent.c

    • fvtest/rastest/traceTest.cpp

  • fvtest/sigtest/sigTest.cpp
  • fvtest/sigtest/sigTestHelpers.cpp
  • fvtest/sigtest/sigTestHelpers.hpp
  • fvtest/threadtest/ospriority.cpp
  • fvtest/threadtest/createTestHelper.h
  • fvtest/threadtest/keyDestructorTest.cpp
  • fvtest/threadextendedtest/processTimeTest.cpp
  • fvtest/utiltest/main.cpp
  • gc/base/Bits.hpp (#if defined(WIN32) && !defined(OMR_ENV_DATA64))
  • glue/UtilGlue.c
  • glue/VerboseManagerImpl.cpp
  • include_core/omr.h
  • include_core/omrcomp.h
  • include_core/omrgcconst.h
  • include_core/omrmutex.h
  • include_core/omragent_internal.h
  • include_core/omrport.h
  • include_core/omrsig.h
  • include_core/omrutil.h (#if defined(WIN32) && !defined(WIN32_IBMC))
  • include_core/thrtypes.h
  • omrsigcompat/omrsig.cpp
  • omrsigcompat/omrsig_internal.hpp
  • omrtrace/omrtrace_internal.h (#if defined(WIN32) && !defined(__clang__))
  • port/common/omrcuda.cpp
  • port/common/omrexit.c
  • port/common/omrfilestreamtext.c
  • port/common/omrportcontrol.c
  • port/common/omrstr.c
  • thread/common/omrthread.c
  • thread/common/thrprof.c
  • thread/common/thread_internal.h
  • tools/hookgen/HookGen.cpp
  • tools/hookgen/main.cpp
  • tools/tracegen/main.cpp
  • tools/tracegen/Port.hpp
  • tools/tracegen/Port.cpp
  • tools/tracegen/TraceGen.cpp
  • tools/tracemerge/DATMerge.cpp
  • tools/tracemerge/main.cpp
  • util/omrutil/detectVMDirectory.c

There the construction #if defined(WIN32) || defined(WIN64) appears:

  • ddr/include/ddr/scaner/pdb/PdbScanner.hpp
  • fvtest/gctest/gcTestHelpers.cpp
  • fvtest/porttest/main.cpp (#if defined (WIN32) | defined (WIN64), TODO change to ||)
  • fvtest/porttest/omrdumpTest.cpp
  • fvtest/porttest/omrtimeTest.cpp
  • fvtest/porttest/si.cpp
  • fvtest/rastest/cpuLoadAgent.c
  • fvtest/threadextendedtest/processTimeTest.cpp
  • port/common/omrfilestreamtext.c

Edit: Some files check if WIN64 is defined:

  • port/win32/omrosbacktrace_impl.c
  • fvtest/porttest/omrsignalTest.cpp
  • fvtest/porttest/omrvmemTest.cpp
    (and if WIN64 is removed from cmake/modules/platform/os/win.cmake, the compilation fails on clang as well as on MSVC).

Two files use _WIN32:

  • compiler/env/DebugSegmentProvider.cpp
  • compiler/env/defines.h

And two files use MS_WINDOWS:

  • fvtest/compilertest/tests/main.cpp
  • fvtest/compilertest/tests/OMRTestEnv.cpp

More exotic WIN32_IBMC (what is this?):

  • include_core/omrutil.h (#if defined(WIN32) && !defined(WIN32_IBMC))
  • util/omrutil/win/omrgetdbghelp.c (#if !defined(WIN32_IBMC))

One check on 32-bits Windows only: defined(WIN32) && !defined (WIN64) is in the file util/omrutil/j9memclr.c. I think it won't work and should be replaced by #if defined(WIN32) && !defined(OMR_ENV_DATA64).

Also some compiler-specific checks are in the project:

  • include_core/omrcomp.h (#if defined(__MINGW32__) || defined(__MINGW64__))
  • port/win32/omrosdump.c (#if defined(_MSC_VER) #elif defined(__MINGW32__))
  • port/win32_include/omrportpg.h (#if _MSC_VER < 1600)
bug build / configure compiler gc jitbuilder port test thread tooling
Source
picture samolisov

All 5 comments

I wonder what WIN32_IBMC means. I went looking in openj9, and it didn't offer any hints. There is this interesting bit of code in j9, where the two blocks of code look identical to me (besides spacing), so it leads me to believe this is a weird problem with a compiler parser.

#ifdef WIN32_IBMC
typedef uintptr_t (* protected_fn)(void *);
typedef void (* handler_fn)(uintptr_t gpType, void *gpInfo, void *userData, struct J9SigContext *gpContext);
#else
typedef uintptr_t (*protected_fn)(void *);
typedef void (*handler_fn)(uintptr_t gpType, void *gpInfo, void *userData, struct J9SigContext *gpContext);
#endif

IBMC is a macro usually defined for xlc or the z/os system compiler, and as far as I know there isn't a Windows version of xlc. Please let me know if you figure it out!

Another note is that WIN64 and WIN32 are always defined together on x86_64 (edit: OMR adds it to the compile command). This comes from msvc defining _win32 and _win64 together. https://msdn.microsoft.com/en-us/library/b0084kay.aspx. I think that in a lot of places we use WIN32 to tell we are building on Windows, while we use WIN64 to tell that we are building for x86_64 platform. It would be cool to use an architecture define, likeX86_64 or AMD64, or OMR_ENV_DATA64 if it's more appropriate, in those places.

This may not be relevant to this issue, but I think it would be good to start using macros prefixed with OMR_. As more people use OMR, it would be nice if we didn't collide with commonly used macro names like WINDOWS.

I guess my vote would be to stop using WIN32 and start using a macro like WINDOWS (and in the future OMR_WINDOWS). The JIT already uses WINDOWS, while the rest of the code would need to be updated.

Another thing (not really related) is we have some more Windows defines that could use some documenting, because I have no idea what they're there for:

    -D_WIN_95
    -D_WIN32_WINDOWS=0x0500
    -D_WIN32_DCOM
    -D_WIN32_WINVER=${OMR_WINVER}
    -D_WIN32_WINNT=${OMR_WINVER}

We should probably consolidate the thread library flags, like OMRTHREAD_LIB_WIN32 while we're at it. I'm not sure how much work that would be.

youngar picture youngar on 30 Mar 2018
👍1

To try to help you with what you really want to do, WINDOWS should really be defined for the JIT. I'm guessing we're just missing the define somehow. Try adding it to https://github.com/eclipse/omr/blob/master/cmake/modules/platform/os/win.cmake#L24.

youngar picture youngar on 30 Mar 2018
👍1

AFAIK jitbuilder is not supported on windows, do i wouldnt be surprised if enabling jitbuilder causes a bunch of errors

dnakamura picture dnakamura on 30 Mar 2018

@youngar Thank you very much for the explanation.

Bellow is the result of my experiments.

If the following definitions are just removed from the win.cmake file, the compilation (except the compiler module and jitbuilder, I can't test them because jitbuilder is just not compiled on Windows at all) will be done without problems (on clang as well as MSVC):

    -D_WIN_95
    -D_WIN32_WINDOWS=0x0500
    -D_WIN32_DCOM
    -D_WIN32_WINVER=${OMR_WINVER}
    -D_WIN32_WINNT=${OMR_WINVER}

My search tool returns no occurrences of _WIN_95, _WIN32_WINDOWS, _WIN32_DCOM, _WIN32_WINVER and _WIN32_WINNT within OMR, except *.mk files. I think the definitions were just copied from the Autotools configuration. Maybe the flags are required for correct linking?

I've found that _X86_ and _AMD64_ are used in the port/win32/omrsysinfo.c file (as well as _PPC_), _amd64_ and _i386_ also appear in the compiler/env/defines.h (_i386_ is not defined in OMR) but what is interesting, the constants are defined only in win.cmake module, and used only for the win32 port. I'm not sure these definitions should be replaced by anything, but what about moving them to the cmake/modules/platform/arch/x86.cmake file? Some flags from J9 also still live there: -DJ9HAMMER and -DJ9X86.

I see no problem if all WINDOWS, WIN32, WIN32 || WIN64 occurrences would be changed to OMR_WINDOWS previously defining OMR_WINDOWS in the corresponding CMake module (and for Autotools as well, of course).

I've added files, which use WIN64 only to be sure the compilation is going on Windows x64, to the issue; if I remove the WIN64 definition from win.cmake, the compilation fails as it is expected. The constructions like #ifdef WIN64 can be replaced by #if defined(OMR_WINDOWS) && defined(OMR_ENV_DATA64), I've tried putting the second variant into the port/win32/omrosbacktrace_impl.c file and the file was compiled successfully. Moreover, the case is covered by fvtests: if WIN64 is not defined at all (and not replaced by OMR_WINDOWS), the tests are compiled well, but the omrporttest gets stuck while running.

I have no idea about WIN32_IBMC, I agree, if there is xlc for Windows, an attempt to compile the code using this compilator is a good idea, otherwise, the preprocessor directives which contain the keyword are candidates for removing from the code, do you agree?

@dnakamura Thank you for the comment, I tried to build JitBuilder on my Windows machine, but the preprocessor generated some strange stuff and I decided to get more information about how Windows is detected in the entire code of OMR. The result is here.

samolisov picture samolisov on 30 Mar 2018

Closed since #2434 has been merged.

samolisov picture samolisov on 5 Jun 2018
Was this page helpful?
0 / 5 - 0 ratings

Related issues

hzongaro picture hzongaro  路  4Comments
fjeremic picture fjeremic  路  3Comments
0xdaryl picture 0xdaryl  路  5Comments
fjeremic picture fjeremic  路  3Comments
0dvictor picture 0dvictor  路  5Comments