Hi Kit, fwindowslist is created in the constructor, which may explain why this bug is dormant. I assume this is supposed to be a defensive check, although fwindowslist is also accessed later in this method without a safety check. Perhaps the "if not? assigned()" check can be omitted since it isn't sufficient protection and the constructor should have automatically created the fwindowslist class.
My 2 cents... On Wed, Feb 21, 2024 at 12:16 PM J. Gareth Moreton via fpc-devel < fpc-devel@lists.freepascal.org> wrote: > Hi everyone, > > While evaluating a new peephole optimisation, I came across a null > pointer dereference in the assembly language. After looking at the > original Pascal code, I came across this starting at line 525 of > packages/chm/src/chmreader.pas: > > procedure TChmReader.ReadWindows(mem:TMemoryStream); > > var > i,cnt, > version : integer; > x : TChmWindow; > begin > if not assigned(fwindowslist) then > fWindowsList.Clear; > mem.Position:=0; > ... > > This code looks very suspicious to me because it calls > fWindowsList.Clear only if fWindowsList is a null pointer. This will > instantly cause an access violation (Clear is not a class method). > > Without the new optimisation, this is what the x86_64-win64 assembly > language looks like: > > cmpq $0,280(%rbx) > jne .Lj189 > movq 280(%rbx),%rcx > movq (%rcx),%rax > call *216(%rax) > .Lj189: > > If JNE doesn't branch, then the value at 280(%rbx) is zero, and this is > then copied into %rcx, then the value referenced by %rcx is stored in > %rax, however because the value at 280(%rbx) is zero, then %rcx is also > zero and (%rcx) is a null pointer dereference. > > Kit > > _______________________________________________ > fpc-devel maillist - fpc-devel@lists.freepascal.org > https://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel >
_______________________________________________ fpc-devel maillist - fpc-devel@lists.freepascal.org https://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel
