Skip to content

Fix ExifTool: eval STRING AST mutation, join LIST context, interpreter improvements #258

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
fglock merged 5 commits into from
Mar 2, 2026
Merged

Fix ExifTool: eval STRING AST mutation, join LIST context, interpreter improvements #258

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e69bf39
Enable interpreter for eval STRING by default, add JPERL_EVAL_NO_INTE…
fglock Mar 2, 2026
0ab4c67
Fix eval STRING AST mutation corrupting my variables in loops, fix jo…
fglock Mar 2, 2026
2995d41
Fix goto LABEL in bytecode interpreter: support intra-function jumps
fglock Mar 2, 2026
84589ad
Fix eval STRING AST mutation: use IdentityHashMap for BEGIN variable IDs
fglock Mar 2, 2026
d0a59dd
Fix utime flaky test: sanitize filenames with embedded NUL characters
fglock Mar 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 25 additions & 19 deletions src/main/java/org/perlonjava/backend/bytecode/BytecodeCompiler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,11 @@ private static class LoopInfo {
}
}

// Goto label support: maps label names to their PC addresses for intra-function goto.
// pendingGotos tracks forward references (goto before label) needing patch-up.
final Map<String, Integer> gotoLabelPcs = new HashMap<>();
final List<Object[]> pendingGotos = new ArrayList<>(); // [patchPc(Integer), labelName(String)]

