Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[C++/CLI] Direct access of unmanaged thread_local variable from managed code causes unexpected NullReferenceException #42187

Open
LEI-Hongfaan opened this issue Sep 14, 2020 · 9 comments
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI bug
Milestone

Comments

@LEI-Hongfaan
Copy link
Contributor

LEI-Hongfaan commented Sep 14, 2020

Description

Consider the following code

// Compile with /clr

#pragma managed(push, off)

thread_local int thread_local_value;

int get_thread_local_value() {
    return thread_local_value;
}

void set_thread_local_value(int value) {
    thread_local_value = value;
}
#pragma managed(pop)

public ref class Test abstract sealed {

public:
    static int ManipulateThreadLocalValue(int value) {
        thread_local_value = value; // Throws a NullReferenceException at runtime
        return thread_local_value;
/* Jitted code
00007FFCF0FCFE70  push        rbp  
00007FFCF0FCFE71  sub         rsp,20h  
00007FFCF0FCFE75  lea         rbp,[rsp+20h]  
00007FFCF0FCFE7A  mov         dword ptr [rbp+10h],ecx  
00007FFCF0FCFE7D  mov         rcx,7FFCF1090020h  
00007FFCF0FCFE87  call        qword ptr [0]  ; <-- ???
00007FFCF0FCFE8E  mov         edx,dword ptr [rbp+10h]  
00007FFCF0FCFE91  mov         dword ptr [rax],edx  
00007FFCF0FCFE93  mov         eax,dword ptr [rbp+10h]  
00007FFCF0FCFE96  lea         rsp,[rbp]  
00007FFCF0FCFE9A  pop         rbp  
00007FFCF0FCFE9B  ret  
*/
    }

    static int ManipulateThreadLocalValue_Workaround(int value) {
        set_thread_local_value(value); // Ok
        return get_thread_local_value();
    }
};

Expected behavior

  1. VC++ Compiler should warn about it if not supported. - Or -
  2. VC++ Compiler should emit suitable metadata to identify thread_local fields and let the .NET runtime to deal with them. - Or -
  3. VC++ Compiler should generate unmanaged stubs to manipulate thread_local fields for the managed code. - Or -
  4. .NET jitter should correctly handle unmanaged thread_local field access.

I'm not very sure it's the right place to report the bug. It's more likely a problem of VC++ Compiler.

category:correctness
theme:managed-c++
skill-level:beginner
cost:medium
impact:small

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI untriaged New issue has not been triaged by the area owner labels Sep 14, 2020
@BruceForstall
Copy link
Member

@LEI-Hongfaan What versions of VC++ and .NET are you using?

@BruceForstall
Copy link
Member

@dotnet/jit-contrib Anyone familiar with the intersection of VC++ /clr, thread local variables, and JIT?

@jkotas
Copy link
Member

jkotas commented Sep 14, 2020

This is related to CORINFO_HELP_GETSTATICFIELDADDR_TLS JIT helper:

JITHELPER(CORINFO_HELP_GETSTATICFIELDADDR_TLS, NULL, CORINFO_HELP_SIG_CANNOT_USE_ALIGN_STUB)

It looks like this helper was not brought back as part of the adding managed C++ support back.

@BruceForstall
Copy link
Member

@jkotas, is this a .NET 5 issue?

@jkotas
Copy link
Member

jkotas commented Sep 14, 2020

.NET 3.1 shipped with this bug. It is not a .NET 3.1 -> 5.0 regression.

We should either fix in both 3.1 and 5.0 or neither.

@BruceForstall BruceForstall added this to the 6.0.0 milestone Sep 14, 2020
@BruceForstall BruceForstall removed the untriaged New issue has not been triaged by the area owner label Sep 14, 2020
@BruceForstall
Copy link
Member

Marking as 6.0 for now.

@BruceForstall BruceForstall added the JitUntriaged CLR JIT issues needing additional triage label Oct 28, 2020
@BruceForstall BruceForstall added bug and removed JitUntriaged CLR JIT issues needing additional triage labels Nov 7, 2020
@JulieLeeMSFT JulieLeeMSFT added the needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration label Mar 23, 2021
@AndyAyersMS
Copy link
Member

@BruceForstall still planning on fixing this in 6.0?

@BruceForstall BruceForstall modified the milestones: 6.0.0, Future May 27, 2021
@BruceForstall BruceForstall removed their assignment May 27, 2021
@JulieLeeMSFT JulieLeeMSFT removed the needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration label Jun 7, 2021
@MrMontana1889
Copy link

Is there an update on this issue?

I am in the process of testing the migration of one the products I develop and it uses TLS in a mixed-mode C++ project. The TLS code (__declspec(thread)) is in a statically linked C++ library (non-managed) but is accessed from managed C++/CLI (code is now CLI as it was old syntax but the migration was required to target .NET Core 3.1).

The exact same code works perfectly fine with .NET Framework 4.7.2 (which we will also be targeting).

If there is a work around on how to get this to work, I am open to any suggestions.

@Chaotikmind
Copy link

Having the same problem here, a simple thread_local static int, is making the application crash
windows 7, net 5.0
i get an access violation at the library initialization

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI bug
Projects
None yet
Development

No branches or pull requests

8 participants