Re: RFR: 8299635: More test issues for deprecated sprintf in Xcode 14 [v4]

2023-01-13 Thread Xue-Lei Andrew Fan
> The sprintf is deprecated in Xcode 14 because of security concerns. The issue > was addressed in [JDK-8296812](https://bugs.openjdk.org/browse/JDK-8296812) > for hotspot impl, and > [JDK-8299378](https://bugs.openjdk.org/browse/JDK-8299378) for building, but > the test case was not covered. T

Re: RFR: 8299635: More test issues for deprecated sprintf in Xcode 14 [v3]

2023-01-13 Thread Xue-Lei Andrew Fan
> The sprintf is deprecated in Xcode 14 because of security concerns. The issue > was addressed in [JDK-8296812](https://bugs.openjdk.org/browse/JDK-8296812) > for hotspot impl, and > [JDK-8299378](https://bugs.openjdk.org/browse/JDK-8299378) for building, but > the test case was not covered. T

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v7]

2023-01-13 Thread Archie L . Cobbs
On Fri, 13 Jan 2023 00:57:14 GMT, Archie L. Cobbs wrote: >> Ok - I thought false negative was the thing to absolutely avoid - and that >> was the no. 1 concern. I think a possible approach to keep both the >> filtering and the code more or less similar to what you have, is to save the >> type

Re: RFR: 8298853: JvmtiVTMSTransitionDisabler should support disabling one virtual thread transitions [v5]

2023-01-13 Thread Serguei Spitsyn
On Wed, 4 Jan 2023 07:45:48 GMT, Serguei Spitsyn wrote: >> src/hotspot/share/prims/jvmtiThreadState.cpp line 482: >> >>> 480: _VTMS_transition_disable_for_all_count > 0) { >>> 481: MonitorLocker ml(JvmtiVTMSTransition_lock, >>> Mutex::_no_safepoint_check_flag); >>> 482: ml.notify_

Re: RFR: 8299635: More test issues for deprecated sprintf in Xcode 14 [v2]

2023-01-13 Thread Mikael Vidstedt
On Fri, 13 Jan 2023 23:27:36 GMT, Xue-Lei Andrew Fan wrote: >> The sprintf is deprecated in Xcode 14 because of security concerns. The >> issue was addressed in >> [JDK-8296812](https://bugs.openjdk.org/browse/JDK-8296812) for hotspot impl, >> and [JDK-8299378](https://bugs.openjdk.org/browse/

Integrated: 8300012: Remove unused JDI VirtualMachineImpl.removeObjectMirror(ObjectReferenceImpl object) method

2023-01-13 Thread Chris Plummer
On Thu, 12 Jan 2023 02:20:04 GMT, Chris Plummer wrote: > `VirtualMachineImpl.removeObjectMirror(ObjectReferenceImpl object)` is not > used. Furthermore it confuses the reader that runs across > `removeObjectMirror()` calls, because what is actually being called is > `removeObjectMirror(SoftObj

Re: RFR: 8299635: More test issues for deprecated sprintf in Xcode 14 [v2]

2023-01-13 Thread Xue-Lei Andrew Fan
> The sprintf is deprecated in Xcode 14 because of security concerns. The issue > was addressed in [JDK-8296812](https://bugs.openjdk.org/browse/JDK-8296812) > for hotspot impl, and > [JDK-8299378](https://bugs.openjdk.org/browse/JDK-8299378) for building, but > the test case was not covered. T

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v10]

2023-01-13 Thread Archie L . Cobbs
> This PR adds a new lint warning category `this-escape`. > > It also adds `@SuppressWarnings` annotations as needed to the JDK itself to > allow the JDK to continue to compile with `-Xlint:all`. > > A 'this' escape warning is generated for a constructor `A()` in a class `A` > when the compiler

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v8]

2023-01-13 Thread Vicente Romero
On Fri, 13 Jan 2023 21:33:02 GMT, Archie L. Cobbs wrote: >> Sounds good - thanks. > > Ah. I just realized that we need to do it your way because of the following > bug: > > Suppose you have a constructor and a field with initializer that both leak, > but you have `@SuppressWarnings("this-escap

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v8]

2023-01-13 Thread Archie L . Cobbs
On Fri, 13 Jan 2023 16:20:41 GMT, Archie L. Cobbs wrote: >> I'm OK either way we can revisit this later either as part of this PR or in >> a future one. I let it to your consideration > > Sounds good - thanks. Ah. I just realized that we need to do it your way because of the following bug: Sup

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v9]

2023-01-13 Thread Archie L . Cobbs
On Fri, 13 Jan 2023 20:21:24 GMT, Vicente Romero wrote: >> Archie L. Cobbs has updated the pull request incrementally with three >> additional commits since the last revision: >> >> - Use more idiomatic test for java.lang.Object. >> - Revert 27cb30129; the error was actually fixed in edf3c3f5

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v9]

