Repository navigation
Type initializer call ordering w.r.t. static field stores does not look right #72354
Description
Reproduction:
using System;
using System.Runtime.CompilerServices;
Problem();
[MethodImpl(MethodImplOptions.NoInlining)]
static void Problem()
{
[MethodImpl(MethodImplOptions.NoInlining)]
static int Call() => throw new Exception("Thrown from 'Call'");
var a = Call();
ClassWithCctor.StaticField = a;
}
class ClassWithCctor
{
public static int StaticField;
static ClassWithCctor() => throw new Exception("Thrown from 'ClassWithCctor'");
}Compile and run.
Expected result: the exception from Call is thrown.
Actual result: the type initializer throws instead:
Unhandled exception. System.TypeInitializationException: The type initializer for 'ClassWithCctor' threw an exception.
---> System.Exception: Thrown from 'ClassWithCctor'Cause - the insertion point of the helper here:
runtime/src/coreclr/jit/importer.cpp
Lines 16140 to 16143 in aafa910
category:correctness
theme:basic-cq
Activity
jakobbotsch commentedon Jul 18, 2022
Is this a bug? I'm not sure we make any guarantees about when static constructors will run except for "some time before the class is accessed".
(edit: but still seems reasonable to change this)
alexrp commentedon Jul 18, 2022
This seems spec-compliant to me, but probably fairly unintuitive to most users.
SingleAccretion commentedon Jul 18, 2022
In the standard we have:
I. e. not-beforefieldinit types have "precise" semantics on when the
.cctorruns.I agree that it is not clear whether "access" means load instructions only, hence name of the issue.
markples commentedon Dec 13, 2022
(First, an aside - I don't see that behavior with your test case as
Callis invoked before the static field write, but that's irrelevant to the general question of whether a write should trigger type initialization.)Thanks for pointing this out; it is an interesting observation. However, I believe that this is the intended behavior:
Type initialization is also triggered by loading the address of a static field. Otherwise reads (and writes) through interior pointers would need some sort of check just in case they are operating on static fields (and maybe something when pinning a value that happens to point to a static field?). Since this address may or may not be used to read or write, it's more consistent to just initialize on any read/write/load-address.
In cases where the type initializer writes the field in question, a "pre-type-initialization" write couldn't be observed because it would be overwritten before the read. So,
static = 5; Console.WriteLine(static);might not print 5.Other uses of the word "access" in the spec (emphasis mine) mostly seem to indicate reads or writes, though this list is not complete:
I did find one case where access did seem to be used only for a read, though it's more that "access" was used for a read but "set" was used for a write.
One other point is that this would be a breaking change. Perhaps that doesn't matter if this doesn't occur in practice, though that also takes away motivation for such a change.
markples commentedon Jan 4, 2023
cc @SingleAccretion who opened this issue before I close this
SingleAccretion commentedon Jan 9, 2023
The main point of the issue is that we see the
.cctorthrow beforeCalland not the other way around. We are on the same page about whether "write access" (stsfld) should trigger type initialization (it should), which is encouraging.Could you share the IR for
Problemthat you observe?For reference, I tried with a recent-ish runtime build and have:
(With a
TypeInitializationExceptionas expected)markples commentedon Jan 11, 2023
This is a difference between C# code that has been compiled with /optimize vs not. Without it, we get call / stloc / ldloc / stsfld, and it is fine. With it, we get call / stsfld, and the importer incorrectly adds the type initialization call before the tree for them. I'm testing a fix that properly runs this case.
JIT: Fix ordering of type initialization and stsfld (#80485)