RFR(S): JDK-8181503 "Can't compile hotspot with c++11"

[coleen.phillimore at oracle.com]

On 1/31/18 2:46 AM, Thomas Stüfe wrote:
> On Wed, Jan 31, 2018 at 2:07 AM, David Holmes <david.holmes at oracle.com>
> wrote:
>
>> Hi Gerard,
>>
>> These seem okay.
>>
>> I'm a little surprised that the LogConfiguration change doesn't cause
>> complaints about passing true/false as int ??
>>
>> And I second Coleen's query re the VMError codes - there seems to be no
>> reason for their strange values. They were added by JDK-4965918.
>>
>>
> They look Windows-ish, like someone wanted them to mimic SEH codes - like
> someone took them directly from the examples here:
> https://msdn.microsoft.com/en-us/library/het71c37.aspx.
>
> They bothered us for some reason I do not really remember anymore, so in
> our port they long have had different values, with no adverse effects.

What are the values in your port?  We should use the same.

Thanks,
Coleen

>
> Regards, Thomas
>
>
>> Thanks,
>> David
>>
>>
>> On 31/01/2018 4:26 AM, Gerard Ziemski wrote:
>>
>>> Hi,
>>>
>>> Please review the fixes for the following issues when compiling on
>>> Xcode9.2 with c++14. There are 5 unique issues fixed here:
>>>
>>>
>>> #1 "error: invalid suffix on literal; C++11 requires a space between
>>> literal and identifier [-Wreserved-user-defined-literal]”
>>>
>>> Example:
>>>
>>>     in: src/hotspot/os_cpu/bsd_x86/os_bsd_x86.cpp
>>> from: __asm__("mov %%"SPELL_REG_SP", %0":"=r"(esp));
>>>     to: __asm__("mov %%" SPELL_REG_SP ", %0":"=r"(esp));
>>>
>>>
>>> #2 "error: comparison between pointer and integer ('address' (aka
>>> 'unsigned char *') and 'int’)"
>>>
>>>     in: src/hotspot/share/code/compiledIC.cpp
>>> from: if (entry == false)
>>>     to: if (entry == NULL)
>>>
>>>
>>> #3 "error: case value evaluates to 3758096384, which cannot be narrowed
>>> to type 'int' [-Wc++11-narrowing]”
>>>
>>> This one requires an explanation: in “debug.hpp" we have:
>>>
>>>     enum VMErrorType {
>>>       INTERNAL_ERROR   = 0xe0000000,
>>>       OOM_MALLOC_ERROR = 0xe0000001,
>>>       OOM_MMAP_ERROR   = 0xe0000002
>>>     };
>>>
>>> With “VMErrorType” being of type “unsigned int”, but in “vmError.hpp” we
>>> have:
>>>
>>>     class VMError : public AllStatic {
>>>     ...
>>>       static int         _id;
>>>     ...
>>>
>>> So, with this code in “vmError.cpp”:
>>>
>>>        switch(_id) {
>>>          case OOM_MALLOC_ERROR:
>>>          case OOM_MMAP_ERROR:
>>>           ...
>>>            break;
>>>          case INTERNAL_ERROR:
>>>          default:
>>>            break;
>>>        }
>>>
>>> We get the error in the switch statement, because "unsigned int" value
>>> like 0xe0000000, doesn’t fit in signed “int”. The easiest and simplest way
>>> to fix this that I propose is to change the switch statement to:
>>>
>>>     switch(static_cast<unsigned int>(_id)) {
>>>
>>> There are other solutions, but they require much more pervasive changes.
>>>
>>> We can’t easily change the “_id” type itself to “unsigned int”, because
>>> all of the platforms, except Windows, use signed “int”, so we would need to
>>> touch lots of code and then cast to “int” eventually.
>>>
>>> We also we can’t cast INTERNAL_ERROR, OOM_MALLOC_ERROR and OOM_MMAP_ERROR
>>> to “int", ex:
>>>
>>>     enum VMErrorType {
>>>       INTERNAL_ERROR   = (int)0xe0000000,
>>>       OOM_MALLOC_ERROR = (int)0xe0000001,
>>>       OOM_MMAP_ERROR   = (int)0xe0000002
>>>     };
>>>
>>> Because we have code like:
>>>
>>>     static bool should_report_bug(unsigned int id) {
>>>       return (id != OOM_MALLOC_ERROR) && (id != OOM_MMAP_ERROR);
>>>     }
>>>
>>> Where we use “unsigned int” to compare against the errors, so we would
>>> get signed vs unsigned comparison warning, so again more code to touch.
>>>
>>>
>>> #4 warning: explicit instantiation of 'arraycopy_conjoint<signed char>'
>>> that occurs after an explicit specialization has no effect
>>> [-Winstantiation-after-specialization]
>>>
>>> I’m not 100% sure about this, but I propose that we simply remove the
>>> explicit instantiations, i.e. simply delete lines like:
>>>
>>> template void AccessInternal::arraycopy_conjoint<jbyte>(jbyte* src,
>>> jbyte* dst, size_t length);
>>>
>>>
>>> #5 warning: passing an object that undergoes default argument promotion
>>> to 'va_start' has undefined behavior [-Wvarargs]
>>>
>>> This one requires an explanation: in “logConfiguration.cpp" we have:
>>>        void LogConfiguration::configure_stdout(LogLevelType level, bool
>>> exact_match, ...) {
>>>       ...
>>>       va_start(ap, exact_match);
>>>
>>> According to https://wiki.sei.cmu.edu/confluence/display/cplusplus/EXP58-
>>> CPP.+Pass+an+object+of+the+correct+type+to+va_start <
>>> https://wiki.sei.cmu.edu/confluence/display/cplusplus/EXP58
>>> -CPP.+Pass+an+object+of+the+correct+type+to+va_start> the 2nd argument
>>> to the “va_start” macro must have the same argument type as the one after
>>> default argument promotion, and in this case we have “bool” that gets
>>> promoted to “int”, so the types are mismatched.
>>>
>>> The proposed fix is to change the type of “exact_match” from “bool” to
>>> “int”, which requires changing it in 3 places.
>>>
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8181503 <
>>> https://bugs.openjdk.java.net/browse/JDK-8181503>
>>> http://cr.openjdk.java.net/~gziemski/8181503_rev1 <
>>> http://cr.openjdk.java.net/~gziemski/8181503_rev1>
>>>
>>> Passes mach5 hs-tier1,2,3
>>>
>>>
>>> cheers
>>>
>>>
>>>
>>>