combining two characters to a string switches them

[dummzeuch 1505]

I had some curious bug today with the following code:

  procedure ReadHeader(out _w, _h, _Depth: Integer);
  const
    PGM_Magic_Number = 'P5';
  var
    s: string;
  begin
    s := ReadChar + ReadChar;
    if s <> PGM_Magic_Number then
      raise ESigError.Create(_('File is not a valid PGM file:') + ' ' + _fn);
    // more code here
  end;

Which failed the test even though the first call to ReadChar returns 'P' and the second returns '5' (I stepped through it in the debugger) so s should have been 'P5', but the actual value of s was '5P'.

Apparently Delphi created code that called the second ReadChar before the first. But why? Maybe because it fills the parameters for the function System._UStrCat3, that it apparently calls to do the concatenation, from right to left?

 

So I changed the code like this:

  procedure ReadHeader(out _w, _h, _Depth: Integer);
  const
    PGM_Magic_Number = 'P5';
  var
    s: string;
  begin
    s := ReadChar;
    s := s + ReadChar;
    if s <> PGM_Magic_Number then
      raise ESigError.Create(_('File is not a valid PGM file:') + ' ' + _fn);
    // more code here
  end;

and it works now.

[Uwe Raabe 2057]

Reminds me of this one: Why does a call to GetDIBits fail on Win64?

[programmerdelphi2k 237]

3 hours ago, dummzeuch said:

I had some curious bug today

and using "System.CONCAT( a, b, c, d, e, .... );

• string1 := Concat( char1, char2);

[programmerdelphi2k 237]

4 hours ago, dummzeuch said:

But why?

When evaluating an expression, is not correct:

1. first evaluate the end (if the result OK then...
2. after evaluating the beginning

That is, in the short circuit we must know whether or not we continue evaluating the expression, exactly as Delphi does when evaluating from "left to right" whether a boolean expression continues or not.
However, we can only concatenate the values if we know the result of the final part, and then concatenate with the initial part.

This happens with "consts" but is not easy see because the breakpoint does not stop/use it!
I don't know if I explained it right (using google)

 

NOTE: CONCAT do the same!

• with 2 functions =  rigth to left order...
• with 3+ functions, the correct order happens:   f1 + f2 + f3
  • s1 := afterThis + mid_func + firstTHIS;
  •  
•   s1 := Format('%s %s', [firstTHIS, afterThis]); // force the correct order 

 

Edited June 29, 2023 by programmerdelphi2k

[Anders Melander 1782]

34 minutes ago, programmerdelphi2k said:

When evaluating an expression, is not correct...

No. The order is undefined. For example, if you evaluate the expression (A and B and C ) the evaluation order might very well be BCA.

 

Only boolean short circuit evaluation has a defined order.

[Daniel 417]

I removed a couple of posts. Please stay on topic.

[Lars Fosdal 1792]

I do a lot of string manipulation, but I have not seen this effect yet.

Weird. I would definitively have assumed that the concatenation would go left to right on evaluation of the functions.

 

Would this change the outcome?

    s := '';
    s := s + ReadChar + ReadChar;

 

[dummzeuch 1505]

A simple test case would be this:

function ReadChar1: Char:
begin
  Result := 'P';
end;

function ReadChar2: Char:
begin
  Result := '5';
end;

procedure ReadHeader(out _w, _h, _Depth: Integer);
  const
    PGM_Magic_Number = 'P5';
  var
    s: string;
  begin
    s := ReadChar1 + ReadChar2;
    if s <> PGM_Magic_Number then
      raise ESigError.Create(_('File is not a valid PGM file:') + ' ' + _fn);
    // more code here
  end;

Edit: I forgot to add for what this is a test case.

 

It's for stepping through with the debugger. Since ReadChar1 and ReadChar2 are two different functions, it's clear in which order they are executed. It's ReadChar2 first and ReadChar1 second.

Edited June 30, 2023 by dummzeuch

[Cristian Peța 103]

Probably I do not understand something and you code do not compile for me.

 

This is working as expected for me. It will show "OK".

 

PS. Now I understand: ReadChar2 is called first and will read the first char from file.

uses
  System.SysUtils;

function ReadChar1: Char;
begin
  Result := 'P';
end;

function ReadChar2: Char;
begin
  Result := '5';
end;

procedure ReadHeader(out _w, _h, _Depth: Integer);
  const
    PGM_Magic_Number = 'P5';
  var
    s: string;
  begin
    s := ReadChar1 + ReadChar2;
    if s <> PGM_Magic_Number then
      raise Exception.Create('File is not a valid PGM file:')
    else
      Writeln('OK');
  end;

var
  _w, _h, _Depth: Integer;

begin
  ReadHeader(_w, _h, _Depth);
  Readln;
end.

 

Edited June 30, 2023 by Cristian Peța

[Sherlock 663]

For me too.

[Cristian Peța 103]

But ReadChar2 is called first and will read the first char from file

[Cristian Peța 103]

Project1.dpr.20: s := ReadChar1 + ReadChar2;
0008E605 E8BEFFFFFF       call $0008e5c8
0008E60A 8BD0             mov edx,eax
0008E60C 8D45EC           lea eax,[ebp-$14]
0008E60F E8289CFEFF       call $0007823c
0008E614 8B45EC           mov eax,[ebp-$14]
0008E617 50               push eax
0008E618 E897FFFFFF       call $0008e5b4
0008E61D 8BD0             mov edx,eax
0008E61F 8D45E8           lea eax,[ebp-$18]
0008E622 E8159CFEFF       call $0007823c
0008E627 8B55E8           mov edx,[ebp-$18]
0008E62A 8D45FC           lea eax,[ebp-$04]
0008E62D 59               pop ecx
0008E62E E8B59DFEFF       call $000783e8

First call to $0008E5C8 is ReadChar2. It is with '5'.

Project1.dpr.10: begin
0008E5C8 55               push ebp
0008E5C9 8BEC             mov ebp,esp
0008E5CB 51               push ecx
Project1.dpr.11: Result := '5';
0008E5CC 66C745FE3500     mov word ptr [ebp-$02],$0035
Project1.dpr.12: end;
0008E5D2 668B45FE         mov ax,[ebp-$02]
0008E5D6 59               pop ecx
0008E5D7 5D               pop ebp
0008E5D8 C3               ret 
0008E5D9 8D4000           lea eax,[eax+$00]

 

[Stefan Glienke 2002]

Of course, the result will be as expected if you name the two functions differently, then it does not matter what order they are getting called - but when it's twice the same function which returns different results then the call order matters

 

Here is the code to simulate the issue:

 

var
  callNo: Integer;

const
  PGM_Magic_Number = 'P5';

function ReadChar: Char;
begin
  Inc(callNo);
  Result := PGM_Magic_Number[callNo];
end;

procedure Test;
var
  s: string;
begin
  s := ReadChar + ReadChar;
  Writeln(s);
end;

begin
  Test;
end;

 

Edited June 30, 2023 by Stefan Glienke

[dummzeuch 1505]

Sorry for the confusion, I got distracted when I wrote that post. That source code was just for showing that ReadChar2 is called first. I have now added an explanation to that post.

Edited June 30, 2023 by dummzeuch

[Cristian Peța 103]

It is because calling convention Right-to-left parameter order?

Edited June 30, 2023 by Cristian Peța

[Stefan Glienke 2002]

16 minutes ago, Cristian Peța said:

It is because calling convention Right-to-left parameter order?

No, because there are no parameters on ReadChar.

No, because even if you mean the string concat method (which is UStrCat3 that will be called here) you will see that if you call ReadChar a third time suddenly order is correct (which then calls UStrCatN)

No, because even parameter passing order is undefined behavior in Delphi

Edited June 30, 2023 by Stefan Glienke

[Lars Fosdal 1792]

Why does your sim code execute in the expected order for me?

[Uwe Raabe 2057]

14 minutes ago, Lars Fosdal said:

Why does your sim code execute in the expected order for me?

Well, as it fails in my case, I guess that is what undefined behavior means.

[dummzeuch 1505]

54 minutes ago, Lars Fosdal said:

Why does your sim code execute in the expected order for me?

No idea. Maybe we are using different Delphi versions (mine is Delphi 2007 in this case) or compiler settings (trying to compare those would open a can of worms).

[Lars Fosdal 1792]

Delphi 11.3 28.0.47991.2819  (Update 3 + Patch 1)

 

Quote

{$A8,B-,C+,D+,E-,F-,G+,H+,I+,J-,K-,L+,M-,N-,O+,P+,Q-,R-,S-,T-,U-,V+,W-,X+,Y+,Z1}
{$MINSTACKSIZE $00004000}
{$MAXSTACKSIZE $00100000}
{$IMAGEBASE $00400000}
{$APPTYPE CONSOLE}
{$WARN SYMBOL_DEPRECATED ON}
{$WARN SYMBOL_LIBRARY ON}
{$WARN SYMBOL_PLATFORM ON}
{$WARN SYMBOL_EXPERIMENTAL ON}
{$WARN UNIT_LIBRARY ON}
{$WARN UNIT_PLATFORM ON}
{$WARN UNIT_DEPRECATED ON}
{$WARN UNIT_EXPERIMENTAL ON}
{$WARN HRESULT_COMPAT ON}
{$WARN HIDING_MEMBER ON}
{$WARN HIDDEN_VIRTUAL ON}
{$WARN GARBAGE ON}
{$WARN BOUNDS_ERROR ON}
{$WARN ZERO_NIL_COMPAT ON}
{$WARN STRING_CONST_TRUNCED ON}
{$WARN FOR_LOOP_VAR_VARPAR ON}
{$WARN TYPED_CONST_VARPAR ON}
{$WARN ASG_TO_TYPED_CONST ON}
{$WARN CASE_LABEL_RANGE ON}
{$WARN FOR_VARIABLE ON}
{$WARN CONSTRUCTING_ABSTRACT ON}
{$WARN COMPARISON_FALSE ON}
{$WARN COMPARISON_TRUE ON}
{$WARN COMPARING_SIGNED_UNSIGNED ON}
{$WARN COMBINING_SIGNED_UNSIGNED ON}
{$WARN UNSUPPORTED_CONSTRUCT ON}
{$WARN FILE_OPEN ON}
{$WARN FILE_OPEN_UNITSRC ON}
{$WARN BAD_GLOBAL_SYMBOL ON}
{$WARN DUPLICATE_CTOR_DTOR ON}
{$WARN INVALID_DIRECTIVE ON}
{$WARN PACKAGE_NO_LINK ON}
{$WARN PACKAGED_THREADVAR ON}
{$WARN IMPLICIT_IMPORT ON}
{$WARN HPPEMIT_IGNORED ON}
{$WARN NO_RETVAL ON}
{$WARN USE_BEFORE_DEF ON}
{$WARN FOR_LOOP_VAR_UNDEF ON}
{$WARN UNIT_NAME_MISMATCH ON}
{$WARN NO_CFG_FILE_FOUND ON}
{$WARN IMPLICIT_VARIANTS ON}
{$WARN UNICODE_TO_LOCALE ON}
{$WARN LOCALE_TO_UNICODE ON}
{$WARN IMAGEBASE_MULTIPLE ON}
{$WARN SUSPICIOUS_TYPECAST ON}
{$WARN PRIVATE_PROPACCESSOR ON}
{$WARN UNSAFE_TYPE OFF}
{$WARN UNSAFE_CODE OFF}
{$WARN UNSAFE_CAST OFF}
{$WARN OPTION_TRUNCATED ON}
{$WARN WIDECHAR_REDUCED ON}
{$WARN DUPLICATES_IGNORED ON}
{$WARN UNIT_INIT_SEQ ON}
{$WARN LOCAL_PINVOKE ON}
{$WARN MESSAGE_DIRECTIVE ON}
{$WARN TYPEINFO_IMPLICITLY_ADDED ON}
{$WARN RLINK_WARNING ON}
{$WARN IMPLICIT_STRING_CAST ON}
{$WARN IMPLICIT_STRING_CAST_LOSS ON}
{$WARN EXPLICIT_STRING_CAST OFF}
{$WARN EXPLICIT_STRING_CAST_LOSS OFF}
{$WARN CVT_WCHAR_TO_ACHAR ON}
{$WARN CVT_NARROWING_STRING_LOST ON}
{$WARN CVT_ACHAR_TO_WCHAR ON}
{$WARN CVT_WIDENING_STRING_LOST ON}
{$WARN NON_PORTABLE_TYPECAST ON}
{$WARN XML_WHITESPACE_NOT_ALLOWED ON}
{$WARN XML_UNKNOWN_ENTITY ON}
{$WARN XML_INVALID_NAME_START ON}
{$WARN XML_INVALID_NAME ON}
{$WARN XML_EXPECTED_CHARACTER ON}
{$WARN XML_CREF_NO_RESOLVE ON}
{$WARN XML_NO_PARM ON}
{$WARN XML_NO_MATCHING_PARM ON}
{$WARN IMMUTABLE_STRINGS OFF}
 

 

Off-topic:

Ctrl+O + Ctrl+O behaves weirdly, IMO. 

It inserts at top of file instead of at cursor, and it doesn't respect existing options like {$APPTYPE CONSOLE}

[Attila Kovacs 629]

28 minutes ago, dummzeuch said:

No idea. Maybe we are using different Delphi versions (mine is Delphi 2007 in this case) or compiler settings (trying to compare those would open a can of worms).

There is no UStrCat3 in D2007, and it works perfectly. Meanwhile, newer versions do have it and are failing. Now you are telling us that yours is failing with D2007, and Lars' is working with D11?

 

[dummzeuch 1505]

6 minutes ago, Attila Kovacs said:

There is no UStrCat3 in D2007, and it works perfectly. Meanwhile, newer versions do have it and are failing. Now you are telling us that yours is failing with D2007, and Lars' is working with D11?

Now that you mention it: There was some Unicode stuff involved somewhere else in the unit (type casts from AnsiChar to Char), so it can't have been Delphi 2007 ...

Must have been Delphi 10.2 then.

(Sorry, I have been trying to get somebody competent at Deutsche Telekom on the phone for the last 2 hours. This is getting on my nerves and wreaking havoc with my concentration.)

[Uwe Raabe 2057]

As expected, I indeed get different outcomes with Win32 and Win64 targets. (Delphi 11.3 Patch 1)