r231451 - Don't crash on non-public referenced dtors in toplevel classes.

[Nico Weber]

On Fri, Mar 6, 2015 at 9:32 AM, David Blaikie <dblaikie at gmail.com> wrote:

>
>
> On Thu, Mar 5, 2015 at 10:01 PM, Nico Weber <nicolasweber at gmx.de> wrote:
>
>> Author: nico
>> Date: Fri Mar  6 00:01:06 2015
>> New Revision: 231451
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=231451&view=rev
>> Log:
>> Don't crash on non-public referenced dtors in toplevel classes.
>>
>> Fixes PR22793, a bug that caused self-hosting to fail after the innocuous
>> r231254. See the bug for details.
>>
>
> Awesome - thanks for jumping on this!
>
>
>>
>> Modified:
>>     cfe/trunk/lib/Sema/SemaExpr.cpp
>>     cfe/trunk/test/CodeGenCXX/trivial-constructor-init.cpp
>>
>> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=231451&r1=231450&r2=231451&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Fri Mar  6 00:01:06 2015
>> @@ -117,7 +117,7 @@ static AvailabilityResult DiagnoseAvaila
>>      case AR_Available:
>>      case AR_NotYetIntroduced:
>>        break;
>> -
>> +
>>      case AR_Deprecated:
>>        if (S.getCurContextAvailability() != AR_Deprecated)
>>          S.EmitAvailabilityWarning(Sema::AD_Deprecation,
>> @@ -11859,8 +11859,11 @@ void Sema::MarkFunctionReferenced(Source
>>    } else if (CXXDestructorDecl *Destructor =
>>                   dyn_cast<CXXDestructorDecl>(Func)) {
>>      Destructor = cast<CXXDestructorDecl>(Destructor->getFirstDecl());
>> -    if (Destructor->isDefaulted() && !Destructor->isDeleted())
>> +    if (Destructor->isDefaulted() && !Destructor->isDeleted()) {
>> +      if (Destructor->isTrivial() &&
>> !Destructor->hasAttr<DLLExportAttr>())
>> +        return;
>>        DefineImplicitDestructor(Loc, Destructor);
>> +    }
>>      if (Destructor->isVirtual() && getLangOpts().AppleKext)
>>        MarkVTableUsed(Loc, Destructor->getParent());
>>    } else if (CXXMethodDecl *MethodDecl = dyn_cast<CXXMethodDecl>(Func)) {
>>
>> Modified: cfe/trunk/test/CodeGenCXX/trivial-constructor-init.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/trivial-constructor-init.cpp?rev=231451&r1=231450&r2=231451&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/CodeGenCXX/trivial-constructor-init.cpp (original)
>> +++ cfe/trunk/test/CodeGenCXX/trivial-constructor-init.cpp Fri Mar  6
>> 00:01:06 2015
>> @@ -32,3 +32,17 @@ static C c[4];
>>
>>  int main() {
>>  }
>> +
>> +namespace PR22793 {
>> +template <typename>
>> +struct foo {
>> +protected:
>>
>
> Naively, I was hoping a/the fix might resolve around this being protected.
> I was sort of hoping that the fix might involve /removing/ code that was
> needlessly checking access rather than adding more conditions. Seems
> strange that the access level could change the behavior of this code. (but
> I haven't fully understood the notes you made on the bug - so I imagine
> it's probably described in there somewhere) Any chance of that?
>

Public defaulted dtors are marked as "irrelevant destructors", and that
triggers an early return somewhere. If a destructor is not public, it isn't
irrelevant to sema though, as it now needs access checks. So the early
return that "protects" irrelevant destructors doesn't fire for this case,
and we need to explicitly check for triviality. As the commit message says,
constructors have the same check a few lines further up.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150306/49e9c9c2/attachment.html>