Recent

Author Topic: Why does this crash? (untyped parameter to string)  (Read 1100 times)

ahydra

  • New Member
  • *
  • Posts: 19
Why does this crash? (untyped parameter to string)
« on: February 11, 2019, 05:52:14 am »
Found a weird issue when converting some code to use generics yesterday. Before, it was using untyped parameters and typecasting the untyped parameter, which seemed to work fine in production code but not in a quick-and-dirty test program for the new code. I narrowed it down to the following:

Code: Pascal  [Select]
  1. program Project1;
  2.  
  3. uses Classes;
  4.  
  5. type
  6.   TUntypedStringTest = class
  7.     private
  8.       FValue: string;
  9.  
  10.     public
  11.       procedure SetValue(const aValue);
  12.       procedure PrintValue();
  13.  
  14.   end;
  15.  
  16. procedure TUntypedStringTest.SetValue(const aValue);
  17.   begin
  18.     FValue := string(aValue);
  19. end;
  20.  
  21. procedure TUntypedStringTest.PrintValue();
  22.   begin
  23.     writeln(FValue);
  24. end;
  25.  
  26. var
  27.   a: TUntypedStringTest;
  28.  
  29. begin
  30.   a := TUntypedStringTest.Create();
  31.   a.SetValue('hello'); // crash here
  32.   a.PrintValue();
  33.   readln();
  34.   a.Free();
  35. end.
  36.  

When run this program crashes with SIGSEGV at the SetValue line, at fpc_ansistr_assign+0x10 (this is x86)

If you add a string variable to hold the 'hello' instead, and call SetValue with that, the program works correctly.

What's going on here? (Something to do with constant strings not being able to be reference-counted, at a wild guess?) I'm asking because it's unclear if this crash should be reported as an FPC bug.

Thanks,

ahydra

GetMem

  • Hero Member
  • *****
  • Posts: 3501
Re: Why does this crash? (untyped parameter to string)
« Reply #1 on: February 11, 2019, 08:08:42 am »
Instead of
Code: Pascal  [Select]
  1. procedure SetValue(const aValue);
you need
Code: Pascal  [Select]
  1. procedure SetValue(const aValue: string);

I prefer the following solution, hiding as much as possible inside the class, expose only what strictly needed:
Code: Pascal  [Select]
  1. program Project1;
  2.  
  3. uses Classes;
  4.  
  5. type
  6.   TUntypedStringTest = class
  7.   private
  8.     FValue: string;
  9.     procedure SetValue(const aValue: string);
  10.   public
  11.     property Value: String read FValue write SetValue;
  12.   end;
  13.  
  14. procedure TUntypedStringTest.SetValue(const aValue: string);
  15.   begin
  16.     FValue := string(aValue);
  17. end;
  18.  
  19. var
  20.   a: TUntypedStringTest;
  21.  
  22. begin
  23.   a := TUntypedStringTest.Create();
  24.   a.Value := 'Hello';
  25.   writeln(a.Value);
  26.   readln();
  27.   a.Free();
  28. end.

Thaddy

  • Hero Member
  • *****
  • Posts: 8919
Re: Why does this crash? (untyped parameter to string)
« Reply #2 on: February 11, 2019, 08:33:28 am »
@getmem: the untyped const was intentional.
@ahydra: see the documentation here https://freepascal.org/docs-html/ref/refsu65.html#x177-19900014.4.2:
An untyped const is incompatible with all other types, but you can take the pointer from it and process.
In your case e.g.:
Code: Pascal  [Select]
  1. {$mode delphi}
  2. uses Classes;
  3.  
  4. type
  5.   TUntypedStringTest = class
  6.     private
  7.       FValue: string;
  8.     public
  9.       procedure SetValue(const aValue);
  10.       procedure PrintValue();
  11.   end;
  12.  
  13. procedure TUntypedStringTest.SetValue(const aValue);
  14.   begin
  15.   // A pchar is a pointer type, so works with untyped const parameters
  16.   // and is assignment compatible with string.
  17.     FValue := pchar(@aValue);
  18. end;
  19.  
  20. procedure TUntypedStringTest.PrintValue();
  21.   begin
  22.     writeln(FValue);
  23. end;
  24.  
  25. var
  26.   a: TUntypedStringTest;
  27. begin
  28.   a := TUntypedStringTest.Create();
  29.   a.SetValue('hello'); // no more crash here
  30.   a.PrintValue();
  31.   readln();
  32.   a.Free();
  33. end.
Note in border cases this can lead to information loss (if the string contains zero's except the terminating zero: 'hell'#0+'o' will print 'hell' )
It may not be immediately clear that you can't take the address of a managed type, like string, directly, because those contain management information, like length, at negative offset.
So you can't cast to string.
This example shows it works as documented and is definitely not a bug.

Note I would prefer your original solution using generics. Why did you change your mind? The above code is quite unsafe, because it is typeless.
 
« Last Edit: February 11, 2019, 12:15:18 pm by Thaddy »
Most people that want to use threading should learn to patch their jeans first: use a needle.

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 5631
    • wiki
Re: Why does this crash? (untyped parameter to string)
« Reply #3 on: February 11, 2019, 03:31:12 pm »
Just a guess, haven't verified it. 'hello' may be stored as shortstring. Then it can not be cast into a (long)string. (it needs to be converted instead)

In any case, typecasting from/to string => always dangerous.
It can be done, but you have to understand the ref-counting behind. And the types of strings in your case.


About "const aValue: string" => I recommend just "aValue: string".
Yes the "const" may save you 0.01 percent of time. (or whatever amount, in almost all cases it is a tiny amount for strings).

But "const aValue: string" does not mean what you think it does. The time saving is a side effect.

Adding const for string param can crash code, that was valid before.
if you use "const aValue: string" then "a.SetValue(a.FValue);" will be undefined behaviour (might work by accident, might crash, might leak, might produce random results,...)