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

<WasiAfterRuntimeLoaded> is broken in wasi-wasm workflow #92551

Closed
filipw opened this issue Sep 24, 2023 · 1 comment · Fixed by #104549
Closed

<WasiAfterRuntimeLoaded> is broken in wasi-wasm workflow #92551

filipw opened this issue Sep 24, 2023 · 1 comment · Fixed by #104549
Assignees
Labels
arch-wasm WebAssembly architecture area-Build-mono in-pr There is an active PR which will close this issue when it is merged os-wasi Related to WASI variant of arch-wasm
Milestone

Comments

@filipw
Copy link
Contributor

filipw commented Sep 24, 2023

Description

Processing of WasiAfterRuntimeLoaded entries in WasiApp.Native.targets is invalid and does not compile.

Reproduction Steps

csproj:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>net8.0</TargetFramework>
    <RuntimeIdentifier>wasi-wasm</RuntimeIdentifier>
    <OutputType>Exe</OutputType>
    <PublishTrimmed>false</PublishTrimmed>
    <WasmSingleFileBundle>true</WasmSingleFileBundle>
    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
    <ImplicitUsings>enable</ImplicitUsings>
  </PropertyGroup>

  <ItemGroup>
    <WasiAfterRuntimeLoaded Include="attach_internal_calls" />
  </ItemGroup>
  
  <ItemGroup>
    <_WasiFilePathForFixup Include="attach_internal_calls.c" />
  </ItemGroup>

</Project>

attach_internal_calls.c:

#include <stdlib.h>

void attach_internal_calls()
{
}

Run

dotnet publish -c Release

Expected behavior

Builds correctly.

Actual behavior

/usr/local/share/dotnet/packs/Microsoft.NET.Runtime.WebAssembly.Wasi.Sdk/8.0.0-rc.1.23419.4/Sdk/WasiApp.Native.targets(468,26): error MSB4012: The expression "-D WASI_AFTER_RUNTIME_LOADED_DECLARATIONS="@(WasiAfterRuntimeLoadedDeclarations, ' ')"" cannot be used in this context. Item lists cannot be concatenated with other strings where an item list is expected. Use a semicolon to separate multiple item lists.

The problem is this list concat within a list.

    <ItemGroup>
      <WasiAfterRuntimeLoadedDeclarations Include="@(WasiAfterRuntimeLoaded->'void %(Identity)();')" />
      <WasiAfterRuntimeLoadedCalls Include="@(WasiAfterRuntimeLoaded->'%(Identity)();')" />
    </ItemGroup>

    <ItemGroup>
      <_WasiSdkClangArgs Condition="'@(WasiAfterRuntimeLoadedDeclarations)' != ''"
                         Include="-D WASI_AFTER_RUNTIME_LOADED_DECLARATIONS=&quot;@(WasiAfterRuntimeLoadedDeclarations, ' ')&quot;" />
      <_WasiSdkClangArgs Condition="'@(WasiAfterRuntimeLoadedCalls)' != ''"
                         Include="-D WASI_AFTER_RUNTIME_LOADED_CALLS=&quot;@(WasiAfterRuntimeLoadedCalls, ' ')&quot;" />
    </ItemGroup>

Regression?

Technically yes. This used to work in the old dotnet-wasi-sdk because it did not use item list to create Clang args

Known Workarounds

I am not an MsBuild expert, but this works (though I assume there is a more elegant workaround):

    <ItemGroup>
      <WasiAfterRuntimeLoadedDeclarations Include="@(WasiAfterRuntimeLoaded->'void %(Identity)();')" />
      <WasiAfterRuntimeLoadedCalls Include="@(WasiAfterRuntimeLoaded->'%(Identity)();')" />
    </ItemGroup>

    <PropertyGroup>
      <_WasiAfterRuntimeLoadedDeclarationsString>@(WasiAfterRuntimeLoadedDeclarations, ' ')</_WasiAfterRuntimeLoadedDeclarationsString>
      <_WasiAfterRuntimeLoadedCallsString>@(WasiAfterRuntimeLoadedCalls, ' ')</_WasiAfterRuntimeLoadedCallsString>
      <_WasiAfterRuntimeLoadedDeclarationsString>$(_WasiAfterRuntimeLoadedDeclarationsString.Replace('%0d%0a', '').Replace('%0a', '').Replace('%0d', ''))</_WasiAfterRuntimeLoadedDeclarationsString>
      <_WasiAfterRuntimeLoadedCallsString>$(_WasiAfterRuntimeLoadedCallsString.Replace('%0d%0a', '').Replace('%0a', '').Replace('%0d', ''))</_WasiAfterRuntimeLoadedCallsString>
    </PropertyGroup>

    <ItemGroup>
      <_WasiSdkClangArgs Condition="'$(_WasiAfterRuntimeLoadedDeclarationsString)' != ''" Include="-D WASI_AFTER_RUNTIME_LOADED_DECLARATIONS=&quot;$(_WasiAfterRuntimeLoadedDeclarationsString)&quot;" />
      <_WasiSdkClangArgs Condition="'$(_WasiAfterRuntimeLoadedCallsString)' != ''" Include="-D WASI_AFTER_RUNTIME_LOADED_CALLS=&quot;$(_WasiAfterRuntimeLoadedCallsString)&quot;" />
    </ItemGroup>

Basically instead of concatenating inside an item, it now happens outside as a property.
But the first projection creates a trailing new line, which needs to be replaced (or, again, perhaps there is a better method - that is why I decided to not PR this "fix").

Configuration

Tested with Microsoft.NET.Runtime.WebAssembly.Wasi.Sdk, version 8.0.0-rc.1.23419.4.

Other information

.NET SDK:
 Version:   8.0.100-rc.1.23463.5
 Commit:    e7f4de8816

Runtime Environment:
 OS Name:     Mac OS X
 OS Version:  13.5
 OS Platform: Darwin
 RID:         osx-arm64
 Base Path:   /usr/local/share/dotnet/sdk/8.0.100-rc.1.23463.5/

.NET workloads installed:
 [wasm-tools]
   Installation Source: SDK 8.0.100-rc.1
   Manifest Version:    8.0.0-rc.1.23419.4/8.0.100-rc.1
   Manifest Path:       /usr/local/share/dotnet/sdk-manifests/8.0.100-rc.1/microsoft.net.workload.mono.toolchain.current/8.0.0-rc.1.23419.4/WorkloadManifest.json
   Install Type:        FileBased

 [wasi-experimental]
   Installation Source: SDK 8.0.100-rc.1
   Manifest Version:    8.0.0-rc.1.23419.4/8.0.100-rc.1
   Manifest Path:       /usr/local/share/dotnet/sdk-manifests/8.0.100-rc.1/microsoft.net.workload.mono.toolchain.current/8.0.0-rc.1.23419.4/WorkloadManifest.json
   Install Type:        FileBased

 [wasm-experimental]
   Installation Source: SDK 8.0.100-rc.1
   Manifest Version:    8.0.0-rc.1.23419.4/8.0.100-rc.1
   Manifest Path:       /usr/local/share/dotnet/sdk-manifests/8.0.100-rc.1/microsoft.net.workload.mono.toolchain.current/8.0.0-rc.1.23419.4/WorkloadManifest.json
   Install Type:        FileBased


Host:
  Version:      8.0.0-rc.1.23419.4
  Architecture: arm64
  Commit:       92959931a3
  RID:          osx-arm64
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Sep 24, 2023
@radical radical added os-wasi Related to WASI variant of arch-wasm arch-wasm WebAssembly architecture labels Sep 25, 2023
@ghost
Copy link

ghost commented Sep 25, 2023

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

Issue Details

Description

Processing of WasiAfterRuntimeLoaded entries in WasiApp.Native.targets is invalid and does not compile.

Reproduction Steps

csproj:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>net8.0</TargetFramework>
    <RuntimeIdentifier>wasi-wasm</RuntimeIdentifier>
    <OutputType>Exe</OutputType>
    <PublishTrimmed>false</PublishTrimmed>
    <WasmSingleFileBundle>true</WasmSingleFileBundle>
    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
    <ImplicitUsings>enable</ImplicitUsings>
  </PropertyGroup>

  <ItemGroup>
    <WasiAfterRuntimeLoaded Include="attach_internal_calls" />
  </ItemGroup>
  
  <ItemGroup>
    <_WasiFilePathForFixup Include="attach_internal_calls.c" />
  </ItemGroup>

</Project>

attach_internal_calls.c:

#include <stdlib.h>

void attach_internal_calls()
{
}

Run

dotnet publish -c Release

Expected behavior

Builds correctly.

Actual behavior

/usr/local/share/dotnet/packs/Microsoft.NET.Runtime.WebAssembly.Wasi.Sdk/8.0.0-rc.1.23419.4/Sdk/WasiApp.Native.targets(468,26): error MSB4012: The expression "-D WASI_AFTER_RUNTIME_LOADED_DECLARATIONS="@(WasiAfterRuntimeLoadedDeclarations, ' ')"" cannot be used in this context. Item lists cannot be concatenated with other strings where an item list is expected. Use a semicolon to separate multiple item lists.

The problem is this list concat within a list.

    <ItemGroup>
      <WasiAfterRuntimeLoadedDeclarations Include="@(WasiAfterRuntimeLoaded->'void %(Identity)();')" />
      <WasiAfterRuntimeLoadedCalls Include="@(WasiAfterRuntimeLoaded->'%(Identity)();')" />
    </ItemGroup>

    <ItemGroup>
      <_WasiSdkClangArgs Condition="'@(WasiAfterRuntimeLoadedDeclarations)' != ''"
                         Include="-D WASI_AFTER_RUNTIME_LOADED_DECLARATIONS=&quot;@(WasiAfterRuntimeLoadedDeclarations, ' ')&quot;" />
      <_WasiSdkClangArgs Condition="'@(WasiAfterRuntimeLoadedCalls)' != ''"
                         Include="-D WASI_AFTER_RUNTIME_LOADED_CALLS=&quot;@(WasiAfterRuntimeLoadedCalls, ' ')&quot;" />
    </ItemGroup>

