[Bug tree-optimization/95663] static_cast checks for null even when the pointer is dereferenced

["rguenth at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95663

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |law at gcc dot gnu.org,
                   |                            |rguenth at gcc dot gnu.org
          Component|c++                         |tree-optimization
           Keywords|                            |missed-optimization
             Target|                            |x86_64-*-* i?86-*-*

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
I suppose the C++ standard says static_cast<Derived *>(nullptr) == nullptr and
we literally follow that.  Note it will make a difference for very large
objects (and thus very large offsets added) which may end up acccessing
actually
mapped memory so IMHO what clang does by default is a security risk.

Now, what we should eventually improve is the code generated in the isolated
path.  On GIMPLE we retain the load:

  <bb 4> [count: 0]:
  _7 ={v} MEM[(struct Derived *)0B].D.2340.y;
  __builtin_trap ();

(because it can trap).  The use of the cold section for the
ud2 is probably also bad since it will cause a larger jump instruction
where very likely

    testq %rdi, %rdi
    jne .L2
    ud2
.L2:
    movl (%rdi), ...

would be both faster and smaller.  For reference the generated code:

_Z5fieldP5Base2:
.LFB1:
        .cfi_startproc
        testq   %rdi, %rdi
        je      .L2
        movl    (%rdi), %eax
        ret
        .cfi_endproc
        .section        .text.unlikely
        .cfi_startproc
        .type   _Z5fieldP5Base2.cold, @function
_Z5fieldP5Base2.cold:
.LFSB1:
.L2:
        movl    4, %eax
        ud2


CCing Jeff for the RTL side representation - IIRC we have some special
CFG magic for gcc_unreachable, not sure if what we end up with
trap() matches that or if we should adjust this somehow.  Currently DCE
marks the load as always necessary because it seems isolate-paths makes the
load
volatile:

  /* We want the NULL pointer dereference to actually occur so that
     code that wishes to catch the signal can do so.

...

fair enough - but as you see above we dereference not NULL but some
derived constant which might not actually trap.  I wonder if it is
more useful/safe to replace the load with a plain *(char *)0?

Note I don't think what clang does here is reasonable.