RFR(S): JDK-8181503 "Can't compile hotspot with c++11"
coleen.phillimore at oracle.com
coleen.phillimore at oracle.com
Wed Jan 31 16:20:01 UTC 2018
On 1/31/18 9:40 AM, Thomas Stüfe wrote:
> Hi Coleen,
>
> On Wed, Jan 31, 2018 at 3:06 PM, <coleen.phillimore at oracle.com
> <mailto:coleen.phillimore at oracle.com>> wrote:
>
>
>
> 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 <mailto: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
> <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.
>
>
> enum ErrorType {
> // internal_error are errors which are not crashes - have no
> // associated signal number or exception id.
> internal_error = 0xffffffffe0000000ULL,
> oom_error = 0xffffffffe0000001ULL,
> };
>
> So, we extended them to 64bit and made the corresponding _id field
> 64bit unsigned. I think the reason was that we had clashes with third
> party DLLs using these SEH codes. So I just extended them to 64bit to
> avoid clashes with any possible Windows SEH codes.
>
> But I do not particularly like this approach now. A cleaner fix would
> be to separate signal number resp. SEH code from our own error id like
> this:
>
> enum ErrorType {
> // Crash. See _signo for signal number / SEH code.
> crash = 1,
> // internal_error are errors which are not crashes - have no
> // associated signal number or exception id.
> internal_error = 2
> oom_error = 3
> ...
> };
>
> and have a second member like _signo to hold the signal number on
> Linux, SEH code on Windows. Ideally that one should be unsigned 32bit
> to hold both worlds (int signals and DWORD SEH codes).
>
> That way we do not have to think about intersecting value ranges of
> _id. What do you think?
Yes, I agree this would be a lot better and save so much confusion. I'm
going to cut/paste your mail into an RFE.
thanks,
Coleen
>
> Kind Regards, Thomas
>
> 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-
> <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
> <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> <
> 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/%7Egziemski/8181503_rev1> <
> http://cr.openjdk.java.net/~gziemski/8181503_rev1
> <http://cr.openjdk.java.net/%7Egziemski/8181503_rev1>>
>
> Passes mach5 hs-tier1,2,3
>
>
> cheers
>
>
>
>
>
>
More information about the hotspot-runtime-dev
mailing list