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

[release/6.0] JIT: fix bug where a gc struct is not zero initialized #68050

Merged
merged 1 commit into from
May 4, 2022

Conversation

AndyAyersMS
Copy link
Member

@AndyAyersMS AndyAyersMS commented Apr 14, 2022

Port of #67825 to release/6.0.

Customer Impact

Reported by RavenDB (see #65694).

This bug can lead to unexpected crashes during GC.

Details

An optimization added in .NET 5 can in some cases lead to the jit reporting an uninitialized struct field as a gc reference.

The attached test case shows a simple app that exhibits this problem. The key components are:

  • a struct type S with one or more ref-class fields
  • a method that explicitly initializes a local s of type S,
    • then updates s via Dictionary.TryGetValue using the .? operator for the dictionary reference,
    • then only accesses the fields of s ifTryGetValue returns true
    // "gc" struct
    struct S 
    {
        object o;
    }

    public int G(Key k, Dictionary<Key, S> d)
    {
        S s = default;   // explicit initialization

        if (d?.TryGetValue(k, out s) == true && (s.o != null)

Testing

Verified with a local RavenDB repro and new test case.

Risk

Low. The IL pattern that leads to this situation is not common. Fix had very minor diffs in SPMI.

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Apr 14, 2022
@AndyAyersMS
Copy link
Member Author

@BruceForstall PTAL (manual merge)

cc @dotnet/jit-contrib

@ghost ghost assigned AndyAyersMS Apr 14, 2022
@BruceForstall BruceForstall self-requested a review April 14, 2022 20:50
@ghost
Copy link

ghost commented Apr 14, 2022

Tagging subscribers to this area: @JulieLeeMSFT
See info in area-owners.md if you want to be subscribed.

Issue Details

Port of #67825 to release/6.0.

Customer Impact

Reported by RavenDB (see #65694)

Details

An optimization added in .NET 5 can in some cases lead to the jit reporting an uninitialized struct field as a gc reference.

Testing

Verified with a local RavenDB repro and new test case.

Risk

Low. The IL pattern that leads to this situation is not common. Fix had very minor diffs in SPMI.

Author: AndyAyersMS
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

Copy link
Member

@jeffschwMSFT jeffschwMSFT left a comment

Choose a reason for hiding this comment

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

Approved. We will take this for consideration in 6.0.x

@JulieLeeMSFT JulieLeeMSFT added the Servicing-consider Issue for next servicing release review label Apr 28, 2022
@JulieLeeMSFT JulieLeeMSFT added this to the 6.0.x milestone May 3, 2022
@leecow leecow added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels May 3, 2022
@leecow leecow modified the milestones: 6.0.x, 6.0.6 May 3, 2022
@carlossanlop carlossanlop merged commit f915e1c into dotnet:release/6.0 May 4, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jun 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants