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