Skip to content

Commit 24ab6dc

Browse files
committed
fix(detector): TThreadDestroy resolves type via constructor call fallback
False positive on FreeAndNil(gDfmRepoIndex) in uStaticAnalyzer2.pas:585. TDfmRepoIndex is a plain class, not a TThread descendant - but the detector's type-resolution only looked at '<Ident> : <Type>;' declarations in the SAME file. gDfmRepoIndex is declared in uDfmRepoIndex.pas, so no declaration was found and the conservative-flag path fired. Fix: when the declaration regex fails, fall back to scanning the same file for a constructor call '<Ident> := T<Type>.Create...'. That covers the common cross-unit-global pattern where a TXxx pointer is declared in one unit and only instantiated/freed in another (caches, repo indexes, registries). LooksLikeThreadType then correctly filters TDfmRepoIndex (no 'thread' substring) and the detector stays silent. TThread descendants whose type name does contain 'thread' continue to be flagged as before. Regression test added: FreeAndNilCrossUnitGlobal_NotReported covers exactly the gDfmRepoIndex pattern.
1 parent 60fea52 commit 24ab6dc

2 files changed

Lines changed: 47 additions & 6 deletions

File tree

StaticCodeAnalyserForm/sources/Detectors/uConcurrencyExt.pas

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -218,21 +218,36 @@ class procedure TConcurrencyExtDetector.AnalyzeUnit(UnitNode: TAstNode;
218218
begin
219219
Ident := M.Groups[1].Value;
220220

221-
// Type-Filter: nur weitermachen wenn die Identifier-Deklaration im
222-
// selben File nach einem TThread-Descendant aussieht (Typ-Token
223-
// enthaelt 'Thread'). Wenn keine Deklaration gefunden wird, weiter
224-
// pruefen (extern deklarierter Identifier - konservativ flaggen).
221+
// Type-Filter: nur weitermachen wenn der Identifier nach einem
222+
// TThread-Descendant aussieht. Lookups in dieser Reihenfolge:
223+
// 1. Spezialfall 'Result': aus Function-Header oben.
224+
// 2. `<Ident> : <Type>;`-Deklaration im selben File.
225+
// 3. `<Ident> := T<Type>.Create...` als Konstruktor-Call im selben
226+
// File - faengt cross-unit deklarierte Globals (z.B.
227+
// `gDfmRepoIndex` in uDfmRepoIndex.pas, instanziiert hier).
228+
// Wenn KEINER der drei Lookups einen Typ liefert, faellt der
229+
// konservative Pfad weiter (Befund + Suppress-Hinweis im Detail).
225230
DeclaredType := '';
226231
if SameText(Ident, 'Result') then
227-
// Spezialfall: Function-Return - Typ kommt aus dem Method-Header.
228232
DeclaredType := ResolveResultType(M.Index)
229233
else
230234
begin
231235
ReDecl := TRegEx.Create(
232236
'(?i)\b' + Ident + '\s*:\s*([A-Za-z0-9_<>,\s.]+?)\s*(?:;|\)|=)');
233237
DeclMatch := ReDecl.Match(Code);
234238
if DeclMatch.Success then
235-
DeclaredType := DeclMatch.Groups[1].Value;
239+
DeclaredType := DeclMatch.Groups[1].Value
240+
else
241+
begin
242+
// Fallback: Konstruktor-Call `<Ident> := TXxx.Create...`. Faengt
243+
// cross-unit-deklarierte Identifier die hier nur instanziiert
244+
// werden - typischer Pfad fuer globale Indizes/Caches.
245+
ReDecl := TRegEx.Create(
246+
'(?i)\b' + Ident + '\s*:=\s*(T\w+)\s*\.\s*Create\b');
247+
DeclMatch := ReDecl.Match(Code);
248+
if DeclMatch.Success then
249+
DeclaredType := DeclMatch.Groups[1].Value;
250+
end;
236251
end;
237252
if (DeclaredType <> '') and not LooksLikeThreadType(DeclaredType) then
238253
Continue; // Kein TThread-Kontext -> kein Befund (vermeidet FP

StaticCodeAnalyserForm/tests/uTestConcurrencyExt.pas

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@ TTestConcurrencyExt = class
2626
// Spezialfall Result: Typ kommt aus dem Function-Header.
2727
[Test] procedure FreeAndNilResult_StringListReturn_NotReported;
2828
[Test] procedure FreeAndNilResult_ThreadReturn_Reported;
29+
// Cross-Unit-Global: Identifier nicht im File deklariert, aber via
30+
// Konstruktor-Call instanziiert -> Typ aus `:= TXxx.Create` ableiten.
31+
[Test] procedure FreeAndNilCrossUnitGlobal_NotReported;
2932
end;
3033

3134
implementation
@@ -209,6 +212,29 @@ procedure TTestConcurrencyExt.FreeAndNilResult_ThreadReturn_Reported;
209212
finally F.Free; end;
210213
end;
211214

215+
procedure TTestConcurrencyExt.FreeAndNilCrossUnitGlobal_NotReported;
216+
// Regression: gDfmRepoIndex / gAstFileCache / gSymbolRefIndex sind in
217+
// anderen Units deklariert, werden im uStaticAnalyzer2 nur instanziiert
218+
// und freigegeben. Vor dem Detector-Fix hat der `<Ident> : <Type>;`-
219+
// Regex die Deklaration im selben File nicht gefunden und konservativ
220+
// geflaggt. Jetzt zieht der Konstruktor-Call-Fallback den Typ aus
221+
// `:= TXxx.Create`.
222+
const SRC =
223+
'unit t; implementation'#13#10 +
224+
'procedure DoStuff;'#13#10 +
225+
'begin'#13#10 +
226+
' gFoo := TMyIndex.Create;'#13#10 +
227+
' try gFoo.Build;'#13#10 +
228+
' except FreeAndNil(gFoo);'#13#10 +
229+
' end;'#13#10 +
230+
'end;';
231+
var F: TObjectList<TLeakFinding>;
232+
begin
233+
F := TFindingHelper.FindingsOfFile(SRC);
234+
try Assert.AreEqual(0, TFindingHelper.Count(F, fkTThreadDestroyWithoutTerminate));
235+
finally F.Free; end;
236+
end;
237+
212238
initialization
213239
TDUnitX.RegisterTestFixture(TTestConcurrencyExt);
214240

0 commit comments

Comments
 (0)