Recent

Author Topic: [SOLVED] Commit ee10850a of 17 Oct, 2021 breaks msegui compatibility.  (Read 7260 times)

Martin_fr

  • Administrator
  • Hero Member
  • *
  • Posts: 7684
  • Debugger - SynEdit - and more
    • wiki
Re: Commit ee10850a of 17 Oct, 2021 breaks msegui compatibility.
« Reply #15 on: October 31, 2021, 11:27:00 am »
For example this procedure (in kernel/msestrings.pas)?
Exactly that.

Additionally the "stringaddref" is not thread safe. Jonas' method is (to the point that any strings is thread save).

Fred vS

  • Hero Member
  • *****
  • Posts: 2524
    • StrumPract is the musicians best friend
Re: Commit ee10850a of 17 Oct, 2021 breaks msegui compatibility.
« Reply #16 on: October 31, 2021, 03:39:16 pm »
Hello Roland, Florian, Jonas and Martin.

Many thanks to help so much!

OK, I did change procedure stringaddref(var str: ansistring) and procedure stringaddref(var str: msestring) with Jonas code.
Note that I did already try, before your post, with this but it crash without any info from debugger:
Code: Pascal  [Select][+][-]
  1. procedure stringaddref(var str: ansistring);
  2. var
  3.  po1: int64;
  4. begin
  5.  if pointer(str) <> nil then begin
  6.  po1:= StringRefCount(str);
  7.    if po1 >= 0 then begin
  8.    inc(po1);
  9.   end;
  10.  end;
  11. end;

The excellent news is that with Jonas procedure, it crash with note from the debugger.

The debugger message:
Quote
Signal: SIGSEGV: File msestrings.pas:878 Function: DOSTRINGTOUTF8

Here the code pointing to error:

Code: Pascal  [Select][+][-]
  1. procedure dostringtoutf8(const value: pmsechar;
  2.                                      var count: integer; const dest: pointer;
  3.                                                    const options: utfoptionsty);
  4. var
  5.  ps,pe: pcard16;
  6.  pd: pcard8;
  7.  ca1: card16;
  8. ...
  9. begin
  10.  pd:= dest;
  11.  ps:= pointer(value);
  12.  pe:= ps + count;
  13.  while ps < pe do begin
  14.   ca1:= ps^;      // Debugger point to this
  15. ...

[EDIT] Tried with changing this:
Code: Pascal  [Select][+][-]
  1. var
  2.  ps,pe: pcard32; // here change to card32
  3.  pd: pcard8;
  4.  ca1: card32; // here change to card32

But still crash and now without any debugger help message, only this:
Quote
An unhandled exception occurred at $000000000060E501 :
??????????????????????
... (+ much other chars)
[Inferior 1 (process 8215) exited with code 0331]

Hum, any idea what is wrong there?

There is also that code that surely needs change (but with what?) :

Code: Pascal  [Select][+][-]
  1. procedure stringsafefree(var str: string; const onlyifunique: boolean);
  2. var
  3. po1: psizeint;
  4. begin
  5.  if pointer(str) <> nil then begin
  6.  po1:= pointer(str)-2*sizeof(sizeint);
  7.    if (po1 >= 0) and (not onlyifunique or (po1 = 1)) then begin
  8.     fillchar(pointer(str)^,length(str)*sizeof(str[1]),0);
  9.    str:= '';
  10.   end;
  11.  end;
  12. end;

Thanks.

Fre;D

« Last Edit: October 31, 2021, 04:31:49 pm by Fred vS »
I use Lazarus 2.2.0 32/64 and FPC 3.2.2 32/64 on Debian 11 64 bit, Windows 10, Windows 7 32/64, Windows XP 32,  FreeBSD 64.
Widgetset: fpGUI, MSEgui, Win32, GTK2, Qt.

https://github.com/fredvs
https://gitlab.com/fredvs

Jonas Maebe

  • Hero Member
  • *****
  • Posts: 935
Re: Commit ee10850a of 17 Oct, 2021 breaks msegui compatibility.
« Reply #17 on: October 31, 2021, 06:11:14 pm »
Note that I did already try, before your post, with this but it crash without any info from debugger:
Code: Pascal  [Select][+][-]
  1. procedure stringaddref(var str: ansistring);
  2. var
  3.  po1: int64;
  4. begin
  5.  if pointer(str) <> nil then begin
  6.  po1:= StringRefCount(str);
  7.    if po1 >= 0 then begin
  8.    inc(po1);
  9.   end;
  10.  end;
  11. end;

This version of that routine does nothing at all, because you just increase the value local variable.

Quote
The excellent news is that with Jonas procedure, it crash with note from the debugger.

The debugger message:
Quote
Signal: SIGSEGV: File msestrings.pas:878 Function: DOSTRINGTOUTF8

Here the code pointing to error:

Code: Pascal  [Select][+][-]
  1. procedure dostringtoutf8(const value: pmsechar;
  2.                                      var count: integer; const dest: pointer;
  3.                                                    const options: utfoptionsty);
  4. var
  5.  ps,pe: pcard16;
  6.  pd: pcard8;
  7.  ca1: card16;
  8. ...
  9. begin
  10.  pd:= dest;
  11.  ps:= pointer(value);
  12.  pe:= ps + count;
  13.  while ps < pe do begin
  14.   ca1:= ps^;      // Debugger point to this
  15. ...
This code is fine. It probably crashes here because it's working on an invalid string.

Quote
[EDIT] Tried with changing this:
Code: Pascal  [Select][+][-]
  1. var
  2.  ps,pe: pcard32; // here change to card32
  3.  pd: pcard8;
  4.  ca1: card32; // here change to card32
Don't start changing the types of random variables used during string processing. Analyse what needs changing, then change.

Quote
There is also that code that surely needs change (but with what?) :

Code: Pascal  [Select][+][-]
  1. procedure stringsafefree(var str: string; const onlyifunique: boolean);
  2. var
  3. po1: psizeint;
  4. begin
  5.  if pointer(str) <> nil then begin
  6.  po1:= pointer(str)-2*sizeof(sizeint);
  7.    if (po1 >= 0) and (not onlyifunique or (po1 = 1)) then begin
  8.     fillchar(pointer(str)^,length(str)*sizeof(str[1]),0);
  9.    str:= '';
  10.   end;
  11.  end;
  12. end;
Here you can change the assignment of po1 with StringRefCount(str).

Fred vS

  • Hero Member
  • *****
  • Posts: 2524
    • StrumPract is the musicians best friend
Re: Commit ee10850a of 17 Oct, 2021 breaks msegui compatibility.
« Reply #18 on: October 31, 2021, 08:21:16 pm »
Hello Jonas.

You are the Champion.

1)
Quote
This code is fine. It probably crashes here because it's working on an invalid string.

Indeed, my (lot of) attempts did corrupt the ini-status file.
Fixed.

2)
Quote
Here you can change the assignment of po1 with StringRefCount(str).

Changed with this for string and msestring:

Code: Pascal  [Select][+][-]
  1. procedure stringsafefree(var str: string; const onlyifunique: boolean);
  2. var
  3.  //po1: psizeint;
  4.  po1: longint;
  5. begin
  6.  if pointer(str) <> nil then begin
  7.  // po1:= pointer(str)-2*sizeof(sizeint);
  8.  po1:= StringRefCount(str);
  9.  //  if (po1^ >= 0) and (not onlyifunique or (po1^ = 1)) then begin
  10.   if (po1 >= 0) and (not onlyifunique or (po1 = 1)) then begin
  11.    fillchar(pointer(str)^,length(str)*sizeof(str[1]),0);
  12.    str:= '';
  13.   end;
  14.  end;
  15. end;

3)   Using your previous code for stringaddref for ansistring and msestring:
Code: Pascal  [Select][+][-]
  1.  procedure stringaddref(var str: ansistring);
  2.     var
  3.      temp: ansistring;
  4.     begin
  5.      { increase refcount if it was > 0}
  6.      temp:=str;
  7.      { prevent the compiler from finalising temp, so it won't decrease the refcount again }
  8.      pointer(temp):=nil;
  9.     end;

4)  Compile + run = Yep it run + loads + all seems to work (not yet tested deeply).   (see picture) ;D

There is still a problem when closing the app, the debugger point to this:

Code: Pascal  [Select][+][-]
  1. function countchars(const str: msestring; achar: msechar): integer;
  2. var
  3.  int1: integer;
  4. begin
  5.  result:= 0;
  6.  for int1:= 1 to length(str) do begin
  7.   if str[int1] = achar then begin // here debugger point
  8.    inc(result);
  9.   end;
  10.  end;
  11. end;

Sure we will find the solution.

Anyway, many thanks for your help Jonas, MSEgui is still on the road, hand in hand with FPC ( and part of you is in MSE code now  ;)).

PS: I fear that the same should be done for ZeosLib.

Fre;D

[EDIT] After some test, there are still problems but it is on the good way...
« Last Edit: October 31, 2021, 08:57:20 pm by Fred vS »
I use Lazarus 2.2.0 32/64 and FPC 3.2.2 32/64 on Debian 11 64 bit, Windows 10, Windows 7 32/64, Windows XP 32,  FreeBSD 64.
Widgetset: fpGUI, MSEgui, Win32, GTK2, Qt.

https://github.com/fredvs
https://gitlab.com/fredvs

Fred vS

  • Hero Member
  • *****
  • Posts: 2524
    • StrumPract is the musicians best friend
Re: Commit ee10850a of 17 Oct, 2021 breaks msegui compatibility.
« Reply #19 on: October 31, 2021, 10:51:28 pm »
Re-hello.

Oooops, this:

Quote
3)   Using your previous code for stringaddref for ansistring and msestring:

     
Code: Pascal  [Select][+][-]
  1. procedure stringaddref(var str: ansistring);
  2.         var
  3.          temp: ansistring;
  4.         begin
  5.          { increase refcount if it was > 0}
  6.          temp:=str;
  7.          { prevent the compiler from finalising temp, so it won't decrease the refcount again }
  8.          pointer(temp):=nil;
  9.         end;
  10.  

temp: ansistring, must of course be changed in procedure stringaddref(var str: msestring) with temp: msestring.
Bad me, it was using temp: ansistring.

Ok, changed this.

Then compiles + runs = PERFECT, no crash at loading, no error at closing, even complicated app like StrumPract compilles + run perfectly with fpc 3.3.1 last trunk.

OK, problem completely solved.

Many thanks to you all particularity for Jonas and his perfect solution.

Yep, Martin would be very happy (and I am very happy too).  :)

[EDIT] : https://github.com/mse-org/mseide-msegui/commit/a069f4a1d9dc6a75a0f140b42ac7dca0adc8ac68
« Last Edit: October 31, 2021, 11:37:06 pm by Fred vS »
I use Lazarus 2.2.0 32/64 and FPC 3.2.2 32/64 on Debian 11 64 bit, Windows 10, Windows 7 32/64, Windows XP 32,  FreeBSD 64.
Widgetset: fpGUI, MSEgui, Win32, GTK2, Qt.

https://github.com/fredvs
https://gitlab.com/fredvs

Roland57

  • Full Member
  • ***
  • Posts: 227
Re: [SOLVED] Commit ee10850a of 17 Oct, 2021 breaks msegui compatibility.
« Reply #20 on: November 01, 2021, 12:42:34 am »
@Jonas, Martin

Thank you for your help.

I wonder if there won't be an issue also with this (still in msestrings.pas):

Code: Pascal  [Select][+][-]
  1.  stringheaderty = packed record
  2. {$ifdef hascodepage}
  3.   CodePage: TSystemCodePage;
  4.   ElementSize: Word;
  5.  {$ifdef CPU64}    { align fields  }
  6.   Dummy: DWord;
  7.  {$endif CPU64}
  8. {$endif}
  9.   ref: sizeint;
  10.   len: sizeint;
  11.  end;

I will look at it closer tomorrow.

Fred vS

  • Hero Member
  • *****
  • Posts: 2524
    • StrumPract is the musicians best friend
Re: [SOLVED] Commit ee10850a of 17 Oct, 2021 breaks msegui compatibility.
« Reply #21 on: November 01, 2021, 06:07:43 pm »
@Jonas, Martin

Thank you for your help.

I wonder if there won't be an issue also with this (still in msestrings.pas):

Code: Pascal  [Select][+][-]
  1.  stringheaderty = packed record
  2. {$ifdef hascodepage}
  3.   CodePage: TSystemCodePage;
  4.   ElementSize: Word;
  5.  {$ifdef CPU64}    { align fields  }
  6.   Dummy: DWord;
  7.  {$endif CPU64}
  8. {$endif}
  9.   ref: sizeint;
  10.   len: sizeint;
  11.  end;

I will look at it closer tomorrow.

Hello Roland.
If I understood well, that code should not make conflict with some fpc because stringheaderty is not defined in fpc.
But that code does not profit of the ee10850a commit from fpc that remove the "Dummy" element in 64 bit, nor do better alignment.

If this is correct, imho it would be good to adapt the mse stringheaderty code and do the same as TAnsiRec, TUnicodeRec in ee10850a fpc commit.

Fre;D

« Last Edit: November 01, 2021, 06:38:07 pm by Fred vS »
I use Lazarus 2.2.0 32/64 and FPC 3.2.2 32/64 on Debian 11 64 bit, Windows 10, Windows 7 32/64, Windows XP 32,  FreeBSD 64.
Widgetset: fpGUI, MSEgui, Win32, GTK2, Qt.

https://github.com/fredvs
https://gitlab.com/fredvs

PascalDragon

  • Hero Member
  • *****
  • Posts: 3638
  • Compiler Developer
Re: [SOLVED] Commit ee10850a of 17 Oct, 2021 breaks msegui compatibility.
« Reply #22 on: November 02, 2021, 02:03:58 pm »
If I understood well, that code should not make conflict with some fpc because stringheaderty is not defined in fpc.
But that code does not profit of the ee10850a commit from fpc that remove the "Dummy" element in 64 bit, nor do better alignment.

There won't be a type conflict, yes, but any code that uses stringheaderty will access wrong data on 64-bit, because the only use of tstringheaderty is to interact with the header record of Ansi- and UnicodeString which now have a mismatching memory layout.

If this is correct, imho it would be good to adapt the mse stringheaderty code and do the same as TAnsiRec, TUnicodeRec in ee10850a fpc commit.

It would not only be good, but also correct to adjust the code. Better yet: check whether you can drop it.

Fred vS

  • Hero Member
  • *****
  • Posts: 2524
    • StrumPract is the musicians best friend
Re: [SOLVED] Commit ee10850a of 17 Oct, 2021 breaks msegui compatibility.
« Reply #23 on: November 02, 2021, 03:43:17 pm »
If I understood well, that code should not make conflict with some fpc because stringheaderty is not defined in fpc.
But that code does not profit of the ee10850a commit from fpc that remove the "Dummy" element in 64 bit, nor do better alignment.

There won't be a type conflict, yes, but any code that uses stringheaderty will access wrong data on 64-bit, because the only use of tstringheaderty is to interact with the header record of Ansi- and UnicodeString which now have a mismatching memory layout.

If this is correct, imho it would be good to adapt the mse stringheaderty code and do the same as TAnsiRec, TUnicodeRec in ee10850a fpc commit.

It would not only be good, but also correct to adjust the code. Better yet: check whether you can drop it.

Hello Sven.

Thanks for that very clear explanation.

Last question before I become Guru es AnsiString and UnicodeString.

About the patch of Sergey Larin, why does it comes now ?
It seems so evident in a 64 bit OS to use a longint vs sizeint and that "Dummy" element added is so strange.

Was something changed in fpc code that makes that patch possible?

Or is it Sergey Larin who discovers just last week that something was strange in AnsiString and UnicodeString in 64 bit Linux OS?

Anyway, many thanks for your patch Sergey, now that it works for msegui I like it so much.  ;)

Have a perfect day.

Fre;D
« Last Edit: November 02, 2021, 05:24:06 pm by Fred vS »
I use Lazarus 2.2.0 32/64 and FPC 3.2.2 32/64 on Debian 11 64 bit, Windows 10, Windows 7 32/64, Windows XP 32,  FreeBSD 64.
Widgetset: fpGUI, MSEgui, Win32, GTK2, Qt.

