wasmtime / issue #9402 Move constant pools in compiled co... · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / issue #9402 Move constant pools in compiled co...

Wasmtime GitHub notifications bot (Oct 08 2024 at 21:23):

alexcrichton opened issue #9402:

Today Cranelift's constant-pools are located in the .text section of the executable, typically located after the function itself. While convenient for code generation this exposes a possible attack vector in Wasmtime where it's trivial to put a "gadget" somewhere in memory. For example using a sequence of v128.const it would be pretty easy to assemble "machine code" at the end of a function. In the face of a bug in Cranelift this could make it possibly easier to amplify into a sandbox escape perhaps.

As a defense-in-depth measure we should try to move the constant pools out of the .text section and into a .data or otherwise read-only section. (not writable or executable). This won't be trivial to do due to the fact that relocations from the text section point at the data section and the relocation range may not always be large enough for the entire text section. Regardless though I wanted to file an issue about this idea.

Wasmtime GitHub notifications bot (Oct 08 2024 at 21:30):

cfallin commented on issue #9402:

It would definitely be nice to have support for this -- in principle we could return two blobs of bytes as the result of per-function compilation instead of one, and have a relocation type that is "offset from start of this function's code to start of this function's constants".

Out of curiosity, do you happen to know how ld handles .rodata references today for very large aarch64/riscv64/... binaries? I wonder if it uses its support for relaxation (assuming most pessimistic range sequence then shrinking if able) -- it'd be unfortunate to have to use adrp/adr/ldr rather than the immediate-pcrel form of ldr for every constant. I'm not able to find anything on this at the moment...

Wasmtime GitHub notifications bot (Oct 08 2024 at 22:52):

alexcrichton commented on issue #9402:

That's an excellent question, and one I don't know the answer to myself. I can try to play around with an assembler though and see what happens perhaps!

Wasmtime GitHub notifications bot (Oct 08 2024 at 23:13):

cfallin commented on issue #9402:

I tried briefly to trigger something interesting, but got stuck at trying to get clang (on macOS/aarch64) to use the short-form LDR-with-immediate instruction; for any load from rodata it seems to use an adrp/adr pair.

For example with (separate files to avoid a neat optimization where clang const-folds the load of constant data):
% cat test.c
extern const char* s;

int foo() {
    return *((int*)s);
}

% cat data.c
const char* s = "1234";

% cat main.c
#include <stdio.h>
extern int foo();
int main() {
    printf("%d\n", foo());
}
I see _foo's body as
0000000100003f44 <_foo>:
100003f44: b0000028     adrp    x8, 0x100008000 <_s>
100003f48: 91000108     add x8, x8, #0x0
100003f4c: f9400108     ldr x8, [x8]
100003f50: b9400100     ldr w0, [x8]
100003f54: d65f03c0     ret
Maybe it wouldn't be so bad to unconditionally emit that form actually; loads from constant pools will be relatively rare. It does burn a register though to compute the address.

Wasmtime GitHub notifications bot (Oct 09 2024 at 00:00):

alexcrichton commented on issue #9402:

Good point!

Looks like
#[no_mangle]
pub extern "C" fn foo() ->f64 {
    1.3484
}
generates
.LCPI0_0:
        .xword  0x3ff5930be0ded289
foo:
        adrp    x8, .LCPI0_0
        ldr     d0, [x8, :lo12:.LCPI0_0]
        ret
