Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WASM] browser-wasm tests fail with "Cannot box IsByRefLike type" #102988

Closed
liveans opened this issue Jun 3, 2024 · 7 comments · Fixed by #103052
Closed

[WASM] browser-wasm tests fail with "Cannot box IsByRefLike type" #102988

liveans opened this issue Jun 3, 2024 · 7 comments · Fixed by #103052
Assignees
Labels
arch-wasm WebAssembly architecture area-Build-mono disabled-test The test is disabled in source code against the issue Known Build Error Use this to report build issues in the .NET Helix tab
Milestone

Comments

@liveans
Copy link
Member

liveans commented Jun 3, 2024

Build Information

Build: https://dev.azure.com/dnceng-public/public/_build/results?buildId=695082
Build error leg or test failing: Build browser-wasm linux Release LibraryTests_Threading

Error Message

Fill the error message using step by step known issues guidance.

{
  "ErrorPattern": "System.InvalidProgramException.*Cannot box IsByRefLike type",
  "BuildRetry": false,
  "ExcludeConsoleLog": false
}

Known issue validation

Build: 🔎 https://dev.azure.com/dnceng-public/public/_build/results?buildId=695082
Error message validated: [System.InvalidProgramException.*Cannot box IsByRefLike type]
Result validation: ✅ Known issue matched with the provided build.
Validation performed at: 6/3/2024 6:16:35 PM UTC

Report

Build Definition Test Pull Request
695911 dotnet/runtime WasmTestOnFirefox-ST-System.Net.Http.Json.Unit.Tests.WorkItemExecution #101443
696308 dotnet/runtime WasmTestOnFirefox-ST-System.Net.Http.Json.Unit.Tests.WorkItemExecution #102845
696281 dotnet/runtime WasmTestOnFirefox-ST-System.Net.Primitives.Functional.Tests.WorkItemExecution #103009
696193 dotnet/runtime WasmTestOnChrome-ST-System.Net.Primitives.Functional.Tests.WorkItemExecution #103005
695634 dotnet/runtime WasmTestOnChrome-ST-System.Net.Primitives.Functional.Tests.WorkItemExecution #102986
696246 dotnet/runtime WasmTestOnChrome-ST-System.Net.Primitives.Functional.Tests.WorkItemExecution #103008
696316 dotnet/runtime WasmTestOnChrome-ST-System.Net.Primitives.Functional.Tests.WorkItemExecution #102475
696268 dotnet/runtime WasmTestOnChrome-ST-System.Net.Primitives.Functional.Tests.WorkItemExecution #102928
696224 dotnet/runtime WasmTestOnChrome-ST-System.Globalization.Calendars.Hybrid.WASM.Tests.WorkItemExecution
696179 dotnet/runtime WasmTestOnChrome-ST-System.Net.Http.Json.Unit.Tests.WorkItemExecution #102295
695848 dotnet/runtime WasmTestOnChrome-ST-System.Net.Http.Json.Unit.Tests.WorkItemExecution #102977
696139 dotnet/runtime WasmTestOnChrome-MT-System.Net.WebSockets.Client.Tests.WorkItemExecution #101801
695932 dotnet/runtime WasmTestOnChrome-ST-System.Net.Http.Json.Functional.Tests.WorkItemExecution #101547
696078 dotnet/runtime WasmTestOnChrome-ST-System.Net.Mail.Functional.Tests.WorkItemExecution #102379
696062 dotnet/runtime WasmTestOnChrome-ST-System.Net.Http.Json.Unit.Tests.WorkItemExecution #102967
695418 dotnet/runtime WasmTestOnChrome-ST-System.Formats.Tar.Manual.Tests.WorkItemExecution #102516
696041 dotnet/runtime WasmTestOnChrome-ST-System.Diagnostics.Tracing.Tests.WorkItemExecution #102679
695995 dotnet/runtime WasmTestOnFirefox-ST-System.Runtime.CompilerServices.Unsafe.Tests.WorkItemExecution #102998
696017 dotnet/runtime WasmTestOnChrome-ST-System.Net.Http.Json.Functional.Tests.WorkItemExecution #102992
695970 dotnet/runtime WasmTestOnChrome-ST-System.Net.Http.Json.Unit.Tests.WorkItemExecution #102996
695914 dotnet/runtime WasmTestOnChrome-ST-System.Net.Http.Json.Unit.Tests.WorkItemExecution #102994
695900 dotnet/runtime WasmTestOnChrome-ST-System.Net.Http.Json.Unit.Tests.WorkItemExecution #102993
695886 dotnet/runtime WasmTestOnChrome-ST-System.Net.Http.Functional.Tests.WorkItemExecution #102992
695893 dotnet/runtime WasmTestOnChrome-ST-System.Net.Http.Json.Unit.Tests.WorkItemExecution #102967
695882 dotnet/runtime WasmTestOnFirefox-ST-System.Net.Http.Json.Unit.Tests.WorkItemExecution #102989
695859 dotnet/runtime WasmTestOnChrome-ST-System.Net.Http.Json.Unit.Tests.WorkItemExecution #102991
695799 dotnet/runtime WasmTestOnChrome-ST-System.Net.Http.Json.Unit.Tests.WorkItemExecution #102699
695795 dotnet/runtime WasmTestOnChrome-ST-System.Diagnostics.TextWriterTraceListener.Tests.WorkItemExecution #102806
695686 dotnet/runtime WasmTestOnChrome-ST-System.Globalization.Hybrid.WASM.Tests.WorkItemExecution #102987
695616 dotnet/runtime WasmTestOnChrome-ST-System.Net.Mail.Functional.Tests.WorkItemExecution #102597
695594 dotnet/runtime WasmTestOnChrome-ST-System.Net.Http.Json.Unit.Tests.WorkItemExecution #102699
695082 dotnet/runtime WasmTestOnChrome-ST-System.Net.Http.Json.Unit.Tests.WorkItemExecution #102963

Summary

24-Hour Hit Count 7-Day Hit Count 1-Month Count
0 31 32
@liveans liveans added arch-wasm WebAssembly architecture blocking-clean-ci Blocking PR or rolling runs of 'runtime' or 'runtime-extra-platforms' Known Build Error Use this to report build issues in the .NET Helix tab labels Jun 3, 2024
Copy link
Contributor

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Jun 3, 2024
@jkotas
Copy link
Member

jkotas commented Jun 3, 2024

[07:12:13] fail: System.InvalidProgramException: Cannot box IsByRefLike type 'System.Span`1'
[07:12:13] fail:    at System.Version.TryFormatCore[Char](Span`1 destination, Int32 fieldCount, Int32& charsWritten)
[07:12:13] fail:    at System.Version.System.ISpanFormattable.TryFormat(Span`1 destination, Int32& charsWritten, ReadOnlySpan`1 format, IFormatProvider provider)
[07:12:13] fail:    at System.Text.ValueStringBuilder.AppendFormatHelper(IFormatProvider provider, String format, ReadOnlySpan`1 args)

Introduced by #102795

cc @AaronRobinsonMSFT @stephentoub Looks like Mono is missing byref-like generics support for some patterns.

@stephentoub
Copy link
Member

Presumably that particular case is due to the change from MemoryMarshal.Cast<TChar, char>(destination) to Unsafe.BitCast<Span<TChar>, Span<char>>. I can submit a PR to undo those call sites if desirable, though that won't help with the next time one of these is hit. @AaronRobinsonMSFT / @lambdageek, is the long-term fix here for mono quick, or should I revert some of those uses until we can get it sorted out?

@jkotas jkotas changed the title [WASM] Build browser-wasm linux Release LibraryTests_Threading fails [WASM] browser-wasm tests fail with "Cannot box IsByRefLike type" Jun 3, 2024
@lambdageek
Copy link
Member

@stephentoub please revert for now. I'm a bit surprised there's a difference. I think the long term fix for mono is probably quick (most likely BitCast needs to be an intrinsic) but on the order of a week, not a few hours.

@lambdageek
Copy link
Member

/cc @steveisok

