[PATCH] D60523: [clang] Don't segfault on incorrect using directive (PR41400)

Tue Apr 23 18:43:15 PDT 2019

jkorous added a comment.

I can't really comment on correctness of your fix but had been willing to do the work I'd suggest making `ASTContext::getDependentNameType` and `DependentNameType::DependentNameType` interface more robust.

With current master (95c18c7beec <https://reviews.llvm.org/rG95c18c7beec32eaa143ed1f4cea4944e09aa9708>) the crash is here (with your test):

thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x10)

    frame #0: 0x0000000105f0c2e0 clang` clang::NestedNameSpecifier::getKind(this=0x0000000000000000) const  + 16 at NestedNameSpecifier.cpp:143
    frame #1: 0x0000000105f0c659 clang` clang::NestedNameSpecifier::containsUnexpandedParameterPack(this=0x0000000000000000) const  + 25 at NestedNameSpecifier.cpp:253
  * frame #2: 0x00000001059f2dc1 clang` clang::DependentNameType::DependentNameType(this=0x0000000116852b30, Keyword=ETK_None, NNS=0x0000000000000000, Name=0x000000011686eff8, CanonType=QualType @ 0x00007ffeefbf7818)  + 65 at Type.h:5238
    frame #3: 0x00000001059c1113 clang` clang::DependentNameType::DependentNameType(this=0x0000000116852b30, Keyword=ETK_None, NNS=0x0000000000000000, Name=0x000000011686eff8, CanonType=QualType @ 0x00007ffeefbf7858)  + 51 at Type.h:5239
    frame #4: 0x00000001059c0d9f clang` clang::ASTContext::getDependentNameType(this=0x0000000116815000, Keyword=ETK_None, NNS=0x0000000000000000, Name=0x000000011686eff8, Canon=QualType @ 0x00007ffeefbf7990) const  + 431 at ASTContext.cpp:4247
    frame #5: 0x000000010509f539 clang` clang::Sema::getConstructorName(this=0x0000000116846600, II=0x000000011686eff8, NameLoc=(ID = 35), S=0x000000011530bc40, SS=0x00007ffeefbf8730, EnteringContext=false)  + 377 at SemaExprCXX.cpp:94

The problem starts in frame #5 in `getConstructorName` where your fix is but

- `DependentNameType` should also have a tighter interface and not accept `NSS = nullptr` in constructor since it obviously expects some value to be present (calling a method on it) - the parameter should be a reference.
- `ASTContext::getDependentNameType` also seems to expect some non-null value - it should either make this obvious in the interface by using a reference or deal with absence of the value in a reasonable manner.

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60523/new/

https://reviews.llvm.org/D60523