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] Memory corruption in aot crosscompiler #56526

Closed
radekdoulik opened this issue Jul 29, 2021 · 3 comments
Closed

[wasm] Memory corruption in aot crosscompiler #56526

radekdoulik opened this issue Jul 29, 2021 · 3 comments
Assignees
Labels
arch-wasm WebAssembly architecture area-Codegen-LLVM-mono
Milestone

Comments

@radekdoulik
Copy link
Member

When running dotnet/performance microbenchmarks, the aot crosscompiler fails with:

/home/helixbot/work/9BB2089B/p/dotnet-wasm/src/mono/wasm/build/WasmApp.Native.targets(447,5): error : Precompiling failed for /home/helixbot/work/9BB2089B/p/performance/artifacts/bin/MicroBenchmarks/Release/net6.0/Job-JTJFRQ/bin/net6.0/browser-wasm/publish/MessagePack.dll: free(): invalid pointer [/home/helixbot/work/9BB2089B/p/performance/artifacts/bin/MicroBenchmarks/Release/net6.0/Job-JTJFRQ/BenchmarkDotNet.Autogenerated.csproj]
@radekdoulik radekdoulik self-assigned this Jul 29, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Jul 29, 2021
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@radekdoulik radekdoulik added this to the 6.0.0 milestone Jul 29, 2021
@radekdoulik radekdoulik added arch-wasm WebAssembly architecture area-Codegen-LLVM-mono labels Jul 29, 2021
@ghost
Copy link

ghost commented Jul 29, 2021

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

Issue Details

When running dotnet/performance microbenchmarks, the aot crosscompiler fails with:

/home/helixbot/work/9BB2089B/p/dotnet-wasm/src/mono/wasm/build/WasmApp.Native.targets(447,5): error : Precompiling failed for /home/helixbot/work/9BB2089B/p/performance/artifacts/bin/MicroBenchmarks/Release/net6.0/Job-JTJFRQ/bin/net6.0/browser-wasm/publish/MessagePack.dll: free(): invalid pointer [/home/helixbot/work/9BB2089B/p/performance/artifacts/bin/MicroBenchmarks/Release/net6.0/Job-JTJFRQ/BenchmarkDotNet.Autogenerated.csproj]
Author: radekdoulik
Assignees: radekdoulik
Labels:

arch-wasm, area-Codegen-LLVM-mono, untriaged

Milestone: 6.0.0

@radekdoulik radekdoulik removed the untriaged New issue has not been triaged by the area owner label Jul 29, 2021
radekdoulik added a commit to radekdoulik/runtime that referenced this issue Jul 29, 2021
Fixes dotnet#56526 and probably also
dotnet#53546

Allocate enough memory when `SIZEOF_REGISTER == 4`, so that code like

    defs [ins->dreg + 1] = NULL;
    defs [ins->dreg + 2] = NULL;

doesn't write after allocated range.
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jul 29, 2021
radekdoulik added a commit that referenced this issue Jul 29, 2021
Fixes #56526 and probably also
#53546

Allocate enough memory when `SIZEOF_REGISTER == 4`, so that code like

    defs [ins->dreg + 1] = NULL;
    defs [ins->dreg + 2] = NULL;

doesn't write after allocated range.
@radekdoulik
Copy link
Member Author

Fixed by #56535

@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jul 29, 2021
Lxiamail added a commit that referenced this issue Aug 5, 2021
* bring up WASM AOT microbenchmarks run

* bring up WASM AOT microbenchmarks run

* adding diagnostice log for testing

* commend out some jobs while testing to save lab run resource

* commend out some jobs while testing to save lab run resource

* Add WASM AOT in not scheduled run

* Only run selected jobs

* add "ls -l" to print out permission before calling performance-setup.sh

* add "ls -l" to print out permission before calling performance-setup.sh

* Add "--buildTimeout 3600" to benchmarkdotnet to extend default timeout time. Adjust -aotcompilermode format to "--aotcompilermode=wasm".

* Add "--buildTimeout 3600" to benchmarkdotnet to extend default timeout time. Adjust -aotcompilermode format to "--aotcompilermode=wasm".

* Correct runtimeSrcDir value

* Correct --runtimeSrcDir location to $HELIX_CORRELATION_PAYLOAD

* Correct --runtimeSrcDir location to $HELIX_CORRELATION_PAYLOAD

* Remove not used parameters in WASM AOT mode and  add temperary diagnostic info

