<AWT Dev> RFR(xs): 8221375: Windows 32bit build (VS2017) broken
Thomas Stüfe
thomas.stuefe at gmail.com
Mon Mar 25 14:58:41 UTC 2019
Hi Alexey,
thanks. I posted an updated fix for this on awt-net (8221405) since David
asked me to split the patch up. Could you please review it?
Thanks! ..Thomas
On Mon, Mar 25, 2019 at 3:46 PM Alexey Ivanov <alexey.ivanov at oracle.com>
wrote:
> Hi Thomas,
>
> There was a thread about failing build on 32 bit Windows:
> http://mail.openjdk.java.net/pipermail/build-dev/2018-November/023767.html
>
> It continued in February 2019:
> http://mail.openjdk.java.net/pipermail/build-dev/2019-February/024925.html
>
> Fast debug builds were affected. Likely there was mismatch between
> CALLBACK / JNICALL macro:
> http://mail.openjdk.java.net/pipermail/build-dev/2019-February/024972.html
> http://mail.openjdk.java.net/pipermail/build-dev/2019-February/024973.html
>
> Magnus suggested a fix:
> http://mail.openjdk.java.net/pipermail/build-dev/2019-February/024996.html
>
> Yet there was no reply as whether it fixed the debug build or not.
>
>
> Hope these pointers help.
>
> Regards,
> Alexey
>
> On 25/03/2019 12:45, Thomas Stüfe wrote:
>
> Hi all,
>
> Following David's suggestion, I withdraw this bug and will open issues for
> each area separately.
>
> Cheers, Thomas
>
>
>
> On Mon, Mar 25, 2019 at 1:44 PM Thomas Stüfe <thomas.stuefe at gmail.com>
> wrote:
>
>> Hi David,
>>
>> Updating vs2017 did not help :/
>>
>> Cheers, Thomas
>>
>> On Mon, Mar 25, 2019 at 8:17 AM David Holmes <david.holmes at oracle.com>
>> wrote:
>>
>>> Hi Thomas,
>>>
>>> On 25/03/2019 5:01 pm, Thomas Stüfe wrote:
>>> > Hi David,
>>> >
>>> > (added net-dev, awt-dev, security-dev since part of these fixes are in
>>> > their territory)
>>>
>>> May be better to split up the reviews, cross-posting that many groups
>>> gets very messy given most people won't be subscribed to them all -
>>> myself included.
>>>
>>
>>
>>>
>>> > On Mon, Mar 25, 2019 at 1:34 AM David Holmes <david.holmes at oracle.com
>>> > <mailto: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
>>>
>>> I think that would equate to an older version - 15.7
>>>
>>> MSVC++ 14.14 _MSC_VER == 1914 (Visual Studio 2017 version 15.7)
>>>
>>> Any chance you can upgrade to latest version? (Especially in light of
>>> the apparent compiler bug you cite below.)
>>>
>>> Thanks,
>>> David
>>> -----
>>>
>>> > > 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.java.net/pipermail/awt-dev/attachments/20190325/85e1bf00/attachment.html>
More information about the awt-dev
mailing list