Skip to content

Commit d3408a4

Browse files
author
Vladimir Kozlov
committed
8277964: ClassCastException with no stack trace is thrown with -Xcomp in method handle invocation
Reviewed-by: dlong, mchung, dholmes
1 parent 69b5d49 commit d3408a4

File tree

7 files changed

+148
-2
lines changed

7 files changed

+148
-2
lines changed

src/hotspot/share/ci/ciMethod.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,9 +117,11 @@ ciMethod::ciMethod(const methodHandle& h_m, ciInstanceKlass* holder) :
117117

118118
if (h_m->method_holder()->is_linked()) {
119119
_can_be_statically_bound = h_m->can_be_statically_bound();
120+
_can_omit_stack_trace = h_m->can_omit_stack_trace();
120121
} else {
121122
// Have to use a conservative value in this case.
122123
_can_be_statically_bound = false;
124+
_can_omit_stack_trace = true;
123125
}
124126

125127
// Adjust the definition of this condition to be more useful:
@@ -176,6 +178,7 @@ ciMethod::ciMethod(ciInstanceKlass* holder,
176178
_intrinsic_id( vmIntrinsics::_none),
177179
_instructions_size(-1),
178180
_can_be_statically_bound(false),
181+
_can_omit_stack_trace(true),
179182
_liveness( NULL)
180183
#if defined(COMPILER2)
181184
,
@@ -766,6 +769,20 @@ bool ciMethod::can_be_statically_bound(ciInstanceKlass* context) const {
766769
return (holder() == context) && can_be_statically_bound();
767770
}
768771

772+
// ------------------------------------------------------------------
773+
// ciMethod::can_omit_stack_trace
774+
//
775+
// Tries to determine whether a method can omit stack trace in throw in compiled code.
776+
bool ciMethod::can_omit_stack_trace() const {
777+
if (!StackTraceInThrowable) {
778+
return true; // stack trace is switched off.
779+
}
780+
if (!OmitStackTraceInFastThrow) {
781+
return false; // Have to provide stack trace.
782+
}
783+
return _can_omit_stack_trace;
784+
}
785+
769786
// ------------------------------------------------------------------
770787
// ciMethod::resolve_invoke
771788
//

src/hotspot/share/ci/ciMethod.hpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ class ciMethod : public ciMetadata {
9292
bool _is_c2_compilable;
9393
bool _can_be_parsed;
9494
bool _can_be_statically_bound;
95+
bool _can_omit_stack_trace;
9596
bool _has_reserved_stack_access;
9697
bool _is_overpass;
9798

@@ -364,6 +365,8 @@ class ciMethod : public ciMetadata {
364365

365366
bool can_be_statically_bound(ciInstanceKlass* context) const;
366367

368+
bool can_omit_stack_trace() const;
369+
367370
// Replay data methods
368371
static void dump_name_as_ascii(outputStream* st, Method* method);
369372
void dump_name_as_ascii(outputStream* st);

src/hotspot/share/classfile/vmSymbols.hpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@
142142
template(java_util_Iterator, "java/util/Iterator") \
143143
template(java_lang_Record, "java/lang/Record") \
144144
template(sun_instrument_InstrumentationImpl, "sun/instrument/InstrumentationImpl") \
145+
template(sun_invoke_util_ValueConversions, "sun/invoke/util/ValueConversions") \
145146
\
146147
template(jdk_internal_loader_NativeLibraries, "jdk/internal/loader/NativeLibraries") \
147148
template(jdk_internal_loader_BuiltinClassLoader, "jdk/internal/loader/BuiltinClassLoader") \

src/hotspot/share/oops/method.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -818,6 +818,18 @@ bool Method::can_be_statically_bound(InstanceKlass* context) const {
818818
return (method_holder() == context) && can_be_statically_bound();
819819
}
820820

821+
/**
822+
* Returns false if this is one of specially treated methods for
823+
* which we have to provide stack trace in throw in compiled code.
824+
* Returns true otherwise.
825+
*/
826+
bool Method::can_omit_stack_trace() {
827+
if (klass_name() == vmSymbols::sun_invoke_util_ValueConversions()) {
828+
return false; // All methods in sun.invoke.util.ValueConversions
829+
}
830+
return true;
831+
}
832+
821833
bool Method::is_accessor() const {
822834
return is_getter() || is_setter();
823835
}

src/hotspot/share/oops/method.hpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -600,6 +600,9 @@ class Method : public Metadata {
600600
bool can_be_statically_bound(InstanceKlass* context) const;
601601
bool can_be_statically_bound(AccessFlags class_access_flags) const;
602602

603+
// true if method can omit stack trace in throw in compiled code.
604+
bool can_omit_stack_trace();
605+
603606
// returns true if the method has any backward branches.
604607
bool has_loops() {
605608
return access_flags().loops_flag_init() ? access_flags().has_loops() : compute_has_loops_flag();

src/hotspot/share/opto/graphKit.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -560,8 +560,7 @@ void GraphKit::builtin_throw(Deoptimization::DeoptReason reason, Node* arg) {
560560
// let us handle the throw inline, with a preconstructed instance.
561561
// Note: If the deopt count has blown up, the uncommon trap
562562
// runtime is going to flush this nmethod, not matter what.
563-
if (treat_throw_as_hot
564-
&& (!StackTraceInThrowable || OmitStackTraceInFastThrow)) {
563+
if (treat_throw_as_hot && method()->can_omit_stack_trace()) {
565564
// If the throw is local, we use a pre-existing instance and
566565
// punt on the backtrace. This would lead to a missing backtrace
567566
// (a repeat of 4292742) if the backtrace object is ever asked
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
/*
2+
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation.
8+
*
9+
* This code is distributed in the hope that it will be useful, but WITHOUT
10+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
11+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
12+
* version 2 for more details (a copy is included in the LICENSE file that
13+
* accompanied this code).
14+
*
15+
* You should have received a copy of the GNU General Public License version
16+
* 2 along with this work; if not, write to the Free Software Foundation,
17+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*
19+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
20+
* or visit www.oracle.com if you need additional information or have any
21+
* questions.
22+
*/
23+
24+
/*
25+
* @test
26+
* @bug 8277964
27+
* @summary Test IllegalArgumentException be thrown when an argument is invalid
28+
* @run testng/othervm IllegalArgumentsTest
29+
*/
30+
31+
import java.lang.reflect.Constructor;
32+
import java.lang.reflect.Method;
33+
import org.testng.annotations.Test;
34+
35+
public class IllegalArgumentsTest {
36+
static class T {
37+
public T(int i) {}
38+
39+
public static void m(int i) {}
40+
41+
public void m1(String s) {}
42+
}
43+
44+
@Test
45+
public void wrongArgumentType() throws ReflectiveOperationException {
46+
for (int i = 0; i < 100_000; ++i) {
47+
try {
48+
Constructor<T> ctor = T.class.getConstructor(int.class);
49+
ctor.newInstance(int.class); // wrong argument type
50+
throw new RuntimeException("Expected IAE not thrown");
51+
} catch (IllegalArgumentException e) {}
52+
}
53+
54+
for (int i = 0; i < 100_000; ++i) {
55+
try {
56+
Method method = T.class.getMethod("m", int.class);
57+
method.invoke(null, int.class); // wrong argument type
58+
throw new RuntimeException("Expected IAE not thrown");
59+
} catch (IllegalArgumentException e) {}
60+
}
61+
}
62+
63+
@Test
64+
public void nullArguments() throws ReflectiveOperationException {
65+
for (int i = 0; i < 100_000; ++i) {
66+
try {
67+
Constructor<T> ctor = T.class.getConstructor(int.class);
68+
ctor.newInstance(new Object[] {null});
69+
throw new RuntimeException("Expected IAE not thrown");
70+
} catch (IllegalArgumentException e) {}
71+
}
72+
73+
for (int i = 0; i < 100_000; ++i) {
74+
try {
75+
Method method = T.class.getMethod("m", int.class);
76+
method.invoke(null, new Object[] {null});
77+
throw new RuntimeException("Expected IAE not thrown");
78+
} catch (IllegalArgumentException e) {}
79+
}
80+
}
81+
82+
@Test
83+
public void illegalArguments() throws ReflectiveOperationException {
84+
for (int i = 0; i < 100_000; ++i) {
85+
try {
86+
Constructor<T> ctor = T.class.getConstructor(int.class);
87+
ctor.newInstance(new Object[] { 10, 20});
88+
throw new RuntimeException("Expected IAE not thrown");
89+
} catch (IllegalArgumentException e) {}
90+
}
91+
92+
for (int i = 0; i < 100_000; ++i) {
93+
try {
94+
Method method = T.class.getMethod("m", int.class);
95+
method.invoke(null, new Object[] { 10, 20});
96+
throw new RuntimeException("Expected IAE not thrown");
97+
} catch (IllegalArgumentException e) {}
98+
}
99+
}
100+
101+
@Test
102+
public void wrongReceiver() throws ReflectiveOperationException {
103+
for (int i = 0; i < 100_000; ++i) {
104+
try {
105+
Method method = T.class.getMethod("m1", String.class);
106+
method.invoke(this, "bad receiver");
107+
throw new RuntimeException("Expected IAE not thrown");
108+
} catch (IllegalArgumentException e) {}
109+
}
110+
}
111+
}

0 commit comments

Comments
 (0)