[wasm] Bump chrome for testing - linux: 117.0.5938.132, windows: 117.0.5938.132 by github-actions[bot] · Pull Request #3 · eduardo-vp/runtime

github-actions · 2023-10-01T00:45:19Z

No description provided.

…#102133) This generalizes the indir reordering optimization (that currently only triggers for loads) to kick in for GT_STOREIND nodes. The main complication with doing this is the fact that the data node of the second indirection needs its own reordering with the previous indirection. The existing logic works by reordering all nodes between the first and second indirection that are unrelated to the second indirection's computation to happen after it. Once that is done we know that there are no uses of the first indirection's result between it and the second indirection, so after doing the necessary interference checks we can safely move the previous indirection to happen after the data node of the second indirection. Example: ```csharp class Body { public double x, y, z, vx, vy, vz, mass; } static void Advance(double dt, Body[] bodies) { foreach (Body b in bodies) { b.x += dt * b.vx; b.y += dt * b.vy; b.z += dt * b.vz; } } ``` Diff: ```diff @@ -1,18 +1,17 @@ -G_M55007_IG04: ;; offset=0x001C +G_M55007_IG04: ;; offset=0x0020 ldr x3, [x0, w1, UXTW #3] ldp d16, d17, [x3, #0x08] ldp d18, d19, [x3, #0x20] fmul d18, d0, d18 fadd d16, d16, d18 - str d16, [x3, #0x08] - fmul d16, d0, d19 - fadd d16, d17, d16 - str d16, [x3, #0x10] + fmul d18, d0, d19 + fadd d17, d17, d18 + stp d16, d17, [x3, #0x08] ldr d16, [x3, #0x18] ldr d17, [x3, #0x30] fmul d17, d0, d17 fadd d16, d16, d17 str d16, [x3, #0x18] add w1, w1, #1 cmp w2, w1 bgt G_M55007_IG04 ```

* bug #1: don't allow for values out of the SerializationRecordType enum range * bug #2: throw SerializationException rather than KeyNotFoundException when the referenced record is missing or it points to a record of different type * bug #3: throw SerializationException rather than FormatException when it's being thrown by BinaryReader (or sth else that we use) * bug #4: document the fact that IOException can be thrown * bug #5: throw SerializationException rather than OverflowException when parsing the decimal fails * bug #6: 0 and 17 are illegal values for PrimitiveType enum * bug #7: throw SerializationException when a surrogate character is read (so far an ArgumentException was thrown)

Currently, offsets are incorrectly treated as indices which is leading to incorrect code being emitted. e.g., `ScatterWithByteOffsets<long>` emits `ST1D Zdata.D, Pg, [Xbase, Zoffsets.D, lsl #3]` instead of, `ST1D Zdata.D, Pg, [Xbase, Zoffsets.D]`

…128163) > [!NOTE] > This PR was authored with assistance from GitHub Copilot. Fixes dotnet#128044. ## Problem createdump SIGSEGVs on Linux when generating a Heap-type minidump for a process running interpreted code. The crash reproduces locally with the `InterpreterStack` DumpTests debuggee and matches the CI failure that prompted `<DumpTypes>Full</DumpTypes>` to be added as a temporary workaround. The faulting backtrace is: ``` #0 Thread::IsAddressInStack threads.cpp:6741 #1 Thread::EnumMemoryRegionsWorker threads.cpp:6909 (calls IsAddressInStack(currentSP)) #2 Thread::EnumMemoryRegions threads.cpp #3 ThreadStore::EnumMemoryRegions #4 ClrDataAccess::EnumMemDumpAllThreadsStack #5 ClrDataAccess::EnumMemoryRegionsWorkerHeap (HEAP2-only path) ``` ## Root cause `Thread::m_pInterpThreadContext` was declared as a raw `InterpThreadContext *`. In non-DAC code that's a normal host pointer, but in DAC mode the field's value is a target-process address. When `IsAddressInStack` (a DAC-callable helper) dereferenced `m_pInterpThreadContext->pStackStart` it read from a target-process address as if it were a host address, which faults inside createdump. ## Fix Change the field type to `PTR_InterpThreadContext` (DPTR), matching the treatment of other Thread fields like `m_pFrame`. In non-DAC builds `DPTR(T)` is just `T*`, so there is no overhead or behavior change. In DAC builds the read goes through `__DPtr<T>` and marshals correctly from the target. Also remove the `<DumpTypes>Full</DumpTypes>` workaround on the `InterpreterStack` DumpTests debuggee so the Heap path that originally failed is exercised again. ## Validation Locally reproduced the original SIGSEGV on Linux x64 with the auto-dump mechanism (`DOTNET_DbgMiniDumpType=2` + `DOTNET_Interpreter=MethodA`) running the `InterpreterStack` debuggee. With this fix applied, createdump produces a complete Heap dump (~74 MB) instead of crashing. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

