Recent

Author Topic: UpCase Crash with this code.  (Read 2050 times)

jamie

  • Hero Member
  • *****
  • Posts: 6091
UpCase Crash with this code.
« on: April 04, 2020, 08:05:42 pm »
Yup, I failed to show it in the bug site so here it is …

Using 3.0.4 FPC 64 bit. with Lazarus 2.0.4 or 2.0.6 does not matter...
Winblows 10

Code: Pascal  [Select][+][-]
  1. Var
  2.   S:String;
  3.   P:Pchar;
  4. begin
  5.    S := 'test';
  6.    P := Pchar(S);
  7.    P^ := UpCase(P^); // upcase the first one... etc.
  8.  //Crash
  9. End;
  10.  

I've already figured out why and worked around it but others claim to see no error..

Please test with same setup of versions and see what you get .

The only true wisdom is knowing you know nothing

jamie

  • Hero Member
  • *****
  • Posts: 6091
Re: UpCase Crash with this code.
« Reply #1 on: April 04, 2020, 09:10:00 pm »
apparently this is Delphi compatible ! Because on my old D3 it does he same,..

Hence the need to use UniqueString( S ) prier to....

Just love those Delphi compatible insurances .  :-[
The only true wisdom is knowing you know nothing

Otto

  • Full Member
  • ***
  • Posts: 226
Re: UpCase Crash with this code.
« Reply #2 on: April 04, 2020, 09:38:08 pm »
Hi jamie.

Code: Pascal  [Select][+][-]
  1. Var
  2.   S,SS:String;
  3.   P,PP:Pchar;
  4. begin
  5.    S := 'test';
  6.    SS:='';
  7.    P := Pchar(S);
  8.    PP :=Pchar(SS);
  9.    PP^ := UpCase(P^);
  10.    //P^ := UpCase(P^); // upcase the first one... etc.
  11.    Writeln(upcase(PP^));
  12.    readln;
  13. end.      
  14.  

Just to do some tests...
(FPC 3.0.4 / FPC 3.3.1  AMD_64/Intel 386) on Windows 10 64bit.
« Last Edit: April 04, 2020, 09:42:36 pm by Otto »
Kind regards.

jamie

  • Hero Member
  • *****
  • Posts: 6091
Re: UpCase Crash with this code.
« Reply #3 on: April 04, 2020, 10:10:57 pm »
Yup, seems that using AnsiProperCase also does it too, because it's using UpCase within...

It's a good thing there are some really schmart people around   :D
The only true wisdom is knowing you know nothing

af0815

  • Hero Member
  • *****
  • Posts: 1289
Re: UpCase Crash with this code.
« Reply #4 on: April 04, 2020, 10:32:51 pm »
I think UpCase(p^) call the overloading for char and this give the wrong result, because it handle only a char not a 0 terminated string.
regards
Andreas

jamie

  • Hero Member
  • *****
  • Posts: 6091
Re: UpCase Crash with this code.
« Reply #5 on: April 04, 2020, 10:34:42 pm »
na, its faulting because it is trying to change READ only memory when there Is a change to do.

if you pass a string that already has a capital at the start then it works because there is no change..
etc..
The only true wisdom is knowing you know nothing

Leledumbo

  • Hero Member
  • *****
  • Posts: 8747
  • Programming + Glam Metal + Tae Kwon Do = Me

440bx

  • Hero Member
  • *****
  • Posts: 3946
Re: UpCase Crash with this code.
« Reply #7 on: April 05, 2020, 10:13:16 am »
na, its faulting because it is trying to change READ only memory when there Is a change to do.
and that behavior depends on the setting of the longstring directive.  if "string" means "shortstring" then it won't fault.
(FPC v3.0.4 and Lazarus 1.8.2) or (FPC v3.2.2 and Lazarus v3.2) on Windows 7 SP1 64bit.

PascalDragon

  • Hero Member
  • *****
  • Posts: 5446
  • Compiler Developer
Re: UpCase Crash with this code.
« Reply #8 on: April 05, 2020, 02:05:54 pm »
Code: Pascal  [Select][+][-]
  1. Var
  2.   S:String;
  3.   P:Pchar;
  4. begin
  5.    S := 'test';
  6.    P := Pchar(S);
  7.    P^ := UpCase(P^); // upcase the first one... etc.
  8.  //Crash
  9. End;
  10.  

I've already figured out why and worked around it but others claim to see no error..

As Leledumbo mentioned, this is indeed by design. You're assigning a constant string to an AnsiString. The compiler won't insert a copy in this case, but instead use the constant string record contained in the readonly .rdata section of the binary (which has reference count -1 by the way). When you cast that to a PChar you simply get the pointer to the first element of the readonly data. And changing that will be blocked by the memory protection of the operating system (so in DOS it would work :P ).

na, its faulting because it is trying to change READ only memory when there Is a change to do.
and that behavior depends on the setting of the longstring directive.  if "string" means "shortstring" then it won't fault.

ShortString does not support a direct conversion to PChar. You need to use PChar(@s[1]) and you need to make sure manually that your ShortString ends with NUL (as the compiler ensures that only for the managed string types as well as strings directly assigned to PChars).

jamie

  • Hero Member
  • *****
  • Posts: 6091
Re: UpCase Crash with this code.
« Reply #9 on: April 05, 2020, 02:13:39 pm »
speaking of constants, I think I've asked this before but I believe I never got a solid answer on it, I suppose I could run some test to confirm it..

  If I have two for example;

Var
 A,B:String;
begin
 A := 'Test';
 B := 'Test';

How many copies are there in the EXE file load ?
 
The only true wisdom is knowing you know nothing

marcov

  • Administrator
  • Hero Member
  • *
  • Posts: 11383
  • FPC developer.
Re: UpCase Crash with this code.
« Reply #10 on: April 05, 2020, 02:33:15 pm »
This morning I double checked the ansipropstr implementation, but it first calls result:=asnilowercase(s) which calls uniquestring (and since rev 1) for anything but an empty string.

After that it only touches the result variable.

That said, since ansilowercase is a widestringmanager function, I only checked the Windows variant(since that was what the bugreport was about).

Jamie: on what target could you reproduce? Theoretically it could be widestringmanager.ansilowercase implementation of that target missing an uniquestring.

PascalDragon

  • Hero Member
  • *****
  • Posts: 5446
  • Compiler Developer
Re: UpCase Crash with this code.
« Reply #11 on: April 05, 2020, 02:37:45 pm »
speaking of constants, I think I've asked this before but I believe I never got a solid answer on it, I suppose I could run some test to confirm it..

  If I have two for example;

Var
 A,B:String;
begin
 A := 'Test';
 B := 'Test';

How many copies are there in the EXE file load ?

If the string constant is "declared" in the same unit, then only once. If it's in different units then it is N times (with N being the number of units the string appears in). This is something that could be improved in the future however at least for those targets that support COMDAT sections (PE/COFF) or section groups (ELF).

Also if you use the string once as AnsiString and once as UnicodeString then you'll have two different string constants.

jamie

  • Hero Member
  • *****
  • Posts: 6091
Re: UpCase Crash with this code.
« Reply #12 on: April 05, 2020, 02:51:46 pm »
So the compiler searches for copies of the same string then ?

 So this means I can use the same constant on the fly and there is only one copy?

 For example
 
  If A ='Test' or b ='Test' or Z='Test' then …..

there should be only one then.... Hmm..

That must make compile time a littler slower searching all those cases.

The only true wisdom is knowing you know nothing

jamie

  • Hero Member
  • *****
  • Posts: 6091
Re: UpCase Crash with this code.
« Reply #13 on: April 05, 2020, 02:54:00 pm »
This morning I double checked the ansipropstr implementation, but it first calls result:=asnilowercase(s) which calls uniquestring (and since rev 1) for anything but an empty string.

After that it only touches the result variable.

That said, since ansilowercase is a widestringmanager function, I only checked the Windows variant(since that was what the bugreport was about).

Jamie: on what target could you reproduce? Theoretically it could be widestringmanager.ansilowercase implementation of that target missing an uniquestring.

I can produce that with a  simple laz app on Windows 10 using laz 2.0.6 and 3.0.4..

and it's UpCase that is the issue because AnsiProperCase using that internally..

 I wouldn't worry too much about it because my old D3 faults on UpCase in the same manner.. there is no call to UniquieString in the background.

and I am using LongSrings of course
The only true wisdom is knowing you know nothing

PascalDragon

  • Hero Member
  • *****
  • Posts: 5446
  • Compiler Developer
Re: UpCase Crash with this code.
« Reply #14 on: April 05, 2020, 03:18:23 pm »
Jamie: on what target could you reproduce? Theoretically it could be widestringmanager.ansilowercase implementation of that target missing an uniquestring.

It's not the UpCase that triggers the error, but the assignment to P^, because P points to the readonly memory of the constant string.

So the compiler searches for copies of the same string then ?

 So this means I can use the same constant on the fly and there is only one copy?

 For example
 
  If A ='Test' or b ='Test' or Z='Test' then …..

there should be only one then.... Hmm..

That must make compile time a littler slower searching all those cases.

The compiler does not search when parsing, but when generating the constant data and there it uses a hashtable to detect duplicates.

 

TinyPortal © 2005-2018