@lambdageek lambdageek self-assigned this Jun 3, 2024
@lewing lewing added this to the 9.0.0 milestone Jun 3, 2024
@lewing lewing removed the untriaged New issue has not been triaged by the area owner label Jun 3, 2024
@stephentoub stephentoub added the disabled-test The test is disabled in source code against the issue label Jun 3, 2024
@jkotas jkotas removed the blocking-clean-ci Blocking PR or rolling runs of 'runtime' or 'runtime-extra-platforms' label Jun 3, 2024
@lambdageek
Copy link
Member

It looks like the problem is this code in Unsafe.Bitcast:

            if (sizeof(TFrom) != sizeof(TTo) || default(TFrom) is null || default(TTo) is null)
            {
                ThrowHelper.ThrowNotSupportedException();
            }
            return ReadUnaligned<TTo>(ref As<TFrom, byte>(ref source));

the test default(TFrom) is null generates:

	IL_000f:  ldloca.s 1
	IL_0011:  initobj !!0
	IL_0017:  ldloc.1 
	IL_0018:  box !!0
	IL_001d:  brfalse.s IL_0035

and I think Mono interpreter doesn't correctly elide the boxing for the box+brfalse pattern.
(Also Unsafe.Bitcast should probably just be an interpreter intrinsic)

@lambdageek
Copy link
Member

lambdageek commented Jun 4, 2024

I have a fix, but for some reason I'm having trouble making a standalone repro. The only way I get it to fail for sure is using our "xunit on chrome" scenario when this line executes:

https://github.com/dotnet/dotnet/blob/98884e3738c664d0950ed42dd9381a27ae630f43/src/source-build-externals/src/xunit/src/xunit.execution/Sdk/Frameworks/XunitTestFrameworkDiscoverer.cs#L21

Doing the same thing in a standalone sample doesn't cause a crash. As far as I can tell it's not due to IL differences. It might be different inlining choices in the interpreter, but I don't see how that would alter what the IL interpreter sees. very strange.

Update Looks like the difference is that our unit tests run with interpreter optimizations disabled, whereas the samples have optimizations.

Update2 Repro is to add this to a browser-wasm sample:

                string s = string.Format(CultureInfo.InvariantCulture, "version is {0}", new object[] {  new Version(1,2,3,4) });

And set these project properties:

  <PropertyGroup>
      <Optimize>false</Optimize>
      <DebuggerSupport>true</DebuggerSupport>
      <WasmDebugLevel>-1</WasmDebugLevel>
      <DebugType>portable</DebugType>
      <EmitDebugInformation>true</EmitDebugInformation>
  </PropertyGroup>

lambdageek added a commit to lambdageek/runtime that referenced this issue Jun 4, 2024
In cases where we don't do cprop and deadce (for example if
~mono_interp_opt~ is 0 because we're debugging) don't emit a box_vt
opcode before a branch.  Instead just emit a constant true

Fixes dotnet#102988
lambdageek added a commit that referenced this issue Jun 9, 2024
…s up front (#103052)

* [interp] handle box + brtrue/brfalse pattern for byreflike types

   In cases where we don't do cprop and deadce (for example if `mono_interp_opt` is 0 because we're debugging) don't emit a box_vt opcode before a branch.  Instead just emit a constant true

   Fixes #102988

* Revert "Revert uses of Unsafe.BitCast with spans to unblock mono (#102998)"

   This reverts commit c286a8e.

   Reverts #102998

* remove the box byreflike retry code

   we handle box byreflike code patterns directly now

* handle box;isinst;{brtrue|brfalse|unbox.any} IL patterns

* Add test locking down mismatched box;isinst;unbox.any pattern

* remove the mismatch case in the interp.  throw IPE at execution time

* Update src/tests/Loader/classloader/generics/ByRefLike/Validate.cs

Co-authored-by: Aaron Robinson <arobins@microsoft.com>

---------

Co-authored-by: Aaron Robinson <arobins@microsoft.com>
@github-actions github-actions bot locked and limited conversation to collaborators Jul 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-Build-mono disabled-test The test is disabled in source code against the issue Known Build Error Use this to report build issues in the .NET Helix tab
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants