RFR(xs): 8221375: Windows 32bit build (VS2017) broken

[Thomas Stüfe]

Hi David,

(added net-dev, awt-dev, security-dev since part of these fixes are in
their territory)

On Mon, Mar 25, 2019 at 1:34 AM David Holmes <david.holmes at oracle.com>
wrote:

> Hi Thomas,
>
> A few queries, comments and concerns ...
>
> On 25/03/2019 6:58 am, Thomas Stüfe wrote:
> > Hi all,
> >
> > After a long time I tried to build a Windows 32bit VM (VS2017) and
> failed;
>
> I'm somewhat surprised as I thought someone was actively doing Windows
> 32-bit builds out there, plus there are shared code changes that should
> also have been caught by non-Windows 32-bit builds. :(
>
>
Not sure what others do. I did a 32bit windows build, slowdebug, warning
enabled, and it failed with those 5+ issues.


> > multiple errors and warnings. Lets reverse the bitrot:
> >
> > cr:
> >
> http://cr.openjdk.java.net/~stuefe/webrevs/8221375--windows-32bit-build-(vs2017)-broken-in-many-places/webrev.00/webrev/
> >
> > Issue: https://bugs.openjdk.java.net/browse/JDK-8221375
> >
> > Most of the fixes are trivial: Calling convention mismatches (awt DTRACE
> > callbacks), printf specifiers etc.
> >
> > Had to supress a warning in os_windows_x86.cpp - I was surprised by this
> > since this did not look 32bit specifc, do we disable warnings on Windows
> > 64bit builds?
>
> What version of VS2017? We use VS2017 15.9.6 and we don't disable warnings.
>
>
I use VS2017 CE. Not sure which version spcifically, but my compiler is at

Microsoft (R) C/C++ Optimizing Compiler Version 19.14.26431 for x86


> > The error I had in vmStructs.cpp was a bit weird: compiler complained
> about
> > an assignment of an enum value defined like this:
> >
> > hash_mask_in_place       = (address_word)hash_mask << hash_shift
> >
> > to an uint64_t variable, complaining about narrowing. I did not find out
> > what his problem was. In the end, I decided to add an explicit cast to
> > GENERATE_VM_LONG_CONSTANT_ENTRY(name) (see vmStructs.hpp).
>
> Not at all sure that's the right fix. In markOop.hpp we see that value
> gets special treatment on Windows-x64:
>

> #ifndef _WIN64
>           ,hash_mask               = right_n_bits(hash_bits),
>           hash_mask_in_place       = (address_word)hash_mask << hash_shift
> #endif
>    };
>
>    // Alignment of JavaThread pointers encoded in object header required
> by biased locking
>    enum { biased_lock_alignment    = 2 << (epoch_shift + epoch_bits)
>    };
>
> #ifdef _WIN64
>      // These values are too big for Win64
>      const static uintptr_t hash_mask = right_n_bits(hash_bits);
>      const static uintptr_t hash_mask_in_place  =
>                              (address_word)hash_mask << hash_shift;
> #endif
>
> perhaps something special is needed for Windows-x86. I'm unclear how the
> values can be "too big" ??
>
>
I banged my head against this for an hour or so and I think this is a
compiler bug.

What I get is:

warning C4838: conversion from '' to 'uint64_t' requires a narrowing
conversion

(Note the empty "from" string.)

Here are my tries to provoke the error:

VMLongConstantEntry iii[] = {  { "hallo", markOopDesc::hash_mask_in_place
}, {0,0}};  <<< this fails
VMLongConstantEntry iii = { "hallo", markOopDesc::hash_mask_in_place };
 << but this succeeds
uint64_t iii = markOopDesc::hash_mask_in_place;   << this succeeds  too

I have no clue what this means. It is difficult to fix since the expression
is hidden in such a macro pile. But I think either we go with my change or
we disable the warning for win32 for the whole section.


> >
> > With this patch we can build with warnings enabled on 32bit and 64bit
> > windows.
> >
> > Since patch fixes both hotspot and JDK parts, I'm sending it to hs-dev
> and
> > core-libs-dev.
>
> Don't see anything that actually comes under core-libs-dev. Looks like
> one net-dev, one awt-dev and one security-dev. Sorry.
>
>
Okay, I will add them.


> Cheers,
> David
> -----
>
>
Thanks for reviewing,

Thomas


> > Thanks, Thomas
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20190325/19c5a7c1/attachment.htm>