9

I am quite excited about Delphi 10.3 Rio inline variable declarations. However I ran into strange problem and it seems that I need to initialize record after it has been inline declared:

program Project8;
{$APPTYPE CONSOLE}
{$R *.res}

uses System.SysUtils,classes;

procedure DoEvil;
  //var sr:TSearchRec; //A
begin
  //var sr:= default(TSearchRec); //B
  var sr:TSearchRec; //C
  sr.Name := EmptyStr; //D
  FindFirst('*.*',faAnyFile,sr);
  while sr.Name<>EmptyStr do
  begin
    Writeln(sr.name);
    sr.Name := EmptyStr;
    FindNext(sr);
  end;
end;

begin
  try
     DoEvil;
    { TODO -oUser -cConsole Main : Insert code here }
  except
    on E: Exception do
      Writeln(E.ClassName, ': ', E.Message);
  end;
  readln;
end.

The code works fine if I declare sr on line:

  • //A (old style declaration) or on line
  • //B (inline declaration with initialization).

However if I declare sr on line

  • //C then it crashes on line //D, respective in system.pas on line 26222:

    MOV     ECX,[EDX-skew].StrRec.refCnt    { fetch refCnt                 }
    

with exception:

Exception class $C0000005 with message 'access violation at 0x0040ac98: read of address 0xfffffff9'. Process Project8.exe (18928)

I would assume from the address -6, that the string member sr.name is not initialized and is nil.

Just to be complete, Delphi is the new 10.3 release 1: Embarcadero® Delphi 10.3 Version 26.0.33219.4899 - Installed Update 1

LU RD
  • 34,438
  • 5
  • 88
  • 296
