Add an implicit argument coercion check. by sandreenko · Pull Request #43386 · dotnet/runtime

sandreenko · 2020-10-14T04:35:56Z

Check what types we are passing to a call and return BADCODE if such implicit conversion is not allowed by ECMA (Table III.9: Signature Matching ).

It fixes some undefined behavior (for example, passing long as int on x86 leads to a read of non-defined register).

The change includes fixes for tests where we used such IL and a fix for VM (thanks @jkoritzinsky, who was guiding me through VM changes).

Two implicit conventions, not allowed by ECMA, were tolerated:

int8->nint on x64, because we have tolerated it on desktop and because Jit's type system does not allow us to catch it, BADCODE on x86.
byref->nint, we had a discussion about it with @janvorli and @jkoritzinsky and decided to postpone this check (and fixes for it) for later so it does not block this PR (and arm64 work)

PTAL @CarolEidt @dotnet/jit-contrib

Note that it is a breaking change, cc @jkotas, @jeffschwMSFT

Fixes#43342, unblocks #43130

jkotas · 2020-10-15T00:36:19Z

byref->nint

This came up before. This mistake seems to be pretty prevalent in the code out there. I think we should keep allowing it.

sandreenko · 2020-10-15T06:18:22Z

ci failures are caused by #43412

sandreenko · 2020-10-28T23:43:58Z

PTAL @dotnet/jit-contrib

I am not familiar with needs-breaking-change-doc-created label, could somebody please explain what is it for and what should be done?

webczat · 2020-10-30T20:32:42Z

hmm, does that fix a case where you make a Dynamic method, trying to pass an int to a method expecting Object, and forget to box or box incorrectly?

sandreenko · 2020-10-30T20:50:17Z

hmm, does that fix a case where you make a Dynamic method, trying to pass an int to a method expecting Object, and forget to box or box incorrectly?

Kind of... There are 2 issues:

ECMA tolerates passing a native int as byref:

and because JIT does not distinguish nint from int on 32-bit platforms such code will be accepted on x86, but rejected on x64.

webczat · 2020-10-30T21:01:39Z

so probably not exactly my issue, I'm x64

sandreenko · 2020-11-03T05:31:26Z

Could somebody please take a look, @dotnet/jit-contrib ?

kunalspathak

LGTM

AndyAyersMS · 2020-11-03T19:02:00Z

Seems like these are generally useful and should be in a header somewhere?

We have GenTree::TypeIs and it is usually sufficient, we don't have many places where we compare types not from tree nodes.
I did not want to make this function general available because it is a little confusing that the first argument is special there, but I am open to other opinions. I thought it would be nice to make it a member function, like var_types::TypeIs(this, types to compare this with) but var_types is not a class, it is an enum.

sandreenko · 2020-11-06T05:32:36Z

I have fixed another test and rerun the tests, the failures are unrelated.

