Re: RFR: 8286122: [macos]: App bundle cannot upload to Mac App Store due to info.plist embedded in java exe [v2]

2022-05-17 Thread Kevin Rushforth
On Thu, 12 May 2022 04:15:50 GMT, Alexander Matveev  
wrote:

>> - It is not possible to support native JDK commands such as "java" inside 
>> Mac App Store bundles due to embedded info.plist. Workarounds suggested in 
>> JDK-8286122 does not seems to be visible.
>>  - With proposed fix we will enforce "--strip-native-commands" option for 
>> jlink, so native JDK commands are not included when generating Mac App Store 
>> bundles.
>>  - Custom runtime provided via --runtime-image should not contain native 
>> commands as well, otherwise jpackage will throw error.
>>  - Added two tests to validate fix.
>
> Alexander Matveev has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8286122: [macos]: App bundle cannot upload to Mac App Store due to 
> info.plist embedded in java exe [v2]

I'm just catching up with this thread. Based on the analysis, it's pretty clear 
that any solution that actually allows bundling native launchers is going to 
take more research, and is out of scope for this bug. Some combination of 
[JDK-8286850](https://bugs.openjdk.java.net/browse/JDK-8286850) and/or 
providing Java launchers that don't have an `Info.plist` is likely needed to 
provide a complete solution.

Given that, I think that this PR seems a reasonable fix to avoid creating an 
app bundle that can't be uploaded to the app store.

-

Marked as reviewed by kcr (Author).

PR: https://git.openjdk.java.net/jdk/pull/8666


Re: RFR: 8286562: GCC 12 reports some compiler warnings [v5]

2022-05-17 Thread Yasumasa Suenaga
On Tue, 17 May 2022 04:52:44 GMT, Kim Barrett  wrote:

>> src/hotspot/share/opto/memnode.cpp line 1413:
>> 
>>> 1411:bt == T_BYTE|| bt == T_SHORT ||
>>> 1412:bt == T_INT || bt == T_LONG, "wrong type = 
>>> %s", type2name(bt));
>>> 1413: PRAGMA_DIAG_POP
>> 
>> The problem here is the definition of type2name, which returns NULL for an 
>> unknown BasicType.  It would probably be better if it returned a 
>> recognizable string for that situation, eliminating this warning at it's 
>> source.
>
> While looking at type2name, I noticed the comment for the immediately 
> preceding type2name_tab is wrong.

The warning has gone in new commit, and I fixed the comment for `type2name_tab` 
in it.

-

PR: https://git.openjdk.java.net/jdk/pull/8646


Re: RFR: 8286562: GCC 12 reports some compiler warnings [v5]

2022-05-17 Thread Yasumasa Suenaga
On Tue, 17 May 2022 03:06:49 GMT, Kim Barrett  wrote:

>> Yasumasa Suenaga has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Revert change for java.c , parse_manifest.c , LinuxPackage.c
>
> src/hotspot/share/opto/type.cpp line 4300:
> 
>> 4298: PRAGMA_FORMAT_OVERFLOW_IGNORED
>> 4299:   fatal("not an element type: %s", type2name(etype));
>> 4300: PRAGMA_DIAG_POP
> 
> Another occurrence of type2name returning NULL for unknown BasicType.

The warning has gone in new commit due to change of `type2name`.

-

PR: https://git.openjdk.java.net/jdk/pull/8646


Re: RFR: 8286562: GCC 12 reports some compiler warnings [v7]

2022-05-17 Thread Yasumasa Suenaga
> I saw some compiler warnings when I tried to build OpenJDK with GCC 12.0.1 on 
> Fedora 36.
> As you can see, the warnings spreads several areas. Let me know if I should 
> separate them by area.
> 
> * -Wstringop-overflow
> * src/hotspot/share/oops/array.hpp
> * 
> src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdBits.inline.hpp
> 
> In member function 'void Array::at_put(int, const T&) [with T = unsigned 
> char]',
> inlined from 'void ConstantPool::tag_at_put(int, jbyte)' at 
> /home/ysuenaga/github-forked/jdk/src/hotspot/share/oops/constantPool.hpp:126:64,
> inlined from 'void ConstantPool::method_at_put(int, int, int)' at 
> /home/ysuenaga/github-forked/jdk/src/hotspot/share/oops/constantPool.hpp:380:15,
> inlined from 'ConstantPool* 
> BytecodeConstantPool::create_constant_pool(JavaThread*) const' at 
> /home/ysuenaga/github-forked/jdk/src/hotspot/share/classfile/bytecodeAssembler.cpp:85:26:

Yasumasa Suenaga has updated the pull request incrementally with one additional 
commit since the last revision:

  revert changes for memnode.cpp and type.cpp

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8646/files
  - new: https://git.openjdk.java.net/jdk/pull/8646/files/73c306d7..88cbf46d

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=8646=06
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8646=05-06

  Stats: 7 lines in 2 files changed: 0 ins; 7 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8646.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8646/head:pull/8646

PR: https://git.openjdk.java.net/jdk/pull/8646


Re: RFR: 8286562: GCC 12 reports some compiler warnings [v5]

2022-05-17 Thread Yasumasa Suenaga
On Tue, 17 May 2022 03:14:05 GMT, Kim Barrett  wrote:

>> Yasumasa Suenaga has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Revert change for java.c , parse_manifest.c , LinuxPackage.c
>
> src/hotspot/share/classfile/bytecodeAssembler.cpp line 95:
> 
>> 93: ShouldNotReachHere();
>> 94: }
>> 95: PRAGMA_DIAG_POP
> 
> One of these is another of the symbol_at_put cases that I don't understand.  
> Are there other cases in the switch that warn?  And if so, which ones and 
> how?  There are no details of this one in the PR cover description.

Most of switch cases are warned. Please see 
[logs](https://github.com/openjdk/jdk/files/8708578/hotspot_variant-server_libjvm_objs_bytecodeAssembler.o.log)

-

PR: https://git.openjdk.java.net/jdk/pull/8646


Re: RFR: 8286562: GCC 12 reports some compiler warnings [v6]

2022-05-17 Thread Yasumasa Suenaga
> I saw some compiler warnings when I tried to build OpenJDK with GCC 12.0.1 on 
> Fedora 36.
> As you can see, the warnings spreads several areas. Let me know if I should 
> separate them by area.
> 
> * -Wstringop-overflow
> * src/hotspot/share/oops/array.hpp
> * 
> src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdBits.inline.hpp
> 
> In member function 'void Array::at_put(int, const T&) [with T = unsigned 
> char]',
> inlined from 'void ConstantPool::tag_at_put(int, jbyte)' at 
> /home/ysuenaga/github-forked/jdk/src/hotspot/share/oops/constantPool.hpp:126:64,
> inlined from 'void ConstantPool::method_at_put(int, int, int)' at 
> /home/ysuenaga/github-forked/jdk/src/hotspot/share/oops/constantPool.hpp:380:15,
> inlined from 'ConstantPool* 
> BytecodeConstantPool::create_constant_pool(JavaThread*) const' at 
> /home/ysuenaga/github-forked/jdk/src/hotspot/share/classfile/bytecodeAssembler.cpp:85:26:

Yasumasa Suenaga has updated the pull request with a new target base due to a 
merge or a rebase. The pull request now contains seven commits:

 - Add assert to check the range of BasicType
 - Merge remote-tracking branch 'upstream/master' into HEAD
 - Revert change for java.c , parse_manifest.c , LinuxPackage.c
 - Calculate char offset before realloc()
 - Use return value from JLI_Snprintf
 - Avoid pragma error in before GCC 12
 - 8286562: GCC 12 reports some compiler warnings

-

Changes: https://git.openjdk.java.net/jdk/pull/8646/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8646=05
  Stats: 56 lines in 11 files changed: 46 ins; 1 del; 9 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8646.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8646/head:pull/8646

PR: https://git.openjdk.java.net/jdk/pull/8646


Re: RFR: 8286562: GCC 12 reports some compiler warnings [v5]

2022-05-17 Thread Yasumasa Suenaga
On Tue, 17 May 2022 03:02:55 GMT, Kim Barrett  wrote:

>> Yasumasa Suenaga has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Revert change for java.c , parse_manifest.c , LinuxPackage.c
>
> src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdBits.inline.hpp
>  line 103:
> 
>> 101: PRAGMA_STRINGOP_OVERFLOW_IGNORED
>> 102:   *dest = op(bits, *dest);
>> 103: PRAGMA_DIAG_POP
> 
> I see no stringop here.  I'm still trying to understand what the compiler is 
> complaining about.

I guess GCC cannot understand `assert(dest != NULL` immediately preceding it.


In file included from 
/home/ysuenaga/github-forked/jdk/src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdLoadBarrier.inline.hpp:33,
 from 
/home/ysuenaga/github-forked/jdk/src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceId.inline.hpp:30,
 from 
/home/ysuenaga/github-forked/jdk/src/hotspot/share/jfr/support/jfrJdkJfrEvent.cpp:30:
In function 'void set_form(jbyte, jbyte*) [with jbyte (* op)(jbyte, jbyte) = 
traceid_or]',
inlined from 'void set(jbyte, jbyte*)' at 
/home/ysuenaga/github-forked/jdk/src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdBits.inline.hpp:129:23,
inlined from 'static void JfrTraceIdBits::store(jbyte, const T*) [with T = 
Klass]' at 
/home/ysuenaga/github-forked/jdk/src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdBits.inline.hpp:135:6,
inlined from 'static void JfrTraceId::tag_as_jdk_jfr_event(const Klass*)' 
at 
/home/ysuenaga/github-forked/jdk/src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceId.inline.hpp:106:3,
inlined from 'static void JdkJfrEvent::tag_as(const Klass*)' at 
/home/ysuenaga/github-forked/jdk/src/hotspot/share/jfr/support/jfrJdkJfrEvent.cpp:176:35:

-

PR: https://git.openjdk.java.net/jdk/pull/8646


Re: RFR: 8286562: GCC 12 reports some compiler warnings [v5]

2022-05-17 Thread Yasumasa Suenaga
On Tue, 17 May 2022 01:43:25 GMT, Kim Barrett  wrote:

>> Yasumasa Suenaga has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Revert change for java.c , parse_manifest.c , LinuxPackage.c
>
> src/hotspot/share/classfile/classFileParser.cpp line 5970:
> 
>> 5968: PRAGMA_STRINGOP_OVERFLOW_IGNORED
>> 5969:   _cp->symbol_at_put(hidden_index, _class_name);
>> 5970: PRAGMA_DIAG_POP
> 
> I don't understand these warning suppressions for symbol_at_put (here and 
> elsewhere).  I don't see any stringops here.  What is the compiler 
> complaining about?  (There's no mention of classfile stuff in the review 
> cover message.)

Like the others, it is caused by `Array::at_put()`.


In file included from 
/home/ysuenaga/github-forked/jdk/src/hotspot/share/oops/annotations.hpp:28,
 from 
/home/ysuenaga/github-forked/jdk/src/hotspot/share/oops/instanceKlass.hpp:29,
 from 
/home/ysuenaga/github-forked/jdk/src/hotspot/share/classfile/javaClasses.hpp:30,
 from 
/home/ysuenaga/github-forked/jdk/src/hotspot/share/precompiled/precompiled.hpp:35:
In member function 'void Array::at_put(int, const T&) [with T = unsigned 
char]',
inlined from 'void ConstantPool::tag_at_put(int, jbyte)' at 
/home/ysuenaga/github-forked/jdk/src/hotspot/share/oops/constantPool.hpp:126:64,
inlined from 'void ConstantPool::symbol_at_put(int, Symbol*)' at 
/home/ysuenaga/github-forked/jdk/src/hotspot/share/oops/constantPool.hpp:362:15,
inlined from 'void 
ClassFileParser::mangle_hidden_class_name(InstanceKlass*)' at 
/home/ysuenaga/github-forked/jdk/src/hotspot/share/classfile/classFileParser.cpp:5966:21:

-

PR: https://git.openjdk.java.net/jdk/pull/8646


Re: RFR: 8286262: Windows: Cleanup deprecation warning suppression

2022-05-17 Thread David Holmes
On Sun, 15 May 2022 20:50:25 GMT, Kim Barrett  wrote:

> Please review this cleanup of deprecation warning suppression when building
> for Windows.
> 
> This change consists of several parts.
> 
> (1) Remove the global deprecation warning suppression when building HotSpot
> for Windows.
> 
> (2) Add macro definitions requesting suppression of selected sets of
> deprecation warnings when building HotSpot for Windows.
> 
> (3) Remove unnecessary forwarding macros for various POSIX functions in
> globalDefinitions_visCPP.hpp.  These were provided to avoid deprecation
> warnings (that were previously also being suppressed by the global request).
> They are now covered by the new macros provided by change (2) above.
> 
> An alternative to item (3) is to not define _CRT_NONSTDC_NO_DEPRECATE (in item
> (2)) and either retain the forwarding macros or define os:: wrapper functions
> for all of the affected functions.  We might eventually do the latter because
> of other reasons for avoiding some of these functions, but the approach being
> taken here is simpler.
> 
> For documentation of _CRT_NONSTDC_NO_DEPRECATE, see:
> https://docs.microsoft.com/en-us/cpp/c-runtime-library/compatibility
> https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4996
> 
> Similarly for _CRT_SECURE_NO_WARNINGS.
> 
> Perhaps similarly for _WINSOCK_DEPRECATED_NO_WARNINGS (though I didn't find
> any documentation for the latter).  But it might be better to not supress the
> warnings and instead use the alternatives (JDK-8286781).
> 
> Testing:
> mach5 tier1

Sorry Kim I'm having trouble seeing what change corresponds to (1) ??

Also the PR talks only about hotspot, but you're changing the JDK flags as 
well. ??

-

PR: https://git.openjdk.java.net/jdk/pull/8718