[wasm coreclr] Prepare for running tests #119807

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.

Sign up for GitHub

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

Jump to bottom

Draft

radekdoulik wants to merge 12 commits into dotnet:main from radekdoulik:clr-interp-wasm-tests-changes

+117 −7

Member

radekdoulik commented Sep 17, 2025

Get further with running interpreter tests.

Fix ICU loading from development hosts

Update wasm calli stubs to handle instance methods

radekdoulik added 2 commits

September 17, 2025 13:37


          Load ICU data on wasm

245f7f1


          Handle instance signatures

4de3190

radekdoulik added this to the Future milestone

radekdoulik added arch-wasm area-VM-coreclr labels

dotnet-policy-service bot assigned radekdoulik

Member Author

radekdoulik commented Sep 17, 2025

With these changes we now fail with

RuntimeError: memory access out of bounds
    at corerun.wasm.DispatchToken::DecodeTypeID(unsigned long) (wasm://wasm/corerun.wasm-1b61dba6:wasm-function[14976]:0x580515)
    at corerun.wasm.DispatchToken::GetTypeID() const (wasm://wasm/corerun.wasm-1b61dba6:wasm-function[14960]:0x57e1f9)
    at corerun.wasm.DispatchToken::IsThisToken() const (wasm://wasm/corerun.wasm-1b61dba6:wasm-function[14974]:0x580158)
    at corerun.wasm.VirtualCallStubManager::GetTarget(DispatchToken, MethodTable*, int) (wasm://wasm/corerun.wasm-1b61dba6:wasm-function[14978]:0x5807fa)
    at corerun.wasm.MethodTable::GetMethodDescForInterfaceMethod(TypeHandle, MethodDesc*, int) (wasm://wasm/corerun.wasm-1b61dba6:wasm-function[12094]:0x47a2b1)
    at corerun.wasm.MethodTable::TryResolveConstraintMethodApprox(TypeHandle, MethodDesc*, int*) (wasm://wasm/corerun.wasm-1b61dba6:wasm-function[12417]:0x49eb13)
    at corerun.wasm.CEEInfo::getCallInfo(CORINFO_RESOLVED_TOKEN*, CORINFO_RESOLVED_TOKEN*, CORINFO_METHOD_STRUCT_*, CORINFO_CALLINFO_FLAGS, CORINFO_CALL_INFO*) (wasm://wasm/corerun.wasm-1b61dba6:wasm-function[10558]:0x3e2548)
    at corerun.wasm.InterpCompiler::EmitCall(CORINFO_RESOLVED_TOKEN*, bool, bool, bool, bool) (wasm://wasm/corerun.wasm-1b61dba6:wasm-function[24442]:0xb4dbbe)
    at corerun.wasm.InterpCompiler::GenerateCode(CORINFO_METHOD_INFO*) (wasm://wasm/corerun.wasm-1b61dba6:wasm-function[24409]:0xb41bcb)
    at corerun.wasm.InterpCompiler::CompileMethod() (wasm://wasm/corerun.wasm-1b61dba6:wasm-function[24405]:0xb3797e)

in the interpreter compiler. It happens during compilation of System.SpanHelpers:IndexOfAnyInRange[char](byref,char,char,int)


          Progress with EH on wasm

f566b4a

radekdoulik mentioned this pull request

Progress with EH on wasm #119850

Merged

radekdoulik added 4 commits

September 18, 2025 19:24


          Feedback

3a0f482


          Revert "Feedback"

4ac4b0d

This reverts commit 3a0f482.


          Revert "Progress with EH on wasm"

a3ca1d0

This reverts commit f566b4a.


          Fix build

737f8a5

AaronRobinsonMSFT reviewed

View reviewed changes

src/coreclr/hosts/corerun/corerun.cpp

    
                      std::fprintf(stderr, "Failed to load the ICU data\n");

                      return -1;

                  }

              #endif

Member

AaronRobinsonMSFT Sep 23, 2025

Suggested change

      
            #endif
          
            #endif // TARGET_WASM

src/coreclr/hosts/corerun/corerun.wasm.hpp

    
              #define __CORERUN_WASM_HPP__

              void wasm_add_pinvoke_override();

              int32_t wasm_load_icu_data(const char* assemblyPath);

Member

AaronRobinsonMSFT Sep 23, 2025

Please update this to be bool instead of int32_t.

src/coreclr/hosts/corerun/corerun.wasm.cpp

    
                  return icuPath;

              }

              int32_t wasm_load_icu_data(const char* assemblyPath)

Member

AaronRobinsonMSFT Sep 23, 2025

I realize this is probably copied from somewhere, but it is using very low leve APIs. For example, open vs fopen. Can we instead clean this up to be as C++ as possible?

src/coreclr/hosts/corerun/corerun.wasm.cpp

    
              extern "C" int32_t mono_wasm_load_icu_data(const void* pData);

              static char* _wasm_get_icu_dat_file_path(const char* assemblyPath)

Member

AaronRobinsonMSFT Sep 23, 2025

Suggested change

      
            static char* _wasm_get_icu_dat_file_path(const char* assemblyPath)
          
            static char* wasm_get_icu_dat_file_path(const char* assemblyPath)

src/coreclr/hosts/corerun/corerun.wasm.cpp

    
              int32_t wasm_load_icu_data(const char* assemblyPath)

              {

                  char* icuFile = _wasm_get_icu_dat_file_path(assemblyPath);

Member

AaronRobinsonMSFT Sep 23, 2025

Suggested change

      
                char* icuFile = _wasm_get_icu_dat_file_path(assemblyPath);
          
                char* icuFile = wasm_get_icu_dat_file_path(assemblyPath);

src/coreclr/hosts/corerun/corerun.wasm.cpp

    
                      return strdup(icuFileName);

                  size_t dirLen = (size_t)(lastSeparator - assemblyPath) + 1; // include separator

                  size_t fileNameLen = sizeof(icuFileName) - 1;

Member

AaronRobinsonMSFT Sep 23, 2025

Suggested change

      
                size_t fileNameLen = sizeof(icuFileName) - 1;
          
                size_t fileNameLen = ARRAY_SIZE(icuFileName) - 1;

Using sizeof is too easy to do wrong. All someone needs to do is change that to const char icuFileName* and it will compile and be wrong.

Member

AaronRobinsonMSFT Sep 23, 2025

Remove the -1 and then you can remove the +1 and icuPath[dirLen + fileNameLen] = '\0';. Doing that and making a comment is all that is needed.

src/coreclr/hosts/corerun/corerun.wasm.cpp

    
              static char* _wasm_get_icu_dat_file_path(const char* assemblyPath)

              {

                  if (!assemblyPath || !*assemblyPath)

Member

AaronRobinsonMSFT Sep 23, 2025

When will this be NULL?

src/coreclr/hosts/corerun/corerun.wasm.cpp

    
                  }

                  close(fd);

                  return mono_wasm_load_icu_data(buffer);

Member

AaronRobinsonMSFT Sep 23, 2025

If this returns failure, we should clean things up.

src/coreclr/hosts/corerun/corerun.wasm.cpp

    
                      return 0;

                  }

                  struct stat st;

Member

AaronRobinsonMSFT Sep 23, 2025

Suggested change

      
                struct stat st;
          
               stat st;

radekdoulik added 2 commits

September 23, 2025 16:19


          Disable fat tokes temporarily and update VirtualCallStubManager::Reso…

b51566f

…lver

To work with protable entrypoints


          Add calli helper for GlobalizationNative_GetLocaleInfoString

4185cad

This was referenced Sep 23, 2025

The Operation will be canceled. The next steps may not contain expected logs. dotnet/dnceng#3008

Open

Unable to pull image from mcr.microsoft.com #117164

Open

radekdoulik added 3 commits

September 25, 2025 11:38


          Merge branch 'main' into clr-interp-wasm-tests-changes

9acd730


          Fix build after merge

e5c7472


          Fix non-working asserts

df7cda4

build-analysis bot mentioned this pull request

AF: *(_UNCHECKED_OBJECTREF *)handle == NULL (HndCreateHandle called by getJitHandleForObject) #117138

Open

AaronRobinsonMSFT reviewed

View reviewed changes

src/coreclr/vm/wasm/cgencpu.h

Comment on lines +35 to +36

    
                  // WASM-TODO: check where we reach that, return nullptr for now to not break runtime initialization

                  //PORTABILITY_ASSERT("The function is not implemented on wasm, it lacks registers");

Member

AaronRobinsonMSFT Sep 25, 2025

How are we getting here? Can you share a callstack? I would prefer us to nicely ifdef out the caller in an intelligent way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arch-wasm area-VM-coreclr