https://github.com/fredvs
https://gitlab.com/fredvs

Fred vS

  • Hero Member
  • *****
  • Posts: 2524
    • StrumPract is the musicians best friend
Re: [SOLVED] Commit ee10850a of 17 Oct, 2021 breaks msegui compatibility.
« Reply #24 on: November 02, 2021, 06:40:48 pm »
Better yet: check whether you can drop it.

Hum, yes we know and I agree.  :-[

To maintain msegui on the road bridge to fpc are needed.
Circular did a class-bridge for all graphic things, now you may chose to use the build-in msegui graphic class vs those of fpc/BGRABitmap (or use both).

For all file-access things, msegui uses now exclusively the fpc offer, it was too complicated to update the build-in msegui file access for each OS and till now no one of the msegui maintainers has the skill of Martin to do it.

Also all the units used in /fpccompatibility/ that had the same name as those from fpc were renamed mse_xxx (and all msegui code adapted for this) to avoid possible conflict.

Since this topic, for msestrings, it will be the same, loose some independence and use the fpc offer.

Personally I am ok with this (and dont see other way to keep msegui in the race).

Fre;D
« Last Edit: November 02, 2021, 07:38:04 pm by Fred vS »
I use Lazarus 2.2.0 32/64 and FPC 3.2.2 32/64 on Debian 11 64 bit, Windows 10, Windows 7 32/64, Windows XP 32,  FreeBSD 64.
Widgetset: fpGUI, MSEgui, Win32, GTK2, Qt.

https://github.com/fredvs
https://gitlab.com/fredvs

Roland57

  • Full Member
  • ***
  • Posts: 227
Re: [SOLVED] Commit ee10850a of 17 Oct, 2021 breaks msegui compatibility.
« Reply #25 on: November 03, 2021, 07:39:48 am »
It would not only be good, but also correct to adjust the code. Better yet: check whether you can drop it.

Thank you for your advice.

stringheaderty is used in the tmemorystringstream class, declared in kernel/msestrings.pas.

Code: Pascal  [Select][+][-]
  1. type
  2.  tmemorystringstream = class(tmemorystream)
  3.         //has room for stringheader

Code: Pascal  [Select][+][-]
  1. { tmemorystringstream }
  2. const
  3.  stringheadersize = sizeof(stringheaderty);

That class is used in some db units:

[roland@localhost common]$ grep -Rni tmemorystringstream
kernel/msestrings.pas:170: tmemorystringstream = class(tmemorystream)
kernel/msestrings.pas:6164:{ tmemorystringstream }
kernel/msestrings.pas:6168:constructor tmemorystringstream.create;
kernel/msestrings.pas:6179:procedure tmemorystringstream.SetCapacity(NewCapacity: PtrInt);
kernel/msestrings.pas:6184:function tmemorystringstream.getcapacity: ptrint;
kernel/msestrings.pas:6189:function tmemorystringstream.Seek(const Offset: Int64;
kernel/msestrings.pas:6204:function tmemorystringstream.getmemory: pointer;
kernel/msestrings.pas:6209:function tmemorystringstream.GetSize: Int64;
kernel/msestrings.pas:6214:function tmemorystringstream.GetPosition: Int64;
kernel/msestrings.pas:6219:procedure tmemorystringstream.SetSize(
kernel/msestrings.pas:6225:procedure tmemorystringstream.destroyasstring(out data: string);
db/mibconnection.pas:1481:  result:= tmemorystringstream.create;
db/mibconnection.pas:1505: tmemorystringstream(getblobstream(acursor,blobid,true)).destroyasstring(result);
db/msefb3connection.pas:1615:   result:= tmemorystringstream.create;
db/msefb3connection.pas:1641: tmemorystringstream(getblobstream(acursor,blobid,true)).destroyasstring(result);
[roland@localhost common]$

Unfortunately I have no knowledge about databases. I don't know if there is a working demo project for these units.

I would like to see clear in that question of string headers. For now I don't see what we could use instead of stringheaderty.

It would be good to have little code examples. Is there a tutorial somewhere about that topic?
« Last Edit: November 03, 2021, 07:41:52 am by Roland57 »

 

TinyPortal © 2005-2018