RFR: JDK-8224257 : fix issues in files generated by pandoc

2019-05-29 Thread Jonathan Gibbons
Please review a fix to provide a new build-time tool to fix up the 
output generated by the "pandoc" tool, which generates output that fails 
to meet all of our documentation standards. Post-processing the output 
was deemed a better solution than trying to modify the tool itself.


Most of the work is in the code for the new tool, written by me. The 
remainder of the work, to integrate the tool into the build, was 
contributed by Erik Joelsson.


The new tool comes in two parts. The first part uses an HTML parser to 
stream the content of a file and to update it on the fly; the second 
part of the file is just a copy-paste of a simple HTML parser that has 
been used in other similar contexts, like the CodeTools DocCheck utility 
[DocCheck].


The following changes are made to files processed by the tool, in order 
to pass various accessibility checking tools.


 * the  element is modified to set `lang="en"` and to remove
   references to XML

 * ... is inserted around palpable content not already
   included in a landmark region

 * in tables, `scope="row"` is added to the cells in the column that
   best define the row

 * (minor) in the `` element, the content is
   updated to indicate that the file has been modified by this tool


-- Jon


JBS: https://bugs.openjdk.java.net/browse/JDK-8224257
Webrev: http://cr.openjdk.java.net/~jjg/8224257/webrev.00/webrev/index.html
DocCheck: https://openjdk.java.net/projects/code-tools/doccheck/



Re: Failure buiilding OPEN JDK 11 on windows cygwin with VC 2017

2019-05-29 Thread David Holmes

Can you provide the configuration summary produced by configure?

It's an issue with precompiled headers but I've no idea what.

David

On 30/05/2019 5:21 am, Moshe Zuisman wrote:
I  thought, that may be probleem ressults form some specific 
configuration on my  machine (for example I have 2 version of MSVCc - 
2012 and 2017.
So I created clean VmWare Virtual machine and installed their only MSVC 
2017 and cygwin. And tried to build OPEN JDK theeir. and got exactly 
same  failure.


ср, 29 мая 2019 г. в 09:40, Moshe Zuisman >:


I am using
http://hg.openjdk.java.net/jdk-updates/jdk11u/rev/a23d4b4ea281



ср, 29 мая 2019 г. в 08:35, David Holmes mailto:david.hol...@oracle.com>>:

Hi Moshe,

On 29/05/2019 2:15 am, Moshe Zuisman wrote:
 > I am trying to build OPEN jdk 11 on cygwin with MSVC 2017
 >

_

VS 2017 is the official supported compiler for building OpenJDK
11 [1]
and we don't see any issues. What version of the 11u source are you
trying to compile?

David

[1]
https://wiki.openjdk.java.net/display/Build/Supported+Build+Platforms

 >
 >

*c:\progra~2\mib055~1\2017\profes~1\vc\tools\msvc\1410~1.250\include\exception(366):
 > warning C4577: 'noexcept' used with no exception handling
mode specified;
 > termination on exception is not guaranteed. Specify
/EHscmake[3]: ***
 > [lib/CompileJvm.gmk:151:
 >

/cygdrive/c/Git_Repos/open_jdk_11/jdk11-1ddf9a99e4ad/jdk11-1ddf9a99e4ad/build/windows-x86_64-normal-server-release/hotspot/variant-server/libjvm/objs/BUILD_LIBJVM_pch.obj]
 > Error 1make[3]: *** Waiting for unfinished jobsmake[2]: ***
 > [make/Main.gmk:257: hotspot-server-libs] Error 2make[2]: ***
Waiting for
 > unfinished jobsERROR: Build failed for target 'default
 > (exploded-image)' in configuration
'windows-x86_64-normal-server-release'
 > (exit code 2)=== Output from failing command(s) repeated here
===* For
 > target
 >

hotspot_variant-server_libjvm_objs_BUILD_LIBJVM_pch.obj:BUILD_LIBJVM_pch.cppc:\progra~2\mib055~1\2017\profes~1\vc\tools\msvc\1410~1.250\include\exception(366):
 > error C2220: warning treated as error - no 'object' file
 >

generatedc:\progra~2\mib055~1\2017\profes~1\vc\tools\msvc\1410~1.250\include\exception(366):
 > warning C4577: 'noexcept' used with no exception handling
mode specified;
 > termination on exception is not guaranteed. Specify /EHsc 
  ... (rest of

 > output omitted)* All command lines available in
 >

/cygdrive/c/Git_Repos/open_jdk_11/jdk11-1ddf9a99e4ad/jdk11-1ddf9a99e4ad/build/windows-x86_64-normal-server-release/make-support/failure-logs.===
 > End of repeated output ===No indication of failed target
found.Hint: Try
 > searching the build log for '] Error'.Hint: See
 > doc/building.html#troubleshooting for assistance.make[1]: ***
 >

[/cygdrive/c/Git_Repos/open_jdk_11/jdk11-1ddf9a99e4ad/jdk11-1ddf9a99e4ad/make/Init.gmk:305:
 > main] Error 2make: ***
 >

[/cygdrive/c/Git_Repos/open_jdk_11/jdk11-1ddf9a99e4ad/jdk11-1ddf9a99e4ad/make/Init.gmk:186:
 > default] Error 2*
 >



Re: RFR (XXS) 8224790: Remove Xusage.txt file

2019-05-29 Thread Mandy Chung

Looks good to me.

Mandy

On 5/25/19 1:49 PM, David Holmes wrote:

Bug: https://bugs.openjdk.java.net/browse/JDK-8224790
webrev: http://cr.openjdk.java.net/~dholmes/8224790/webrev/

Before Java 7 the Xusage.txt file was used to print the "java -X" help 
information in English. From Java 7 this information was provided in 
localised format by the launcher. The Xusage.txt file was not removed 
at the time because it was also used by the gamma launcher. But the 
gamma launcher was removed in java 8 - JDK-8008772. The information in 
Xusage.txt has become stale and incomplete (despite several edits post 
Java 7). It should just be removed.


Also removed the build logic that copied the file into the JDK image.

Thanks,
David




Re: RFR: JDK-8224011: Failure handling in ExecuteWithLog fails in run-test-prebuilt

2019-05-29 Thread Tim Bell

Erik:


When running tests using "make run-test-prebuilt", we bypass a lot of
Init.gmk. One of the things we skip is initialization of the
make-support/failure-logs dir. This causes make to print errors about
failing to copy files into that directory if for example the AOT
compilation fails, partly hiding the original error. This change simply
adds a MakeDir on that directory as part of the RunTestPrebuilt.gmk
bootstrap.

Bug: https://bugs.openjdk.java.net/browse/JDK-8224011

Webrev: http://cr.openjdk.java.net/~erikj/8224011/webrev.01/index.html


Looks good.


(The empty change in RunTests.gmk was a trailing tab that my Emacs
thoroughly disliked, so including that here too.)


Thanks for fixing that also.

Tim



RFR: JDK-8224011: Failure handling in ExecuteWithLog fails in run-test-prebuilt

2019-05-29 Thread Erik Joelsson
When running tests using "make run-test-prebuilt", we bypass a lot of 
Init.gmk. One of the things we skip is initialization of the 
make-support/failure-logs dir. This causes make to print errors about 
failing to copy files into that directory if for example the AOT 
compilation fails, partly hiding the original error. This change simply 
adds a MakeDir on that directory as part of the RunTestPrebuilt.gmk 
bootstrap.


Bug: https://bugs.openjdk.java.net/browse/JDK-8224011

Webrev: http://cr.openjdk.java.net/~erikj/8224011/webrev.01/index.html

(The empty change in RunTests.gmk was a trailing tab that my Emacs 
thoroughly disliked, so including that here too.)


/Erik



Re: Failure buiilding OPEN JDK 11 on windows cygwin with VC 2017

2019-05-29 Thread Moshe Zuisman
I  thought, that may be probleem ressults form some specific configuration
on my  machine (for example I have 2 version of MSVCc - 2012 and 2017.
So I created clean VmWare Virtual machine and installed their only MSVC
2017 and cygwin. And tried to build OPEN JDK theeir. and got exactly same
failure.

ср, 29 мая 2019 г. в 09:40, Moshe Zuisman :

> I am using  http://hg.openjdk.java.net/jdk-updates/jdk11u/rev/a23d4b4ea281
>
>
> ср, 29 мая 2019 г. в 08:35, David Holmes :
>
>> Hi Moshe,
>>
>> On 29/05/2019 2:15 am, Moshe Zuisman wrote:
>> > I am trying to build OPEN jdk 11 on cygwin with MSVC 2017
>> >
>> _
>>
>> VS 2017 is the official supported compiler for building OpenJDK 11 [1]
>> and we don't see any issues. What version of the 11u source are you
>> trying to compile?
>>
>> David
>>
>> [1] https://wiki.openjdk.java.net/display/Build/Supported+Build+Platforms
>>
>> >
>> >
>> *c:\progra~2\mib055~1\2017\profes~1\vc\tools\msvc\1410~1.250\include\exception(366):
>> > warning C4577: 'noexcept' used with no exception handling mode
>> specified;
>> > termination on exception is not guaranteed. Specify /EHscmake[3]: ***
>> > [lib/CompileJvm.gmk:151:
>> >
>> /cygdrive/c/Git_Repos/open_jdk_11/jdk11-1ddf9a99e4ad/jdk11-1ddf9a99e4ad/build/windows-x86_64-normal-server-release/hotspot/variant-server/libjvm/objs/BUILD_LIBJVM_pch.obj]
>> > Error 1make[3]: *** Waiting for unfinished jobsmake[2]: ***
>> > [make/Main.gmk:257: hotspot-server-libs] Error 2make[2]: *** Waiting for
>> > unfinished jobsERROR: Build failed for target 'default
>> > (exploded-image)' in configuration
>> 'windows-x86_64-normal-server-release'
>> > (exit code 2)=== Output from failing command(s) repeated here ===* For
>> > target
>> >
>> hotspot_variant-server_libjvm_objs_BUILD_LIBJVM_pch.obj:BUILD_LIBJVM_pch.cppc:\progra~2\mib055~1\2017\profes~1\vc\tools\msvc\1410~1.250\include\exception(366):
>> > error C2220: warning treated as error - no 'object' file
>> >
>> generatedc:\progra~2\mib055~1\2017\profes~1\vc\tools\msvc\1410~1.250\include\exception(366):
>> > warning C4577: 'noexcept' used with no exception handling mode
>> specified;
>> > termination on exception is not guaranteed. Specify /EHsc   ... (rest of
>> > output omitted)* All command lines available in
>> >
>> /cygdrive/c/Git_Repos/open_jdk_11/jdk11-1ddf9a99e4ad/jdk11-1ddf9a99e4ad/build/windows-x86_64-normal-server-release/make-support/failure-logs.===
>> > End of repeated output ===No indication of failed target found.Hint: Try
>> > searching the build log for '] Error'.Hint: See
>> > doc/building.html#troubleshooting for assistance.make[1]: ***
>> >
>> [/cygdrive/c/Git_Repos/open_jdk_11/jdk11-1ddf9a99e4ad/jdk11-1ddf9a99e4ad/make/Init.gmk:305:
>> > main] Error 2make: ***
>> >
>> [/cygdrive/c/Git_Repos/open_jdk_11/jdk11-1ddf9a99e4ad/jdk11-1ddf9a99e4ad/make/Init.gmk:186:
>> > default] Error 2*
>> >
>>
>


Re: RFR: 8223678: Add Visual Studio Code workspace generation support (for native code)

2019-05-29 Thread Erik Joelsson

Thanks, looks good!

/Erik

On 2019-05-29 06:52, Robin Westberg wrote:

Hi Erik,

Thanks for taking a look!


On 28 May 2019, at 15:52, Erik Joelsson  wrote:

Hello Robin,

Looks good.

Adding of ide.md is very nice, but could you try to format it like testing.md 
and building.md in regards to line lengths? I believe we are trying for 80 
chars in those to make them read reasonably in a standard editor.

Sure, did a bit of reflowing and updated some formatting in order to look more 
like the other .md files.

Webrevs:
  - Full: https://cr.openjdk.java.net/~rwestberg/8223678/webrev.02/
  - Inc: https://cr.openjdk.java.net/~rwestberg/8223678/webrev.01-02/

Best regards,
Robin


/Erik

On 2019-05-27 09:03, Robin Westberg wrote:

Hi all,

Please review this change that adds build system support for generating a 
Visual Studio Code workspace configured for working with the JDK native code. 
It configures the default C/C++ IntelliSense Engine to allow code 
completion/navigation and similar features. It also configures two executable 
targets (gtestLauncher and java) that can be built and debugged from the IDE.

The main target is "make vscode-project”, additional information can be found 
in doc/ide.[md|html].

Issue:
https://bugs.openjdk.java.net/browse/JDK-8223678

Webrev:
https://cr.openjdk.java.net/~rwestberg/8223678/webrev.01/

Testing:
Manual testing on Linux, MacOS and Windows

Thanks Erik Joelsson for taking a look at an earlier version of it!

Best regards,
Robin


Re: RFR: 8224851: AArch64: fix warnings and errors with Clang and GCC 8.3

2019-05-29 Thread Kim Barrett
Not a review, just briefly jumping in because I noticed one thing to correct.

> On May 29, 2019, at 9:12 AM, Andrew Haley  wrote:
> 
> On 5/29/19 11:26 AM, Nick Gasson wrote:
> 
> Oh dear. Experience has shown me (and probably you, too) that this is
> one of the most error-prone parts of software development. Many
> very serious failures have been caused by trying to shut up
> compilers. These failures have even included major security breaches.
> 
> With that, let's press on with a heavy heart…

+1 on all that.

>> This first one is with GCC 8.3:
>> 
>> /home/nicgas01/jdk/src/hotspot/cpu/aarch64/frame_aarch64.cpp: In function 
>> 'void pf(long unsigned int, long unsigned int, long unsigned int, long 
>> unsigned int, long unsigned int)':
>> /home/nicgas01/jdk/src/hotspot/cpu/aarch64/frame_aarch64.cpp:775:35: error: 
>> 'void* memcpy(void*, const void*, size_t)' writing to an object of type 
>> 'class RegisterMap' with no trivial copy-assignment; use copy-assignment or 
>> copy-initialization instead [-Werror=class-memaccess]
>>   memcpy(reg_map, , sizeof map);
>>   ^
>> 
>> RegisterMap is non-POD because its base class AllocatedObj has virtual
>> methods, so we need to use copy-assignment rather than memcpy. But
>> because we're allocating reg_map with os::malloc we ought to use
>> placement new to properly construct it first. But we can't do that
>> because RegisterMap inherits from StackObj which disallows new. So I
>> just put in some casts to void * to silence the warning.
> 
> We can't do that. It seems to me that this must be Undefined
> Behaviour, and we must never attempt to cover up UB with casts.

I think the solution here is to use placement new, but explicitly qualified to 
actually get there, e.g.

::new (reg_map) RegisterMap(map);

Using unqualified new will do the lookup in RegisterMap, find some declarations 
in StackObj (but not the one we want) and fail.
This is a pretty common pattern.

>> /home/nicgas01/jdk/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp:2684:17:
>>  error: shifting a negative signed value is undefined 
>> [-Werror,-Wshift-negative-value]
>>offset &= -1<<12;
>>  ~~^
> 
> Oh FFS. :-) This is perfectly legal in GCC and probably clang as well.

Yeah, I sometimes think we should disable -Wshift-negative-value.




Re: RFR: 8223678: Add Visual Studio Code workspace generation support (for native code)

2019-05-29 Thread Volker Simonis
On Wed, May 29, 2019 at 3:43 PM Robin Westberg
 wrote:
>
> Hi Volker,
>
> > On 28 May 2019, at 17:33, Volker Simonis  wrote:
> >
> > Hi Robin,
> >
> > thanks a lot for doing this!
> >
> > I have just a quick question. Do you know if any of the VSC indexers
> > (default, clangd) support call hierarchies (i.e. "called by",
> > "callers" of a function/method) and "used by" for variables/class
> > fields?
>
> Sure, I can make a quick summary of the various pros and cons of the indexers 
> that I’ve found so far. They are all moving pretty fast though, so didn’t 
> think it was a good fit for the documentation file.
>
> In general, VS Code itself only just recently gained proper support for 
> displaying call hierarchies: 
> https://code.visualstudio.com/updates/v1_33#_call-hierarchy - but alternative 
> indexers have worked around this by showing them differently.
>
> Default (Microsoft - C/C++ for Visual Studio Code)
> + Easy to setup, as no additional dependencies are needed
> + Good “go to definition”, the only one that can make sense of the 
> template+macro stuff in log.hpp for example
> - Find references (used by) not done yet (said to be coming in the June 
> update: https://github.com/microsoft/vscode-cpptools/issues/15)
> - Call hierarchies also not there (scheduled for September apparently: 
> https://github.com/microsoft/vscode-cpptools/issues/16)
>
> clangd
> + Actively developed and part of the llvm/clang project
> + Support for find references
> - ..but not call hierarchies yet
> - Full project indexing is still flagged as experimental, and currently seems 
> to fail when editing commonly used header files for example
>
> Full feature list: 
> https://clang.llvm.org/extra/clangd/Features.html#complete-list-of-features
>
> rtags
> This is currently the most feature-complete indexer I think. But the VS Code 
> integration is a bit minimal and not part of the rtags project itself, and it 
> is missing things like indexer progress.
> + The indexer is actively developed and has been around for quite some time
> - You will probably have to build the indexer from source
> + Full support for call hierarchies and find references
> - VS Code integration a bit limited, missing convenience features like switch 
> between header/source file, showing method/class documentation on hover.
>
> There are even more indexers of course, a promising one used to be “cquery", 
> but that project seems to be defunct now. It lives on in the “ccls" indexer, 
> but ithat one is a bit tricky to configure unless you use clang for building 
> the JDK as well.
>
> So in summary, after the summer the default indexer might be the obvious best 
> choice, but right now it depends on which features you think are the most 
> important I guess..
>

Hi Robin,

thanks a lot for the great summary! Incidentally I've just did some
Google research as well and basically came to the same conclusion.
"Cquery" seemed quite promising and also pretends to have call
hierarchies. But it seems to be defunct and I've also found some bug
reports about problems with the call hierarchies.

For me "Called Hierarchy", and "Find References" (in this order) are
really the most important IDE features. I'm using Eclipse and if you
setup your HotSpot project correctly, these features work pretty well
and reliably.

Have you personally tried the rtags "call hierarchies" / "find
references" support in VSC? From your summary it sounds like it's
worth giving it a try.

Thanks again for caring about these topics! Hotspot development with
IDE support has always been a pain and every improvement in this area
will be highly welcome :)

Best regards,
Volker

> Best regards,
> Robin
>
> >
> > Regards,
> > Volker
> >
> > On Mon, May 27, 2019 at 6:03 PM Robin Westberg
> >  wrote:
> >>
> >> Hi all,
> >>
> >> Please review this change that adds build system support for generating a 
> >> Visual Studio Code workspace configured for working with the JDK native 
> >> code. It configures the default C/C++ IntelliSense Engine to allow code 
> >> completion/navigation and similar features. It also configures two 
> >> executable targets (gtestLauncher and java) that can be built and debugged 
> >> from the IDE.
> >>
> >> The main target is "make vscode-project”, additional information can be 
> >> found in doc/ide.[md|html].
> >>
> >> Issue:
> >> https://bugs.openjdk.java.net/browse/JDK-8223678
> >>
> >> Webrev:
> >> https://cr.openjdk.java.net/~rwestberg/8223678/webrev.01/
> >>
> >> Testing:
> >> Manual testing on Linux, MacOS and Windows
> >>
> >> Thanks Erik Joelsson for taking a look at an earlier version of it!
> >>
> >> Best regards,
> >> Robin
>


Re: RFR: 8223678: Add Visual Studio Code workspace generation support (for native code)

2019-05-29 Thread Robin Westberg
Hi Erik,

Thanks for taking a look!

> On 28 May 2019, at 15:52, Erik Joelsson  wrote:
> 
> Hello Robin,
> 
> Looks good.
> 
> Adding of ide.md is very nice, but could you try to format it like testing.md 
> and building.md in regards to line lengths? I believe we are trying for 80 
> chars in those to make them read reasonably in a standard editor.

Sure, did a bit of reflowing and updated some formatting in order to look more 
like the other .md files.

Webrevs:
 - Full: https://cr.openjdk.java.net/~rwestberg/8223678/webrev.02/
 - Inc: https://cr.openjdk.java.net/~rwestberg/8223678/webrev.01-02/

Best regards,
Robin

> 
> /Erik
> 
> On 2019-05-27 09:03, Robin Westberg wrote:
>> Hi all,
>> 
>> Please review this change that adds build system support for generating a 
>> Visual Studio Code workspace configured for working with the JDK native 
>> code. It configures the default C/C++ IntelliSense Engine to allow code 
>> completion/navigation and similar features. It also configures two 
>> executable targets (gtestLauncher and java) that can be built and debugged 
>> from the IDE.
>> 
>> The main target is "make vscode-project”, additional information can be 
>> found in doc/ide.[md|html].
>> 
>> Issue:
>> https://bugs.openjdk.java.net/browse/JDK-8223678
>> 
>> Webrev:
>> https://cr.openjdk.java.net/~rwestberg/8223678/webrev.01/
>> 
>> Testing:
>> Manual testing on Linux, MacOS and Windows
>> 
>> Thanks Erik Joelsson for taking a look at an earlier version of it!
>> 
>> Best regards,
>> Robin



Re: RFR: 8223678: Add Visual Studio Code workspace generation support (for native code)

2019-05-29 Thread Robin Westberg
Hi Volker,

> On 28 May 2019, at 17:33, Volker Simonis  wrote:
> 
> Hi Robin,
> 
> thanks a lot for doing this!
> 
> I have just a quick question. Do you know if any of the VSC indexers
> (default, clangd) support call hierarchies (i.e. "called by",
> "callers" of a function/method) and "used by" for variables/class
> fields?

Sure, I can make a quick summary of the various pros and cons of the indexers 
that I’ve found so far. They are all moving pretty fast though, so didn’t think 
it was a good fit for the documentation file.

In general, VS Code itself only just recently gained proper support for 
displaying call hierarchies: 
https://code.visualstudio.com/updates/v1_33#_call-hierarchy - but alternative 
indexers have worked around this by showing them differently. 

Default (Microsoft - C/C++ for Visual Studio Code)
+ Easy to setup, as no additional dependencies are needed
+ Good “go to definition”, the only one that can make sense of the 
template+macro stuff in log.hpp for example
- Find references (used by) not done yet (said to be coming in the June update: 
https://github.com/microsoft/vscode-cpptools/issues/15)
- Call hierarchies also not there (scheduled for September apparently: 
https://github.com/microsoft/vscode-cpptools/issues/16)

clangd
+ Actively developed and part of the llvm/clang project
+ Support for find references
- ..but not call hierarchies yet
- Full project indexing is still flagged as experimental, and currently seems 
to fail when editing commonly used header files for example

Full feature list: 
https://clang.llvm.org/extra/clangd/Features.html#complete-list-of-features

rtags
This is currently the most feature-complete indexer I think. But the VS Code 
integration is a bit minimal and not part of the rtags project itself, and it 
is missing things like indexer progress.
+ The indexer is actively developed and has been around for quite some time
- You will probably have to build the indexer from source
+ Full support for call hierarchies and find references
- VS Code integration a bit limited, missing convenience features like switch 
between header/source file, showing method/class documentation on hover.

There are even more indexers of course, a promising one used to be “cquery", 
but that project seems to be defunct now. It lives on in the “ccls" indexer, 
but ithat one is a bit tricky to configure unless you use clang for building 
the JDK as well.

So in summary, after the summer the default indexer might be the obvious best 
choice, but right now it depends on which features you think are the most 
important I guess..

Best regards,
Robin

> 
> Regards,
> Volker
> 
> On Mon, May 27, 2019 at 6:03 PM Robin Westberg
>  wrote:
>> 
>> Hi all,
>> 
>> Please review this change that adds build system support for generating a 
>> Visual Studio Code workspace configured for working with the JDK native 
>> code. It configures the default C/C++ IntelliSense Engine to allow code 
>> completion/navigation and similar features. It also configures two 
>> executable targets (gtestLauncher and java) that can be built and debugged 
>> from the IDE.
>> 
>> The main target is "make vscode-project”, additional information can be 
>> found in doc/ide.[md|html].
>> 
>> Issue:
>> https://bugs.openjdk.java.net/browse/JDK-8223678
>> 
>> Webrev:
>> https://cr.openjdk.java.net/~rwestberg/8223678/webrev.01/
>> 
>> Testing:
>> Manual testing on Linux, MacOS and Windows
>> 
>> Thanks Erik Joelsson for taking a look at an earlier version of it!
>> 
>> Best regards,
>> Robin



RFR: 8224851: AArch64: fix warnings and errors with Clang and GCC 8.3

2019-05-29 Thread Andrew Haley
On 5/29/19 11:26 AM, Nick Gasson wrote:

> Please review this set of fixes to building hotspot with GCC 8.3 or 
> Clang 7.0 on AArch64.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8224851
> Webrev: http://cr.openjdk.java.net/~ngasson/8224851/webrev.0/
> 
> Ran jtreg with no new failures.

Oh dear. Experience has shown me (and probably you, too) that this is
one of the most error-prone parts of software development. Many
very serious failures have been caused by trying to shut up
compilers. These failures have even included major security breaches.

With that, let's press on with a heavy heart...

> 
> This first one is with GCC 8.3:
> 
> /home/nicgas01/jdk/src/hotspot/cpu/aarch64/frame_aarch64.cpp: In function 
> 'void pf(long unsigned int, long unsigned int, long unsigned int, long 
> unsigned int, long unsigned int)':
> /home/nicgas01/jdk/src/hotspot/cpu/aarch64/frame_aarch64.cpp:775:35: error: 
> 'void* memcpy(void*, const void*, size_t)' writing to an object of type 
> 'class RegisterMap' with no trivial copy-assignment; use copy-assignment or 
> copy-initialization instead [-Werror=class-memaccess]
>memcpy(reg_map, , sizeof map);
>^
> 
> RegisterMap is non-POD because its base class AllocatedObj has virtual
> methods, so we need to use copy-assignment rather than memcpy. But
> because we're allocating reg_map with os::malloc we ought to use
> placement new to properly construct it first. But we can't do that
> because RegisterMap inherits from StackObj which disallows new. So I
> just put in some casts to void * to silence the warning.

We can't do that. It seems to me that this must be Undefined
Behaviour, and we must never attempt to cover up UB with casts.

> I can't see where `pf' and `internal_pf' are used - perhaps they can be
> called manually from the debugger?

They're debugger commands. That's why they have external C linkage.

> If no one uses them maybe they should be deleted?

Perhaps so. We have the debugger command pfl() which does all the
frame printing I find I ever need.

> The remaining warnings / errors are with Clang 7.0.
> 
> /home/nicgas01/jdk/src/hotspot/cpu/aarch64/assembler_aarch64.hpp:279:22: 
> error: & has lower precedence than ==; == will be evaluated first 
> [-Werror,-Wparentheses]
> assert_cond(bits & mask == mask);
>  ^~
> 
> To preserve the behaviour we should write `bits & (mask == mask)' but I
> don't think this was what was intended so I changed it to
> `(bits & mask) == mask'.

That's right. It's supposed to check that the field being read from an
instruction has already been set.

> /home/nicgas01/jdk/src/hotspot/cpu/aarch64/interp_masm_aarch64.hpp:44:25: 
> error: redeclaration of using declaration
>   using MacroAssembler::call_VM_leaf_base;
> ^
> 
> This using declaration appears twice in a row.

OK.

> /home/nicgas01/jdk/src/hotspot/cpu/aarch64/aarch64.ad:13866:80: error: 
> invalid suffix 'D' on floating constant
> __ fcmps(as_FloatRegister(opnd_array(1)->reg(ra_,this,idx1)/* src1 */), 
> 0.0D);
>   
>  ^
> 
> The "d" or "D" suffix on floating point literals is not in C++98. I
> believe it comes from an extension to C to support decimal floating
> point? Anyway it's not necessary here.

OK.

> /home/nicgas01/jdk/src/hotspot/cpu/aarch64/c1_LIRGenerator_aarch64.cpp:429:66:
>  error: implicit conversion of NULL constant to 'bool' 
> [-Werror,-Wnull-conversion]
>   arithmetic_op_fpu(x->op(), reg, left.result(), right.result(), NULL);
>   ~  ^~~~
>  false
>
> The NULL here is for the `bool is_strictfp' argument to
> LIRGenerator::arithmetic_op_fpu. Changing it to `false' would preserve
> the existing behaviour but it seems like it should be `x->is_strictfp()'
> to match other platforms.

OK, use x->is_strictfp(). I don't think that it can possibly make any
difference on AArch64.

> As a consequence of this we need to handle the lir_{div,mul}_scriptfp
> opcodes in LIR_Assembler::arith_op, but these just fall through to the
> non-strictfp variants so there should be no behaviour change.

Yep.

> /home/nicgas01/jdk/src/hotspot/cpu/aarch64/c1_LIRAssembler_aarch64.cpp:1074:24:
>  error: '&&' within '||' [-Werror,-Wlogical-op-parentheses]
>   if (is_unordered && op->cond() == lir_cond_equal
>   ~^~~
> 
> Added parenthesis to make this explicit. No behaviour change.

OK. I don't quite get what this is for, but I guess it shuts the
compiler up and is harmless.

> /home/nicgas01/jdk/src/hotspot/os_cpu/linux_aarch64/copy_linux_aarch64.s:162:32:
>  error: index must be a multiple of 8 in range [0, 32760].
> prfmpldl1keep, [s, #-256]
>^
> 
> The immediate offset in `prfm' 

Re: RFR: 8224851: AArch64: fix warnings and errors with Clang and GCC 8.3

2019-05-29 Thread Andrew Haley
On 5/29/19 11:26 AM, Nick Gasson wrote:

> Please review this set of fixes to building hotspot with GCC 8.3 or 
> Clang 7.0 on AArch64.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8224851
> Webrev: http://cr.openjdk.java.net/~ngasson/8224851/webrev.0/
> 
> Ran jtreg with no new failures.

Oh dear. Experience has shown me (and probably you, too) that this is
one of the most error-prone parts of software development. Many
very serious failures have been recorded when trying to shut up
compilers. These failures have even included major security breaches.

With that, let's press on with a heavy heart...

> 
> This first one is with GCC 8.3:
> 
> /home/nicgas01/jdk/src/hotspot/cpu/aarch64/frame_aarch64.cpp: In function 
> 'void pf(long unsigned int, long unsigned int, long unsigned int, long 
> unsigned int, long unsigned int)':
> /home/nicgas01/jdk/src/hotspot/cpu/aarch64/frame_aarch64.cpp:775:35: error: 
> 'void* memcpy(void*, const void*, size_t)' writing to an object of type 
> 'class RegisterMap' with no trivial copy-assignment; use copy-assignment or 
> copy-initialization instead [-Werror=class-memaccess]
>memcpy(reg_map, , sizeof map);
>^
> 
> RegisterMap is non-POD because its base class AllocatedObj has virtual
> methods, so we need to use copy-assignment rather than memcpy. But
> because we're allocating reg_map with os::malloc we ought to use
> placement new to properly construct it first. But we can't do that
> because RegisterMap inherits from StackObj which disallows new. So I
> just put in some casts to void * to silence the warning.

We can't do that. It seems to me that this must be Undefined
Behaviour, and we must never attempt to cover up UB with casts.

> I can't see where `pf' and `internal_pf' are used - perhaps they can be
> called manually from the debugger?

They're debugger commands. That's why they have external C linkage.

> If no one uses them maybe they should be deleted?

Perhaps so. We have the debugger command pfl() which does all the
frame printing I find I ever need.

> The remaining warnings / errors are with Clang 7.0.
> 
> /home/nicgas01/jdk/src/hotspot/cpu/aarch64/assembler_aarch64.hpp:279:22: 
> error: & has lower precedence than ==; == will be evaluated first 
> [-Werror,-Wparentheses]
> assert_cond(bits & mask == mask);
>  ^~
> 
> To preserve the behaviour we should write `bits & (mask == mask)' but I
> don't think this was what was intended so I changed it to
> `(bits & mask) == mask'.

That's right. It's supposed to check that the field being read from an
instruction has already been set.

> /home/nicgas01/jdk/src/hotspot/cpu/aarch64/interp_masm_aarch64.hpp:44:25: 
> error: redeclaration of using declaration
>   using MacroAssembler::call_VM_leaf_base;
> ^
> 
> This using declaration appears twice in a row.

OK.

> /home/nicgas01/jdk/src/hotspot/cpu/aarch64/aarch64.ad:13866:80: error: 
> invalid suffix 'D' on floating constant
> __ fcmps(as_FloatRegister(opnd_array(1)->reg(ra_,this,idx1)/* src1 */), 
> 0.0D);
>   
>  ^
> 
> The "d" or "D" suffix on floating point literals is not in C++98. I
> believe it comes from an extension to C to support decimal floating
> point? Anyway it's not necessary here.

OK.

> /home/nicgas01/jdk/src/hotspot/cpu/aarch64/c1_LIRGenerator_aarch64.cpp:429:66:
>  error: implicit conversion of NULL constant to 'bool' 
> [-Werror,-Wnull-conversion]
>   arithmetic_op_fpu(x->op(), reg, left.result(), right.result(), NULL);
>   ~  ^~~~
>  false
>
> The NULL here is for the `bool is_strictfp' argument to
> LIRGenerator::arithmetic_op_fpu. Changing it to `false' would preserve
> the existing behaviour but it seems like it should be `x->is_strictfp()'
> to match other platforms.

OK, use x->is_strictfp(). I don't think that it can possibly make any
difference on AArch64.

> As a consequence of this we need to handle the lir_{div,mul}_scriptfp
> opcodes in LIR_Assembler::arith_op, but these just fall through to the
> non-strictfp variants so there should be no behaviour change.

Yep.

> /home/nicgas01/jdk/src/hotspot/cpu/aarch64/c1_LIRAssembler_aarch64.cpp:1074:24:
>  error: '&&' within '||' [-Werror,-Wlogical-op-parentheses]
>   if (is_unordered && op->cond() == lir_cond_equal
>   ~^~~
> 
> Added parenthesis to make this explicit. No behaviour change.

OK. I don't quite get what this is for, but I guess it shuts the
compiler up.

> /home/nicgas01/jdk/src/hotspot/os_cpu/linux_aarch64/copy_linux_aarch64.s:162:32:
>  error: index must be a multiple of 8 in range [0, 32760].
> prfmpldl1keep, [s, #-256]
>^
> 
> The immediate offset in `prfm' gets 

RFR: 8224851: AArch64: fix warnings and errors with Clang and GCC 8.3

2019-05-29 Thread Nick Gasson
Hi,

Please review this set of fixes to building hotspot with GCC 8.3 or 
Clang 7.0 on AArch64.

Bug: https://bugs.openjdk.java.net/browse/JDK-8224851
Webrev: http://cr.openjdk.java.net/~ngasson/8224851/webrev.0/

Ran jtreg with no new failures.

This first one is with GCC 8.3:

/home/nicgas01/jdk/src/hotspot/cpu/aarch64/frame_aarch64.cpp: In function 'void 
pf(long unsigned int, long unsigned int, long unsigned int, long unsigned int, 
long unsigned int)':
/home/nicgas01/jdk/src/hotspot/cpu/aarch64/frame_aarch64.cpp:775:35: error: 
'void* memcpy(void*, const void*, size_t)' writing to an object of type 'class 
RegisterMap' with no trivial copy-assignment; use copy-assignment or 
copy-initialization instead [-Werror=class-memaccess]
   memcpy(reg_map, , sizeof map);
   ^

RegisterMap is non-POD because its base class AllocatedObj has virtual
methods, so we need to use copy-assignment rather than memcpy. But
because we're allocating reg_map with os::malloc we ought to use
placement new to properly construct it first. But we can't do that
because RegisterMap inherits from StackObj which disallows new. So I
just put in some casts to void * to silence the warning.

I can't see where `pf' and `internal_pf' are used - perhaps they can be
called manually from the debugger? If no one uses them maybe they should
be deleted?

The remaining warnings / errors are with Clang 7.0.

/home/nicgas01/jdk/src/hotspot/cpu/aarch64/assembler_aarch64.hpp:279:22: error: 
& has lower precedence than ==; == will be evaluated first 
[-Werror,-Wparentheses]
assert_cond(bits & mask == mask);
 ^~

To preserve the behaviour we should write `bits & (mask == mask)' but I
don't think this was what was intended so I changed it to
`(bits & mask) == mask'.

/home/nicgas01/jdk/src/hotspot/cpu/aarch64/interp_masm_aarch64.hpp:44:25: 
error: redeclaration of using declaration
  using MacroAssembler::call_VM_leaf_base;
^

This using declaration appears twice in a row.

/home/nicgas01/jdk/src/hotspot/cpu/aarch64/aarch64.ad:13866:80: error: invalid 
suffix 'D' on floating constant
__ fcmps(as_FloatRegister(opnd_array(1)->reg(ra_,this,idx1)/* src1 */), 
0.0D);
   ^

The "d" or "D" suffix on floating point literals is not in C++98. I
believe it comes from an extension to C to support decimal floating
point? Anyway it's not necessary here.

/home/nicgas01/jdk/src/hotspot/cpu/aarch64/c1_LIRGenerator_aarch64.cpp:429:66: 
error: implicit conversion of NULL constant to 'bool' 
[-Werror,-Wnull-conversion]
  arithmetic_op_fpu(x->op(), reg, left.result(), right.result(), NULL);
  ~  ^~~~
 false

The NULL here is for the `bool is_strictfp' argument to
LIRGenerator::arithmetic_op_fpu. Changing it to `false' would preserve
the existing behaviour but it seems like it should be `x->is_strictfp()'
to match other platforms.

As a consequence of this we need to handle the lir_{div,mul}_scriptfp
opcodes in LIR_Assembler::arith_op, but these just fall through to the
non-strictfp variants so there should be no behaviour change.

/home/nicgas01/jdk/src/hotspot/cpu/aarch64/c1_LIRAssembler_aarch64.cpp:1074:24: 
error: '&&' within '||' [-Werror,-Wlogical-op-parentheses]
  if (is_unordered && op->cond() == lir_cond_equal
  ~^~~

Added parenthesis to make this explicit. No behaviour change.

/home/nicgas01/jdk/src/hotspot/os_cpu/linux_aarch64/copy_linux_aarch64.s:162:32:
 error: index must be a multiple of 8 in range [0, 32760].
prfmpldl1keep, [s, #-256]
   ^

The immediate offset in `prfm' gets zero-extended to 64-bits and then
left shifted three places so can only be unsigned. GNU as actually
assembles [s, #-256] the same as [s]:

   0:   f980prfmpldl1keep, [x0]

Seems like GNU as ought to report an error here instead. I've replaced
this with `prfum' which has a sign-extended offset.

/home/nicgas01/jdk/src/hotspot/os_cpu/linux_aarch64/atomic_linux_aarch64.hpp:43:39:
 error: cannot initialize a parameter of type 'char *' with an lvalue of type 
'unsigned long'
return __sync_add_and_fetch(dest, add_value);
  ^

If the add_and_fetch template is instantiated with I=integer and
D=pointer then we need an explicit cast here.

/home/nicgas01/jdk/src/hotspot/os_cpu/linux_aarch64/os_linux_aarch64.cpp:679:8: 
error: conflicting types for '_Copy_conjoint_jshorts_atomic'
  void _Copy_conjoint_jshorts_atomic(jshort* from, jshort* to, size_t count) {
   ^

This family of functions is declared with const `from' pointer but in
the definition it is non-const. Made it const everywhere.


Re: Failure buiilding OPEN JDK 11 on windows cygwin with VC 2017

2019-05-29 Thread Moshe Zuisman
I am using  http://hg.openjdk.java.net/jdk-updates/jdk11u/rev/a23d4b4ea281


ср, 29 мая 2019 г. в 08:35, David Holmes :

> Hi Moshe,
>
> On 29/05/2019 2:15 am, Moshe Zuisman wrote:
> > I am trying to build OPEN jdk 11 on cygwin with MSVC 2017
> > _
>
> VS 2017 is the official supported compiler for building OpenJDK 11 [1]
> and we don't see any issues. What version of the 11u source are you
> trying to compile?
>
> David
>
> [1] https://wiki.openjdk.java.net/display/Build/Supported+Build+Platforms
>
> >
> >
> *c:\progra~2\mib055~1\2017\profes~1\vc\tools\msvc\1410~1.250\include\exception(366):
> > warning C4577: 'noexcept' used with no exception handling mode specified;
> > termination on exception is not guaranteed. Specify /EHscmake[3]: ***
> > [lib/CompileJvm.gmk:151:
> >
> /cygdrive/c/Git_Repos/open_jdk_11/jdk11-1ddf9a99e4ad/jdk11-1ddf9a99e4ad/build/windows-x86_64-normal-server-release/hotspot/variant-server/libjvm/objs/BUILD_LIBJVM_pch.obj]
> > Error 1make[3]: *** Waiting for unfinished jobsmake[2]: ***
> > [make/Main.gmk:257: hotspot-server-libs] Error 2make[2]: *** Waiting for
> > unfinished jobsERROR: Build failed for target 'default
> > (exploded-image)' in configuration 'windows-x86_64-normal-server-release'
> > (exit code 2)=== Output from failing command(s) repeated here ===* For
> > target
> >
> hotspot_variant-server_libjvm_objs_BUILD_LIBJVM_pch.obj:BUILD_LIBJVM_pch.cppc:\progra~2\mib055~1\2017\profes~1\vc\tools\msvc\1410~1.250\include\exception(366):
> > error C2220: warning treated as error - no 'object' file
> >
> generatedc:\progra~2\mib055~1\2017\profes~1\vc\tools\msvc\1410~1.250\include\exception(366):
> > warning C4577: 'noexcept' used with no exception handling mode specified;
> > termination on exception is not guaranteed. Specify /EHsc   ... (rest of
> > output omitted)* All command lines available in
> >
> /cygdrive/c/Git_Repos/open_jdk_11/jdk11-1ddf9a99e4ad/jdk11-1ddf9a99e4ad/build/windows-x86_64-normal-server-release/make-support/failure-logs.===
> > End of repeated output ===No indication of failed target found.Hint: Try
> > searching the build log for '] Error'.Hint: See
> > doc/building.html#troubleshooting for assistance.make[1]: ***
> >
> [/cygdrive/c/Git_Repos/open_jdk_11/jdk11-1ddf9a99e4ad/jdk11-1ddf9a99e4ad/make/Init.gmk:305:
> > main] Error 2make: ***
> >
> [/cygdrive/c/Git_Repos/open_jdk_11/jdk11-1ddf9a99e4ad/jdk11-1ddf9a99e4ad/make/Init.gmk:186:
> > default] Error 2*
> >
>