* Remove not used parameters in WASM AOT mode and  add temperary diagnostic info

* make \$HELIX_CORRELATION_PAYLOAD/dotnet-wasm to be the root directory of runtime repo source

* Only copy runtime repo source directory to helix payload when it's wasm aot

* Add diagnosic log for testing only

* omite unneeded runtime repo file from helix payload

* omite unneeded runtime repo file from helix payload

* use rsync instead of cp exclusion command since cp doesn't work with sub directory as expected.

* Add temp diagnostic logging, should be removed before PR.

* remove artifacts/obj from helix payload

* remove artifacts/obj from helix payload

* copy back helix job for diagnosis and silent rsync

* For testing only, clone performance repor alicial/hikeMicro-benchmark private branch, which contains copying back helix job folder for diagnosis

* enable rsync verbose for diagnosis

* add --keepfiles for wasm intepreter as well for diagnosis

* Remove temporary diagnostic code

* recover all the jobs in the pipeline

* recover all the jobs in the pipeline

* copy wasm build drop to the location that aot build expects

* using private perf branch to enable binlog for benchmarkdotnet for diagnosis

* comment out none wasm runs temporarily for testing only

* comment out none wasm runs temporarily for testing only

* add "--keepfiles" to keep diagnostic files

* fixed $wasm_dotnet_path/artifacts/BrowserWasm/artifacts path

* instead of copy, move $wasm_dotnet_path/artifacts/BrowserWasm/artifacts/* to save space

* benchmarkdotnet using --cli \$HELIX_CORRELATION_PAYLOAD/dotnet-wasm/dotnet.sh instead

* benchmarkdotnet using --cli \$HELIX_CORRELATION_PAYLOAD/dotnet-wasm/dotnet.sh instead

* add executable permission

* Enable other non run to compare

* Enable other non run to compare

* remove "--cli \$HELIX_CORRELATION_PAYLOAD/dotnet-wasm/dotnet.sh"

* fixes per CR

* copy EMSDK_PATH to expected location in helix payload

* install emsdk

* fix emsdk path

* minor fix

* removed more not used files to reduce helix payload size

* remove more jobs for testing purpose

* Fix the download path

* Add BenchmarkDotNet sources to the payload

* Fix __download__ path

* set BenchmarkDotNetSources to use benchmarkdotnet built from source

* use benchmarkdotnet private branch:alicial/tempwasmaot for BenchmarkDotNetSources

* use benchmarkdonet private branch with BenchmarkDotNetSources option

* Debug prints

* Clone B.NET to different location

Fix rsync exclusion
Add more debug prints

* Move B.NET back, clone upstream B.NET

To avoid:

    [INFO] ERROR(S):
    [INFO]   Option 'BenchmarkDotNetSources' is unknown.

* Use my branch for performance repo

* Revert "set BenchmarkDotNetSources to use benchmarkdotnet built from source"

This reverts commit bccfadd.

To avoid:

    [INFO] ERROR(S):
    [INFO]   Option 'BenchmarkDotNetSources' is unknown.

* Disable one more job I missed in merge

* Switch back to nuget packages for BenchmarkDotNet

* tempararily use pr-alicias-branch to test new fixes

* [mono] Fix invalid memory write

Fixes #56526 and probably also
#53546

Allocate enough memory when `SIZEOF_REGISTER == 4`, so that code like

    defs [ins->dreg + 1] = NULL;
    defs [ins->dreg + 2] = NULL;

doesn't write after allocated range.

* [mono] Fix an uninitialized memory access in the ABCREM pass.

Detected by valgrind.

* clean up PR

* add missing param

* fix perf.yml and make wasm interpreter to be consistent with wasm AOT

* comment out more job for fast testing

* remove extra job

* exclude copying __download__ dir instead of delete it.

* add parameters.codeGenType

* fix aot javascriptengine

* correct livelibrariesbuildconfig from release to Release

* install emsdk before copy $source_directory

* add back all the temporarily commented out jobs in perf.yml

* clean up work around code

* minor clean up

* remove performance submodule

* switch back to use performance repo main branch

* add --quiet to git clone

Co-authored-by: Radek Doulik <radekdoulik@gmail.com>
Co-authored-by: Zoltan Varga <vargaz@gmail.com>
@ghost ghost locked as resolved and limited conversation to collaborators Aug 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-Codegen-LLVM-mono
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant