7

My program has following code:

function FooBar(const s: string): string;
var
  sa: AnsiString;
begin

  // ..........................

  sa := AnsiString(s);
  sa := AnsiString(StringReplace(string(sa), '*', '=', [rfReplaceAll]));
  sa := AnsiString(StringReplace(string(sa), ' ', '+', [rfReplaceAll]));
  result := string(sa);

  // ..........................

end;

I noticed that the program did crash "somewhere" and FastMM4 said that I had written to a freed object. As soon as I have commented out "const", the program did work.

I have read the Delphi documentation about const arguments, but I can't figure out why the const argument crashes the program. I would love to understand it.

UPDATE: The program does only crash in Delphi 6 and only if optimization is ON. If the optimization is OFF, the program will work normally. Might it be a Delphi bug?

Daniel Marschall
  • 3,739
  • 2
  • 28
  • 67
  • Could you try without sa like this: `Result:=AnsiString(s);Result:=AnsiString(StringReplace(string(Result),`... ? – Stijn Sanders Apr 29 '14 at 10:56
  • Why do you use these conversions in Delphi6, where string = AnsiString by default? – MBo Apr 29 '14 at 11:00
  • Because I am writing a component which has to be used by Delphi 6 and XE4. I don't want any warnings in my code, so I explicitly casted them (and this function did only do Base64-stuff, so the casts are OK without influencing Unicode-capability). – Daniel Marschall Apr 29 '14 at 11:21
  • Just use string. No conversions are needed here. StringReplace will work. – MBo Apr 29 '14 at 11:43
  • @MBo: yes but you'll get those nasty implicit conversion warnings. – Stijn Sanders Apr 29 '14 at 12:04
  • I noticed that "sa := AnsiString(s)" will result in "sa" and "s" having the same pointer. "sa" is a local variable and will be freed on procedure exit, while "s" belongs to the caller and may not be freed. I assume that automatic-reference-counting did not work... why? "sa := s" should have set the counter to 2. If I remove "const" or if I pass "s" directly to the StringReplace() function, it will work. – Daniel Marschall Apr 29 '14 at 12:09
  • 2
    If you remove all the type-casting and all mention of `AnsiString` from this function, then you won't get any implicit-conversion warnings because there won't be any implicit conversions. – Rob Kennedy Apr 29 '14 at 20:45
  • This might be a Delphi 6 bug. I don't remember anything like it, but Delphi 6 is a rather old version (around 2001, IIRC), so my memory is a bit vague. – Rudy Velthuis Apr 30 '14 at 10:07

5 Answers5

3

There are a few peculiar gotchas when it comes to const string parameters.
Many years ago I helped a colleague resolve a similar peculiar problem (D3 iirc). The following simplified example doesn't look like your specific issue, but it may give you some ideas:

type
  TMyClass
    FString: string;
    procedure AppendString(const S: string);
  end;

procedure TMyClass.AppendString;
begin
  FString := FString + S;
end;

Now if you have an instance of TMyClass and try to call AppendString(FString); to double up the string, you may get an access violation. (There are a few other factors that can affect if you do.) The reason is as follows:

  • The const prevents refcounting the string on the method call.
  • So FString may have refCount = 1 when its value is changed.
  • In which case Copy-on-Write doesn't apply and the string is reallocated. (Most likely at a different address.)
  • So when the method returns, S is referring to an invalid address, and triggers an AV.
Disillusioned
  • 14,635
  • 3
  • 43
  • 77
  • The current version of _UStrCat (the RTL routine called when strings are appended) knows this and will not use the wrong address. I don't have D6 around, so it could be that it did not check this yet. – Rudy Velthuis Apr 30 '14 at 10:29
2

For this case:

 sa := s;

automatic reference counting (ARC) works. This is idiomatic way, compiler know how to work with these string - if sa will change, it creates new copy and so on.

For case of hard typecasting (though type is the same)

sa := AnsiString(s);

you tell compiler that you want to get only pointer to string, and you know how to work with this string reference. Compiler will not interfere and annoy you, but you are responsible for correct operations

P.S. I can not reproduce the problem with Delphi XE5 - both simple assigning and typecasting causes LStrLAsg (internal function) call with ARC. (Of course, compiler magic could be changed a bit)

MBo
  • 77,366
  • 5
  • 53
  • 86
  • Thank you very much for this helpful hint. So, did I understand it correct, that "sa := AnsiString(s)" will leave the ARC-counter to 1 (no matter if s is defined as "const s" or just "s"?) and since the local variable "sa" is then cleared, s will be cleared too (because they are at the same address), which will cause the caller to have an illegal reference to s? – Daniel Marschall Apr 29 '14 at 14:53
  • ISTM that this is simply a bug in D6. It does not exist in current versions anymore. I looked at the code in XE4, and there this is handled properly. I don't think that the casts are the problem. The compiler should be able to handle (or ignore) those. – Rudy Velthuis Apr 30 '14 at 10:32
  • I have read somewhere that "const" lost his functionality in newer Delphi versions (see http://stackoverflow.com/a/1134704/3544341 , the 2nd comment by Andreas Hausladen). This might be the reason why it works on newer versions. – Daniel Marschall Apr 30 '14 at 15:33
  • I masked this answer as solution, even though the other answers are correct too, because my code in the original posts had many different issues (e.g. returning a local variable). However, you mentioned a very interesting fact (typecast does not increase the ARC counter) which I didn't knew. – Daniel Marschall May 08 '14 at 17:17
2

We debugged a crash today that was a Delphi 5 compiler code-gen bug:

procedure TForm1.Button1Click(Sender: TObject);
var
   s: string;
begin
   s := 'Hello, world! '+IntToStr(7);
   DoSomething(s);

   //String s now has a reference count of zero, and has already been freed
   ShowMessage(s);
end;

procedure TForm1.DoSomething(const Title: string);
var
    s: AnsiString;
begin
    s := AnsiString(Title);

    if Now = 7 then
        OutputDebugString('This is a string that is irrelevant');
end;

The Delphi 5 compiler mistakenly tries to decrement the reference counts of both

  • Title
  • s

and causes the const string's reference count to go to zero. This causes it to be freed. So when DoSomething returns, you are using a freed string.

An access violation waiting to happen.

Or, in the case of customers: happening.

Ian Boyd
  • 246,734
  • 253
  • 869
  • 1,219
0

That becouse AnsiString is a pointer. So, you could'n operate it as a usual strings (which mean a array of chars). Crash is random, so you can gain the crash as result of stackoverflow or access violations at any time, not depend on optimization.

Than you function FooBar return the result, sa variable are already free from mamory.

Try to use the PChar or PAnsiChar and allocate the memory for this variables as your needs.

Y.N
  • 4,989
  • 7
  • 34
  • 61
  • I learned that Pascal-strings use automatic reference counting and therefore I have not to care about allocating / freeing the memory. Is there a different between "string" and "AnsiString" in Delphi 6? I thought they were equal. – Daniel Marschall Apr 29 '14 at 11:24
  • Are you using Delphi 6 or Delphi XE6? – Rudy Velthuis Apr 30 '14 at 10:04
0

Since it doesn't make sense to convert to AnsiString and back here, I would rewrite this as

function FooBar(const s:string):string;
begin
  Result:=
    StringReplace(
    StringReplace(
      s
      ,'*','=',[rfReplaceAll])
      ,' ','+',[rfReplaceAll]);
end;
Stijn Sanders
  • 35,982
  • 11
  • 45
  • 67