fix(codegen-native): address record/aggregate threadvars via TLS
A threadvar of record (or any aggregate whose base address is taken via
leaq) was addressed with a bare `leaq Name(%rip)` for both direct field
access and method-call receivers, with no %fs:0/@tpoff. The symbol was
correctly placed in .tbss, but a %rip-relative reference to a TLS symbol
does not resolve to the per-thread slot, so all threads shared one static
slot and clobbered each other. Scalar threadvars were already correct
(VarOperand emits %fs:Name@tpoff); QBE was unaffected.
Route every global record/array base load through the TLS-aware
EmitLeaqGlobal helper instead of bare `leaq Name(%rip)`:
- 16 field-access base ladders (FAE/FA/AFA.RecordName)
- 4 record-method receiver sites (the >6-slot and <=6-slot ladders)
- the offset-0 scalar-field-read fast path, which read the field
directly off Name(%rip); now branches on IsThreadVarGlobal and falls
through to EmitLeaqGlobal + indirect load for threadvars.
EmitLeaqGlobal emits the identical bare leaq for non-threadvar globals,
so codegen for ordinary globals is byte-identical (verified by .s diff)
and all four fixpoints are unchanged.
Tests: cp.test.e2e.threading gains
TestRun_ThreadVar_RecordField_PerThreadIsolation and
TestRun_ThreadVar_RecordMethod_PerThreadIsolation — 3 threads each stamp
a distinct id into a record threadvar (via field access / via method
call), spin re-reading, and assert no cross-thread clobber. Both fail
before the fix (native prints CORRUPT) and pass after.
This commit is contained in:
Graeme Geldenhuys
parent
180fbb62b4
commit
a86fb84eea
|
|
@ -7231,7 +7231,7 @@ begin
|
|||
else if Self.IsLocal(FAE.RecordName) then
|
||||
Self.EmitLocalRecordBase(FAE.RecordName, '%rcx')
|
||||
else
|
||||
Self.Emit(Format(#9'leaq %s(%%rip), %%rcx', [FAE.RecordName]));
|
||||
Self.EmitLeaqGlobal(FAE.RecordName, '%rcx');
|
||||
if FAE.FieldInfo.Offset > 0 then
|
||||
Self.Emit(Format(#9'addq $%d, %%rcx', [FAE.FieldInfo.Offset]));
|
||||
Self.Emit(#9'movq (%rcx), %rcx'); { string data pointer }
|
||||
|
|
@ -7281,7 +7281,7 @@ begin
|
|||
else if Self.IsLocal(FAE.RecordName) then
|
||||
Self.EmitLocalRecordBase(FAE.RecordName, '%rcx')
|
||||
else
|
||||
Self.Emit(Format(#9'leaq %s(%%rip), %%rcx', [FAE.RecordName]));
|
||||
Self.EmitLeaqGlobal(FAE.RecordName, '%rcx');
|
||||
if FAE.FieldInfo.Offset > 0 then
|
||||
Self.Emit(Format(#9'addq $%d, %%rcx', [FAE.FieldInfo.Offset]));
|
||||
if FAE.FieldInfo.TypeDesc.Kind = tyDynArray then
|
||||
|
|
@ -7479,17 +7479,22 @@ begin
|
|||
if (IsJumboSet(FAE.FieldInfo.TypeDesc) or
|
||||
(FAE.FieldInfo.TypeDesc.Kind in [tyRecord, tyStaticArray])) then
|
||||
begin
|
||||
Self.Emit(Format(#9'leaq %s(%%rip), %%rcx', [FAE.RecordName]));
|
||||
Self.EmitLeaqGlobal(FAE.RecordName, '%rcx');
|
||||
if FAE.FieldInfo.Offset = 0 then
|
||||
Self.Emit(#9'movq %rcx, %rax')
|
||||
else
|
||||
Self.Emit(Format(#9'leaq %d(%%rcx), %%rax', [FAE.FieldInfo.Offset]));
|
||||
end
|
||||
else if FAE.FieldInfo.Offset = 0 then
|
||||
else if (FAE.FieldInfo.Offset = 0) and
|
||||
(not Self.IsThreadVarGlobal(FAE.RecordName)) then
|
||||
{ Non-threadvar offset-0 fast path: read the field directly off the
|
||||
PC-relative global. threadvars must not use this — the slot is
|
||||
TLS-resident, so fall through to EmitLeaqGlobal (which emits the
|
||||
%fs:0 + @tpoff sequence) and an indirect load. }
|
||||
Self.EmitLoadVar(FAE.RecordName + '(%rip)', FAE.FieldInfo.TypeDesc)
|
||||
else
|
||||
begin
|
||||
Self.Emit(Format(#9'leaq %s(%%rip), %%rcx', [FAE.RecordName]));
|
||||
Self.EmitLeaqGlobal(FAE.RecordName, '%rcx');
|
||||
Self.EmitLoadVar(Format('%d(%%rcx)', [FAE.FieldInfo.Offset]),
|
||||
FAE.FieldInfo.TypeDesc);
|
||||
end;
|
||||
|
|
@ -7695,7 +7700,7 @@ begin
|
|||
if Self.IsLocal(FAE.RecordName) then
|
||||
Self.EmitLocalRecordBase(FAE.RecordName, '%rcx')
|
||||
else
|
||||
Self.Emit(Format(#9'leaq %s(%%rip), %%rcx', [FAE.RecordName]));
|
||||
Self.EmitLeaqGlobal(FAE.RecordName, '%rcx');
|
||||
end;
|
||||
if FAE.FieldInfo.Offset > 0 then
|
||||
Self.Emit(Format(#9'addq $%d, %%rcx', [FAE.FieldInfo.Offset]));
|
||||
|
|
@ -7772,7 +7777,7 @@ begin
|
|||
if Self.IsLocal(FAE.RecordName) then
|
||||
Self.EmitLocalRecordBase(FAE.RecordName, '%rcx')
|
||||
else
|
||||
Self.Emit(Format(#9'leaq %s(%%rip), %%rcx', [FAE.RecordName]));
|
||||
Self.EmitLeaqGlobal(FAE.RecordName, '%rcx');
|
||||
end;
|
||||
if (FAE.FieldInfo <> nil) and (FAE.FieldInfo.Offset > 0) then
|
||||
Self.Emit(Format(#9'addq $%d, %%rcx', [FAE.FieldInfo.Offset]));
|
||||
|
|
@ -8273,7 +8278,7 @@ begin
|
|||
if Self.IsLocal(ACall.ObjectName) then
|
||||
Self.Emit(Format(#9'leaq %s, %%r10', [Self.VarOperand(ACall.ObjectName)]))
|
||||
else
|
||||
Self.Emit(Format(#9'leaq %s(%%rip), %%r10', [ACall.ObjectName]));
|
||||
Self.EmitLeaqGlobal(ACall.ObjectName, '%r10');
|
||||
end
|
||||
else if ACall.IsVarParam then
|
||||
begin
|
||||
|
|
@ -8489,7 +8494,7 @@ begin
|
|||
else if Self.IsLocal(FAE.RecordName) then
|
||||
Self.EmitLocalRecordBase(FAE.RecordName, '%rax')
|
||||
else
|
||||
Self.Emit(Format(#9'leaq %s(%%rip), %%rax', [FAE.RecordName]));
|
||||
Self.EmitLeaqGlobal(FAE.RecordName, '%rax');
|
||||
if FAE.FieldInfo.Offset > 0 then
|
||||
Self.Emit(Format(#9'addq $%d, %%rax', [FAE.FieldInfo.Offset]));
|
||||
end
|
||||
|
|
@ -9298,7 +9303,7 @@ begin
|
|||
else if Self.IsLocal(ACall.ObjectName) then
|
||||
Self.Emit(Format(#9'leaq %s, %%r10', [Self.VarOperand(ACall.ObjectName)]))
|
||||
else
|
||||
Self.Emit(Format(#9'leaq %s(%%rip), %%r10', [ACall.ObjectName]));
|
||||
Self.EmitLeaqGlobal(ACall.ObjectName, '%r10');
|
||||
end
|
||||
else if ACall.IsVarParam then
|
||||
begin
|
||||
|
|
@ -9360,7 +9365,7 @@ begin
|
|||
else if Self.IsLocal(ACall.ObjectName) then
|
||||
Self.Emit(Format(#9'leaq %s, %%rax', [Self.VarOperand(ACall.ObjectName)]))
|
||||
else
|
||||
Self.Emit(Format(#9'leaq %s(%%rip), %%rax', [ACall.ObjectName]));
|
||||
Self.EmitLeaqGlobal(ACall.ObjectName, '%rax');
|
||||
end
|
||||
else if ACall.IsVarParam then
|
||||
begin
|
||||
|
|
@ -12054,7 +12059,7 @@ begin
|
|||
else if Self.IsLocal(FA.RecordName) then
|
||||
Self.Emit(Format(#9'leaq %s, %%rdx', [Self.VarOperand(FA.RecordName)]))
|
||||
else
|
||||
Self.Emit(Format(#9'leaq %s(%%rip), %%rdx', [FA.RecordName]));
|
||||
Self.EmitLeaqGlobal(FA.RecordName, '%rdx');
|
||||
if FA.FieldInfo.Offset > 0 then
|
||||
Self.Emit(Format(#9'addq $%d, %%rdx', [FA.FieldInfo.Offset]));
|
||||
if FA.FieldInfo.TypeDesc.Kind = tyDynArray then
|
||||
|
|
@ -12148,7 +12153,7 @@ begin
|
|||
else if Self.IsLocal(FA.RecordName) then
|
||||
Self.EmitLocalRecordBase(FA.RecordName, '%rcx')
|
||||
else
|
||||
Self.Emit(Format(#9'leaq %s(%%rip), %%rcx', [FA.RecordName]));
|
||||
Self.EmitLeaqGlobal(FA.RecordName, '%rcx');
|
||||
Self.EmitInterfaceToFieldSlotsAt(FA.Expr, '%rcx', FA.FieldInfo.Offset,
|
||||
FA.FieldInfo.TypeDesc);
|
||||
Exit;
|
||||
|
|
@ -12204,7 +12209,7 @@ begin
|
|||
else if Self.IsLocal(FA.RecordName) then
|
||||
Self.EmitLocalRecordBase(FA.RecordName, '%rcx')
|
||||
else
|
||||
Self.Emit(Format(#9'leaq %s(%%rip), %%rcx', [FA.RecordName]));
|
||||
Self.EmitLeaqGlobal(FA.RecordName, '%rcx');
|
||||
if FA.FieldInfo.Offset > 0 then
|
||||
Self.Emit(Format(#9'addq $%d, %%rcx', [FA.FieldInfo.Offset]));
|
||||
{ Store the code pointer at offset 0. }
|
||||
|
|
@ -12276,7 +12281,7 @@ begin
|
|||
if Self.IsLocal(FA.RecordName) then
|
||||
Self.Emit(Format(#9'leaq %s, %%rbx', [Self.VarOperand(FA.RecordName)]))
|
||||
else
|
||||
Self.Emit(Format(#9'leaq %s(%%rip), %%rbx', [FA.RecordName]));
|
||||
Self.EmitLeaqGlobal(FA.RecordName, '%rbx');
|
||||
end;
|
||||
if FA.FieldInfo.Offset > 0 then
|
||||
Self.Emit(Format(#9'addq $%d, %%rbx', [FA.FieldInfo.Offset]));
|
||||
|
|
@ -12323,7 +12328,7 @@ begin
|
|||
if Self.IsLocal(FA.RecordName) then
|
||||
Self.Emit(Format(#9'leaq %s, %%rbx', [Self.VarOperand(FA.RecordName)]))
|
||||
else
|
||||
Self.Emit(Format(#9'leaq %s(%%rip), %%rbx', [FA.RecordName]));
|
||||
Self.EmitLeaqGlobal(FA.RecordName, '%rbx');
|
||||
end;
|
||||
if FA.FieldInfo.Offset > 0 then
|
||||
Self.Emit(Format(#9'addq $%d, %%rbx', [FA.FieldInfo.Offset]));
|
||||
|
|
@ -12394,7 +12399,7 @@ begin
|
|||
if Self.IsLocal(FA.RecordName) then
|
||||
Self.EmitLocalRecordBase(FA.RecordName, '%rcx')
|
||||
else
|
||||
Self.Emit(Format(#9'leaq %s(%%rip), %%rcx', [FA.RecordName]));
|
||||
Self.EmitLeaqGlobal(FA.RecordName, '%rcx');
|
||||
end;
|
||||
if FA.FieldInfo.Offset > 0 then
|
||||
Self.Emit(Format(#9'addq $%d, %%rcx', [FA.FieldInfo.Offset]));
|
||||
|
|
@ -12438,7 +12443,7 @@ begin
|
|||
else if Self.IsLocal(FA.RecordName) then
|
||||
Self.EmitLocalRecordBase(FA.RecordName, '%rcx')
|
||||
else
|
||||
Self.Emit(Format(#9'leaq %s(%%rip), %%rcx', [FA.RecordName]));
|
||||
Self.EmitLeaqGlobal(FA.RecordName, '%rcx');
|
||||
if FA.FieldInfo.Offset > 0 then
|
||||
Self.Emit(Format(#9'addq $%d, %%rcx', [FA.FieldInfo.Offset]));
|
||||
Self.Emit(#9'pushq %rbx');
|
||||
|
|
@ -12714,7 +12719,7 @@ begin
|
|||
if Self.IsLocal(FA.RecordName) then
|
||||
Self.EmitLocalRecordBase(FA.RecordName, '%rcx')
|
||||
else
|
||||
Self.Emit(Format(#9'leaq %s(%%rip), %%rcx', [FA.RecordName]));
|
||||
Self.EmitLeaqGlobal(FA.RecordName, '%rcx');
|
||||
if FA.FieldInfo.TypeDesc.IsString() then
|
||||
begin
|
||||
if not NativeExprOwnsRef(FA.Expr) then
|
||||
|
|
@ -14661,7 +14666,7 @@ begin
|
|||
else if Self.IsLocal(AFA.RecordName) then
|
||||
Self.EmitLocalRecordBase(AFA.RecordName, '%rcx')
|
||||
else
|
||||
Self.Emit(Format(#9'leaq %s(%%rip), %%rcx', [AFA.RecordName]));
|
||||
Self.EmitLeaqGlobal(AFA.RecordName, '%rcx');
|
||||
if AFA.FieldInfo.Offset > 0 then
|
||||
Self.Emit(Format(#9'addq $%d, %%rcx', [AFA.FieldInfo.Offset]));
|
||||
end;
|
||||
|
|
@ -15450,7 +15455,7 @@ begin
|
|||
if Self.IsLocal(ACall.ObjectName) then
|
||||
Self.Emit(Format(#9'leaq %s, %%rsi', [Self.VarOperand(ACall.ObjectName)]))
|
||||
else
|
||||
Self.Emit(Format(#9'leaq %s(%%rip), %%rsi', [ACall.ObjectName]));
|
||||
Self.EmitLeaqGlobal(ACall.ObjectName, '%rsi');
|
||||
end
|
||||
else if ACall.IsVarParam then
|
||||
begin
|
||||
|
|
|
|||
|
|
@ -32,6 +32,8 @@ type
|
|||
procedure TestRun_Thread_InheritedDestroy_CleanExit;
|
||||
procedure TestRun_ThreadVar_MainThread_ReadWrite;
|
||||
procedure TestRun_ThreadVar_MixedWithGlobalVar;
|
||||
procedure TestRun_ThreadVar_RecordField_PerThreadIsolation;
|
||||
procedure TestRun_ThreadVar_RecordMethod_PerThreadIsolation;
|
||||
procedure TestRun_PerThreadAllocator_IndependentAllocs;
|
||||
procedure TestRun_AtomicARC_SharedObject_NoCorruption;
|
||||
end;
|
||||
|
|
@ -367,6 +369,154 @@ begin
|
|||
AssertEquals('stdout', '30' + LE, Output)
|
||||
end;
|
||||
|
||||
const
|
||||
{ A RECORD-typed threadvar mutated and read back per-thread. Each thread
|
||||
stamps its own id into the record-field threadvar, then spins re-reading it
|
||||
and asserting it never changes. If the record threadvar is addressed
|
||||
statically (leaq Name(%rip)) instead of via TLS (%fs:0 + @tpoff), all
|
||||
threads share one slot and clobber each other -> 'CORRUPT'. Scalar
|
||||
threadvars already use TLS; the bug was that record/aggregate threadvar
|
||||
field access bypassed it. }
|
||||
SrcThreadVarRecordIsolation =
|
||||
'''
|
||||
program P;
|
||||
uses Classes;
|
||||
type
|
||||
TRec = record
|
||||
Tag: Integer;
|
||||
end;
|
||||
TStampThread = class(TThread)
|
||||
MyId: Integer;
|
||||
protected
|
||||
procedure Execute; override;
|
||||
end;
|
||||
threadvar
|
||||
TVR: TRec;
|
||||
var
|
||||
Corrupt: Integer;
|
||||
procedure TStampThread.Execute;
|
||||
var I: Integer;
|
||||
begin
|
||||
TVR.Tag := Self.MyId;
|
||||
for I := 0 to 200000 do
|
||||
if TVR.Tag <> Self.MyId then
|
||||
begin
|
||||
Corrupt := 1;
|
||||
Exit
|
||||
end
|
||||
end;
|
||||
var
|
||||
A, B, C: TStampThread;
|
||||
begin
|
||||
Corrupt := 0;
|
||||
A := TStampThread.Create(True);
|
||||
B := TStampThread.Create(True);
|
||||
C := TStampThread.Create(True);
|
||||
A.MyId := 11;
|
||||
B.MyId := 22;
|
||||
C.MyId := 33;
|
||||
A.Start();
|
||||
B.Start();
|
||||
C.Start();
|
||||
A.WaitFor();
|
||||
B.WaitFor();
|
||||
C.WaitFor();
|
||||
if Corrupt = 0 then
|
||||
WriteLn('ok')
|
||||
else
|
||||
WriteLn('CORRUPT')
|
||||
end.
|
||||
''';
|
||||
|
||||
procedure TE2EThreadingTests.TestRun_ThreadVar_RecordField_PerThreadIsolation;
|
||||
var
|
||||
Output: string;
|
||||
RCode: Integer;
|
||||
begin
|
||||
if not ToolchainAvailable() then begin Ignore('toolchain unavailable'); Exit end;
|
||||
AssertTrue('compile+run', CompileAndRunWithRTL(SrcThreadVarRecordIsolation, Output, RCode));
|
||||
AssertEquals('exit code', 0, RCode);
|
||||
AssertEquals('stdout', 'ok' + LE, Output)
|
||||
end;
|
||||
|
||||
const
|
||||
{ Same per-thread isolation guarantee, but the record threadvar is accessed
|
||||
through METHOD CALLS (GR.Put / GR.Get) rather than direct field access. The
|
||||
method-call receiver ladder loads the record base too, and used the same
|
||||
static `leaq Name(%rip)` for a global receiver — so this exercises the
|
||||
receiver-load path specifically. }
|
||||
SrcThreadVarRecordMethod =
|
||||
'''
|
||||
program P;
|
||||
uses Classes;
|
||||
type
|
||||
TRec = record
|
||||
N: Integer;
|
||||
procedure Put(V: Integer);
|
||||
function Get: Integer;
|
||||
end;
|
||||
TStampThread = class(TThread)
|
||||
MyId: Integer;
|
||||
protected
|
||||
procedure Execute; override;
|
||||
end;
|
||||
procedure TRec.Put(V: Integer);
|
||||
begin
|
||||
N := V
|
||||
end;
|
||||
function TRec.Get: Integer;
|
||||
begin
|
||||
Result := N
|
||||
end;
|
||||
threadvar
|
||||
GR: TRec;
|
||||
var
|
||||
Corrupt: Integer;
|
||||
procedure TStampThread.Execute;
|
||||
var I: Integer;
|
||||
begin
|
||||
GR.Put(Self.MyId);
|
||||
for I := 0 to 200000 do
|
||||
if GR.Get() <> Self.MyId then
|
||||
begin
|
||||
Corrupt := 1;
|
||||
Exit
|
||||
end
|
||||
end;
|
||||
var
|
||||
A, B, C: TStampThread;
|
||||
begin
|
||||
Corrupt := 0;
|
||||
A := TStampThread.Create(True);
|
||||
B := TStampThread.Create(True);
|
||||
C := TStampThread.Create(True);
|
||||
A.MyId := 11;
|
||||
B.MyId := 22;
|
||||
C.MyId := 33;
|
||||
A.Start();
|
||||
B.Start();
|
||||
C.Start();
|
||||
A.WaitFor();
|
||||
B.WaitFor();
|
||||
C.WaitFor();
|
||||
if Corrupt = 0 then
|
||||
WriteLn('ok')
|
||||
else
|
||||
WriteLn('CORRUPT')
|
||||
end.
|
||||
''';
|
||||
|
||||
procedure TE2EThreadingTests.TestRun_ThreadVar_RecordMethod_PerThreadIsolation;
|
||||
var
|
||||
Output: string;
|
||||
RCode: Integer;
|
||||
begin
|
||||
if not ToolchainAvailable() then begin Ignore('toolchain unavailable'); Exit end;
|
||||
AssertTrue('compile+run', CompileAndRunWithRTL(SrcThreadVarRecordMethod, Output, RCode));
|
||||
AssertEquals('exit code', 0, RCode);
|
||||
AssertEquals('stdout', 'ok' + LE, Output)
|
||||
end;
|
||||
|
||||
const
|
||||
SrcPerThreadAlloc =
|
||||
'''
|
||||
|
|
|
|||
Loading…
Reference in a new issue