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

Detect use of uninitialised memory in finally blocks #216

Open
2 tasks done
zaneduffield opened this issue Apr 16, 2024 · 1 comment
Open
2 tasks done

Detect use of uninitialised memory in finally blocks #216

zaneduffield opened this issue Apr 16, 2024 · 1 comment
Labels
enhancement Improvements to an existing feature rule Improvements or additions to rules

Comments

@zaneduffield
Copy link
Collaborator

Prerequisites

  • This improvement has not already been suggested.
  • This improvement should not be implemented as a separate rule.

Rule to improve

VariableInitialization

Improvement description

An anti-pattern in Delphi is

try
  Foo := TFoo.Create;
  {more code}
finally
  Foo.Free;
end;

which is problematic because if TFoo.Create throws an exception then Foo will be uninitialised when the finally block is executed.

The correct version of that code is

Foo := TFoo.Create;
try
  {more code}
finally
  Foo.Free;
end;

but another correct version would be

Foo := nil;
try
  Foo := TFoo.Create;
  {more code}
finally
  Foo.Free;
end;

In order to catch the issue in the first example, without creating a false positive in the last example, the rule would need to have a better understanding of control flow.

There is an endless supply of similar cases that would benefit from enhanced control flow analysis; this rule improvement isn't really about the try-finally anti-pattern specifically.

Rationale

The current variable initialisation rule is quite rudimentary and fails to catch many common cases of uninitialised memory.
It's too permissive, because it has little-to-no understanding about code structure and control flow.

One of the real benefits of static analysis tools is that they can detect issues that are subtle, and only become apparent after excruciatingly evaluating all the logical paths.

@zaneduffield zaneduffield added enhancement Improvements to an existing feature triage This needs to be triaged by a maintainer rule Improvements or additions to rules labels Apr 16, 2024
@Cirras
Copy link
Collaborator

Cirras commented Apr 17, 2024

Thanks for the suggestion, and agreed with your assessment on the benefits of implementing better control flow analysis.
This is definitely an area for improvement.

@Cirras Cirras removed the triage This needs to be triaged by a maintainer label Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvements to an existing feature rule Improvements or additions to rules
Projects
None yet
Development

No branches or pull requests

2 participants