From 723588a4e78d25f0ef3c4cdaeb377aedc3a352d4 Mon Sep 17 00:00:00 2001 From: Daniel Fuchs Date: Thu, 29 Aug 2024 08:54:02 +0000 Subject: [PATCH 01/11] 8338569: HTTP/1.1 CleanupTrigger may be triggerred after the next exchange started Reviewed-by: jpai --- .../jdk/internal/net/http/ConnectionPool.java | 53 +++++++-- .../jdk/internal/net/http/SocketTube.java | 104 ++++++++++++------ .../internal/net/http/common/FlowTube.java | 6 +- .../java/net/httpclient/DigestEchoClient.java | 4 +- test/jdk/java/net/httpclient/ShutdownNow.java | 22 ++-- test/jdk/java/net/httpclient/SmokeTest.java | 42 +++++-- 6 files changed, 167 insertions(+), 64 deletions(-) diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/ConnectionPool.java b/src/java.net.http/share/classes/jdk/internal/net/http/ConnectionPool.java index 0ad7b9d5992..1d8cb013295 100644 --- a/src/java.net.http/share/classes/jdk/internal/net/http/ConnectionPool.java +++ b/src/java.net.http/share/classes/jdk/internal/net/http/ConnectionPool.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2015, 2024, 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 @@ -44,6 +44,7 @@ import jdk.internal.net.http.common.Deadline; import jdk.internal.net.http.common.FlowTube; +import jdk.internal.net.http.common.Log; import jdk.internal.net.http.common.Logger; import jdk.internal.net.http.common.TimeLine; import jdk.internal.net.http.common.TimeSource; @@ -492,13 +493,13 @@ void clear() { // Remove a connection from the pool. // should only be called while holding the ConnectionPool stateLock. - private void removeFromPool(HttpConnection c) { + private boolean removeFromPool(HttpConnection c) { assert stateLock.isHeldByCurrentThread(); if (c instanceof PlainHttpConnection) { - removeFromPool(c, plainPool); + return removeFromPool(c, plainPool); } else { assert c.isSecure() : "connection " + c + " is not secure!"; - removeFromPool(c, sslPool); + return removeFromPool(c, sslPool); } } @@ -524,18 +525,36 @@ private boolean contains0(HttpConnection c) { return false; } - void cleanup(HttpConnection c, Throwable error) { + void cleanup(HttpConnection c, long pendingData, Throwable error) { if (debug.on()) debug.log("%s : ConnectionPool.cleanup(%s)", String.valueOf(c.getConnectionFlow()), error); stateLock.lock(); + boolean removed; try { - removeFromPool(c); + removed = removeFromPool(c); expiryList.remove(c); } finally { stateLock.unlock(); } - c.close(); + if (!removed && pendingData != 0) { + // this should not happen; the cleanup may have consumed + // some data that wasn't supposed to be consumed, so + // the only thing we can do is log it and close the + // connection. + if (Log.errors()) { + Log.logError("WARNING: CleanupTrigger triggered for" + + " a connection not found in the pool: closing {0}", c.dbgString()); + } + if (debug.on()) { + debug.log("WARNING: CleanupTrigger triggered for" + + " a connection not found in the pool: closing %s", c.dbgString()); + } + Throwable cause = new IOException("Unexpected cleanup triggered for non pooled connection", error); + c.close(cause); + } else { + c.close(); + } } /** @@ -549,6 +568,7 @@ private final class CleanupTrigger implements private final HttpConnection connection; private volatile boolean done; + private volatile boolean dropped; public CleanupTrigger(HttpConnection connection) { this.connection = connection; @@ -556,9 +576,12 @@ public CleanupTrigger(HttpConnection connection) { public boolean isDone() { return done;} - private void triggerCleanup(Throwable error) { + private void triggerCleanup(long pendingData, Throwable error) { done = true; - cleanup(connection, error); + if (debug.on()) { + debug.log("Cleanup triggered for %s: pendingData:%s error:%s", this, pendingData, error); + } + cleanup(connection, pendingData, error); } @Override public void request(long n) {} @@ -566,15 +589,16 @@ private void triggerCleanup(Throwable error) { @Override public void onSubscribe(Flow.Subscription subscription) { + if (dropped || done) return; subscription.request(1); } @Override - public void onError(Throwable error) { triggerCleanup(error); } + public void onError(Throwable error) { triggerCleanup(0, error); } @Override - public void onComplete() { triggerCleanup(null); } + public void onComplete() { triggerCleanup(0, null); } @Override public void onNext(List item) { - triggerCleanup(new IOException("Data received while in pool")); + triggerCleanup(Utils.remaining(item), new IOException("Data received while in pool")); } @Override @@ -586,5 +610,10 @@ public void subscribe(Flow.Subscriber> subscriber) { public String toString() { return "CleanupTrigger(" + connection.getConnectionFlow() + ")"; } + + @Override + public void dropSubscription() { + dropped = true; + } } } diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/SocketTube.java b/src/java.net.http/share/classes/jdk/internal/net/http/SocketTube.java index cbdf6633576..d0e30806d53 100644 --- a/src/java.net.http/share/classes/jdk/internal/net/http/SocketTube.java +++ b/src/java.net.http/share/classes/jdk/internal/net/http/SocketTube.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2017, 2024, 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 @@ -29,6 +29,7 @@ import java.nio.ByteBuffer; import java.util.List; import java.util.Objects; +import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.Flow; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; @@ -557,7 +558,7 @@ private final class InternalReadPublisher implements Flow.Publisher> { private final InternalReadSubscription subscriptionImpl = new InternalReadSubscription(); - AtomicReference pendingSubscription = new AtomicReference<>(); + ConcurrentLinkedQueue pendingSubscriptions = new ConcurrentLinkedQueue<>(); private volatile ReadSubscription subscription; @Override @@ -565,14 +566,14 @@ public void subscribe(Flow.Subscriber> s) { Objects.requireNonNull(s); TubeSubscriber sub = FlowTube.asTubeSubscriber(s); - ReadSubscription target = new ReadSubscription(subscriptionImpl, sub); - ReadSubscription previous = pendingSubscription.getAndSet(target); - - if (previous != null && previous != target) { + ReadSubscription previous; + while ((previous = pendingSubscriptions.poll()) != null) { if (debug.on()) debug.log("read publisher: dropping pending subscriber: " + previous.subscriber); previous.errorRef.compareAndSet(null, errorRef.get()); + // make sure no data will be routed to the old subscriber. + previous.stopReading(); previous.signalOnSubscribe(); if (subscriptionImpl.completed) { previous.signalCompletion(); @@ -580,8 +581,10 @@ public void subscribe(Flow.Subscriber> s) { previous.subscriber.dropSubscription(); } } + ReadSubscription target = new ReadSubscription(subscriptionImpl, sub); + pendingSubscriptions.offer(target); - if (debug.on()) debug.log("read publisher got subscriber"); + if (debug.on()) debug.log("read publisher got new subscriber: " + s); subscriptionImpl.signalSubscribe(); debugState("leaving read.subscribe: "); } @@ -606,6 +609,7 @@ final class ReadSubscription implements Flow.Subscription { volatile boolean subscribed; volatile boolean cancelled; volatile boolean completed; + private volatile boolean stopped; public ReadSubscription(InternalReadSubscription impl, TubeSubscriber subscriber) { @@ -623,11 +627,12 @@ public void cancel() { @Override public void request(long n) { - if (!cancelled) { + if (!cancelled && !stopped) { + // should be safe to not synchronize here. impl.request(n); } else { if (debug.on()) - debug.log("subscription cancelled, ignoring request %d", n); + debug.log("subscription stopped or cancelled, ignoring request %d", n); } } @@ -661,6 +666,32 @@ void signalOnSubscribe() { signalCompletion(); } } + + /** + * Called when switching subscriber on the {@link InternalReadSubscription}. + * This subscriber is the old subscriber. Demand on the internal + * subscription will be reset and reading will be paused until the + * new subscriber is subscribed. + * This should ensure that no data is routed to this subscriber + * until the new subscriber is subscribed. + */ + synchronized void stopReading() { + stopped = true; + impl.demand.reset(); + } + + synchronized boolean tryDecrementDemand() { + if (stopped) return false; + return impl.demand.tryDecrement(); + } + + synchronized boolean isStopped() { + return stopped; + } + + synchronized void increaseDemand(long n) { + if (!stopped) impl.demand.increase(n); + } } final class InternalReadSubscription implements Flow.Subscription { @@ -835,7 +866,7 @@ final void read() { // If we reach here then we must be in the selector thread. assert client.isSelectorThread(); - if (demand.tryDecrement()) { + if (current.tryDecrementDemand()) { // we have demand. try { List bytes = readAvailable(current.bufferSource); @@ -881,8 +912,10 @@ final void read() { // event. This ensures that this loop is // executed again when the socket becomes // readable again. - demand.increase(1); - resumeReadEvent(); + if (!current.isStopped()) { + current.increaseDemand(1); + resumeReadEvent(); + } if (errorRef.get() != null) continue; debugState("leaving read() loop with no bytes"); return; @@ -922,30 +955,35 @@ final void read() { } boolean handlePending() { - ReadSubscription pending = pendingSubscription.getAndSet(null); - if (pending == null) return false; - if (debug.on()) - debug.log("handling pending subscription for %s", + ReadSubscription pending; + boolean subscribed = false; + while ((pending = pendingSubscriptions.poll()) != null) { + subscribed = true; + if (debug.on()) + debug.log("handling pending subscription for %s", pending.subscriber); - ReadSubscription current = subscription; - if (current != null && current != pending && !completed) { - current.subscriber.dropSubscription(); - } - if (debug.on()) debug.log("read demand reset to 0"); - subscriptionImpl.demand.reset(); // subscriber will increase demand if it needs to. - pending.errorRef.compareAndSet(null, errorRef.get()); - if (!readScheduler.isStopped()) { - subscription = pending; - } else { - if (debug.on()) debug.log("socket tube is already stopped"); - } - if (debug.on()) debug.log("calling onSubscribe"); - pending.signalOnSubscribe(); - if (completed) { + ReadSubscription current = subscription; + if (current != null && current != pending && !completed) { + debug.log("dropping pending subscription for current %s", + current.subscriber); + current.subscriber.dropSubscription(); + } + if (debug.on()) debug.log("read demand reset to 0"); + subscriptionImpl.demand.reset(); // subscriber will increase demand if it needs to. pending.errorRef.compareAndSet(null, errorRef.get()); - pending.signalCompletion(); + if (!readScheduler.isStopped()) { + subscription = pending; + } else { + if (debug.on()) debug.log("socket tube is already stopped"); + } + if (debug.on()) debug.log("calling onSubscribe on " + pending.subscriber); + pending.signalOnSubscribe(); + if (completed) { + pending.errorRef.compareAndSet(null, errorRef.get()); + pending.signalCompletion(); + } } - return true; + return subscribed; } } diff --git a/src/java.net.http/share/classes/jdk/internal/net/http/common/FlowTube.java b/src/java.net.http/share/classes/jdk/internal/net/http/common/FlowTube.java index 023086e3dee..b87c4601763 100644 --- a/src/java.net.http/share/classes/jdk/internal/net/http/common/FlowTube.java +++ b/src/java.net.http/share/classes/jdk/internal/net/http/common/FlowTube.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, 2018, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2017, 2024, 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 @@ -180,6 +180,10 @@ public void onError(Throwable throwable) { public void onComplete() { delegate.onComplete(); } + @Override + public String toString() { + return "TubeSubscriberWrapper("+delegate.toString()+")"; + } } } diff --git a/test/jdk/java/net/httpclient/DigestEchoClient.java b/test/jdk/java/net/httpclient/DigestEchoClient.java index 3b6d1a1773f..7038d82d91f 100644 --- a/test/jdk/java/net/httpclient/DigestEchoClient.java +++ b/test/jdk/java/net/httpclient/DigestEchoClient.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2018, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2018, 2024, 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 @@ -64,7 +64,7 @@ * @test * @summary this test verifies that a client may provides authorization * headers directly when connecting with a server. - * @bug 8087112 + * @bug 8087112 8336655 8338569 * @library /test/lib /test/jdk/java/net/httpclient/lib * @build jdk.httpclient.test.lib.common.HttpServerAdapters jdk.test.lib.net.SimpleSSLContext * DigestEchoServer ReferenceTracker DigestEchoClient diff --git a/test/jdk/java/net/httpclient/ShutdownNow.java b/test/jdk/java/net/httpclient/ShutdownNow.java index de2279277c7..77504b5052a 100644 --- a/test/jdk/java/net/httpclient/ShutdownNow.java +++ b/test/jdk/java/net/httpclient/ShutdownNow.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2023, 2024, 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 @@ -210,12 +210,16 @@ void testConcurrent(String uriString) throws Exception { } CompletableFuture.allOf(responses.toArray(new CompletableFuture[0])).get(); } finally { - if (client.awaitTermination(Duration.ofMillis(2000))) { + if (client.awaitTermination(Duration.ofMillis(2500))) { out.println("Client terminated within expected delay"); + assertTrue(client.isTerminated()); } else { - throw new AssertionError("client still running"); + client = null; + var error = TRACKER.check(500); + if (error != null) throw error; + throw new AssertionError("client was still running, but exited after further delay: " + + "timeout should be adjusted"); } - assertTrue(client.isTerminated()); } } @@ -272,12 +276,16 @@ void testSequential(String uriString) throws Exception { }).thenCompose((c) -> c).get(); } } finally { - if (client.awaitTermination(Duration.ofMillis(2000))) { + if (client.awaitTermination(Duration.ofMillis(2500))) { out.println("Client terminated within expected delay"); + assertTrue(client.isTerminated()); } else { - throw new AssertionError("client still running"); + client = null; + var error = TRACKER.check(500); + if (error != null) throw error; + throw new AssertionError("client was still running, but exited after further delay: " + + "timeout should be adjusted"); } - assertTrue(client.isTerminated()); } } diff --git a/test/jdk/java/net/httpclient/SmokeTest.java b/test/jdk/java/net/httpclient/SmokeTest.java index cff2cee4d80..56294e8f02f 100644 --- a/test/jdk/java/net/httpclient/SmokeTest.java +++ b/test/jdk/java/net/httpclient/SmokeTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2015, 2024, 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 8087112 8178699 + * @bug 8087112 8178699 8338569 * @modules java.net.http * java.logging * jdk.httpserver @@ -54,6 +54,8 @@ import java.util.Collections; import java.util.Optional; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; import java.util.concurrent.atomic.AtomicInteger; import java.net.InetSocketAddress; import java.net.PasswordAuthentication; @@ -572,10 +574,12 @@ static void test7(String target) throws Exception { System.out.print("test7: " + target); Path requestBody = getTempFile(128 * 1024); // First test - URI uri = new URI(target); - HttpRequest request = HttpRequest.newBuilder().uri(uri).GET().build(); + AtomicInteger count = new AtomicInteger(); for (int i=0; i<4; i++) { + URI uri = new URI(target+"?get-sync;count="+count.incrementAndGet()); + System.out.println("Sending " + uri); + HttpRequest request = HttpRequest.newBuilder().uri(uri).GET().build(); HttpResponse r = client.send(request, BodyHandlers.ofString()); String body = r.body(); if (!body.equals("OK")) { @@ -584,12 +588,15 @@ static void test7(String target) throws Exception { } // Second test: 4 x parallel - request = HttpRequest.newBuilder() - .uri(uri) - .POST(BodyPublishers.ofFile(requestBody)) - .build(); + List> futures = new LinkedList<>(); for (int i=0; i<4; i++) { + URI uri = new URI(target+"?post-async;count="+count.incrementAndGet()); + System.out.println("Sending " + uri); + HttpRequest request = HttpRequest.newBuilder() + .uri(uri) + .POST(BodyPublishers.ofFile(requestBody)) + .build(); futures.add(client.sendAsync(request, BodyHandlers.ofString()) .thenApply((response) -> { if (response.statusCode() == 200) @@ -610,11 +617,17 @@ static void test7(String target) throws Exception { } // Third test: Multiple of 4 parallel requests - request = HttpRequest.newBuilder(uri).GET().build(); BlockingQueue q = new LinkedBlockingQueue<>(); + Set inFlight = ConcurrentHashMap.newKeySet(); for (int i=0; i<4; i++) { + URI uri = new URI(target+"?get-async;count="+count.incrementAndGet()); + inFlight.add(uri.getQuery()); + System.out.println("Sending " + uri); + HttpRequest request = HttpRequest.newBuilder(uri).GET().build(); client.sendAsync(request, BodyHandlers.ofString()) .thenApply((HttpResponse resp) -> { + inFlight.remove(uri.getQuery()); + System.out.println("Got response for: " + uri); String body = resp.body(); putQ(q, body); return body; @@ -630,8 +643,15 @@ static void test7(String target) throws Exception { if (!body.equals("OK")) { throw new RuntimeException(body); } + URI uri = new URI(target+"?get-async-next;count="+count.incrementAndGet()); + inFlight.add(uri.getQuery()); + System.out.println("Sending " + uri); + HttpRequest request = HttpRequest.newBuilder(uri).GET().build(); client.sendAsync(request, BodyHandlers.ofString()) .thenApply((resp) -> { + inFlight.remove(uri.getQuery()); + System.out.println("Got response for: " + uri); + System.out.println("In flight: " + inFlight); if (resp.statusCode() == 200) putQ(q, resp.body()); else @@ -639,9 +659,13 @@ static void test7(String target) throws Exception { return null; }); } + System.out.println("Waiting: In flight: " + inFlight); + System.out.println("Queue size: " + q.size()); // should be four left for (int i=0; i<4; i++) { takeQ(q); + System.out.println("Waiting: In flight: " + inFlight); + System.out.println("Queue size: " + q.size()); } System.out.println(" OK"); } From d35ffa4f6afb7df052103cee8544e4e707b72cc1 Mon Sep 17 00:00:00 2001 From: Andrey Turbanov Date: Thu, 29 Aug 2024 09:57:52 +0000 Subject: [PATCH 02/11] 8339017: Make a couple of fields in DoubleByte static Reviewed-by: bpb, naoto --- .../share/classes/sun/nio/cs/DoubleByte.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/java.base/share/classes/sun/nio/cs/DoubleByte.java b/src/java.base/share/classes/sun/nio/cs/DoubleByte.java index d832d236d1c..2978a6f5dec 100644 --- a/src/java.base/share/classes/sun/nio/cs/DoubleByte.java +++ b/src/java.base/share/classes/sun/nio/cs/DoubleByte.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2009, 2021, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2009, 2024, 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 @@ -402,7 +402,7 @@ protected CoderResult decodeBufferLoop(ByteBuffer src, CharBuffer dst) { else currentState = SBCS; } else { - char c = UNMAPPABLE_DECODING; + char c; if (currentState == SBCS) { c = b2cSB[b1]; if (c == UNMAPPABLE_DECODING) @@ -452,7 +452,7 @@ public int decode(byte[] src, int sp, int len, char[] dst) { else currentState = SBCS; } else { - char c = UNMAPPABLE_DECODING; + char c; if (currentState == SBCS) { c = b2cSB[b1]; if (c == UNMAPPABLE_DECODING) @@ -503,8 +503,8 @@ public Decoder_DBCSONLY(Charset cs, char[][] b2c, char[] b2cSB, int b2Min, int b // The only thing we need to "override" is to check SS2/SS3 and // return "malformed" if found public static class Decoder_EUC_SIM extends Decoder { - private final int SS2 = 0x8E; - private final int SS3 = 0x8F; + private static final int SS2 = 0x8E; + private static final int SS3 = 0x8F; public Decoder_EUC_SIM(Charset cs, char[][] b2c, char[] b2cSB, int b2Min, int b2Max, @@ -556,7 +556,7 @@ public int decode(byte[] src, int sp, int len, char[] dst) { public static class Encoder extends CharsetEncoder implements ArrayEncoder { - protected final int MAX_SINGLEBYTE = 0xff; + protected static final int MAX_SINGLEBYTE = 0xff; private final char[] c2b; private final char[] c2bIndex; protected Surrogate.Parser sgp; @@ -659,7 +659,7 @@ protected CoderResult encodeBufferLoop(CharBuffer src, ByteBuffer dst) { dst.put((byte)(bb)); } else { if (dst.remaining() < 1) - return CoderResult.OVERFLOW; + return CoderResult.OVERFLOW; dst.put((byte)bb); } mark++; From 8c8b5801fd9d28a71edf3bd8d1fae857817e27de Mon Sep 17 00:00:00 2001 From: Jan Lahoda Date: Thu, 29 Aug 2024 10:06:08 +0000 Subject: [PATCH 03/11] 8338281: jshell does not run shutdown hooks Reviewed-by: asotona --- .../execution/ExecutionControlForwarder.java | 2 +- .../execution/JdiDefaultExecutionControl.java | 17 ++++++ test/langtools/jdk/jshell/ShutdownTest.java | 56 ++++++++++++++++++- 3 files changed, 73 insertions(+), 2 deletions(-) diff --git a/src/jdk.jshell/share/classes/jdk/jshell/execution/ExecutionControlForwarder.java b/src/jdk.jshell/share/classes/jdk/jshell/execution/ExecutionControlForwarder.java index fdb126f93ec..54c4131ee3b 100644 --- a/src/jdk.jshell/share/classes/jdk/jshell/execution/ExecutionControlForwarder.java +++ b/src/jdk.jshell/share/classes/jdk/jshell/execution/ExecutionControlForwarder.java @@ -177,7 +177,7 @@ private boolean processCommand() throws IOException { } catch (Throwable ex) { // JShell-core not waiting for a result, ignore } - return true; + return false; } default: { Object arg = in.readObject(); diff --git a/src/jdk.jshell/share/classes/jdk/jshell/execution/JdiDefaultExecutionControl.java b/src/jdk.jshell/share/classes/jdk/jshell/execution/JdiDefaultExecutionControl.java index 2d429691b60..889b8494bab 100644 --- a/src/jdk.jshell/share/classes/jdk/jshell/execution/JdiDefaultExecutionControl.java +++ b/src/jdk.jshell/share/classes/jdk/jshell/execution/JdiDefaultExecutionControl.java @@ -49,6 +49,7 @@ import com.sun.jdi.VirtualMachine; import java.io.PrintStream; import java.util.Optional; +import java.util.concurrent.TimeUnit; import java.util.stream.Stream; import jdk.jshell.JShellConsole; import jdk.jshell.execution.JdiDefaultExecutionControl.JdiStarter.TargetDescription; @@ -70,6 +71,8 @@ */ public class JdiDefaultExecutionControl extends JdiExecutionControl { + private static final int SHUTDOWN_TIMEOUT = 1; //1 second + private VirtualMachine vm; private Process process; private final String remoteAgent; @@ -267,6 +270,20 @@ public void stop() throws EngineTerminationException, InternalException { @Override public void close() { super.close(); + + Process remoteProcess; + + synchronized (this) { + remoteProcess = this.process; + } + + if (remoteProcess != null) { + try { + remoteProcess.waitFor(SHUTDOWN_TIMEOUT, TimeUnit.SECONDS); + } catch (InterruptedException ex) { + debug(ex, "waitFor remote"); + } + } disposeVM(); } diff --git a/test/langtools/jdk/jshell/ShutdownTest.java b/test/langtools/jdk/jshell/ShutdownTest.java index 45431f66162..da4e516f94c 100644 --- a/test/langtools/jdk/jshell/ShutdownTest.java +++ b/test/langtools/jdk/jshell/ShutdownTest.java @@ -28,6 +28,11 @@ * @run testng ShutdownTest */ +import java.io.IOException; +import java.lang.reflect.Method; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.function.Consumer; import jdk.jshell.JShell; @@ -35,8 +40,9 @@ import org.testng.annotations.Test; import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertFalse; +import org.testng.annotations.BeforeMethod; -@Test public class ShutdownTest extends KullaTesting { int shutdownCount; @@ -53,6 +59,7 @@ public void testExit() { assertEquals(shutdownCount, 1); } + @Test public void testCloseCallback() { shutdownCount = 0; getState().onShutdown(this::shutdownCounter); @@ -60,6 +67,7 @@ public void testCloseCallback() { assertEquals(shutdownCount, 1); } + @Test public void testCloseUnsubscribe() { shutdownCount = 0; Subscription token = getState().onShutdown(this::shutdownCounter); @@ -68,6 +76,7 @@ public void testCloseUnsubscribe() { assertEquals(shutdownCount, 0); } + @Test public void testTwoShutdownListeners() { ShutdownListener listener1 = new ShutdownListener(); ShutdownListener listener2 = new ShutdownListener(); @@ -118,6 +127,51 @@ public void testSubscriptionAfterShutdown() { getState().onShutdown(e -> {}); } + @Test + public void testRunShutdownHooks() throws IOException { + Path temporary = Paths.get("temp"); + Files.newOutputStream(temporary).close(); + assertEval("import java.io.*;"); + assertEval("import java.nio.file.*;"); + assertEval(""" + Runtime.getRuntime().addShutdownHook(new Thread(() -> { + try { + Files.delete(Paths.get("$TEMPORARY")); + } catch (IOException ex) { + //ignored + } + })) + """.replace("$TEMPORARY", temporary.toAbsolutePath() + .toString() + .replace("\\", "\\\\"))); + getState().close(); + assertFalse(Files.exists(temporary)); + } + + private Method currentTestMethod; + + @BeforeMethod + public void setUp(Method testMethod) { + currentTestMethod = testMethod; + super.setUp(); + } + + @BeforeMethod + public void setUp() { + } + + @Override + public void setUp(Consumer bc) { + Consumer augmentedBuilder = switch (currentTestMethod.getName()) { + case "testRunShutdownHooks" -> builder -> { + builder.executionEngine(Presets.TEST_STANDARD_EXECUTION); + bc.accept(builder); + }; + default -> bc; + }; + super.setUp(augmentedBuilder); + } + private static class ShutdownListener implements Consumer { private int count; From e57b59325831247818cb4b07c4fd43e4556effca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johan=20Sj=C3=B6len?= Date: Thu, 29 Aug 2024 11:23:04 +0000 Subject: [PATCH 04/11] 8335062: NMT: Make StackIndex non-opaque Reviewed-by: stuefe, gziemski --- .../share/nmt/nmtNativeCallStackStorage.hpp | 31 ++++++++++--------- src/hotspot/share/nmt/vmatree.hpp | 4 +-- .../nmt/test_nmt_nativecallstackstorage.cpp | 4 +-- 3 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/hotspot/share/nmt/nmtNativeCallStackStorage.hpp b/src/hotspot/share/nmt/nmtNativeCallStackStorage.hpp index 1b09002028e..258c3284a18 100644 --- a/src/hotspot/share/nmt/nmtNativeCallStackStorage.hpp +++ b/src/hotspot/share/nmt/nmtNativeCallStackStorage.hpp @@ -28,6 +28,7 @@ #include "nmt/arrayWithFreeList.hpp" #include "utilities/growableArray.hpp" #include "utilities/nativeCallStack.hpp" +#include // Virtual memory regions that are tracked by NMT also have their NativeCallStack (NCS) tracked. // NCS:s are: @@ -41,19 +42,19 @@ // We achieve this by using a closed hashtable for finding previously existing NCS:s and referring to them by an index that's smaller than a pointer. class NativeCallStackStorage : public CHeapObjBase { public: - struct StackIndex { - friend NativeCallStackStorage; - int32_t _stack_index; - public: - static constexpr const int32_t invalid = -1; - static bool equals(const StackIndex& a, const StackIndex& b) { - return a._stack_index == b._stack_index; - } + using StackIndex = int; - bool is_invalid() { - return _stack_index == invalid; - } - }; +private: + constexpr static const StackIndex invalid = std::numeric_limits::max() - 1; + +public: + static bool equals(const StackIndex a, const StackIndex b) { + return a == b; + } + + static bool is_invalid(StackIndex a) { + return a == invalid; + } private: struct TableEntry; @@ -83,16 +84,16 @@ class NativeCallStackStorage : public CHeapObjBase { StackIndex push(const NativeCallStack& stack) { // Not in detailed mode, so not tracking stacks. if (!_is_detailed_mode) { - return StackIndex{StackIndex::invalid}; + return invalid; } return put(stack); } const inline NativeCallStack& get(StackIndex si) { - if (si._stack_index == -1) { + if (is_invalid(si)) { return _fake_stack; } - return _stacks.at(si._stack_index); + return _stacks.at(si); } NativeCallStackStorage(bool is_detailed_mode, int table_size = default_table_size); diff --git a/src/hotspot/share/nmt/vmatree.hpp b/src/hotspot/share/nmt/vmatree.hpp index a93c282f4d2..3316219a1d3 100644 --- a/src/hotspot/share/nmt/vmatree.hpp +++ b/src/hotspot/share/nmt/vmatree.hpp @@ -78,7 +78,7 @@ class VMATree { static bool equals(const RegionData& a, const RegionData& b) { return a.flag == b.flag && - NativeCallStackStorage::StackIndex::equals(a.stack_idx, b.stack_idx); + NativeCallStackStorage::equals(a.stack_idx, b.stack_idx); } }; @@ -112,7 +112,7 @@ class VMATree { return RegionData{sidx, flag()}; } - const NativeCallStackStorage::StackIndex stack() const { + NativeCallStackStorage::StackIndex stack() const { return sidx; } }; diff --git a/test/hotspot/gtest/nmt/test_nmt_nativecallstackstorage.cpp b/test/hotspot/gtest/nmt/test_nmt_nativecallstackstorage.cpp index 71e924b7b9d..867d32c4a66 100644 --- a/test/hotspot/gtest/nmt/test_nmt_nativecallstackstorage.cpp +++ b/test/hotspot/gtest/nmt/test_nmt_nativecallstackstorage.cpp @@ -35,7 +35,7 @@ TEST_VM_F(NMTNativeCallStackStorageTest, DoNotStoreStackIfNotDetailed) { NativeCallStack ncs{}; NCSS ncss(false); NCSS::StackIndex si = ncss.push(ncs); - EXPECT_TRUE(si.is_invalid()); + EXPECT_TRUE(NCSS::is_invalid(si)); NativeCallStack ncs_received = ncss.get(si); EXPECT_TRUE(ncs_received.is_empty()); } @@ -57,7 +57,7 @@ TEST_VM_F(NMTNativeCallStackStorageTest, CollisionsReceiveDifferentIndexes) { for (int i = 0; i < nr_of_stacks; i++) { for (int j = 0; j < nr_of_stacks; j++) { if (i == j) continue; - EXPECT_FALSE(NCSS::StackIndex::equals(si_arr[i],si_arr[j])); + EXPECT_FALSE(NCSS::equals(si_arr[i],si_arr[j])); } } } From 777ed2b5d2ef8371407cc9bf0370a7cef937cfb7 Mon Sep 17 00:00:00 2001 From: Chen Liang Date: Thu, 29 Aug 2024 15:45:52 +0000 Subject: [PATCH 05/11] 8339132: Make DirectCodeBuilder write through without allocating instruction objects Reviewed-by: asotona, redestad --- .../java/lang/classfile/CodeBuilder.java | 59 +- .../classfile/impl/AbstractInstruction.java | 5 - .../classfile/impl/BytecodeHelpers.java | 54 +- .../classfile/impl/DirectCodeBuilder.java | 744 +++++++++++++++++- 4 files changed, 784 insertions(+), 78 deletions(-) diff --git a/src/java.base/share/classes/java/lang/classfile/CodeBuilder.java b/src/java.base/share/classes/java/lang/classfile/CodeBuilder.java index 64a677d63ad..1a3166f8b32 100644 --- a/src/java.base/share/classes/java/lang/classfile/CodeBuilder.java +++ b/src/java.base/share/classes/java/lang/classfile/CodeBuilder.java @@ -639,32 +639,34 @@ default CodeBuilder loadConstant(ConstantDesc value) { //avoid switch expressions here if (value == null || value == ConstantDescs.NULL) return aconst_null(); - if (value instanceof Integer iVal) - return switch (iVal) { - case -1 -> iconst_m1(); - case 0 -> iconst_0(); - case 1 -> iconst_1(); - case 2 -> iconst_2(); - case 3 -> iconst_3(); - case 4 -> iconst_4(); - case 5 -> iconst_5(); - default -> (iVal >= Byte.MIN_VALUE && iVal <= Byte.MAX_VALUE) ? bipush(iVal) - : (iVal >= Short.MIN_VALUE && iVal <= Short.MAX_VALUE) ? sipush(iVal) - : ldc(constantPool().intEntry(iVal)); - }; - if (value instanceof Long lVal) - return lVal == 0l ? lconst_0() - : lVal == 1l ? lconst_1() - : ldc(constantPool().longEntry(lVal)); - if (value instanceof Float fVal) - return Float.floatToRawIntBits(fVal) == 0 ? fconst_0() - : fVal == 1.0f ? fconst_1() - : fVal == 2.0f ? fconst_2() - : ldc(constantPool().floatEntry(fVal)); - if (value instanceof Double dVal) - return Double.doubleToRawLongBits(dVal) == 0l ? dconst_0() - : dVal == 1.0d ? dconst_1() - : ldc(constantPool().doubleEntry(dVal)); + if (value instanceof Number) { + if (value instanceof Integer iVal) + return switch (iVal) { + case -1 -> iconst_m1(); + case 0 -> iconst_0(); + case 1 -> iconst_1(); + case 2 -> iconst_2(); + case 3 -> iconst_3(); + case 4 -> iconst_4(); + case 5 -> iconst_5(); + default -> (iVal >= Byte.MIN_VALUE && iVal <= Byte.MAX_VALUE) ? bipush(iVal) + : (iVal >= Short.MIN_VALUE && iVal <= Short.MAX_VALUE) ? sipush(iVal) + : ldc(constantPool().intEntry(iVal)); + }; + if (value instanceof Long lVal) + return lVal == 0L ? lconst_0() + : lVal == 1L ? lconst_1() + : ldc(constantPool().longEntry(lVal)); + if (value instanceof Float fVal) + return Float.floatToRawIntBits(fVal) == 0 ? fconst_0() + : fVal == 1.0f ? fconst_1() + : fVal == 2.0f ? fconst_2() + : ldc(constantPool().floatEntry(fVal)); + if (value instanceof Double dVal) + return Double.doubleToRawLongBits(dVal) == 0L ? dconst_0() + : dVal == 1.0d ? dconst_1() + : ldc(constantPool().doubleEntry(dVal)); + } return ldc(value); } @@ -2122,10 +2124,7 @@ default CodeBuilder ldc(ConstantDesc value) { * @return this builder */ default CodeBuilder ldc(LoadableConstantEntry entry) { - return with(ConstantInstruction.ofLoad( - entry.typeKind().slotSize() == 2 ? Opcode.LDC2_W - : entry.index() > 0xff ? Opcode.LDC_W - : Opcode.LDC, entry)); + return with(ConstantInstruction.ofLoad(BytecodeHelpers.ldcOpcode(entry), entry)); } /** diff --git a/src/java.base/share/classes/jdk/internal/classfile/impl/AbstractInstruction.java b/src/java.base/share/classes/jdk/internal/classfile/impl/AbstractInstruction.java index 23bf3769608..463668dcb00 100644 --- a/src/java.base/share/classes/jdk/internal/classfile/impl/AbstractInstruction.java +++ b/src/java.base/share/classes/jdk/internal/classfile/impl/AbstractInstruction.java @@ -1317,11 +1317,6 @@ public UnboundIntrinsicConstantInstruction(Opcode op) { constant = op.constantValue(); } - @Override - public void writeTo(DirectCodeBuilder writer) { - super.writeTo(writer); - } - @Override public ConstantDesc constantValue() { return constant; diff --git a/src/java.base/share/classes/jdk/internal/classfile/impl/BytecodeHelpers.java b/src/java.base/share/classes/jdk/internal/classfile/impl/BytecodeHelpers.java index 474189121fd..170772542ff 100644 --- a/src/java.base/share/classes/jdk/internal/classfile/impl/BytecodeHelpers.java +++ b/src/java.base/share/classes/jdk/internal/classfile/impl/BytecodeHelpers.java @@ -230,26 +230,38 @@ public static Opcode convertOpcode(TypeKind from, TypeKind to) { }; } - static void validateSIPUSH(ConstantDesc d) { - if (d instanceof Integer iVal && Short.MIN_VALUE <= iVal && iVal <= Short.MAX_VALUE) - return; - - if (d instanceof Long lVal && Short.MIN_VALUE <= lVal && Short.MAX_VALUE <= lVal) - return; - - throw new IllegalArgumentException("SIPUSH: value must be within: Short.MIN_VALUE <= value <= Short.MAX_VALUE" - + ", found: " + d); + static void validateSipush(long value) { + if (value < Short.MIN_VALUE || Short.MAX_VALUE < value) + throw new IllegalArgumentException( + "SIPUSH: value must be within: Short.MIN_VALUE <= value <= Short.MAX_VALUE, found: " + .concat(Long.toString(value))); } - static void validateBIPUSH(ConstantDesc d) { - if (d instanceof Integer iVal && Byte.MIN_VALUE <= iVal && iVal <= Byte.MAX_VALUE) - return; + static void validateBipush(long value) { + if (value < Byte.MIN_VALUE || Byte.MAX_VALUE < value) + throw new IllegalArgumentException( + "BIPUSH: value must be within: Byte.MIN_VALUE <= value <= Byte.MAX_VALUE, found: " + .concat(Long.toString(value))); + } - if (d instanceof Long lVal && Byte.MIN_VALUE <= lVal && Byte.MAX_VALUE <= lVal) - return; + static void validateSipush(ConstantDesc d) { + if (d instanceof Integer iVal) { + validateSipush(iVal.longValue()); + } else if (d instanceof Long lVal) { + validateSipush(lVal.longValue()); + } else { + throw new IllegalArgumentException("SIPUSH: not an integral number: ".concat(d.toString())); + } + } - throw new IllegalArgumentException("BIPUSH: value must be within: Byte.MIN_VALUE <= value <= Byte.MAX_VALUE" - + ", found: " + d); + static void validateBipush(ConstantDesc d) { + if (d instanceof Integer iVal) { + validateBipush(iVal.longValue()); + } else if (d instanceof Long lVal) { + validateBipush(lVal.longValue()); + } else { + throw new IllegalArgumentException("BIPUSH: not an integral number: ".concat(d.toString())); + } } public static MethodHandleEntry handleDescToHandleInfo(ConstantPoolBuilder constantPool, DirectMethodHandleDesc bootstrapMethod) { @@ -289,9 +301,9 @@ public static void validateValue(Opcode opcode, ConstantDesc v) { throw new IllegalArgumentException("value must be null or ConstantDescs.NULL with opcode ACONST_NULL"); } case SIPUSH -> - validateSIPUSH(v); + validateSipush(v); case BIPUSH -> - validateBIPUSH(v); + validateBipush(v); case LDC, LDC_W, LDC2_W -> { if (v == null) throw new IllegalArgumentException("`null` must use ACONST_NULL"); @@ -308,6 +320,12 @@ public static void validateValue(Opcode opcode, ConstantDesc v) { } } + public static Opcode ldcOpcode(LoadableConstantEntry entry) { + return entry.typeKind().slotSize() == 2 ? Opcode.LDC2_W + : entry.index() > 0xff ? Opcode.LDC_W + : Opcode.LDC; + } + public static LoadableConstantEntry constantEntry(ConstantPoolBuilder constantPool, ConstantDesc constantValue) { // this method is invoked during JVM bootstrap - cannot use pattern switch diff --git a/src/java.base/share/classes/jdk/internal/classfile/impl/DirectCodeBuilder.java b/src/java.base/share/classes/jdk/internal/classfile/impl/DirectCodeBuilder.java index a7652ac369b..1d2cbb5c84b 100644 --- a/src/java.base/share/classes/jdk/internal/classfile/impl/DirectCodeBuilder.java +++ b/src/java.base/share/classes/jdk/internal/classfile/impl/DirectCodeBuilder.java @@ -24,15 +24,6 @@ */ package jdk.internal.classfile.impl; -import java.util.ArrayList; -import java.util.Comparator; -import java.util.HashMap; -import java.util.IdentityHashMap; -import java.util.List; -import java.util.Map; -import java.util.function.Consumer; -import java.util.function.Function; - import java.lang.classfile.Attribute; import java.lang.classfile.Attributes; import java.lang.classfile.ClassFile; @@ -43,7 +34,6 @@ import java.lang.classfile.Label; import java.lang.classfile.Opcode; import java.lang.classfile.TypeKind; -import java.lang.classfile.instruction.SwitchCase; import java.lang.classfile.attribute.CodeAttribute; import java.lang.classfile.attribute.LineNumberTableAttribute; import java.lang.classfile.constantpool.ClassEntry; @@ -55,19 +45,23 @@ import java.lang.classfile.constantpool.LoadableConstantEntry; import java.lang.classfile.constantpool.LongEntry; import java.lang.classfile.constantpool.MemberRefEntry; +import java.lang.classfile.constantpool.MethodRefEntry; import java.lang.classfile.instruction.CharacterRange; import java.lang.classfile.instruction.ExceptionCatch; import java.lang.classfile.instruction.LocalVariable; import java.lang.classfile.instruction.LocalVariableType; +import java.lang.classfile.instruction.SwitchCase; +import java.lang.constant.ConstantDesc; +import java.util.ArrayList; +import java.util.Comparator; +import java.util.HashMap; +import java.util.IdentityHashMap; +import java.util.List; +import java.util.Map; +import java.util.function.Consumer; +import java.util.function.Function; -import static java.lang.classfile.Opcode.GOTO; -import static java.lang.classfile.Opcode.GOTO_W; -import static java.lang.classfile.Opcode.IINC; -import static java.lang.classfile.Opcode.IINC_W; -import static java.lang.classfile.Opcode.JSR; -import static java.lang.classfile.Opcode.JSR_W; -import static java.lang.classfile.Opcode.LDC2_W; -import static java.lang.classfile.Opcode.LDC_W; +import static java.lang.classfile.Opcode.*; public final class DirectCodeBuilder extends AbstractDirectBuilder @@ -543,7 +537,7 @@ public void writeBranch(Opcode op, Label target) { public void writeLookupSwitch(Label defaultTarget, List cases) { int instructionPc = curPc(); - writeBytecode(Opcode.LOOKUPSWITCH); + writeBytecode(LOOKUPSWITCH); int pad = 4 - (curPc() % 4); if (pad != 4) bytecodesBufWriter.writeIntBytes(pad, 0); @@ -564,7 +558,7 @@ public int compare(SwitchCase c1, SwitchCase c2) { public void writeTableSwitch(int low, int high, Label defaultTarget, List cases) { int instructionPc = curPc(); - writeBytecode(Opcode.TABLESWITCH); + writeBytecode(TABLESWITCH); int pad = 4 - (curPc() % 4); if (pad != 4) bytecodesBufWriter.writeIntBytes(pad, 0); @@ -600,28 +594,28 @@ public void writeInvokeInterface(Opcode opcode, } public void writeInvokeDynamic(InvokeDynamicEntry ref) { - writeBytecode(Opcode.INVOKEDYNAMIC); + writeBytecode(INVOKEDYNAMIC); bytecodesBufWriter.writeIndex(ref); bytecodesBufWriter.writeU2(0); } public void writeNewObject(ClassEntry type) { - writeBytecode(Opcode.NEW); + writeBytecode(NEW); bytecodesBufWriter.writeIndex(type); } public void writeNewPrimitiveArray(int newArrayCode) { - writeBytecode(Opcode.NEWARRAY); + writeBytecode(NEWARRAY); bytecodesBufWriter.writeU1(newArrayCode); } public void writeNewReferenceArray(ClassEntry type) { - writeBytecode(Opcode.ANEWARRAY); + writeBytecode(ANEWARRAY); bytecodesBufWriter.writeIndex(type); } public void writeNewMultidimensionalArray(int dimensions, ClassEntry type) { - writeBytecode(Opcode.MULTIANEWARRAY); + writeBytecode(MULTIANEWARRAY); bytecodesBufWriter.writeIndex(type); bytecodesBufWriter.writeU1(dimensions); } @@ -777,4 +771,704 @@ public LabelOverflowException() { super("Label target offset overflow"); } } + + // Fast overrides to avoid intermediate instructions + // These are helpful for direct class building + + @Override + public CodeBuilder return_(TypeKind tk) { + writeBytecode(BytecodeHelpers.returnOpcode(tk)); + return this; + } + + @Override + public CodeBuilder storeLocal(TypeKind tk, int slot) { + writeLocalVar(BytecodeHelpers.storeOpcode(tk, slot), slot); + return this; + } + + @Override + public CodeBuilder loadLocal(TypeKind tk, int slot) { + writeLocalVar(BytecodeHelpers.loadOpcode(tk, slot), slot); + return this; + } + + @Override + public CodeBuilder invoke(Opcode opcode, MemberRefEntry ref) { + if (opcode == INVOKEINTERFACE) { + int slots = Util.parameterSlots(Util.methodTypeSymbol(ref.nameAndType())) + 1; + writeInvokeInterface(opcode, (InterfaceMethodRefEntry) ref, slots); + } else { + writeInvokeNormal(opcode, ref); + } + return this; + } + + @Override + public CodeBuilder fieldAccess(Opcode opcode, FieldRefEntry ref) { + writeFieldAccess(opcode, ref); + return this; + } + + @Override + public CodeBuilder arrayLoad(TypeKind tk) { + writeBytecode(BytecodeHelpers.arrayLoadOpcode(tk)); + return this; + } + + @Override + public CodeBuilder arrayStore(TypeKind tk) { + writeBytecode(BytecodeHelpers.arrayStoreOpcode(tk)); + return this; + } + + @Override + public CodeBuilder branch(Opcode op, Label target) { + writeBranch(op, target); + return this; + } + + @Override + public CodeBuilder loadConstant(Opcode opcode, ConstantDesc value) { + BytecodeHelpers.validateValue(opcode, value); + // avoid non-local enum switch for bootstrap + if (opcode == BIPUSH || opcode == SIPUSH) { + writeArgumentConstant(opcode, ((Number) value).intValue()); + } else if (opcode == LDC || opcode == LDC_W || opcode == LDC2_W) { + writeLoadConstant(opcode, BytecodeHelpers.constantEntry(constantPool(), value)); + } else { + // intrinsics + writeBytecode(opcode); + } + return this; + } + + @Override + public CodeBuilder nop() { + writeBytecode(NOP); + return this; + } + + @Override + public CodeBuilder aconst_null() { + writeBytecode(ACONST_NULL); + return this; + } + + @Override + public CodeBuilder anewarray(ClassEntry entry) { + writeNewReferenceArray(entry); + return this; + } + + @Override + public CodeBuilder arraylength() { + writeBytecode(ARRAYLENGTH); + return this; + } + + @Override + public CodeBuilder athrow() { + writeBytecode(ATHROW); + return this; + } + + @Override + public CodeBuilder bipush(int b) { + BytecodeHelpers.validateBipush(b); + writeArgumentConstant(BIPUSH, b); + return this; + } + + @Override + public CodeBuilder checkcast(ClassEntry type) { + writeTypeCheck(CHECKCAST, type); + return this; + } + + @Override + public CodeBuilder d2f() { + writeBytecode(D2F); + return this; + } + + @Override + public CodeBuilder d2i() { + writeBytecode(D2I); + return this; + } + + @Override + public CodeBuilder d2l() { + writeBytecode(D2L); + return this; + } + + @Override + public CodeBuilder dadd() { + writeBytecode(DADD); + return this; + } + + @Override + public CodeBuilder dcmpg() { + writeBytecode(DCMPG); + return this; + } + + @Override + public CodeBuilder dcmpl() { + writeBytecode(DCMPL); + return this; + } + + @Override + public CodeBuilder dconst_0() { + writeBytecode(DCONST_0); + return this; + } + + @Override + public CodeBuilder dconst_1() { + writeBytecode(DCONST_1); + return this; + } + + @Override + public CodeBuilder ddiv() { + writeBytecode(DDIV); + return this; + } + + @Override + public CodeBuilder dmul() { + writeBytecode(DMUL); + return this; + } + + @Override + public CodeBuilder dneg() { + writeBytecode(DNEG); + return this; + } + + @Override + public CodeBuilder drem() { + writeBytecode(DREM); + return this; + } + + @Override + public CodeBuilder dsub() { + writeBytecode(DSUB); + return this; + } + + @Override + public CodeBuilder dup() { + writeBytecode(DUP); + return this; + } + + @Override + public CodeBuilder dup2() { + writeBytecode(DUP2); + return this; + } + + @Override + public CodeBuilder dup2_x1() { + writeBytecode(DUP2_X1); + return this; + } + + @Override + public CodeBuilder dup2_x2() { + writeBytecode(DUP2_X2); + return this; + } + + @Override + public CodeBuilder dup_x1() { + writeBytecode(DUP_X1); + return this; + } + + @Override + public CodeBuilder dup_x2() { + writeBytecode(DUP_X2); + return this; + } + + @Override + public CodeBuilder f2d() { + writeBytecode(F2D); + return this; + } + + @Override + public CodeBuilder f2i() { + writeBytecode(F2I); + return this; + } + + @Override + public CodeBuilder f2l() { + writeBytecode(F2L); + return this; + } + + @Override + public CodeBuilder fadd() { + writeBytecode(FADD); + return this; + } + + @Override + public CodeBuilder fcmpg() { + writeBytecode(FCMPG); + return this; + } + + @Override + public CodeBuilder fcmpl() { + writeBytecode(FCMPL); + return this; + } + + @Override + public CodeBuilder fconst_0() { + writeBytecode(FCONST_0); + return this; + } + + @Override + public CodeBuilder fconst_1() { + writeBytecode(FCONST_1); + return this; + } + + @Override + public CodeBuilder fconst_2() { + writeBytecode(FCONST_2); + return this; + } + + @Override + public CodeBuilder fdiv() { + writeBytecode(FDIV); + return this; + } + + @Override + public CodeBuilder fmul() { + writeBytecode(FMUL); + return this; + } + + @Override + public CodeBuilder fneg() { + writeBytecode(FNEG); + return this; + } + + @Override + public CodeBuilder frem() { + writeBytecode(FREM); + return this; + } + + @Override + public CodeBuilder fsub() { + writeBytecode(FSUB); + return this; + } + + @Override + public CodeBuilder i2b() { + writeBytecode(I2B); + return this; + } + + @Override + public CodeBuilder i2c() { + writeBytecode(I2C); + return this; + } + + @Override + public CodeBuilder i2d() { + writeBytecode(I2D); + return this; + } + + @Override + public CodeBuilder i2f() { + writeBytecode(I2F); + return this; + } + + @Override + public CodeBuilder i2l() { + writeBytecode(I2L); + return this; + } + + @Override + public CodeBuilder i2s() { + writeBytecode(I2S); + return this; + } + + @Override + public CodeBuilder iadd() { + writeBytecode(IADD); + return this; + } + + @Override + public CodeBuilder iand() { + writeBytecode(IAND); + return this; + } + + @Override + public CodeBuilder iconst_0() { + writeBytecode(ICONST_0); + return this; + } + + @Override + public CodeBuilder iconst_1() { + writeBytecode(ICONST_1); + return this; + } + + @Override + public CodeBuilder iconst_2() { + writeBytecode(ICONST_2); + return this; + } + + @Override + public CodeBuilder iconst_3() { + writeBytecode(ICONST_3); + return this; + } + + @Override + public CodeBuilder iconst_4() { + writeBytecode(ICONST_4); + return this; + } + + @Override + public CodeBuilder iconst_5() { + writeBytecode(ICONST_5); + return this; + } + + @Override + public CodeBuilder iconst_m1() { + writeBytecode(ICONST_M1); + return this; + } + + @Override + public CodeBuilder idiv() { + writeBytecode(IDIV); + return this; + } + + @Override + public CodeBuilder iinc(int slot, int val) { + writeIncrement(slot, val); + return this; + } + + @Override + public CodeBuilder imul() { + writeBytecode(IMUL); + return this; + } + + @Override + public CodeBuilder ineg() { + writeBytecode(INEG); + return this; + } + + @Override + public CodeBuilder instanceOf(ClassEntry target) { + writeTypeCheck(INSTANCEOF, target); + return this; + } + + @Override + public CodeBuilder invokedynamic(InvokeDynamicEntry ref) { + writeInvokeDynamic(ref); + return this; + } + + @Override + public CodeBuilder invokeinterface(InterfaceMethodRefEntry ref) { + writeInvokeInterface(INVOKEINTERFACE, ref, Util.parameterSlots(ref.typeSymbol()) + 1); + return this; + } + + @Override + public CodeBuilder invokespecial(InterfaceMethodRefEntry ref) { + writeInvokeNormal(INVOKESPECIAL, ref); + return this; + } + + @Override + public CodeBuilder invokespecial(MethodRefEntry ref) { + writeInvokeNormal(INVOKESPECIAL, ref); + return this; + } + + @Override + public CodeBuilder invokestatic(InterfaceMethodRefEntry ref) { + writeInvokeNormal(INVOKESTATIC, ref); + return this; + } + + @Override + public CodeBuilder invokestatic(MethodRefEntry ref) { + writeInvokeNormal(INVOKESTATIC, ref); + return this; + } + + @Override + public CodeBuilder invokevirtual(MethodRefEntry ref) { + writeInvokeNormal(INVOKEVIRTUAL, ref); + return this; + } + + @Override + public CodeBuilder ior() { + writeBytecode(IOR); + return this; + } + + @Override + public CodeBuilder irem() { + writeBytecode(IREM); + return this; + } + + @Override + public CodeBuilder ishl() { + writeBytecode(ISHL); + return this; + } + + @Override + public CodeBuilder ishr() { + writeBytecode(ISHR); + return this; + } + + @Override + public CodeBuilder isub() { + writeBytecode(ISUB); + return this; + } + + @Override + public CodeBuilder iushr() { + writeBytecode(IUSHR); + return this; + } + + @Override + public CodeBuilder ixor() { + writeBytecode(IXOR); + return this; + } + + @Override + public CodeBuilder lookupswitch(Label defaultTarget, List cases) { + writeLookupSwitch(defaultTarget, cases); + return this; + } + + @Override + public CodeBuilder l2d() { + writeBytecode(L2D); + return this; + } + + @Override + public CodeBuilder l2f() { + writeBytecode(L2F); + return this; + } + + @Override + public CodeBuilder l2i() { + writeBytecode(L2I); + return this; + } + + @Override + public CodeBuilder ladd() { + writeBytecode(LADD); + return this; + } + + @Override + public CodeBuilder land() { + writeBytecode(LAND); + return this; + } + + @Override + public CodeBuilder lcmp() { + writeBytecode(LCMP); + return this; + } + + @Override + public CodeBuilder lconst_0() { + writeBytecode(LCONST_0); + return this; + } + + @Override + public CodeBuilder lconst_1() { + writeBytecode(LCONST_1); + return this; + } + + @Override + public CodeBuilder ldc(LoadableConstantEntry entry) { + writeLoadConstant(BytecodeHelpers.ldcOpcode(entry), entry); + return this; + } + + @Override + public CodeBuilder ldiv() { + writeBytecode(LDIV); + return this; + } + + @Override + public CodeBuilder lmul() { + writeBytecode(LMUL); + return this; + } + + @Override + public CodeBuilder lneg() { + writeBytecode(LNEG); + return this; + } + + @Override + public CodeBuilder lor() { + writeBytecode(LOR); + return this; + } + + @Override + public CodeBuilder lrem() { + writeBytecode(LREM); + return this; + } + + @Override + public CodeBuilder lshl() { + writeBytecode(LSHL); + return this; + } + + @Override + public CodeBuilder lshr() { + writeBytecode(LSHR); + return this; + } + + @Override + public CodeBuilder lsub() { + writeBytecode(LSUB); + return this; + } + + @Override + public CodeBuilder lushr() { + writeBytecode(LUSHR); + return this; + } + + @Override + public CodeBuilder lxor() { + writeBytecode(LXOR); + return this; + } + + @Override + public CodeBuilder monitorenter() { + writeBytecode(MONITORENTER); + return this; + } + + @Override + public CodeBuilder monitorexit() { + writeBytecode(MONITOREXIT); + return this; + } + + @Override + public CodeBuilder multianewarray(ClassEntry array, int dims) { + writeNewMultidimensionalArray(dims, array); + return this; + } + + @Override + public CodeBuilder new_(ClassEntry clazz) { + writeNewObject(clazz); + return this; + } + + @Override + public CodeBuilder newarray(TypeKind typeKind) { + int atype = typeKind.newarrayCode(); // implicit null check + if (atype < 0) + throw new IllegalArgumentException("Illegal component type: " + typeKind.typeName()); + writeNewPrimitiveArray(atype); + return this; + } + + @Override + public CodeBuilder pop() { + writeBytecode(POP); + return this; + } + + @Override + public CodeBuilder pop2() { + writeBytecode(POP2); + return this; + } + + @Override + public CodeBuilder sipush(int s) { + BytecodeHelpers.validateSipush(s); + writeArgumentConstant(SIPUSH, s); + return this; + } + + @Override + public CodeBuilder swap() { + writeBytecode(SWAP); + return this; + } + + @Override + public CodeBuilder tableswitch(int low, int high, Label defaultTarget, List cases) { + writeTableSwitch(low, high, defaultTarget, cases); + return this; + } } From a4962ace4d3afb36e9d6822a4f02a1515fac40ed Mon Sep 17 00:00:00 2001 From: David Holmes Date: Thu, 29 Aug 2024 20:38:52 +0000 Subject: [PATCH 06/11] 8338257: UTF8 lengths should be size_t not int Reviewed-by: stuefe, coleenp, dlong --- .../share/classfile/compactHashtable.cpp | 4 +- .../share/classfile/compactHashtable.hpp | 2 +- src/hotspot/share/classfile/javaClasses.cpp | 65 +++++++--- src/hotspot/share/classfile/javaClasses.hpp | 18 +-- src/hotspot/share/classfile/modules.cpp | 14 ++- src/hotspot/share/classfile/stringTable.cpp | 4 +- src/hotspot/share/classfile/symbolTable.cpp | 17 +-- src/hotspot/share/jfr/dcmd/jfrDcmds.cpp | 6 +- src/hotspot/share/jfr/jni/jfrJavaSupport.cpp | 6 +- .../checkpoint/types/jfrThreadState.cpp | 7 +- src/hotspot/share/oops/symbol.cpp | 2 +- src/hotspot/share/prims/jni.cpp | 7 +- src/hotspot/share/prims/jvmtiEnv.cpp | 2 +- .../share/services/finalizerService.cpp | 4 +- src/hotspot/share/utilities/utf8.cpp | 113 +++++++++++------- src/hotspot/share/utilities/utf8.hpp | 80 ++++++++++--- 16 files changed, 229 insertions(+), 122 deletions(-) diff --git a/src/hotspot/share/classfile/compactHashtable.cpp b/src/hotspot/share/classfile/compactHashtable.cpp index d4657e35a84..57991589fdc 100644 --- a/src/hotspot/share/classfile/compactHashtable.cpp +++ b/src/hotspot/share/classfile/compactHashtable.cpp @@ -431,10 +431,10 @@ void HashtableTextDump::get_utf8(char* utf8_buffer, int utf8_length) { } // NOTE: the content is NOT the same as -// UTF8::as_quoted_ascii(const char* utf8_str, int utf8_length, char* buf, int buflen). +// UTF8::as_quoted_ascii(const char* utf8_str, int utf8_length, char* buf, size_t buflen). // We want to escape \r\n\t so that output [1] is more readable; [2] can be more easily // parsed by scripts; [3] quickly processed by HashtableTextDump::get_utf8() -void HashtableTextDump::put_utf8(outputStream* st, const char* utf8_string, int utf8_length) { +void HashtableTextDump::put_utf8(outputStream* st, const char* utf8_string, size_t utf8_length) { const char *c = utf8_string; const char *end = c + utf8_length; for (; c < end; c++) { diff --git a/src/hotspot/share/classfile/compactHashtable.hpp b/src/hotspot/share/classfile/compactHashtable.hpp index 6cb689ad20d..73e9f7fc092 100644 --- a/src/hotspot/share/classfile/compactHashtable.hpp +++ b/src/hotspot/share/classfile/compactHashtable.hpp @@ -431,7 +431,7 @@ class HashtableTextDump { int unescape(const char* from, const char* end, int count); void get_utf8(char* utf8_buffer, int utf8_length); - static void put_utf8(outputStream* st, const char* utf8_string, int utf8_length); + static void put_utf8(outputStream* st, const char* utf8_string, size_t utf8_length); }; #endif // SHARE_CLASSFILE_COMPACTHASHTABLE_HPP diff --git a/src/hotspot/share/classfile/javaClasses.cpp b/src/hotspot/share/classfile/javaClasses.cpp index 8a6c8a8ce0b..b6ef682ae09 100644 --- a/src/hotspot/share/classfile/javaClasses.cpp +++ b/src/hotspot/share/classfile/javaClasses.cpp @@ -304,7 +304,8 @@ Handle java_lang_String::create_from_unicode(const jchar* unicode, int length, T #ifdef ASSERT { ResourceMark rm; - char* expected = UNICODE::as_utf8(unicode, length); + size_t utf8_len = static_cast(length); + char* expected = UNICODE::as_utf8(unicode, utf8_len); char* actual = as_utf8_string(h_obj()); if (strcmp(expected, actual) != 0) { fatal("Unicode conversion failure: %s --> %s", expected, actual); @@ -346,7 +347,7 @@ Handle java_lang_String::create_from_str(const char* utf8_str, TRAPS) { #ifdef ASSERT // This check is too strict when the input string is not a valid UTF8. // For example, it may be created with arbitrary content via jni_NewStringUTF. - if (UTF8::is_legal_utf8((const unsigned char*)utf8_str, (int)strlen(utf8_str), false)) { + if (UTF8::is_legal_utf8((const unsigned char*)utf8_str, strlen(utf8_str), false)) { ResourceMark rm; const char* expected = utf8_str; char* actual = as_utf8_string(h_obj()); @@ -554,7 +555,7 @@ char* java_lang_String::as_quoted_ascii(oop java_string) { if (length == 0) return nullptr; char* result; - int result_length; + size_t result_length; if (!is_latin1) { jchar* base = value->char_at_addr(0); result_length = UNICODE::quoted_ascii_length(base, length) + 1; @@ -566,8 +567,8 @@ char* java_lang_String::as_quoted_ascii(oop java_string) { result = NEW_RESOURCE_ARRAY(char, result_length); UNICODE::as_quoted_ascii(base, length, result, result_length); } - assert(result_length >= length + 1, "must not be shorter"); - assert(result_length == (int)strlen(result) + 1, "must match"); + assert(result_length >= (size_t)length + 1, "must not be shorter"); + assert(result_length == strlen(result) + 1, "must match"); return result; } @@ -582,8 +583,9 @@ Symbol* java_lang_String::as_symbol(oop java_string) { } else { ResourceMark rm; jbyte* position = (length == 0) ? nullptr : value->byte_at_addr(0); - const char* base = UNICODE::as_utf8(position, length); - Symbol* sym = SymbolTable::new_symbol(base, length); + size_t utf8_len = static_cast(length); + const char* base = UNICODE::as_utf8(position, utf8_len); + Symbol* sym = SymbolTable::new_symbol(base, checked_cast(utf8_len)); return sym; } } @@ -598,12 +600,13 @@ Symbol* java_lang_String::as_symbol_or_null(oop java_string) { } else { ResourceMark rm; jbyte* position = (length == 0) ? nullptr : value->byte_at_addr(0); - const char* base = UNICODE::as_utf8(position, length); - return SymbolTable::probe(base, length); + size_t utf8_len = static_cast(length); + const char* base = UNICODE::as_utf8(position, utf8_len); + return SymbolTable::probe(base, checked_cast(utf8_len)); } } -int java_lang_String::utf8_length(oop java_string, typeArrayOop value) { +size_t java_lang_String::utf8_length(oop java_string, typeArrayOop value) { assert(value_equals(value, java_lang_String::value(java_string)), "value must be same as java_lang_String::value(java_string)"); int length = java_lang_String::length(java_string, value); @@ -617,18 +620,39 @@ int java_lang_String::utf8_length(oop java_string, typeArrayOop value) { } } -int java_lang_String::utf8_length(oop java_string) { +size_t java_lang_String::utf8_length(oop java_string) { typeArrayOop value = java_lang_String::value(java_string); return utf8_length(java_string, value); } +int java_lang_String::utf8_length_as_int(oop java_string) { + typeArrayOop value = java_lang_String::value(java_string); + return utf8_length_as_int(java_string, value); +} + +int java_lang_String::utf8_length_as_int(oop java_string, typeArrayOop value) { + assert(value_equals(value, java_lang_String::value(java_string)), + "value must be same as java_lang_String::value(java_string)"); + int length = java_lang_String::length(java_string, value); + if (length == 0) { + return 0; + } + if (!java_lang_String::is_latin1(java_string)) { + return UNICODE::utf8_length_as_int(value->char_at_addr(0), length); + } else { + return UNICODE::utf8_length_as_int(value->byte_at_addr(0), length); + } +} + char* java_lang_String::as_utf8_string(oop java_string) { - int length; + size_t length; return as_utf8_string(java_string, length); } -char* java_lang_String::as_utf8_string(oop java_string, int& length) { +char* java_lang_String::as_utf8_string(oop java_string, size_t& length) { typeArrayOop value = java_lang_String::value(java_string); + // `length` is used as the incoming number of characters to + // convert, and then set as the number of bytes in the UTF8 sequence. length = java_lang_String::length(java_string, value); bool is_latin1 = java_lang_String::is_latin1(java_string); if (!is_latin1) { @@ -642,7 +666,7 @@ char* java_lang_String::as_utf8_string(oop java_string, int& length) { // Uses a provided buffer if it's sufficiently large, otherwise allocates // a resource array to fit -char* java_lang_String::as_utf8_string_full(oop java_string, char* buf, int buflen, int& utf8_len) { +char* java_lang_String::as_utf8_string_full(oop java_string, char* buf, size_t buflen, size_t& utf8_len) { typeArrayOop value = java_lang_String::value(java_string); int len = java_lang_String::length(java_string, value); bool is_latin1 = java_lang_String::is_latin1(java_string); @@ -663,7 +687,7 @@ char* java_lang_String::as_utf8_string_full(oop java_string, char* buf, int bufl } } -char* java_lang_String::as_utf8_string(oop java_string, typeArrayOop value, char* buf, int buflen) { +char* java_lang_String::as_utf8_string(oop java_string, typeArrayOop value, char* buf, size_t buflen) { assert(value_equals(value, java_lang_String::value(java_string)), "value must be same as java_lang_String::value(java_string)"); int length = java_lang_String::length(java_string, value); @@ -677,25 +701,28 @@ char* java_lang_String::as_utf8_string(oop java_string, typeArrayOop value, char } } -char* java_lang_String::as_utf8_string(oop java_string, char* buf, int buflen) { +char* java_lang_String::as_utf8_string(oop java_string, char* buf, size_t buflen) { typeArrayOop value = java_lang_String::value(java_string); return as_utf8_string(java_string, value, buf, buflen); } char* java_lang_String::as_utf8_string(oop java_string, int start, int len) { + // `length` is used as the incoming number of characters to + // convert, and then set as the number of bytes in the UTF8 sequence. + size_t length = static_cast(len); typeArrayOop value = java_lang_String::value(java_string); bool is_latin1 = java_lang_String::is_latin1(java_string); assert(start + len <= java_lang_String::length(java_string), "just checking"); if (!is_latin1) { jchar* position = value->char_at_addr(start); - return UNICODE::as_utf8(position, len); + return UNICODE::as_utf8(position, length); } else { jbyte* position = value->byte_at_addr(start); - return UNICODE::as_utf8(position, len); + return UNICODE::as_utf8(position, length); } } -char* java_lang_String::as_utf8_string(oop java_string, typeArrayOop value, int start, int len, char* buf, int buflen) { +char* java_lang_String::as_utf8_string(oop java_string, typeArrayOop value, int start, int len, char* buf, size_t buflen) { assert(value_equals(value, java_lang_String::value(java_string)), "value must be same as java_lang_String::value(java_string)"); assert(start + len <= java_lang_String::length(java_string), "just checking"); diff --git a/src/hotspot/share/classfile/javaClasses.hpp b/src/hotspot/share/classfile/javaClasses.hpp index eb3c5e29fdb..6f82a75e8ff 100644 --- a/src/hotspot/share/classfile/javaClasses.hpp +++ b/src/hotspot/share/classfile/javaClasses.hpp @@ -131,17 +131,21 @@ class java_lang_String : AllStatic { static inline bool deduplication_requested(oop java_string); static inline int length(oop java_string); static inline int length(oop java_string, typeArrayOop string_value); - static int utf8_length(oop java_string); - static int utf8_length(oop java_string, typeArrayOop string_value); + static size_t utf8_length(oop java_string); + static size_t utf8_length(oop java_string, typeArrayOop string_value); + // Legacy variants that truncate the length if needed + static int utf8_length_as_int(oop java_string); + static int utf8_length_as_int(oop java_string, typeArrayOop string_value); // String converters static char* as_utf8_string(oop java_string); - static char* as_utf8_string(oop java_string, int& length); - static char* as_utf8_string_full(oop java_string, char* buf, int buflen, int& length); - static char* as_utf8_string(oop java_string, char* buf, int buflen); + // `length` is set to the length of the utf8 sequence. + static char* as_utf8_string(oop java_string, size_t& length); + static char* as_utf8_string_full(oop java_string, char* buf, size_t buflen, size_t& length); + static char* as_utf8_string(oop java_string, char* buf, size_t buflen); static char* as_utf8_string(oop java_string, int start, int len); - static char* as_utf8_string(oop java_string, typeArrayOop value, char* buf, int buflen); - static char* as_utf8_string(oop java_string, typeArrayOop value, int start, int len, char* buf, int buflen); + static char* as_utf8_string(oop java_string, typeArrayOop value, char* buf, size_t buflen); + static char* as_utf8_string(oop java_string, typeArrayOop value, int start, int len, char* buf, size_t buflen); static char* as_platform_dependent_str(Handle java_string, TRAPS); static jchar* as_unicode_string(oop java_string, int& length, TRAPS); static jchar* as_unicode_string_or_null(oop java_string, int& length); diff --git a/src/hotspot/share/classfile/modules.cpp b/src/hotspot/share/classfile/modules.cpp index ddbb84db3be..14e730f7a33 100644 --- a/src/hotspot/share/classfile/modules.cpp +++ b/src/hotspot/share/classfile/modules.cpp @@ -72,7 +72,9 @@ static char* get_module_name(oop module, int& len, TRAPS) { if (name_oop == nullptr) { THROW_MSG_NULL(vmSymbols::java_lang_NullPointerException(), "Null module name"); } - char* module_name = java_lang_String::as_utf8_string(name_oop, len); + size_t utf8_len; + char* module_name = java_lang_String::as_utf8_string(name_oop, utf8_len); + len = checked_cast(utf8_len); // module names are < 64K if (!verify_module_name(module_name, len)) { THROW_MSG_NULL(vmSymbols::java_lang_IllegalArgumentException(), err_msg("Invalid module name: %s", module_name)); @@ -84,9 +86,9 @@ static Symbol* as_symbol(jstring str_object) { if (str_object == nullptr) { return nullptr; } - int len; + size_t len; char* str = java_lang_String::as_utf8_string(JNIHandles::resolve_non_null(str_object), len); - return SymbolTable::new_symbol(str, len); + return SymbolTable::new_symbol(str, checked_cast(len)); } ModuleEntryTable* Modules::get_module_entry_table(Handle h_loader) { @@ -142,8 +144,10 @@ bool Modules::is_package_defined(Symbol* package, Handle h_loader) { // Will use the provided buffer if it's sufficiently large, otherwise allocates // a resource array // The length of the resulting string will be assigned to utf8_len -static const char* as_internal_package(oop package_string, char* buf, int buflen, int& utf8_len) { - char* package_name = java_lang_String::as_utf8_string_full(package_string, buf, buflen, utf8_len); +static const char* as_internal_package(oop package_string, char* buf, size_t buflen, int& utf8_len) { + size_t full_utf8_len; + char* package_name = java_lang_String::as_utf8_string_full(package_string, buf, buflen, full_utf8_len); + utf8_len = checked_cast(full_utf8_len); // package names are < 64K // Turn all '/'s into '.'s for (int index = 0; index < utf8_len; index++) { diff --git a/src/hotspot/share/classfile/stringTable.cpp b/src/hotspot/share/classfile/stringTable.cpp index b01ecb24ac9..3a6cf166ff5 100644 --- a/src/hotspot/share/classfile/stringTable.cpp +++ b/src/hotspot/share/classfile/stringTable.cpp @@ -686,7 +686,7 @@ static void print_string(Thread* current, outputStream* st, oop s) { st->print("%d: ", length); } else { ResourceMark rm(current); - int utf8_length = length; + size_t utf8_length = length; char* utf8_string; if (!is_latin1) { @@ -697,7 +697,7 @@ static void print_string(Thread* current, outputStream* st, oop s) { utf8_string = UNICODE::as_utf8(bytes, utf8_length); } - st->print("%d: ", utf8_length); + st->print("%zu: ", utf8_length); HashtableTextDump::put_utf8(st, utf8_string, utf8_length); } st->cr(); diff --git a/src/hotspot/share/classfile/symbolTable.cpp b/src/hotspot/share/classfile/symbolTable.cpp index 95094238946..19306a2a9db 100644 --- a/src/hotspot/share/classfile/symbolTable.cpp +++ b/src/hotspot/share/classfile/symbolTable.cpp @@ -349,6 +349,7 @@ Symbol* SymbolTable::lookup_common(const char* name, // to be used for arbitrary strings. For debug builds we will assert if // a string is too long, whereas product builds will truncate it. static int check_length(const char* name, int len) { + assert(len >= 0, "negative length %d suggests integer overflow in the caller", len); assert(len <= Symbol::max_length(), "String length %d exceeds the maximum Symbol length of %d", len, Symbol::max_length()); if (len > Symbol::max_length()) { @@ -461,33 +462,33 @@ Symbol* SymbolTable::lookup_only(const char* name, int len, unsigned int& hash) // and probing logic, so there is no need for convert_to_utf8 until // an actual new Symbol* is created. Symbol* SymbolTable::new_symbol(const jchar* name, int utf16_length) { - int utf8_length = UNICODE::utf8_length((jchar*) name, utf16_length); + size_t utf8_length = UNICODE::utf8_length((jchar*) name, utf16_length); char stack_buf[ON_STACK_BUFFER_LENGTH]; - if (utf8_length < (int) sizeof(stack_buf)) { + if (utf8_length < sizeof(stack_buf)) { char* chars = stack_buf; UNICODE::convert_to_utf8(name, utf16_length, chars); - return new_symbol(chars, utf8_length); + return new_symbol(chars, checked_cast(utf8_length)); } else { ResourceMark rm; char* chars = NEW_RESOURCE_ARRAY(char, utf8_length + 1); UNICODE::convert_to_utf8(name, utf16_length, chars); - return new_symbol(chars, utf8_length); + return new_symbol(chars, checked_cast(utf8_length)); } } Symbol* SymbolTable::lookup_only_unicode(const jchar* name, int utf16_length, unsigned int& hash) { - int utf8_length = UNICODE::utf8_length((jchar*) name, utf16_length); + size_t utf8_length = UNICODE::utf8_length((jchar*) name, utf16_length); char stack_buf[ON_STACK_BUFFER_LENGTH]; - if (utf8_length < (int) sizeof(stack_buf)) { + if (utf8_length < sizeof(stack_buf)) { char* chars = stack_buf; UNICODE::convert_to_utf8(name, utf16_length, chars); - return lookup_only(chars, utf8_length, hash); + return lookup_only(chars, checked_cast(utf8_length), hash); } else { ResourceMark rm; char* chars = NEW_RESOURCE_ARRAY(char, utf8_length + 1); UNICODE::convert_to_utf8(name, utf16_length, chars); - return lookup_only(chars, utf8_length, hash); + return lookup_only(chars, checked_cast(utf8_length), hash); } } diff --git a/src/hotspot/share/jfr/dcmd/jfrDcmds.cpp b/src/hotspot/share/jfr/dcmd/jfrDcmds.cpp index c6944cfa219..56e006ab25c 100644 --- a/src/hotspot/share/jfr/dcmd/jfrDcmds.cpp +++ b/src/hotspot/share/jfr/dcmd/jfrDcmds.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2012, 2024, 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 @@ -292,10 +292,10 @@ static const char* get_as_dcmd_arena_string(oop string) { char* str = nullptr; const typeArrayOop value = java_lang_String::value(string); if (value != nullptr) { - const size_t length = static_cast(java_lang_String::utf8_length(string, value)) + 1; + const size_t length = java_lang_String::utf8_length(string, value) + 1; str = dcmd_arena_allocate(length); assert(str != nullptr, "invariant"); - java_lang_String::as_utf8_string(string, value, str, static_cast(length)); + java_lang_String::as_utf8_string(string, value, str, length); } return str; } diff --git a/src/hotspot/share/jfr/jni/jfrJavaSupport.cpp b/src/hotspot/share/jfr/jni/jfrJavaSupport.cpp index 0b3097ca1fa..4e2493fc251 100644 --- a/src/hotspot/share/jfr/jni/jfrJavaSupport.cpp +++ b/src/hotspot/share/jfr/jni/jfrJavaSupport.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2016, 2024, 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 @@ -502,7 +502,7 @@ Klass* JfrJavaSupport::klass(const jobject handle) { return obj->klass(); } -static char* allocate_string(bool c_heap, int length, Thread* thread) { +static char* allocate_string(bool c_heap, size_t length, Thread* thread) { return c_heap ? NEW_C_HEAP_ARRAY(char, length, mtTracing) : NEW_RESOURCE_ARRAY_IN_THREAD(thread, char, length); } @@ -511,7 +511,7 @@ const char* JfrJavaSupport::c_str(oop string, Thread* thread, bool c_heap /* fal char* str = nullptr; const typeArrayOop value = java_lang_String::value(string); if (value != nullptr) { - const int length = java_lang_String::utf8_length(string, value); + const size_t length = java_lang_String::utf8_length(string, value); str = allocate_string(c_heap, length + 1, thread); if (str == nullptr) { return nullptr; diff --git a/src/hotspot/share/jfr/recorder/checkpoint/types/jfrThreadState.cpp b/src/hotspot/share/jfr/recorder/checkpoint/types/jfrThreadState.cpp index bbc14327801..f6bf3e685b6 100644 --- a/src/hotspot/share/jfr/recorder/checkpoint/types/jfrThreadState.cpp +++ b/src/hotspot/share/jfr/recorder/checkpoint/types/jfrThreadState.cpp @@ -1,5 +1,5 @@ /* -* Copyright (c) 2016, 2022, Oracle and/or its affiliates. All rights reserved. +* Copyright (c) 2016, 2024, 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 @@ -121,7 +121,10 @@ static const char* get_java_thread_name(const JavaThread* jt, int& length, oop v } assert(thread_obj != nullptr, "invariant"); const oop name = java_lang_Thread::name(thread_obj); - return name != nullptr ? java_lang_String::as_utf8_string(name, length) : nullptr; + size_t utf8_len; + const char* ret = name != nullptr ? java_lang_String::as_utf8_string(name, utf8_len) : nullptr; + length = checked_cast(utf8_len); // Thread names should be short + return ret; } const char* JfrThreadName::name(const Thread* t, int& length, oop vthread) { diff --git a/src/hotspot/share/oops/symbol.cpp b/src/hotspot/share/oops/symbol.cpp index 8fe7c2aadbf..276a2018241 100644 --- a/src/hotspot/share/oops/symbol.cpp +++ b/src/hotspot/share/oops/symbol.cpp @@ -166,7 +166,7 @@ void Symbol::print_symbol_on(outputStream* st) const { char* Symbol::as_quoted_ascii() const { const char *ptr = (const char *)&_body[0]; - int quoted_length = UTF8::quoted_ascii_length(ptr, utf8_length()); + size_t quoted_length = UTF8::quoted_ascii_length(ptr, utf8_length()); char* result = NEW_RESOURCE_ARRAY(char, quoted_length + 1); UTF8::as_quoted_ascii(ptr, utf8_length(), result, quoted_length + 1); return result; diff --git a/src/hotspot/share/prims/jni.cpp b/src/hotspot/share/prims/jni.cpp index c9357fe0216..8a20d5f85b0 100644 --- a/src/hotspot/share/prims/jni.cpp +++ b/src/hotspot/share/prims/jni.cpp @@ -2223,7 +2223,7 @@ JNI_END JNI_ENTRY(jsize, jni_GetStringUTFLength(JNIEnv *env, jstring string)) HOTSPOT_JNI_GETSTRINGUTFLENGTH_ENTRY(env, string); oop java_string = JNIHandles::resolve_non_null(string); - jsize ret = java_lang_String::utf8_length(java_string); + jsize ret = java_lang_String::utf8_length_as_int(java_string); HOTSPOT_JNI_GETSTRINGUTFLENGTH_RETURN(ret); return ret; JNI_END @@ -2236,10 +2236,11 @@ JNI_ENTRY(const char*, jni_GetStringUTFChars(JNIEnv *env, jstring string, jboole typeArrayOop s_value = java_lang_String::value(java_string); if (s_value != nullptr) { size_t length = java_lang_String::utf8_length(java_string, s_value); - /* JNI Specification states return null on OOM */ + // JNI Specification states return null on OOM. + // The resulting sequence doesn't have to be NUL-terminated but we do. result = AllocateHeap(length + 1, mtInternal, AllocFailStrategy::RETURN_NULL); if (result != nullptr) { - java_lang_String::as_utf8_string(java_string, s_value, result, (int) length + 1); + java_lang_String::as_utf8_string(java_string, s_value, result, length + 1); if (isCopy != nullptr) { *isCopy = JNI_TRUE; } diff --git a/src/hotspot/share/prims/jvmtiEnv.cpp b/src/hotspot/share/prims/jvmtiEnv.cpp index ccccb5f1bda..63cc33222ec 100644 --- a/src/hotspot/share/prims/jvmtiEnv.cpp +++ b/src/hotspot/share/prims/jvmtiEnv.cpp @@ -1321,7 +1321,7 @@ JvmtiEnv::GetThreadInfo(jthread thread, jvmtiThreadInfo* info_ptr) { if (name() != nullptr) { n = java_lang_String::as_utf8_string(name()); } else { - int utf8_length = 0; + size_t utf8_length = 0; n = UNICODE::as_utf8((jchar*) nullptr, utf8_length); } diff --git a/src/hotspot/share/services/finalizerService.cpp b/src/hotspot/share/services/finalizerService.cpp index ecd9168cd65..fd46827ee00 100644 --- a/src/hotspot/share/services/finalizerService.cpp +++ b/src/hotspot/share/services/finalizerService.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2021, 2024, 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 @@ -45,7 +45,7 @@ static const char* allocate(oop string) { char* str = nullptr; const typeArrayOop value = java_lang_String::value(string); if (value != nullptr) { - const int length = java_lang_String::utf8_length(string, value); + const size_t length = java_lang_String::utf8_length(string, value); str = NEW_C_HEAP_ARRAY(char, length + 1, mtServiceability); java_lang_String::as_utf8_string(string, value, str, length + 1); } diff --git a/src/hotspot/share/utilities/utf8.cpp b/src/hotspot/share/utilities/utf8.cpp index 47cbb04da4b..cd28e715009 100644 --- a/src/hotspot/share/utilities/utf8.cpp +++ b/src/hotspot/share/utilities/utf8.cpp @@ -98,15 +98,21 @@ char* UTF8::next_character(const char* str, jint* value) { return next_ch; } -// Count bytes of the form 10xxxxxx and deduct this count +// The number of unicode characters in a utf8 sequence can be easily +// determined by noting that bytes of the form 10xxxxxx are part of +// a 2 or 3-byte multi-byte sequence, all others are either characters +// themselves or else the start of a multi-byte character. + +// Calculate the unicode length of a utf8 string of known size +// by counting bytes of the form 10xxxxxx and deducting this count // from the total byte count. The utf8 string must be in // legal form which has been verified in the format checker. -int UTF8::unicode_length(const char* str, int len, bool& is_latin1, bool& has_multibyte) { - int num_chars = len; +int UTF8::unicode_length(const char* str, size_t len, bool& is_latin1, bool& has_multibyte) { + size_t num_chars = len; has_multibyte = false; is_latin1 = true; unsigned char prev = 0; - for (int i = 0; i < len; i++) { + for (size_t i = 0; i < len; i++) { unsigned char c = str[i]; if ((c & 0xC0) == 0x80) { // Multibyte, check if valid latin1 character. @@ -118,12 +124,12 @@ int UTF8::unicode_length(const char* str, int len, bool& is_latin1, bool& has_mu } prev = c; } - return num_chars; + return checked_cast(num_chars); } -// Count bytes of the utf8 string except those in form -// 10xxxxxx which only appear in multibyte characters. -// The utf8 string must be in legal form and has been +// Calculate the unicode length of a nul-terminated utf8 string +// by counting bytes of the utf8 string except those in the form +// 10xxxxxx. The utf8 string must be in legal form and has been // verified in the format checker. int UTF8::unicode_length(const char* str, bool& is_latin1, bool& has_multibyte) { int num_chars = 0; @@ -195,10 +201,10 @@ template void UTF8::convert_to_unicode(const char* utf8_str, jchar* unico template void UTF8::convert_to_unicode(const char* utf8_str, jbyte* unicode_str, int unicode_length); // returns the quoted ascii length of a 0-terminated utf8 string -int UTF8::quoted_ascii_length(const char* utf8_str, int utf8_length) { +size_t UTF8::quoted_ascii_length(const char* utf8_str, size_t utf8_length) { const char *ptr = utf8_str; const char* end = ptr + utf8_length; - int result = 0; + size_t result = 0; while (ptr < end) { jchar c; ptr = UTF8::next(ptr, &c); @@ -212,7 +218,7 @@ int UTF8::quoted_ascii_length(const char* utf8_str, int utf8_length) { } // converts a utf8 string to quoted ascii -void UTF8::as_quoted_ascii(const char* utf8_str, int utf8_length, char* buf, int buflen) { +void UTF8::as_quoted_ascii(const char* utf8_str, size_t utf8_length, char* buf, size_t buflen) { const char *ptr = utf8_str; const char *utf8_end = ptr + utf8_length; char* p = buf; @@ -248,7 +254,7 @@ const char* UTF8::from_quoted_ascii(const char* quoted_ascii_str) { return quoted_ascii_str; } // everything up to this point was ok. - int length = ptr - quoted_ascii_str; + size_t length = ptr - quoted_ascii_str; char* buffer = nullptr; for (int round = 0; round < 2; round++) { while (*ptr != '\0') { @@ -330,11 +336,11 @@ jint UTF8::get_supplementary_character(const unsigned char* str) { + ((str[4] & 0x0f) << 6) + (str[5] & 0x3f); } -bool UTF8::is_legal_utf8(const unsigned char* buffer, int length, +bool UTF8::is_legal_utf8(const unsigned char* buffer, size_t length, bool version_leq_47) { - int i = 0; - int count = length >> 2; - for (int k=0; k> 2; + for (size_t k = 0; k < count; k++) { unsigned char b0 = buffer[i]; unsigned char b1 = buffer[i+1]; unsigned char b2 = buffer[i+2]; @@ -405,7 +411,7 @@ static bool is_starting_byte(unsigned char b) { // To avoid that the caller can choose to check for validity first. // The incoming buffer is still expected to be NUL-terminated. // The incoming buffer is expected to be a realistic size - we assert if it is too small. -void UTF8::truncate_to_legal_utf8(unsigned char* buffer, int length) { +void UTF8::truncate_to_legal_utf8(unsigned char* buffer, size_t length) { assert(length > 5, "invalid length"); assert(buffer[length - 1] == '\0', "Buffer should be NUL-terminated"); @@ -433,7 +439,7 @@ void UTF8::truncate_to_legal_utf8(unsigned char* buffer, int length) { // then we insert NUL at that location to terminate the buffer. There is an added complexity with 6 byte // encodings as the first and fourth bytes are the same and overlap with the 3 byte encoding. - for (int index = length - 2; index > 0; index--) { + for (size_t index = length - 2; index > 0; index--) { if (is_starting_byte(buffer[index])) { if (buffer[index] == 0xED) { // Could be first byte of 3 or 6, or fourth byte of 6. @@ -441,7 +447,7 @@ void UTF8::truncate_to_legal_utf8(unsigned char* buffer, int length) { // surrogate value in the range EDA080 to EDAFBF. We only // need to check for EDA to establish this as the "missing" // values in EDAxxx would not be valid 3 byte encodings. - if ((index - 3) >= 0 && + if (index >= 3 && (buffer[index - 3] == 0xED) && ((buffer[index - 2] & 0xF0) == 0xA0)) { assert(buffer[index - 1] >= 0x80 && buffer[index - 1] <= 0xBF, "sanity check"); @@ -470,7 +476,7 @@ bool UNICODE::is_latin1(const jchar* base, int length) { return true; } -int UNICODE::utf8_size(jchar c) { +size_t UNICODE::utf8_size(jchar c) { if ((0x0001 <= c) && (c <= 0x007F)) { // ASCII character return 1; @@ -481,7 +487,7 @@ int UNICODE::utf8_size(jchar c) { } } -int UNICODE::utf8_size(jbyte c) { +size_t UNICODE::utf8_size(jbyte c) { if (c >= 0x01) { // ASCII character. Check is equivalent to // (0x01 <= c) && (c <= 0x7F) because c is signed. @@ -494,11 +500,23 @@ int UNICODE::utf8_size(jbyte c) { } template -int UNICODE::utf8_length(const T* base, int length) { +size_t UNICODE::utf8_length(const T* base, int length) { + size_t result = 0; + for (int index = 0; index < length; index++) { + result += utf8_size(base[index]); + } + return result; +} + +template +int UNICODE::utf8_length_as_int(const T* base, int length) { size_t result = 0; for (int index = 0; index < length; index++) { T c = base[index]; - int sz = utf8_size(c); + size_t sz = utf8_size(c); + // If the length is > INT_MAX-1 we truncate at a completed + // modified-UTF8 encoding. This allows for +1 to be added + // by the caller for NUL-termination, without overflow. if (result + sz > INT_MAX-1) { break; } @@ -508,41 +526,44 @@ int UNICODE::utf8_length(const T* base, int length) { } template -char* UNICODE::as_utf8(const T* base, int& length) { - int utf8_len = utf8_length(base, length); +char* UNICODE::as_utf8(const T* base, size_t& length) { + // Incoming length must be <= INT_MAX + size_t utf8_len = utf8_length(base, static_cast(length)); u_char* buf = NEW_RESOURCE_ARRAY(u_char, utf8_len + 1); - char* result = as_utf8(base, length, (char*) buf, utf8_len + 1); - assert((int) strlen(result) == utf8_len, "length prediction must be correct"); - // Set string length to uft8 length + char* result = as_utf8(base, static_cast(length), (char*) buf, utf8_len + 1); + assert(strlen(result) == utf8_len, "length prediction must be correct"); + // Set outgoing string length to uft8 length length = utf8_len; return (char*) result; } -char* UNICODE::as_utf8(const jchar* base, int length, char* buf, int buflen) { +char* UNICODE::as_utf8(const jchar* base, int length, char* buf, size_t buflen) { assert(buflen > 0, "zero length output buffer"); u_char* p = (u_char*)buf; for (int index = 0; index < length; index++) { jchar c = base[index]; - buflen -= utf8_size(c); - if (buflen <= 0) break; // string is truncated + size_t sz = utf8_size(c); + if (sz >= buflen) break; // string is truncated + buflen -= sz; p = utf8_write(p, c); } *p = '\0'; return buf; } -char* UNICODE::as_utf8(const jbyte* base, int length, char* buf, int buflen) { +char* UNICODE::as_utf8(const jbyte* base, int length, char* buf, size_t buflen) { assert(buflen > 0, "zero length output buffer"); u_char* p = (u_char*)buf; for (int index = 0; index < length; index++) { jbyte c = base[index]; - int sz = utf8_size(c); + size_t sz = utf8_size(c); + if (sz >= buflen) break; // string is truncated buflen -= sz; - if (buflen <= 0) break; // string is truncated if (sz == 1) { // Copy ASCII characters (UTF-8 is ASCII compatible) *p++ = c; } else { + assert(sz == 2, "must be!"); // Non-ASCII character or 0x00 which should // be encoded as 0xC080 in "modified" UTF8. p = utf8_write(p, ((jchar) c) & 0xff); @@ -561,8 +582,8 @@ void UNICODE::convert_to_utf8(const jchar* base, int length, char* utf8_buffer) // returns the quoted ascii length of a unicode string template -int UNICODE::quoted_ascii_length(const T* base, int length) { - int result = 0; +size_t UNICODE::quoted_ascii_length(const T* base, int length) { + size_t result = 0; for (int i = 0; i < length; i++) { T c = base[i]; if (c >= 32 && c < 127) { @@ -576,7 +597,7 @@ int UNICODE::quoted_ascii_length(const T* base, int length) { // converts a unicode string to quoted ascii template -void UNICODE::as_quoted_ascii(const T* base, int length, char* buf, int buflen) { +void UNICODE::as_quoted_ascii(const T* base, int length, char* buf, size_t buflen) { char* p = buf; char* end = buf + buflen; for (int index = 0; index < length; index++) { @@ -594,11 +615,13 @@ void UNICODE::as_quoted_ascii(const T* base, int length, char* buf, int buflen) } // Explicit instantiation for all supported types. -template int UNICODE::utf8_length(const jbyte* base, int length); -template int UNICODE::utf8_length(const jchar* base, int length); -template char* UNICODE::as_utf8(const jbyte* base, int& length); -template char* UNICODE::as_utf8(const jchar* base, int& length); -template int UNICODE::quoted_ascii_length(const jbyte* base, int length); -template int UNICODE::quoted_ascii_length(const jchar* base, int length); -template void UNICODE::as_quoted_ascii(const jbyte* base, int length, char* buf, int buflen); -template void UNICODE::as_quoted_ascii(const jchar* base, int length, char* buf, int buflen); +template size_t UNICODE::utf8_length(const jbyte* base, int length); +template size_t UNICODE::utf8_length(const jchar* base, int length); +template int UNICODE::utf8_length_as_int(const jbyte* base, int length); +template int UNICODE::utf8_length_as_int(const jchar* base, int length); +template char* UNICODE::as_utf8(const jbyte* base, size_t& length); +template char* UNICODE::as_utf8(const jchar* base, size_t& length); +template size_t UNICODE::quoted_ascii_length(const jbyte* base, int length); +template size_t UNICODE::quoted_ascii_length(const jchar* base, int length); +template void UNICODE::as_quoted_ascii(const jbyte* base, int length, char* buf, size_t buflen); +template void UNICODE::as_quoted_ascii(const jchar* base, int length, char* buf, size_t buflen); diff --git a/src/hotspot/share/utilities/utf8.hpp b/src/hotspot/share/utilities/utf8.hpp index 9a18dd0ff93..3b4ff30e3c3 100644 --- a/src/hotspot/share/utilities/utf8.hpp +++ b/src/hotspot/share/utilities/utf8.hpp @@ -29,6 +29,45 @@ #include "memory/allStatic.hpp" #include "utilities/debug.hpp" +/** + +String handling within Java and the VM requires a bit of explanation. + +Logically a java.lang.String is a sequence of 16-bit Unicode characters +encoded in UTF-16. In the past a String contained a Java char[] and so +could theoretically contain INT_MAX 16-bit characters. Then came JEP 254: +Compact Strings. + +With Compact Strings the Java char[] becomes a Java byte[], and that byte[] +contains either latin-1 characters all of which fit in 8-bits, or else each +pair of bytes represents a UTF-16 character. Consequently the maximum length +in characters of a latin-1 string is INT_MAX, whilst for non-latin-1 it is INT_MAX/2. + +In the code below if we have latin-1 content then we treat the String's data +array as a jbyte[], else a jchar[]. The lengths of these arrays are specified +as an int value, with a nominal maximum of INT_MAX. + +The modified UTF-8 encoding specified for the VM, nominally encodes characters +in 1, 2, 3 or 6 bytes. The 6-byte representation is actually two 3-byte representations +for two UTF-16 characters forming a surrogate pair. If we are dealing with +a latin-1 string then each character will be encoded as either 1 or 2 bytes and so the +maximum UTF8 length is 2*INT_MAX. This can't be stored in an int so utf8 buffers must +use a size_t length. For non-latin-1 strings each UTF-16 character will encode as either +2 or 3 bytes, so the maximum UTF8 length in that case is 3 * INT_MAX/2 i.e. 1.5*INT_MAX. + +The "quoted ascii" form of a unicode string is at worst 6 times longer than its +regular form, and so these lengths must always be size_t - though if we know we only +ever do this to symbols (or small symbol combinations) then we could use int. + +There is an additional assumption/expectation that our UTF8 API's are never dealing with +invalid UTF8, and more generally that all UTF8 sequences could form valid Strings. +Consequently the Unicode length of a UTF8 sequence is assumed to always be representable +by an int. However, there are API's, such as JNI NewStringUTF, that do deal with such input +and could potentially have an unrepresentable string. The long standing position with JNI +is that the user must supply valid input so we do not try to account for these cases. + +*/ + // Low-level interface for UTF8 strings class UTF8 : AllStatic { @@ -41,20 +80,20 @@ class UTF8 : AllStatic { static int unicode_length(const char* utf8_str, bool& is_latin1, bool& has_multibyte); // returns the unicode length of a non-0-terminated utf8 string - static int unicode_length(const char* utf8_str, int len) { + static int unicode_length(const char* utf8_str, size_t len) { bool is_latin1, has_multibyte; return unicode_length(utf8_str, len, is_latin1, has_multibyte); } - static int unicode_length(const char* utf8_str, int len, bool& is_latin1, bool& has_multibyte); + static int unicode_length(const char* utf8_str, size_t len, bool& is_latin1, bool& has_multibyte); // converts a utf8 string to a unicode string template static void convert_to_unicode(const char* utf8_str, T* unicode_str, int unicode_length); // returns the quoted ascii length of a utf8 string - static int quoted_ascii_length(const char* utf8_str, int utf8_length); + static size_t quoted_ascii_length(const char* utf8_str, size_t utf8_length); // converts a utf8 string to quoted ascii - static void as_quoted_ascii(const char* utf8_str, int utf8_length, char* buf, int buflen); + static void as_quoted_ascii(const char* utf8_str, size_t utf8_length, char* buf, size_t buflen); #ifndef PRODUCT // converts a quoted ascii string to utf8 string. returns the original @@ -82,13 +121,13 @@ class UTF8 : AllStatic { while(--length >= 0 && base[length] != c); return (length < 0) ? nullptr : &base[length]; } - static bool equal(const jbyte* base1, int length1, const jbyte* base2,int length2); + static bool equal(const jbyte* base1, int length1, const jbyte* base2, int length2); static bool is_supplementary_character(const unsigned char* str); static jint get_supplementary_character(const unsigned char* str); - static bool is_legal_utf8(const unsigned char* buffer, int length, + static bool is_legal_utf8(const unsigned char* buffer, size_t length, bool version_leq_47); - static void truncate_to_legal_utf8(unsigned char* buffer, int length); + static void truncate_to_legal_utf8(unsigned char* buffer, size_t length); }; @@ -99,6 +138,12 @@ class UTF8 : AllStatic { // units, so a supplementary character uses two positions in a unicode string. class UNICODE : AllStatic { + + // returns the utf8 size of a unicode character + // uses size_t for convenience in overflow checks + static size_t utf8_size(jchar c); + static size_t utf8_size(jbyte c); + public: // checks if the given unicode character can be encoded as latin1 static bool is_latin1(jchar c); @@ -106,28 +151,27 @@ class UNICODE : AllStatic { // checks if the given string can be encoded as latin1 static bool is_latin1(const jchar* base, int length); - // returns the utf8 size of a unicode character - static int utf8_size(jchar c); - static int utf8_size(jbyte c); - // returns the utf8 length of a unicode string - template static int utf8_length(const T* base, int length); + template static size_t utf8_length(const T* base, int length); + + // returns the utf8 length of a unicode string as an int - truncated if needed + template static int utf8_length_as_int(const T* base, int length); // converts a unicode string to utf8 string static void convert_to_utf8(const jchar* base, int length, char* utf8_buffer); // converts a unicode string to a utf8 string; result is allocated // in resource area unless a buffer is provided. The unicode 'length' - // parameter is set to the length of the result utf8 string. - template static char* as_utf8(const T* base, int& length); - static char* as_utf8(const jchar* base, int length, char* buf, int buflen); - static char* as_utf8(const jbyte* base, int length, char* buf, int buflen); + // parameter is set to the length of the resulting utf8 string. + template static char* as_utf8(const T* base, size_t& length); + static char* as_utf8(const jchar* base, int length, char* buf, size_t buflen); + static char* as_utf8(const jbyte* base, int length, char* buf, size_t buflen); // returns the quoted ascii length of a unicode string - template static int quoted_ascii_length(const T* base, int length); + template static size_t quoted_ascii_length(const T* base, int length); // converts a unicode string to quoted ascii - template static void as_quoted_ascii(const T* base, int length, char* buf, int buflen); + template static void as_quoted_ascii(const T* base, int length, char* buf, size_t buflen); }; #endif // SHARE_UTILITIES_UTF8_HPP From f2968b34a55009fb195e381ffa615488974e9ba6 Mon Sep 17 00:00:00 2001 From: Matias Saavedra Silva Date: Thu, 29 Aug 2024 21:06:05 +0000 Subject: [PATCH 07/11] 8339020: Remove unused HeapShared::calculate_oopmap Reviewed-by: coleenp --- src/hotspot/share/cds/heapShared.cpp | 24 ------------------------ src/hotspot/share/cds/heapShared.hpp | 3 --- 2 files changed, 27 deletions(-) diff --git a/src/hotspot/share/cds/heapShared.cpp b/src/hotspot/share/cds/heapShared.cpp index 0a0dc56c894..abfc0f9d64b 100644 --- a/src/hotspot/share/cds/heapShared.cpp +++ b/src/hotspot/share/cds/heapShared.cpp @@ -1644,30 +1644,6 @@ class FindEmbeddedNonNullPointers: public BasicOopIterateClosure { }; #endif -#ifndef PRODUCT -ResourceBitMap HeapShared::calculate_oopmap(MemRegion region) { - size_t num_bits = region.byte_size() / (UseCompressedOops ? sizeof(narrowOop) : sizeof(oop)); - ResourceBitMap oopmap(num_bits); - - HeapWord* p = region.start(); - HeapWord* end = region.end(); - FindEmbeddedNonNullPointers finder((void*)p, &oopmap); - - int num_objs = 0; - while (p < end) { - oop o = cast_to_oop(p); - o->oop_iterate(&finder); - p += o->size(); - ++ num_objs; - } - - log_info(cds, heap)("calculate_oopmap: objects = %6d, oop fields = %7d (nulls = %7d)", - num_objs, finder.num_total_oops(), finder.num_null_oops()); - return oopmap; -} - -#endif // !PRODUCT - void HeapShared::count_allocation(size_t size) { _total_obj_count ++; _total_obj_size += size; diff --git a/src/hotspot/share/cds/heapShared.hpp b/src/hotspot/share/cds/heapShared.hpp index 2c87b050ca6..0ba20f1e313 100644 --- a/src/hotspot/share/cds/heapShared.hpp +++ b/src/hotspot/share/cds/heapShared.hpp @@ -371,9 +371,6 @@ class HeapShared: AllStatic { KlassSubGraphInfo* subgraph_info, oop orig_obj); -#ifndef PRODUCT - static ResourceBitMap calculate_oopmap(MemRegion region); // marks all the oop pointers -#endif static void add_to_dumped_interned_strings(oop string); // Scratch objects for archiving Klass::java_mirror() From b711c41d442fc369a84745c0203db638e0b7e671 Mon Sep 17 00:00:00 2001 From: Shaojin Wen Date: Thu, 29 Aug 2024 21:21:16 +0000 Subject: [PATCH 08/11] 8339196: Optimize BufWriterImpl#writeU1/U2/Int/Long Reviewed-by: liach, redestad --- .../classfile/impl/BufWriterImpl.java | 49 +++++++++++++++---- 1 file changed, 39 insertions(+), 10 deletions(-) diff --git a/src/java.base/share/classes/jdk/internal/classfile/impl/BufWriterImpl.java b/src/java.base/share/classes/jdk/internal/classfile/impl/BufWriterImpl.java index ac34d78e0e5..255e5e21cf0 100644 --- a/src/java.base/share/classes/jdk/internal/classfile/impl/BufWriterImpl.java +++ b/src/java.base/share/classes/jdk/internal/classfile/impl/BufWriterImpl.java @@ -91,17 +91,30 @@ public ClassFileImpl context() { @Override public void writeU1(int x) { - writeIntBytes(1, x); + reserveSpace(1); + elems[offset++] = (byte) x; } @Override public void writeU2(int x) { - writeIntBytes(2, x); + reserveSpace(2); + byte[] elems = this.elems; + int offset = this.offset; + elems[offset ] = (byte) (x >> 8); + elems[offset + 1] = (byte) x; + this.offset = offset + 2; } @Override public void writeInt(int x) { - writeIntBytes(4, x); + reserveSpace(4); + byte[] elems = this.elems; + int offset = this.offset; + elems[offset ] = (byte) (x >> 24); + elems[offset + 1] = (byte) (x >> 16); + elems[offset + 2] = (byte) (x >> 8); + elems[offset + 3] = (byte) x; + this.offset = offset + 4; } @Override @@ -111,7 +124,18 @@ public void writeFloat(float x) { @Override public void writeLong(long x) { - writeIntBytes(8, x); + reserveSpace(8); + byte[] elems = this.elems; + int offset = this.offset; + elems[offset ] = (byte) (x >> 56); + elems[offset + 1] = (byte) (x >> 48); + elems[offset + 2] = (byte) (x >> 40); + elems[offset + 3] = (byte) (x >> 32); + elems[offset + 4] = (byte) (x >> 24); + elems[offset + 5] = (byte) (x >> 16); + elems[offset + 6] = (byte) (x >> 8); + elems[offset + 7] = (byte) x; + this.offset = offset + 8; } @Override @@ -153,15 +177,20 @@ public void writeIntBytes(int intSize, long intValue) { @Override public void reserveSpace(int freeBytes) { - if (offset + freeBytes > elems.length) { - int newsize = elems.length * 2; - while (offset + freeBytes > newsize) { - newsize *= 2; - } - elems = Arrays.copyOf(elems, newsize); + int minCapacity = offset + freeBytes; + if (minCapacity > elems.length) { + grow(minCapacity); } } + private void grow(int minCapacity) { + int newsize = elems.length * 2; + while (minCapacity > newsize) { + newsize *= 2; + } + elems = Arrays.copyOf(elems, newsize); + } + @Override public int size() { return offset; From 4675913edb16ec1dde5f0ba2dfcfada134ce17f1 Mon Sep 17 00:00:00 2001 From: Gui Cao Date: Fri, 30 Aug 2024 01:06:00 +0000 Subject: [PATCH 09/11] 8339237: RISC-V: Builds fail after JDK-8339120 Reviewed-by: fyang --- src/hotspot/cpu/riscv/c1_Runtime1_riscv.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/hotspot/cpu/riscv/c1_Runtime1_riscv.cpp b/src/hotspot/cpu/riscv/c1_Runtime1_riscv.cpp index e999b703b3e..74f303ffb02 100644 --- a/src/hotspot/cpu/riscv/c1_Runtime1_riscv.cpp +++ b/src/hotspot/cpu/riscv/c1_Runtime1_riscv.cpp @@ -223,8 +223,6 @@ StubFrame::~StubFrame() { #define __ sasm-> -const int float_regs_as_doubles_size_in_slots = pd_nof_fpu_regs_frame_map * 2; - // Stack layout for saving/restoring all the registers needed during a runtime // call (this includes deoptimization) // Note: note that users of this frame may well have arguments to some runtime From f927c1210ee0675bb1196572177ffb505826d57a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eirik=20Bj=C3=B8rsn=C3=B8s?= Date: Fri, 30 Aug 2024 06:21:49 +0000 Subject: [PATCH 10/11] 8339154: Cleanups and JUnit conversion of test/jdk/java/util/zip/Available.java Reviewed-by: lancea --- test/jdk/java/util/zip/Available.java | 153 ++++++++++++++++++-------- 1 file changed, 110 insertions(+), 43 deletions(-) diff --git a/test/jdk/java/util/zip/Available.java b/test/jdk/java/util/zip/Available.java index f2272243101..373282c894b 100644 --- a/test/jdk/java/util/zip/Available.java +++ b/test/jdk/java/util/zip/Available.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 1998, 2011, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1998, 2024, 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 @@ -22,69 +22,136 @@ */ /* @test - @bug 4028605 4109069 4234207 4401122 - @summary Make sure ZipInputStream/InflaterInputStream.available() will - return 0 after EOF has reached and 1 otherwise. - */ + * @bug 4028605 4109069 4234207 4401122 8339154 + * @summary Verify that ZipInputStream, InflaterInputStream, ZipFileInputStream, + * ZipFileInflaterInputStream.available() return values according + * to their specification or long-standing behavior + * @run junit Available + */ +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import java.io.*; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; import java.util.zip.*; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + public class Available { - public static void main(String[] args) throws Exception { - // 4028605 4109069 4234207 - test1(); - // test 4401122 - test2(); - } + // ZIP file produced in this test + private final Path zip = Path.of("available.jar"); - private static void test1() throws Exception { - File f = new File(System.getProperty("test.src", "."), "input.jar"); + /** + * Create the ZIP file used in this test, containing + * one deflated and one stored entry. + * + * @throws IOException if an unexpected error occurs + */ + @BeforeEach + public void setup() throws IOException { + byte[] contents = "contents".repeat(10).getBytes(StandardCharsets.UTF_8); - // test ZipInputStream - try (FileInputStream fis = new FileInputStream(f); - ZipInputStream z = new ZipInputStream(fis)) - { - z.getNextEntry(); - tryAvail(z); - } + try (ZipOutputStream zo = new ZipOutputStream(Files.newOutputStream(zip))) { + // First entry uses DEFLATE method + zo.putNextEntry(new ZipEntry("deflated.txt")); + zo.write(contents); - // test InflaterInputStream - try (ZipFile zfile = new ZipFile(f)) { - tryAvail(zfile.getInputStream(zfile.getEntry("Available.java"))); + // Second entry uses STORED method + ZipEntry stored = new ZipEntry("stored.txt"); + stored.setMethod(ZipEntry.STORED); + stored.setSize(contents.length); + CRC32 crc32 = new CRC32(); + crc32.update(contents); + stored.setCrc(crc32.getValue()); + zo.putNextEntry(stored); + zo.write(contents); } } - static void tryAvail(InputStream in) throws Exception { - byte[] buf = new byte[1024]; - int n; + /** + * Delete the ZIP file created by this test + * + * @throws IOException if an unexpected error occurs + */ + @AfterEach + public void cleanup() throws IOException { + Files.deleteIfExists(zip); + } + + /** + * Verify that ZipInputStream.available() returns 0 after EOF or + * closeEntry, otherwise 1, as specified in the API description. + * This tests 4028605 4109069 4234207 + * @throws IOException if an unexpected error occurs + */ + @Test + public void testZipInputStream() throws IOException { + try (InputStream in = Files.newInputStream(zip)) { + ZipInputStream z = new ZipInputStream(in); + z.getNextEntry(); + assertEquals(1, z.available()); + z.read(); + assertEquals(1, z.available()); + z.transferTo(OutputStream.nullOutputStream()); + assertEquals(0, z.available(), + "ZipInputStream.available() should return 0 after EOF"); + + z.close(); + assertThrows(IOException.class, () -> z.available(), + "Expected an IOException when calling available on a closed stream"); + } - while ((n = in.read(buf)) != -1); - if (in.available() != 0) { - throw new Exception("available should return 0 after EOF"); + try (InputStream in = Files.newInputStream(zip); + ZipInputStream z = new ZipInputStream(in)) { + z.getNextEntry(); + z.closeEntry(); + assertEquals(0, z.available(), + "ZipInputStream.available() should return 0 after closeEntry"); } } - // To reproduce 4401122 - private static void test2() throws Exception { - File f = new File(System.getProperty("test.src", "."), "input.jar"); - try (ZipFile zf = new ZipFile(f)) { - InputStream in = zf.getInputStream(zf.getEntry("Available.java")); + /** + * Verify that ZipFileInputStream|ZipFileInflaterInputStream.available() + * return the number of remaining uncompressed bytes. + * + * This verifies unspecified, but long-standing behavior. See 4401122. + * + * @throws IOException if an unexpected error occurs + */ + @ParameterizedTest + @ValueSource(strings = { "stored.txt", "deflated.txt" }) + public void testZipFileStreamsRemainingBytes(String entryName) throws IOException { + try (ZipFile zfile = new ZipFile(zip.toFile())) { + ZipEntry entry = zfile.getEntry(entryName); + // Could be ZipFileInputStream or ZipFileInflaterInputStream + InputStream in = zfile.getInputStream(entry); int initialAvailable = in.available(); - in.read(); - if (in.available() != initialAvailable - 1) - throw new RuntimeException("Available not decremented."); - for(int j=0; j 0; i--) { + // Reading a single byte should decrement available by 1 in.read(); - if (in.available() != 0) - throw new RuntimeException(); + assertEquals(i - 1, in.available(), "Available not decremented"); + } + + // No remaining uncompressed bytes + assertEquals(0, in.available()); + + // available() should still return 0 after close in.close(); - if (in.available() != 0) - throw new RuntimeException(); + assertEquals(0, in.available()); } } - } From b9e65f982fe6ae69d3912f32465a688d67c1c765 Mon Sep 17 00:00:00 2001 From: Matthias Baesken Date: Fri, 30 Aug 2024 06:47:49 +0000 Subject: [PATCH 11/11] 8337662: Improve os::print_hex_dump for printing Instructions sections Reviewed-by: stuefe, lucy --- src/hotspot/share/runtime/os.cpp | 13 +++++++--- src/hotspot/share/runtime/os.hpp | 6 ++--- test/hotspot/gtest/runtime/test_os.cpp | 34 ++++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 6 deletions(-) diff --git a/src/hotspot/share/runtime/os.cpp b/src/hotspot/share/runtime/os.cpp index 6cb57e9dcb6..a8f3bc5c1d5 100644 --- a/src/hotspot/share/runtime/os.cpp +++ b/src/hotspot/share/runtime/os.cpp @@ -1016,11 +1016,14 @@ static void print_hex_location(outputStream* st, const_address p, int unitsize, } void os::print_hex_dump(outputStream* st, const_address start, const_address end, int unitsize, - bool print_ascii, int bytes_per_line, const_address logical_start) { + bool print_ascii, int bytes_per_line, const_address logical_start, const_address highlight_address) { constexpr int max_bytes_per_line = 64; assert(unitsize == 1 || unitsize == 2 || unitsize == 4 || unitsize == 8, "just checking"); assert(bytes_per_line > 0 && bytes_per_line <= max_bytes_per_line && is_power_of_2(bytes_per_line), "invalid bytes_per_line"); + assert(highlight_address == nullptr || (highlight_address >= start && highlight_address < end), + "address %p to highlight not in range %p - %p", highlight_address, start, end); + start = align_down(start, unitsize); logical_start = align_down(logical_start, unitsize); @@ -1037,7 +1040,11 @@ void os::print_hex_dump(outputStream* st, const_address start, const_address end // Print out the addresses as if we were starting from logical_start. while (p < end) { if (cols == 0) { - st->print(PTR_FORMAT ": ", p2i(logical_p)); + // highlight start of line if address of interest is located in the line + const bool should_highlight = (highlight_address >= p && highlight_address < p + bytes_per_line); + const char* const prefix = + (highlight_address != nullptr) ? (should_highlight ? "=>" : " ") : ""; + st->print("%s" PTR_FORMAT ": ", prefix, p2i(logical_p)); } print_hex_location(st, p, unitsize, ascii_form); p += unitsize; @@ -1082,7 +1089,7 @@ void os::print_tos(outputStream* st, address sp) { void os::print_instructions(outputStream* st, address pc, int unitsize) { st->print_cr("Instructions: (pc=" PTR_FORMAT ")", p2i(pc)); - print_hex_dump(st, pc - 256, pc + 256, unitsize, /* print_ascii=*/false); + print_hex_dump(st, pc - 256, pc + 256, unitsize, /* print_ascii=*/false, pc); } void os::print_environment_variables(outputStream* st, const char** env_list) { diff --git a/src/hotspot/share/runtime/os.hpp b/src/hotspot/share/runtime/os.hpp index 63ea1721667..1d81612c033 100644 --- a/src/hotspot/share/runtime/os.hpp +++ b/src/hotspot/share/runtime/os.hpp @@ -857,9 +857,9 @@ class os: AllStatic { static frame current_frame(); static void print_hex_dump(outputStream* st, const_address start, const_address end, int unitsize, bool print_ascii, - int bytes_per_line, const_address logical_start); - static void print_hex_dump(outputStream* st, const_address start, const_address end, int unitsize, bool print_ascii = true) { - print_hex_dump(st, start, end, unitsize, print_ascii, /*bytes_per_line=*/16, /*logical_start=*/start); + int bytes_per_line, const_address logical_start, const_address highlight_address = nullptr); + static void print_hex_dump(outputStream* st, const_address start, const_address end, int unitsize, bool print_ascii = true, const_address highlight_address = nullptr) { + print_hex_dump(st, start, end, unitsize, print_ascii, /*bytes_per_line=*/16, /*logical_start=*/start, highlight_address); } // returns a string to describe the exception/signal; diff --git a/test/hotspot/gtest/runtime/test_os.cpp b/test/hotspot/gtest/runtime/test_os.cpp index 4d3e6ba7f52..78e5212ab37 100644 --- a/test/hotspot/gtest/runtime/test_os.cpp +++ b/test/hotspot/gtest/runtime/test_os.cpp @@ -178,6 +178,16 @@ static void do_test_print_hex_dump(const_address from, const_address to, int uni EXPECT_STREQ(buf, expected); } +// version with a highlighted pc location +static void do_test_print_hex_dump_highlighted(const_address from, const_address to, int unitsize, int bytes_per_line, + const_address logical_start, const char* expected, const_address highlight) { + char buf[2048]; + buf[0] = '\0'; + stringStream ss(buf, sizeof(buf)); + os::print_hex_dump(&ss, from, to, unitsize, /* print_ascii=*/true, bytes_per_line, logical_start, highlight); + EXPECT_STREQ(buf, expected); +} + TEST_VM(os, test_print_hex_dump) { #ifdef _LP64 @@ -197,6 +207,24 @@ TEST_VM(os, test_print_hex_dump) { ADDRESS2 ": ff ff e0 dc 23 00 6a 64 6b 2f 69 6e 74 65 72 6e 61 6c 2f 6c 6f 61 64 65 72 2f 4e 61 74 69 76 65 " ASCII_1 "\n" \ ADDRESS3 ": 4c 69 62 72 61 72 69 65 73 00 00 00 00 00 00 00 " ASCII_2 "\n" +#define PAT_HL_1A "=>" ADDRESS1 ": ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ??\n" \ + " " ADDRESS2 ": ff ff e0 dc 23 00 6a 64 6b 2f 69 6e 74 65 72 6e 61 6c 2f 6c 6f 61 64 65 72 2f 4e 61 74 69 76 65 " ASCII_1 "\n" \ + " " ADDRESS3 ": 4c 69 62 72 61 72 69 65 73 00 00 00 00 00 00 00 " ASCII_2 "\n" + +#define PAT_HL_1B " " ADDRESS1 ": ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ??\n" \ + "=>" ADDRESS2 ": ff ff e0 dc 23 00 6a 64 6b 2f 69 6e 74 65 72 6e 61 6c 2f 6c 6f 61 64 65 72 2f 4e 61 74 69 76 65 " ASCII_1 "\n" \ + " " ADDRESS3 ": 4c 69 62 72 61 72 69 65 73 00 00 00 00 00 00 00 " ASCII_2 "\n" + +#ifdef VM_LITTLE_ENDIAN +#define PAT_HL_1C " " ADDRESS1 ": ???? ???? ???? ???? ???? ???? ???? ???? ???? ???? ???? ???? ???? ???? ???? ????\n" \ + "=>" ADDRESS2 ": ffff dce0 0023 646a 2f6b 6e69 6574 6e72 6c61 6c2f 616f 6564 2f72 614e 6974 6576 " ASCII_1 "\n" \ + " " ADDRESS3 ": 694c 7262 7261 6569 0073 0000 0000 0000 " ASCII_2 "\n" +#else +#define PAT_HL_1C " " ADDRESS1 ": ???? ???? ???? ???? ???? ???? ???? ???? ???? ???? ???? ???? ???? ???? ???? ????\n" \ + "=>" ADDRESS2 ": ffff e0dc 2300 6a64 6b2f 696e 7465 726e 616c 2f6c 6f61 6465 722f 4e61 7469 7665 " ASCII_1 "\n" \ + " " ADDRESS3 ": 4c69 6272 6172 6965 7300 0000 0000 0000 " ASCII_2 "\n" +#endif + #ifdef VM_LITTLE_ENDIAN #define PAT_2 ADDRESS1 ": ???? ???? ???? ???? ???? ???? ???? ???? ???? ???? ???? ???? ???? ???? ???? ????\n" \ ADDRESS2 ": ffff dce0 0023 646a 2f6b 6e69 6574 6e72 6c61 6c2f 616f 6564 2f72 614e 6974 6576 " ASCII_1 "\n" \ @@ -252,6 +280,12 @@ TEST_VM(os, test_print_hex_dump) { do_test_print_hex_dump(from + 1, to, 4, 32, logical_start, PAT_4); do_test_print_hex_dump(from + 1, to, 8, 32, logical_start, PAT_8); + // print with highlighted address + do_test_print_hex_dump_highlighted(from, to, 1, 32, logical_start, PAT_HL_1A, from+5); + do_test_print_hex_dump_highlighted(from, to, 1, 32, logical_start, PAT_HL_1B, from+32); + do_test_print_hex_dump_highlighted(from, to, 1, 32, logical_start, PAT_HL_1B, from+60); + do_test_print_hex_dump_highlighted(from, to, 2, 32, logical_start, PAT_HL_1C, from+60); + os::release_memory(two_pages, ps * 2); } #endif // not AIX