Skip to content

Fix isla-sail/isla-lib against current Sail 0.20.2 + current sail-riscv - #105

Closed
benlaurie wants to merge 2 commits into
rems-project:masterfrom
benlaurie:fix/sail-file-api-compat
Closed

benlaurie wants to merge 2 commits into
rems-project:masterfrom
benlaurie:fix/sail-file-api-compat

Conversation

@benlaurie

Copy link
Copy Markdown

Two small, independent compatibility fixes found while getting isla-sail
and isla-footprint working against a current Sail release (0.20.2) and
a recent riscv/sail-riscv commit.

isla-sail against current Sail 0.20.2 API (342231a)

  • jib_ir.ml: Sail_file.to_path/Path.to_string no longer exist.
    The values being passed through them were always plain filename
    strings (collected from Lexing.position.pos_fname), never real
    Sail_file handles -- printed directly instead.
  • sail_plugin_isla.ml: Preprocess.add_default_symbol was removed;
    add_symbol is now the only registration mechanism, with the same
    effect at this call site (still runs before any user-provided
    symbols are processed).

Verified by building the plugin and running it against a real Sail
RISC-V model (riscv/sail-riscv): isla-sail --all-modules riscv.sail_project -o out now produces a complete .ir file.

i64_to_i128 accepting an already-widened I128 operand (70d2085)

i64_to_i128 (backing Sail's %i64->%i conversion) only handled
Val::I64 and Val::Symbolic inputs. A real, reachable path in
current sail-riscv -- exponentiation with a dependently-typed exponent,
as in 2 ^ physaddr_bits -- produces an already-Val::I128 value on a
path statically typed %i64 before it reaches this conversion. Since
I128 is what this function widens to, an already-I128 input just
needs to pass through unchanged.

Verified against a snapshot built from riscv/sail-riscv HEAD:
isla-footprint now correctly derives instruction semantics as symbolic
SMT-LIB (confirmed with ADD) from that model, where it previously
failed with a type error before any instruction-level execution began.

isla-sail hadn't been updated for two Sail API changes since this commit
was last tested against a current Sail release:

- Sail_file.to_path / Sail_file.Path.to_string don't exist. The value
  being passed through them (file_map's entries) was never a Sail_file
  handle to begin with -- it's collected from Lexing.position.pos_fname
  in file_number above, which is a plain string. Print it directly.
- Preprocess.add_default_symbol was removed; add_symbol is now the only
  registration mechanism. Since this call still runs before any
  user-provided symbols are processed, add_symbol has the same effect.

Verified by building the plugin and running it against a real Sail RISC-V
model (riscv/sail-riscv, as vendored in external/sail-riscv), not just
compiling: `isla-sail --all-modules riscv.sail_project -o out` produces a
complete .ir file. Isla's own symbolic execution then hits a separate,
deeper incompatibility with that specific model's current physical-memory
validation logic (unrelated to these two fixes) -- tracked separately, not
fixed here.

These two fixes are self-contained and upstreamable as-is.
isla-footprint failed during model initialization against a current
sail-riscv build (specifically postlude/validate_config.sail's
check_pma_regions/check_mem_layout, evaluating `2 ^ physaddr_bits`) with
"Type error: %i64->%i I128(4)". i64_to_i128 is the primop backing Sail's
%i64->%i conversion, and only handled Val::I64 and Val::Symbolic inputs;
it had no case for an operand that arrives already as Val::I128.

That's a real, reachable case: exponentiation with a dependently-typed
exponent (as in `2 ^ physaddr_bits`) can produce an I128 value on a path
that's statically typed %i64, before it reaches this conversion. Since
I128 is what this function widens *to*, an already-I128 input just needs
to pass through unchanged -- the same reasoning already applied to the
existing Val::I64 and Val::Symbolic cases, just missing this one.

Verified against a snapshot built (via isla-sail, see the prior two
commits on this branch) from riscv/sail-riscv HEAD (external/sail-riscv's
pinned commit in the formal-c repo, 29e6158f): isla-footprint now
correctly derives ADD's semantics as a genuine symbolic SMT-LIB formula
(x12 := x11 + x10 for free variables v408/v409) from that exact model,
where it previously failed before any instruction-level execution began.
@bacam

bacam commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

The API issue has happened because isla-sail has been updated for the development version of Sail (which should be released shortly), although there's another small tweak I need to make.

The i64 issue is happening because the code Sail adds for the initialisation of pma_regions from the config file is adding an extra conversion, even though the struct it goes into expects an i64. I'm trying to track that down.

Alasdair pushed a commit to rems-project/sail that referenced this pull request Oct 21, 2026
Otherwise isla-sail attempts to use large integers in some places where a
small one is expected, e.g. structs.  Fixes rems-project/isla#105.
bacam added a commit to bacam/sail that referenced this pull request Oct 22, 2026
Otherwise isla-sail attempts to use large integers in some places where a
small one is expected, e.g. structs.  Fixes rems-project/isla#105.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants