Skip to content

Commit ccd93e9

Browse files
Jean-Philippe BempelPaul Hohensee
authored andcommitted
8240908: RetransformClass does not know about MethodParameters attribute
Backport-of: 86c24b319ed5e2f0097cfb4b1afe2eb358eb5f75
1 parent dd0f81f commit ccd93e9

5 files changed

Lines changed: 466 additions & 18 deletions

File tree

src/hotspot/share/prims/jvmtiClassFileReconstituter.cpp

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2005, 2021, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2005, 2022, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -287,6 +287,31 @@ void JvmtiClassFileReconstituter::write_exceptions_attribute(ConstMethod* const_
287287
}
288288
}
289289

290+
// Write MethodParameters attribute
291+
// JVMSpec| MethodParameters_attribute {
292+
// JVMSpec| u2 attribute_name_index;
293+
// JVMSpec| u4 attribute_length;
294+
// JVMSpec| u1 parameters_count;
295+
// JVMSpec| { u2 name_index;
296+
// JVMSpec| u2 access_flags;
297+
// JVMSpec| } parameters[parameters_count];
298+
// JVMSpec| }
299+
void JvmtiClassFileReconstituter::write_method_parameter_attribute(const ConstMethod* const_method) {
300+
const MethodParametersElement *parameters = const_method->method_parameters_start();
301+
int length = const_method->method_parameters_length();
302+
assert(length <= max_jubyte, "must fit u1");
303+
int size = 1 // parameters_count
304+
+ (2 + 2) * length; // parameters
305+
306+
write_attribute_name_index("MethodParameters");
307+
write_u4(size);
308+
write_u1(length);
309+
for (int index = 0; index < length; index++) {
310+
write_u2(parameters[index].name_cp_index);
311+
write_u2(parameters[index].flags);
312+
}
313+
}
314+
290315
// Write SourceFile attribute
291316
// JVMSpec| SourceFile_attribute {
292317
// JVMSpec| u2 attribute_name_index;
@@ -689,6 +714,9 @@ void JvmtiClassFileReconstituter::write_method_info(const methodHandle& method)
689714
if (default_anno != NULL) {
690715
++attr_count; // has AnnotationDefault attribute
691716
}
717+
if (const_method->has_method_parameters()) {
718+
++attr_count; // has MethodParameters attribute
719+
}
692720
// Deprecated attribute would go here
693721
if (access_flags.is_synthetic()) { // FIXME
694722
// ++attr_count;
@@ -716,6 +744,9 @@ void JvmtiClassFileReconstituter::write_method_info(const methodHandle& method)
716744
if (default_anno != NULL) {
717745
write_annotations_attribute("AnnotationDefault", default_anno);
718746
}
747+
if (const_method->has_method_parameters()) {
748+
write_method_parameter_attribute(const_method);
749+
}
719750
// Deprecated attribute would go here
720751
if (access_flags.is_synthetic()) {
721752
// write_synthetic_attribute();

src/hotspot/share/prims/jvmtiClassFileReconstituter.hpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2005, 2020, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2005, 2022, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -101,6 +101,7 @@ class JvmtiClassFileReconstituter : public JvmtiConstantPoolReconstituter {
101101
void write_method_info(const methodHandle& method);
102102
void write_code_attribute(const methodHandle& method);
103103
void write_exceptions_attribute(ConstMethod* const_method);
104+
void write_method_parameter_attribute(const ConstMethod* const_method);
104105
void write_synthetic_attribute();
105106
void write_class_attributes();
106107
void write_source_file_attribute();

src/hotspot/share/prims/jvmtiRedefineClasses.cpp

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2003, 2021, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2003, 2022, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -2270,21 +2270,6 @@ void VM_RedefineClasses::rewrite_cp_refs_in_method(methodHandle method,
22702270
break;
22712271
}
22722272
} // end for each bytecode
2273-
2274-
// We also need to rewrite the parameter name indexes, if there is
2275-
// method parameter data present
2276-
if(method->has_method_parameters()) {
2277-
const int len = method->method_parameters_length();
2278-
MethodParametersElement* elem = method->method_parameters_start();
2279-
2280-
for (int i = 0; i < len; i++) {
2281-
const u2 cp_index = elem[i].name_cp_index;
2282-
const u2 new_cp_index = find_new_index(cp_index);
2283-
if (new_cp_index != 0) {
2284-
elem[i].name_cp_index = new_cp_index;
2285-
}
2286-
}
2287-
}
22882273
} // end rewrite_cp_refs_in_method()
22892274

22902275

@@ -3697,6 +3682,19 @@ void VM_RedefineClasses::set_new_constant_pool(
36973682
} // end for each local variable table entry
36983683
} // end if there are local variable table entries
36993684

