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

JsonSerializer throws AccessViolationException for struct with field initializers #62983

Closed
yufeih opened this issue Dec 18, 2021 · 5 comments · Fixed by #62989
Closed

JsonSerializer throws AccessViolationException for struct with field initializers #62983

yufeih opened this issue Dec 18, 2021 · 5 comments · Fixed by #62989
Labels
area-System.Text.Json bug Servicing-consider Issue for next servicing release review
Milestone

Comments

@yufeih
Copy link
Contributor

yufeih commented Dec 18, 2021

Description

Deserialize a struct with field initializer causes AccessViolationException .

Reproduction Steps

JsonSerializer.Deserialize<Foo>("{}");

public struct Foo
{
    public float A = 1;
}

Expected behavior

The struct is deserialized successfully with the field initialized using the default field initializer.

Actual behavior

Throws this exception:

Fatal error. System.AccessViolationException: Attempted to read or write protected memory. This is often an indication that other memory is corrupt.
   at System.Runtime.CompilerServices.CastHelpers.IsInstanceOfInterface(Void*, System.Object)
   at System.Text.Json.Serialization.Converters.ObjectDefaultConverter`1[[Foo, console, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]].OnTryRead(System.Text.Json.Utf8JsonReader ByRef, System.Type, System.Text.Json.JsonSerializerOptions, System.Text.Json.ReadStack ByRef, Foo ByRef)
   at System.Text.Json.Serialization.JsonConverter`1[[Foo, console, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]].TryRead(System.Text.Json.Utf8JsonReader ByRef, System.Type, System.Text.Json.JsonSerializerOptions, System.Text.Json.ReadStack ByRef, Foo ByRef)
   at System.Text.Json.Serialization.JsonConverter`1[[Foo, console, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]].ReadCore(System.Text.Json.Utf8JsonReader ByRef, System.Text.Json.JsonSerializerOptions, System.Text.Json.ReadStack ByRef)
   at System.Text.Json.JsonSerializer.ReadFromSpan[[Foo, console, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]](System.ReadOnlySpan`1<Byte>, System.Text.Json.Serialization.Metadata.JsonTypeInfo, System.Nullable`1<Int32>)
   at System.Text.Json.JsonSerializer.ReadFromSpan[[Foo, console, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]](System.ReadOnlySpan`1<Char>, System.Text.Json.Serialization.Metadata.JsonTypeInfo)
   at System.Text.Json.JsonSerializer.Deserialize[[Foo, console, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]](System.String, System.Text.Json.JsonSerializerOptions)
   at Program.<Main>$(System.String[])

Regression?

No response

Known Workarounds

No response

Configuration

.NET SDK Version: 6.0.101

Other information

No response

@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Text.Json untriaged New issue has not been triaged by the area owner labels Dec 18, 2021
@ghost
Copy link

ghost commented Dec 18, 2021

Tagging subscribers to this area: @dotnet/area-system-text-json
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

Deserialize a struct with field initializer causes AccessViolationException .

Reproduction Steps

JsonSerializer.Deserialize<Foo>("{}");

public struct Foo
{
    public float A = 1;
}

Expected behavior

The struct is deserialized successfully with the field initialized using the default field initializer.

Actual behavior

Throws this exception:

Fatal error. System.AccessViolationException: Attempted to read or write protected memory. This is often an indication that other memory is corrupt.
   at System.Runtime.CompilerServices.CastHelpers.IsInstanceOfInterface(Void*, System.Object)
   at System.Text.Json.Serialization.Converters.ObjectDefaultConverter`1[[Foo, console, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]].OnTryRead(System.Text.Json.Utf8JsonReader ByRef, System.Type, System.Text.Json.JsonSerializerOptions, System.Text.Json.ReadStack ByRef, Foo ByRef)
   at System.Text.Json.Serialization.JsonConverter`1[[Foo, console, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]].TryRead(System.Text.Json.Utf8JsonReader ByRef, System.Type, System.Text.Json.JsonSerializerOptions, System.Text.Json.ReadStack ByRef, Foo ByRef)
   at System.Text.Json.Serialization.JsonConverter`1[[Foo, console, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]].ReadCore(System.Text.Json.Utf8JsonReader ByRef, System.Text.Json.JsonSerializerOptions, System.Text.Json.ReadStack ByRef)
   at System.Text.Json.JsonSerializer.ReadFromSpan[[Foo, console, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]](System.ReadOnlySpan`1<Byte>, System.Text.Json.Serialization.Metadata.JsonTypeInfo, System.Nullable`1<Int32>)
   at System.Text.Json.JsonSerializer.ReadFromSpan[[Foo, console, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]](System.ReadOnlySpan`1<Char>, System.Text.Json.Serialization.Metadata.JsonTypeInfo)
   at System.Text.Json.JsonSerializer.Deserialize[[Foo, console, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]](System.String, System.Text.Json.JsonSerializerOptions)
   at Program.<Main>$(System.String[])

Regression?

No response

Known Workarounds

No response

Configuration

.NET SDK Version: 6.0.101

Other information

No response

Author: yufeih
Assignees: -
Labels:

area-System.Text.Json, untriaged

Milestone: -

@EgorBo
Copy link
Member

EgorBo commented Dec 18, 2021

Checked JIT fails with

Assert failure(PID 15204 [0x00003b64], Thread: 19020 [0x4a4c]): Assertion failed '(genActualType(op2->TypeGet()) == genActualType(info.compRetType)) || ((op2->TypeGet() == TYP_I_IMPL) && TypeIs(info.compRetType, TYP_BYREF)) || (op2->TypeIs(TYP_BYREF, TYP_REF) && (info.compRetType == TYP_I_IMPL)) || (varTypeIsFloating(op2->gtType) && varTypeIsFloating(info.compRetType)) || (varTypeIsStruct(op2) && varTypeIsStruct(info.compRetType)) : Possibly bad IL with CEE_ret at offset 0005h (op1=NULL op2=struct stkDepth=0)' in 'DynamicClass:.ctor():System.Object' during 'Importation' (IL size 6)

    File: C:\prj\runtime-2\src\coreclr\jit\importer.cpp Line: 17650
    Image: C:\prj\runtime-2\artifacts\bin\coreclr\windows.x64.Checked\CoreRun.exe

I guess it's slightly confused with the input IL which creates a Foo struct but is reported to have a ref-type return type 🤔

IL to import:
IL_0000  73 02 00 00 06    newobj       0x6000002
IL_0005  2a                ret

@EgorBo
Copy link
Member

EgorBo commented Dec 18, 2021

I suspect it's JSON's ReflectionEmitMemberAccessor emits an invalid dynamic code (missed boxing?)

@EgorBo
Copy link
Member

EgorBo commented Dec 18, 2021

So Foo struct has a parameterless constructor (C# 10 feature) due to field initialization, it means it goes this path https://github.com/dotnet/runtime/blob/main/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionEmitMemberAccessor.cs#L51 where boxing is still needed if it's a value type.

Explicit parameterless ctor also reproduces the AVE:

using System.Text.Json;

JsonSerializer.Deserialize<Foo>("{}");

public struct Foo
{
    public float A;

    public Foo()
    {
        A = 42;
    }
}

@EgorBo EgorBo added the bug label Dec 18, 2021
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Dec 18, 2021
@krwq krwq added the Servicing-consider Issue for next servicing release review label Dec 21, 2021
@krwq krwq added this to the 7.0.0 milestone Dec 21, 2021
@krwq krwq removed the untriaged New issue has not been triaged by the area owner label Dec 21, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Dec 21, 2021
@yufeih
Copy link
Contributor Author

yufeih commented Dec 22, 2021

Thanks for the fix!

@ghost ghost locked as resolved and limited conversation to collaborators Jan 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Text.Json bug Servicing-consider Issue for next servicing release review
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants