Global initialization is not thread-safe (especially in DLLs) in Silver

IDE: Visual Studio 2015
Version: 10.0.0.2255
Target: Island (Windows 32-bit)
Description:

It seems that globals (file scope variables) are lazy initialized (this kinda makes sense given that Main isn’t called). The problem is that as far as I can see the initialization isn’t thread-safe.

For example, code similar to:

import rtl

struct SomeStruct {
    var s: String
    init() {
        s = "Hello, World!"
    }
}

var aStruct = SomeStruct()

@DllExport
@CallingConvention(CallingConvention.Stdcall)
@SymbolName("_Foo")
func fgdgdf() -> Bool {
    MessageBox(nil, "A Message", "Foo", 0)
    MessageBox(nil, aStruct.s.FirstChar, "Foo", 0)
    return true
}

results in something like:

SilverDLL1!SilverDLL1.<Global>::fgdgdf:
613d4470 55              push    ebp
613d4471 89e5            mov     ebp,esp
613d4473 833d40a33861ff  cmp     dword ptr [SilverDLL1!GC_n_smashed+0x8 (6138a340)],0FFFFFFFFh
613d447a 7405            je      SilverDLL1!SilverDLL1.<Global>::fgdgdf+0x11 (613d4481)
613d447c e80fffffff      call    SilverDLL1!SilverDLL1.<Global>::get_aStruct+0x30 (613d4390)
613d4481 6a00            push    0

Note the third instruction.

There’s some problem with the symbols (worthy of a separate post later perhaps) or WinDbg, but SilverDLL1!GC_n_smashed+0x8 is actually SilverDLL1!@_aStruct (according to the WinDbg ln command too).

This patter of comparing to 0xFFFFFFFF and jumping to get_aStruct happens all the time. The problem is that no one guarantees that the first access to a global comes before multiple threads are spawned. If multiple thread are present and are executing this code bad things may happen.

In an EXE this can be circumvented by forcing access to all the globals before starting threads, although this is not completely true1, but a DLL doesn’t control thr program flow. It gets loaded by someone else, perhaps after lots of threads already exist, and its exports may be immediately called by them.

C++ statics had a similar problem (being initialized the first time a function is called, not thread-safe in most implementation). This WG21 paper by a Google employee proposed an algorithm (“A Fast Implementation of Synchronized Initialization”) and it’s actualyl being used by MSVC for a couple of years now. Or you can implement in any other way you want.


1 Windows may start threads on its own for stuff like RPC, and actually since Windows 10 the loader is parallelized and thus every process starts with a bunch of threads for loading all the static imports.

Hrmm. that’s fairly odd; Your code compiles to (removed the irrelevant stuff):

getter for Struct:

define i32 @ms_t13_issuez_d_____Globalc_get__aStruct() #0 {
BasicBlock0:
  %0 = load i32, i32* @_cctorinit_t13_issuez_d_____Global, align 4
  %1 = icmp eq i32 %0, -1
  br i1 %1, label %BasicBlock1, label %2

; <label>:2:                                      ; preds = %BasicBlock0
  tail call void @_cctorinit_t13_issuez_d_____Global_helper()
  br label %BasicBlock1, !dbg !67

BasicBlock1:                                      ; preds = %BasicBlock0, %2
  %.unpack2 = load i32, i32* bitcast (%._issuez.SomeStruct* @f_t13_issuez_d_____Global._at_aStruct to i32*), align 4, !dbg !69
  ret i32 %.unpack2, !dbg !69
}

Logically this translates to:

Helper method for class constructor for the __Global class:

define void @_cctorinit_t13_issuez_d_____Global_helper() {
BasicBlock16:
  %0 = tail call i8 @SpinLockClassEnter(i32* nonnull @_cctorinit_t13_issuez_d_____Global)
  %1 = and i8 %0, 1
  %2 = icmp eq i8 %1, 0
  br i1 %2, label %BasicBlock19, label %BasicBlock17

BasicBlock17:                                     ; preds = %BasicBlock16
  call void @ms_t13_issuez_d_____Global8__d_cctor() #0
  tail call void @SpinLockClassExit(i32* nonnull @_cctorinit_t13_issuez_d_____Global)
  br %BasicBlock19

%BasicBlock19:
  ret void
}

Actual class constructor:

define void @ms_t13_issuez_d_____Global8__d_cctor() #0 !dbg !62 {
BasicBlock13:
  %0 = alloca %._issuez.SomeStruct, align 4
  call void @mi_t13_issuez_d_SomeStruct3__d_(%._issuez.SomeStruct* nonnull %0), !dbg !87
  %1 = bitcast %._issuez.SomeStruct* %0 to i32*, !dbg !87
  %.fca.0.load1 = load i32, i32* %1, align 4, !dbg !87
  store i32 %.fca.0.load1, i32* bitcast (%._issuez.SomeStruct* @f_t13_issuez_d_____Global._at_aStruct to i32*), align 4, !dbg !87
  ret void, !dbg !87
}
method get_Struct: Struct;
begin
  if _cctorinit_t13_issuez_d_____Global <> -1 then _cctorinit_t13_issuez_d_____Global_helper;
  exit fStruct;
end;

method _cctorinit_t13_issuez_d_____Global_helper;
begin
  if SpinLockClassEnter(_cctorinit_t13_issuez_d_____Global) then 
  try
   ms_t13_issuez_d_____Global8__d_cctor();
  finally
    SpinLockClassExit(_cctorinit_t13_issuez_d_____Global);
end;

method ms_t13_issuez_d_____Global8__d_cctor;
begin
   var x: Struct := default(Struct);
  __ctor_for_struct(@x);
end;

The two spinlock methods are defined here:

So it should be thread safe?

Essentially, the first check:

  if _cctorinit_t13_issuez_d_____Global <> -1 then _cctorinit_t13_issuez_d_____Global_helper;

is what’s called a double checked lock. -1 is the “this is already initialized” value (default is 0). It checks again in the helper with interlocked checks.

My apologies. You’re right. I looked at the external check and assumed it was all. Now that I looked inside I see the spinlock protecting the initialization.

However experimenting with DllMain the way you suggested on my other post I noticed that if I have DllEntry function, the initialization happens in it, even if the function does not refer to the global (see example below), at least in debug builds.

This should be at least documented since this means that if there’s a user-defined DLL entry point, global initialization would happen with the loader lock held, which puts considerable restrictions on what is allowed to bone. (Or changed so that the initialization doesn’t happen in this function.)

The code is a simple variations on the example I put in my recent posts:

import rtl

struct SomeStruct {
    var s: String
    init() {
        s = "Hello, World!"
    }
}

var aStruct = SomeStruct()
var i : uint32_t

@SymbolName("__elements_dll_entry")
static func DllEntry() {
    i = 5
}

@DllExport
@CallingConvention(CallingConvention.Stdcall)
@SymbolName("_Foo")
func fgdgdf() -> Bool {
    MessageBox(nil, "A Message", "Foo", 0)
    MessageBox(nil, aStruct.s.FirstChar, "Foo", 0)
    return true
}

The constructor is called at this point:

0:000> k
 # ChildEBP RetAddr  