2023-01-13 Thread Archie L . Cobbs
On Fri, 13 Jan 2023 20:16:25 GMT, Vicente Romero wrote: >> Archie L. Cobbs has updated the pull request incrementally with three >> additional commits since the last revision: >> >> - Use more idiomatic test for java.lang.Object. >> - Revert 27cb30129; the error was actually fixed in edf3c3f5

Re: RFR: 8300032: DwarfParser resource leak

2023-01-13 Thread Chris Plummer
On Thu, 12 Jan 2023 12:08:51 GMT, Daniel Jeliński wrote: > Please review this fix for DwarfParser cleaner. > > The original code registered the cleaner using a lambda that captured a > reference to the parser object; as a result, the object was never GCed, and > the cleaner never ran. > > In

Re: RFR: 8300032: DwarfParser resource leak

2023-01-13 Thread Chris Plummer
On Fri, 13 Jan 2023 07:45:59 GMT, Daniel Jeliński wrote: > As far as I could tell, the cleaners you pointed out use local variables > only. As long as the lambda does not reference any object fields or instance > methods, `this` is not captured. Ok. So the lambda needs to reference a field of

Re: RFR: 8300024: Replace use of JNI_COMMIT mode with mode 0 [v2]

2023-01-13 Thread Chris Plummer
On Fri, 13 Jan 2023 07:16:35 GMT, Daniel Jeliński wrote: >> Please review this patch that fixes a few memory leaks in JNI code. >> >> [The latest >> documentation](https://docs.oracle.com/en/java/javase/17/docs/specs/jni/functions.html#releaseprimitivetypearrayelements-routines) >> of JNI func

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v9]

2023-01-13 Thread Vicente Romero
On Fri, 13 Jan 2023 19:09:54 GMT, Archie L. Cobbs wrote: >> This PR adds a new lint warning category `this-escape`. >> >> It also adds `@SuppressWarnings` annotations as needed to the JDK itself to >> allow the JDK to continue to compile with `-Xlint:all`. >> >> A 'this' escape warning is gene

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v9]

2023-01-13 Thread Vicente Romero
On Fri, 13 Jan 2023 19:09:54 GMT, Archie L. Cobbs wrote: >> This PR adds a new lint warning category `this-escape`. >> >> It also adds `@SuppressWarnings` annotations as needed to the JDK itself to >> allow the JDK to continue to compile with `-Xlint:all`. >> >> A 'this' escape warning is gene

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v8]

2023-01-13 Thread Vicente Romero
On Fri, 13 Jan 2023 17:49:05 GMT, Archie L. Cobbs wrote: >> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ThisEscapeAnalyzer.java >> line 685: >> >>> 683: >>> 684: @Override >>> 685: public void visitDoLoop(JCDoWhileLoop tree) { >> >> I was thinking, code can also loop using

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v9]

2023-01-13 Thread Archie L . Cobbs
> This PR adds a new lint warning category `this-escape`. > > It also adds `@SuppressWarnings` annotations as needed to the JDK itself to > allow the JDK to continue to compile with `-Xlint:all`. > > A 'this' escape warning is generated for a constructor `A()` in a class `A` > when the compiler

Re: RFR: 8300024: Replace use of JNI_COMMIT mode with mode 0 [v2]

2023-01-13 Thread Serguei Spitsyn
On Fri, 13 Jan 2023 07:16:35 GMT, Daniel Jeliński wrote: >> Please review this patch that fixes a few memory leaks in JNI code. >> >> [The latest >> documentation](https://docs.oracle.com/en/java/javase/17/docs/specs/jni/functions.html#releaseprimitivetypearrayelements-routines) >> of JNI func

Re: RFR: 8299915: Remove ArrayAllocatorMallocLimit and associated code [v8]

2023-01-13 Thread Justin King
On Fri, 13 Jan 2023 17:39:41 GMT, Justin King wrote: >> Remove abstraction that is a holdover from Solaris. Direct usages of >> `MmapArrayAllocator` have been switched to normal `malloc`. The >> justification is that none of the code paths are called from signal >> handlers, so using `mmap` di

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v8]

2023-01-13 Thread Archie L . Cobbs
On Fri, 13 Jan 2023 17:35:08 GMT, Vicente Romero wrote: >> Archie L. Cobbs has updated the pull request incrementally with 16 >> additional commits since the last revision: >> >> - Fix bug where all but the last yeild statement were being ignored. >> - Add method RefSet.mapInto() and use to r

Re: RFR: 8299915: Remove ArrayAllocatorMallocLimit and associated code [v9]

2023-01-13 Thread Justin King
> Remove abstraction that is a holdover from Solaris. Direct usages of > `MmapArrayAllocator` have been switched to normal `malloc`. The justification > is that none of the code paths are called from signal handlers, so using > `mmap` directly does not make sense and is potentially slower than g

Re: RFR: 8299915: Remove ArrayAllocatorMallocLimit and associated code [v4]

2023-01-13 Thread Justin King
On Wed, 11 Jan 2023 15:08:23 GMT, Stefan Karlsson wrote: >>> I'm happy to see this flag getting removed. I'm less happy about seeing >>> usages of the array allocators being replaced by macros. I'd rather see an >>> effort towards getting rid of these macros. Could we limit this patch to >>> o

Re: RFR: 8299915: Remove ArrayAllocatorMallocLimit and associated code [v8]

2023-01-13 Thread Justin King
> Remove abstraction that is a holdover from Solaris. Direct usages of > `MmapArrayAllocator` have been switched to normal `malloc`. The justification > is that none of the code paths are called from signal handlers, so using > `mmap` directly does not make sense and is potentially slower than g

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v8]

2023-01-13 Thread Vicente Romero
On Fri, 13 Jan 2023 04:04:36 GMT, Archie L. Cobbs wrote: >> This PR adds a new lint warning category `this-escape`. >> >> It also adds `@SuppressWarnings` annotations as needed to the JDK itself to >> allow the JDK to continue to compile with `-Xlint:all`. >> >> A 'this' escape warning is gene

Re: RFR: 8299795: Relativize locals in interpreter frames [v2]

2023-01-13 Thread Coleen Phillimore
On Wed, 11 Jan 2023 09:22:03 GMT, Fredrik Bredberg wrote: >> Implementation of relativized locals in interpreter frames for x86. x64, >> arm, aarch64, ppc64le and riscv. >> Not relativized locals on zero and s390 but done some changes to cope with >> the changed generic code. >> Tested tier1-ti

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v8]

2023-01-13 Thread Archie L . Cobbs
On Fri, 13 Jan 2023 16:12:50 GMT, Vicente Romero wrote: >> Yes... I did it that way is so that it doesn't require any adaptation >> if/when JDK-8194743 ever gets implemented. And it keeps the code a little >> simpler in exchange for a little redundancy. >> >> I'm happy to fix this if you think

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v7]

2023-01-13 Thread Archie L . Cobbs
On Fri, 13 Jan 2023 16:06:04 GMT, Maurizio Cimadamore wrote: >>> Something seems to be up with the lint description for this-escape - >>> compare this: >> >> Oops, will fix - thanks. > >> The decision was to go with drawing the "warning boundary" at the >> compilation unit. The reasoning is t

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v8]

2023-01-13 Thread Vicente Romero
On Fri, 13 Jan 2023 15:14:05 GMT, Archie L. Cobbs wrote: >> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ThisEscapeAnalyzer.java >> line 516: >> >>> 514: Name name = TreeInfo.name(invoke.meth); >>> 515: if (name == names._super) { >>> 516: scanInitializers

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v7]

2023-01-13 Thread Maurizio Cimadamore
On Fri, 13 Jan 2023 15:08:59 GMT, Archie L. Cobbs wrote: >>> I guess I was confused because, while subclasses are a particularly sneaky >>> case where uninitialized values can show up, the above leak seems >>> potentially dangerous as well... >> >> Yes - and this very question did come up in t

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v8]

2023-01-13 Thread Archie L . Cobbs
On Fri, 13 Jan 2023 12:42:24 GMT, Vicente Romero wrote: >> Archie L. Cobbs has updated the pull request incrementally with 16 >> additional commits since the last revision: >> >> - Fix bug where all but the last yeild statement were being ignored. >> - Add method RefSet.mapInto() and use to r

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v7]

