View Issue Details

IDProjectCategoryView StatusLast Update
0036643FPCPackagespublic2021-02-21 23:05
ReporterPetr-K Assigned To 
PrioritynormalSeverityminorReproducibilitysometimes
Status newResolutionopen 
Product Version3.3.1 
Summary0036643: Range check error in packages/paszlib/src/trees.pas
DescriptionIf compiled with range checking in function send_tree() construction nextlen := tree[n+1].dl.Len;
The same construction in function scan_tree() was wrapped with {$push}{$R-} ... {$pop} by florian in rev 23890.
Similar patch is included.
TagsNo tags attached.
Fixed in Revision
FPCOldBugId
FPCTarget
Attached Files

Activities

Petr-K

2020-01-31 11:16

reporter  

trees.patch (374 bytes)   
Index: trees.pas
===================================================================
--- trees.pas	(revision 39611)
+++ trees.pas	(working copy)
@@ -1553,7 +1553,9 @@
   for n := 0 to max_code do
   begin
     curlen := nextlen;
+{$push}{$R-}
     nextlen := tree[n+1].dl.Len;
+{$pop}
     inc(count);
     if (count < max_count) and (curlen = nextlen) then
       continue
trees.patch (374 bytes)   

J. Gareth Moreton

2020-01-31 12:14

developer   ~0120823

This might be an ongoing issue with a couple of my recent optimisation improvements on i386 and x86_64. It seems to have messed up the range checking a bit, so I'll have to see what's going on.

Marco van de Voort

2020-02-04 10:54

manager   ~0120878

nextlen is integer, dl.len is word, no mode, so that seems definitely a range violation.

J. Gareth Moreton

2020-02-04 12:07

developer   ~0120879

Does the issue still exist?

J. Gareth Moreton

2020-02-04 12:41

developer   ~0120880

(Range check issue at 0036630 has been resolved - uncertain if this is related)

Pierre Muller

2021-02-21 22:59

developer   ~0129075

Working on another test case, I stumbled on the same range check error.

  Nevertheless, it seems to me that the size of tree variable is max_code,
and that thus, we should ever access tree[max_code+1]

  Maybe adding
  if n=max_code then
    nextlen:=0
else
  nextlen:=tree[n+1].dl.len;

  would be a correct fix?
  However, I don't know the compression algorithm, and have thus no idea if setting nextlen to zero is correct...

  Pierre

Pierre Muller

2021-02-21 23:05

developer   ~0129076

I suspect that the problem, in my case, inherited from Charlie,
is in FLUSH_BLOCK_ONLY function:

(gdb) bt
#0 fpc_rangeerror () at ../inc/system.inc:929
0000001 0x0000000000521bd6 in SEND_TREE (S=..., TREE=..., highTREE=572, MAX_CODE=285) at paszlib/src/trees.pas:1556
0000002 0x0000000000522343 in SEND_ALL_TREES (S=..., LCODES=286, DCODES=30, BLCODES=16) at paszlib/src/trees.pas:1699
0000003 0x0000000000523675 in _TR_FLUSH_BLOCK (S=..., BUF=0x0, STORED_LEN=393472, EOF=true) at paszlib/src/trees.pas:2099
0000004 0x000000000051d990 in FLUSH_BLOCK_ONLY (S=..., EOF=true) at paszlib/src/zdeflate.pas:1666
0000005 0x000000000051e803 in DEFLATE_SLOW (S=..., FLUSH=4) at paszlib/src/zdeflate.pas:2102
0000006 0x000000000051c536 in DEFLATE (STRM=..., FLUSH=4) at paszlib/src/zdeflate.pas:947
0000007 0x0000000000515f27 in FLUSH (this=0x7ffff7ec2040) at paszlib/src/zstream.pp:230
0000008 0x000000000051608a in DESTROY (this=0x7ffff7ec2040, vmt=0x1) at paszlib/src/zstream.pp:245
0000009 0x000000000041cb1e in FREE (this=0x7ffff7ec2040) at ../inc/objpas.inc:336
0000010 0x00000000004e4494 in WRITECOMPRESSEDDATA (this=0x7ffff7fda140) at fcl-image/src/fpwritepng.pp:743
0000011 0x00000000004e4617 in WRITEIDAT (this=0x7ffff7fda140) at fcl-image/src/fpwritepng.pp:756
0000012 0x00000000004e4c3a in INTERNALWRITE (this=0x7ffff7fda140, STR=0x7ffff7ff2200, IMG=0x7ffff7fe2160) at fcl-image/src/fpwritepng.pp:851
0000013 0x00000000004ea3a3 in IMAGEWRITE (this=0x7ffff7fda140, STR=0x7ffff7ff2200, IMG=0x7ffff7fe2160) at fcl-image/src/fphandler.inc:309
0000014 0x00000000004e52e5 in SAVETOSTREAM (this=0x7ffff7fe2160, STR=0x7ffff7ff2200, HANDLER=0x7ffff7fda140) at fcl-image/src/fpimage.inc:59
0000015 0x00000000004e537c in SAVETOFILE (this=0x7ffff7fe2160, FILENAME=0x52b080 <.Ld6$strlab+24> "ErrorFile2.png", HANDLER=0x7ffff7fda140) at fcl-image/src/fpimage.inc:68
0000016 0x00000000004012d1 in main () at testpng2.pas:31
(gdb) f 4
0000004 0x000000000051d990 in FLUSH_BLOCK_ONLY (S=..., EOF=true) at paszlib/src/zdeflate.pas:1666
1666 _tr_flush_block(s, nil,
(gdb) li
1661 begin
1662 if (s.block_start >= 0) then
1663 _tr_flush_block(s, Pbyte(@s.window^[s.block_start]),
1664 longint(longint(s.strstart) - s.block_start), eof)
1665 else
1666 _tr_flush_block(s, nil,
1667 longint(longint(s.strstart) - s.block_start), eof);
1668
1669 s.block_start := s.strstart;
1670 flush_pending(s.strm^);
(gdb) p S.BLOCK_START
$19 = -360448
(gdb) p /x S.BLOCK_START
$20 = 0xfffa8000
(gdb) p /x S.STRSTART
$21 = 0x8100

  FLUSH_BLOCK_ONLY calls _TR_FLUSH_BLOCK (S=..., BUF=0x0, STORED_LEN=393472, EOF=true)
this STORED_LEN is huge compared to the size of the PNG file which is only 8143 bytes!!!

Pierre

Issue History

Date Modified Username Field Change
2020-01-31 11:16 Petr-K New Issue
2020-01-31 11:16 Petr-K File Added: trees.patch
2020-01-31 12:14 J. Gareth Moreton Note Added: 0120823
2020-02-04 10:54 Marco van de Voort Note Added: 0120878
2020-02-04 12:07 J. Gareth Moreton Note Added: 0120879
2020-02-04 12:41 J. Gareth Moreton Note Added: 0120880
2021-02-21 22:59 Pierre Muller Note Added: 0129075
2021-02-21 23:05 Pierre Muller Note Added: 0129076