Repository navigation
Passing in parameters in constructor of read-only struct shouldn't be allowed #122237
Description
Activity
I don't understand. There's some bug with the runtime+android. Shouldn't that big just be fixed wherever it exists?
Why does Roslyn need to make changes here?
@CyrusNajmabadi I do not think this is a bug from Android runtime. A behavior different from other runtime? yes, but I don't think Android runtime is necessarily wrong here.
a = new A(a.x)whenAis a struct and the constructor's parameter isinshouldn't be possible.I don't think the runtime is wrong to zero the stack before running the constructor. But either a defensive copy should have been done despite the parameter being
inOR it shouldn't compile OR some analyzer should warn me that I am doing something ending up in uncharted territory.when A is a struct and the constructor's parameter is in shouldn't be possible.
Why not? It seems perfectly reasonable (and very important to support) to me.
A.x might be a very large struct that you do not want copied. The constructor may be reading a few small pieces of data from it. Passing by
inis the way you want to do this efficiently.I don't think the runtime is wrong to zero the stack before running the constructor.
Zeroing the stack inside the constructor is fine. But it shouldn't zero references to data being passed in, or the location it is going to write into. That's a bug.
But either a defensive copy should have been done
That would defeat the point of 'in'. You want to cheaply pass a reference without a copy, knowing it can't mutate.
OR it shouldn't compile
The code is legal and sensible. It should compile. Imagine the data you are passing in is huge, but you will only grab a little of it in the constructor. If you don't allow
in, then that forces expensive copies.inexists exactly to allow this to be efficient. :-)OR some analyzer should warn me
We're not going to earn because some runtime has a bug and doesn't execute this code properly. The right thing to do is fix that runtime.
C# specification is very clear, what behavior is expected here. It, in fact, require that
structconstructor should be invoked on temporary local variable, and only then assigned to named local.That program, for example, required to print 25, but not 36:
#:property Configuration=Release using System; Test1(); Test2(); void Test1() { S s = new(); s = new(); try { s = new(); } catch { } Console.Write(s.I); } void Test2() { S s = new(0); s = new(0); try { s = new(0); } catch { } Console.Write(s.I); } readonly struct S { static int s_i; public readonly int I; public S() { I = ++s_i; if(I % 3 is 0) { throw new Exception(); } } public S(in int i) : this() { } }
C# compiler sometimes emit call to constructor directly on named local, but only when it can guaranty, that previous value of local can not be observed. For example, first two calls to
new S()inTest1are emitted as:call instance void S::.ctor()
and only last as:
newobj instance void S::.ctor()
But, when you use constructor with
inparameter (inTest2) all three calls to constructor use temporary local variable forSinstance:newobj instance void S::.ctor(int32&)
Now you should inspect what IL emitted for your code
a = new GEJ(a.x, a.y, a.z, a.infinity);
If it is
newobj(as I expect it should be), but notcall, then your "defensive copy" is already here: runtime is not allowed to modify named locala, until constructor successfully complete. Now if when tiered compilation kicks in, and JIT compiler decides to eliminate that temporary local and call constructor on named local directly, while it is still in use, then it is clear violation on specification.Reacted by Paulus Pärssinen and Cole TobinThis indeed sounds like an issue in runtime (probably JIT tiered compilation given it reproes only after some iterations), not roslyn. But we would need more details like a self-contained reproducible example to investigate further. Thanks.
Thanks for the input, I will work on a repro. If that's indeed a bug, I want to note that this never happened on anything else than Android runtime.
@jjonescz I have managed to repro into a self-contained project... I added a
Reproductionsection to the description of this issue.I have no clue why this repro is taking 1000 iterations while with my library it takes only 5, but at least I could reproduce!
@PetSerAl, if the spec is clear on this, this is probably a runtime bug. If that is the case, I am not sure this is the right repo, as it doesn't happen on linux or windows runtime. Only Android.
@jakobbotsch, PTAL.
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.Tagging subscribers to this area: @steveisok, @vitek-karas
See info in area-owners.md if you want to be subscribed.Fix in #131586. It's an interpreter optimization issue when running with INTERP_OPT_SUPER_INSTRUCTIONS, that only kicks in after executing a method 1000 times.
Reacted by Nicolas Dorier- added a commit that references this issue
on Jul 31, 2026 - locked and limited conversation to collaborators
on Aug 31, 2026
Problem
Passing
inparameters to a constructor of a readonly struct shouldn't be possible. Or at least an analyzer should tell us something is wrong.I made a video explaining the issue.
I am the maintainer of a widely used library, and a user reported a bug to me.
This happened:
Program.csdid not reproduce itI managed to pin down the specific code that was causing the issue.
This code just create a variable a of type
GEJ(readonly struct), then re-affect to itself 10 times.This code, when running outside of Android or outside my library, behaved as expected: the value of
adid not change.However, on Android and within my library’s code, it would deterministically stop working after roughly five iterations of the loop. After the fifth iteration,
awould become0.The bug was here... can you see it?
And the fix was to remove in from the constructor.
What I believe happens is that when executing
a = new GEJ(a.x, a.y, a.z, a.infinity);, in some unclear circumstances, the runtime zeros the stack space for variableabefore running the constructor. But because those wereinparameters, no copy was made anda.xis passed by reference and is equal to zero due to the stack cleaning.Reproduction
dotnet build -t:Run -f net9.0-android -p:JavaSdkDirectory=/usr/lib/jvm/jdk-21.0.9+10 -p:AndroidSdkDirectory=/home/nicolasdorier/Android/Sdk)Run Testin the app.The code run is
What you will see is that, at the 1000th iteration,
a.x.n0becomes inexplicably 0:Conclusion
This bug is triggered under such precise circumstances that I feel the language, or at least a Roslyn analyzer, should prevent us from using in in a struct constructor. Or maybe the runtime should detect that a defensive copy is needed.
Video
I made a video explaining the issue as best as I could.