JDK-8006357 : Need a stable way to select parts of the VM to compile in and out
  • Type: Bug
  • Component: embedded
  • Sub-Component: hotspot
  • Affected Version: hs24,hs25
  • Priority: P3
  • Status: Resolved
  • Resolution: Not an Issue
  • Submitted: 2013-01-15
  • Updated: 2013-05-29
  • Resolved: 2013-05-29
Related Reports
Relates :  
Description
With the change for JDK-8005915 code guarded by 

 #ifndef SERIALGC

was replaced with code guarded by

#if INCLUDE_ALL_GCS

This is an improvement in readability, but it is a bit fragile since we don't have a way to guarantee that INCLUDE_ALL_GCS has been defined. In cases where it is not defined the default value is 0 which will be interpreted as false by the if statement.

So, in effect we have gone from two cases (SERIALGC defined or not) to three cases (INCLUDE_ALL_GCS defined as true, defined as false or not defined at all).

We don't want the last case, but since INCLUDE_ALL_GCS is defined in macros.hpp, this means that we need to make sure that all source files include macros.hpp. Unfortunately we have no way of guaranteeing that at the moment.

We need to find a way to make sure that INCLUDE_ALL_GCS is always defined. This was discussed in the GC team and we can think of three different ways of achieving this:

(1) Explicitly include macros.hpp in all source files. Make it a guideline to always have this include in all source files.

(2) Change from defining INCLUDE_ALL_GCS in macros.hpp and instead set in on the compile command line as -DINCLUDE_ALL_GCS=1

(3) Figure out if there is a warning that can be turned on to warn for #if statements on undefined variables.

We don't have to pick one of these solutions, but we need to find a way to be sure that we know that  INCLUDE_ALL_GCS is always set up.

Also, this is not limited to the  INCLUDE_ALL_GCS define. There are more flags that are used to compile in or out parts of the VM that probably have the same issue. Got this list in an email. I have not verified if they have the same problems or not:

INCLUDE_JVMTI
INCLUDE_FPROF
INCLUDE_VM_STRUCTS
INCLUDE_JNI_CHECK
INCLUDE_JNI_CHECK
INCLUDE_SERVICES
INCLUDE_MANAGEMENT
INCLUDE_CDS
INCLUDE_ALTERNATE_GCS
INCLUDE_NMT
INCLUDE_TRACE
Comments
Comment already added.
29-05-2013

Bengt, thank you for the very clear and detailed description of the problem! When working on the minimal VM we were very concerned about this very problem. To ensure that case number 3 is caught, hotspot is now built with the option -Wundef. This warns if there are undefined symbols. Also, hotspot builds with -Werror turns warnings into errors. x.c: int x; #if FOO #endif gcc -o x -c -Wundef -Werror x.c x.c:3:5: error: "FOO" is not defined. I think the behavior you want is already there. So I'm going to mark this bug as not an issue. If you think there's still a problem, let me know.
29-05-2013

8008474 added -Wundef to the build. Is this still an issue?
28-05-2013

This is a hotspot issue not an infrastructure issue, but I moved it to embedded/hotspot as it is the minimal VM changes that are behind this.
22-01-2013

Another possible solution would be to have a "config.hpp" that sets up all the defines we need. Then we can force all files to include config.hpp by adding "-incluce config.hpp" to the CCFLAGS in the make files. That way we are certain that all files include config.hpp. We could also generate the config.hpp file dynamically (similar to .configure). I prototyped this and attached a patch based on Joe's latest webrev. It works, but there is one big drawback. Precompiled headers have stopped working since it is no longer the first header file to be included. If we can fix the precompiled issue I think this is a nice way to go. If not I think we have to skip it. -include works fine with gcc. On windows /FI seems to do the same thing. I have not found any documentation for solaris studio, but it said that -include works there as well.
21-01-2013

Draft of how to use config.hpp to set up defines
21-01-2013

Some possible solutions: * gcc and g++ support the -Wundef option (warn about undefined preprocessor symbols). Combined with -Werror, this would treat undefined symbols as errors (brought up in a recent thread by Stefan Karlsson). Would need a similar feature in the other compilers we use (Solaris Studio and Visual C++). * dump the include dependencies at build time and use a utility to scan them to make sure that every compilation includes macros.hpp; if any compilation doesn't, trigger an error (causing the build to fail). gcc/g++ support the -include option which may help with this (pointed out by Mikael Vidstedt); Solaris studio has the -xM1 option. * If the feature should be enabled by default, then use this idiom to check if it's enabled: #if INCLUDE_ALL_GCS || !defined(INCLUDE_ALL_GCS) ... code ... #endif it's portable, but more verbose than ideal.
16-01-2013

I am not sure which sub component to assign this issue to. It is not really a GC issue since more than just INCLUDE_ALL_GCS is affected. The best I could come up with was runtime. Please feel free to assign a different sub component.
15-01-2013