Bad handling of attributes which aren't parsed by the JVM

[samczsun]

Not really sure what happened here on Krakatau's end, but the JVM doesn't parse StackMapTable attributes for version < 50 so I assume you can stick any arbitrary data in there and Krakatau won't handle it too well

.version 49 0 
.class public final a
.super java/lang/Object 

.method public static main : ([Ljava/lang/String;)V
    .code stack 10 locals 10
        getstatic java/lang/System out Ljava/io/PrintStream;
        ldc "Hello"
        invokevirtual java/io/PrintStream println (Ljava/lang/String;)V
        return
        .stackmaptable
        .stackmaptable
    .end code
.end method 

.end class 

Traceback (most recent call last):
  File "disassemble.py", line 75, in <module>
    disassembleSub(readFile, out, targets, roundtrip=args.roundtrip, outputClassName=False)
  File "disassemble.py", line 44, in disassembleSub
    Disassembler(clsdata, output.write, roundtrip=roundtrip).disassemble()
  File "C:\Users\Sam\IdeaProjects\Krakatau\Krakatau\assembler\disassembly.py", line 341, in disassemble
    a.method(m)
  File "C:\Users\Sam\IdeaProjects\Krakatau\Krakatau\assembler\disassembly.py", line 372, in method
    a.attribute(attr, in_method=True)
  File "C:\Users\Sam\IdeaProjects\Krakatau\Krakatau\assembler\disassembly.py", line 571, in attribute
    a.code(r)
  File "C:\Users\Sam\IdeaProjects\Krakatau\Krakatau\assembler\disassembly.py", line 418, in code
    stackreader.setdata(attr.stream())
  File "C:\Users\Sam\IdeaProjects\Krakatau\Krakatau\assembler\disassembly.py", line 56, in setdata
    self.count = r.u16() + 1
  File "C:\Users\Sam\IdeaProjects\Krakatau\Krakatau\classfileformat\reader.py", line 13, in u16
    def u16(self): return self.get('>H')
  File "C:\Users\Sam\IdeaProjects\Krakatau\Krakatau\classfileformat\reader.py", line 21, in get
    val = struct.unpack_from(fmt, self.d, self.off)
struct.error: unpack_from requires a buffer of at least 2 bytes

[samczsun]

Actually, looks like a lot of attributes fall into this category. For example, according to the hotspot source RuntimeInvisibleAnnotations isn't parsed for anything older than 49, so .attribute RuntimeInvisibleAnnotations '\x00\x00\x00\x00' will cause a crash in Krakatau but no error in the JVM. I assume that Class#getAnnotations will be affected for 49+ but I haven't tested it

[Storyyeller]

I'd been meaning to look into what happens when you use predefined attributes with a version before they were defined, but never got around to it and then forgot. Thanks for catching this.

[Storyyeller]

I did some testing to try to figure out which versions each attribute is verified in, but I ran into some issues - it looks like some attributes aren't verified at all, regardless of the version. There's also the question of how to handle e.g. stack maps in version 50.0, where they are optional, as well as attributes defined in the wrong place.

I can't even use the brute force approach of wrapping attribute assembly in a try block and emitting binary if it fails because the disassembler is single pass so attribute assembly may have already written partial output by the time it throws.

[samczsun]

Sounds like a fun problem.

Because of the single pass issue, I guess you could write separate verifiers for each attribute type to make sure that the binary data conforms to the spec. Then if the blob passes verification you could pass it onto the current disassembling code.

[Storyyeller]

I think it would be easier to just change the disassembler to output to a temporary buffer when disassembling attributes.

[Storyyeller]

I decided to go with the approach of trying to disassemble the attributes and falling back to binary output if there is an error. It took some horrible hacks to properly support the case of version 50 StackMapTable attributes, but at least it works. Anyway, please try it out and see what you think.

[samczsun]

Awesome! Bit busy this week but I can't wait to give it a shot