Skip to content

module load / compiler fixes - #490

Merged
jow- merged 14 commits into
ucode-lang:masterfrom
nbd168:compiler-fixes
Oct 16, 2026
Merged

module load / compiler fixes#490
jow- merged 14 commits into
ucode-lang:masterfrom
nbd168:compiler-fixes

Conversation

@nbd168

@nbd168 nbd168 commented Oct 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Commit f2d62f8 made every load of one path share one source object. This series removes that cache and fixes the defects that followed from it. It also fixes older defects in the lifetime of modules and their exports.

  • Sources: a source closes its file after compilation. Error messages and the debugger open the file again by its absolute path. Each loadfile() call compiles from its own source, so line numbers are correct on a second load. A file that cannot seek is read into a buffer.
  • Module load: a failed module is not stored and keeps its exception. A file loaded from a module body compiles as a program. Leaks in import(), require() and the rebinding of an import are fixed.
  • Module exports: the main closure of a program owns its exports, not the program. A program is thus freed with its last closure, and a repeated uc_vm_execute() or loadfile() call keeps its module state. The exports of a precompiled module are found by their count.

@jow-

jow- commented Oct 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Need to look into this in more detail but I am not entirely happy with the direction this goes into. We keep piling up complexity (introducing new caches, fstats, pid based comparisons, different load types, ...). My gut feeling is that we can vastly simplify all that if we stop retaining open fds on sources. Keeping a handle on running sources was an ad-hoc decision at some point to ensure that source text remains accessible for error reporting purposes in case the source file has been removed in the meanwhile, but this is just a cosmetic corner case.

Maybe we should simply attempt to reopen the source file by path when we want to extract source context and accept the possibility of a missing or mismatching file or a failing fopen.

@nbd168

nbd168 commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

If you want, I can rework this PR along that angle

@jow-

jow- commented Oct 13, 2026

Copy link
Copy Markdown
Collaborator

If you don't mind, please do - at least to gauge whether I missed some vital detail or whether it brings substantial simplifications after all.

@nbd168

nbd168 commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

Now that I think about it, this does add some overhead, because it means that the file has to be re-opened on every exception, even if handled. Do you think the memory overhead of caching the source text would be too big? Because that would allow us to sidestep the fd issue entirely.

@jow-

jow- commented Oct 13, 2026

Copy link
Copy Markdown
Collaborator

Yes, I believe the heap memory overhead would not be tolerable (think large, auto generated files etc.). Is the file reopen overhead that severe? In doubt I'd trade speed for memory.

@nbd168

nbd168 commented Oct 13, 2026

Copy link
Copy Markdown
Contributor Author

I guess people shouldn't use exceptions excessively anyway, so let's ignore the overhead. I'll push a reworked set soon.

jow-
jow- reviewed Oct 14, 2026
View reviewed changes
Comment thread include/ucode/types.h Resolved
jow-
jow- reviewed Oct 14, 2026
View reviewed changes
Comment thread lib.c Resolved
@jow-

jow- commented Oct 15, 2026

Copy link
Copy Markdown
Collaborator

To avoid stalling those PRs for too long I'll cherry-pick single non-controversial commits and rebase the remainder onto main, should make review and continuous adaptations easier.

@jow-

jow- commented Oct 16, 2026

Copy link
Copy Markdown
Collaborator

I was not entirely happy with the uc_source_t api surface, they way the internal properties ->fp and ->pos got accessed and the leaky abstractions, so I redesigned the API and rebased your work on top.

The result is in https://github.com/ucode-lang/ucode/tree/fix/pr490-module-and-compiler-fixes - if you're fine with it, I'll force push into this PR and merge from here.

@nbd168

nbd168 commented Oct 16, 2026

Copy link
Copy Markdown
Contributor Author

I'm fine with your branch, I just have a few small fixups - please check them and I can fold them in

jow- and others added 14 commits October 16, 2026 21:26
@jow-
source: rework the text access API
0dc042e 
The lexer, the error context formatter, the callee recovery, the debugger
and the loader of precompiled programs all reached into the source
object, using its file pointer and read offset directly.

Give the source an interface for reading its text instead:

 - uc_source_read_block() lends a block of text and its offset to the
   caller, which lets the lexer drop its own read buffer and keep its
   own byte counter.
 - uc_source_read_line() returns the line holding a given offset and
   uc_source_callee_expr() reads the bytes in front of a call into a
   window and scans that in memory. Both leave the read position as they
   found it.
 - uc_source_read() with uc_source_offset() and uc_source_remaining()
   serve the loader of precompiled programs, which no longer needs a
   FILE pointer.
 - uc_source_read_content() returns the whole text for the debugger.
 - uc_source_type_test() is renamed to uc_source_begin(), as it
   positions the source on its first byte of code.

The callee recovery no longer keeps its state in static variables. It reads
a window of the source in front of a call into a heap buffer, which starts
at 256 bytes and doubles up to 4096 bytes while the scan runs into its
start, finds the start of the invoked expression, then collapses and
copies the expression in place, and returns the buffer to the caller,
which releases it using free(). The scan now also follows an expression
which is broken across lines in front of a subscript or nested call.

