Skip to content

Commit

Permalink
8288303: C1: Miscompilation due to broken Class.getModifiers intrinsic
Browse files Browse the repository at this point in the history
Backport-of: 8cd87e731bcaff2d7838995c68056742d577ad3b
  • Loading branch information
shipilev committed Jun 23, 2022
1 parent a2be28d commit 72603ff
Show file tree
Hide file tree
Showing 2 changed files with 135 additions and 10 deletions.
27 changes: 17 additions & 10 deletions src/hotspot/share/c1/c1_LIRGenerator.cpp
Expand Up @@ -1311,20 +1311,27 @@ void LIRGenerator::do_getModifiers(Intrinsic* x) {
info = state_for(x);
}

LabelObj* L_not_prim = new LabelObj();
LabelObj* L_done = new LabelObj();
// While reading off the universal constant mirror is less efficient than doing
// another branch and returning the constant answer, this branchless code runs into
// much less risk of confusion for C1 register allocator. The choice of the universe
// object here is correct as long as it returns the same modifiers we would expect
// from the primitive class itself. See spec for Class.getModifiers that provides
// the typed array klasses with similar modifiers as their component types.

Klass* univ_klass_obj = Universe::byteArrayKlassObj();
assert(univ_klass_obj->modifier_flags() == (JVM_ACC_ABSTRACT | JVM_ACC_FINAL | JVM_ACC_PUBLIC), "Sanity");
LIR_Opr prim_klass = LIR_OprFact::metadataConst(univ_klass_obj);

LIR_Opr recv_klass = new_register(T_METADATA);
__ move(new LIR_Address(receiver.result(), java_lang_Class::klass_offset(), T_ADDRESS), recv_klass, info);

// Check if this is a Java mirror of primitive type, and select the appropriate klass.
LIR_Opr klass = new_register(T_METADATA);
// Checking if it's a java mirror of primitive type
__ move(new LIR_Address(receiver.result(), java_lang_Class::klass_offset(), T_ADDRESS), klass, info);
__ cmp(lir_cond_notEqual, klass, LIR_OprFact::metadataConst(0));
__ branch(lir_cond_notEqual, L_not_prim->label());
__ move(LIR_OprFact::intConst(JVM_ACC_ABSTRACT | JVM_ACC_FINAL | JVM_ACC_PUBLIC), result);
__ branch(lir_cond_always, L_done->label());
__ cmp(lir_cond_equal, recv_klass, LIR_OprFact::metadataConst(0));
__ cmove(lir_cond_equal, prim_klass, recv_klass, klass, T_ADDRESS);

__ branch_destination(L_not_prim->label());
// Get the answer.
__ move(new LIR_Address(klass, in_bytes(Klass::modifier_flags_offset()), T_INT), result);
__ branch_destination(L_done->label());
}

// Example: Thread.currentThread()
Expand Down
118 changes: 118 additions & 0 deletions test/hotspot/jtreg/compiler/intrinsics/klass/TestGetModifiers.java
@@ -0,0 +1,118 @@
/*
* Copyright (c) 2022, Red Hat, Inc. 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
* @library /test/lib
* @run main/othervm -Xint
* -XX:CompileCommand=dontinline,*TestGetModifiers.test
* compiler.intrinsics.klass.TestGetModifiers
*/

/*
* @test
* @requires vm.compiler1.enabled
* @library /test/lib
* @run main/othervm -XX:TieredStopAtLevel=1 -XX:+TieredCompilation
* -XX:CompileCommand=dontinline,*TestGetModifiers.test
* compiler.intrinsics.klass.TestGetModifiers
*/

/*
* @test
* @requires vm.compiler2.enabled
* @library /test/lib
* @run main/othervm -XX:-TieredCompilation
* -XX:CompileCommand=dontinline,*TestGetModifiers.test
* compiler.intrinsics.klass.TestGetModifiers
*/

package compiler.intrinsics.klass;

import java.lang.reflect.Modifier;
import static java.lang.reflect.Modifier.*;

import jdk.test.lib.Asserts;

public class TestGetModifiers {
public static class T1 {
}

public static final class T2 {
}

private static class T3 {
}

protected static class T4 {
}

class T5 {
}

interface T6 {
}

static void test(Class cl, int expectedMods) {
for (int i = 0; i < 100_000; i++) {
int actualMods = cl.getModifiers();
if (actualMods != expectedMods) {
throw new IllegalStateException("Error with: " + cl);
}
}
}

public static void main(String... args) {
test(T1.class, PUBLIC | STATIC);
test(T2.class, PUBLIC | FINAL | STATIC);
test(T3.class, PRIVATE | STATIC);
test(T4.class, PROTECTED | STATIC);
test(new TestGetModifiers().new T5().getClass(), 0);
test(T6.class, ABSTRACT | STATIC | INTERFACE);

test(int.class, PUBLIC | ABSTRACT | FINAL);
test(long.class, PUBLIC | ABSTRACT | FINAL);
test(double.class, PUBLIC | ABSTRACT | FINAL);
test(float.class, PUBLIC | ABSTRACT | FINAL);
test(char.class, PUBLIC | ABSTRACT | FINAL);
test(byte.class, PUBLIC | ABSTRACT | FINAL);
test(short.class, PUBLIC | ABSTRACT | FINAL);
test(void.class, PUBLIC | ABSTRACT | FINAL);
test(int[].class, PUBLIC | ABSTRACT | FINAL);
test(long[].class, PUBLIC | ABSTRACT | FINAL);
test(double[].class, PUBLIC | ABSTRACT | FINAL);
test(float[].class, PUBLIC | ABSTRACT | FINAL);
test(char[].class, PUBLIC | ABSTRACT | FINAL);
test(byte[].class, PUBLIC | ABSTRACT | FINAL);
test(short[].class, PUBLIC | ABSTRACT | FINAL);
test(Object[].class, PUBLIC | ABSTRACT | FINAL);
test(TestGetModifiers[].class, PUBLIC | ABSTRACT | FINAL);

test(new TestGetModifiers().getClass(), PUBLIC);
test(new T1().getClass(), PUBLIC | STATIC);
test(new T2().getClass(), PUBLIC | FINAL | STATIC);
test(new T3().getClass(), PRIVATE | STATIC);
test(new T4().getClass(), PROTECTED | STATIC);
}
}

1 comment on commit 72603ff

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.