diff --git a/src/hotspot/share/nmt/memBaseline.cpp b/src/hotspot/share/nmt/memBaseline.cpp index 637acc5245811..3363268ef9054 100644 --- a/src/hotspot/share/nmt/memBaseline.cpp +++ b/src/hotspot/share/nmt/memBaseline.cpp @@ -110,22 +110,25 @@ class MallocAllocationSiteWalker : public MallocSiteWalker { } }; -// Compare virtual memory region's base address -int compare_virtual_memory_base(const ReservedMemoryRegion& r1, const ReservedMemoryRegion& r2) { - return r1.compare(r2); -} - // Walk all virtual memory regions for baselining class VirtualMemoryAllocationWalker : public VirtualMemoryWalker { private: - SortedLinkedList - _virtual_memory_regions; - size_t _count; - + typedef LinkedListImpl EntryList; + EntryList _virtual_memory_regions; + size_t _count; + DEBUG_ONLY(address _last_base;) public: - VirtualMemoryAllocationWalker() : _count(0) { } + VirtualMemoryAllocationWalker() : + _count(0) +#ifdef ASSERT + , _last_base(nullptr) +#endif + {} bool do_allocation_site(const ReservedMemoryRegion* rgn) { + assert(rgn->base() >= _last_base, "region unordered?"); + DEBUG_ONLY(_last_base = rgn->base()); if (rgn->size() > 0) { if (_virtual_memory_regions.add(*rgn) != nullptr) { _count ++; diff --git a/src/hotspot/share/runtime/deoptimization.cpp b/src/hotspot/share/runtime/deoptimization.cpp index 6d8c1e2e2237d..f809cd031c358 100644 --- a/src/hotspot/share/runtime/deoptimization.cpp +++ b/src/hotspot/share/runtime/deoptimization.cpp @@ -83,6 +83,7 @@ #include "runtime/stackValue.hpp" #include "runtime/stackWatermarkSet.hpp" #include "runtime/stubRoutines.hpp" +#include "runtime/synchronizer.hpp" #include "runtime/threadSMR.hpp" #include "runtime/threadWXSetters.inline.hpp" #include "runtime/vframe.hpp" @@ -1636,9 +1637,19 @@ bool Deoptimization::relock_objects(JavaThread* thread, GrowableArraylock(); - ObjectSynchronizer::enter(obj, lock, deoptee_thread); - assert(mon_info->owner()->is_locked(), "object must be locked now"); + if (LockingMode == LM_LIGHTWEIGHT && exec_mode == Unpack_none) { + // We have lost information about the correct state of the lock stack. + // Inflate the locks instead. Enter then inflate to avoid races with + // deflation. + ObjectSynchronizer::enter(obj, nullptr, deoptee_thread); + assert(mon_info->owner()->is_locked(), "object must be locked now"); + ObjectMonitor* mon = ObjectSynchronizer::inflate(deoptee_thread, obj(), ObjectSynchronizer::inflate_cause_vm_internal); + assert(mon->owner() == deoptee_thread, "must be"); + } else { + BasicLock* lock = mon_info->lock(); + ObjectSynchronizer::enter(obj, lock, deoptee_thread); + assert(mon_info->owner()->is_locked(), "object must be locked now"); + } } } } diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/taglets/SnippetTaglet.java b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/taglets/SnippetTaglet.java index 875ce98223551..e705fb0cc1027 100644 --- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/taglets/SnippetTaglet.java +++ b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/taglets/SnippetTaglet.java @@ -376,9 +376,10 @@ private Content generateContent(Element holder, DocTree tag) StyledText externalSnippet = null; try { - Diags d = (text, pos) -> { + Diags d = (key, pos) -> { var path = utils.getCommentHelper(holder) .getDocTreePath(snippetTag.getBody()); + var text = resources.getText(key); config.getReporter().print(Diagnostic.Kind.WARNING, path, pos, pos, pos, text); }; @@ -397,7 +398,7 @@ private Content generateContent(Element holder, DocTree tag) try { var finalFileObject = fileObject; - Diags d = (text, pos) -> messages.warning(finalFileObject, pos, pos, pos, text); + Diags d = (key, pos) -> messages.warning(finalFileObject, pos, pos, pos, key); if (externalContent != null) { externalSnippet = parse(resources, d, language, externalContent); } @@ -484,7 +485,7 @@ private StyledText parse(Resources resources, Diags diags, Optional la } public interface Diags { - void warn(String text, int pos); + void warn(String key, int pos); } private static String stringValueOf(AttributeTree at) throws BadSnippetException { diff --git a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/taglets/snippet/Parser.java b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/taglets/snippet/Parser.java index 7c60da78dbe35..907c2231a5c3f 100644 --- a/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/taglets/snippet/Parser.java +++ b/src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/taglets/snippet/Parser.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2020, 2023, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -161,7 +161,7 @@ record OffsetAndLine(int offset, String line) { } } } if (parsedTags.isEmpty()) { // (2) - diags.warn(resources.getText("doclet.snippet.markup.spurious"), next.offset() + markedUpLine.start("markup")); + diags.warn("doclet.snippet.markup.spurious", next.offset() + markedUpLine.start("markup")); line = rawLine + (addLineTerminator ? "\n" : ""); } else { // (3) hasMarkup = true; diff --git a/test/jdk/com/sun/jdi/EATests.java b/test/jdk/com/sun/jdi/EATests.java index 67351e36cb77d..d3a615e8d8de6 100644 --- a/test/jdk/com/sun/jdi/EATests.java +++ b/test/jdk/com/sun/jdi/EATests.java @@ -42,6 +42,7 @@ * -XX:+WhiteBoxAPI * -Xbatch * -XX:+DoEscapeAnalysis -XX:+EliminateAllocations -XX:+EliminateLocks -XX:+EliminateNestedLocks + * -XX:LockingMode=1 * @run driver EATests * -XX:+UnlockDiagnosticVMOptions * -Xms256m -Xmx256m @@ -50,6 +51,7 @@ * -XX:+WhiteBoxAPI * -Xbatch * -XX:+DoEscapeAnalysis -XX:+EliminateAllocations -XX:-EliminateLocks -XX:+EliminateNestedLocks + * -XX:LockingMode=1 * @run driver EATests * -XX:+UnlockDiagnosticVMOptions * -Xms256m -Xmx256m @@ -58,6 +60,7 @@ * -XX:+WhiteBoxAPI * -Xbatch * -XX:+DoEscapeAnalysis -XX:-EliminateAllocations -XX:+EliminateLocks -XX:+EliminateNestedLocks + * -XX:LockingMode=1 * @run driver EATests * -XX:+UnlockDiagnosticVMOptions * -Xms256m -Xmx256m @@ -66,6 +69,44 @@ * -XX:+WhiteBoxAPI * -Xbatch * -XX:-DoEscapeAnalysis -XX:-EliminateAllocations -XX:+EliminateLocks -XX:+EliminateNestedLocks + * -XX:LockingMode=1 + * + * @run driver EATests + * -XX:+UnlockDiagnosticVMOptions + * -Xms256m -Xmx256m + * -Xbootclasspath/a:. + * -XX:CompileCommand=dontinline,*::dontinline_* + * -XX:+WhiteBoxAPI + * -Xbatch + * -XX:+DoEscapeAnalysis -XX:+EliminateAllocations -XX:+EliminateLocks -XX:+EliminateNestedLocks + * -XX:LockingMode=2 + * @run driver EATests + * -XX:+UnlockDiagnosticVMOptions + * -Xms256m -Xmx256m + * -Xbootclasspath/a:. + * -XX:CompileCommand=dontinline,*::dontinline_* + * -XX:+WhiteBoxAPI + * -Xbatch + * -XX:+DoEscapeAnalysis -XX:+EliminateAllocations -XX:-EliminateLocks -XX:+EliminateNestedLocks + * -XX:LockingMode=2 + * @run driver EATests + * -XX:+UnlockDiagnosticVMOptions + * -Xms256m -Xmx256m + * -Xbootclasspath/a:. + * -XX:CompileCommand=dontinline,*::dontinline_* + * -XX:+WhiteBoxAPI + * -Xbatch + * -XX:+DoEscapeAnalysis -XX:-EliminateAllocations -XX:+EliminateLocks -XX:+EliminateNestedLocks + * -XX:LockingMode=2 + * @run driver EATests + * -XX:+UnlockDiagnosticVMOptions + * -Xms256m -Xmx256m + * -Xbootclasspath/a:. + * -XX:CompileCommand=dontinline,*::dontinline_* + * -XX:+WhiteBoxAPI + * -Xbatch + * -XX:-DoEscapeAnalysis -XX:-EliminateAllocations -XX:+EliminateLocks -XX:+EliminateNestedLocks + * -XX:LockingMode=2 * * @comment Excercise -XX:+DeoptimizeObjectsALot. Mostly to prevent bit-rot because the option is meant to stress object deoptimization * with non-synthetic workloads. @@ -208,11 +249,13 @@ public static void main(String[] args) { // Relocking test cases new EARelockingSimpleTarget() .run(); + new EARelockingSimpleWithAccessInOtherThreadTarget() .run(); new EARelockingRecursiveTarget() .run(); new EARelockingNestedInflatedTarget() .run(); new EARelockingNestedInflated_02Target() .run(); new EARelockingArgEscapeLWLockedInCalleeFrameTarget() .run(); new EARelockingArgEscapeLWLockedInCalleeFrame_2Target() .run(); + new EARelockingArgEscapeLWLockedInCalleeFrameNoRecursiveTarget() .run(); new EAGetOwnedMonitorsTarget() .run(); new EAEntryCountTarget() .run(); new EARelockingObjectCurrentlyWaitingOnTarget() .run(); @@ -328,11 +371,13 @@ protected void runTests() throws Exception { // Relocking test cases new EARelockingSimple() .run(this); + new EARelockingSimpleWithAccessInOtherThread() .run(this); new EARelockingRecursive() .run(this); new EARelockingNestedInflated() .run(this); new EARelockingNestedInflated_02() .run(this); new EARelockingArgEscapeLWLockedInCalleeFrame() .run(this); new EARelockingArgEscapeLWLockedInCalleeFrame_2() .run(this); + new EARelockingArgEscapeLWLockedInCalleeFrameNoRecursive() .run(this); new EAGetOwnedMonitors() .run(this); new EAEntryCount() .run(this); new EARelockingObjectCurrentlyWaitingOn() .run(this); @@ -1707,6 +1752,62 @@ public void dontinline_testMethod() { ///////////////////////////////////////////////////////////////////////////// +// The debugger reads and publishes an object with eliminated locking to an instance field. +// A 2nd thread in the debuggee finds it there and changes its state using a synchronized method. +// Without eager relocking the accesses are unsynchronized which can be observed. +class EARelockingSimpleWithAccessInOtherThread extends EATestCaseBaseDebugger { + + public void runTestCase() throws Exception { + BreakpointEvent bpe = resumeTo(TARGET_TESTCASE_BASE_NAME, "dontinline_brkpt", "()V"); + printStack(bpe.thread()); + String l1ClassName = EARelockingSimpleWithAccessInOtherThreadTarget.SyncCounter.class.getName(); + ObjectReference ctr = getLocalRef(bpe.thread().frame(1), l1ClassName, "l1"); + setField(testCase, "sharedCounter", ctr); + terminateEndlessLoop(); + } +} + +class EARelockingSimpleWithAccessInOtherThreadTarget extends EATestCaseBaseTarget { + + public static class SyncCounter { + private int val; + public synchronized int inc() { return val++; } + } + + public volatile SyncCounter sharedCounter; + + @Override + public void setUp() { + super.setUp(); + doLoop = true; + Thread.ofPlatform().daemon().start(() -> { + while (doLoop) { + SyncCounter ctr = sharedCounter; + if (ctr != null) { + ctr.inc(); + } + } + }); + } + + public void dontinline_testMethod() { + SyncCounter l1 = new SyncCounter(); + synchronized (l1) { // Eliminated locking + l1.inc(); + dontinline_brkpt(); // Debugger publishes l1 to sharedCounter. + iResult = l1.inc(); // Changes by the 2nd thread will be observed if l1 + // was not relocked before passing it to the debugger. + } + } + + @Override + public int getExpectedIResult() { + return 1; + } +} + +///////////////////////////////////////////////////////////////////////////// + // Test recursive locking class EARelockingRecursiveTarget extends EATestCaseBaseTarget { @@ -1905,6 +2006,48 @@ public int getExpectedIResult() { ///////////////////////////////////////////////////////////////////////////// +/** + * Similar to {@link EARelockingArgEscapeLWLockedInCalleeFrame_2Target}. It does + * not use recursive locking and exposed a bug in the lightweight-locking implementation. + */ +class EARelockingArgEscapeLWLockedInCalleeFrameNoRecursive extends EATestCaseBaseDebugger { + + public void runTestCase() throws Exception { + BreakpointEvent bpe = resumeTo(TARGET_TESTCASE_BASE_NAME, "dontinline_brkpt", "()V"); + printStack(bpe.thread()); + @SuppressWarnings("unused") + ObjectReference o = getLocalRef(bpe.thread().frame(2), XYVAL_NAME, "l1"); + } +} + +class EARelockingArgEscapeLWLockedInCalleeFrameNoRecursiveTarget extends EATestCaseBaseTarget { + + @Override + public void setUp() { + super.setUp(); + testMethodDepth = 2; + } + + public void dontinline_testMethod() { + XYVal l1 = new XYVal(1, 1); // NoEscape, scalar replaced + XYVal l2 = new XYVal(4, 2); // NoEscape, scalar replaced + XYVal l3 = new XYVal(5, 3); // ArgEscape + synchronized (l1) { // eliminated + synchronized (l2) { // eliminated + l3.dontinline_sync_method(this); // l3 escapes + } + } + iResult = l2.x + l2.y; + } + + @Override + public int getExpectedIResult() { + return 6; + } +} + +///////////////////////////////////////////////////////////////////////////// + /** * Test relocking eliminated (nested) locks of an object on which the * target thread currently waits. diff --git a/test/langtools/jdk/javadoc/doclet/testSnippetTag/TestSnippetMarkup.java b/test/langtools/jdk/javadoc/doclet/testSnippetTag/TestSnippetMarkup.java index c7936bcb7e22a..5d9eb794613fb 100644 --- a/test/langtools/jdk/javadoc/doclet/testSnippetTag/TestSnippetMarkup.java +++ b/test/langtools/jdk/javadoc/doclet/testSnippetTag/TestSnippetMarkup.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2021, 2023, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -23,7 +23,7 @@ /* * @test - * @bug 8266666 8281969 + * @bug 8266666 8281969 8319339 * @summary Implementation for snippets * @library /tools/lib ../../lib * @modules jdk.compiler/com.sun.tools.javac.api @@ -259,8 +259,12 @@ public void testNegativeInlineExternalHybridTagMarkup_NextLinePutOnLastLine(Path Path srcDir = base.resolve("src"); Path outDir = base.resolve("out"); var goodFile = "good.txt"; + // use two files that differ in name but not content, to work around + // error deduplication, whereby an error related to coordinates + // (file, pos) reported before is suppressed; see: + // com.sun.tools.javac.util.Log.shouldReport(JavaFileObject, int) var badFile = "bad.txt"; - var badFile2 = "bad2.txt"; // to workaround error deduplication + var badFile2 = "bad2.txt"; new ClassBuilder(tb, "pkg.A") .setModifiers("public", "class") .addMembers( @@ -625,7 +629,7 @@ public void testPositiveInlineTagMarkup_BlankLinesFromNextLineMarkup(Path base) } @Test - public void testPositiveInlineTagMarkup_FalseMarkup(Path base) throws Exception { + public void testPositiveInlineTagMarkup_SpuriousMarkup(Path base) throws Exception { var testCases = List.of( new TestCase( """ @@ -661,6 +665,134 @@ public void testPositiveInlineTagMarkup_FalseMarkup(Path base) throws Exception """) ); testPositive(base, testCases); + checkOutput(Output.OUT, true, """ + A.java:6: warning: spurious markup + // @formatter:off + ^""",""" + A.java:9: warning: spurious markup + // @formatter:on + ^""",""" + A.java:17: warning: spurious markup + // @formatter:off + ^""",""" + A.java:22: warning: spurious markup + // @formatter:on + ^"""); + } + + /* + * If spurious markup appears in an external snippet or either side of a + * hybrid snippet, then all of the below is true: + * + * - no error is raised + * - relevant warnings are emitted + * - spurious markup is output literally + */ + @Test + public void testPositiveExternalHybridTagMarkup_SpuriousMarkup(Path base) throws Exception { + Path srcDir = base.resolve("src"); + Path outDir = base.resolve("out"); + var plain = "plain.txt"; + var withRegion = "withRegion.txt"; + new ClassBuilder(tb, "pkg.A") + .setModifiers("public", "class") + .addMembers( + ClassBuilder.MethodBuilder + .parse("public void external() { }") + .setComments(""" + {@snippet file="%s"} + """.formatted(plain))) + .addMembers( + ClassBuilder.MethodBuilder + .parse("public void hybrid1() { }") + .setComments(""" + {@snippet file="%s": + First line + // @formatter:off + Second Line + Third line + // @formatter:on + Fourth line + } + """.formatted(plain))) + .addMembers( + ClassBuilder.MethodBuilder + .parse("public void hybrid2() { }") + .setComments(""" + {@snippet file="%s" region="showThis" : + Second Line + Third line + } + """.formatted(withRegion))) + .addMembers( + ClassBuilder.MethodBuilder + .parse("public void hybrid3() { }") + .setComments(""" + {@snippet file="%s" region="showThis" : + First line + // @formatter:off + Second Line // @start region=showThis + Third line + // @end + // @formatter:on + Fourth line + } + """.formatted(withRegion))) + .write(srcDir); + + addSnippetFile(srcDir, "pkg", plain, """ + First line + // @formatter:off + Second Line + Third line + // @formatter:on + Fourth line +"""); + addSnippetFile(srcDir, "pkg", withRegion, """ + First line + // @formatter:off + Second Line // @start region=showThis + Third line + // @end + // @formatter:on + Fourth line +"""); + javadoc("-d", outDir.toString(), + "-sourcepath", srcDir.toString(), + "pkg"); + checkExit(Exit.OK); + checkNoCrashes(); + checkOutput(Output.OUT, true, """ + %s:2: warning: spurious markup + // @formatter:off + ^""".formatted(plain), """ + %s:5: warning: spurious markup + // @formatter:on + ^""".formatted(plain), """ + A.java:11: warning: spurious markup + // @formatter:off + ^""", """ + A.java:14: warning: spurious markup + // @formatter:on + ^""", """ + %s:2: warning: spurious markup + // @formatter:off + ^""".formatted(plain), """ + %s:5: warning: spurious markup + // @formatter:on + ^""".formatted(plain), """ + %s:2: warning: spurious markup + // @formatter:off + ^""".formatted(withRegion), """ + %s:6: warning: spurious markup + // @formatter:on + ^""".formatted(withRegion), """ + A.java:31: warning: spurious markup + // @formatter:off + ^""", """ + A.java:35: warning: spurious markup + // @formatter:on + ^"""); } @Test