Fix ExifTool crashes and improve write support#257
Merged
Conversation
…dling - Fix undef $coderef: ref() now returns "" and boolean is false for undefined CODE scalars, matching Perl 5 behavior. undef &foo still correctly reports "Undefined subroutine" on call. - Add Compress::Zlib module using java.util.zip for inflate/deflate, enabling ExifTool's PNG, PDF, and other compressed format support. - Fix Encode::is_utf8/utf8::is_utf8 to only return true for strings with actual wide characters (>255), preventing binary data corruption in ExifTool's ParseArguments. - Fix slurp-mode readline when $/ is undef (local $/), preserving BYTE_STRING type for binary data. - Fix named sub definition return values: skip compile-time-only nodes when computing block return value, fixing modules without explicit 1; before __END__. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <noreply@cognition.ai>
\&foo where foo was only forward-declared (sub foo;) was returning undef instead of a CODE reference. This broke ExifTool's AUTOLOAD mechanism for lazily loading write modules (WriteExif.pl, etc), causing all TIFF/EXIF write operations to fail. The fix adds an IdentifierNode case in EmitOperator's backslash handler that creates a RuntimeScalar with the sub name and calls createCodeReference, matching how Perl 5 handles \& on stash entries. Also make Utf8.java is_utf8 consistent (true for all non-BYTE_STRING) and fix perl_test_runner.pl to not chdir for ExifTool tests. ExifTool test improvements vs baseline: GIF: 2→5, IPTC: 7→8, GeoTiff: 3→4, PLUS: 2→3, PPM: 2→3, Writer: 30→33, CanonVRD: 13→21, XMP: 40→41, PNG: 4+crash→5 Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <noreply@cognition.ai>
The previous change to skip empty ListNodes when finding the last
expression in a block caused sub foo($a) { } to return the argument
value instead of undef. The \&foo code reference fix was the actual
solution for the ExifTool named sub return issue.
Fixes 13 regressions in op/signatures.t (589→602).
Generated with [Devin](https://cli.devin.ai/docs)
Co-Authored-By: Devin <noreply@cognition.ai>
…an context In Perl 5, all code references are truthy regardless of whether the sub body is defined. Changed getBooleanLarge() CODE case to return true (zero memory cost). This fixes can() returning falsy for forward-declared subs (uni/universal.t test 16) while preserving identity comparison (mro/basic.t test 56). Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <noreply@cognition.ai>
fglock
force-pushed
the
fix-exiftool-crashes
branch
from
948abc9 to
52f7409
Compare
Previously alarm only worked in the JVM codegen path. Add ALARM_OP (opcode 155) with compilation, interpretation, and disassembly support so alarm works correctly in eval STRING contexts. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <noreply@cognition.ai>
…stem alarm, stale flag cleanup - select(undef,undef,undef,timeout): dispatch pending signals on InterruptedException, clear stale interrupt before sleeping (fixes sigdispatch.t tests 22-23) - kill to self-PID: route through PerlSignalQueue instead of sending real OS signals that crash the JVM (fixes sigdispatch.t test 28) - PerlSignalQueue: clear thread interrupt flag and update hasPendingSignal before handler invocation so die in handlers does not leave stale state - system(): dispatch pending signals on InterruptedException instead of throwing PerlCompilerException (fixes alarm.t tests 3-4) sigdispatch.t: 23/29 -> 26/29, alarm.t: 3/5 -> 5/5 Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <noreply@cognition.ai>
fglock
force-pushed
the
fix-exiftool-crashes
branch
from
7ac5330 to
94389eb
Compare
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes multiple issues discovered while running ExifTool's test suite, with the most significant being a fundamental bug in
\&foocode reference handling for forward-declared subs.Key fixes:
\&foofor forward-declared subs returned undef instead of a CODE reference. This broke ExifTool's AUTOLOAD mechanism for lazily loading writer modules (WriteExif.pl, WritePNG.pl, etc.), causing all TIFF/EXIF write operations to fail silently. Fixed by addingIdentifierNodehandling inEmitOperator's backslash handler to callRuntimeCode.createCodeReference().undef $coderefdidn't clear CODE type -ref(),getBoolean(), andtoString()now check if a CODE value is actually defined, fixing ExifTool's PNG ProcessDirectory crash (test 7).Binary data (BYTE_STRING) corruption in concat/substr/read - STRING type was "sticky" in concatenation, causing binary data round-trips to lose BYTE_STRING type. Fixed type propagation in
StringOperators,RuntimeSubstrLvalue,Readline, andCustomFileChannel.Encode::is_utf8()returning inverted results and inconsistent behavior betweenEncode::is_utf8andutf8::is_utf8.Named sub return value fix -
EmitBlockwas incorrectly usingRETURN_VALUEopcode (pops stack) instead of storing the last expression value.AUTOLOAD not triggering for forward-declared subs in bytecode interpreter.
New
Compress::Zlibmodule stub for ExifTool compatibility.Renamed MOVE opcode to ALIAS to fix my/our variable aliasing semantics.
ExifTool test results (baseline -> now):
Test plan
makepasses (build + unit tests)make test-unitpasses (all shards)Generated with Devin