- 25 Feb, 2022 2 commits
-
-
Clemens Backes authored
After https://crrev.com/c/3484317, passing {nullptr} to the {CodeSpaceWriteScope} won't work any more. Since the tests do not have a {NativeModule} to pass instead, make them use {pthread_jit_write_protect_np} directly. The jump-table assembler tests have dedicated threads for writing and executing the code, so we just switch once per thread. The icache test switches between writing and executing, so we use a little struct for switching. R=jkummerow@chromium.org, tebbi@chromium.org Bug: v8:12644, v8:11974 Change-Id: I116f3ad75454f749cdc4635802a4617ff91548b2 Cq-Include-Trybots: luci.v8.try:v8_mac_arm64_rel_ng Cq-Include-Trybots: luci.v8.try:v8_mac_arm64_dbg_ng Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3487995Reviewed-by: Tobias Tebbi <tebbi@chromium.org> Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> Commit-Queue: Clemens Backes <clemensb@chromium.org> Cr-Commit-Position: refs/heads/main@{#79290}
-
Clemens Backes authored
In very rare cases we open CodeSpaceWriteScopes for multiple native modules at the same time, e.g. for tier down (debugging) via ExecuteCompilationUnits --> PublishCompilationResults --> OnFinishedUnits --> TriggerCallbacks --> AsyncCompileJob::CompilationStateCallback::call --> WasmEngine::UpdateNativeModuleCache --> RecompileNativeModule [for tier down] --> InitializeRecompilation --> FindFunctionsToRecompile --> CodeSpaceWriteScope Fixing this would be difficult because we actually want to keep the CodeSpaceWriteScope open during subsequent publishing. So instead, remove the assumption that scopes are always only open for one module at a time. In order to do this, we remove the {code_space_write_nesting_level_} counter and instead use the {current_native_module_} field in all configurations to check whether a scope is currently open, and for which module. R=jkummerow@chromium.org Bug: v8:12644, v8:11974 Change-Id: Idd24c87f5938c43e867c41fa1cd3879def6f3873 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3484317Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> Commit-Queue: Clemens Backes <clemensb@chromium.org> Cr-Commit-Position: refs/heads/main@{#79286}
-
- 17 Feb, 2022 1 commit
-
-
Clemens Backes authored
Publishing Wasm code often happens in a loop (in particular for deserialization), so hold the {CodeSpaceWriteScope} outside that loop to avoid repeated switching between writable and executable. R=ahaas@chromium.org Bug: v8:11974, chromium:1297999 Change-Id: Ic4bf859685e66c4ba297fed968d0df6ae7d24cba Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3468896Reviewed-by: Thibaud Michaud <thibaudm@chromium.org> Commit-Queue: Clemens Backes <clemensb@chromium.org> Cr-Commit-Position: refs/heads/main@{#79135}
-
- 11 Aug, 2021 1 commit
-
-
Clemens Backes authored
This is a reland of 1c0cca0f. It is modified now to use V8_HAS_PTHREAD_JIT_WRITE_PROTECT and is rebased on https://crrev.com/c/3085271 which fixes the definition of that macro. Original change's description: > Reland: [wasm] Check correctness of thread-local write protection > > The fix landed as a separate CL: https://crrev.com/c/3081522 > This is an unmodified reland. > > Original description: > We make an undocumented assumption in {CodeSpaceWriteScope} that a > single thread will only work on one module at a time. If this is > violated, the thread-local {code_space_write_nesting_level_} would > prevent the second module from being switched to writable. > > This CL adds a second thread local (in debug only) to check that if > there is already a {CodeSpaceWriteScope} open that it contains the same > {NativeModule} as any nested scope. > > R=jkummerow@chromium.org > > Bug: v8:11974 > Cq-Include-Trybots: luci.v8.try:v8_linux_gc_stress_dbg_ng > Cq-Include-Trybots: luci.v8.try:v8_mac64_gc_stress_dbg_ng > Change-Id: Id827b6ca472f695e4500584349aba159aa07eed1 > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3080578 > Commit-Queue: Clemens Backes <clemensb@chromium.org> > Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> > Cr-Commit-Position: refs/heads/master@{#76177} Bug: v8:11974 Change-Id: Iffc510e85c8c56f26bfa88115ed3a8bdd33ef422 Cq-Include-Trybots: luci.v8.try:v8_linux_gc_stress_dbg_ng Cq-Include-Trybots: luci.v8.try:v8_mac64_gc_stress_dbg_ng Cq-Include-Trybots: luci.v8.try:v8_mac_arm64_rel_ng Cq-Include-Trybots: luci.v8.try:v8_mac_arm64_dbg_ng Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3085269Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> Commit-Queue: Clemens Backes <clemensb@chromium.org> Cr-Commit-Position: refs/heads/master@{#76235}
-
- 10 Aug, 2021 1 commit
-
-
Minoru Chikamune authored
This reverts commit 1c0cca0f. Reason for revert: this made tree closed. (https://crbug.com/1238174) Original change's description: > Reland: [wasm] Check correctness of thread-local write protection > > The fix landed as a separate CL: https://crrev.com/c/3081522 > This is an unmodified reland. > > Original description: > We make an undocumented assumption in {CodeSpaceWriteScope} that a > single thread will only work on one module at a time. If this is > violated, the thread-local {code_space_write_nesting_level_} would > prevent the second module from being switched to writable. > > This CL adds a second thread local (in debug only) to check that if > there is already a {CodeSpaceWriteScope} open that it contains the same > {NativeModule} as any nested scope. > > R=jkummerow@chromium.org > > Bug: v8:11974 > Cq-Include-Trybots: luci.v8.try:v8_linux_gc_stress_dbg_ng > Cq-Include-Trybots: luci.v8.try:v8_mac64_gc_stress_dbg_ng > Change-Id: Id827b6ca472f695e4500584349aba159aa07eed1 > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3080578 > Commit-Queue: Clemens Backes <clemensb@chromium.org> > Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> > Cr-Commit-Position: refs/heads/master@{#76177} Bug: v8:11974, 1238174 Change-Id: I74d8723344e4b9015d956ebfe3fda492280356b6 Cq-Include-Trybots: luci.v8.try:v8_linux_gc_stress_dbg_ng Cq-Include-Trybots: luci.v8.try:v8_mac64_gc_stress_dbg_ng No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3084041 Auto-Submit: Minoru Chikamune <chikamune@google.com> Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Cr-Commit-Position: refs/heads/master@{#76184}
-
- 09 Aug, 2021 1 commit
-
-
Clemens Backes authored
The fix landed as a separate CL: https://crrev.com/c/3081522 This is an unmodified reland. Original description: We make an undocumented assumption in {CodeSpaceWriteScope} that a single thread will only work on one module at a time. If this is violated, the thread-local {code_space_write_nesting_level_} would prevent the second module from being switched to writable. This CL adds a second thread local (in debug only) to check that if there is already a {CodeSpaceWriteScope} open that it contains the same {NativeModule} as any nested scope. R=jkummerow@chromium.org Bug: v8:11974 Cq-Include-Trybots: luci.v8.try:v8_linux_gc_stress_dbg_ng Cq-Include-Trybots: luci.v8.try:v8_mac64_gc_stress_dbg_ng Change-Id: Id827b6ca472f695e4500584349aba159aa07eed1 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3080578 Commit-Queue: Clemens Backes <clemensb@chromium.org> Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> Cr-Commit-Position: refs/heads/master@{#76177}
-
- 06 Aug, 2021 3 commits
-
-
Clemens Backes authored
This reverts commit fee168ce. Reason for revert: The DCHECK fails when freeing code (https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux64%20GC%20Stress%20-%20custom%20snapshot/38292/overview), which means that the current code is not correct. The added DCHECK makes the bots red though, so the state before this CL was still better. Original change's description: > [wasm] Check correctness of thread-local write protection > > We make an undocumented assumption in {CodeSpaceWriteScope} that a > single thread will only work on one module at a time. If this is > violated, the thread-local {code_space_write_nesting_level_} would > prevent the second module from being switched to writable. > > This CL adds a second thread local (in debug only) to check that if > there is already a {CodeSpaceWriteScope} open that it contains the same > {NativeModule} as any nested scope. > > R=jkummerow@chromium.org > > Change-Id: I43fa886d9d0fdf0e1846137dc411745fcca471fa > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3074477 > Commit-Queue: Clemens Backes <clemensb@chromium.org> > Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> > Cr-Commit-Position: refs/heads/master@{#76134} TBR=jkummerow@chromium.org Change-Id: I5262b0e886f99a64452966345fc084a1ab750459 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3078360Reviewed-by: Clemens Backes <clemensb@chromium.org> Commit-Queue: Clemens Backes <clemensb@chromium.org> Cr-Commit-Position: refs/heads/master@{#76146}
-
Clemens Backes authored
pthread_jit_write_protect* functions are only available on arm64 Mac, not on iOS (which also sets V8_{TARGET_,}OS_MACOSX). This CL refactors the logic to detect whether pthread_jit_write_protect and MAP_JIT are available and defines a global preprocessor macro which can subsequently be used instead of the existing complex condition. R=jkummerow@chromium.org, mlippautz@chromium.org Change-Id: I63894f42df35406d6eee90a4ce5070c2fde7b566 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3077154Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> Reviewed-by: Adam Klein <adamk@chromium.org> Commit-Queue: Adam Klein <adamk@chromium.org> Auto-Submit: Clemens Backes <clemensb@chromium.org> Cr-Commit-Position: refs/heads/master@{#76143}
-
Clemens Backes authored
We make an undocumented assumption in {CodeSpaceWriteScope} that a single thread will only work on one module at a time. If this is violated, the thread-local {code_space_write_nesting_level_} would prevent the second module from being switched to writable. This CL adds a second thread local (in debug only) to check that if there is already a {CodeSpaceWriteScope} open that it contains the same {NativeModule} as any nested scope. R=jkummerow@chromium.org Change-Id: I43fa886d9d0fdf0e1846137dc411745fcca471fa Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3074477 Commit-Queue: Clemens Backes <clemensb@chromium.org> Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> Cr-Commit-Position: refs/heads/master@{#76134}
-
- 19 Jul, 2021 1 commit
-
-
Clemens Backes authored
Since {CodeSpaceWriteScope} is now used from cctests (since https://crrev.com/c/3024150), its constructor and destructor need to be exported. R=jkummerow@chromium.org Change-Id: I30627d632a9f4e4fde51bcb037bad69ceaedf6fd Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3038062 Commit-Queue: Nico Hartmann <nicohartmann@chromium.org> Reviewed-by: Nico Hartmann <nicohartmann@chromium.org> Cr-Commit-Position: refs/heads/master@{#75786}
-
- 15 Jul, 2021 2 commits
-
-
Clemens Backes authored
This removes the low-level {SwitchMemoryPermissionsToWritable()} and {SwitchMemoryPermissionsToExecutable()} functions. They are only used in tests and can be replaced by {CodeSpaceWriteScope} objects that we also use in production. R=jkummerow@chromium.org Change-Id: I7ba702c836f3ac2dd7c7a81d6362040b28e8bef4 Cq-Include-Trybots: luci.v8.try:v8_mac_arm64_rel_ng Cq-Include-Trybots: luci.v8.try:v8_mac_arm64_dbg_ng Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3024150 Commit-Queue: Clemens Backes <clemensb@chromium.org> Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> Cr-Commit-Position: refs/heads/master@{#75739}
-
Clemens Backes authored
We had two implementations of a thread-local counter for the number of writers: One in {CodeSpaceWriteScope} and one in {WasmCodeManager::SetThreadWritable}. This CL removes the latter, and uses the counter in {CodeSpaceWriteScope} for all implementations. R=jkummerow@chromium.org Bug: v8:11974 Cq-Include-Trybots: luci.v8.try:v8_mac_arm64_rel_ng Cq-Include-Trybots: luci.v8.try:v8_mac_arm64_dbg_ng Change-Id: I683131296c6106a2b12986942bb18e6c0e716612 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3024148Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> Commit-Queue: Clemens Backes <clemensb@chromium.org> Cr-Commit-Position: refs/heads/master@{#75733}
-
- 21 Jun, 2021 2 commits
-
-
Daniel Lehmann authored
Merges `NativeModuleModificationScope` (with an implementation using Intel PKU, if available, and mprotect otherwise) and `CodeSpaceWriteScope` (for Apple Silicon, where switching to RWX with mprotect is disallowed anyway, so MAP_JIT and thread-local switching must be used). Because `CodeSpaceWriteScope` sounded better (and is shorter), we kept its name (which unfortunately makes the diff a bit harder to read). R=clemensb@chromium.org CC=jkummerow@chromium.org Bug: v8:11714 Cq-Include-Trybots: luci.v8.try:v8_linux64_fyi_rel_ng Change-Id: Ib2a7d18e72797a725ed34b904c70769166d811dd Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2972911Reviewed-by: Clemens Backes <clemensb@chromium.org> Commit-Queue: Daniel Lehmann <dlehmann@google.com> Cr-Commit-Position: refs/heads/master@{#75272}
-
Daniel Lehmann authored
In an effort to merge `CODE_SPACE_WRITE_SCOPE` and `NativeModuleModificationScope`, this CL moves the interface and implementation of the latter into code-space-access.{h,cc}, where the former already lives. No other changes to the code itself. R=clemensb@chromium.org CC=jkummerow@chromium.org Bug: v8:11714 Cq-Include-Trybots: luci.v8.try:v8_linux64_fyi_rel_ng Change-Id: I1aabce26f2033430523a7a3a0a4864e7267bee21 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2972803Reviewed-by: Clemens Backes <clemensb@chromium.org> Commit-Queue: Daniel Lehmann <dlehmann@google.com> Cr-Commit-Position: refs/heads/master@{#75267}
-
- 16 Mar, 2021 1 commit
-
-
Clemens Backes authored
This will make accidental includes much easier to see and fix. Without this, you might get compiler or linker errors instead. R=jkummerow@chromium.org Bug: v8:11238 Cq-Include-Trybots: luci.v8.try:v8_linux64_no_wasm_compile_rel Change-Id: I235d779f9c1ed3af5d736f1554ded427935ddc9b Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2756531 Commit-Queue: Clemens Backes <clemensb@chromium.org> Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> Cr-Commit-Position: refs/heads/master@{#73422}
-
- 26 Nov, 2020 1 commit
-
-
Santiago Aboy Solanes authored
Scopes in V8 are used to guarantee one or more properties during its lifetimes. If a scope is not named e.g MyClassScope(args) instead of MyClassScope scope(args) it will get created and automatically destroyed and therefore, being useless as a scope. This CL would produce a compiling warning when that happens to ward off this developer error. Follow-up to ccrev.com/2552415 in which it was introduced and implemented for Guard classes. Change-Id: Ifa0fb89cc3d9bdcdee0fd8150a2618af5ef45cbf Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2555001 Commit-Queue: Santiago Aboy Solanes <solanes@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Reviewed-by: Leszek Swirski <leszeks@chromium.org> Reviewed-by: Michael Lippautz <mlippautz@chromium.org> Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> Reviewed-by: Ross McIlroy <rmcilroy@chromium.org> Reviewed-by: Tobias Tebbi <tebbi@chromium.org> Cr-Commit-Position: refs/heads/master@{#71425}
-
- 09 Sep, 2020 1 commit
-
-
Jakob Kummerow authored
Apple's upcoming arm64 devices will prevent rwx access to memory, but in turn provide a new per-thread way to switch between write and execute permissions. This patch puts that system to use for the WebAssembly subsystem. The approach relies on CodeSpaceWriteScope objects for now. That isn't optimal for background threads (which could stay in "write" mode permanently instead of toggling), but its simplicity makes it a good first step. Background: https://developer.apple.com/documentation/apple_silicon/porting_just-in-time_compilers_to_apple_silicon Bug: chromium:1117591 Change-Id: I3b60f0efd34c0fed924dfc71ee2c7805801c5d42 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2378307 Commit-Queue: Jakob Kummerow <jkummerow@chromium.org> Reviewed-by: Michael Lippautz <mlippautz@chromium.org> Reviewed-by: Thibaud Michaud <thibaudm@chromium.org> Cr-Commit-Position: refs/heads/master@{#69791}
-