An Android production app reported a native abort while building an X.509 chain on arm64. The available tombstone snippet showed the process aborting in `AndroidCryptoNative_X509ChainBuild` from `pal_x509chain.c`, with the native guard reporting that parameter `ctx` was not a valid pointer. The report did not include a repro or full tombstone, but the observed failure mode means managed code reached the native build entry point with a null `X509ChainContext*`. ``` Thread /__w/1/s/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509chain.c:113 (AndroidCryptoNative_X509ChainBuild): Parameter 'ctx' must be a valid pointer *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** pid: 0, tid: 31609 >>> com.app.name <<< backtrace: #00 pc 0x000000000002232c /system/lib64/libc.so (abort+116) #1 pc 0x0000000000021fe8 [removed]-KwPZdoEumri00C7kBm3pQw==/lib/arm64/libSystem.Security.Cryptography.Native.Android.so #2 pc 0x00000000000220b0 [removed]-KwPZdoEumri00C7kBm3pQw==/lib/arm64/libSystem.Security.Cryptography.Native.Android.so (AndroidCryptoNative_X509ChainBuild+88) #3 pc 0x000000000000cfcc ``` `X509ChainContext` is created by `AndroidCryptoNative_X509ChainCreateContext`. That initialization can fail if Android certificate store setup or PKIX parameter construction throws, or if required JNI global references cannot be created. Previously, the managed Android chain path stored the returned `SafeHandle` without checking whether context creation failed, so a later build could pass a null native context to `AndroidCryptoNative_X509ChainBuild` and terminate the app process. This change makes context creation fail gracefully: - The native create path checks Java exceptions around object creation and method calls more consistently. - Partial native contexts are destroyed if global-reference creation fails. - The Android interop wrapper checks the returned chain context immediately, including a null safe-handle return, and throws `CryptographicException` if initialization failed. No regression test is included because the reliable failure modes depend on Android platform/provider state or artificial fault injection, and a test hook would be fragile and not representative. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Kevin Jones <kevin@vcsjones.com>

…dotnet#131279) [wasm] Restrict enclosing-try throw-helper attach to the same funclet ## Problem crossgen2 emits an invalid WASM (wasm32) R2R module for methods with certain try/catch shapes: the terminal `end` opcode (0x0b) is dropped for an EH funclet, so `wasm-tools validate` (and V8) reject the module with *"function body must end with end opcode"*. A related symptom is a miscomputed branch target depth in the same funclet (tracked as dotnet#131252). ## Root cause The defect is in wasm block layout, in `FgWasm::VisitWasmSuccs` (`src/coreclr/jit/fgwasm.h`). dotnet#130945 added an "enclosing try" relaxation: when a block is a try side-entry and the throw-helper ACD key is `KD_TRY`, the helper is also attached as a successor if ```cpp comp->bbInTryRegions(key.RegionIndex(), block) ``` is true. `bbInTryRegions` is a purely **lexical** try-nesting test — it does not check that the throw helper and the side-entry live in the same **function region (funclet)**. So a throw helper for an outer try region that belongs to the **main method** can be attached to a catch-resumption side-entry (`BBF_CATCH_RESUMPTION`) that merely nests inside that try but physically lives in a **handler funclet**. RPO layout then lays the main-method helper *inside* the funclet, interleaving it with funclet blocks. Because a funclet is a distinct wasm function body, that interleaving drops the funclet's terminal `end` and corrupts its branch target depths. Concrete trace from `System.Data.DataColumn:set_Expression` (instrumented `VisitWasmSuccs`, deduped): ``` sideEntry BB50 (tryIdx=4 hndIdx=3) preds[async=0 catch=1 other=2] pulls dst BB76 (hndIdx=0) crossFunclet=1 sideEntry BB73 (tryIdx=4 hndIdx=3) preds[async=0 catch=1 other=0] pulls dst BB76 (hndIdx=0) crossFunclet=1 ``` BB76 is the throw helper for root try region #4 (`KD_TRY`, no handler index → main method); BB50/BB73 are catch-resumption side-entries in handler funclet #3 (which lexically nests in try #4), so the enclosing-try rule pulls the main-method helper into funclet #3. This is an ordinary catch-resumption EH shape (`async=0`); it is independent of runtime-async, and `fgwasm.h` is byte-identical to `main`. It is latent on `main` only because R2R-wasm codegen isn't enabled there yet. ## Fix Gate the enclosing-try relaxation on same-function-region ownership. A new `funcRegionOf` lambda returns the funclet index that physically contains an arbitrary block (0 == main method); it mirrors `funGetFuncIdx` but works for non-entry blocks and distinguishes a filter funclet from its filter-handler. The helper is attached only when it shares the side-entry's function region: ```cpp if (!viaEnclosingTry || (funcRegionOf(block) == funcRegionOf(acd->acdDstBlk))) { RETURN_ON_ABORT(func(acd->acdDstBlk)); } ``` The exact-match path (a helper keyed to the block's own region) is unchanged, and dotnet#130945's intended case (an inner-try side-entry pulling its enclosing try's helper *within the same funclet*) still matches — so this only removes the cross-funclet edge. ## Validation Standalone `System.Data.Common` R2R wasm crossgen, release JIT, `wasm-tools validate` (only `fgwasm.h` differs between runs): | | `wasm-tools validate` | |---|---| | baseline (`origin/main` layout) | `func 597 failed to validate` ❌ | | with this fix | passes ✅ | ## Notes - Supersedes dotnet#131251, which hardened the terminal-`end` emission (the *effect*); this fixes the *cause* in layout. Closing dotnet#131251 in favor of this. - Related: dotnet#129335 (incomplete predecessor for this defect class, do not reopen); dotnet#130945 (introduced the lexical enclosing-try match); dotnet#131252 (miscomputed branch target depth — same corrupted layout, very likely subsumed by this fix). - Follow-up idea (not in this PR): a JIT-time assert that a funclet's blocks form a contiguous RPO range, so any future mis-layout traps at compile time instead of surfacing as an invalid module. > [!NOTE] > This change was authored with the assistance of GitHub Copilot. --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2e627b94-2658-41ce-a880-d9c27b7febd9

