Flush instruction cache after thunk pool allocation

[jkotas]

Fixes #74710

[filipnavara]

I will rebase #75264 if this lands first.

[filipnavara]

I noticed the same issue when fixing the code for macOS ARM64 so I am actually glad to have a more complete fix. Not being too familiar with the ARM64 architecture I have a follow-up question. Since the data pages are written separately (from managed code) is there any need to flush data cache for these pages too? I originally came to the conclusion that it should not be necessary but since you are way more familiar with the code I assume you'll know better.

[jkotas]

The data pages will get written to when the marshalled delegate is created. Locks taken during that process should take care of the data catches flushing.

[VSadov]

We probably see Windows issues due to this as well, perhaps not as often for some reason.

[jkotas]

We probably see Windows issues due to this as well, perhaps not as often for some reason.

Yes.

This path is only hit by marshalled delegates. We tried to rid of marshalled delegates from everywhere in product code and replace them with function pointers. We have hit the bug in libraries test only because this one marshalled delegate was missed during the conversion. I am going to submit a separate PR to convert this marshalled delegate to a function pointer.

[VSadov]

LGTM! It was probably hard to figure the root cause.

[VSadov]

With this and the dealings with JIT memory on OSX in the other PR, I wonder - is there a configuration where NativeAOT does not use executable memory? (even if with some tradeoffs)

[jkotas]

is there a configuration where NativeAOT does not use executable memory? (even if with some tradeoffs)

We have a configuration under ifdef (that is currently disabled) that compiles these entrypoints into the binary as static code, and then creates copies of this static code if necessary using file mapping. It is not very portable and makes the binary bigger. We can make it work if somebody asks for it.

[MichalPetryka]

is there a configuration where NativeAOT does not use executable memory? (even if with some tradeoffs)

We have a configuration under ifdef (that is currently disabled) that compiles these entrypoints into the binary as static code, and then creates copies of this static code if necessary using file mapping. It is not very portable and makes the binary bigger. We can make it work if somebody asks for it.

I'd assume that mode is used by the teams that made NativeAOT work on consoles that are AOT only.

[jkotas]

I'd assume that mode is used by the teams that made NativeAOT work on consoles that are AOT only.

Yep. Or they just avoid marshalled delegates to avoid the problem altogether.

[jkotas]

/backport to release/7.0

[github-actions]

Started backporting to release/7.0: https://github.com/dotnet/runtime/actions/runs/3029769600

[bartonjs]

This path is only hit by marshalled delegates. We tried to rid of marshalled delegates from everywhere in product code and replace them with function pointers.

Does that mean there's some test coverage missing somewhere for the scenario? If it took us two weeks to figure out what was wrong I'd hate to think what it would mean for the average customer.

[jkotas]

Does that mean there's some test coverage missing somewhere for the scenario?

We do have a test coverage. The crash is caused by a race condition with processor instruction cache. It is impossible to write tests that would reliably catch these types of race conditions.