RFR : JDK-8032045 : (m/l) Enable compiler and linker safety switches for debug builds

[Mike Duigou]

On Feb 24 2014, at 06:09 , Magnus Ihse Bursie <magnus.ihse.bursie at oracle.com> wrote:

> On 2014-02-21 00:43, Mike Duigou wrote:
>> Hello all;
>> 
>> This issue is a followon to JDK-8030350 (https://bugs.openjdk.java.net/browse/JDK-8030350) which enhanced the compiler warnings used for compiling native code. The proposed changes principally impact the Linux platform.
>> 
>> While 8030350 was focused on compiler warnings which did not impact code generation, this changeset will, for some configurations, change the native code generated and likely change performance. These proposed option changes prevent specific types of relocation table, stack and heap memory corruption in native code. Preventing these types of memory corruption may be useful for finding certain kinds of bugs though and do provide some minimal additional protections against malicious attacks. They aren't, by any means, a substitute for following appropriate secure coding guidelines.
>> 
>> The rationale behind the implementation is as follows. For release builds during the initial phase of JDK 9 I would like to enable only compile time checks. This ends up being similar to the warnings in JDK-8030350. These options have no runtime impact on footprint or performance and very minimal additional compile time cost while providing value. **Release builds are not expected to see any performance or footprint change as a result of this changeset**
>> 
>> For fast debug builds we can enable linker protections (relro) and static compile time bounds checks (FORTIFY_SOURCE=1). FORTIFY_SOURCE=1 might be moved to the production builds as well because it has no runtime cost or executable size cost.
>> 
>> For slow debug builds we can enable full linker protection (at a potential cost in startup time), runtime bounds checks and stack protection (FORTIFY_SOURCE=2 -fprotect-stack-all). We will likely enable -fprotect-stack-strong when available in GCC 4.9
>> 
>> The basis for enabling the additional protections in debug builds is that it will help us find bugs in our native code and we aren't as concerned in debug builds with footprint and performance. Since many developers already do their personal builds in fastdebug or slowdebug mode for testing this will provide good opportunity to shake out any problems with the options while not impacting release builds. Should we find that any of the options provide significant value for their cost we can move them to fastdebug or release. If any of the options prove too costly they can be demoted or removed entirely.
>> 
>> https://bugs.openjdk.java.net/browse/JDK-8032045
>> http://cr.openjdk.java.net/~mduigou/JDK-8032045/2/webrev/
>> 
>> Additional to enabling the various compiler options I attempted to rationalize some of the skew between the various hotspot/make/{platform}/makefiles/gcc.make files while avoiding changing existing behaviour. I have also introduced the new -Og "optimize for debugging" option and there are now an explicit C{XX}_O_FLAG_DEBUG definitions to complement the C{XX}_O_FLAG_{DEBUG|NORM|HI|HIGHEST|NONE} optimization options.
> 
> Mike,
> 
> I have only looked at toolchain.m4.
> 
> First of all, I like the $C_O_FLAG_DEBUG logic.
> 
> Some questions/comments:
> * Have you tested the -qnoopt option on xlc?

No, though I did see another version of the same flag in the hotspot makefiles. I tried to use the canonical flag names rather than the shorthand versions. I will find a reference if I can or remove it.

> * This code and comment is not really aligned. Is the comment overly broad, or is it a mistake in C_O_FLAG_HIGHEST?
>  # Disable optimization
>  C_O_FLAG_HIGHEST="$C_O_FLAG_NORM"
>  C_O_FLAG_HI="$C_O_FLAG_DEBUG"


It looks like an error.

> * Maybe it's worth considering to extract the duplication of "-U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-all --param ssp-buffer-size=1" into a separate variable, something like CFLAGS_EXTRA_CHECKS or whatever. The same argument is also valid for the fastdebug extra switches, I think, even the amount of code duplication is not as high. Hm. Actually. In fact, from what I can tell this change will add these extra flags on all compilers, even though they are GCC only (?). So breaking out these flags into a variable, that can be set to empty if not using gcc, is probably not just a matter of style but a matter of correctness.

Sounds reasonable. 

> * I'd also have a preference for moving the "compiler supports -Og" check outside the actual flag setting. Maybe you can do the checking first and set a variable indicating the availablility of this flag, e.g. GCC_SUPPORTS_OG_FLAG, and then just check on that. The point here is that it's hard as it is to see the pattern between different optimization levels and compilers, but the more the code resembles a simple structured assignment matrix, the easier it is to see it. With checking code like this in the midst, it's easier to get lost.

Understood. Perhaps do the GCC_SUPPORTS_OG_FLAG in compiler setup?