00 004ff26c 5bb8b43b SilverDLL1!SilverDLL1.SomeStruct::. [\__windows_drive__c\users\conio\documents\visual studio 2015\projects\silverdll1\silverdll1\program.swift @ 5] 
01 004ff27c 5bb8b3da SilverDLL1!SilverDLL1.<Global>::.cctor+0x1b [\__windows_drive__c\users\conio\documents\visual studio 2015\projects\silverdll1\silverdll1\program.swift @ 10] 
02 004ff2a0 5bb8b4c1 SilverDLL1!SilverDLL1.<Global>::get_aStruct+0x7a
03 004ff2a8 5bbb651b SilverDLL1!SilverDLL1.<Global>::DllEntry+0x11 [\__windows_drive__c\users\conio\documents\visual studio 2015\projects\silverdll1\silverdll1\program.swift @ 15] 
04 004ff2b4 771be746 SilverDLL1!RemObjects.Elements.System.ExternalCalls::DllMainCRTStartup+0x2b [\__windows_drive__c\ci\b\elements\937\source\islandrtl\source\windowshelpers.pas @ 1122] 
05 004ff2d4 7718cbef ntdll!LdrxCallInitRoutine+0x16
06 004ff320 77187baa ntdll!LdrpCallInitRoutine+0x7f
07 004ff3a0 77187a44 ntdll!LdrpInitializeNode+0x10e
08 004ff3c4 771a014d ntdll!LdrpInitializeGraphRecurse+0x5d
09 004ff3e0 77189012 ntdll!LdrpPrepareModuleForExecution+0x8f
0a 004ff428 7719b63e ntdll!LdrpLoadDllInternal+0x128
0b 004ff574 7719e8de ntdll!LdrpLoadDll+0xa2
0c 004ff5f8 753eda74 ntdll!LdrLoadDll+0x7e
0d 004ff63c 753eb881 KERNELBASE!LoadLibraryExW+0x144
0e 004ff650 0130172b KERNELBASE!LoadLibraryW+0x11
0f 004ff740 01301e8e SilverLoader1!main+0x2b [c:\users\conio\documents\visual studio 2015\projects\silverloader1\silverloader1\silverloader1.cpp @ 20] 
10 004ff754 01301cf0 SilverLoader1!invoke_main+0x1e [f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl @ 64] 
11 004ff7ac 01301b8d SilverLoader1!__scrt_common_main_seh+0x150 [f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl @ 253] 
12 004ff7b4 01301ea8 SilverLoader1!__scrt_common_main+0xd [f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl @ 296] 
13 004ff7bc 76ca8654 SilverLoader1!mainCRTStartup+0x8 [f:\dd\vctools\crt\vcstartup\src\startup\exe_main.cpp @ 17] 
14 004ff7d0 771b4a77 KERNEL32!BaseThreadInitThunk+0x24
15 004ff818 771b4a47 ntdll!__RtlUserThreadStart+0x2f
16 004ff828 00000000 ntdll!_RtlUserThreadStart+0x1b

By the way I noticed another problem I’m pretty sure is valid :slight_smile: - Even if I change Generate Debug Symbols to True, I don’t get a PDB when building in Release mode. That’s why I couldn’t verify this happens in Release too. :slight_smile:

And since we’re at it, one more question - I noticed that when building a DLL I get a small .fx file produced along with the DLL (and the PDB, when in Debug). This doesn’t happen for Exe and WinExe projects. Is there a reason for that? Do the .fx files have any function or use on Windows?

Thanks, logged as bugs://79748

Well you could put the dllentry/dllexit as static methods inside a class; that will ensure they don’t hit for globals. But agreed this should be documented.

Logged a bug for that.

fx files are our metadata files. They don’t do anything on Windows but they will let you reference that dll directly from another Elements project (only contains the exported classes/functions though). you can’t do that with exe and winexe of course.

Checked it; the lack of pdb is because of GeneratePDB. That’s currently not exposed in the gui but if you open the project file in a text editor, under the Release profile, add or set <GeneratePDB>True</GeneratePDB>

:+1:

I guessed as much, but there’s a documentation page saying they’re needed only for Cocoa targets so I thought I’d ask.

I just tried it and it doesn’t work. The Release configuration (to which I added the <GeneratePDB>) fails with:

C:\Program Files (x86)\MSBuild\RemObjects Software\Elements\RemObjects.Elements.Island.Windows.targets(84,5): error : C:\Program Files (x86)\RemObjects Software\Elements\Bin\lld.exe: error: could not open '/pdb:SilverDLL1.dll': no such file or directory

They used to be, before Island existed. i’ll review/revise the docs.

The page I’m referring to is: .fx Files

Thanks again for all the help!

1 Like

Fixed: https://docs.elementscompiler.com/Projects/References/FXFiles/

1 Like

Thanks!

Do you by any chance understand the problem I described above with the <GeneratePDB>True</GeneratePDB> tag @ck suggested?

I saw it. I can confirm it happens, but don’t know why just yet.

1 Like

Replace RemObjects.Elements.Island.Windows.targets in
C:\Program Files (x86)\MSBuild\RemObjects Software\Elements
and
C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\MSBuild\RemObjects Software\Elements\RemObjects.Elements.Island.Windows.targets

with:

<Project DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">

  <PropertyGroup>
    <IslandTargetArchitecture>$(Architecture)</IslandTargetArchitecture>
    <IslandTargetTriple></IslandTargetTriple>
    <IslandTargetTriple Condition="'$(IslandTargetArchitecture)'=='i386'">i686-pc-windows-msvc</IslandTargetTriple>
    <IslandTargetTriple Condition="'$(IslandTargetArchitecture)'=='i686'">i686-pc-windows-msvc</IslandTargetTriple>
    <IslandTargetTriple Condition="'$(IslandTargetArchitecture)'=='x86_64'">x86_64-pc-windows-msvc</IslandTargetTriple>

    <IslandTargetArchitecture Condition="'$(IslandTargetTriple)'==''">i386</IslandTargetArchitecture>
    <IslandTargetTriple Condition="'$(IslandTargetTriple)'==''">i686-pc-windows-msvc</IslandTargetTriple>

    <IslandTargetOS Condition="'$(IslandTargetOS)'==''">Windows</IslandTargetOS>
    <IslandTargetExt Condition="'$(IslandTargetExt)' == '' and ('$(OutputType)'=='winexe' or '$(OutputType)' == 'winexecutable')">.exe</IslandTargetExt>
    <IslandTargetExt Condition="'$(IslandTargetExt)' == '' and ('$(OutputType)'=='exe' or '$(OutputType)' == 'executable')">.exe</IslandTargetExt>
    <IslandTargetExt Condition="'$(IslandTargetExt)' == '' and '$(OutputType)'=='library'">.dll</IslandTargetExt>
    <IslandTargetExt Condition="'$(IslandTargetExt)' == '' and '$(OutputType)'=='staticlibrary'">.lib</IslandTargetExt>
    <IslandTargetExt Condition="'$(IslandTargetExt)' == '' and '$(OutputType)'=='object'">.o</IslandTargetExt>
    
    <IslandDebuggerExpectedUName>Windows</IslandDebuggerExpectedUName>
    <IslandDebuggerType>Island</IslandDebuggerType>
    <IslandDebuggerRemoteDebuggerType>Elements</IslandDebuggerRemoteDebuggerType>
    
    <ArchiveOutputType>coff</ArchiveOutputType>

    <IslandTargetType>Windows</IslandTargetType>
    <TargetPrefix></TargetPrefix>
    <TargetExt>.fx</TargetExt>
    <TargetFileName Condition=" '$(TargetFileName)' == ''">$(TargetPrefix)$(AssemblyName)$(TargetExt)</TargetFileName>
    <TargetPath Condition=" '$(TargetPath)' == '' ">$(OutputPath)\$(IslandTargetType)\$(IslandTargetArchitecture)\$(TargetFileName)</TargetPath>
    <IslandTargetFileName>$(TargetPrefix)$(AssemblyName)$(IslandTargetExt)</IslandTargetFileName>
    <IslandTargetPDBFileName>$(TargetPrefix)$(AssemblyName)$(IslandTargetExt).pdb</IslandTargetPDBFileName>
    <OutDir>$(OutputPath)\$(IslandTargetType)\$(IslandTargetArchitecture)</OutDir>
  </PropertyGroup>
  <Import Project="$(MSBuildExtensionsPath)\RemObjects Software\Elements\RemObjects.Elements.Island.targets" />
  <PropertyGroup>
    <CompilerFlags>--codeview+ --thinlto</CompilerFlags>
    <IntermediateOutputPath>$(IntermediateOutputPath)\$(IslandTargetType)\</IntermediateOutputPath>
    <OutputLibraryName Condition="'$(OutputType)'=='library' or '$(OutputType)'=='staticlibrary' or '$(OutputType)'=='object'">$(IslandTargetFileName)</OutputLibraryName>
    <AvailablePlatforms>i386,x86_64</AvailablePlatforms>
  </PropertyGroup>
  <ItemGroup>
    <IslandBinFiles Include="$(IntermediateOutputPath)\$(IslandTargetFileName)"/>
    <IslandBinFiles Include="$(IntermediateOutputPath)\$(IslandTargetFileName).pdb"/>
    <IslandOutputBinFiles Include="$(OutDir)\$(IslandTargetFileName)"/>
  </ItemGroup>
  <Target Name="IslandLink" DependsOnTargets="$(IslandLinkDependsOn)" Condition="'@(GeneratedOutput)'!='' and '$(OutputType)'!='object' and '$(OutputType)'!='staticlibrary'">
    <ResolveIslandPath Parameter="LLD" FailIfMissing="False">
      <Output TaskParameter="Path" PropertyName="IslandLinkerExecutable" />
    </ResolveIslandPath>
    <ResolveIslandPath Parameter="DefaultLLD" Condition="$(IslandLinkerExecutable) == ''">
      <Output TaskParameter="Path" PropertyName="IslandLinkerExecutable" />
    </ResolveIslandPath>

    <SplitString Input="$(IslandLibraries)">
      <Output TaskParameter="Items" ItemName="IslandLibrariesSplit" />
    </SplitString>
    <ItemGroup>
      <IslandLinkerCommand Remove="@(IslandLinkerCommand)" />
      <IslandLinkerCommand Condition="'$(Optimize)'!='True'" Include="/opt:lldlto=0" />
      <IslandLinkerCommand Include="/libpath:$(IntermediateOutputPath)/."/>
      <IslandLinkerCommand Include="/out:$(IntermediateOutputPath)\$(IslandTargetFileName)"/>
      <IslandLinkerCommand Include="@(IslandReferencePaths->'&quot;/libpath:%(FullPath)&quot; ')" />
      <IslandLinkerCommand Include="@(GeneratedOutput->'&quot;%(fullpath)&quot; ')" />
      <IslandLinkerCommand Include="@(IslandLibrariesSplit->'&quot;%(fullpath)&quot; ')" />
      <IslandLinkerCommand Include="$(IslandReferencesLinkerOptions)" />
      <IslandLinkerCommand Include="/nodefaultlib:uuid.lib" />
      <IslandLinkercommand Include="/pdb:&quot;$(IntermediateOutputPath)\$(IslandTargetPDBFileName)&quot;" Condition="'$(GenerateDebugInfo)' == 'true' and '$(GeneratePDB)'=='true'" />
      <IslandLinkerCommand Condition="'$(OutputType)'=='library'" Include="/dll" />
      <IslandLinkerCommand Condition="'$(OutputType)'=='library' and '$(IslandDefFile)' != ''" Include="/def:$(IslandDefFile)" />
      <IslandLinkerCommand Condition="'$(OutputType)'=='exe'" Include="/subsystem:Console" />
      <IslandLinkerCommand Condition="'$(OutputType)'=='winexe'" Include="/subsystem:Windows" />
      <IslandLinkerCommand Condition="'$(GenerateDebugInfo)' == 'true' or '$(GeneratePDB)'=='true'" Include="$(IslandLinkerCommand) /debug" />
    </ItemGroup>
    <PropertyGroup>
      <IslandLinkerTemp>$(IntermediateOutputPath)\linkercmd</IslandLinkerTemp>
    </PropertyGroup>
    <DeleteFileEx Filename="$(OutDir)\$(IslandTargetFileName)" />
    <WriteLinesToFile
          File="$(IslandLinkerTemp)"
          Lines="@(IslandLinkerCommand)"
          Overwrite="true"/>

    <ExecuteLinker Executable="$(IslandLinkerExecutable)" Arguments="-flavor link &quot;@$(IslandLinkerTemp)&quot;" />
  </Target>
</Project>

bugs://79748 got closed with status fixed.

Thanks. I tried it and it fixed the error to lld.exe caused by the <GeneratePDB> tag, but I still didn’t see a PDB. Diffing against the previous .targets file I noticed the line
<IslandLinkercommand Include="/pdb:'$(IntermediateOutputPath)\$(IslandTargetPDBFileName)'" Condition="'$(GenerateDebugInfo)' == 'true' and '$(GeneratePDB)'=='true'" />

For some reason the PDB gets to the bin folder in Debug builds but stays in the obj folder in Release build. When I noticed it I reverted to the previous .targets file and noticed that it too generates a PDB (with <GenerateDebugInfo> but without <GeneratePDB>) but keeps it in the obj directory.

what happens if you use EBuild?