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

BadImageFormatException : Bad IL format when using base #13926

Closed
abelbraaksma opened this issue Sep 19, 2022 · 2 comments · Fixed by #16773
Closed

BadImageFormatException : Bad IL format when using base #13926

abelbraaksma opened this issue Sep 19, 2022 · 2 comments · Fixed by #16773
Assignees
Labels
Area-Compiler-Checking Type checking, attributes and all aspects of logic checking Bug Impact-Medium (Internal MS Team use only) Describes an issue with moderate impact on existing code.
Milestone

Comments

@abelbraaksma
Copy link
Contributor

abelbraaksma commented Sep 19, 2022

Using base.XXX to call a base method causes a BadImageFormatException to be raised.

EDIT: it looks like F# allowing me to call an abstract method, as the base class is abstract. This, I believe, should not be allowed and JIT therefore throws a BadImageFormatException. If you try to write code in F# with an AbstractClass and a derived class, you'd receive a normal compile-time error.

Repro steps

The following code (requires Giraffe and FsUnit to run the test) throws a BadImageFormatException

namespace Test

open Giraffe
open Xunit
open FsUnit.Xunit

open System.Text.Json
open System.Text.Json.Serialization

type StringTrimJsonSerializer(o: JsonSerializerOptions) =
    inherit JsonConverter<string>()

    override this.Read(reader, _, _) =
        match reader.TokenType with
        | JsonTokenType.String -> reader.GetString().Trim()
        | _ -> JsonException("Type is not a string") |> raise

    /// This causes a BadImageFormatException
    override this.Write(writer, objectToWrite, options) = base.Write(writer, objectToWrite, options)


module SerializationTests =

    type SomeType = { Amount: decimal; Currency: string }

    let serialize item =
        let options = SystemTextJson.Serializer.DefaultOptions
        StringTrimJsonSerializer options |> options.Converters.Add
        JsonSerializer.Serialize(item, options)

    let deserialize<'T> (stringValue: string) =
        let options = SystemTextJson.Serializer.DefaultOptions
        StringTrimJsonSerializer options |> options.Converters.Add
        JsonSerializer.Deserialize<'T>(stringValue, options)

    [<Fact>]
    let ``Roundtip type should trim currency whitespace`` () =
        { Amount = 42.99M; Currency = "  USD   " }
        |> serialize
        |> deserialize<SomeType>
        |> should equal { Amount = 42.99M; Currency = "USD" }

Expected behavior

Should call the base method or give compile error if such method doesn't exist.

Actual behavior

Throws:

BadImageFormatException : Bad IL format.

Known workarounds

Don't use base in a derived method.

Related information

On dotnet 6, using System.Text.Json serialization BCL classes, Windows 10/11.

EDIT, the IL of the offending code looks as follows (Release build):

.method public hidebysig virtual 
	instance void Write (
		class [System.Text.Json]System.Text.Json.Utf8JsonWriter writer,
		string objectToWrite,
		class [System.Text.Json]System.Text.Json.JsonSerializerOptions options
	) cil managed 
{
	// Method begins at RVA 0x316c
	// Header size: 1
	// Code size: 10 (0xa)
	.maxstack 8

	// base.Write(writer, objectToWrite, options);
	IL_0000: ldarg.0
	IL_0001: ldarg.1
	IL_0002: ldarg.2
	IL_0003: ldarg.3
	IL_0004: call instance void class [System.Text.Json]System.Text.Json.Serialization.JsonConverter`1<string>::Write(class [System.Text.Json]System.Text.Json.Utf8JsonWriter, !0, class [System.Text.Json]System.Text.Json.JsonSerializerOptions)
	// }
	IL_0009: ret
} 
@vzarytovskii
Copy link
Member

vzarytovskii commented Sep 19, 2022

Reproduces on

module Testing

open System.Text.Json
open System.Text.Json.Serialization

type StringTrimJsonSerializer(o: JsonSerializerOptions) =
    inherit JsonConverter<string>()
    override this.Read(reader, _, _) =
        match reader.TokenType with
        | JsonTokenType.String -> reader.GetString().Trim()
        | _ -> JsonException("Type is not a string") |> raise
    override this.Write(writer, objectToWrite, options) = base.Write(writer, objectToWrite, options)

type SomeType = { Foo: string}

let serialize item =
    let options = JsonSerializerOptions()
    StringTrimJsonSerializer options |> options.Converters.Add
    JsonSerializer.Serialize(item, options)

[<EntryPoint>]
let main _ =
    { Foo = "a" } |> serialize
    0

Does not reproduce on:

module Testing

open System.Text.Json
open System.Text.Json.Serialization

type StringTrimJsonSerializer(o: JsonSerializerOptions) =
    inherit JsonConverter<string>()
    override this.Read(reader, _, _) =
        match reader.TokenType with
        | JsonTokenType.String -> reader.GetString().Trim()
        | _ -> JsonException("Type is not a string") |> raise
    override this.Write(writer, objectToWrite, options) = base.Write(writer, objectToWrite, options)

type SomeType = { Foo: int }

let serialize item =
    let options = JsonSerializerOptions()
    StringTrimJsonSerializer options |> options.Converters.Add
    JsonSerializer.Serialize(item, options)

[<EntryPoint>]
let main _ =
    { Foo = 1 } |> serialize
    0

Both release and debug

@abelbraaksma
Copy link
Contributor Author

abelbraaksma commented Sep 19, 2022

@vzarytovskii The latter never calls the StringTrimJsonSerializer.Write method (there's no string in the type), so it makes sense that it doesn't fail. The JIT never has to emit the method, so you won't get the BadImageFormatException.

The problem seems to be that base here is an abstract class and the Write method itself is also abstract. In other words, it should never compile. If you try this with F# classes (abstract A, concrete B derives from A) then if B uses base.Foo it'll give a compile error saying that you cannot call an abstract method.

The same is true here, but for some reason, F# doesn't consider it an abstract method.

@vzarytovskii vzarytovskii added Area-Compiler-Checking Type checking, attributes and all aspects of logic checking Area-Compiler-CodeGen IlxGen, ilwrite and things at the backend labels Sep 21, 2022
@vzarytovskii vzarytovskii added this to the Backlog milestone Sep 21, 2022
@KevinRansom KevinRansom self-assigned this Jun 1, 2023
@KevinRansom KevinRansom modified the milestones: Backlog, June-2023 Jun 1, 2023
@vzarytovskii vzarytovskii modified the milestones: February-2024, March-2024 Mar 4, 2024
KevinRansom added a commit to KevinRansom/fsharp that referenced this issue Mar 5, 2024
psfinaki added a commit that referenced this issue Mar 5, 2024
…16773)

* Fix #13926 - BadImageFormatException : Bad IL format when using base

* fantomas

* Update src/Compiler/AbstractIL/il.fs

Co-authored-by: Tomas Grosup <tomasgrosup@microsoft.com>

* Update src/Compiler/FSharp.Compiler.Service.fsproj

Co-authored-by: Tomas Grosup <tomasgrosup@microsoft.com>

* Update 8.0.md

---------

Co-authored-by: Tomas Grosup <tomasgrosup@microsoft.com>
Co-authored-by: Petr <psfinaki@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compiler-Checking Type checking, attributes and all aspects of logic checking Bug Impact-Medium (Internal MS Team use only) Describes an issue with moderate impact on existing code.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants