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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2020-08-20
                 CC|                            |jakub at gcc dot gnu.org
             Status|UNCONFIRMED                 |NEW

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The __builtin_trap is there because of:
            if (isEmptyBucket(oldEntry)) {
                do { if (!(std::addressof(oldEntry) != entry)) {
WTFReportAssertionFailure("WTF/wtf/HashTable.h", 1337, __PRETTY_FUNCTION__,
"std::
                oldTable[i].~ValueType();
                continue;
            }
where the type of oldTable[i] and oldEntry is struct Thread *, isEmptyBucket is
== NULL check and invoking a pseudo-destructor.

Playing with this, I've come up with:
typedef int *T;

void
foo (T a)
{
  if (a)
    return;
  a.~T ();
}

int
main ()
{
  int p;
  foo (&p);
  foo (nullptr);
  return 0;
}
testcase.

Starting with r11-2238-ge443d8213864ac337c29092d4767224f280d2062 we emit:
  if (a != 0B) goto <D.2343>; else goto <D.2344>;
  <D.2343>:
  // predicted unlikely by early return (on trees) predictor.
  return;
  <D.2344>:
  *a = {CLOBBER};
which doesn't look right to me, that is as if the (pseudo)destructor is run on
what the pointer points to (so if the testcase is changed to also have typedef
int V; and use a->~V (); rather than a.~T ();).
That is one thing, but I'd say in that case it ought to crash only if a is
NULL, because the pseudo destructor is invoked only in that case.
With that we have:
  if (a_2(D) != 0B)
    goto <bb 3>; [INV]
  else
    goto <bb 4>; [INV]

  <bb 3> :
  // predicted unlikely by early return (on trees) predictor.
  goto <bb 5>; [INV]

  <bb 4> :
  MEM[(int *)0B] ={v} {CLOBBER};

  <bb 5> :
  return;
before cddce1, but we then "optimize" it into:
  <bb 2> :
  MEM[(int *)0B] ={v} {CLOBBER};
  return;

So, I'd say we have two bugs here, one is that the FE since r1-2238 mishandles
pseudo-destructors of pointer types, for that use the above testcase, and then
that we mishandle clobbers in DCE or so, for which the testcase is:
struct S { int s; ~S () {} };

void
foo (S *a)
{
  if (a)
    return;
  a->~S ();
}

int
main ()
{
  S s;
  foo (&s);
  foo ((S *) 0);
}
which started to be really miscompiled with r249450, but had the questionable
unconditional
  MEM[(struct S *)0B] ={v} {CLOBBER};
as first stmt in main already since r197375.

So, in addition to fixing the C++ FE, I think we either shall declare that
clobbers never trap even if invoked with NULL pointers, then cddce can do the
optimization it does, but we shouldn't turn it into __builtin_trap later, or we
disable the optimization that happens during dce if the pointer in MEM_REF lhs
of a clobber is or might be NULL.

Now, whether fixing this (both or just one) fixes the above reported bug is
unclear.

Reply via email to