Using uninitialized object works on Win32, throws AV on Win64

[aehimself 396]

Hello,

 

Not that long ago we managed to fix up the codebase enough so our large legacy application could be compiled to Win64. Since then this is the second time when something causes an AV on Win64 which worked for 15+ years on Win32.

 

Pseudocode:

Procedure TForm1.Button1Click(Sender: TObject);

 Procedure DoStuff(Var sl: TStringList);
 Begin
  If Not Assigned(sl) Then
    sl := TStringList.Create;
  sl.Text := 'Just add' + sLineBreak + 'some data';
 End;

Var
 sl: TStringList;
Begin
 Try
  If condition Then
    Exit;

  DoStuff(sl);

  [...]

 Finally
  sl.Free;
 End;
End;

There are two points where said code can fail:

- sl wasn't initialized to nil before passing it to DoStuff. Win32 happily created the TStringList, Win64 didn't and therefore the next method doing anything with it threw an AV

- Before even having a chance to initialize sl, we might break out of the method, but sl.Free is called anyway. On Win32 this was fine, Win64 threw an AV first chance

 

As far as I'm aware Delphi is not initializing local variables and I also don't think we were that lucky for 15+ years that these variables were always reserved on a previously cleared memory area (therefore, pointing to nil).

Before you say anything, I know that this is a coding issue and was corrected quickly, I'm interested why it behaves differently; maybe some compiler differences...?

What can be the explanation?

 

Cheers!

[Dalija Prasnikar 1396]

19 minutes ago, aehimself said:

As far as I'm aware Delphi is not initializing local variables and I also don't think we were that lucky for 15+ years that these variables were always reserved on a previously cleared memory area (therefore, pointing to nil).

Yes, you were lucky. Uninitialized variable will contain whatever was previously written at that location. If you were lucky enough that value is zero, broken code would work. 

 

19 minutes ago, aehimself said:

Before you say anything, I know that this is a coding issue and was corrected quickly, I'm interested why it behaves differently; maybe some compiler differences...?

Yes, there is a compiler difference one is 32bit compiler another is 64bit one. Pointer sizes are different. Memory layout will be different. 

 

But, different compiler is besides the point. Working with uninitialized variables is undefined behavior. That means anything can happen even with the same compiler. 

[aehimself 396]

Based on what I know about local variable initialization, luck was the only logical explanation. But considering the amount of chances it could have broken (15+ years, ~100 clients, software is running on 10 machines at each, minimum, in daily use) I started to have doubts.

 

If this is pure luck, I'll advise our CEO to start to play the lottery 🙂

 

 

[David Heffernan 2345]

2 hours ago, aehimself said:

I also don't think we were that lucky for 15+ years 

You weren't lucky. You were unlucky. Lucky would have been if the error had shown itself immediately. 

 

No point analysing luck. Fix your code and move on. 

[Uwe Raabe 2057]

In theory the chance of a 32-bit pointer being 0 is 1:2^32 while the chance for a 64-bit pointer being 0 is 1:2^64. That is quite a big difference.

 

I am aware that this estimate is probably pretty wrong because the memory contents are anything but evenly distributed. If you have some spare time you can analyze the 32-bit code and find out if some other method called clears the part of the stack later used for sl with zero most of the time. Note, that the benefit of this research is just to ease your mind.

[David Heffernan 2345]

7 hours ago, Uwe Raabe said:

I am aware that this estimate is probably pretty wrong

Completely wrong. And rather unhelpfully so. 

[Kryvich 165]

I created a minimal reproducible example for this case.

program TestInitialization;
{$APPTYPE CONSOLE}

procedure Test;

 procedure DoStuff(var a: Integer);
 begin
  if a = 0 then
    a := 1;
 end;

var
 a: Integer;
begin
  DoStuff(a);
  Writeln(a);
end;

begin
  Test;
  Readln;
end.

It's strange that the compiler doesn't show a warning W1036 Variable 'a' might not have been initialized. This is definitely an oversight.

[Kryvich 165]

The FPC compiler shows a hint like this in this case:

Hint: Local variable "a" does not seem to be initialized

But I'm sure this should be a warning, not a hint.

Edited January 30, 2022 by Kryvich

[David Heffernan 2345]

42 minutes ago, Kryvich said:

It's strange that the compiler doesn't show a warning W1036 Variable 'a' might not have been initialized. This is definitely an oversight.

I think this is because Delphi doesn't really distinguish between in/out and out semantics. Since it doesn't know whether var implies in/out or out, it can't warn, because the var parameter may be an out parameter. 

 

And yes I know that the out keyword exists but certainly for value types it is purely cosmetic. 

 

Contrast this with the clarity of C#. 

[Vandrovnik 214]

4 hours ago, Kryvich said:

I created a minimal reproducible example for this case.

program TestInitialization;
{$APPTYPE CONSOLE}

procedure Test;

 procedure DoStuff(var a: Integer);
 begin
  if a = 0 then
    a := 1;
 end;

var
 a: Integer;
begin
  DoStuff(a);
  Writeln(a);
end;

begin
  Test;
  Readln;
end.

It's strange that the compiler doesn't show a warning W1036 Variable 'a' might not have been initialized. This is definitely an oversight.

Please will you report it and post here the link? I would vote for it.

[Attila Kovacs 629]

7 minutes ago, Vandrovnik said:

Please will you report it and post here the link? I would vote for it.

I'll downvote it.

[Anders Melander 1783]

18 minutes ago, Attila Kovacs said:

I'll downvote it.

How?

[Attila Kovacs 629]

1 minute ago, Anders Melander said:

How?

As a comment? 😉

[Vandrovnik 214]

25 minutes ago, Attila Kovacs said:

I'll downvote it.

Why?

[Attila Kovacs 629]

1 minute ago, Vandrovnik said:

Why?

It's not the compilers job to do code analysis for you?

[Uwe Raabe 2057]

Would you expect this code issue such a warning?

{$APPTYPE CONSOLE}

procedure EchoLineUntilEmpty;
var S: string;
begin
  repeat
    Readln(S);
    Writeln(S);
  until S = '';
end;

 

[Vandrovnik 214]

1 hour ago, Attila Kovacs said:

It's not the compilers job to do code analysis for you?

It helps to avoid mistakes.

 

Here it produces a warning: ([dcc32 Warning] Test79.dpr(22): W1036 Variable 'a' might not have been initialized)

procedure Test2;
 var a: integer;
 begin
 if a=1 then exit;
end;

I believe it should be consistent and produce a warning in Kryvich's example, too.

 

 

[Uwe Raabe 2057]

If a variable is given to a method as var or out parameter that should be sufficient to rate that variable as initialized inside that method. If that would always issue a warning, you will have to make a redundant initialization just to make the warning vanish. I strongly reject this request.

[Attila Kovacs 629]

1 hour ago, Vandrovnik said:

I believe it should be consistent and produce a warning in Kryvich's example, too.

 

Really? And what if the var in the sub-roc is passed further? How deep do you want the compiler work for you? What if the decision is based on a variant? Should the compiler evaluate every outcomes? How slow should be the compiler just for your entertainment? Buy TMS FixInsight, write unit tests and first of all, develop a coding pattern which doesn't even uses such pitfalls.

 

[Vandrovnik 214]

2 minutes ago, Attila Kovacs said:

Really? And what if the var in the sub-roc is passed further? How deep do you want the compiler work for you? What if the decision is based on a variant? Should the compiler evaluate every outcomes? How slow should be the compiler just for your entertainment? Buy TMS FixInsight, write unit tests and first of all, develop a coding pattern which doesn't even uses such pitfalls.

 

It does not have to look inside - it sees (immediatelly), that uninitialized variable is passed as "var" parameter. It means that I should initialize the variable, or change the procedure and use "out" parameter instead of "var".

[Attila Kovacs 629]

10 minutes ago, Vandrovnik said:

It does not have to look inside - it sees (immediatelly), that uninitialized variable is passed as "var" parameter. It means that I should initialize the variable, or change the procedure and use "out" parameter instead of "var".

OK, but it's not wrong passing uninitialized variable to a var parameter. Why the hint then?

 

Edited January 30, 2022 by Attila Kovacs

[Vandrovnik 214]

4 minutes ago, Attila Kovacs said:

Every time you compile on every parameter, not just the one, on every single. Every time.

How would that affect the compiler?

Thx, not for me.

 

And it's not even wrong. Why the hint?

 

From my point of view, it is wrong. For "var" parameters, I expect they are already initialized. For uninitialized variables, that are used as output, there is "out".

 

https://docwiki.embarcadero.com/RADStudio/Sydney/en/Parameters_(Delphi)

- Out parameters are frequently used with distributed-object models like COM. In addition, you should use out parameters when you pass an uninitialized variable to a function or procedure.

[Attila Kovacs 629]

Why are you using "var" then? Why do you need the hint?

[Attila Kovacs 629]

This is more interesting to me, no hint, but I could pass values from b() to a(): 

procedure a(out sl: TStringList);
begin
  showmessage(sl.text);
end;

procedure b;
var
  sl: TStringList;
begin
  //sl := TStringList.Create;
  //sl.Add('hi');
  a(sl);
end;

I'd stick with "var" parameters and no superfluous hints...

Edited January 30, 2022 by Attila Kovacs

[dummzeuch 1505]

23 minutes ago, Vandrovnik said:

It does not have to look inside - it sees (immediatelly), that uninitialized variable is passed as "var" parameter. It means that I should initialize the variable, or change the procedure and use "out" parameter instead of "var".

And other calls to that procedure might actually require an input value:

 

procedure bla(var _Value1: integer; _Value2: integer);
begin
  if _Value2 > 0 then
    _Value1 := _Value2+1;
end;

procedure blub;
var
  Value1: integer;
begin
  Value1 := 5;
  bla(Value1, 3);
end;

How should the compiler determine whether passing an uninitialized variable to procedure bla is a problem, without analyzing the procedure itself?