…stem (dotnet#131877) Replaces the hardcoded struct-size table in the CoreCLR wasm P/Invoke generator with crossgen2's real field-layout engine. ## The problem `ManagedToNativeGenerator` computed wasm ABI signature strings from `System.Reflection.MetadataLoadContext`, which has no field-layout engine. Struct sizes came from a 7-entry hardcoded table, and anything outside it was a hard build error: ``` error WASM0067: SignatureMapper: unknown multi-field struct 'X' (fields: N) - add its size to s_knownStructSizes in SignatureMapper.cs ``` Size matters because the CoreCLR interpreter lays struct arguments out inline across 8-byte slots — `TokenToSlotCount` returns `max((size + 7) / 8, 1)` for an `S<N>` token. A wrong `N` misaligns the interpreter frame. Mono's generator needs none of this: its alphabet has no `S`, and it encodes every struct as a pointer. ## The change crossgen2 gains `--generate-portable-callhelpers <dir>`, which writes the three C++ call-helper files directly. It sets up its type system as for a real wasm compilation, scans the input assemblies and emits — no JIT, no R2R image. The option requires `--targetarch wasm` with `--targetos browser|wasi`. The CoreCLR half of the MSBuild task is deleted rather than adapted: `ManagedToNativeGenerator`, `PInvokeCollector`, `PInvokeTableGenerator`, `SignatureMapper`, `InternalCallSignatureCollector`, `InterpToNativeGenerator`. `_CoreCLRGenerateManagedToNative` keeps its name and position in the target graph; its final step changes from `<UsingTask>` to `<Exec>`. The regeneration scripts move next to their output under `src/coreclr/vm/wasm/` and drive `generate-coreclr-helpers.proj`. Mono's generator is untouched. **−2269 lines under `src/tasks`, +1541 under `ILCompiler.ReadyToRun/PortableCallHelpers`.** A move, not an addition: the second implementation of wasm ABI lowering is gone, and the one that remains is the one the JIT interface itself calls. Sizes are computed, not enumerated. The only change to `WasmLowering` is widening `WasmValueTypeToSigChar` from `private` to `internal`. ### Naming Portable entry points exist for any platform that cannot generate code at run time; wasm is the only one today. Per [review feedback](dotnet#131877) nothing in this functionality is named after wasm. Symbols shared by the runtime and the generated tables were renamed on both sides at once: | before | after | |---|---| | `StringToWasmSigThunk` | `StringToPortableSigThunk` | | `g_wasmThunks[Count]` | `g_portableCallHelperThunks[Count]` | | `wasm_ret_S<n>` | `portable_callhelper_ret_S<n>` | What keeps wasm in its name is what is genuinely about wasm: the ABI in `WasmLowering`, the `--targetos browser|wasi` requirement, and the wasm-specific corerun the runtime tests link. ### Finding crossgen2 Three paths, tried in order: - **Override** — `$(PortableCallHelpersGeneratorPath)`, which must name a crossgen2 executable. - **In repo** — `$(Crossgen2InBuildDir)`; crossgen2 is built unconditionally by the `clr` subset. - **Out of repo** — the `wasm-tools` workload declares the existing `Microsoft.NETCore.App.Crossgen2.<host-rid>` pack, whose `Sdk/Sdk.props` defines `$(Crossgen2ToolPath)`. ~12.5 MB. The SDK resolves this pack only when `PublishReadyToRun` is set, which wasm CoreCLR apps never set — hence the workload. dotnet/sdk#56119 proposes acquiring it directly instead, which would let the workload entry go. If none of the three resolve, the targets error rather than passing an empty path down. The pack is named for the machine that *runs* crossgen2, not the target: generation never loads the JIT, so a host-targeting crossgen2 answers wasm ABI questions correctly. The workload-testing legs do not set `$(BuildHostTools)`, so nothing produced a crossgen2 pack for their local package feed. (The perf browser-wasm leg does produce one, but only because it opts in — dotnet#133143.) `Microsoft.NETCore.App.Crossgen2.Host.sfxproj` pins the RID to the build host, and is now built by the CoreCLR browser-wasm leg behind `$(BuildCrossgen2HostPackForWorkloadTesting)`, guarded on `$(BuildHostTools)` being unset so the two paths can never emit the same package id twice. The official build is untouched — it already publishes this pack from the host platform legs. ## Behaviour changes **`WASM0066` is removed.** The old task warned for every `DllImport` whose module did not resolve to a linked-in native library — a CoreCLR-only divergence that fires on ordinary cross-platform code never executed on wasm (dotnet#131874 reports ten from SkiaSharp alone on a shipped Preview 7 SDK). In-tree it had already accumulated two `NoWarn` suppressions and a `WarnOnUnresolvedPInvokeModules=false`; all three go, along with the `--no-warn-unresolved-directpinvoke` opt-out that existed only to silence it. An unresolved module is not knowably wrong at build time: `callhelpers_pinvoke_override` returns `nullptr` on a miss, so a call that actually happens throws `DllNotFoundException` naming the module, as on every other platform. Dropping a warning is strictly loosening. **`WASM0065` is added, as a message.** Per module, when it declares P/Invokes without `[assembly: DisableRuntimeMarshalling]`, since the generated helpers assume signatures cross unmarshalled. A message rather than a warning: it reports something the app author often cannot fix, and as a warning it would fail `-warnaserror` builds. Four fire across the 181 framework assemblies. **Exported callbacks with an ambiguous name are rejected.** An export wrapper resolves its `MethodDesc` through `LookupUnmanagedCallersOnlyMethodByName`, which takes the first `[UnmanagedCallersOnly]` method of matching name and compares no signature — so two exported overloads resolve to the same method and one wrapper calls it with the wrong arguments. Everything the generator controls carries the arity, so the existing duplicate-key and duplicate-symbol checks both pass. Generation now fails instead, naming both signatures. Only exports: a non-exported callback is found by the arity-aware key and never reaches the name lookup. ## Known limitations - **wasi has no out-of-repo acquisition path.** `wasi-experimental` extends `microsoft-net-runtime-mono-tooling`, not `wasm-tools`, so it picks up no crossgen2 pack; the targets error explicitly there. Browser is the shipping wasm/CoreCLR target. - **Reverse thunks allocate one `int64_t` slot per managed parameter**, while a by-value struct argument occupies `ceil(size/8)` interpreter slots. No `[UnmanagedCallersOnly]` callback in CoreLib or the libraries takes a by-value struct, so nothing exercises this. The old generator rejected such callbacks with `WASM0067`; this one accepts them, so user code would get a bad thunk rather than a diagnostic. - **`'V'` (v128) has no case in the C++ emission helpers.** Pre-existing; fails loudly. - **Multi-slot types (`Int128`, `Vector256`, …) are rejected at the thunk emitter** rather than at the interop boundary, so the diagnostic differs from the old `WASM0068`. Still a clean `crossgen2 : error :` with exit 1. No such P/Invoke exists today. - Does not re-enable the tests disabled in dotnet#131811 (dotnet#133187), and does not address gaps #3–#7 there. ## Verification - **Regeneration reproduces the committed helpers byte for byte**, apart from the rename above, with zero `WASM0001`/`WASM0060`/`WASM0061`/`WASM0062` warnings across a full CoreLib+libraries scan. (The checked-in P/Invoke table is already slightly stale against `main` independently of this PR; that drift is left alone.) - `WasmArgumentLayoutTests` goes from 17 to 22 test methods. The two covering the rejection above were checked against a disabled check, so they test it rather than agree with it. - `clr+libs` builds clean for `browser` and `wasi`; `WasmAppBuilder` still builds for both `net11.0` and `net472`. - Both flavors build end to end from the in-tree samples, with per-architecture native payloads, a non-PE file and duplicate-culture satellites injected into the bundle. - The renamed runtime contract was checked by building: `libcoreclr_static.a` exports `g_portableCallHelperThunks` and no `g_wasmThunks`, and the browser sample links its generated tables against it. Contributes to dotnet#131811, closing blocking gap #1 and the struct half of gap #2: a 3-int and a 5-double struct in `[UnmanagedFunctionPointer]` signatures now resolve to `vS12` / `S12i` / `vS40i`, where all three previously threw `NotSupportedException`. > [!NOTE] > This pull request description was drafted with the help of GitHub Copilot. --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: Jan Kotas <jkotas@microsoft.com> Copilot-Session: f6d6e5a4-5b25-4198-b42d-d5b2dc781f47

Experiment to get the SPMI diffs. On arm64 `IsContainableUnaryOrBinaryOp` bails out for any byref-producing parent (`TYP_BYREF` has no `VTF_INT`), so shifts are never folded into `ref`-based address arithmetic: ```diff ;; base + (index * 24), e.g. CastCache.TryGet - add w5, w3, #1 - add x5, x5, x5, LSL #1 - lsl x5, x5, #3 - add x5, x5, x0 - ldar w6, [x5] + add w5, w3, #1 + add x5, x5, x5, LSL #1 + add x5, x0, x5, LSL #3 + ldar w6, [x5] ``` Some noticeable [diffs](https://dev.azure.com/dnceng-public/public/_build/results?buildId=1550527&view=ms.vss-build-web.run-extensions-tab) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0d35d524-d3e6-4da0-85a3-75a5d07d9143

Automated bump of chrome version

4102df5

github-actions Bot added arch-wasm area-infrastructure-mono labels Oct 1, 2023

Navigation Menu

[wasm] Bump chrome for testing - linux: 117.0.5938.132, windows: 117.0.5938.132 - #3

[wasm] Bump chrome for testing - linux: 117.0.5938.132, windows: 117.0.5938.132#3
github-actions[bot] wants to merge 1 commit into
mainfrom
update-chrome-version-6366741546

github-actions Bot commented Oct 1, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

0 participants

Conversation

github-actions Bot commented Oct 1, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

0 participants