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

[coleen.phillimore at oracle.com]

Hi Gerard,   This looks good to me.  One question.

On 1/30/18 1:26 PM, 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
>    };

Why are these defined with these odd values anyway?  why not define as 
0, 1 and 2?  Or do they map to some Windows error values?

thanks,
Coleen
+
> 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
>
>
>