Radek Hladík
  • 547
  • 4
  • 15
  • 1
    I used exactly the code you present above. Works without any flaw. As I said: "Embarcadero® RAD Studio 10.3 Version 26.0.33219.4899". Targets: 32bit and 64bit Windows. I tried all 3 varieties: 1) old-style var block, 2) inline with Default and the one you posted above, i.e. 3) inline but no init at all. All three work equally well. – Rudy Velthuis Mar 04 '19 at 17:36
  • 1
    I assume something went wrong in your installation. But sometimes it helps if you copy the code to a completely new console app. That is usually doable for such simple test projects. And do a computer re-start before you continue. I have had problems with newly installed Delphis. A re-start usually helped. – Rudy Velthuis Mar 04 '19 at 17:38
  • FWIW, I had problems with the **compiler** (completely bad code generation) when some third-party tools were installed. Is this a plain vanilla installation (i.e. one without any third-party packages)? – Rudy Velthuis Mar 04 '19 at 17:45
  • I will try tommorow on colege's computer. But I made fresh project for this as I noticed the behaviour in other and bigger project. Only addons installed are alpha controls and cnpack. – Radek Hladík Mar 04 '19 at 17:46
  • Note that you are leaking if `FindFirst` is returning 0. In that case you will have to call `FindClose` after the loop. – LU RD Mar 04 '19 at 17:50
  • @LURD I stripped the code to bare minimum to demonstrate the problem but thanks for your comment – Radek Hladík Mar 04 '19 at 17:54
  • 2
    Hmmm... Could well be I was just lucky. If I use the old-style var-block, `System._InitializeRecord` is called. If I use your `// C` line, it is not. So probably, in my case, it happens to be nil, while in your case, it isn't. If I single step the code in the CPU window, I do get your error. **So it looks like it is a bug after all and I was just lucky** – Rudy Velthuis Mar 04 '19 at 17:57
  • You are also using the wrong condition in your `while`, fwiw. – MartynA Mar 04 '19 at 18:57
  • @rudy uninitialized variables have undefined state. They may be zero – David Heffernan Mar 04 '19 at 18:59
  • @David: I know. As I said: I was lucky. – Rudy Velthuis Mar 04 '19 at 19:06
  • @rudy That's why you should never treat one success as proof that the code is correct when dealing with undefined behaviour – David Heffernan Mar 04 '19 at 19:16
  • @David: what is the problem? And what *undefined behaviour*? Strings in records are supposed to be initialized. In other words: the **code** was correct. But the compiler is faulty. – Rudy Velthuis Mar 04 '19 at 19:35
  • @Rudy Yes, the compiler is faulty. Because of that the behaviour is undefined. – David Heffernan Mar 04 '19 at 20:20
  • Well yes, every bug in the compiler or RTL can cause unexpected behaviour. But "In computer programming, undefined behavior (UB) is the result of executing computer code whose behavior is not prescribed by the language specification to which the code adheres" (Wikipedia). The language spec requires that every string is initialized. So this was not **undefined** behaviour. Again, what is your problem? – Rudy Velthuis Mar 04 '19 at 21:06
  • FWIW, if I program, I assume the compiler is not buggy and I don't constantly check for any bugs in the compiler. Only if things go haywire and I am sure I did not make a mistake and programmed according to the spec, I start thinking of a bug in the compiler instead of in my code. – Rudy Velthuis Mar 04 '19 at 21:09
  • @Rudy Well, UB isn't an official Delphi term is it. I'm using it loosely here. The compiler bug leads to an uninitialized string variable. My point is that your successful execution of the code is absolutely no evidence that the variable has been initialized. And when the question strongly suggests a compiler bug, I wouldn't rule it out. – David Heffernan Mar 04 '19 at 21:49
  • @David: "My point is that your successful execution of the code is absolutely no evidence that the variable has been initialized." I guess that is why I said I was lucky. – Rudy Velthuis Mar 04 '19 at 21:52
  • @Rudy I was really struggling to understand your first comment. The question clearly asks if the variable is properly initialized when declared inline. And your comment just didn't tally with that. That's all. – David Heffernan Mar 04 '19 at 21:58
  • My first comment was that I couldn't reproduce it. Well, that was indeed the case. But since I suspected that post was not made in vain, and the code looked right, I looked further and found out what I posted. – Rudy Velthuis Mar 04 '19 at 22:03
  • @Rudy You suspected a defective installation, which is kinda comical. Sadly, with the low quality of Emba development and testing, compiler bug was the obvious explanation. – David Heffernan Mar 04 '19 at 22:20
  • @David: that is almost certainly the result of the very short term retraction of the managed record code. Shit happens. Unlike you, I don't think the R&D and QA of Embarcadero are low quality. – Rudy Velthuis Mar 04 '19 at 22:30
  • @Rudy Removing a major development and introducing a bug at the last minute that is not caught because there is insufficient testing is pretty much the dictionary definition of a poor quality process. – David Heffernan Mar 05 '19 at 06:55
  • @David: there were good reasons to remove the major development. And yes, that was at **very** short notice and that disappointed me a lot, but this is not generally how it works there, so, in my view, this was an unfortunate misstep. – Rudy Velthuis Mar 05 '19 at 08:19
  • @Rudy Speaking as a professional developer, from my own bitter experience, a major change at the last minute always carries enhanced risk. Good quality processes are designed to avoid and contain those risks. Emba's history of poor quality releases (at least in the past 5 years) is clear evidence that their quality process is deficient. Emba themselves know this, evidenced by statements that they will concentrate on quality. However, it's one thing to acknowledge a problem and resolve to address it, but quite another to succeed in addressing it. – David Heffernan Mar 05 '19 at 08:26
  • @David: "a major change at the last minute always carries enhanced risk". Sure, but sometimes you have no choice. – Rudy Velthuis Mar 05 '19 at 08:36
  • 1
    @RudyVelthuis You always have a choice. You can delay the release until your testing of the change is solid. You can avoid needing to make the change at all by only merging the feature branch onto develop when you are sure it is ready for the release. This is how large scale development is done. There need to be solid processes to support the development. Don't make excuses for Emba. When they get it wrong, call them out. Aren't you an MVP? You should be using that to speak truth to power. – David Heffernan Mar 05 '19 at 08:42
  • Every time I see someone write "You always have a choice" I cringe. No, you don't always have a (viable) choice. – Rudy Velthuis Mar 05 '19 at 08:47
  • @RudyVelthuis There's the excuse. – David Heffernan Mar 05 '19 at 09:14
  • @David: first, I think this is a useless discussion and second: huh? – Rudy Velthuis Mar 05 '19 at 09:37
  • 2
    Delphi have a lot IDE problems that do exists for years or even a decade and the generated code is sometimes not effective as it could be but on the other hand it is Pascal, can compile to many platforms, UI design is easy and straightforward,... So what I am trying to say is that every developer must know his/hers tools and choose them accordingly. And to be honest Delphi did improve a lot in a lot ways. I am still using Delphi 6 (6 not XE6) for one project and the difference is huge. But that version of Delphi still works on Win7 and produces code that runs on Win2016...So good job Borland.. – Radek Hladík Mar 05 '19 at 10:01