2023-01-13 Thread Archie L . Cobbs
On Fri, 13 Jan 2023 15:08:43 GMT, Archie L. Cobbs wrote: >> Something seems to be up with the lint description for this-escape - compare >> this: >> >> >> serial Warn about Serializable classes that do not have a >> serialVersionUID field. >> Also war

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v7]

2023-01-13 Thread Archie L . Cobbs
On Fri, 13 Jan 2023 11:08:33 GMT, Maurizio Cimadamore wrote: >> Perhaps my confusion might come from the name `this-escape` of the lint >> warning - which seems overpromising in this respect. But I looked at the >> description of the lint warning using `javac --help-lint` and I got this: >> >

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v8]

2023-01-13 Thread Vicente Romero
On Fri, 13 Jan 2023 04:04:36 GMT, Archie L. Cobbs wrote: >> This PR adds a new lint warning category `this-escape`. >> >> It also adds `@SuppressWarnings` annotations as needed to the JDK itself to >> allow the JDK to continue to compile with `-Xlint:all`. >> >> A 'this' escape warning is gene

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v8]

2023-01-13 Thread Vicente Romero
On Fri, 13 Jan 2023 04:04:36 GMT, Archie L. Cobbs wrote: >> This PR adds a new lint warning category `this-escape`. >> >> It also adds `@SuppressWarnings` annotations as needed to the JDK itself to >> allow the JDK to continue to compile with `-Xlint:all`. >> >> A 'this' escape warning is gene

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v7]

2023-01-13 Thread Maurizio Cimadamore
On Fri, 13 Jan 2023 10:58:33 GMT, Maurizio Cimadamore wrote: >> Caring about the proper initialization of any class in the _current_ >> compilation unit is an explicit non-goal. >> >> We only care about bugs where a superclass and subclass are in separate >> compilation units. > > So, to clar

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v7]

2023-01-13 Thread Maurizio Cimadamore
On Fri, 13 Jan 2023 11:05:51 GMT, Maurizio Cimadamore wrote: >> So, to clarify, in this case: >> >> >> import java.util.*; >> >> class B { >> final Object ref; >> >> private B(Object ref) { >> Foo.consume(this); >> this.ref = ref; >> } >> } >> >> >> Even

Re: RFR: 8015831: Add lint check for calling overridable methods from a constructor [v7]

2023-01-13 Thread Maurizio Cimadamore
On Thu, 12 Jan 2023 21:04:09 GMT, Archie L. Cobbs wrote: >> but what if `m` is a static method in a separate compilation unit? Should it >> be able to observe a partially initialized Foo? > > Caring about the proper initialization of any class in the _current_ > compilation unit is an explicit

Re: RFR: 8299635: More test issues for deprecated sprintf in Xcode 14

2023-01-13 Thread Xue-Lei Andrew Fan
On Thu, 12 Jan 2023 07:25:07 GMT, Xue-Lei Andrew Fan wrote: >>> This PR does not address all the remaining sprintf:s in hotspot, and with >>> it now explicitly forbidden the build will fail: >>> >> >> This is a question to me as well. I noticed there are still some use of >> sprintf, but the

Re: RFR: 8299635: More test issues for deprecated sprintf in Xcode 14

2023-01-13 Thread Serguei Spitsyn
On Wed, 11 Jan 2023 06:26:18 GMT, Xue-Lei Andrew Fan wrote: > The sprintf is deprecated in Xcode 14 because of security concerns. The issue > was addressed in [JDK-8296812](https://bugs.openjdk.org/browse/JDK-8296812) > for hotspot impl, and > [JDK-8299378](https://bugs.openjdk.org/browse/JDK-

Re: RFR: 8299635: More test issues for deprecated sprintf in Xcode 14

2023-01-13 Thread Serguei Spitsyn
On Thu, 12 Jan 2023 07:25:07 GMT, Xue-Lei Andrew Fan wrote: >>> This PR does not address all the remaining sprintf:s in hotspot, and with >>> it now explicitly forbidden the build will fail: >>> >> >> This is a question to me as well. I noticed there are still some use of >> sprintf, but the

Integrated: JDK-8299957: Enhance error logging in instrument coding with additional jplis_assert_msg

2023-01-13 Thread Matthias Baesken
On Thu, 12 Jan 2023 08:10:29 GMT, Matthias Baesken wrote: > There are a few places in the instrument coding where errors occur in our > jtreg test, but the already existing error logging method jplis_assert_msg / > jplis_assert is unfortunately missing so not much details are shown. This > cou