JDK-8071690 : Include local HotSpot headers before the system headers
  • Type: Enhancement
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 9
  • Priority: P4
  • Status: Resolved
  • Resolution: Duplicate
  • Submitted: 2015-01-27
  • Updated: 2015-06-08
  • Resolved: 2015-06-08
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 10
10Resolved
Related Reports
Relates :  
Description
There's no general rule in which order header files should be included but I think a good hint is to include local, project specific headers before system headers. This is mainly for two reasons:

1. It prevents that dependencies from local header files to system headers are hidden because a local header is included after a system header by chance. Instead, every local header should explicitly specify the system headers it depends on.

2. It enables the definition definition of local macros which control the behaviour of the system headers which are included later on.

Point two may be considered bad style, but we actually use it for example in src/share/vm/utilities/globalDefinitions.hpp where we define "__STDC_FORMAT_MACROS" before we include "<inttypes.h>" in the compiler specific global definitions file. 

"__STDC_FORMAT_MACROS" controls the definition of the printf format macros in "<inttypes.h>" but this can only work if "<inttypes.h>" is really included AFTER the definition of "__STDC_FORMAT_MACROS". And this can only wok if every file includes the local HotSpot headers before any system headers, because otherwise "<inttypes.h>" may be indirectly included by a system header before we had a chance to define "__STDC_FORMAT_MACROS".

This is exactly what happened after the integration of  8050807 which added the system include "<fcntl.h>" to vmError.cpp as follows:

#include <fcntl.h>
#include "precompiled.hpp"
#include "code/codeCache.hpp"

This change broke the build on AIX because "<fcntl.h>" indirectly included "<inttypes.h>" BEFORE we defined "__STDC_FORMAT_MACROS".

To prevent such errors in the future I propose to change all HotSpot source files to always include the system headers AFTER the inclusion of the project specific HotSpot headers.

I���ve attached a small Pythin script to this bug which can be used as follows to detect the files which are currently violating this rule:

find src/ \( -name "*.cpp" -o -name "*.hpp" \) -type f -exec python include.py {} \;

src/os/solaris/dtrace/generateJvmOffsets.cpp: system header #include <proc_service.h> included before local header #include "code/codeBlob.hpp"
src/os/windows/vm/decoder_windows.hpp: system header #include <imagehlp.h> included before local header #include "utilities/decoder.hpp"
src/os_cpu/bsd_zero/vm/os_bsd_zero.cpp: system header # include <pthread_np.h> included before local header #include "assembler_zero.inline.hpp"
src/share/vm/adlc/adlc.hpp: system header #include <iostream> included before local header #include "string.h"
src/share/vm/libadt/port.hpp: system header #include <string.h> included before local header #include "port_tandem.hpp"
src/share/vm/runtime/os.hpp: system header # include <setjmp.h> included before local header # include "jvm_solaris.h"
src/share/vm/trace/traceDataTypes.hpp: system header #include <stddef.h> included before local header #include "utilities/globalDefinitions.hpp"
src/share/vm/utilities/dtrace.hpp: system header #include <sys/types.h> included before local header #include "dtracefiles/hotspot.h"
src/share/vm/utilities/elfFile.cpp: system header #include <new> included before local header #include "memory/allocation.inline.hpp"
src/share/vm/utilities/elfFile.hpp: system header #include <stdio.h> included before local header #include "globalDefinitions.hpp"
src/share/vm/utilities/vmError.cpp: system header #include <fcntl.h> included before local header #include "precompiled.hpp"

The script is written in Python intentionally such that it can be easily added as an automated hook to jcheck to prevent violations of this inclusion rule in the future.

Of course we can also try to not rely on the inclusion order at all. IMHO it actually seems that this is the cleanest solution, but it may require moving some macro definitions from header files right into the command line (e.g. -D__STDC_FORMAT_MACROS for the example above). By the way, that's actually the way how I've fixed the above mentioned compile error on AIX without the need to touch any shared files.

What do you think?

Comments
The real problem here is the same as JDK-8086005
08-06-2015

@Kim: for me it's perfectly fine to close this bug and create a new one as proposed by you. Do you want me to close it or can you close it yourself?
08-06-2015

I see that ./hotspot/make/aix/makefiles/xlc.make adds -D__STDC_FORMAT_MACROS to CFLAGS, but that otherwise we're still depending on definitions of the configuration macros in our headers. As I said in the review of the proposed change to "order" headers per the description above (http://mail.openjdk.java.net/pipermail/hotspot-dev/2015-January/016952.html), I think using the build system is the only reliable way for us to deal with this. We should probably close this bug and create a new one to define via the build system all three of __STDC_{CONSTANT,FORMAT,LIMIT}_MACROS.
04-06-2015

I don't see how you can apply this as a general rule - or at least not without some further rules. If local foo.h depends on a system header then it will #include it, so a file that #includes foo.h can't control the order of that system header include. We need to recognize (and detect!) where we have implicit ordering constraints but I don't think a general rule actually helps with that. And there may be cases where we rely on a system include coming first eg: #ifndef SOME_SYSTEM_THING #define SOME_SYSTEM_THING xxx #endif
28-01-2015

Python script which detects if a file includes system headers before or in-between the local project headers. Use as follows: find src/ \( -name "*.cpp" -o -name "*.hpp" \) -type f -exec python include.py {} \;
27-01-2015