1 Answers1

6

I took a look in the CPU window and found a few oddities.

If I use the old-style var-block (your version // A), the CPU window shows a call to System._InitializeRecord, as it should. All is fine and normal.

If I use the inline declaration with Default() (your version // B), the local record is nilled out, then finalized using System._FinalizeRecord and then nilled out again. That is pretty weird and useless, but it works.

But if I use your version // C, nothing is done to initialize the record: no nilling out, no _InitializeRecord. When I tested your code, things did work, but I was probably just lucky.

So this is clearly a bug. Please report it to the Embarcadero Quality Portal.


I guess this is a remainder of the changes made to the compiler when default constructors, default destructors and overloaded assignment operators were tested (but then removed and postponed to a next release). Some of these changes were rather braindead (like the nilling, finalization and nilling again, as in version B), and I guess some of these changes were forgotten.

Update

Obviously this was already reported to QP:

Rudy Velthuis
  • 28,387
  • 5
  • 46
  • 94
  • Already reported as: https://quality.embarcadero.com/browse/RSP-21632 https://quality.embarcadero.com/browse/RSP-23417 https://quality.embarcadero.com/browse/RSP-23747 – Stefan Glienke Mar 04 '19 at 18:30
  • `Assert(sr.Name = '','sr not initialized');` is a quick way to test the bug in debug mode. – LU RD Mar 04 '19 at 18:36
  • @LURD: sure, but if you are lucky, like I was, that won't find the problem either. The current way was just as sure to find it. I was lucky, and it worked. But now, a little later, I get the same error as OP. – Rudy Velthuis Mar 04 '19 at 18:42
  • It can help if the stack is scrambled before the call. Anyway it is a bit disappointing that such a bug could slip through a release. It would have been first thing to test on a unit test. – LU RD Mar 04 '19 at 20:12
  • 1
    @LURD I did wait for first update after they introduced such a major syntax change. And I find it funny, that even now the syntax checker in editor does not understand this new syntax - but that is only inconvenience during development. However creating code that crashes randomly is a serious problem. Especially because the main reason why I am excited about this feature is limiting scope of variables and thus reducing number of errors :-) – Radek Hladík Mar 04 '19 at 21:19
  • @Radek: I assume they will have a proper update for the compiler too. But I guess getting the compiler sorted out is not so easy. – Rudy Velthuis Mar 04 '19 at 21:32
  • @RadekHladík, I'm a slow adapter, but an early tester. Bringing in new language features to existing code is hazardous. – LU RD Mar 04 '19 at 22:37
  • @LURD 100% agree. I tested this on one experimental project for my own use and this feature really is really great :-) On the other side it makes code backwards incompatible and incompatible with FreePascal.... – Radek Hladík Mar 05 '19 at 09:47