Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

JIT: tolerate nonzero constant byrefs in impCheckForNullPointer #17042

Merged
merged 1 commit into from
Mar 20, 2018

Conversation

AndyAyersMS
Copy link
Member

With the advent of #16966 we may now see constant nonzero byrefs from
things like RVA statics. Tolerate these in impCheckForNullPointer.

Note previously we'd type these as ints/longs and so bail out of
impCheckForNullPointer after the first check.

Closes #17008.

With the advent of dotnet#16966 we may now see constant nonzero byrefs from
things like RVA statics. Tolerate these in `impCheckForNullPointer`.

Note previously we'd type these as ints/longs and so bail out of
`impCheckForNullPointer` after the first check.

Closes #17008.
@AndyAyersMS
Copy link
Member Author

@BruceForstall PTAL
cc @dotnet/jit-contrib

@BruceForstall
Copy link
Member

@dotnet-bot test Windows_NT x86 Checked Build and Test
@dotnet-bot test Windows_NT x64 Build and Test

@AndyAyersMS
Copy link
Member Author

Were there other pri-1 failure cases? I moved the offending test to pri-0 as part of this change.

Copy link
Member

@BruceForstall BruceForstall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

I triggered pri-1 runs before I saw you'd moved the failing case to pri-0.

Copy link

@CarolEidt CarolEidt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link

@briansull briansull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh Joy - non-zero constant byrefs
LGTM

@AndyAyersMS
Copy link
Member Author

Tizen and armel failures are known and this fix is not arch specific. So will ignore.

@AndyAyersMS AndyAyersMS merged commit 58f8744 into dotnet:master Mar 20, 2018
@AndyAyersMS AndyAyersMS deleted the Fix17008 branch March 20, 2018 00:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants