Printing instruction list for debugging/troubleshooting

[chylex]

Discussed this a bit on stream before, so I'm making an issue to figure out the approach.

I generate a log with all instructions of the transformed method when the transformer fails, which makes it really easy to diagnose issues with two mods transforming the same method. This is what I used in 1.12 and older:

TraceMethodVisitor visitor = new TraceMethodVisitor(new Textifier());

for(Iterator<AbstractInsnNode> iter = method.instructions.iterator(); iter.hasNext();){
    iter.next().accept(visitor);
}

// print visitor.p.getText()

with an example section of the output:

ALOAD 0
GETFIELD bud.S : Ljava/util/Random;
INVOKEVIRTUAL java/util/Random.nextFloat ()F
LDC 0.4

On stream you said Textifier is no longer available as asm-util is not included anymore, so I could either:

• Re-add asm-util and add a utility method to ASMAPI with the code above, or
• Implement custom printouts for each instruction type that'd look similar to Textifier output, this could perhaps also include remapped names

Let me know what you think, I could PR this sometime next week.

[cpw]

This should probably be in the modlauncher, rather than the coremod system, since coremods are just a participant there.

[chylex]

I manually do the instruction printing from coremods when the transformation fails, how would it work in the modlauncher?

[cpw]

This logic already exists in ModLauncher. If you want a post-processed class to analyze at your leisure, you should be able to set "CLASSDUMP" and trace for that, and it'll dump every class to a temporary directory.

https://github.com/cpw/modlauncher/blob/4e700d932ea22eb30c5517a5553ef7e981d3f360/src/main/java/cpw/mods/modlauncher/ClassTransformer.java#L121

Also

https://github.com/cpw/modlauncher/blob/5c88bc49740952971a89e67d214d739d5249402a/src/test/resources/log4j2.xml#L7

So, you can set the system property -Dmodlauncher.logging.marker.classdump=ACCEPT to get a dump of classes, I believe.

[chylex]

Can't get it to work, I set the system property but it's not getting through the MarkerManager.exists("CLASSDUMP") condition.

The main point of this is so users who run into conflicting transformers can send me the class dump which caused my transformer to fail, so I want to make it really easy for them to find what I need.

Just throwing it into the log file is the easiest and what I do now, but a system property that forces a class dump would also work - but in that case I'd rather make it a single property that overrides all the other conditions, so there's no need to deal with log configuration (unless there is a way to enable both the classdump marker and trace logging only via system properties).

[cpw]

It should work, I was testing using the same flag. :thinking:

[chylex]

I'm probably doing something wrong... I tried using the system property both in the forge profile in the launcher, and in dev environment. Stepping through the latter returns false on MarkerManager.exists so it never calls dumpClass.

property 'modlauncher.logging.marker.classdump', 'ACCEPT'

[HellFirePvP]

Along with this, maybe just dumping the transformed bytes into a file could also be an option?

At least that's how i used to double check on my ASM results by dumping them into a file and re-opening them with a decompiler. This might not be very conceptually pleasing, but i do have to say, whenever there was a mistake, it was very obvious from looking at the decompiled class, whereas looking at written-out bytecode instructions might not be too obvious. Needless to say, i do miss having that option with the newer forge.

Example: https://github.com/HellFirePvP/AstralSorcery/blob/master/src/main/java/hellfirepvp/astralsorcery/core/AstralTransformer.java#L74-L86 (And yes, i am aware that the implementation isn't exactly cross-platform friendly as the directory path shows, but it's the concept that counts here)

[cpw]

Dumping classes is already a thing. It's supposed to be working. I'll investigate.

[chylex]

Dumping is good for devs, but as I said, I use this as a tool to diagnose errors/conflicts on the user's side - if a transformer fails, I dump instructions into the log and just tell the user to pastebin the log.

I don't know how other devs deal with coremod conflicts when a user issue report has 100+ mods, but my way has already helped me find the conflicting mod within seconds. Since support for multiple JS files was added, my current solution is fine since it's no longer cluttering the main JS file:

• https://github.com/chylex/Better-Sprinting/blob/master/src/main/resources/coremods/utils/helpers.js#L58-L108
• https://github.com/chylex/Better-Sprinting/blob/master/src/main/resources/coremods/utils/constants.js

but it could be nice to have it as a utility.

[covers1624]

Having something JS side to print instructions like the OP would be really nice. I have had some cases where my injection was seemingly correct but was failing to merge stack frames when writing the class. I ended up rolling something similar to TraceMethodVIsitor + Textifier in JS to debug that. Whilst general class dumps are nice, being able to log it is also nice.

[HellFirePvP]

As was noted before here, there is uility to dump a class' bytes into a file. This can be found here: https://github.com/cpw/modlauncher/blob/master/src/main/java/cpw/mods/modlauncher/ClassTransformer.java#L122-L150 (Edit: Woops, this was apparently already linked above)

However, as also stated here, this doesn't seem to always function. So, imo, this should be fixed rather than adding homebrew solutions.

The dumped classes can then be opened by decompilers to both see the code in a somewhat readable form & have access to how its bytecode looks like. Readable bytecode-only output as dumped by textifier, one crude way can be seen on Stackoverflow would potentially be perferable i suppose..? Though personally i can conduct more information from having the side-by-side view of decompiled readable class + readable bytecode instructions like stuff BytecodeViewer spews out.

[gabizou]

Just adding on to this issue, there's a real oddity with SpongeForge running on 1.16 (not nearly relevant) that has some mixins applied and effectively, according to Mixin's dumped class file (through -Dmixin.debug.export=true) the class should be fine, but apparently there's another transformation going on after Mixin that ends up with a class verify error:

All of the debugging logs are available here: https://gist.github.com/gabizou/beef7061791cdf3fdebb46b810362300

And the post transformed class Mixin can see: EntityClassification.class.zip

Versus the actual post transformed class: net.minecraft.entity.EntityClassification.class.zip

The actual classes are different!

BUT, trying to enable mod launcher's final class transformed export is a brain ache and a half because it's not obvious what the actual flags required are (having tried with the flags mentioned above doesn't appear to actually function), let alone having to basically figure out "oh, I need to download that dev log4j file and use the flags: -Dlog4j.configurationFile=foo.xml -Dmodlauncher.logging.marker.classdump=ACCEPT and then find the dumped directory in your system temp directory is just a bad experience....

What this should really be is a standard property to dump classes to file (Seriously, we should learn from log4shell, just don't let logging configurations be a deterministic behavior change) and just maybe put that folder in the same server directory if possible? Mixin does this out of the box already and that's one of the very well defined properties to help in debugging what's going wrong, but in this case, I spent so much time to find out just how to get the final transformed classes to find out "what's going on"....

[Jonathing]

This was fixed in 5579913. Additionally, I am adding methods in #49 that allow for printing single instructions or instruction lists.