Signed-off-by: Jo-Philipp Wich <jo@mein.io>
@nbd168 @jow-
source: close the file of a source after its compilation
1dd1f88 
A source keeps its file open until the source is released, and a
program holds its sources for as long as it lives. Each live program
thus holds one open file per source file, which is what runs an
application with many loaded files into the limit on open files. After
the compilation, the file only serves the text for error messages.

Open the file when its text is first read, and release it with
uc_source_reset() when the compilation or the load of a precompiled
program ends, or when the reading of the text reaches its end. Read the
text for the error context, for the name of a failed call and for the
debugger from the path of the source. Keep the absolute path for that,
since a script can change its working directory. That file can be gone
or changed by then; the message then has no source context or shows the
text of the current file. A buffer source keeps its text in memory as
before.

Do not open the source files that a precompiled program names when it
is loaded. The warning about a file that does not exist goes away with
that.

Signed-off-by: Felix Fietkau <nbd@nbd.name>
[adapted to the reworked source API: implicit open, uc_source_reset()]
Signed-off-by: Jo-Philipp Wich <jo@mein.io>
@nbd168 @jow-
lib: remove the source cache of loadfile()
69db946 
Since commit f2d62f8 ("lib: deduplicate source files by resolved
path in vm->sources cache"), uc_loadfile() and thus include() hand the
same source object to uc_compile() on each call for one file. A
compilation records its read position, its line information and its
export table in the source, and the root source also holds the export
count of the program. The next compilation of that source continues
from this state, so the export slots of its imports lie behind the
export table of the new program. A named import then yields null and a
wildcard import leaves its name operands unread, which the VM decodes
as instructions:

  Runtime error: unknown opcode 0

A file without imports is affected as well: the error locations of a
later compilation are wrong, and the cache keeps each source until the
VM is freed.

The cache was added to limit the number of open files. A source no
longer holds its file open after the compilation, so the cache has no
purpose. Revert that commit: each call compiles a source of its own
again, and each program keeps the line information and the export table
of its own compilation. Keep the field of the VM as a reserved pointer,
since nothing uses it.

Fixes: f2d62f8 ("lib: deduplicate source files by resolved path in vm->sources cache")
Signed-off-by: Felix Fietkau <nbd@nbd.name>
@nbd168 @jow-
compiler: start each compilation from a clean source
4436a93 
A compilation records its read position, its line information and its
export table in the source. uc_compile() does not reset that state, so
a second uc_compile() call for one source object continues from it. The
error locations of the new program are wrong, and the export slots of
its imports lie behind the export table of the new program.
uc_loadfile() no longer passes a source twice, but uc_compile() and the
source objects are public API and an embedder can do so.

Drop the state of the previous compilation in uc_source_begin(), which
every compilation of a source starts with. The sources of statically
imported modules are created per program and need no reset.

The source then describes its latest compilation only. A program of an
earlier compilation that is still in use needs a source of its own.

Signed-off-by: Felix Fietkau <nbd@nbd.name>
[adapted to the reworked source API: state is cleared in uc_source_begin()]
Signed-off-by: Jo-Philipp Wich <jo@mein.io>
@nbd168 @jow-
source: read a file that cannot seek into a buffer
4503599 
uc_source_begin() reads the first bytes of a source to look for an
interpreter line and for the bytecode magic, and seeks back afterwards.
The seek fails on a pipe, so the compiler never sees these bytes. A
script that is passed as /dev/stdin from a pipe, or loaded from there
with loadfile(), fails:

  Failed to rewind source buffer: Illegal seek
  Syntax error: Unexpected token

The content of a pipe is also gone when an error message needs the
source text afterwards.

Read such a file into a buffer in uc_source_new_file() and create a
buffer source from it, like main.c already does for the "-" argument.
Move that code to the new uc_source_new_slurp(), which both use.

While here, terminate the buffer of uc_source_new_slurp(), since
uc_program_write() stores it as a string, and let uc_load_common()
name the file in a compile error of a file that was read into a buffer.

Signed-off-by: Felix Fietkau <nbd@nbd.name>
[adapted to the reworked source API: uc_source_new_stream() is uc_source_new_slurp()]
Signed-off-by: Jo-Philipp Wich <jo@mein.io>
@nbd168 @jow-
compiler: release the name list of an import() statement
330cdab 
uc_compiler_compile_import() allocates the list of imported names
before it checks for the call form. A statement that consists of an
import() call returns early and leaks that list on each compilation.

Release the list on that path.

Fixes: 926f20b ("compiler: add support for runtime module imports")
Signed-off-by: Felix Fietkau <nbd@nbd.name>
@nbd168 @jow-
lib: do not store a failed module in the module table
153c627 
uc_require_path() stores the result of a load in the module table as
soon as the file exists, also if the module raised an exception. The
next import() or require() of that name then returns the incomplete
result of the failed load, for example an empty namespace, and raises
no error.

Store the module only if the load raised no exception, so that a later
load runs the module again.

Signed-off-by: Felix Fietkau <nbd@nbd.name>
@nbd168 @jow-
vm: keep the exception of a failed dynamic module load
3bf6049 
uc_require_library() returns the incomplete namespace of a module whose
body raised an exception. uc_vm_insn_dynload() tests the return value
only and goes on to bind the imported names. A name that the module
did not export before it failed replaces the exception of the module:

  Reference error: Module does not export b

Return if the load raised an exception.

Signed-off-by: Felix Fietkau <nbd@nbd.name>
@nbd168 @jow-
lib: compile a file loaded from a module body as a program
71b4a14 
uc_require_ucode() points vm->config at a local configuration with
compile_module set, and keeps it there while the body of the module
runs. uc_load_common() copies that configuration, so a loadfile() or
loadstring() call in the body of a dynamically loaded module compiles
its argument as a module. A script that returns a value then fails:

  Syntax error: return must be inside function body

Reset compile_module once the module itself is compiled.

Signed-off-by: Felix Fietkau <nbd@nbd.name>
@nbd168 @jow-
lib: release the main function of a required file
080cbd7 
uc_require_ucode() pushes the closure that uc_loadfile() returns on the
stack, runs it, and pops it again without a release. Each include(),
render(), require() and import() thus leaks one closure, which keeps
the program of the file, until a collection or the end of the VM.

Release the closure after the run.

Signed-off-by: Felix Fietkau <nbd@nbd.name>
@nbd168 @jow-
vm: release the previous binding of an import
6c30ef7 
uc_vm_insn_import() and uc_vm_insn_dynload() store the binding of an
imported name in an upvalue slot of the running closure, and overwrite
what the slot holds. A script can call the function that loadfile()
returns more than once. Each call after the first one runs the import
instructions of the same closure again and leaks the upvalue reference
of the call before it, for a dynamically loaded module together with
the reference to the imported value.

Release the binding that a slot holds before it gets a new one.

Signed-off-by: Felix Fietkau <nbd@nbd.name>
@nbd168 @jow-
vm: let the main closure of a program own its module exports
6e404de 
Since commit d64d5d6 ("vm: maintain export symbol tables per
program"), uc_vm_insn_export() stores the exports of statically linked
modules in the program and keeps them until the program is freed. This
has several defects:

 - An exported function is a closure that holds the program, so the
   table keeps the program alive. Without the collector, each
   loadfile() or include() of a file that imports such a module keeps
   its memory.
 - The collector does not treat the table as a root. A collection
   between two uc_vm_execute() calls of one program frees the exports,
   and the second call fails:

     Type error: `inc` is not a function, got null instead

 - A program that runs in a second VM binds the values of the first
   VM, also after that VM is freed.
 - Each run of a program appends its exports to the table again.
 - A call that starts while another call of the same main function is
   active shifts the entries, so an import that the outer call resolves
   afterwards binds an unrelated export.

Keep the exports per closure of the entry function of a program,
release them when that closure is freed, and mark them in the collector
from that closure. A program that is loaded and run once is thus freed
with its last closure. A function from loadfile() keeps its module
instances for as long as the script holds it, so module state still
carries over from one call to the next. A later or nested run of a
closure runs the module bodies again, as before, but adds no entries:
its imports resolve to the instances of the first run.

For uc_vm_execute(), keep the closure that runs in the registry of the
VM once it has module instances. A later execution of the program in
that VM runs the same closure. Only the caller can tell that the
program is still in use, and its reference is the one that no closure
accounts for. Count the closures of a program and drop the registry
entry in the collector once no other reference exists.

Read the exports of a dynamically loaded module from its closure in
uc_require_imports().

While here, read all name operands of a wildcard import if an export is
missing.

Signed-off-by: Felix Fietkau <nbd@nbd.name>
@jow-
vm: do not interrupt the exports of a module
0adb7ba 
The VM runs the handlers of pending signals after each instruction, and
uc_vm_insn_export() adds one export to the module instance of the
running closure. A signal handler that raises an exception between two
export instructions of one module leaves a part of that module instance
for the next run of the closure.

The compiler emits the exports of a module as one block. Do not run the
signal handlers between two export instructions, so that no handler runs
inside that block.

Signed-off-by: Felix Fietkau <nbd@nbd.name>
[reworked: the main loop skips the signal dispatch between exports]
Signed-off-by: Jo-Philipp Wich <jo@mein.io>
@nbd168 @jow-
lib: find the exports of a precompiled module by their count
8cfcffe 
uc_require_imports() takes the position of the own exports of a module
from the export offset of its source. uc_program_write() does not store
that offset, so it is zero for a precompiled module. If such a module
links another module statically, its namespace object holds the exports
of the linked module under the names of its own exports.

A module exports its own symbols last, after those of the modules that
it links. Take the position from the number of own exports, which the
source and the bytecode both record.

Signed-off-by: Felix Fietkau <nbd@nbd.name>
@jow-
jow- merged commit 077b830 into ucode-lang:master Oct 16, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jow- jow- jow- left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@nbd168 @jow-