Segmentation fault with your recent new[]/delete[] warning patch

[Ismail Pazarbasi]

Reduced test case:
  struct C { };

  struct A {
    C *p;
    A() : p{nullptr} { }
    ~A() { delete p; }
  };

This is an oversight. I will fix it by replacing:
  return dyn_cast<const CXXNewExpr>(E);
with:
  return dyn_cast_or_null<const CXXNewExpr>(E);


On Thu, May 14, 2015 at 11:57 PM, Ismail Pazarbasi
<ismail.pazarbasi at gmail.com> wrote:
> On Thu, May 14, 2015 at 11:02 PM, Diego Novillo <dnovillo at google.com> wrote:
>> I've reverted this change for now (r237391). I am reducing the
>> original file. I hope to have a test case for you later
>> today/tomorrow.
>>
>>
>> Thanks.  Diego.
>>
>> On Thu, May 14, 2015 at 3:20 PM, Diego Novillo <dnovillo at google.com> wrote:
>>>
>>> Ismail,
>>>
>>> I ran into a segfault inside the new warning you added in
>>> http://reviews.llvm.org/D4661.  I don't have a repro case for you, but I'm
>>> in a debugging session now and this may help you (and me getting a test case
>>> for you).
>>>
>>> We die in
>>>
>>> clang: llvm/llvm/include/llvm/Support/Casting.h:95: static bool
>>> llvm::isa_impl_cl<const clang::CXXNewExpr, const clang::Expr *>::doit(const
>>> From *) [To = const clang::CXXNewExpr, From = const clang::Expr *]:
>>> Assertion `Val && "isa<> used on a null pointer"' failed.
>>>
>>> The issue starts in MismatchingNewDeleteDetector::analyzeInClassInitializer.
>>> The call to Field->getInClassInitializer() returns the expression:
>>>
>>> (gdb) call E->dump()
>>> InitListExpr 0x7ffff6cdf010 'class Cord *'
>>> `-ImplicitCastExpr 0x7ffff6cdf050 'class Cord *' <NullToPointer>
>>>   `-CXXNullPtrLiteralExpr 0x7ffff6cdefb0 'nullptr_t'
>>>
>>> We then call MismatchingNewDeleteDetector::getNewExprFromInitListOrExpr()
>>> with that expression. The function checks that the initial expression is not
>>> null, but we then decide to get the first init from the list of inits, which
>>> returns a nullptr:
>>>
>>> 2451        if (ILE->getNumInits() == 1)
>>> (gdb) call ILE->dump()
>>> InitListExpr 0x7ffff6cdf010 'class Cord *'
>>> `-ImplicitCastExpr 0x7ffff6cdf050 'class Cord *' <NullToPointer>
>>>   `-CXXNullPtrLiteralExpr 0x7ffff6cdefb0 'nullptr_t'
>>> (gdb) n
>>> 2452          E = dyn_cast<const
>>> CXXNewExpr>(ILE->getInit(0)->IgnoreParenImpCasts());
>>> (gdb) n
>>> 2453      }
>>> (gdb) p E
>>> $6 = (const clang::Expr *) 0x0
>>>
>>> Then we try to dyn_cast<> it, which segfaults:
>>>
>>> (gdb) n
>>> 2455      return dyn_cast<const CXXNewExpr>(E);
>>> (gdb) n
>>> clang: third_party/llvm/llvm/include/llvm/Support/Casting.h:95: static bool
>>> llvm::isa_impl_cl<const clang::CXXNewExpr, const clang::Expr *>::doit(const
>>> From *) [To = const clang::CXXNewExpr, From = const clang::Expr *]:
>>> Assertion `Val && "isa<> used on a null pointer"' failed.
>>>
>>> Program received signal SIGABRT, Aborted.
>>>
>>>
>>>
>>> I don't know whether we should just return nil when E gets nullfiied or this
>>> is a sign of some other problem.
>>>
>>>
>>> Thanks.  Diego.
>
> Thank you for detailed analysis, Diego! I was away last few hours. A
> reduced test case would be perfect. I will try it myself tomorrow, and
> hopefully fix it during the weekend.
>
> Ismail