author Stephen Toub <stoub@microsoft.com> 1604601164 -0500 committer Tammy Qiu <tammy.qiu@yahoo.com> 1604960878 -0500 Add stream conformance tests for TranscodingStream (dotnet#44248) * Add stream conformance tests for TranscodingStream * Special-case 0-length input buffers to TranscodingStream.Write{Async} The base implementation of Encoder.Convert doesn't like empty inputs. Regardless, if the input is empty, we can avoid a whole bunch of unnecessary work. JIT: minor inliner refactoring (dotnet#44215) Extract out the budget check logic so it can vary by inlining policy. Use this to exempt the FullPolicy from budget checking. Fix inline xml to dump the proper (full name) hash for inlinees. Update range dumper to dump ranges in hex. Remove unused QCall for WinRTSupported (dotnet#44278) ConcurrentQueueSegment allows spinning threads to sleep. (dotnet#44265) * Allow threads to sleep when ConcurrentQueue has many enqueuers/dequeuers. * Update src/libraries/System.Private.CoreLib/src/System/Collections/Concurrent/ConcurrentQueueSegment.cs Co-authored-by: Stephen Toub <stoub@microsoft.com> * Apply suggestions from code review Co-authored-by: Stephen Toub <stoub@microsoft.com> Co-authored-by: AMD DAYTONA EPYC <amd@amd-DAYTONA-X0.com> Co-authored-by: Stephen Toub <stoub@microsoft.com> File.Exists() is not null when true (dotnet#44310) * File.Exists() is not null when true * Fix compile * Fix compile 2 [master][watchOS] Add simwatch64 support (dotnet#44303) Xcode 12.2 removed 32 bits support for watchOS simulators, this PR helps to fixdotnet/macios#9949, we have tested the new binaries and they are working as expected ![unknown](https://user-images.githubusercontent.com/204671/98253709-64413200-1f49-11eb-9774-8c5aa416fc57.png) Co-authored-by: dalexsoto <dalexsoto@users.noreply.github.com> Implementing support to Debugger::Break. (dotnet#44305) Set fgOptimizedFinally flag correctly (dotnet#44268) - Initialize to 0 at compiler startup - Set flag when finally cloning optimization kicks in Fixes non-deterministic generation of nop opcodes into ARM32 code Forbid `- byref cnst` -> `+ (byref -cnst)` transformation. (dotnet#44266) * Add a repro test. * Forbid the transformation for byrefs. * Update src/coreclr/src/jit/morph.cpp Co-authored-by: Andy Ayers <andya@microsoft.com> * Update src/coreclr/src/jit/morph.cpp * Fix the test return value. WriteLine is just to make sure we don't delete the value. * improve the test. avoid a possible overflow and don't waste time on printing. Co-authored-by: Andy Ayers <andya@microsoft.com> Pick libmonosgen-2.0.so from cmake install directory instead of .libs (dotnet#44291) This aligns Linux with what we already do for all the other platforms. Update SharedPerformanceCounter assert (dotnet#44333) Remove silly ToString in GetCLRInstanceString (dotnet#44335) Use targetPlatformMoniker for net5.0 and newer tfms (dotnet#43965) * Use targetPlatformMoniker for net5.0 and newer tfms * disabling analyzer, update version to 0.0, and use new format. * update the targetFramework.sdk * removing supportedOS assembly level attribute * fix linker errors and addressing feedback * making _TargetFrameworkWithoutPlatform as private [sgen] Add Ward annotations to sgen_get_total_allocated_bytes (dotnet#43833) Attempt to fix https://jenkins.mono-project.com/job/test-mono-mainline-staticanalysis/ Co-authored-by: lambdageek <lambdageek@users.noreply.github.com> [tests] Re-enable tests fixed by dotnet#44081 (dotnet#44212) Fixesmono/mono#15030 and fixesmono/mono#15031 and fixesmono/mono#15032 Add an implicit argument coercion check. (dotnet#43386) * Add `impCheckImplicitArgumentCoercion`. * Fix tests with type mismatch. * Try to fix VM signature. * Allow to pass byref as native int. * another fix. * Fix another IL test. [mono] Change CMakelists.txt "python" -> Python3_EXECUTABLE (dotnet#44340) Debian doesn't install a "python" binary for python3. Tweak StreamConformanceTests for cancellation (dotnet#44342) - Avoid unnecessary timers - Separate tests for precancellation, ReadAsync(byte[], ...) cancellation, and ReadAsync(Memory, ...) cancellation Use Dictionary for underlying cache of ResourceSet (dotnet#44104) Simplify catch-rethrow logic in NetworkStream (dotnet#44246) A follow-up on dotnet#40772 (comment), simplifies and harmonizes the way we wrap exceptions into IOException. Having one catch block working with System.Exception seems to be enough here, no need for specific handling of SocketException. Simple GT_NEG optimization for dotnet#13837 (dotnet#43921) * Simple arithmetic optimization with GT_NEG * Skip GT_NEG optimization when an operand is constant. Revert bitwise rotation pattern * Fixed Value Numbering assert * Cleaned up code and comments for simple GT_NEG optimization * Formatting Co-authored-by: Julie Lee <jeonlee@microsoft.com> [master] Update dependencies from mono/linker (dotnet#44322) * Update dependencies from https://github.com/mono/linker build 20201105.1 Microsoft.NET.ILLink.Tasks From Version 6.0.0-alpha.1.20527.2 -> To Version 6.0.0-alpha.1.20555.1 * Update dependencies from https://github.com/mono/linker build 20201105.2 Microsoft.NET.ILLink.Tasks From Version 6.0.0-alpha.1.20527.2 -> To Version 6.0.0-alpha.1.20555.2 * Disable new optimization for libraries mode (it cannot work in this mode) Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: Marek Safar <marek.safar@gmail.com> Tighten argument validation in StreamConformanceTests (dotnet#44326) Add threshold on number of files / partition in SPMI collection (dotnet#44180) * Add check for files count * Fix the OS check * decrese file limit to 1500: * misc fix * Do not upload to azure if mch files are zero size Fix ELT profiler tests (dotnet#44285) [master] Update dependencies from dotnet/arcade dotnet/llvm-project dotnet/icu (dotnet#44336) [master] Update dependencies from dotnet/arcade dotnet/llvm-project dotnet/icu - Merge branch 'master' into darc-master-2211df94-2a02-4c3c-abe1-e3534e896267 Fix Send_TimeoutResponseContent_Throws (dotnet#44356) If the client times out too quickly, the server may never have a connection to accept and will hang forever. Match CoreCLR behaviour on thread start failure (dotnet#44124) Co-authored-by: Aleksey Kliger (λgeek) <akliger@gmail.com> Add slash in Windows SoD tool build (dotnet#44359) * Add slash in Windows SoD tool build * Update SoD search path to match output dir * Fixup dotnet version * Remove merge commit headers * Disable PRs Co-authored-by: Drew Scoggins <andrew.g.scoggins@gmail> Reflect test path changes in .gitattributes; remove nonexistent files (dotnet#44371) Bootstrapping a test for R2RDump (dotnet#42150) Improve performance of Enum's generic IsDefined / GetName / GetNames (dotnet#44355) Eliminates the boxing in IsDefined/GetName/GetValues, and in GetNames avoids having to go through RuntimeType's GetEnumNames override. clarify http version test (dotnet#44379) Co-authored-by: Geoffrey Kizer <geoffrek@windows.microsoft.com> Update dependencies from https://github.com/mono/linker build 20201106.1 (dotnet#44367) Microsoft.NET.ILLink.Tasks From Version 6.0.0-alpha.1.20555.2 -> To Version 6.0.0-alpha.1.20556.1 Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> Disable RunThreadLocalTest8_Values on Mono (dotnet#44357) * Disable RunThreadLocalTest8_Values on Mono It's failing on SLES * fix typo LongProcessNamesAreSupported: make test work on distros where sleep is a symlink/script (dotnet#44299) * LongProcessNamesAreSupported: make test work on distros where sleep is a symlink/script * PR feedback Co-authored-by: Stephen Toub <stoub@microsoft.com> * fix compilation Co-authored-by: Stephen Toub <stoub@microsoft.com> add missing constructor overloads (dotnet#44380) Co-authored-by: Geoffrey Kizer <geoffrek@windows.microsoft.com> change using in ConnectCallback_UseUnixDomainSocket_Success (dotnet#44366) Clean up the samples (dotnet#44293) Update dotnet/roslyn issue link Delete stale comment about dotnet/roslyn#30797 Fix/remove TODO-NULLABLEs (dotnet#44300) * Fix/remove TODO-NULLABLEs * remove redundant ! * apply Jozkee's feedback * address feedback Update glossary (dotnet#44274) Co-authored-by: Juan Hoyos <juan.hoyos@microsoft.com> Co-authored-by: Stephen Toub <stoub@microsoft.com> Co-authored-by: Günther Foidl <gue@korporal.at> Add files need for wasm executable relinking/aot to the wasm runtime pack. (dotnet#43785) Co-authored-by: Alexander Köplinger <alex.koeplinger@outlook.com> Move some more UnmanagedCallersOnly tests to IL now that they're invalid C# (dotnet#43366) Fix C++ build for mono/metadata/threads.c (dotnet#44413) `throw` is a reserved keyword in C++. Disable a failing test. (dotnet#44404) Change async void System.Text.Json test to be async Task (dotnet#44418) Improve crossgen2 comparison jobs (dotnet#44119) - Fix compilation on unix platforms - Wrap use of wildcard in quotes - Print better display name into log - Fix X86 constant comparison handling - Add ability to compile specific overload via single method switches Remove some unnecessary GetTypeInfo usage (dotnet#44414) Fix MarshalTypedArrayByte and re-enable it. Re-enable TestFunctionApply

kunalspathak · 2021-10-01T18:10:57Z

Added breaking change docs: dotnet/docs#26346

sandreenko added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Oct 14, 2020

runfoappBot mentioned this pull request Oct 14, 2020
System.Net.Security.Tests timing out #43403
Closed

sandreenko force-pushed the implicitArgumentCoercionCheck branch from 86dc74e to 2959b74Compare October 14, 2020 23:28

sandreenko closed this Oct 14, 2020

sandreenko reopened this Oct 14, 2020

sandreenko marked this pull request as ready for review October 15, 2020 00:09

runfoappBot mentioned this pull request Oct 15, 2020
Bundle_Is_Extracted test failing intermittently on Linux #43316
Closed

jeffschwMSFT added the breaking-change Issue or PR that represents a breaking API or functional change over a previous release. label Oct 15, 2020

ghost added the needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet label Oct 15, 2020

sandreenko force-pushed the implicitArgumentCoercionCheck branch from 75a3b4a to ed005c3Compare October 28, 2020 19:20

kunalspathak approved these changes Nov 3, 2020
View reviewed changes

AndyAyersMS approved these changes Nov 3, 2020
View reviewed changes

sandreenko force-pushed the implicitArgumentCoercionCheck branch from ed005c3 to 3213c95Compare November 4, 2020 08:30

sandreenko mentioned this pull request Nov 5, 2020
Forbid - byref cnst -> + (byref -cnst) transformation. #44266
Merged

Sergey Andreenko added 6 commits November 5, 2020 16:26

Add impCheckImplicitArgumentCoercion.
d54e56f

Fix tests with type mismatch.
b7d7d77

Try to fix VM signature.
8158156

Allow to pass byref as native int.
07bb68d

another fix.
02452ba

Fix another IL test.
e63f15b

sandreenko force-pushed the implicitArgumentCoercionCheck branch from 3213c95 to e63f15bCompare November 6, 2020 00:26

sandreenko merged commit d7c4602 into dotnet:masterNov 6, 2020

sandreenko deleted the implicitArgumentCoercionCheck branch November 6, 2020 05:32

This was referenced Dec 2, 2020
fix invalid IL in profiler stub helper #45453
Merged
COR_PRF_MONITOR_CODE_TRANSITIONS causes InvalidProgramException #45411
Closed

ghost locked as resolved and limited conversation to collaborators Dec 7, 2020

ericstj added this to the 6.0.0 milestone Sep 29, 2021

ericstj assigned kunalspathak Sep 30, 2021

kunalspathak removed the needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet label Oct 1, 2021

Navigation Menu

Uh oh!

Add an implicit argument coercion check. - #43386

Add an implicit argument coercion check.#43386
sandreenko merged 6 commits into
dotnet:masterfrom
sandreenko:implicitArgumentCoercionCheck

sandreenko commented Oct 14, 2020•
edited
Loading

Uh oh!

jkotas commented Oct 15, 2020

Uh oh!

sandreenko commented Oct 15, 2020

Uh oh!

sandreenko commented Oct 28, 2020

Uh oh!

webczat commented Oct 30, 2020

Uh oh!

sandreenko commented Oct 30, 2020

Uh oh!

webczat commented Oct 30, 2020

Uh oh!

sandreenko commented Nov 3, 2020

Uh oh!

kunalspathak left a comment

Uh oh!

AndyAyersMSNov 3, 2020

Uh oh!

sandreenkoNov 3, 2020•
edited
Loading

Uh oh!

sandreenko commented Nov 6, 2020

Uh oh!

kunalspathak commented Oct 1, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Uh oh!

Conversation

sandreenko commented Oct 14, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jkotas commented Oct 15, 2020

Uh oh!

sandreenko commented Oct 15, 2020

Uh oh!

sandreenko commented Oct 28, 2020

Uh oh!

webczat commented Oct 30, 2020

Uh oh!

sandreenko commented Oct 30, 2020

Uh oh!

webczat commented Oct 30, 2020

Uh oh!

sandreenko commented Nov 3, 2020

Uh oh!

kunalspathak left a comment

Choose a reason for hiding this comment

Uh oh!

AndyAyersMSNov 3, 2020

Choose a reason for hiding this comment

Uh oh!

sandreenkoNov 3, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sandreenko commented Nov 6, 2020

Uh oh!

kunalspathak commented Oct 1, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

sandreenko commented Oct 14, 2020•
edited
Loading

sandreenkoNov 3, 2020•
edited
Loading