Lazarus

Programming => General => Topic started by: jamie on April 04, 2020, 08:05:42 pm

Title: UpCase Crash with this code.
Post by: jamie 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 .

Title: Re: UpCase Crash with this code.
Post by: jamie 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 .  :-[
Title: Re: UpCase Crash with this code.
Post by: Otto 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.
Title: Re: UpCase Crash with this code.
Post by: jamie 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
Title: Re: UpCase Crash with this code.
Post by: af0815 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.
Title: Re: UpCase Crash with this code.
Post by: jamie 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..
Title: Re: UpCase Crash with this code.
Post by: Leledumbo on April 05, 2020, 09:52:54 am
In case you missed it:
https://wiki.freepascal.org/User_Changes_3.0#Literal_storage_memory_has_been_made_read-only
Title: Re: UpCase Crash with this code.
Post by: 440bx 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.
Title: Re: UpCase Crash with this code.
Post by: PascalDragon 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).
Title: Re: UpCase Crash with this code.
Post by: jamie 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 ?
 
Title: Re: UpCase Crash with this code.
Post by: marcov 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.
Title: Re: UpCase Crash with this code.
Post by: PascalDragon 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.
Title: Re: UpCase Crash with this code.
Post by: jamie 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.

Title: Re: UpCase Crash with this code.
Post by: jamie 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
Title: Re: UpCase Crash with this code.
Post by: PascalDragon 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.
Title: Re: UpCase Crash with this code.
Post by: marcov on April 05, 2020, 04:22:45 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

Please read my message carefully. AnsiPropCase calls AnsiLowerCase that is OS dependent. The Windows version has uniquestring, but if some other OS doesn't, that is a bug.

So again, on which target did you test AnsiPropCase?

bug reference: https://bugs.freepascal.org/view.php?id=36866
Title: Re: UpCase Crash with this code.
Post by: jamie on April 05, 2020, 04:35:33 pm
I don't know what the issue is here. I have stated it several times.. This is the source code that gets
excuted.

Code: Pascal  [Select][+][-]
  1. function AnsiProperCase(const S: string; const WordDelims: TSysCharSet): string;
  2.  
  3. var
  4.   P,PE : PChar;
  5.  
  6. begin
  7.   Result:=AnsiLowerCase(S);
  8.   P:=PChar(pointer(Result));
  9.   PE:=P+Length(Result);
  10.   while (P<PE) do
  11.     begin
  12.     while (P<PE) and (P^ in WordDelims) do
  13.       inc(P);
  14.     if (P<PE) then
  15.       P^:=UpCase(P^);  // <<<<< You see here
  16.     while (P<PE) and not (P^ in WordDelims) do
  17.       inc(P);
  18.     end;
  19. end;
  20.  
  21.  

This is from 2.0.6 and 2.0.4 of 3.0.4 Fpc generating a 64 bit windows target.

from the unit "strUtils"

If I copy and paste that code within a local call it will crash where I indicated...
Title: Re: UpCase Crash with this code.
Post by: marcov on April 05, 2020, 04:51:49 pm

This is from 2.0.6 and 2.0.4 of 3.0.4 Fpc generating a 64 bit windows target.

from the unit "strUtils"

If I copy and paste that code within a local call it will crash where I indicated...

Copy and pasted it, used the calling code from the bug report. Tested with FPC 3.3.1 64-bit (since I don't have 3.0.4 64-bit), everything fine.

As said, if it does, it is probably not related to the literal thingy, since ansilowercase does uniquestring() already.
Title: Re: UpCase Crash with this code.
Post by: jamie on April 05, 2020, 05:16:24 pm
This is my last post on this, I am sure it will make many happy...
I know the issue but others claim its not happening on their end with the supposed same tool versions.
TinyPortal © 2005-2018