Sign in Join

Live chat for Free Pascal, Lazarus, Delphi and every Pascal programmer.

Mailing lists / fpc-devel / 2021-01

double free of internal temp string with fpc 3.2.0

3 messages · 2 people · –

Reply by email Subscribe to this list Your subscription

Read-only archive. Use j and k to move between messages.

Martin Frb

I only tested with 3.2.0 so far. Maybe someone recalls if this is fixed or not.

Also because it seems to happen only in very rare conditions, it may be that trunk does not cause it in this place even if the bug is not yet fixed. (as other code changes could simply change the context enough to mitigate it)

Using fpc 3.2.0
compiling lazarus  SVN with -gw -gl -gt -Criot -Sa -O-1 -gh -gv (this also happened at -O3 / but I could not trace it as valgrinds reported stack was unreadable  / The attached valgrind is with -O-1)

unit MaskEdit
procedure TCustomMaskEdit.SetMask(Value : String);

From the generated ASM

# Temps allocated between rbp-696 and rbp-296

# [575] for I := 1 To Utf8Length(S) do
    leaq    -400(%rbp),%rdi
    call   
    leaq    -408(%rbp),%rdi
    call   
    leaq    -272(%rbp),%rsi
    xorl    %edx,%edx
    leaq    -408(%rbp),%rdi
    call   
    movq    -408(%rbp),%rsi   //////
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
    movq    %rsi,-400(%rbp)
    testq    %rsi,%rsi
    je    .Lj156
    movq    -8(%rsi),%rsi
.Lj156:
    movq    -400(%rbp),%rdi
    testq    %rdi,%rdi
    jne    .Lj157
    movq    (%rip),%rdi
.Lj157:
    call   

On the marked line, the string (ref) in temp(-408) is copied to temp(-400). But the refcount is NOT increased

*****
Temp(-408) is later passed as "var param Result" to

# [741] FTextOnEnter := inherited RealGetText;
    movq    -16(%rbp),%rdi
    leaq    -408(%rbp),%rsi
    call

This will assign an empty string (in this case), and free the memory (apparently the refcount happens to be 1 at that time)

*****
in the autogenerated code for the procedures "end" statement:

# [743] end;
    leaq    -408(%rbp),%rdi
    call   
    leaq    -400(%rbp),%rdi
    call   

Temp-408 is already free (or may have a new value that gets correctly freed)

Temp-400 still points to the already freed string => double free.

Reply by email

baldzhang
to Martin Frb

I have the same problem after upgrade to 3.2.0, 3.0.4 is all good.

In my program, when free a TStringList that used in a thread, exception occurs in 80%~90% time. I try to finger out what happening and got fpc_ansistr_decr_ref calling FreeMem then got wrong memory. The program was x86-64 on Windows 10.

I haven't dig so deep like Martin has done, but I think it's the same problem.

"Martin Frb wrote 76 lines
I only tested with 3.2.0 so far. Maybe someone recalls if this is fixed or not. Also because it seems to happen only in very rare conditions, it may be that trunk does not cause it in this place even if the bug is not yet fixed. (as other code changes could simply change the context enough to mitigate it) Using fpc 3.2.0 compiling lazarus  SVN with -gw -gl -gt -Criot -Sa -O-1 -gh -gv (this also happened at -O3 / but I could not trace it as valgrinds reported stack was unreadable  / The attached valgrind is with -O-1) unit MaskEdit procedure TCustomMaskEdit.SetMask(Value : String); From the generated ASM # Temps allocated between rbp-696 and rbp-296 # [575] for I := 1 To Utf8Length(S) do     leaq    -400(%rbp),%rdi     call        leaq    -408(%rbp),%rdi     call        leaq    -272(%rbp),%rsi     xorl    %edx,%edx     leaq    -408(%rbp),%rdi     call        movq    -408(%rbp),%rsi   ////// <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<     movq    %rsi,-400(%rbp)     testq    %rsi,%rsi     je    .Lj156     movq    -8(%rsi),%rsi .Lj156:     movq    -400(%rbp),%rdi     testq    %rdi,%rdi     jne    .Lj157     movq    (%rip),%rdi .Lj157:     call    On the marked line, the string (ref) in temp(-408) is copied to temp(-400). But the refcount is NOT increased ***** Temp(-408) is later passed as "var param Result" to # [741] FTextOnEnter := inherited RealGetText;     movq    -16(%rbp),%rdi     leaq    -408(%rbp),%rsi     call This will assign an empty string (in this case), and free the memory (apparently the refcount happens to be 1 at that time) ***** in the autogenerated code for the procedures "end" statement: # [743] end;     leaq    -408(%rbp),%rdi     call        leaq    -400(%rbp),%rdi     call    Temp-408 is already free (or may have a new value that gets correctly freed) Temp-400 still points to the already freed string => double free.

Reply by email

Martin Frb
to Martin Frb: I only tested with 3.2.0 so far. Maybe someone recalls if this is fixed or not. Also…
Martin Frb wrote 7 lines
I only tested with 3.2.0 so far. Maybe someone recalls if this is fixed or not. Also because it seems to happen only in very rare conditions, it may be that trunk does not cause it in this place even if the bug is not yet fixed. (as other code changes could simply change the context enough to mitigate it)

I was able to reproduce with a "smaller" example (do not judge the quality of the example please, I just cut it out of the bigger LCL code....) It appears to give the same error with 3.2.0

The error appears fixed in trunk.
Both the example and the IDE run without error, if build with fpc trunk (and also fpc 3.0.4).

However some error (albeit in different locations, yet also freeing strings that are in use), still happens in 3.2.1 fixes. Tested with the test project / I have not checked the entire IDE against 3.2.1.

I have therefore reported the issue, so whatever un-merged fixes exist in trunk can be merged.
https://bugs.freepascal.org/view.php?id=38337

-----------
I am aware the test code heavily plays with "const param: string", and the dangers of that construct.
However the string passed as "const arg" is kept unmodified in the caller, and not modified by any of the callees either.

-----------
On Windows the example also produces errors, however the errors manifest different from those on Linux.
I have only tested with 3.2.0 on Windows. Since there is no valgrind on windows, I only can see the output.
Strings passed to GetCodePoint are corrupted (or getting corrupted on "result := '';" ?)

Reply by email