wabt icon indicating copy to clipboard operation
wabt copied to clipboard

Fallback to portable(-ish) SIMD reads against guard pages on not-yet-supported platforms

Open SoniEx2 opened this issue 9 months ago • 6 comments

Closes #2240

SoniEx2 avatar Feb 18 '25 15:02 SoniEx2

(wait, is this slower than the old way? hmm... think we might still be able to do something about it tho...)

SoniEx2 avatar Feb 18 '25 15:02 SoniEx2

(regardless of speed, this is certainly way cleaner now.)

SoniEx2 avatar Feb 18 '25 15:02 SoniEx2

we can understand that.

in theory, guard pages are in-bounds of the memory object as per C rules, so we are arguably not triggering UB by accessing guard pages but merely side-effects. (this is unlike, say, trying to use NULL pointers to access the zero page.) but even then, volatile access is implementation-defined. we would personally consider any compilers that simultaneously:

  1. target platforms where we can use guard pages
  2. allow us to use guard pages
  3. ignore the volatile access

as buggy. but there's a good chance the compiler would emit an additional read (and possibly a register spill) for the volatile read (especially with optimizations disabled), which isn't ideal as it degrades performance.

(there's also the question of whether the memory object itself should be considered a volatile object, such that regular accesses to it are UB - at least when guard pages are in use. we don't have a good answer to that one.)

so, considering all of that... yeah, fair enough.

(a more proper implementation would use volatile access to access the memory itself, but we don't believe there's such thing as a "volatile memcpy" we could use. after all, the value read by the memcpy isn't actually used, and that's a regular access as far as the compiler is concerned...)

SoniEx2 avatar Mar 17 '25 19:03 SoniEx2

My understanding is that #2240 (specifically the failure on aarch64) was already fixed by #2406.

It seems like a minimalist option could be to enforce that every platform either uses explicit bounds-checks, or (if using guard pages) that it has a working "force read" macro. We could just bomb out if the user tries to compile with guard pages but we don't have a working force-read on that platform.

keithw avatar Mar 18 '25 01:03 keithw

hmm, we would still like to try the volatile-based best-effort support. the tests should catch platforms where it doesn't work, then we can revisit what to do about it (probably include the bomb out for the specific platform, with an error message that instructs how to make it work). how does that sound?

SoniEx2 avatar Mar 18 '25 02:03 SoniEx2

hmm, we would still like to try the volatile-based best-effort support. the tests should catch platforms where it doesn't work, then we can revisit what to do about it (probably include the bomb out for the specific platform, with an error message that instructs how to make it work). how does that sound?

I'm not opposed to either trying it out and seeing how it fairs on different platforms, or falling back on bounds checks. I'll let @keithw and @sbc100 weigh in on their preferences.

If we do end up relying on bounds checks due to lack of force_read on some platforms, I think one nice to have would be to use bounds checks for reads but rely on guard pages for writes. This is still a non trivial performance benefit. An easy way to do this would be to adjust the force read macro to do a bounds checks (you'll probably want to rename the macro and adjust its parameters if you go this route)

shravanrn avatar Mar 18 '25 02:03 shravanrn