// Token index tracking for error reporting
private final TreeMap<Integer, Integer> pcToTokenIndex = new TreeMap<>();
int currentTokenIndex = -1; // Track current token for error reporting
Expand Down Expand Up @@ -1734,15 +1739,9 @@ void compileVariableDeclaration(OperatorNode node, String op) {
boolean isDeclaredReference = node.annotations != null &&
Boolean.TRUE.equals(node.annotations.get("isDeclaredReference"));

// Check if this variable is captured by closures (sigilOp.id != 0) or is a state variable
// State variables always use persistent storage
if (sigilOp.id != 0 || op.equals("state")) {
// Variable is captured by compiled named subs or is a state variable
// Store as persistent variable so both interpreted and compiled code can access it
// Don't use a local register; instead load/store through persistent globals

// For state variables, retrieve or initialize the persistent variable
// For captured variables, retrieve the BEGIN-initialized variable
Integer beginId = RuntimeCode.evalBeginIds.get(sigilOp);
if (beginId != null || op.equals("state")) {
int persistId = beginId != null ? beginId : sigilOp.id;
int reg = allocateRegister();
int nameIdx = addToStringPool(varName);

Expand All @@ -1751,21 +1750,21 @@ void compileVariableDeclaration(OperatorNode node, String op) {
emitWithToken(Opcodes.RETRIEVE_BEGIN_SCALAR, node.getIndex());
emitReg(reg);
emit(nameIdx);
emit(sigilOp.id);
emit(persistId);
registerVariable(varName, reg);
}
case "@" -> {
emitWithToken(Opcodes.RETRIEVE_BEGIN_ARRAY, node.getIndex());
emitReg(reg);
emit(nameIdx);
emit(sigilOp.id);
emit(persistId);
registerVariable(varName, reg);
}
case "%" -> {
emitWithToken(Opcodes.RETRIEVE_BEGIN_HASH, node.getIndex());
emitReg(reg);
emit(nameIdx);
emit(sigilOp.id);
emit(persistId);
registerVariable(varName, reg);
}
default -> throwCompilerException("Unsupported variable type: " + sigil);
Expand Down Expand Up @@ -2106,9 +2105,9 @@ void compileVariableDeclaration(OperatorNode node, String op) {
continue;
}

// Check if this variable is captured by closures or is a state variable
if (sigilOp.id != 0 || op.equals("state")) {
// Variable is captured or is a state variable - use persistent storage
Integer beginId2 = RuntimeCode.evalBeginIds.get(sigilOp);
if (beginId2 != null || op.equals("state")) {
int persistId = beginId2 != null ? beginId2 : sigilOp.id;
int reg = allocateRegister();
int nameIdx = addToStringPool(varName);

Expand All @@ -2117,21 +2116,21 @@ void compileVariableDeclaration(OperatorNode node, String op) {
emitWithToken(Opcodes.RETRIEVE_BEGIN_SCALAR, node.getIndex());
emitReg(reg);
emit(nameIdx);
emit(sigilOp.id);
emit(persistId);
registerVariable(varName, reg);
}
case "@" -> {
emitWithToken(Opcodes.RETRIEVE_BEGIN_ARRAY, node.getIndex());
emitReg(reg);
emit(nameIdx);
emit(sigilOp.id);
emit(persistId);
registerVariable(varName, reg);
}
case "%" -> {
emitWithToken(Opcodes.RETRIEVE_BEGIN_HASH, node.getIndex());
emitReg(reg);
emit(nameIdx);
emit(sigilOp.id);
emit(persistId);
registerVariable(varName, reg);
}
default -> throwCompilerException("Unsupported variable type in list declaration: " + sigil);
Expand Down Expand Up @@ -4473,7 +4472,14 @@ public void visit(TryNode node) {

@Override
public void visit(LabelNode node) {
// Labels are tracked in loops, standalone labels are no-ops
int pc = bytecode.size();
gotoLabelPcs.put(node.label, pc);
for (Object[] pending : pendingGotos) {
if (node.label.equals(pending[1])) {
patchIntOffset((Integer) pending[0], pc);
}
}
pendingGotos.removeIf(p -> node.label.equals(p[1]));
lastResultReg = -1;
}

Expand Down
31 changes: 14 additions & 17 deletions src/main/java/org/perlonjava/backend/bytecode/CompileAssignment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import org.perlonjava.frontend.analysis.LValueVisitor;
import org.perlonjava.frontend.astnode.*;
import org.perlonjava.runtime.runtimetypes.NameNormalizer;
import org.perlonjava.runtime.runtimetypes.RuntimeCode;
import org.perlonjava.runtime.runtimetypes.RuntimeContextType;

import java.util.ArrayList;
Expand Down Expand Up @@ -51,10 +52,9 @@ public static void compileAssignmentOperator(BytecodeCompiler bytecodeCompiler,
if (sigilOp.operator.equals("$") && sigilOp.operand instanceof IdentifierNode) {
String varName = "$" + ((IdentifierNode) sigilOp.operand).name;

// Check if this variable is captured by named subs (Parser marks with id)
if (sigilOp.id != 0) {
// RETRIEVE the persistent variable (creates if doesn't exist)
int beginId = sigilOp.id;
Integer beginIdObj = RuntimeCode.evalBeginIds.get(sigilOp);
if (beginIdObj != null) {
int beginId = beginIdObj;
int nameIdx = bytecodeCompiler.addToStringPool(varName);
int reg = bytecodeCompiler.allocateRegister();

Expand Down Expand Up @@ -98,10 +98,9 @@ public static void compileAssignmentOperator(BytecodeCompiler bytecodeCompiler,
// Handle my @array = ...
String varName = "@" + ((IdentifierNode) sigilOp.operand).name;

// Check if this variable is captured by named subs
if (sigilOp.id != 0) {
// RETRIEVE the persistent array
int beginId = sigilOp.id;
Integer beginIdArr = RuntimeCode.evalBeginIds.get(sigilOp);
if (beginIdArr != null) {
int beginId = beginIdArr;
int nameIdx = bytecodeCompiler.addToStringPool(varName);
int arrayReg = bytecodeCompiler.allocateRegister();

Expand Down Expand Up @@ -168,10 +167,9 @@ public static void compileAssignmentOperator(BytecodeCompiler bytecodeCompiler,
// Handle my %hash = ...
String varName = "%" + ((IdentifierNode) sigilOp.operand).name;

// Check if this variable is captured by named subs
if (sigilOp.id != 0) {
// RETRIEVE the persistent hash
int beginId = sigilOp.id;
Integer beginIdHash = RuntimeCode.evalBeginIds.get(sigilOp);
if (beginIdHash != null) {
int beginId = beginIdHash;
int nameIdx = bytecodeCompiler.addToStringPool(varName);
int hashReg = bytecodeCompiler.allocateRegister();

Expand Down Expand Up @@ -261,10 +259,9 @@ public static void compileAssignmentOperator(BytecodeCompiler bytecodeCompiler,

int varReg;

// Check if this variable is captured by named subs (Parser marks with id)
if (sigilOp.id != 0) {
// This variable is captured - use RETRIEVE_BEGIN to get persistent storage
int beginId = sigilOp.id;
Integer beginIdList = RuntimeCode.evalBeginIds.get(sigilOp);
if (beginIdList != null) {
int beginId = beginIdList;
int nameIdx = bytecodeCompiler.addToStringPool(varName);
varReg = bytecodeCompiler.allocateRegister();

Expand Down Expand Up @@ -326,7 +323,7 @@ public static void compileAssignmentOperator(BytecodeCompiler bytecodeCompiler,

// Assign to variable
if (sigil.equals("$")) {
if (sigilOp.id != 0) {
if (beginIdList != null) {
bytecodeCompiler.emit(Opcodes.SET_SCALAR);
bytecodeCompiler.emitReg(varReg);
bytecodeCompiler.emitReg(elemReg);
Expand Down
3 changes: 3 additions & 0 deletions src/main/java/org/perlonjava/backend/bytecode/CompileBinaryOperator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -715,11 +715,14 @@ private static void compileJoinBinaryOp(BytecodeCompiler bytecodeCompiler, Binar

int listReg;
if (node.right instanceof ListNode listNode) {
int savedContext = bytecodeCompiler.currentCallContext;
bytecodeCompiler.currentCallContext = RuntimeContextType.LIST;
java.util.List<Integer> argRegs = new java.util.ArrayList<>();
for (Node arg : listNode.elements) {
arg.accept(bytecodeCompiler);
argRegs.add(bytecodeCompiler.lastResultReg);
}
bytecodeCompiler.currentCallContext = savedContext;
listReg = bytecodeCompiler.allocateRegister();
bytecodeCompiler.emit(Opcodes.CREATE_LIST);
bytecodeCompiler.emitReg(listReg);
Expand Down
17 changes: 10 additions & 7 deletions src/main/java/org/perlonjava/backend/bytecode/CompileOperator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2941,13 +2941,16 @@ public static void visitOperator(BytecodeCompiler bytecodeCompiler, OperatorNode
if (labelStr == null) {
bytecodeCompiler.throwCompilerException("goto must be given label");
}
int rd = bytecodeCompiler.allocateRegister();
bytecodeCompiler.emit(Opcodes.CREATE_GOTO);
bytecodeCompiler.emitReg(rd);
int labelIdx = bytecodeCompiler.addToStringPool(labelStr);
bytecodeCompiler.emitReg(labelIdx);
bytecodeCompiler.emit(Opcodes.RETURN);
bytecodeCompiler.emitReg(rd);
Integer targetPc = bytecodeCompiler.gotoLabelPcs.get(labelStr);
if (targetPc != null) {
bytecodeCompiler.emit(Opcodes.GOTO);
bytecodeCompiler.emitInt(targetPc);
} else {
bytecodeCompiler.emit(Opcodes.GOTO);
int patchPc = bytecodeCompiler.bytecode.size();
bytecodeCompiler.emitInt(0);
bytecodeCompiler.pendingGotos.add(new Object[]{patchPc, labelStr});
}
bytecodeCompiler.lastResultReg = -1;
} else {
bytecodeCompiler.throwCompilerException("Unsupported operator: " + op);
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/perlonjava/backend/jvm/EmitEval.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,7 @@ static void handleEvalOperator(EmitterVisitor emitterVisitor, OperatorNode node)

/**
* Emit bytecode for the interpreter path: compile to InterpretedCode and execute directly.
* This path is used when JPERL_EVAL_USE_INTERPRETER is set.
* This path is used by default (disable with JPERL_EVAL_NO_INTERPRETER=1).
*
* @param emitterVisitor The visitor that traverses the AST
* @param evalTag The unique identifier for this eval site
Expand Down
10 changes: 3 additions & 7 deletions src/main/java/org/perlonjava/backend/jvm/EmitVariable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1110,9 +1110,8 @@ static void handleMyOperator(EmitterVisitor emitterVisitor, OperatorNode node) {
String className = EmitterMethodCreator.getVariableClassName(sigil);

if (operator.equals("my")) {
// "my":
if (sigilNode.id == 0) {
// Create a new instance of the determined class
Integer beginId = RuntimeCode.evalBeginIds.get(sigilNode);
if (beginId == null) {
ctx.mv.visitTypeInsn(Opcodes.NEW, className);
ctx.mv.visitInsn(Opcodes.DUP);
ctx.mv.visitMethodInsn(
Expand All @@ -1122,9 +1121,6 @@ static void handleMyOperator(EmitterVisitor emitterVisitor, OperatorNode node) {
"()V",
false);
} else {
// The variable was initialized by a BEGIN block

// Determine the method to call and its descriptor based on the sigil
String methodName;
String methodDescriptor;
switch (var.charAt(0)) {
Expand All @@ -1145,7 +1141,7 @@ static void handleMyOperator(EmitterVisitor emitterVisitor, OperatorNode node) {
}

ctx.mv.visitLdcInsn(var);
ctx.mv.visitLdcInsn(sigilNode.id);
ctx.mv.visitLdcInsn(beginId);
ctx.mv.visitMethodInsn(
Opcodes.INVOKESTATIC,
"org/perlonjava/runtime/runtimetypes/PersistentVariable",
Expand Down
10 changes: 4 additions & 6 deletions src/main/java/org/perlonjava/frontend/parser/SpecialBlockParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,13 +142,11 @@ static RuntimeList runSpecialBlock(Parser parser, String blockPhase, Node block)
new OperatorNode("package",
new IdentifierNode(packageName, tokenIndex), tokenIndex));
} else {
// "my" or "state" variable live in a special BEGIN package
// Retrieve the variable id from the AST; create a new id if needed
OperatorNode ast = entry.ast();
if (ast.id == 0) {
ast.id = EmitterMethodCreator.classCounter++;
}
packageName = PersistentVariable.beginPackage(ast.id);
int beginId = RuntimeCode.evalBeginIds.computeIfAbsent(
ast,
k -> EmitterMethodCreator.classCounter++);
packageName = PersistentVariable.beginPackage(beginId);
// Emit: package BEGIN_PKG
nodes.add(
new OperatorNode("package",
Expand Down
11 changes: 4 additions & 7 deletions src/main/java/org/perlonjava/frontend/parser/SubroutineParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -700,16 +700,13 @@ public static ListNode handleNamedSubWithFilter(Parser parser, String subName, S
entry.name().substring(1),
entry.perlPackage());
} else {
// Handle "my" or "state" variables which live in a special BEGIN package
// Retrieve the variable id from the AST; create a new id if needed
OperatorNode ast = entry.ast();
if (ast.id == 0) {
ast.id = EmitterMethodCreator.classCounter++;
}
// Normalize variable name for 'my' or 'state' declarations
int beginId = RuntimeCode.evalBeginIds.computeIfAbsent(
ast,
k -> EmitterMethodCreator.classCounter++);
variableName = NameNormalizer.normalizeVariableName(
entry.name().substring(1),
PersistentVariable.beginPackage(ast.id));
PersistentVariable.beginPackage(beginId));
}
// Determine the class type based on the sigil
classList.add(
Expand Down
1 change: 1 addition & 0 deletions src/main/java/org/perlonjava/runtime/operators/ModuleOperators.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -577,6 +577,7 @@ else if (code == null) {
parsedArgs.fileName = actualFileName;
parsedArgs.incHook = incHookRef;
parsedArgs.applySourceFilters = shouldApplyFilters; // Enable source filter preprocessing if needed
parsedArgs.disassembleEnabled = RuntimeCode.DISASSEMBLE;
if (code == null) {
try {
code = FileUtils.readFileWithEncodingDetection(Paths.get(parsedArgs.fileName), parsedArgs);
Expand Down
5 changes: 3 additions & 2 deletions src/main/java/org/perlonjava/runtime/operators/UtimeOperator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import com.sun.jna.platform.win32.WinBase;
import com.sun.jna.platform.win32.WinNT;
import org.perlonjava.runtime.runtimetypes.RuntimeBase;
import org.perlonjava.runtime.runtimetypes.RuntimeIO;
import org.perlonjava.runtime.runtimetypes.RuntimeScalar;
import org.perlonjava.runtime.runtimetypes.RuntimeScalarType;

Expand Down Expand Up @@ -89,8 +90,8 @@ private static boolean changeFileTimes(RuntimeScalar fileArg, long accessTime, l
}

// Regular filename case
String filename = fileArg.toString();
if (filename.isEmpty()) {
String filename = RuntimeIO.sanitizePathname("utime", fileArg.toString());
if (filename == null || filename.isEmpty()) {
return false;
}

Expand Down
Loading