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] problem with native linking against skiasharp and emscripten 3.1.56 #109289

Closed
radekdoulik opened this issue Oct 28, 2024 · 7 comments · Fixed by #109232
Closed

[wasm] problem with native linking against skiasharp and emscripten 3.1.56 #109289

radekdoulik opened this issue Oct 28, 2024 · 7 comments · Fixed by #109232
Assignees
Labels
arch-wasm WebAssembly architecture area-Build-mono area-System.IO.Compression in-pr There is an active PR which will close this issue when it is merged
Milestone

Comments

@radekdoulik
Copy link
Member

radekdoulik commented Oct 28, 2024

Application referencing skiasharp packages in Debug configuration are triggering these errors during runtime in chrome:

CompileError: WebAssembly.compileStreaming(): Compiling function #16745:"mono_ppdb_load_file" failed: expected 0 elements on the stack for fallthru, found 4 @+5989718
  [error] Error in mono_download_assets: CompileError: WebAssembly.compileStreaming(): Compiling function #16745:"mono_ppdb_load_file" failed: expected 0 elements on the stack for fallthru, found 4 @+5989718
  [error] Error in mono_download_assets: CompileError: WebAssembly.compileStreaming(): Compiling function #16745:"mono_ppdb_load_file" failed: expected 0 elements on the stack for fallthru, found 4 @+5989718
  [ERROR] PageError: CompileError: WebAssembly.compileStreaming(): Compiling function #16745:"mono_ppdb_load_file" failed: expected 0 elements on the stack for fallthru, found 4 @+5989718
  [ERROR] PageError: CompileError: WebAssembly.compileStreaming(): Compiling function #16745:"mono_ppdb_load_file" failed: expected 0 elements on the stack for fallthru, found 4 @+5989718
  [ERROR] PageError: CompileError: WebAssembly.compileStreaming(): Compiling function #16745:"mono_ppdb_load_file" failed: expected 0 elements on the stack for fallthru, found 4 @+5989718
  [ERROR] PageError: CompileError: WebAssembly.compileStreaming(): Compiling function #16745:"mono_ppdb_load_file" failed: expected 0 elements on the stack for fallthru, found 4 @+5989718
  [ERROR] PageError: CompileError: WebAssembly.compileStreaming(): Compiling function #16745:"mono_ppdb_load_file" failed: expected 0 elements on the stack for fallthru, found 4 @+5989718
  [ERROR] PageError: CompileError: WebAssembly.compileStreaming(): Compiling function #16745:"mono_ppdb_load_file" failed: expected 0 elements on the stack for fallthru, found 4 @+5989718
  [ERROR] PageError: CompileError: WebAssembly.compileStreaming(): Compiling function #16745:"mono_ppdb_load_file" failed: expected 0 elements on the stack for fallthru, found 4 @+5989718
  [ERROR] PageError: Error: Failed to start platform. Reason: CompileError: WebAssembly.compileStreaming(): Compiling function #16745:"mono_ppdb_load_file" failed: expected 0 elements on the stack for fallthru, found 4 @+5989718
      at en (http://localhost:5191/_framework/blazor.webassembly.js:1:55760)

It is also reproducible on CI, #109232 is now triggering this problem.

@radekdoulik radekdoulik added this to the 10.0.0 milestone Oct 28, 2024
@radekdoulik radekdoulik self-assigned this Oct 28, 2024
@radekdoulik
Copy link
Member Author

I tried various version of llvm to test assumption of this being related to the recent llvm bump to 19.1.0.

The problem exists on 19.1.0, 19.0.x alpha (same commit as emsdk) and even with 16.x after our emscripten bump to 3.1.56. So it looks unrelated.

@radekdoulik
Copy link
Member Author

Later I was trying to find which part of mono_ppdb_load_file might be causing issues.

When I removed code guarded by #ifndef DISABLE_EMBEDDED_PDB, the mono_ppdb_load_file wasn't causing problems anymore. Instead I am getting similar problem with

[error] MONO_WASM: instantiate_wasm_module() failed CompileError: WebAssembly.compileStreaming(): Compiling function #26259:"CompressionNative_InflateInit2_" failed: expected 1 elements on the stack for fallthru, found 4 @+7856472

Which lead me to the zlib linking, which I think is the issue here. What is happening here is that this line

int res = inflateInit2 (&stream, -15);

is being inlined and the same issue as with the CompressionNative_InflateInit2_ is occurring here. Later I found that the CompressionNative_Inflate has similar issue.

I wonder if we compile and/or link against two different zlib versions. I will dig deeper.

@lewing
Copy link
Member

lewing commented Oct 28, 2024

cc @carlossanlop

@radekdoulik
Copy link
Member Author

The inflateInit2_ function looks like this

/* Function used by zlib.h and zlib-ng version 2.0 macros */
int32_t Z_EXPORT PREFIX(inflateInit2_)(PREFIX3(stream) *strm, int32_t windowBits, const char *version, int32_t stream_size) {
...
}

and there's redefining macro with only 2 parameters

#  define inflateInit2(strm, windowBits) \
... inflateInit2_ ...

and function with same parameters as the redefining macro

/* ===========================================================================
 * Initialize inflate state and buffers.
 * This function is hidden in ZLIB_COMPAT builds.
 */
int32_t ZNG_CONDEXPORT PREFIX(inflateInit2)(PREFIX3(stream) *strm, int32_t windowBits) {
...
}

in our .wasm binary we have

(func Cr_z_inflateInit2_(param $0 i32, $1 i32, $2 i32, $3 i32) (result i32))
(func inflateInit2_(param $0 i32, $1 i32) (result i32))

It looks like possibly wrong function was inlined, maybe wasm-ld or wasm-opt issue?

radekdoulik added a commit to radekdoulik/runtime that referenced this issue Oct 30, 2024
Put NativeFileReference files after the runtime libs. This server as
a workaround of dotnet#109289
@radekdoulik
Copy link
Member Author

radekdoulik commented Oct 30, 2024

Got more details today.

I checked the libSkiaSharp.a. It contains libzlib.inflate.o. Unfortunately llvm's objdump is unable to read wasm object files. So I improved wa-info to read these files and show custom sections content. (make sure to update to have at least version 0.4 of wa-info to read these object files)

> wa-info -vv libzlib.inflate.o
...
Reading section:    Custom size:         1247 name: linking
bytes
  2  8 92 86 80 80  0 30  0  4  7 15 43 72 5f 7a |.......0....Cr_z|
 5f 69 6e 66 6c 61 74 65 52 65 73 65 74 4b 65 65 |_inflateResetKee|
 70  0  4  8 11 43 72 5f 7a 5f 69 6e 66 6c 61 74 |p....Cr_z_inflat|
 65 52 65 73 65 74  0  4  9 12 43 72 5f 7a 5f 69 |eReset....Cr_z_i|
 6e 66 6c 61 74 65 52 65 73 65 74 32  0  4  a 12 |nflateReset2....|
 43 72 5f 7a 5f 69 6e 66 6c 61 74 65 49 6e 69 74 |Cr_z_inflateInit|
 32 5f  0 10  0  0 10  1  0  4  b 11 43 72 5f 7a |2_..........Cr_z|

I don't know (yet) the linking custom section format. I speculate that the object file contain Cr_z_inflateInit2_ symbol.

Next I checked that mono_ppdb_load_file is calling inflateInit2_.

> wa-info -d -f mono_ppdb_load_file /Users/rodo/git/runtime/artifacts/bin/native/net10.0-browser-Debug-wasm/dotnet.native.wasm|grep call
...
   call inflateInit2_
...

and that inflateInit2_ is calling inflateInit2

> wa-info -d -f inflateInit2_ /Users/rodo/git/runtime/artifacts/bin/native/net10.0-browser-Debug-wasm/dotnet.native.wasm|grep call
  call inflateInit2

In our runtime libs, the _inflateInit2 comes from libz.a / inflate.c.o.

> wa-info -vv inflate.c.o
...
Reading section:    Custom size:         1261 name: linking
bytes
  2  8 bd 86 80 80  0 3f  0  4  5 10 69 6e 66 6c |.......?....infl|
 61 74 65 52 65 73 65 74 4b 65 65 70  0  2  6 11 |ateResetKeep....|
 69 6e 66 6c 61 74 65 53 74 61 74 65 43 68 65 63 |inflateStateChec|
 6b  0  4  7  c 69 6e 66 6c 61 74 65 52 65 73 65 |k....inflateRese|
 74  0  4  8  d 69 6e 66 6c 61 74 65 52 65 73 65 |t....inflateRese|
 74 32  0  4  9  d 61 6c 6c 6f 63 5f 69 6e 66 6c |t2....alloc_infl|
 61 74 65  5 b0  1  0  0  4  a  c 66 72 65 65 5f |ate........free_|
 69 6e 66 6c 61 74 65  0  4  b  c 69 6e 66 6c 61 |inflate....infla|
 74 65 49 6e 69 74 32  2 10  0  1 14  9 66 75 6e |teInit2......fun|
 63 74 61 62 6c 65  2 10  1  0 14  0  0 14  1  0 |ctable..........|
  4  c  c 69 6e 66 6c 61 74 65 49 6e 69 74 5f  0 |...inflateInit_.|
  4  d  d 69 6e 66 6c 61 74 65 49 6e 69 74 32 5f |...inflateInit2_|
...

Don't have complete image yet. So far I suspect, that wasm-ld or wasm-opt is inlining wrong function (with wrong signature). Or even wrong name, Cr_z_inflateInit2_ intead of inflateInit2_? edit: it is probably wasm-ld as we don't run wasm-opt in Debug configuration

Playing with these I found a workaround, which we might be able to use. I changed the order of linked libs, so that native reference libs are added after runtime libs. That worked for me locally. Testing that in #109232

@jeromelaban
Copy link
Contributor

I can confirm that changing the native libraries order is fixing the issue for my scenario.

The following target hacks its way through for rc2:

<Target Name="Issue109289_Workaround" AfterTargets="_BrowserWasmWriteRspForLinking" Condition=" $(NETCoreSdkVersion.Contains('rc.2')) ">
	<ItemGroup>
		<_WasmLinkStepArgs Remove="@(_EmccLinkStepArgs)" />
		<_EmccLinkStepArgs Remove="&quot;%(_WasmNativeFileForLinking.Identity)&quot;" />
		<_WasmLinkDependencies Remove="@(_WasmNativeFileForLinking)" />

		<_SkiaSharpToReorder Include="@(_WasmNativeFileForLinking)" Condition="$([System.String]::Copy('%(FullPath)').Contains('SkiaSharp'))" />
		<_WasmNativeFileForLinking Remove="@(_SkiaSharpToReorder)" />
		<_WasmNativeFileForLinking Include="@(_SkiaSharpToReorder)" />
			
		<_EmccLinkStepArgs Include="&quot;%(_WasmNativeFileForLinking.Identity)&quot;" />
		<_WasmLinkDependencies Include="@(_WasmNativeFileForLinking)" />
		<_WasmLinkStepArgs Include="@(_EmccLinkStepArgs)" />
	</ItemGroup>
</Target>

@radekdoulik
Copy link
Member Author

So the "wrong" _inflateInit2 comes from libSkiaSharp.a/3.1.56/st/libSkiaSharp.a(libfreetype2.ftgzip.o):(inflateInit2_).

(note: wasm-ld can print the link map with --print-map to the output.)

maxkatz6 added a commit to AvaloniaUI/Avalonia that referenced this issue Nov 14, 2024
maxkatz6 added a commit to AvaloniaUI/Avalonia that referenced this issue Nov 14, 2024
* Add dotnet/runtime#109289 workaround

* Add target framework version condition
maxkatz6 added a commit to AvaloniaUI/Avalonia that referenced this issue Nov 14, 2024
* Add dotnet/runtime#109289 workaround

* Add target framework version condition
@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Nov 19, 2024
radekdoulik added a commit that referenced this issue Nov 20, 2024
* [wasm] Re-enable skiasharp WBT tests

* Disable Debug/AOT combination

That would trigger build error, because we don't support that combination
anymore

* Change order of native libs

Put NativeFileReference files after the runtime libs. This server as
a workaround of #109289

* Update llvm, emsdk and icu deps

* Revert "Change order of native libs"

This reverts commit e320fd6.
mikelle-rogers pushed a commit to mikelle-rogers/runtime that referenced this issue Dec 10, 2024
* [wasm] Re-enable skiasharp WBT tests

* Disable Debug/AOT combination

That would trigger build error, because we don't support that combination
anymore

* Change order of native libs

Put NativeFileReference files after the runtime libs. This server as
a workaround of dotnet#109289

* Update llvm, emsdk and icu deps

* Revert "Change order of native libs"

This reverts commit e320fd6.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch-wasm WebAssembly architecture area-Build-mono area-System.IO.Compression in-pr There is an active PR which will close this issue when it is merged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants