mORMot Open Source

rvk

Member

Registered: 2022-04-14

Posts: 93

Access violation in Drawbitmap in PDF engine with small bitmaps

I'm trying to find out why sometimes the DrawBitmap crashes with an exception.
BTW. The exceptions are handled by the SaveToFile code but it still leaks a TPDFWriter, but that's not the topic here/

The code crashes on this line in TPdfDocument.CreateOrGetImage for bitmaps smaller than 5 pixels:

for y := 0 to h - 1 do
   DefaultHasher128(@hash, B.{%H-}ScanLine[y], row);

I was originally doing this in Win32 and there it was really unpredictable. One run ok, other run crash.
When switching to Win64 I could let it crash almost consistently.

My concrete question is... is for that DefaultHasher128 function a bitmap needed with a width bigger or multiple of 4 bytes or 128 bits or something??
If yes, the code should check for that.

Below the code to reproduce (with just a blank bitmap).

Crashes almost consistently with Win64 in mormot.crypt.core.asmx86.inc (_AesNiHashXmm0) and in Win32 sometimes/unpredictable in mormot.crypt.core.asmx86.inc (_AesNiHash128).
(The access violation is handled by exception but results in a PDF with size 0.)

program test_pdf_small;

{$APPTYPE CONSOLE}

uses
  System.SysUtils, System.Classes, mormot.ui.pdf, Vcl.Graphics, WinApi.Windows, ShellApi, System.IOUtils;

function GetFileSize(const FileName : string) : Int64;
var
  Reader: TFileStream;
begin
  Reader := TFile.OpenRead(FileName);
  try
    result := Reader.Size;
  finally
    Reader.Free;
  end;
end;

var
  pdf: TPdfDocumentGDI;
  MemBmp: Vcl.Graphics.TBitmap;
  I: Integer;
begin
  for I := 1 to 100 do
  begin

    pdf := TPdfDocumentGDI.Create;
    try
      pdf.DefaultPageWidth := 30;
      pdf.DefaultPageHeight := 30;
      pdf.AddPage;

      // create a very small bitmap and copy to canvas
      MemBmp := Vcl.Graphics.TBitmap.Create;
      try
        MemBmp.Width := 4;
        MemBmp.Height := 4;
        BitBlt(pdf.VclCanvas.Handle, 0, 0, MemBmp.Width, MemBmp.Height, MemBmp.Canvas.Handle, 0, 0, SRCCOPY);
      finally
          MemBmp.Free;
      end;

      // crashes here, also note the leak when savetofile has an silent exception
      pdf.SaveToFile('test.pdf');

    finally
        pdf.Free;
    end;

    if GetFileSize('test.pdf') = 0 then
      writeln('error on ' + I.ToString)
    else
      writeln('correct  ' + I.ToString);

  end;

  // ShellExecute(0, 'open', 'test.pdf', nil, nil, SW_NORMAL);

  writeln('done');
  readln;

end.

ab

Administrator

From: France

Registered: 2010-06-21

Posts: 14,240

Website

Re: Access violation in Drawbitmap in PDF engine with small bitmaps

DefaultHasher128 = AesNiHash should not read more than the number of bytes specified.

What is the value of "row" integer in your case?

What is the exact GPF line in the AesNiHash asm?

rvk

Member

Registered: 2022-04-14

Posts: 93

Re: Access violation in Drawbitmap in PDF engine with small bitmaps

DefaultHasher128 = AesNiHash should not read more than the number of bytes specified.

What is the value of "row" integer in your case?

What is the exact GPF line in the AesNiHash asm?

Ok, then there is something going on with the row.

It's a 24bit bitmap (default)

row := PERROW[B.PixelFormat];    = 24 bits = 3 bytes

w = 3    ---> that should be 3 * 3 = 9 bytes per row ???

row := (((w * row) + 31) and (not 31)) shr 3; // inlined BytesPerScanLine

row = 12

for y := 0 to h - 1 do
  DefaultHasher128(@hash, B.{%H-}ScanLine[y], row);

So the row should be 9 bytes for a 3 pixel bitmap, wouldn't it?

The row value is 12. Seems 3 bytes too much

Crashed line is in _AesNiHashXmm0 for 64 bit.
But if row (len) is indeed 12 for a 3 pixel bitmap then that seems logical that it crashes there:

        movups  xmm1, dqword ptr [data + len - 16] // no read after end of page

I don't know if the scanline is supposed to be aligned at 32 bit but seeing that access violation it seems not.
(the access violation is always on the last scanline)

Last edited by rvk (2023-09-09 20:34:13)

rvk

Member

Registered: 2022-04-14

Posts: 93

Re: Access violation in Drawbitmap in PDF engine with small bitmaps

Mmm, Weird.
According to this it should indeed be 12.

// Alignment must be a power of 2.  Color BMPs require DWORD alignment (32).
function BytesPerScanline(PixelsPerScanline, BitsPerPixel, Alignment: Longint): Longint;
begin
  Dec(Alignment);
  Result := ((PixelsPerScanline * BitsPerPixel) + Alignment) and not Alignment;
  Result := Result div 8;
end;

((3 * 24) + 31) and not 31 = 96 div 8 = 12

Then it must be further on in _AesNiHashXmm0 .

rvk

Member

Registered: 2022-04-14

Posts: 93

Re: Access violation in Drawbitmap in PDF engine with small bitmaps

Wait??? Doesn't this line read bytes BEFORE the data structure if len is 12????

movups  xmm1, dqword ptr [data + len - 16] // no read after end of page

data is the first pointer, len is 12 so data + len - 16 is going to do data - 4 ?

So using a array of less than 128 bits (16 bytes) is really dangerous with this function

(that would be any bitmap with width < 6 pixels for 24bit pixels which seems to be consistent with my experience)

~~
I tried to test with this but I can't fool it.

var
  a: array[1..1] of byte;
  hash: THash128Rec;
begin
  DefaultHasher128(@hash, @a, 12);

You think that should give an AV.

~~
There would not be an immediate need to fix this, but we should be aware of it

(or a check could be added in drawbitmap and any other place where this hash is used)

And if someone needs a quick "fix".... this works too (just using another hasher128)

  DefaultHasher128 := @crc32c128;

Last edited by rvk (2023-09-09 21:45:05)