so yeah it looks like we may want to be a tiny bit clever (don't always "just" materialize the address) but otherwise looks like solving this issue would involve always doing adrp on aarch64 and the equivalent on riscv64

Wasmtime GitHub notifications bot (Oct 10 2024 at 22:54):

alexcrichton added the cranelift:area:security label to Issue #9402.

Wasmtime GitHub notifications bot (Oct 08 2026 at 21:06):

ilyas-mallah commented on issue #9402:

I'd like to work on this. What I have in mind follows @cfallin's idea of returning the constants as a second blob. MachBuffer would get an option to hand back a function's constants separately instead of emitting them after the code, with every constant use becoming a relocation into that blob. Wasmtime then places the blobs in a read-only section after .text, page-aligned so no constant shares an executable page with code.

x64, riscv64 and s390x already reach far enough. AArch64 is the hard part, since ldr literal would have to become an adrp-based load, which costs an instruction and, for float and vector constants, a GPR. I'd benchmark the options with Sightglass before picking one.

With every function's constants in one section, identical constants could also be coalesced across functions (the part @fitzgen moved here from #1385), and constants would no longer ride along in islands, which was part of what made #12968 tricky.

I'd land it as a few small PRs, starting with the buffer support, then a PR each for the x64, AArch64, riscv64 and s390x backends, and finally turning it on in Wasmtime.

A few things I'm unsure about. Should this be a Cranelift setting or something Wasmtime always enables? Would you prefer a separate section or a page-aligned tail of .text? And would you want an RFC first?

Wasmtime GitHub notifications bot (Oct 08 2026 at 22:20):

alexcrichton commented on issue #9402:

It'd be awesome if you're willing to pick this up @ilyas-mallah! As a heads up we as maintainers have a lot of issues in flight right now so we may be a bit slower to respond here or on PRs, but it'd be great to resolve this still. What you describe all sounds reasonable to me, and IIRC some time ago we were thinking of changing all data relocations to adrp+add or similar for aarch64, although maybe that was in relation to this issue as well... A sightglass run would be definitely helpful to evaluate!

As for landing, piecemeal is totally fine. I wouldn't necessarily go out of your way for a Cranelift setting for this, but if it's natural to implement then it seems reasonable to have and would pave the path for testing this easily without committing to it just yet. No need for an RFC on this one, good to just work on and have PRs!

One thing of note -- Wasmtime at least already has some page-aligned stuff after the .text section, for example wasm data segments and such. That should be relatively easy to piggy-back on. Wasmtime also puts the .eh_frame section (IIRC) in a page-aligned section after .text so it's not executable as well, which could perhaps show inspiration for how to do this since that also has to do with relocations across the sections.

Wasmtime GitHub notifications bot (Oct 08 2026 at 23:23):

ilyas-mallah commented on issue #9402:

Thanks, that's really helpful, and no worries about review speed. Good pointers on the data segments and .eh_frame, I'll look at how they handle relocations across sections first, and I'll include Sightglass numbers with the PRs. I'm planning to start early next year.

Wasmtime GitHub notifications bot (Oct 09 2026 at 20:54):

ilyas-mallah commented on issue #9402:

I'd like to split this into PRs this way, following my earlier comment:

Cranelift: MachBuffer support for the separate constant blob, off by default.

x64: RIP-relative constant references become relocations into the blob.

AArch64: adrp-based loads in place of ldr literal, using whichever sequence Sightglass favors.

riscv64: auipc plus a load into the blob.

s390x: 32-bit PC-relative references into the blob.

Winch, since it registers its constants in the same MachBuffer. Pulley bytecode isn't run as machine code, so it stays as it is.

Wasmtime: emit the new section following the .eh_frame model and turn it on by default. This covers serialized modules, a test that no executable page holds constants, and an update to docs/security.md.

I'd leave coalescing identical constants across functions for later, it's easier once all of this has landed.

I'd also like to take on #11181, opting wasmtime modules into the unsafe Clippy lints one PR at a time. Each PR adds # Safety docs and SAFETY comments, with a safe wrapper where a contract can be checked locally. The modules in order are vm/cow.rs with pooling/decommit_queue.rs, vm/mmap.rs with vm/sys/unix/mmap.rs, vm/libcalls.rs, vm/component/libcalls.rs, vm/table.rs, vm/traphandlers.rs with its backtrace.rs, and vm/sys/unix/signals.rs.

@alexcrichton you mentioned moving all AArch64 data relocations to adrp+add. Should the AArch64 PR handle LoadExtNameFar too (it puts an 8-byte address inline in .text), or just the constant pools? And would you rather the last PR turn this on by default, or add a Config option that stays off for now?

Wasmtime GitHub notifications bot (Oct 12 2026 at 14:50):

alexcrichton commented on issue #9402:

For this that sounds reasonable enough, but I'll caution that this isn't guaranteed to all work out exactly as you laid out. It might be that having this as a conditional option is too onerous, or maybe updating one architecture is extremely difficult relative to others, or something like that. This'll require experimentation/measurement and as a modification to the core compiler primitives it'll have to be done with a good deal of care as well.

For #11181 I'd recommend discussing over on that issue, but I'll also caution that adding SAFETY comments often requires in-depth and detailed knowledge of what's going on, so it may not be the best first-issue to take on.

Wasmtime GitHub notifications bot (Oct 12 2026 at 17:35):

ilyas-mallah commented on issue #9402:

Makes sense. I'll prototype the AArch64 load sequences and measure them with Sightglass before the first PR, and adjust the plan to what that shows. A few quick questions, your first impression is plenty.

Do you also expect AArch64 to be the hard one, since ldr literal only reaches ±1 MB and each use would need adrp plus an address register? What about the other backends?

If keeping both paths in every backend turns out too onerous, would you lean toward one code path instead of a conditional option?

Is the MachBuffer island and deadline logic, which constants go through today, where you'd expect most of the care to be needed, or somewhere else?

Wasmtime GitHub notifications bot (Oct 12 2026 at 21:01):

alexcrichton commented on issue #9402:

Fully answering most of those questions would likely require a level of detail which, if I could give, would likely mean that the change was already done. Basically I think answering those questions requires doing some work and seeing how it goes.

Wasmtime GitHub notifications bot (Oct 12 2026 at 21:35):

ilyas-mallah commented on issue #9402:

15\. Fully answering most of those questions would likely require a level of detail which, if I could give, would likely mean that the change was already done. Basically I think answering those questions requires doing some work and seeing how it goes.

That's fair. A prototype of the AArch64 sequences with Sightglass numbers is probably the way to find out, and I'll post them here if I go that way.

Wasmtime GitHub notifications bot (Oct 12 2026 at 21:36):

ilyas-mallah edited a comment on issue #9402:

Fully answering most of those questions would likely require a level of detail which, if I could give, would likely mean that the change was already done. Basically I think answering those questions requires doing some work and seeing how it goes.

That's fair. A prototype of the AArch64 sequences with Sightglass numbers is probably the way to find out, and I'll post them here if I go that way.

Wasmtime GitHub notifications bot (Oct 21 2026 at 11:04):

ilyas-mallah commented on issue #9402:

I built a prototype to see how this goes. It adds a Cranelift setting, separate_constant_pool, off by default. With it on, MachBuffer hands back each function's constants as a separate blob, with every use a relocation. Wasmtime puts the blobs in a .wasmtime.constants section on the page after .text. It covers x86-64 and AArch64, not yet riscv64, s390x, Winch, cranelift-jit or cranelift-object.

On x86-64 the backend change is 7 lines. The loads stay RIP-relative and only their displacements change. With the setting off, .text is byte-identical to main for the 28 benchmark modules I compiled. With it on, a running wasmtime run process has no v128.const bytes left in executable mappings (3 before, 0 after). The wast suite and tests/all give the same results with the setting on and off.

On AArch64 each ldr literal becomes adrp x16 and an ldr with the low 12 bits. The x16 register is already kept out of allocation as the spill temporary, so the register allocator needed no change. That relies on no emission sequence keeping a live value in x16 across a constant load, which I haven't audited beyond the tests. Under qemu the wast suite and tests/all give the same results both ways. A module that folds 1,200 distinct constants returns the reference values.

Sightglass on x86-64 (28 benchmarks, 100 samples each with perf counters, on a shared desktop) shows nothing measurable. Execution cycles change by +0.09% in the geomean (99% interval -1.49% to +1.41%). An A/A control of the same engine shows +0.19% (-1.21% to +1.61%). Single-threaded wasmtime compile retires 0.006% to 0.032% more instructions (10 runs per module). I have no AArch64 hardware numbers yet. Statically, the 1,431 constant loads in those modules add 5,724 bytes of code, 0.074% of .text.

One cost showed up. .eh_frame has to start on a page, so the constants section is padded to the text alignment. That grows .cwasm files by 0.49% on x86-64 and 5.0% on AArch64, where the alignment is 64 KiB. Putting the constants after .eh_frame would avoid it.

Is x16 fine for the address, or would you want a register from the allocator? And is this enough to start the MachBuffer and x64 PRs, or do you want AArch64 hardware numbers first? I can push the prototype branch if it helps to see the whole change.

Wasmtime GitHub notifications bot (Oct 21 2026 at 19:50):

alexcrichton commented on issue #9402:

For x16 -- whether or not that's suitable sort of depends on how the instructions are generated. If this is generated after register allocation runs then that's all that can be done, but if it can be done before register allocation runs that would be preferable to allow regalloc to go ham on these instructions. In that sense this may make more sense as a compiler flag/setting instead of a MachBuffer flag.

For .eh_frame -- one of the constants section or .eh_frame needs to be page-aligned to ensure that it doesn't overlap on an executable page, but not both. It should be possible to refactor things such that the two sections are adjacent in the *.cwasm file.

For perf numbers, yeah aarch64 is probably the more interesting one since it has substantive changes. If you're able to share some code a number of us have aarch64-based macOS machines we could test on.

Wasmtime GitHub notifications bot (Oct 22 2026 at 00:11):

cfallin commented on issue #9402:

(I have some thoughts on the regalloc implications of the above but am at a team offsite this week -- just pre-registering that I will try to write more soon, sorry!)

Wasmtime GitHub notifications bot (Oct 23 2026 at 19:25):

cfallin commented on issue #9402:

On AArch64 each ldr literal becomes adrp x16 and an ldr with the low 12 bits. The x16 register is already kept out of allocation as the spill temporary, so the register allocator needed no change. That relies on no emission sequence keeping a live value in x16 across a constant load, which I haven't audited beyond the tests. Under qemu the wast suite and tests/all give the same results both ways. A module that folds 1,200 distinct constants returns the reference values.

This is unfortunately probably not workable as-is, if you are generating separate MachInsts into vcode during lowering: the invariant is that regalloc is free to insert spills/reloads at any point between instructions, and it may use x16 when needed (e.g. to generate a stack-frame offset if the stack frame is larger than what can be reached by the immediate offset in ordinary loads/stores).

Question: do you generate a single relocation that needs both instructions next to each other (i.e. the Reloc handling processes a single 8-byte sequence with a defined form), or are you generating two relocations, for the adrp and the ldr? Depending on the answer I think one or the other of these is the right solution:

If two relocs: allocate a temp vreg as the result of the adrp, then use it as the base of the ldr. In other words, just reify what's going on in the ordinary SSA that the regalloc processes. It will do the right thing and ensure the dataflow works.

If one reloc: do adrp+ldr as one macroinstruction (one MachInst).

I suspect the former (I know I've seen the separate high/low-bit relocs) and that's the slightly cleaner approach so I'd lean that way by default.

Wasmtime GitHub notifications bot (Oct 23 2026 at 20:24):

ilyas-mallah commented on issue #9402:

Two relocations. The adrp gets its own page-offset relocation and the ldr gets the low 12 bits one, and each patches only its own 4-byte word.

My earlier comment described the first prototype, which was the single-instruction form with adrp x16 hardcoded. The second prototype already does what you suggest. The adrp is its own instruction that defines a fresh vreg during lowering, and the load uses that vreg as its base. x16 isn't involved, so regalloc is free to place the pair and use x16 for spills. Across the 28 benchmark modules the adrp destinations are x0 to x15.

To cover your frame-size case I added a test with a function holding 4200 live v128 constants. Its 80 KB of spill slots are past the reach of a scaled ldr q, so spills go through x16. It passes on x86-64 and under qemu AArch64, with the setting on and off. I haven't run anything on real AArch64 hardware.

Alex, the branch is https://github.com/ilyas-mallah/wasmtime/tree/constant-pools-prototype-v2 if you want to try it on AArch64 macOS. The commit marked Temporary only makes the test suites run with the setting on.

Wasmtime GitHub notifications bot (Oct 26 2026 at 20:23):

cfallin commented on issue #9402:

OK, I think the solution I outlined with two separate VCode instructions, one for adrp and one for ldr, with an ordinary vreg temp between them, should work then. Could you go ahead and post a PR so we can review and comment on it?

Wasmtime GitHub notifications bot (Oct 26 2026 at 21:55):

ilyas-mallah commented on issue #9402:

Which PR do you mean? The branch is a rough prototype: the MachBuffer blob with the x64 relocations, the Wasmtime section behind a config option, the AArch64 adrp pair, a test, and a temporary hook that turns the setting on for the suites. The plan on this issue splits that into smaller PRs, and I'd rather open something you can actually review than the prototype as it is. What would you want to see reviewed first?

Wasmtime GitHub notifications bot (Oct 26 2026 at 21:59):

cfallin commented on issue #9402:

Right, I mean, we seem to have converged on a design; please go ahead and post PR(s). I actually am happy to review a PR with all of your steps above, as long as it's not excessive (say <1kLoC without CLIF tests, which I expect it should be). I'm asking for this rather than looking at that branch, as a PR is our usual means of code-review in this project; it's very cumbersome to go fetch a branch manually, figure out the diff-base and look at the diff myself, then try to communicate my feedback manually, etc.

Last updated: Oct 31 2026 at 23:03 UTC