patch: javap does not show SourceDebugExtension properly

[Jonathan Gibbons]

This is now being tracked as issue 8019397.

-- Jon

On 06/26/2013 08:28 AM, Jonathan Gibbons wrote:
> Dmytro,
>
> Thank you. I have investigated this issue and agree with your 
> findings.  I'll file an issue and work towards fixing it.
>
> -- Jon
>
> On 06/26/2013 12:58 AM, Dmytro Sheyko wrote:
>> Hello,
>>
>> I am sending class file sample with sources and javap output:
>> Hello.jsp, greeting.jsp - original sources
>> Hello_jsp.java, Hello_jsp.class - files generated by Jasper (Apache 
>> Tomcat 7.0.41)
>> Hello_jsp.*.current.javap, Hello_jsp.*.patched.javap - javap output 
>> (without patch and with patch, used jdk1.7.0_25 and jdk1.8.0-ea-b91, 
>> however it seems there is no difference in patched classes between 
>> repositoried jdk7u/langtools and jdk8/langtools, whatever the case 
>> patch (which I sent earlier) is made on jdk8/langtools)
>>
>> Thanks,
>> Dmytro
>> ________________________________
>>> From: dmytro_sheyko at hotmail.com
>>> To: jonathan.gibbons at oracle.com; compiler-dev at openjdk.java.net
>>> Subject: RE: patch: javap does not show SourceDebugExtension properly
>>> Date: Tue, 25 Jun 2013 23:52:02 +0300
>>>   Jonathan,
>>>   This bug does not manifest itself with exception. javap just prints
>>>   SourceDebugExtension: null
>>>   In a class file content of SDE attribute appears as UTF-8 string, 
>>> where
>>> the first 4 bytes are 'S', 'M', 'A', 'P'. Existing code of
>>> SourceDebugExtension_attribute.getValue() uses readUTF and hence
>>> attempts to interpret the first two bytes (i.e. 'S' and 'M') as length
>>> of the string and then fails with EOFException. Then it catches this
>>> exception and returns null, which, in turn, appears in output.
>>>   As for sample class file, it's generated by Tomcat 7.0 runtime. I 
>>> will
>>> send it later.
>>>   Regards,
>>> Dmytro
>>>> Date: Tue, 25 Jun 2013 12:01:32 -0700
>>>> From: jonathan.gibbons at oracle.com
>>>> To: compiler-dev at openjdk.java.net
>>>> Subject: Re: patch: javap does not show SourceDebugExtension properly
>>>>
>>>> Dmytro,
>>>>
>>>> Thank you for your suggestion. Can you provide more details on why the
>>>> existing code does not work. Presumably you get an exception -- 
>>>> what is
>>>> it? Alternative, can you provide a sample class file for which the
>>>> existing code does not work, together with info on which version of
>>>> which compiler generated the class file?
>>>>
>>>> I agree that the change to visitSourceDebugExtension is probably 
>>>> desirable.
>>>>
>>>> -- Jon
>>>>
>>>>
>>>> On 06/25/2013 11:32 AM, Dmytro Sheyko wrote:
>>>>> Hello,
>>>>>
>>>>> I was trying to get SourceDebugExtension for compiled jsp class
>>> using javap (1.7.0_25, 1.8.0-ea-b91) and it printed something like:
>>>>> SourceFile: "x_jsp.java"
>>>>> SourceDebugExtension: null
>>>>> minor version: 0
>>>>> major version: 50
>>>>>
>>>>> where expected output is following:
>>>>>
>>>>> SourceFile: "x_jsp.java"
>>>>> SourceDebugExtension:
>>>>> SMAP
>>>>> x_jsp.java
>>>>> JSP
>>>>> *S JSP
>>>>> *F
>>>>> + 0 x.jsp
>>>>> x.jsp
>>>>> *L
>>>>> 1,3:62
>>>>> 4:65,4
>>>>> 6,4:70
>>>>> 9,4:74
>>>>> *E
>>>>> minor version: 0
>>>>> major version: 50
>>>>>
>>>>> Getting SourceDebugExtension using JDI works well (i.e. class file
>>> is correct).
>>>>> And I would like to propose a patch to fix this issue. Please review.
>>>>>
>>>>> diff -r b3458329d060
>>> src/share/classes/com/sun/tools/classfile/SourceDebugExtension_attribute.java 
>>>
>>>>> ---
>>> a/src/share/classes/com/sun/tools/classfile/SourceDebugExtension_attribute.java 
>>>
>>> Mon Jun 24 14:27:32 2013 -0700
>>>>> +++
>>> b/src/share/classes/com/sun/tools/classfile/SourceDebugExtension_attribute.java 
>>>
>>> Tue Jun 25 20:22:06 2013 +0300
>>>>> @@ -25,9 +25,8 @@
>>>>>
>>>>> package com.sun.tools.classfile;
>>>>>
>>>>> -import java.io.ByteArrayInputStream;
>>>>> -import java.io.DataInputStream;
>>>>> import java.io.IOException;
>>>>> +import java.nio.charset.Charset;
>>>>>
>>>>> /**
>>>>> * See JVMS, section 4.8.15.
>>>>> @@ -38,6 +37,8 @@
>>>>> * deletion without notice.</b>
>>>>> */
>>>>> public class SourceDebugExtension_attribute extends Attribute {
>>>>> + private static final Charset UTF8 = Charset.forName("UTF-8");
>>>>> +
>>>>> SourceDebugExtension_attribute(ClassReader cr, int name_index, int
>>> length) throws IOException {
>>>>> super(name_index, length);
>>>>> debug_extension = new byte[attribute_length];
>>>>> @@ -55,12 +56,7 @@
>>>>> }
>>>>>
>>>>> public String getValue() {
>>>>> - DataInputStream d = new DataInputStream(new
>>> ByteArrayInputStream(debug_extension));
>>>>> - try {
>>>>> - return d.readUTF();
>>>>> - } catch (IOException e) {
>>>>> - return null;
>>>>> - }
>>>>> + return new String(debug_extension, UTF8);
>>>>> }
>>>>>
>>>>> public <R, D> R accept(Visitor<R, D> visitor, D data) {
>>>>> diff -r b3458329d060
>>> src/share/classes/com/sun/tools/javap/AttributeWriter.java
>>>>> --- a/src/share/classes/com/sun/tools/javap/AttributeWriter.java
>>> Mon Jun 24 14:27:32 2013 -0700
>>>>> +++ b/src/share/classes/com/sun/tools/javap/AttributeWriter.java
>>> Tue Jun 25 20:22:06 2013 +0300
>>>>> @@ -511,7 +511,10 @@
>>>>> }
>>>>>
>>>>> public Void
>>> visitSourceDebugExtension(SourceDebugExtension_attribute attr, Void
>>> ignore) {
>>>>> - println("SourceDebugExtension: " + attr.getValue());
>>>>> + println("SourceDebugExtension:");
>>>>> + indent(+1);
>>>>> + print(attr.getValue());
>>>>> + indent(-1);
>>>>> return null;
>>>>> }
>>>>>
>>>>> Regards,
>>>>> Dmytro
>>>>
>