.NET (fixed) ValueTuple arrays considered harmful

Applied Mediocrity

Hello, folks.

So I have this data structure that among other things stores last 20 error codes and their respective subtypes. An array of ValueTuple<T1, T2> shall do fine, I thought. It's a glorified struct after all. You have to use language features, you know. And so I did. Right until the thing went kerplunk with InvalidProgramException. Took me some sweet time to find out why exactly. I was about to post this on SO, but was afraid that one Eric Lippert might take to very carefully explaining how everything I know about life in general is wrong. He'd be quite right, of course, but I can't have that. Perhaps posting here I may yet fly under. Then again it is likely a compiler bug. Guess I should see about reporting it or whether it's been already fixed or something. A lot of work, that. Hmm.

Anyway, this snippet should, as far as I can determine, allow you to reproduce the problem, if you feel so inclined.

// C# compiler version: 2.8.3.63029
// VS version: 15.7.6
// Target framework: 4.7

using System;
using System.Runtime.InteropServices;

internal static class Structures
{
    [StructLayout(LayoutKind.Sequential, Pack = 1, Size = 25)]
    internal struct Foo
    {
        internal uint TotalErrors;
        internal byte ErrorPointer;
        [MarshalAs(UnmanagedType.ByValArray, SizeConst = 10)] internal (byte Code, byte Sub)[] Errors;

        internal void Add(byte Code, byte Sub)
        {
            TotalErrors += 1;
            if (Errors == null)
                Errors = new(byte, byte)[10];
            Errors[ErrorPointer] = (Code, Sub);
            if (++ErrorPointer == Errors.Length)
                ErrorPointer = 0;
        }
    }
}

internal class Program
{
    internal static void Main(string[] args)
    {
        var St = new Structures.Foo();
        St.Add(42, 255);

        Console.WriteLine("It works!"); // 'fraid not, guv'.
        Console.ReadKey(true);
    }
}

PEVerify has this to say:
[IL]: Error: [C:\yada-yada\bin\x64\Debug\Example.exe : Program::Main] Invalid IL or CLR metadata.

A more complex case of the code above will also give this wherever the evil class is being used:
[IL]: Error: [C:\yada-yada\bin\x64\Debug\Stuff.exe : Initrode.Foo.Bar::IsFrameValid][offset 0x0000008A] Unable to resolve token.

Oh alright, struct it is then.

ben_lubar

Go only lets you use tuples as return values from functions, and I'm pretty sure that ValueTuple was (at least in part) implemented to shut up Go devs who wanted multi-value returns.

marczellm

Why do you need the annotations?

sockpuppet7

“Considered Harmful” Essays Considered Harmful

masonwheeler

Commenting out the [MarshalAs] attribute causes this to work. With it active, the program crashes with:

System.InvalidProgramException: 'The metadata is corrupt.'

If the compiler is generating corrupt metadata, this is a compiler bug. You shouldn't ask about it on StackOverflow; you should file a bug report against the compiler. Explain what you're trying to do, include the snippet, and mention that the thing that causes it to break is the MarshalAs attribute. With that, the devs should have no trouble tracking this down and fixing it.

sockpuppet7

@masonwheeler said in .NET (fixed) ValueTuple arrays considered harmful:

Commenting out the [MarshalAs] attribute causes this to work

@Applied-Mediocrity If you're using it to interface with C code, you could use a C++/CLI DLL to glue things together

Applied Mediocrity

@masonwheeler
I see. Well, thanks for confirming that the issue isn't with me. I will see to it then.

Also, yes, removing MarshalAs works. Removing it, however, prevents unsafe serialization of the struct (which happens to be necessary, because that's how we fall down the stairs here). I thought perhaps I'm using it wrong.

That is to say I did not feel sufficiently proficient to declare it a compiler bug by myself. Especially given the unspeakable WTFs I'm responsible of (but because they're unspeakable, we shall not speak of them).

Bulb

@Applied-Mediocrity said in .NET (fixed) ValueTuple arrays considered harmful:

unspeakable WTFs

On this site, every WTF is speakable.

sockpuppet7

@Applied-Mediocrity said in .NET (fixed) ValueTuple arrays considered harmful:

Also, yes, removing MarshalAs works. Removing it, however, prevents unsafe serialization of the struct (which happens to be necessary, because that's how we fall down the stairs here). I thought perhaps I'm using it wrong.

You can have the low level without this hackish stuff if you use it as a vanilla byte array, without any struct. Maybe it is still a bit hacky, but I think it would be a lesser WTF

PJH

@masonwheeler said in .NET (fixed) ValueTuple arrays considered harmful:

Commenting out the [MarshalAs] attribute causes this to work.

That's also Help - this is in Sidebar unless I'm very much mistaken...

Unperverted Vixen

This post is deleted!

Rhywden

@Bulb said in .NET (fixed) ValueTuple arrays considered harmful:

@Applied-Mediocrity said in .NET (fixed) ValueTuple arrays considered harmful:

unspeakable WTFs

On this site, every WTF is speakable.

Come to think of it, if you merely write it and never speak it, is it really speech?

_{filed under: What sound does one hand make when trying to clap?}

stillwater

@Applied-Mediocrity unrelated and unhelpful but your avatar basically describes my entire life in a picture.

stillwater

@Applied-Mediocrity You're definitely dealing with Eric Lippert level of fuckery here. I'm pretty sure he can write a couple blog posts about the issue.

Gribnit

@Bulb

synchronized (id.intern()) {

pie_flavor

I think it is because you cannot MarshalAs a tuple.

TwelveBaud

System.ValueTuple has a [StructLayout(LayoutKind.Auto)] annotation with no size or pack constraints, which means you're practically guaranteed not to get the marshalling behavior desired.

ben_lubar

@TwelveBaud said in .NET (fixed) ValueTuple arrays considered harmful:

System.ValueTuple has a [StructLayout(LayoutKind.Auto)] annotation with no size or pack constraints, which means you're practically guaranteed not to get the marshalling behavior desired.

How many ways are there to lay out a structure that consists of two one-byte fields?

How many ways are there to do that that a compiler that wasn't written by someone trying to give the wrong answer to the first question would do?

jmp

@ben_lubar said in .NET (fixed) ValueTuple arrays considered harmful:

@TwelveBaud said in .NET (fixed) ValueTuple arrays considered harmful:

System.ValueTuple has a [StructLayout(LayoutKind.Auto)] annotation with no size or pack constraints, which means you're practically guaranteed not to get the marshalling behavior desired.

How many ways are there to lay out a structure that consists of two one-byte fields?

How many ways are there to do that that a compiler that wasn't written by someone trying to give the wrong answer to the first question would do?

If you're compiling for a machine where individual byte access is hard, I could see a compiler storing the struct with 3 padding bytes after/before each value, depending on endianness. Alternately, just padding the struct out to a 4-byte boundary because it thinks all structs should be 4-byte aligned by default. So at least three options.

masonwheeler

@TwelveBaud said in .NET (fixed) ValueTuple arrays considered harmful:

System.ValueTuple has a [StructLayout(LayoutKind.Auto)] annotation with no size or pack constraints, which means you're practically guaranteed not to get the marshalling behavior desired.

If so, then it should result in a compiler error to that effect. Silently accepting something invalid and generating bad metadata is a bug no matter what.

ben_lubar

@jmp said in .NET (fixed) ValueTuple arrays considered harmful:

If you're compiling for a machine where individual byte access is hard

Is first-party .NET available on those machines using a PE executable?

jmp

@ben_lubar said in .NET (fixed) ValueTuple arrays considered harmful:

@jmp said in .NET (fixed) ValueTuple arrays considered harmful:

If you're compiling for a machine where individual byte access is hard

Is first-party .NET available on those machines using a PE executable?

Dunno, but it might design for that case anyway.

Tsaukpaetra

@Rhywden said in .NET (fixed) ValueTuple arrays considered harmful:

_{filed under: What sound does one hand make when trying to clap?}

Something very similar to the sound a half-clap does.

topspin

@PJH said in .NET (fixed) ValueTuple arrays considered harmful:

@masonwheeler said in .NET (fixed) ValueTuple arrays considered harmful:

Commenting out the [MarshalAs] attribute causes this to work.

That's also Help - this is in Sidebar unless I'm very much mistaken...

Did you take Gribnit as an elective?

Applied Mediocrity

Oy vez mear! TRWTF is me not checking beforehand whether any sort of ValueTuple can even be marshalled as unmanaged. It cannot.

See, that's why I didn't want to awaken St. Lippert, Jon Skeet et al. I'd have been enlightened with a proper answer, but also left with a burning urge to nuke all the codebase, throw the source server in front of the nearest heavy machinery, repay my last years pay to the company and relocate myself to Bali to live as a hermit (shh-shush what's the matter with that one - I hear he used unsafe in C# - *spits* pox is too good for him)

I'm not sure about reporting this either. I mean, if ValueTuple cannot be reliably marshalled, I see very little use in fixing it so that it still cannot be done. The compiler actually allows to marshal anything I please, but outright fsckery will result in access violation of unmanaged memory. This InvalidProgramException was actually more helpful than a full CLR crash. Logged it, fixed it, made a big deal about it.

dkf

@Applied-Mediocrity said in .NET (fixed) ValueTuple arrays considered harmful:

I'm not sure about reporting this either. I mean, if ValueTuple cannot be reliably marshalled, I see very little use in fixing it so that it still cannot be done. The compiler actually allows to marshal anything I please, but outright fsckery will result in access violation of unmanaged memory. This InvalidProgramException was actually more helpful than a full CLR crash. Logged it, fixed it, made a big deal about it.

This all seems horrible. But the most horrible bit of all is just how inscrutable the errors are; the compiler really ought to have told you earlier that it was generating something that wasn't going to work (or given you a polite refusal). Spitting bad code out? Worst option detected.

cheong

@masonwheeler said in .NET (fixed) ValueTuple arrays considered harmful:

@TwelveBaud said in .NET (fixed) ValueTuple arrays considered harmful:

System.ValueTuple has a [StructLayout(LayoutKind.Auto)] annotation with no size or pack constraints, which means you're practically guaranteed not to get the marshalling behavior desired.

If so, then it should result in a compiler error to that effect. Silently accepting something invalid and generating bad metadata is a bug no matter what.

In my experience, bugs related to Attributes almost always result in runtime error (except the things related to syntax, of course).

Never seen it generates compiler error before, because it won't have any change to the MSIL instruction it compiles to. The compiler have no idea what it means - to the compiler these are just something that attach to the code blocks and nothing special. These will only be read by the JIT compiler.

To catch this you'll need static code analysis tools, or require the compiler try to call JIT compiler when completes. However calling JIT compiler makes very little sense because even if it works on your machine, it may be nonsense for the other archs, and vice versa. And it could be nonsense if the target machine uses a different JIT compiler version too. If it expects different parameter than the one you supply, it'll also treat that as bad metadata.

pie_flavor

@dkf said in .NET (fixed) ValueTuple arrays considered harmful:

@Applied-Mediocrity said in .NET (fixed) ValueTuple arrays considered harmful:

I'm not sure about reporting this either. I mean, if ValueTuple cannot be reliably marshalled, I see very little use in fixing it so that it still cannot be done. The compiler actually allows to marshal anything I please, but outright fsckery will result in access violation of unmanaged memory. This InvalidProgramException was actually more helpful than a full CLR crash. Logged it, fixed it, made a big deal about it.

This all seems horrible. But the most horrible bit of all is just how inscrutable the errors are; the compiler really ought to have told you earlier that it was generating something that wasn't going to work (or given you a polite refusal). Spitting bad code out? Worst option detected.

I believe ReSharper does this.
Edit: No it doesn't.

pie_flavor

FYI we're now the top search result for 'valuetuple marshalling'.

Applied Mediocrity

@pie_flavor
Excellent! Now frustrated experienced programmers in their attempts to fulfill their daily craving for Black Magic will come here and learn from my arguably bad example!

...and rewrite it with ushort and bitshifting!

_{Filed under: Dijkstra was right}