Delete support for TLS RVA statics from the Jit

[SingleAccretion]

This construct is not supported on .NET Core+, so the code in question is dead. Deleting it frees one handle kind (which will be helpful in my future changes), and deletes one path from fgMorphField (which will also be helpful in my future changes).

Fixes #4852.

Part of #58312.

[ghost]

Tagging subscribers to this area: @JulieLeeMSFT
See info in area-owners.md if you want to be subscribed.

Issue Details

---

This construct is not supported on .NET Core, so the code in question is dead. Deleting it frees one handle kind (which will be helpful in my future changes), and deletes one path from fgMorphField (which will also be helpful in my future changes).

Fixes #4852.

Part of #58312.

Author:	SingleAccretion
Assignees:	-
Labels:	area-CodeGen-coreclr
Milestone:	-

[jkotas]

TLS RVA statics have been used by managed C++. Are we sure that managed C++ does not use them anymore?

Note that the issue #4852 filled against these tests is from 2015. The managed C++ was not supported in 2015, but it has changed since then.

[SingleAccretion]

Are we sure that managed C++ does not use them anymore?

That is something for me to investigate before this would be ready for review (or closure).

The first approximation here is that none of the IL tests work if I build/run them, though mysteriously enough they fail with FileNotFoundException, not BadImageException (and if I remove .data tls from them, they do run fine).

Edit: I can get one test to run after all, but now it fails with AV. Hmm.

[jkoritzinsky]

MSVC's __declspec(thread) isn't supported in C++/CLI, and it looks like (from my experimentation) that C++11's thread_local behaves similarly.

Here's what the field and data declarations for a class-static thread_local (or __declspec(thread) class-static) int with an default value of 42 looks like:

.field static assembly int32 '?i@Unmanaged@@2HA' at D_00021C90
.data D_00021C90 = bytearray (
                 2A 00 00 00)                                     // *...

I think the TLS RVA statics may have only ever been used by Managed C++, which is not supported on .NET 5+. Only C++/CLI is supported on .NET 5+ since it requires a recompilation using the /clr:netcore switch.

[SingleAccretion]

I was about to write a comment on how it actually does work 😄.

I tested it is with this (properly /clr:netcore):

__declspec(thread) static int TlsInt = 100;

public ref class CppCliClass
{
public:
    static int GetTlsStatic()
    {
        return TlsInt;
    }

    static void SetTlsStatic(int value)
    {
        TlsInt = value;
    }
};

And it does indeed have the semantics expected of TLS, and produces inline code on x86 (not without first failing on some DEBUG-only asserts, but those are not critical):

// CppCliClass:GetTlsStatic

IN0004: 000000 push     ebp
IN0005: 000001 mov      ebp, esp

G_M10086_IG02:

IN0001: 000003 mov      eax, dword ptr FS:[0x002C]
IN0002: 000009 mov      eax, dword ptr [eax+28]
IN0003: 00000C mov      eax, dword ptr [eax+4]

G_M10086_IG03:

IN0006: 00000F pop      ebp
IN0007: 000010 ret

If you look at the ldasms output, there is no .data tls in there indeed, which is very puzzling because it does survive the roundtripping if you use it on one of the test assemblies.

[jkoritzinsky]

@SingleAccretion did you make sure to force the TlsInt global variable to generate in managed code instead of in native code? It's possible that it is in the native code segment and as a result uses the native mechanisms to mark it as TLS.

Can you recompile with /clr:pure and see what the IL looks like?

[SingleAccretion]

@jkoritzinsky the assembly excerpt is from the Jit compiling the IL for the methods, so, it should be the "real thing".

The methods according to ildasm

  .method public hidebysig static int32  GetTlsStatic() cil managed
  {
    // Code size       6 (0x6)
    .maxstack  1
    IL_0000:  ldsfld     int32 '?A0xfa31f548.TlsInt'
    IL_0005:  ret
  } // end of method CppCliClass::GetTlsStatic

  .method public hidebysig static void  SetTlsStatic(int32 'value') cil managed
  {
    // Code size       7 (0x7)
    .maxstack  1
    IL_0000:  ldarg.0
    IL_0001:  stsfld     int32 '?A0xfa31f548.TlsInt'
    IL_0006:  ret
  } // end of method CppCliClass::SetTlsStatic

Can you recompile with /clr:pure and see what the IL looks like?

Let's see...

[jkoritzinsky]

Can you also share the IL from ildasm for the field definition?

[SingleAccretion]

Just as in your case, it is disassembled as an ordinary .data:

.field static assembly int32 '?A0xfa31f548.TlsInt' at D_0000F77C

.data D_0000F77C = bytearray (
                 64 00 00 00)                                     // d...

[SingleAccretion]

Can you recompile with /clr:pure and see what the IL looks like?

Looks like that will not compile: C3389 __declspec(thread) cannot be used with /clr:pure or /clr:safe.

[jkoritzinsky]

Based on that, I'd say that the TLS support is somehow reliant on native code or native PE features. Not sure exactly how though.

[jkotas]

TLS support is somehow reliant on native code or native PE features

Correct. I expect that you can only hit it in mixed mode C++ CLI images. I am not sure whether ilasm can produce images that can hit it.

[SingleAccretion]

That is the conclusion I came to as well. I do note that it is Jitted code that gets run, not some kind of native stub, I have verified that by modifying the compiler (making it import the fields in question as constants).

With ilasm-produced images, while they do contain the .tls section with the correct metadata, the TLS index of the image at runtime remains uninitialized, which is why the IL tests AV on execution (the index given to the Jit by getFieldThreadLocalStoreID is bogus).

The C++/CLI-produced images cause the index to be properly initialized, and the code produced for them is functional, on x86. On all other platforms, it crashes as the helper path is not implemented, and the Jit generates call [null].

(Side note: ildasm does not see the .data declarations as .tls simply because the native toolchain puts them in the .rdata section)

So it appears we have this somewhat functioning on x86 for some C++/CLI code, and as such the Jit code needs to be kept. Should we keep the tests though? They will not work unless we somehow fix the TLS index situation (which I imagine is not something we'd want to do). That said, there is not a lot of value in deleting them either...

[jkotas]

On all other platforms, it crashes as the helper path is not implemented

It is #42187 (comment)