3685+
// Update constant pool indices in the method's method_parameters.
3686+
int mp_length = method->method_parameters_length();
3687+
if (mp_length > 0) {
3688+
MethodParametersElement* elem = method->method_parameters_start();
3689+
for (int j = 0; j < mp_length; j++) {
3690+
const int cp_index = elem[j].name_cp_index;
3691+
const int new_cp_index = find_new_index(cp_index);
3692+
if (new_cp_index != 0) {
3693+
elem[j].name_cp_index = (u2)new_cp_index;
3694+
}
3695+
}
3696+
}
3697+
37003698
rewrite_cp_refs_in_stack_map_table(method);
37013699
} // end for each method
37023700
} // end set_new_constant_pool()
Lines changed: 255 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,255 @@
1+
/*
2+
* Copyright (c) 2022, 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 8240908
27+
*
28+
* @library /test/lib
29+
* @run compile -g -parameters RetransformWithMethodParametersTest.java
30+
* @run shell MakeJAR.sh retransformAgent
31+
*
32+
* @run main/othervm -javaagent:retransformAgent.jar RetransformWithMethodParametersTest
33+
*/
34+
35+
import java.io.File;
36+
import java.io.FileOutputStream;
37+
import java.lang.instrument.ClassFileTransformer;
38+
import java.lang.reflect.Method;
39+
import java.lang.reflect.Parameter;
40+
import java.nio.file.Files;
41+
import java.nio.file.Paths;
42+
import java.security.ProtectionDomain;
43+
import java.util.Arrays;
44+
45+
import jdk.test.lib.JDKToolLauncher;
46+
import jdk.test.lib.process.ProcessTools;
47+
import jdk.test.lib.process.OutputAnalyzer;
48+
import jdk.test.lib.util.ClassTransformer;
49+
50+
/*
51+
* The test verifies Instrumentation.retransformClasses() (and JVMTI function RetransformClasses)
52+
* correctly handles MethodParameter attribute:
53+
* 1) classfile bytes passed to transformers (and JVMTI ClassFileLoadHook event callback) contain the attribute;
54+
* 2) the attribute is updated if new version has the attribute with different values;
55+
* 3) the attribute is removed if new version doesn't contain the attribute.
56+
*/
57+
58+
// See ClassTransformer.transform(int) comment for @1 tag explanations.
59+
class MethodParametersTarget {
60+
// The class contains the only method, so we don't have issue with method sorting
61+
// and ClassFileReconstituter should restore the same bytes as original classbytes.
62+
public void method1(
63+
int intParam1, String stringParam1 // @1 commentout
64+
// @1 uncomment int intParam2, String stringParam2
65+
)
66+
{
67+
// @1 uncomment System.out.println(stringParam2); // change CP
68+
}
69+
}
70+
71+
public class RetransformWithMethodParametersTest extends ATransformerManagementTestCase {
72+
73+
public static void main (String[] args) throws Throwable {
74+
ATestCaseScaffold test = new RetransformWithMethodParametersTest();
75+
test.runTest();
76+
}
77+
78+
private String targetClassName = "MethodParametersTarget";
79+
private String classFileName = targetClassName + ".class";
80+
private String sourceFileName = "RetransformWithMethodParametersTest.java";
81+
private Class targetClass;
82+
private byte[] originalClassBytes;
83+
84+
private byte[] seenClassBytes;
85+
private byte[] newClassBytes;
86+
87+
public RetransformWithMethodParametersTest() throws Throwable {
88+
super("RetransformWithMethodParametersTest");
89+
90+
File origClassFile = new File(System.getProperty("test.classes", "."), classFileName);
91+
log("Reading test class from " + origClassFile);
92+
originalClassBytes = Files.readAllBytes(origClassFile.toPath());
93+
log("Read " + originalClassBytes.length + " bytes.");
94+
}
95+
96+
private void log(Object o) {
97+
System.out.println(String.valueOf(o));
98+
}
99+
100+
private Parameter[] getTargetMethodParameters() throws ClassNotFoundException {
101+
Class cls = Class.forName(targetClassName);
102+
// the class contains 1 method (method1)
103+
Method method = cls.getDeclaredMethods()[0];
104+
Parameter[] params = method.getParameters();
105+
log("Params of " + method.getName() + " method (" + params.length + "):");
106+
for (int i = 0; i < params.length; i++) {
107+
log(" " + i + ": " + params[i].getName()
108+
+ " (" + (params[i].isNamePresent() ? "present" : "absent") + ")");
109+
}
110+
return params;
111+
}
112+
113+
// Verifies MethodParameters attribute is present and contains the expected values.
114+
private void verifyPresentMethodParams(String... expectedNames) throws Throwable {
115+
Parameter[] params = getTargetMethodParameters();
116+
assertEquals(expectedNames.length, params.length);
117+
for (int i = 0; i < params.length; i++) {
118+
assertTrue(params[i].isNamePresent());
119+
assertEquals(expectedNames[i], params[i].getName());
120+
}
121+
}
122+
123+
// Verifies MethodParameters attribute is absent.
124+
private void verifyAbsentMethodParams() throws Throwable {
125+
Parameter[] params = getTargetMethodParameters();
126+
for (int i = 0; i < params.length; i++) {
127+
assertTrue(!params[i].isNamePresent());
128+
}
129+
}
130+
131+
// Retransforms target class using provided class bytes;
132+
// Returns class bytes passed to the transformer.
133+
private byte[] retransform(byte[] classBytes) throws Throwable {
134+
seenClassBytes = null;
135+
newClassBytes = classBytes;
136+
fInst.retransformClasses(targetClass);
137+
assertTrue(targetClassName + " was not seen by transform()", seenClassBytes != null);
138+
return seenClassBytes;
139+
}
140+
141+
// Prints dissassembled class bytes.
142+
private void printDisassembled(String description, byte[] bytes) throws Exception {
143+
log(description + " -------------------");
144+
145+
File f = new File(classFileName);
146+
try (FileOutputStream fos = new FileOutputStream(f)) {
147+
fos.write(bytes);
148+
}
149+
JDKToolLauncher javap = JDKToolLauncher.create("javap")
150+
.addToolArg("-verbose")
151+
.addToolArg("-p") // Shows all classes and members.
152+
//.addToolArg("-c") // Prints out disassembled code
153+
//.addToolArg("-s") // Prints internal type signatures.
154+
.addToolArg(f.toString());
155+
ProcessBuilder pb = new ProcessBuilder(javap.getCommand());
156+
OutputAnalyzer out = ProcessTools.executeProcess(pb);
157+
out.shouldHaveExitValue(0);
158+
try {
159+
Files.delete(f.toPath());
160+
} catch (Exception ex) {
161+
// ignore
162+
}
163+
out.asLines().forEach(s -> log(s));
164+
log("==========================================");
165+
}
166+
167+
// Verifies class bytes are equal.
168+
private void compareClassBytes(byte[] expected, byte[] actual) throws Exception {
169+
170+
int pos = Arrays.mismatch(expected, actual);
171+
if (pos < 0) {
172+
log("Class bytes are identical.");
173+
return;
174+
}
175+
log("Class bytes are different.");
176+
printDisassembled("expected", expected);
177+
printDisassembled("expected", actual);
178+
fail(targetClassName + " did not match .class file");
179+
}
180+
181+
protected final void doRunTest() throws Throwable {
182+
beVerbose();
183+
184+
ClassLoader loader = getClass().getClassLoader();
185+
targetClass = loader.loadClass(targetClassName);
186+
// sanity check
187+
assertEquals(targetClassName, targetClass.getName());
188+
// sanity check
189+
verifyPresentMethodParams("intParam1", "stringParam1");
190+
191+
addTransformerToManager(fInst, new Transformer(), true);
192+
193+
{
194+
log("Testcase 1: ensure ClassFileReconstituter restores MethodParameters attribute");
195+
196+
byte[] classBytes = retransform(null);
197+
compareClassBytes(originalClassBytes, classBytes);
198+
199+
log("");
200+
}
201+
202+
{
203+
log("Testcase 2: redefine class with changed parameter names");
204+
205+
byte[] classBytes = Files.readAllBytes(Paths.get(
206+
ClassTransformer.fromTestSource(sourceFileName)
207+
.transform(1, targetClassName, "-g", "-parameters")));
208+
retransform(classBytes);
209+
// MethodParameters attribute should be updated.
210+
verifyPresentMethodParams("intParam2", "stringParam2");
211+
212+
log("");
213+
}
214+
215+
{
216+
log("Testcase 3: redefine class with no parameter names");
217+
// compile without "-parameters"
218+
byte[] classBytes = Files.readAllBytes(Paths.get(
219+
ClassTransformer.fromTestSource(sourceFileName)
220+
.transform(1, targetClassName, "-g")));
221+
retransform(classBytes);
222+
// MethodParameters attribute should be dropped.
223+
verifyAbsentMethodParams();
224+
225+
log("");
226+
}
227+
}
228+
229+
230+
public class Transformer implements ClassFileTransformer {
231+
public Transformer() {
232+
}
233+
234+
public String toString() {
235+
return Transformer.this.getClass().getName();
236+
}
237+
238+
public byte[] transform(ClassLoader loader, String className,
239+
Class<?> classBeingRedefined, ProtectionDomain protectionDomain, byte[] classfileBuffer) {
240+
241+
if (className.equals(targetClassName)) {
242+
log(this + ".transform() sees '" + className
243+
+ "' of " + classfileBuffer.length + " bytes.");
244+
seenClassBytes = classfileBuffer;
245+
if (newClassBytes != null) {
246+
log(this + ".transform() sets new classbytes for '" + className
247+
+ "' of " + newClassBytes.length + " bytes.");
248+
}
249+
return newClassBytes;
250+
}
251+
252+
return null;
253+
}
254+
}
255+
}

0 commit comments

Comments
 (0)