From df5e6e5d482e70b33612639b3c1c04eaa1ed361e Mon Sep 17 00:00:00 2001 From: Jonathan Gibbons Date: Wed, 30 Aug 2023 21:52:31 +0000 Subject: [PATCH] 8315248: AssertionError in Name.compareTo Reviewed-by: vromero --- .../sun/tools/javac/util/Utf8NameTable.java | 38 +++--- .../tools/javac/nametable/TestNameTables.java | 127 ++++++++++++++++++ 2 files changed, 149 insertions(+), 16 deletions(-) create mode 100644 test/langtools/tools/javac/nametable/TestNameTables.java diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/util/Utf8NameTable.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/util/Utf8NameTable.java index c9fdd7df939..46d8a295efa 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/util/Utf8NameTable.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/util/Utf8NameTable.java @@ -25,10 +25,6 @@ package com.sun.tools.javac.util; -import java.lang.ref.SoftReference; - -import com.sun.tools.javac.util.DefinedBy.Api; - /** * Support superclass for {@link Name.Table} implementations that store * names as Modified UTF-8 data in {@code byte[]} arrays. @@ -51,7 +47,7 @@ public abstract class Utf8NameTable extends Name.Table { /** Generate a hash value for a subarray. */ - protected static int hashValue(byte buf[], int off, int len) { + protected static int hashValue(byte[] buf, int off, int len) { int hash = 0; while (len-- > 0) hash = (hash << 5) - hash + buf[off++]; @@ -100,6 +96,11 @@ public abstract class Utf8NameTable extends Name.Table { */ protected abstract int getNameIndex(); + @Override + protected boolean nameEquals(Name that) { + return ((NameImpl)that).getNameIndex() == getNameIndex(); + } + // CharSequence @Override @@ -112,17 +113,12 @@ public abstract class Utf8NameTable extends Name.Table { try { return Convert.utf2string(getByteData(), getByteOffset(), getByteLength(), Convert.Validation.NONE); } catch (InvalidUtfException e) { - throw new AssertionError(); + throw new AssertionError("invalid UTF8 data", e); } } // javax.lang.model.element.Name - @Override - protected boolean nameEquals(Name that) { - return ((NameImpl)that).getNameIndex() == getNameIndex(); - } - @Override public int hashCode() { return getNameIndex(); @@ -132,8 +128,18 @@ public abstract class Utf8NameTable extends Name.Table { @Override public int compareTo(Name name0) { - NameImpl name = (NameImpl)name0; - Assert.check(name.table == table); + // While most operations on Name that take a Name as an argument expect the argument + // to come from the same table, in many cases, including here, that is not strictly + // required. Moreover, javac.util.Name implements javax.lang.model.element.Name, + // which extends CharSequence, which provides + // static int compare(CharSequence cs1, CharSequence cs2) + // which ends up calling to this method via the Comparable interface + // and a bridge method when the two arguments have the same class. + // Therefore, for this method, we relax "same table", and delegate to the more + // general super method if necessary. + if (!(name0 instanceof NameImpl name)) { + return super.compareTo(name0); + } byte[] buf1 = getByteData(); byte[] buf2 = name.getByteData(); int off1 = getByteOffset(); @@ -180,7 +186,7 @@ public abstract class Utf8NameTable extends Name.Table { try { return table.fromUtf(result, 0, result.length, Convert.Validation.NONE); } catch (InvalidUtfException e) { - throw new AssertionError(); + throw new AssertionError("invalid UTF8 data", e); } } @@ -202,7 +208,7 @@ public abstract class Utf8NameTable extends Name.Table { try { return table.fromUtf(result, 0, result.length, Convert.Validation.NONE); } catch (InvalidUtfException e) { - throw new AssertionError(); + throw new AssertionError("invalid UTF8 data", e); } } @@ -252,7 +258,7 @@ public abstract class Utf8NameTable extends Name.Table { } @Override - public void getUtf8Bytes(byte buf[], int off) { + public void getUtf8Bytes(byte[] buf, int off) { System.arraycopy(getByteData(), getByteOffset(), buf, off, getByteLength()); } } diff --git a/test/langtools/tools/javac/nametable/TestNameTables.java b/test/langtools/tools/javac/nametable/TestNameTables.java new file mode 100644 index 00000000000..18a5c2f485b --- /dev/null +++ b/test/langtools/tools/javac/nametable/TestNameTables.java @@ -0,0 +1,127 @@ +/* + * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/* + * @test + * @bug 8315248 + * @summary AssertionError in Name.compareTo + * @modules jdk.compiler/com.sun.tools.javac.util + * + * @run main TestNameTables + */ + +import java.io.PrintStream; +import java.util.List; +import java.util.Objects; + +import com.sun.tools.javac.util.Context; +import com.sun.tools.javac.util.Name; +import com.sun.tools.javac.util.Names; +import com.sun.tools.javac.util.Options; + +/** + * Tests that CharSequence.compareTo works on CharSequence objects that may + * come from different name tables. + * + * While the java.util.Name class generally specifies that operations involving + * multiple Name objects should use names from the same table, that restriction + * is harder to specify when the names are widened CharSequence objects. + * + * The test can be extended if necessary to cover other methods on Name, + * if the restrictions on Name are relaxed to permit more mix-and-match usages. + */ +public class TestNameTables { + public static void main(String... args) { + new TestNameTables().run(); + } + + public static final String USE_SHARED_TABLE = "useSharedTable"; + public static final String USE_UNSHARED_TABLE = "useUnsharedTable"; + public static final String USE_STRING_TABLE = "useStringTable"; + + public final List ALL_TABLES = List.of(USE_SHARED_TABLE, USE_UNSHARED_TABLE, USE_STRING_TABLE); + + private final PrintStream out = System.err; + + void run() { + for (var s : ALL_TABLES) { + test(createNameTable(s)); + } + + for (var s1 : ALL_TABLES) { + for (var s2 : ALL_TABLES) { + test(createNameTable(s1), createNameTable(s2)); + } + } + } + + Name.Table createNameTable(String option) { + Context c = new Context(); + Options o = Options.instance(c); + o.put(option, "true"); + Names n = new Names(c); + return n.table; + } + + /** + * Tests operations using a single name table. + * + * @param table the name table + */ + void test(Name.Table table) { + test(table, table); + } + + /** + * Tests operations using distinct name tables, of either the same + * or different impl types. + * + * @param table1 the first name table + * @param table2 the second name table + */ + void test(Name.Table table1, Name.Table table2) { + if (table1 == table2) { + out.println("Testing " + table1); + } else { + out.println("Testing " + table1 + " : " + table2); + } + + // tests are primarily that there are no issues manipulating names from + // distinct name tables + testCharSequenceCompare(table1, table2); + } + + void testCharSequenceCompare(Name.Table table1, Name.Table table2) { + Name n1 = table1.fromString("abc"); + Name n2 = table2.fromString("abc"); + checkEquals(CharSequence.compare(n1, n2), 0, "CharSequence.compare"); + } + + void checkEquals(Object found, Object expect, String op) { + if (!Objects.equals(found, expect)) { + out.println("Failed: " + op); + out.println(" found: " + found); + out.println(" expect: " + expect); + } + } +} \ No newline at end of file