Regression?

Technically yes. This used to work in the old dotnet-wasi-sdk because it did not use item list to create Clang args

Known Workarounds

I am not an MsBuild expert, but this works (though I assume there is a more elegant workaround):

    <ItemGroup>
      <WasiAfterRuntimeLoadedDeclarations Include="@(WasiAfterRuntimeLoaded->'void %(Identity)();')" />
      <WasiAfterRuntimeLoadedCalls Include="@(WasiAfterRuntimeLoaded->'%(Identity)();')" />
    </ItemGroup>

    <PropertyGroup>
      <_WasiAfterRuntimeLoadedDeclarationsString>@(WasiAfterRuntimeLoadedDeclarations, ' ')</_WasiAfterRuntimeLoadedDeclarationsString>
      <_WasiAfterRuntimeLoadedCallsString>@(WasiAfterRuntimeLoadedCalls, ' ')</_WasiAfterRuntimeLoadedCallsString>
      <_WasiAfterRuntimeLoadedDeclarationsString>$(_WasiAfterRuntimeLoadedDeclarationsString.Replace('%0d%0a', '').Replace('%0a', '').Replace('%0d', ''))</_WasiAfterRuntimeLoadedDeclarationsString>
      <_WasiAfterRuntimeLoadedCallsString>$(_WasiAfterRuntimeLoadedCallsString.Replace('%0d%0a', '').Replace('%0a', '').Replace('%0d', ''))</_WasiAfterRuntimeLoadedCallsString>
    </PropertyGroup>

    <ItemGroup>
      <_WasiSdkClangArgs Condition="'$(_WasiAfterRuntimeLoadedDeclarationsString)' != ''" Include="-D WASI_AFTER_RUNTIME_LOADED_DECLARATIONS=&quot;$(_WasiAfterRuntimeLoadedDeclarationsString)&quot;" />
      <_WasiSdkClangArgs Condition="'$(_WasiAfterRuntimeLoadedCallsString)' != ''" Include="-D WASI_AFTER_RUNTIME_LOADED_CALLS=&quot;$(_WasiAfterRuntimeLoadedCallsString)&quot;" />
    </ItemGroup>

Basically instead of concatenating inside an item, it now happens outside as a property.
But the first projection creates a trailing new line, which needs to be replaced (or, again, perhaps there is a better method - that is why I decided to not PR this "fix").

Configuration

Tested with Microsoft.NET.Runtime.WebAssembly.Wasi.Sdk, version 8.0.0-rc.1.23419.4.

Other information

.NET SDK:
 Version:   8.0.100-rc.1.23463.5
 Commit:    e7f4de8816

Runtime Environment:
 OS Name:     Mac OS X
 OS Version:  13.5
 OS Platform: Darwin
 RID:         osx-arm64
 Base Path:   /usr/local/share/dotnet/sdk/8.0.100-rc.1.23463.5/

.NET workloads installed:
 [wasm-tools]
   Installation Source: SDK 8.0.100-rc.1
   Manifest Version:    8.0.0-rc.1.23419.4/8.0.100-rc.1
   Manifest Path:       /usr/local/share/dotnet/sdk-manifests/8.0.100-rc.1/microsoft.net.workload.mono.toolchain.current/8.0.0-rc.1.23419.4/WorkloadManifest.json
   Install Type:        FileBased

 [wasi-experimental]
   Installation Source: SDK 8.0.100-rc.1
   Manifest Version:    8.0.0-rc.1.23419.4/8.0.100-rc.1
   Manifest Path:       /usr/local/share/dotnet/sdk-manifests/8.0.100-rc.1/microsoft.net.workload.mono.toolchain.current/8.0.0-rc.1.23419.4/WorkloadManifest.json
   Install Type:        FileBased

 [wasm-experimental]
   Installation Source: SDK 8.0.100-rc.1
   Manifest Version:    8.0.0-rc.1.23419.4/8.0.100-rc.1
   Manifest Path:       /usr/local/share/dotnet/sdk-manifests/8.0.100-rc.1/microsoft.net.workload.mono.toolchain.current/8.0.0-rc.1.23419.4/WorkloadManifest.json
   Install Type:        FileBased


Host:
  Version:      8.0.0-rc.1.23419.4
  Architecture: arm64
  Commit:       92959931a3
  RID:          osx-arm64
Author: filipw
Assignees: -
Labels:

arch-wasm, untriaged, area-Build-mono, os-wasi

Milestone: -

@lambdageek lambdageek added this to the 8.0.0 milestone Sep 27, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Sep 27, 2023
@lewing lewing modified the milestones: 8.0.0, 9.0.0 May 16, 2024
@ilonatommy ilonatommy self-assigned this Jul 8, 2024
@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 Jul 8, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Aug 9, 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 in-pr There is an active PR which will close this issue when it is merged os-wasi Related to WASI variant of arch-wasm
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants