Skip to content

Commit 968b3c4

Browse files
authored
Merge pull request #551 from fglock/feature/dbi-phase10-profile
fix(runtime): package Foo; lexical scoping + DBI parity Phase 10
2 parents 720a04d + 93797b7 commit 968b3c4

9 files changed

Lines changed: 602 additions & 117 deletions

File tree

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
#!/usr/bin/env perl
2+
# Minimal reproduction of PerlOnJava bug:
3+
# `local (HASH_OR_ARRAY_ELEMENT) = value;` inside eval-STRING-compiled
4+
# subs is a no-op for the value assignment (scope restoration still works).
5+
#
6+
# See dev/modules/dbi_test_parity.md "Root cause of t/06attrs.t and
7+
# t/08keeperr.t failures" for context. Blocks proper DBI::PurePerl
8+
# error-message formatting.
9+
#
10+
# Run with both:
11+
# ./jperl dev/known-bugs/local_list_assign_eval_string.pl
12+
# perl dev/known-bugs/local_list_assign_eval_string.pl
13+
# and compare outputs.
14+
15+
use strict;
16+
use warnings;
17+
18+
my $h = { x => 0 };
19+
my @a = (0);
20+
21+
# Case A: direct file-scope compile — works on both
22+
sub directA { local ($h->{x}) = 42; print "A: h->{x}=$h->{x}\n"; }
23+
directA();
24+
print "A: after: h->{x}=$h->{x}\n";
25+
26+
# Case B: eval-STRING compiled sub, hash-element, list form — BUG on jperl
27+
my $subB = eval q{ sub { local ($h->{x}) = 99; print "B: h->{x}=$h->{x}\n"; } };
28+
die $@ if $@;
29+
$subB->();
30+
31+
# Case C: eval-STRING compiled sub, hash-element, SCALAR form — works
32+
my $subC = eval q{ sub { local $h->{x} = 77; print "C: h->{x}=$h->{x}\n"; } };
33+
die $@ if $@;
34+
$subC->();
35+
36+
# Case D: eval-STRING compiled sub, array-element, list form — BUG on jperl
37+
my $subD = eval q{ sub { local ($a[0]) = 88; print "D: a[0]=$a[0]\n"; } };
38+
die $@ if $@;
39+
$subD->();
40+
41+
print "\nExpected (real perl):\n";
42+
print "A: h->{x}=42\nA: after: h->{x}=0\nB: h->{x}=99\nC: h->{x}=77\nD: a[0]=88\n";

dev/modules/dbi_test_parity.md

Lines changed: 375 additions & 107 deletions
Large diffs are not rendered by default.

src/main/java/org/perlonjava/backend/bytecode/CompileAssignment.java

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,24 @@ private static boolean handleLocalListAssignment(BytecodeCompiler bc, BinaryOper
261261
bc.lastResultReg = localReg;
262262
return true;
263263
}
264+
// Single-element list with an lvalue like $h->{k}, $a[i], $obj->method->{k}, etc.
265+
// Delegate to the scalar-local handler (matches the `local EXPR = RHS` path at
266+
// line 20). Without this, the element falls through the main loop below and
267+
// emits nothing - a silent no-op assignment. Reproduced by:
268+
// local ($h->{x}) = 99; inside an eval-STRING-compiled sub
269+
if (element instanceof BinaryOperatorNode binOp) {
270+
bc.compileNode(binOp, -1, rhsContext);
271+
int elemReg = bc.lastResultReg;
272+
bc.emit(Opcodes.PUSH_LOCAL_VARIABLE);
273+
bc.emitReg(elemReg);
274+
bc.compileNode(node.right, -1, rhsContext);
275+
int valueReg = bc.lastResultReg;
276+
bc.emit(Opcodes.SET_SCALAR);
277+
bc.emitReg(elemReg);
278+
bc.emitReg(valueReg);
279+
bc.lastResultReg = elemReg;
280+
return true;
281+
}
264282
}
265283
bc.compileNode(node.right, -1, rhsContext);
266284
int valueReg = bc.lastResultReg;
@@ -292,6 +310,26 @@ private static boolean handleLocalListAssignment(BytecodeCompiler bc, BinaryOper
292310
bc.emitReg(localReg);
293311
bc.emitReg(elemReg);
294312
if (i == 0) bc.lastResultReg = localReg;
313+
} else if (element instanceof BinaryOperatorNode binOp) {
314+
// Element is an lvalue expression (e.g. $h->{k}, $a[i], $obj->attr).
315+
// Compile to get the element reference, localize it, and assign RHS[i].
316+
bc.compileNode(binOp, -1, RuntimeContextType.SCALAR);
317+
int elemLvalReg = bc.lastResultReg;
318+
bc.emit(Opcodes.PUSH_LOCAL_VARIABLE);
319+
bc.emitReg(elemLvalReg);
320+
int idxReg = bc.allocateRegister();
321+
bc.emit(Opcodes.LOAD_INT);
322+
bc.emitReg(idxReg);
323+
bc.emit(i);
324+
int rhsElemReg = bc.allocateRegister();
325+
bc.emit(Opcodes.ARRAY_GET);
326+
bc.emitReg(rhsElemReg);
327+
bc.emitReg(valueReg);
328+
bc.emitReg(idxReg);
329+
bc.emit(Opcodes.SET_SCALAR);
330+
bc.emitReg(elemLvalReg);
331+
bc.emitReg(rhsElemReg);
332+
if (i == 0) bc.lastResultReg = elemLvalReg;
295333
}
296334
}
297335
return true;

src/main/java/org/perlonjava/backend/bytecode/CompileOperator.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -828,9 +828,12 @@ public static void visitOperator(BytecodeCompiler bytecodeCompiler, OperatorNode
828828
}
829829
bytecodeCompiler.symbolTable.setCurrentPackage(packageName, isClass);
830830
if (isClass) ClassRegistry.registerClass(packageName);
831-
boolean isScoped = Boolean.TRUE.equals(node.getAnnotation("isScoped"));
831+
// Always emit PUSH_PACKAGE so the runtime tracker is restored when
832+
// the enclosing block/sub/file exits. Perl 5's `package Foo;` is
833+
// lexically scoped; the `isScoped` annotation used to distinguish
834+
// `package Foo { BLOCK }` but bare `package Foo;` is equally scoped.
832835
int nameIdx = bytecodeCompiler.addToStringPool(packageName);
833-
bytecodeCompiler.emit(isScoped ? Opcodes.PUSH_PACKAGE : Opcodes.SET_PACKAGE);
836+
bytecodeCompiler.emit(Opcodes.PUSH_PACKAGE);
834837
bytecodeCompiler.emit(nameIdx);
835838
bytecodeCompiler.lastResultReg = -1;
836839
} else {

src/main/java/org/perlonjava/backend/bytecode/InterpreterState.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,23 @@ public class InterpreterState {
5454
public static void setCurrentPackageStatic(String name) {
5555
currentPackage.get().set(name);
5656
}
57+
58+
/**
59+
* Scoped variant of {@link #setCurrentPackageStatic}: pushes the current
60+
* package value onto the DynamicVariableManager stack so it will be
61+
* restored when the enclosing scope exits, then sets the new value.
62+
* <p>
63+
* Matches Perl 5 semantics: {@code package Foo;} is lexically scoped to
64+
* the enclosing block / eval / file. Without the push, a {@code package Foo;}
65+
* inside e.g. {@code Carp::caller_info}'s {@code { package DB; ... }} block
66+
* would leak "DB" past the block, corrupting subsequent {@code do FILE}
67+
* calls (which inherit the caller's package).
68+
*/
69+
public static void setCurrentPackageLocal(String name) {
70+
RuntimeScalar pkg = currentPackage.get();
71+
org.perlonjava.runtime.runtimetypes.DynamicVariableManager.pushLocalVariable(pkg);
72+
pkg.set(name);
73+
}
5774
private static final ThreadLocal<Deque<InterpreterFrame>> frameStack =
5875
ThreadLocal.withInitial(ArrayDeque::new);
5976
// Use ArrayList of mutable int holders for O(1) PC updates (no pop/push overhead)

src/main/java/org/perlonjava/backend/jvm/EmitOperator.java

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1123,13 +1123,18 @@ static void handlePackageOperator(EmitterVisitor emitterVisitor, OperatorNode no
11231123
// `require FILE` (which inspects InterpreterState.currentPackage to
11241124
// compile the required file in the correct namespace) see the right
11251125
// package after a `package Foo;` declaration in JVM-compiled code.
1126-
// Without this, the runtime tracker stays at "main" in compiled code,
1127-
// and `require FILE` incorrectly installs subs in main::.
1126+
//
1127+
// Use the *scoped* (local) variant so the runtime tracker is restored
1128+
// when the enclosing block / sub / file exits. Perl 5's `package Foo;`
1129+
// is lexically scoped; without the restore, a `package DB;` inside
1130+
// e.g. Carp::caller_info's inner `{ package DB; ... }` block would
1131+
// leak past the block and break subsequent `do FILE` calls which
1132+
// compile the loaded file in the *current* runtime package.
11281133
emitterVisitor.ctx.mv.visitLdcInsn(name);
11291134
emitterVisitor.ctx.mv.visitMethodInsn(
11301135
org.objectweb.asm.Opcodes.INVOKESTATIC,
11311136
"org/perlonjava/backend/bytecode/InterpreterState",
1132-
"setCurrentPackageStatic",
1137+
"setCurrentPackageLocal",
11331138
"(Ljava/lang/String;)V",
11341139
false);
11351140
// Set debug information for the file name.

src/main/java/org/perlonjava/core/Configuration.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public final class Configuration {
3333
* Automatically populated by Gradle/Maven during build.
3434
* DO NOT EDIT MANUALLY - this value is replaced at build time.
3535
*/
36-
public static final String gitCommitId = "1cdf0926f";
36+
public static final String gitCommitId = "7a0687aef";
3737

3838
/**
3939
* Git commit date of the build (ISO format: YYYY-MM-DD).
@@ -48,7 +48,7 @@ public final class Configuration {
4848
* Parsed by App::perlbrew and other tools via: perl -V | grep "Compiled at"
4949
* DO NOT EDIT MANUALLY - this value is replaced at build time.
5050
*/
51-
public static final String buildTimestamp = "Apr 23 2026 13:55:28";
51+
public static final String buildTimestamp = "Apr 23 2026 18:51:50";
5252

5353
// Prevent instantiation
5454
private Configuration() {

src/main/java/org/perlonjava/runtime/perlmodule/XSLoader.java

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,30 @@ public class XSLoader extends PerlModuleBase {
3333
"XSLoader"
3434
);
3535

36+
/**
37+
* Modules that are pure-XS in real Perl with no PerlOnJava-side implementation.
38+
* When XSLoader::load is called for one of these, we die cleanly so that
39+
* {@code eval { require SomeModule }} in CPAN code (and test suites that
40+
* probe for optional backends like DBM engines) correctly falls through
41+
* to alternatives instead of silently "succeeding" and then crashing
42+
* later when methods are actually called.
43+
* <p>
44+
* Rule of thumb: the module's whole functionality lives in a shared
45+
* library shipped with CPAN, and there is no pure-Perl or Java-backed
46+
* replacement in PerlOnJava. Pre-registered Java modules (File::Glob,
47+
* Encode, Time::HiRes, etc.) must NOT appear here.
48+
* <p>
49+
* Kept in sync with the Perl-side copy in {@code lib/XSLoader.pm}.
50+
*/
51+
private static final Set<String> XS_ONLY_NOT_SUPPORTED = Set.of(
52+
"DB_File",
53+
"BerkeleyDB",
54+
"GDBM_File",
55+
"NDBM_File",
56+
"ODBM_File",
57+
"SDBM_File"
58+
);
59+
3660
/**
3761
* Constructor for XSLoader.
3862
* Initializes the module with the name "XSLoader".
@@ -57,6 +81,41 @@ public static void initialize() {
5781
}
5882
}
5983

84+
/**
85+
* Installs no-op Perl subroutines for XS symbols that a failed-to-load
86+
* module's END block is known to call. Without these, the END queue
87+
* aborts on interpreter shutdown with a non-zero exit status, which
88+
* prove/TAP::Harness counts as a failed test program even when the
89+
* program's actual assertions all passed or were SKIPped.
90+
*
91+
* Keyed by the module name passed to {@code XSLoader::load}.
92+
*/
93+
private static void installEndBlockStubs(String moduleName) {
94+
String[] symbols = switch (moduleName) {
95+
case "BerkeleyDB" -> new String[] { "BerkeleyDB::Term::close_everything" };
96+
default -> null;
97+
};
98+
if (symbols == null) return;
99+
try {
100+
java.lang.invoke.MethodHandle mh = RuntimeCode.lookup.findStatic(
101+
XSLoader.class, "noopStub", RuntimeCode.methodType);
102+
for (String sym : symbols) {
103+
if (GlobalVariable.existsGlobalCodeRef(sym)) continue;
104+
RuntimeCode code = new RuntimeCode(mh, null, null);
105+
code.isStatic = true;
106+
GlobalVariable.getGlobalCodeRef(sym).set(new RuntimeScalar(code));
107+
}
108+
} catch (Exception e) {
109+
// Non-fatal: the test program may still report a spurious non-zero
110+
// exit, but the module-load failure path is unaffected.
111+
}
112+
}
113+
114+
/** No-op Perl sub used by {@link #installEndBlockStubs}. */
115+
public static RuntimeList noopStub(RuntimeArray args, int ctx) {
116+
return new RuntimeList();
117+
}
118+
60119
/**
61120
* Loads a PerlOnJava module.
62121
* <p>
@@ -90,6 +149,29 @@ public static RuntimeList load(RuntimeArray args, int ctx) {
90149
moduleName = args.getFirst().toString();
91150
}
92151

152+
// Bail out cleanly for pure-XS modules PerlOnJava can't back.
153+
// Without this, modules like DB_File load but their XS helpers
154+
// (constant, etc.) are undefined, leading to infinite AUTOLOAD
155+
// recursion (StackOverflowError) the first time the module is
156+
// actually used. CPAN test suites commonly probe optional backends
157+
// with `eval { require SomeDBM }` and rely on require FAILING to
158+
// fall through to alternatives; silent success breaks them.
159+
if (XS_ONLY_NOT_SUPPORTED.contains(moduleName)) {
160+
// Install no-op stubs for any functions the module registers in an
161+
// END block — the `.pm` file was already compiled end-to-end before
162+
// we reach this `load`, so its END queue entries will fire at
163+
// interpreter shutdown regardless of whether `require` succeeds.
164+
// Without these, CPAN prove-style runners report the (otherwise-
165+
// passing / SKIPped) test program as "exited 1" from the END die.
166+
installEndBlockStubs(moduleName);
167+
168+
return WarnDie.die(
169+
new RuntimeScalar("Can't load '" + moduleName + "' for module " + moduleName
170+
+ ": XS module not supported on PerlOnJava"),
171+
new RuntimeScalar("\n")
172+
).getList();
173+
}
174+
93175
// Convert Perl::Module::Name to org.perlonjava.runtime.perlmodule.PerlModuleName
94176
String[] parts = moduleName.split("::");
95177
StringBuilder className1 = new StringBuilder("org.perlonjava.runtime.perlmodule.");

src/main/perl/lib/XSLoader.pm

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,26 +15,56 @@ package XSLoader;
1515

1616
our $VERSION = "0.32";
1717

18+
# Modules that are pure-XS in real Perl with no PerlOnJava-side implementation.
19+
# When XSLoader::load is called for one of these, we die cleanly so that
20+
# `eval { require SomeModule }` in CPAN code (and test suites that probe for
21+
# optional backends like DBM engines) correctly falls through to alternatives
22+
# instead of silently "succeeding" and then crashing later when methods are
23+
# actually called.
24+
#
25+
# Rule of thumb for adding to this list: the module's whole functionality
26+
# lives in a `.so`/DLL shipped with CPAN, and there is no pure-Perl or
27+
# Java-backed replacement in PerlOnJava. Pre-registered Java modules
28+
# (File::Glob, Encode, Time::HiRes, etc.) must NOT appear here.
29+
our %XS_ONLY_NOT_SUPPORTED = map { $_ => 1 } qw(
30+
DB_File
31+
BerkeleyDB
32+
GDBM_File
33+
NDBM_File
34+
ODBM_File
35+
SDBM_File
36+
);
37+
1838
# Only define our load() if it's not already defined by Java
1939
BEGIN {
2040
unless (defined &load) {
2141
*load = sub {
2242
my ($module, $version) = @_;
2343
$module = caller() unless defined $module;
24-
44+
45+
# Bail out cleanly for pure-XS modules PerlOnJava can't back.
46+
# Without this, modules like DB_File load but XS functions such
47+
# as `constant` are undefined, which triggers infinite AUTOLOAD
48+
# recursion (StackOverflowError) the first time the module is
49+
# actually used.
50+
if ($XS_ONLY_NOT_SUPPORTED{$module}) {
51+
die "Can't load '$module' for module $module: "
52+
. "XS module not supported on PerlOnJava\n";
53+
}
54+
2555
# Check if the module has a bootstrap function (like standard XSLoader)
2656
my $boots = "${module}::bootstrap";
2757
if (defined &{$boots}) {
2858
goto &{$boots};
2959
}
30-
60+
3161
# For Java-backed modules, the methods are already registered.
3262
# For pure-Perl modules, nothing needs to be done.
3363
# Either way, just return success.
3464
return 1;
3565
};
3666
}
37-
67+
3868
# Alias for compatibility
3969
*bootstrap_inherit = \&load unless defined &bootstrap_inherit;
4070
}

0 commit comments

Comments
 (0)