2
votes

I had a topic 1-2 weeks ago about a "The block header has been corrupted" and "Block has been modified after being freed" error.

Somebody gave me a good tip (thanks Alexander) about setting FullDebugModeScanMemoryPoolBeforeEveryOperation to true and finally I have some indications about where is the TRUE location of the error.

The error log points to TScObj object. I have a second object very similar to this one and when I use it the error does not appear. So, this somehow confirms that the error is in this specific object (TScObj).

The log is like this:

FastMM has detected an error during a free block scan operation. 
FastMM detected that a block has been modified after being freed. 
Modified byte offsets (and lengths): 15656(1)

The previous block size was: 15672
This block was previously allocated by thread 0xC88, and the stack trace (return addresses) at the time was:
402EC9 [System][@ReallocMem]
40666C [System][DynArraySetLength]
40A17D [FastMM4][UpdateHeaderAndFooterCheckSums]
40674E [System][@DynArraySetLength]
4CE329 [ReadSC.pas][ReadSC][TScObj.ReadData][239]
4CDD0C [ReadSC.pas][ReadSC][TScObj.LoadFromFile][168]
4D013E [SmplCubImport.pas][SmplCubImport][TCubImport.ImportSample][164]
40461A [System][@AfterConstruction]
4DC151 [UnitAsmJob.pas][UnitAsmJob][TAsmJob.LoadSample][960]

The allocation number was: 78709
The block was previously freed by thread 0xC88, and the stack trace (return addresses) at the time was:
402E6F [System][@FreeMem]
4068A8 [System][@DynArrayClear]
405DF9 [System][@FinalizeArray]
4CE9F9 [ReadSC.pas][ReadSC][TScObj.ReadData][298]
4CDD0C [ReadSC.pas][ReadSC][TScObj.LoadFromFile][168]
4D013E [SmplCubImport.pas][SmplCubImport][TCubImport.ImportSample][164]
40461A [System][@AfterConstruction]
4DC151 [UnitAsmJob.pas][UnitAsmJob][TAsmJob.LoadSample][960]

The thing is that I don't see any place in my code where I could wrongfully allocate memory.

type 
 TWordTrace  = array of Word;   
 TDiskTrc  = array of Smallint; 
 var Tracea,Tracec: TWordTrace;

procedure TScObj.ReadData;
Var i: Integer;
    DiskTrc1: TDiskTrc;
    DiskTrc2: TDiskTrc;
    DiskTrc3: TDiskTrc;
    DiskTrc4: TDiskTrc;
begin
 SetLength(DiskTrc1, H.NrOfSamples+1);
 SetLength(DiskTrc2, H.NrOfSamples+1);
 SetLength(DiskTrc3, H.NrOfSamples+1);
 SetLength(DiskTrc4, H.NrOfSamples+1);    <------ log shows error here. <- on DynArraySetLength

 FStream.Seek( H.SOffset, soFromBeginning);

 if H.SampleSize = 1 then
  begin
    for i:= 1 TO H.NrOfSamples DO
     FStream.Read( DiskTrc1[i], 1);
    Unpack(DiskTrc1);

    for i:= 1 TO H.NrOfSamples DO
     FStream.Read( DiskTrc2[i], 1);
    Unpack(DiskTrc2);

    etc...
  end

 else
  begin
   for i:= 1 TO H.NrOfSamples DO
    begin
     FStream.Read( DiskTrc1[i], 2);
     DiskTrc1[i]:= Swap(DiskTrc1[i]);
    end;
   Unpack(DiskTrc1);

   for i:= 1 TO H.NrOfSamples DO
    begin
     FStream.Read( DiskTrc2[i], 2);
     DiskTrc2[i]:= Swap(DiskTrc2[i]);
    end;
   Unpack(DiskTrc2);

   etc...
  end;

 SetLength(Tracea, H.NrOfSamples+1);   
 SetLength(Tracec, H.NrOfSamples+1);
 SetLength(Traceg, H.NrOfSamples+1);
 SetLength(Tracet, H.NrOfSamples+1);   <------ log shows error here. <- on FinalizeArray

 for i:=1 to H.NrOfSamples DO
   begin
     if DiskTrc1[i]< 0
     then Tracea[i]:= 0
     else Tracea[i]:= DiskTrc1[i];

     if DiskTrc2[i]< 0
     then Tracec[i]:= 0
     else Tracec[i]:= DiskTrc2[i];

     etc...
   end;
end;


procedure TScObj.Unpack(VAR DiskTrc: TDiskTrc);
var i: integer;
    Prev: Integer;
    Recover: Integer;
begin
 Prev:= 0;
 for i:= 1 to H.NrOfSamples do
   begin
     Recover := DiskTrc[i] + Prev;
     if (Recover>  32767) OR (Recover< -32768)
     then Recover:= 0;
     DiskTrc[i]:= Recover;
     Prev:= DiskTrc[i];
   end;
 Prev:= 0;
 for i:= 1 to H.NrOfSamples do
   begin
     Recover := DiskTrc[i] + Prev;
     if (Recover>  32767) OR (Recover< -32768)
     then Recover:= 0;
     DiskTrc[i]:= Recover;
     Prev:= DiskTrc[i];
   end;
end;

Later during the "load from disk" procedure, the information from the temporary loader object (SC) is transfered into a more "definitive" object, like this:

 TSam = class
 etc...                                                
 for i:= 1 to NrOfSamples DO
  begin
   CMX[i].Tracea:= SC.Tracea[i];
   CMX[i].Tracec:= SC.Tracec[i];
   etc...
  end;

Edit 2: The bug appears only when I try to open/load a very specific set of (two) files. For all other files the bug doesn't show.

7
What's the declaration of TDiskTrc? Is the element size big enough (at least 2) or are you perhaps overwriting memory by trying to read too many (2) bytes into each element? Also, what do Swap and Unpack do?Ondrej Kelle
Hi TOndrej. I have updated my post to show Unpack. The TDiskTrc can hold those two bytes. Swap is defined in Delphi.system.Z80
BTW, it would be nice to add a link to this question for old one and visa versa.Alex

7 Answers

2
votes

Did you try to run this code with FullDebugModeScanMemoryPoolBeforeEveryOperation enabled? Did you try to call ScanMemoryPoolForCorruptions at TScObj.ReadData's start?

If that doesn't help - try to step into that problem call (GetMem?) and follow FastMM's code to see the address of that corrupted header. Just write it down on the paper and restart the program. There are very high chances that the address of this block will be the same.

Set a breakpoint at safe location - i.e. right before "bad things" happens. Once stopped on it - then set a new breakpoint on memory's location - right at this header, which will become corrupted later (be sure not to set it too early).

Then just run your program - the debugger will stop right at this bad code, which tries to modify header.

1
votes

I think you've got corruption happening elsewhere, and what looks like a routine call is just unmasking the problem. The locations you're pointing to don't seem likely to currupt memory. Without looking at the code in detail, isolating procedures and testing them carefully, it's awfully hard to guess where the problem lies. Do you get any warnings when you compile?

1
votes

I'm thinking the error is not ion the code shown, but there is something you should really check:

SetLength(DiskTrc1, H.NrOfSamples+1);

  for i:= 1 TO H.NrOfSamples DO
     FStream.Read( DiskTrc1[i], 1);

This works thanks to the +1 in Setlength, but are you aware that you are allocating 1 extra item at DiskTrc1[0] that is never used?

I suspect that you mix this with a Setlength(xx, N) somewhere (how is CMX created/dimensioned?).

Note that the normal pattern is

SetLength(DiskTrc1, H.NrOfSamples);

  for i:= 0 TO H.NrOfSamples-1 DO
     FStream.Read( DiskTrc1[i], 1);
1
votes

You need to show more code (like, how is TDiskTrc defined?)

A few points:

Remember that Dynamic arrays are reference-counted, have you tried running with and without Optimizations?

Before using any of these items you may want to assert:

  assert(CMX <> nil);
  assert(Length(CMX) = NrOfSamples+1);

Is there any code (Swap, Unpack, ...) that stores a reference to an Array?

Are you re-using thos SC objects? (Hint: don't)

Are you clearing any arrays (like DiskTrc1 := nil or SetLength(DiskTrc1, 0) ?

1
votes

Everyone seems to be skipping the important error at the start:

FastMM has detected an error during a free block scan operation. FastMM detected that a block has been modified after being freed.

An old pointer is being used somewhere.

Since you have no clue on the pointer error you are looking for I would forget it for now and go look for the other one--they might turn out to be related.

1
votes

Solved. Today I spent the day manually inspecting each line of code. I made quite few changes and finally the problem went away. I haven't tried to see which specific line generated the problem.

Thanks a lot to every body for help !!!

0
votes

Are your arrays 0-based or 1-based?

Multiple times you run loops like this:

if H.SampleSize = 1 then
begin
  for i:= 1 TO H.NrOfSamples DO
    FStream.Read( DiskTrc1[i], 1);
  Unpack(DiskTrc1);
  ...

Are you sure this is not reading past the end of DiskTrc1?