From 06f3c20da89474cde04f74f128a2145657562838 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jindra=20Pet=C5=99=C3=ADk?= Date: Sun, 24 Aug 2025 00:11:01 +0200 Subject: [PATCH] Added: #2519 AS1/2 direct editation - better handling of "too large" error messages - highlighting actual problematic line --- CHANGELOG.md | 3 +- .../action/model/FunctionActionItem.java | 3 + .../clauses/IfFrameLoadedActionItem.java | 3 + .../parser/script/ActionScript2Parser.java | 25 +++- .../parser/script/ActionSourceGenerator.java | 138 ++++++++++++++---- .../decompiler/graph/GraphTargetItem.java | 5 + .../flash/gui/editor/HighlightsPanel.java | 3 + 7 files changed, 149 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 349b76b55..6d25ff7c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,7 +22,8 @@ All notable changes to this project will be documented in this file. - [#2499] Information about frames which do not have ShowFrame tag - [#2504] WebP image format for export/import (not animated) Limitation: It's not available on Mac x86-64 platform -- [#2519] AS1/2 P-code editation - better handling of "too large" error messages +- [#2519] AS1/2 Direct editation + P-code - better handling of "too large" + error messages - highlight actual problematic structure ### Fixed - [#2474] Gotos incorrectly decompiled diff --git a/libsrc/ffdec_lib/src/com/jpexs/decompiler/flash/action/model/FunctionActionItem.java b/libsrc/ffdec_lib/src/com/jpexs/decompiler/flash/action/model/FunctionActionItem.java index 19af3495e..c039e5b0d 100644 --- a/libsrc/ffdec_lib/src/com/jpexs/decompiler/flash/action/model/FunctionActionItem.java +++ b/libsrc/ffdec_lib/src/com/jpexs/decompiler/flash/action/model/FunctionActionItem.java @@ -548,6 +548,9 @@ public class FunctionActionItem extends ActionItem implements BranchStackResista } } int len = Action.actionsToBytes(asGenerator.toActionList(ret), false, SWF.DEFAULT_VERSION).length; + if (len > 0xFFFF) { + throw new CompilationException("Function body is too large to fit into UI16.", line); + } if (!needsFun2 && paramNames.isEmpty()) { ret.add(0, new ActionDefineFunction(functionName, paramNames, len, SWF.DEFAULT_VERSION, charset)); } else { diff --git a/libsrc/ffdec_lib/src/com/jpexs/decompiler/flash/action/model/clauses/IfFrameLoadedActionItem.java b/libsrc/ffdec_lib/src/com/jpexs/decompiler/flash/action/model/clauses/IfFrameLoadedActionItem.java index ec07f9d5a..9fd2a02bb 100644 --- a/libsrc/ffdec_lib/src/com/jpexs/decompiler/flash/action/model/clauses/IfFrameLoadedActionItem.java +++ b/libsrc/ffdec_lib/src/com/jpexs/decompiler/flash/action/model/clauses/IfFrameLoadedActionItem.java @@ -116,6 +116,9 @@ public class IfFrameLoadedActionItem extends ActionItem implements Block { List body = generator.generate(localData, actions); ActionSourceGenerator actionGenerator = (ActionSourceGenerator) generator; String charset = actionGenerator.getCharset(); + if (body.size() > 0xFF) { + throw new CompilationException("ifFrameLoaded body exceeds limit of 255 actions", line); + } return toSourceMerge(localData, generator, frame, new ActionWaitForFrame2(body.size(), charset), body); } diff --git a/libsrc/ffdec_lib/src/com/jpexs/decompiler/flash/action/parser/script/ActionScript2Parser.java b/libsrc/ffdec_lib/src/com/jpexs/decompiler/flash/action/parser/script/ActionScript2Parser.java index 6f1085777..a6e89c41f 100644 --- a/libsrc/ffdec_lib/src/com/jpexs/decompiler/flash/action/parser/script/ActionScript2Parser.java +++ b/libsrc/ffdec_lib/src/com/jpexs/decompiler/flash/action/parser/script/ActionScript2Parser.java @@ -416,6 +416,7 @@ public class ActionScript2Parser { } private FunctionActionItem function(boolean withBody, String functionName, boolean isMethod, List variables, List functions, boolean inTellTarget, Reference hasEval) throws IOException, ActionParseException, InterruptedException { + int functionLine = lexer.yyline(); GraphTargetItem ret = null; ParsedSymbol s; expectedType(SymbolType.PARENT_OPEN); @@ -455,6 +456,7 @@ public class ActionScript2Parser { FunctionActionItem retf = new FunctionActionItem(null, null, functionName, paramNames, new HashMap<>() /*?*/, body, constantPool, -1, subvariables, subfunctions, subHasEval.getVal(), new ArrayList<>(), null); functions.add(retf); + retf.line = functionLine; return retf; } @@ -1082,6 +1084,7 @@ public class ActionScript2Parser { switch (s.type) { case WITH: + int withLine = lexer.yyline(); expectedType(SymbolType.PARENT_OPEN); GraphTargetItem wvar = expression(inFunction, inMethod, inTellTarget, false, variables, functions, false, hasEval); expectedType(SymbolType.PARENT_CLOSE); @@ -1089,6 +1092,7 @@ public class ActionScript2Parser { List wcmd = commands(inFunction, inMethod, forinlevel, inTellTarget, variables, functions, hasEval); expectedType(SymbolType.CURLY_CLOSE); ret = new WithActionItem(null, null, wvar, wcmd); + ret.line = withLine; break; case DELETE: GraphTargetItem varDel = expression(inFunction, inMethod, inTellTarget, false, variables, functions, false, hasEval); @@ -1107,6 +1111,7 @@ public class ActionScript2Parser { } break; case TELLTARGET: + int tellTargetLine = lexer.yyline(); expectedType(SymbolType.PARENT_OPEN); GraphTargetItem tellTarget = expression(inFunction, inMethod, inTellTarget, true, variables, functions, false, hasEval); expectedType(SymbolType.PARENT_CLOSE); @@ -1116,11 +1121,13 @@ public class ActionScript2Parser { TellTargetActionItem tt = new TellTargetActionItem(null, null, tellTarget, tellcmds); if (inTellTarget) { tt.nested = true; - } + } ret = tt; + ret.line = tellTargetLine; break; case IFFRAMELOADED: + int ifFrameLoadedLine = lexer.yyline(); expectedType(SymbolType.PARENT_OPEN); GraphTargetItem iflExpr = (expression(inFunction, inMethod, inTellTarget, true, variables, functions, false, hasEval)); expectedType(SymbolType.PARENT_CLOSE); @@ -1128,8 +1135,10 @@ public class ActionScript2Parser { List iflComs = commands(inFunction, inMethod, forinlevel, inTellTarget, variables, functions, hasEval); expectedType(SymbolType.CURLY_CLOSE); ret = new IfFrameLoadedActionItem(iflExpr, iflComs, null, null); + ret.line = ifFrameLoadedLine; break; case CLASS: + int classLine = lexer.yyline(); GraphTargetItem classTypeStr = type(variables); s = lex(); GraphTargetItem extendsTypeStr = null; @@ -1147,6 +1156,7 @@ public class ActionScript2Parser { } expected(s, lexer.yyline(), SymbolType.CURLY_OPEN); ret = (traits(false, classTypeStr, extendsTypeStr, implementsTypeStrs, variables, functions, inTellTarget, hasEval)); + ret.line = classLine; expectedType(SymbolType.CURLY_CLOSE); break; case INTERFACE: @@ -1217,6 +1227,7 @@ public class ActionScript2Parser { } break; case IF: + int ifLine = lexer.yyline(); expectedType(SymbolType.PARENT_OPEN); GraphTargetItem ifExpr = (expression(inFunction, inMethod, inTellTarget, true, variables, functions, false, hasEval)); expectedType(SymbolType.PARENT_CLOSE); @@ -1232,8 +1243,10 @@ public class ActionScript2Parser { lexer.pushback(s); } ret = new IfItem(DIALECT, null, null, ifExpr, onTrueList, onFalseList); + ret.line = ifLine; break; case WHILE: + int whileLine = lexer.yyline(); expectedType(SymbolType.PARENT_OPEN); List whileExpr = new ArrayList<>(); whileExpr.add(expression(inFunction, inMethod, inTellTarget, true, variables, functions, true, hasEval)); @@ -1241,8 +1254,10 @@ public class ActionScript2Parser { List whileBody = new ArrayList<>(); whileBody.add(command(inFunction, inMethod, forinlevel, inTellTarget, true, variables, functions, hasEval)); ret = new WhileItem(DIALECT, null, null, null, whileExpr, whileBody); + ret.line = whileLine; break; case DO: + int doLine = lexer.yyline(); List doBody = new ArrayList<>(); doBody.add(command(inFunction, inMethod, forinlevel, inTellTarget, true, variables, functions, hasEval)); expectedType(SymbolType.WHILE); @@ -1251,8 +1266,10 @@ public class ActionScript2Parser { doExpr.add(expression(inFunction, inMethod, inTellTarget, true, variables, functions, true, hasEval)); expectedType(SymbolType.PARENT_CLOSE); ret = new DoWhileItem(DIALECT, null, null, null, doBody, doExpr); + ret.line = doLine; break; case FOR: + int forLine = lexer.yyline(); expectedType(SymbolType.PARENT_OPEN); s = lex(); boolean forin = false; @@ -1353,8 +1370,10 @@ public class ActionScript2Parser { } else { ret = new ForItem(DIALECT, null, null, null, forFirstCommands, forExpr, forFinalCommands, forBody); } + ret.line = forLine; break; case SWITCH: + int switchLine = lexer.yyline(); expectedType(SymbolType.PARENT_OPEN); GraphTargetItem switchExpr = expression(inFunction, inMethod, inTellTarget, true, variables, functions, false, hasEval); expectedType(SymbolType.PARENT_CLOSE); @@ -1392,6 +1411,7 @@ public class ActionScript2Parser { } expected(s, lexer.yyline(), SymbolType.CURLY_CLOSE); ret = new SwitchItem(DIALECT, null, null, null, switchExpr, caseExprsAll, caseCmds, valueMapping); + ret.line = switchLine; break; case BREAK: ret = new BreakItem(DIALECT, null, null, 0); //? There is no more than 1 level continue/break in AS1/2 @@ -1407,6 +1427,7 @@ public class ActionScript2Parser { ret = new ReturnActionItem(null, null, retexpr); break; case TRY: + int tryLine = lexer.yyline(); List tryCommands = new ArrayList<>(); tryCommands.add(command(inFunction, inMethod, forinlevel, inTellTarget, true, variables, functions, hasEval)); s = lex(); @@ -1446,6 +1467,7 @@ public class ActionScript2Parser { } lexer.pushback(s); ret = new TryActionItem(tryCommands, catchExceptionNames, catchExceptionTypes, catchCommands, finallyCommands); + ret.line = tryLine; break; case THROW: ret = new ThrowActionItem(null, null, expression(inFunction, inMethod, inTellTarget, true, variables, functions, false, hasEval)); @@ -1590,6 +1612,7 @@ public class ActionScript2Parser { case TERNAR: lhs = new TernarOpItem(DIALECT, null, null, lhs, mhs, rhs); + lhs.line = lexer.yyline(); break; case SHIFT_LEFT: lhs = new LShiftActionItem(null, null, lhs, rhs); diff --git a/libsrc/ffdec_lib/src/com/jpexs/decompiler/flash/action/parser/script/ActionSourceGenerator.java b/libsrc/ffdec_lib/src/com/jpexs/decompiler/flash/action/parser/script/ActionSourceGenerator.java index 60e592d4c..fa55a8fe5 100644 --- a/libsrc/ffdec_lib/src/com/jpexs/decompiler/flash/action/parser/script/ActionSourceGenerator.java +++ b/libsrc/ffdec_lib/src/com/jpexs/decompiler/flash/action/parser/script/ActionSourceGenerator.java @@ -73,8 +73,10 @@ import com.jpexs.helpers.Helper; import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.logging.Logger; @@ -156,7 +158,14 @@ public class ActionSourceGenerator implements SourceGenerator { private List groupPushes(List items) { - //TODO: This should take in account important offsets (jumps) + if (swfVersion <= 4) { + return items; + } + + + return items; + + //TODO: This should take into account important offsets (jumps) //And not group Pushes over different parts of code /*Like: If locA @@ -166,12 +175,25 @@ public class ActionSourceGenerator implements SourceGenerator { Push "C" Should not be grouped to Push "A","B","C" - */ - return items; - //Commented out for now... - /*if (swfVersion <= 4) { - return items; + + + //commented out for now... + Map addresses = new LinkedHashMap<>(); + List referencedAddresses = new ArrayList<>(); + long addr = 0; + for (GraphSourceItem item : items) { + addresses.put(item, addr); + if (item instanceof ActionIf) { + ActionIf aIf = (ActionIf) item; + referencedAddresses.add(addr + 5 + aIf.getJumpOffset()); + } + if (item instanceof ActionJump) { + ActionJump aJump = (ActionJump) item; + referencedAddresses.add(addr + 5 + aJump.getJumpOffset()); + } + addr += item.getBytesLength(); } + List ret = new ArrayList<>(); ActionPush prevPush = null; for (GraphSourceItem s : items) { @@ -180,15 +202,21 @@ public class ActionSourceGenerator implements SourceGenerator { prevPush = (ActionPush) s; ret.add(prevPush); } else { - prevPush.values.addAll(((ActionPush) s).values); - ((ActionPush) s).values.clear(); + long itemAddr = addresses.get(s); + if (referencedAddresses.contains(itemAddr)) { + prevPush = (ActionPush) s; + ret.add(prevPush); + } else { + prevPush.values.addAll(((ActionPush) s).values); + ((ActionPush) s).values.clear(); + } } } else { ret.add(s); prevPush = null; } } - return ret;*/ + return ret; */ } private List nonempty(List list) { @@ -198,7 +226,7 @@ public class ActionSourceGenerator implements SourceGenerator { return list; } - private List generateIf(SourceGeneratorLocalData localData, GraphTargetItem expression, List onTrueCmds, List onFalseCmds, boolean ternar) throws CompilationException { + private List generateIf(SourceGeneratorLocalData localData, GraphTargetItem expression, List onTrueCmds, List onFalseCmds, boolean ternar, int errorLine, String errorTitle) throws CompilationException { List ret = new ArrayList<>(); if (expression instanceof NotItem) { ret.addAll(expression.value.toSource(localData, this)); @@ -223,6 +251,10 @@ public class ActionSourceGenerator implements SourceGenerator { } byte[] onTrueBytes = Action.actionsToBytes(onTrue, false, SWF.DEFAULT_VERSION); int onTrueLen = onTrueBytes.length; + + if (onTrueLen > 32767) { + throw new CompilationException("Generated offset for onTrue part of " + errorTitle + " is larger than maximum allowed for SI16.", errorLine); + } ActionIf ifaif = new ActionIf(0, charset); ret.add(ifaif); @@ -237,33 +269,48 @@ public class ActionSourceGenerator implements SourceGenerator { ret.add(ajmp); onTrueLen += ajmp.getTotalActionLength(); } + if (onTrueLen > 32767) { + throw new CompilationException("Generated offset for onTrue part of " + errorTitle + " is larger than maximum allowed for SI16.", errorLine); + } ifaif.setJumpOffset(onTrueLen); byte[] onFalseBytes = Action.actionsToBytes(onFalse, false, SWF.DEFAULT_VERSION); int onFalseLen = onFalseBytes.length; if (ajmp != null) { ajmp.setJumpOffset(onFalseLen); } + if (onTrueLen > 32767) { + throw new CompilationException("Generated offset for onFalse part of " + errorTitle + " is larger than maximum allowed for SI16.", errorLine); + } ret.addAll(onFalse); } return ret; } - private void fixLoop(List code, int breakOffset) { - fixLoop(code, breakOffset, Integer.MAX_VALUE); + private void fixLoop(List code, int breakOffset, GraphTargetItem item) throws CompilationException { + fixLoop(code, breakOffset, Integer.MAX_VALUE, item); } - private void fixLoop(List code, int breakOffset, int continueOffset) { + private void fixLoop(List code, int breakOffset, int continueOffset, GraphTargetItem item) throws CompilationException { int pos = 0; for (Action a : code) { pos += a.getBytesLength(); if (a instanceof ActionJump) { ActionJump aj = (ActionJump) a; if (aj.isContinue && (continueOffset != Integer.MAX_VALUE)) { - aj.setJumpOffset(-pos + continueOffset); + int nOffset = -pos + continueOffset; + if (nOffset < -32768 || nOffset > 32767) { + throw new CompilationException("Generated offset for Continue is outside bounds of SI16.", item.line); + } + + aj.setJumpOffset(nOffset); aj.isContinue = false; } if (aj.isBreak) { - aj.setJumpOffset(-pos + breakOffset); + int nOffset = -pos + breakOffset; + if (nOffset < -32768 || nOffset > 32767) { + throw new CompilationException("Generated offset for Break is outside bounds of SI16.", item.line); + } + aj.setJumpOffset(nOffset); aj.isBreak = false; } } @@ -835,7 +882,7 @@ public class ActionSourceGenerator implements SourceGenerator { @Override public List generate(SourceGeneratorLocalData localData, IfItem item) throws CompilationException { - return generateIf(localData, item.expression, item.onTrue, item.onFalse, false); + return generateIf(localData, item.expression, item.onTrue, item.onFalse, false, item.line, "If"); } @Override @@ -844,7 +891,7 @@ public class ActionSourceGenerator implements SourceGenerator { onTrue.add(item.onTrue); List onFalse = new ArrayList<>(); onFalse.add(item.onFalse); - return generateIf(localData, item.expression, onTrue, onFalse, true); + return generateIf(localData, item.expression, onTrue, onFalse, true, item.line, "Ternar"); } @Override @@ -867,11 +914,19 @@ public class ActionSourceGenerator implements SourceGenerator { whileBody.add(whileajmp); int whileExprLen = Action.actionsToBytes(whileExpr, false, SWF.DEFAULT_VERSION).length; int whileBodyLen = Action.actionsToBytes(whileBody, false, SWF.DEFAULT_VERSION).length; - whileajmp.setJumpOffset(-(whileExprLen - + whileBodyLen)); + + if (whileBodyLen > 32767) { + throw new CompilationException("Generated offset for While is larger than maximum allowed for SI16.", item.line); + } + + int whileJumpOffset = -(whileExprLen + whileBodyLen); + if (whileJumpOffset < -32768) { + throw new CompilationException("Generated offset for While is lower than mininum allowed for SI16.", item.line); + } + whileajmp.setJumpOffset(whileJumpOffset); whileaif.setJumpOffset(whileBodyLen); ret.addAll(whileExpr); - fixLoop(whileBody, whileBodyLen, -whileExprLen); + fixLoop(whileBody, whileBodyLen, -whileExprLen, item); ret.addAll(whileBody); return ret; } @@ -896,9 +951,13 @@ public class ActionSourceGenerator implements SourceGenerator { ret.addAll(doExpr); ActionIf doif = new ActionIf(0, charset); ret.add(doif); - int offset = doBodyLen + doExprLen + doif.getTotalActionLength(); + int offset = doBodyLen + doExprLen + doif.getTotalActionLength(); + if (-offset < -32768) { + throw new CompilationException("Generated offset for DoWhile is lower than mininum allowed for SI16.", item.line); + } + doif.setJumpOffset(-offset); - fixLoop(doBody, offset, doBodyLen); + fixLoop(doBody, offset, doBodyLen, item); return ret; } @@ -917,18 +976,26 @@ public class ActionSourceGenerator implements SourceGenerator { int forExprLen = Action.actionsToBytes(forExpr, false, SWF.DEFAULT_VERSION).length; int forBodyLen = Action.actionsToBytes(forBody, false, SWF.DEFAULT_VERSION).length; int forFinalLen = Action.actionsToBytes(forFinalCommands, false, SWF.DEFAULT_VERSION).length; - forajmp.setJumpOffset(-(forExprLen - + forBodyLen + forFinalLen + forajmpLen)); - foraif.setJumpOffset(forBodyLen + forFinalLen + forajmpLen); + int jmpOffset = -(forExprLen + forBodyLen + forFinalLen + forajmpLen); + int ifOffset = forBodyLen + forFinalLen + forajmpLen; + if (jmpOffset < -32768) { + throw new CompilationException("Generated offset for For is lower than mininum allowed for SI16.", item.line); + } + if (ifOffset > 32767) { + throw new CompilationException("Generated offset for For is larger than maximum allowed for SI16.", item.line); + } + + forajmp.setJumpOffset(jmpOffset); + foraif.setJumpOffset(ifOffset); ret.addAll(generateToActionList(localData, item.firstCommands)); ret.addAll(forExpr); ret.addAll(forBody); ret.addAll(forFinalCommands); ret.add(forajmp); - fixLoop(forBody, forBodyLen + forFinalLen + forajmpLen, forBodyLen); + fixLoop(forBody, forBodyLen + forFinalLen + forajmpLen, forBodyLen, item); return ret; - } - + } + @Override public List generate(SourceGeneratorLocalData localData, SwitchItem item) throws CompilationException { List ret = new ArrayList<>(); @@ -1022,6 +1089,7 @@ public class ActionSourceGenerator implements SourceGenerator { for (int n = 0; n < i; n++) { jmpPos += caseLengths.get(n); } + checkOffsetBounds(jmpPos, "Switch", item.line); caseIfs.get(i).get(c).setJumpOffset(jmpPos); } } @@ -1032,6 +1100,8 @@ public class ActionSourceGenerator implements SourceGenerator { } } + checkOffsetBounds(defJmpPos, "Switch", item.line); + defJump.setJumpOffset(defJmpPos); List caseCmdsAll = new ArrayList<>(); int breakOffset = 0; @@ -1039,7 +1109,7 @@ public class ActionSourceGenerator implements SourceGenerator { caseCmdsAll.addAll(caseCmds.get(i)); breakOffset += caseLengths.get(i); } - fixLoop(caseCmdsAll, breakOffset); + fixLoop(caseCmdsAll, breakOffset, item); return ret; } @@ -1121,4 +1191,14 @@ public class ActionSourceGenerator implements SourceGenerator { return ret; } + + private int checkOffsetBounds(int offset, String errorItem, int errorLine) throws CompilationException { + if (offset < -32768) { + throw new CompilationException("Generated offset for " + errorItem + " is lower than mininum allowed for SI16.", errorLine); + } + if (offset > 32767) { + throw new CompilationException("Generated offset for " + errorItem + " is larger than maximum allowed for SI16.", errorLine); + } + return offset; + } } diff --git a/libsrc/ffdec_lib/src/com/jpexs/decompiler/graph/GraphTargetItem.java b/libsrc/ffdec_lib/src/com/jpexs/decompiler/graph/GraphTargetItem.java index cdd294964..cbd208fc2 100644 --- a/libsrc/ffdec_lib/src/com/jpexs/decompiler/graph/GraphTargetItem.java +++ b/libsrc/ffdec_lib/src/com/jpexs/decompiler/graph/GraphTargetItem.java @@ -132,6 +132,11 @@ public abstract class GraphTargetItem implements Serializable, Cloneable { */ public int outputPos = -1; + /** + * Line in decompiled source code. Used mainly in Parsers/Code generators to report bugs. + */ + public int line; + /** * Gets the line start item * diff --git a/src/com/jpexs/decompiler/flash/gui/editor/HighlightsPanel.java b/src/com/jpexs/decompiler/flash/gui/editor/HighlightsPanel.java index 9b35fe809..489d83ce1 100644 --- a/src/com/jpexs/decompiler/flash/gui/editor/HighlightsPanel.java +++ b/src/com/jpexs/decompiler/flash/gui/editor/HighlightsPanel.java @@ -179,6 +179,9 @@ public class HighlightsPanel extends JPanel { int currentLine = editorPane.getLine(); for (Highlighter.Highlight highlight : highlights) { + if (highlight == null) { + continue; + } Highlighter.HighlightPainter painter = highlight.getPainter(); if ((painter instanceof UnderlinePainter) && ((UnderlinePainter) painter).getColor() == null) { continue;