[PATCH] D31697: Check for null before using TUScope
kimgr added a comment. This was eventually fixed in IWYU. I believe the problem is/was: - After code is parsed and the AST is built, Sema resets its `TUScope` member to null - We use Sema to lookup and define default constructors before traversing the AST using a RAV - Some yet-not-fully-identified constructs cause Sema to need a TUScope for this (something to do with `LazilyCreateBuiltin`, but I haven't been able to construct a reduced testcase) So in https://github.com/include-what-you-use/include-what-you-use/commit/5e7843434169a8af05ebd6e1434cc3cffb66, we explicitly re-point `Sema::TUScope` to `Sema::getCurScope()`, which seems to have fixed the crash. This revision can be closed. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D31697/new/ https://reviews.llvm.org/D31697 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31697: Check for null before using TUScope
kimgr added a comment.
> only for the function templates that use Microsoft intrinsics (e.g.
> _BitScanForward in TrailingZerosCounter.)
> So there's something in the parsing of builtins/intrinsics that requires
> TUScope to be non-null.
For posterity, this was misdiagnosed on my part. It turns out the pp
conditionals in MathExtras.h select the GCC-style intrinsics for Clang, even in
MS compat mode (because `_MSC_VER` is consistently checked *after*
`__has_builtin` and friends).
So the problem is really with GCC builtins inside function templates. Here's a
minimal repro:
template
int myctz(unsigned int value) {
return __builtin_ctz(value);
}
https://reviews.llvm.org/D31697
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D31697: Check for null before using TUScope
Might want to ask on cfe-dev or irc for more visibility since this review thread isn't getting much action On Thu, Jun 29, 2017 at 5:12 AM Kim Gräsman via Phabricator < revi...@reviews.llvm.org> wrote: > kimgr added a comment. > > I did some more debugging today. This happens when we attempt to analyze > llvm/Support/MathExtras.h, and only for the function templates that use > Microsoft intrinsics (e.g. `_BitScanForward` in `TrailingZerosCounter`.) > So there's something in the parsing of builtins/intrinsics that requires > `TUScope` to be non-null. > > Can we seed Sema with a valid `TUScope` before invoking > `LateTemplateParser`, and if so, how? Or is this because we invoke the > parser multiple times? I'm guessing Clang is already done parsing when we > invoke the late template parsing. > > Grateful for any ideas here. > > > https://reviews.llvm.org/D31697 > > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31697: Check for null before using TUScope
kimgr added a comment. I did some more debugging today. This happens when we attempt to analyze llvm/Support/MathExtras.h, and only for the function templates that use Microsoft intrinsics (e.g. `_BitScanForward` in `TrailingZerosCounter`.) So there's something in the parsing of builtins/intrinsics that requires `TUScope` to be non-null. Can we seed Sema with a valid `TUScope` before invoking `LateTemplateParser`, and if so, how? Or is this because we invoke the parser multiple times? I'm guessing Clang is already done parsing when we invoke the late template parsing. Grateful for any ideas here. https://reviews.llvm.org/D31697 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31697: Check for null before using TUScope
kimgr added a comment. We'd love to see this addressed, either in our code or in Clang. But we're not sure what to do on our end, so... a gentle ping for help! https://reviews.llvm.org/D31697 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31697: Check for null before using TUScope
kimgr added a comment.
For some context, the backtrace when this happens is:
include-what-you-use.exe!clang::Scope::getEntity() Line 313C++
include-what-you-use.exe!clang::Sema::PushOnScopeChains(clang::NamedDecl *
D=0x07ed05b0, clang::Scope * S=0x, bool AddToContext=true) Line 1302 C++
include-what-you-use.exe!clang::Sema::LazilyCreateBuiltin(clang::IdentifierInfo
* II=0x008087f0, unsigned int ID=0x0114, clang::Scope * S=0x, bool
ForRedeclaration=false, clang::SourceLocation Loc={...}) Line 1910 C++
include-what-you-use.exe!LookupBuiltin(clang::Sema & S={...},
clang::LookupResult & R={...}) Line 699 C++
include-what-you-use.exe!LookupDirect(clang::Sema & S={...},
clang::LookupResult & R={...}, const clang::DeclContext * DC=0x007c1a14) Line
850 C++
include-what-you-use.exe!CppNamespaceLookup(clang::Sema & S={...},
clang::LookupResult & R={...}, clang::ASTContext & Context={...},
clang::DeclContext * NS=0x007c1a14,
`anonymous-namespace'::UnqualUsingDirectiveSet & UDirs={...}) Line 931 C++
include-what-you-use.exe!clang::Sema::CppLookupName(clang::LookupResult
& R={...}, clang::Scope * S=0x0078c870) Line 1310 C++
include-what-you-use.exe!clang::Sema::LookupName(clang::LookupResult &
R={...}, clang::Scope * S=0x079fcd10, bool AllowBuiltinCreation=false) Line
1798 C++
include-what-you-use.exe!clang::Sema::getTypeName(const
clang::IdentifierInfo & II={...}, clang::SourceLocation NameLoc={...},
clang::Scope * S=0x079fcd10, clang::CXXScopeSpec * SS=0x0449db94, bool
isClassName=false, bool HasTrailingDot=false, clang::OpaquePtr
ObjectTypePtr={...}, bool IsCtorOrDtorName=false, bool
WantNontrivialTypeSourceInfo=true, bool IsClassTemplateDeductionContext=true,
clang::IdentifierInfo * * CorrectedII=0x) Line 331 C++
include-what-you-use.exe!clang::Parser::TryAnnotateTypeOrScopeTokenAfterScopeSpec(clang::CXXScopeSpec
& SS={...}, bool IsNewScope=true) Line 1727 C++
include-what-you-use.exe!clang::Parser::TryAnnotateTypeOrScopeToken()
Line 1717 C++
include-what-you-use.exe!clang::Parser::ParseCastExpression(bool
isUnaryExpression=false, bool isAddressOfOperand=false, bool &
NotCastExpr=false, clang::Parser::TypeCastState isTypeCast=NotTypeCast, bool
isVectorLiteral=false) Line 876C++
include-what-you-use.exe!clang::Parser::ParseCastExpression(bool
isUnaryExpression=false, bool isAddressOfOperand=false,
clang::Parser::TypeCastState isTypeCast=NotTypeCast, bool
isVectorLiteral=false) Line 484 C++
include-what-you-use.exe!clang::Parser::ParseAssignmentExpression(clang::Parser::TypeCastState
isTypeCast=NotTypeCast) Line 171 C++
include-what-you-use.exe!clang::Parser::ParseExpression(clang::Parser::TypeCastState
isTypeCast=NotTypeCast) Line 121 C++
include-what-you-use.exe!clang::Parser::ParseReturnStatement() Line
1905C++
include-what-you-use.exe!clang::Parser::ParseStatementOrDeclarationAfterAttributes(llvm::SmallVector & Stmts={...}, clang::Parser::AllowedConstructsKind Allowed=ACK_Any,
clang::SourceLocation * TrailingElseLoc=0x,
clang::Parser::ParsedAttributesWithRange & Attrs={...}) Line 265C++
include-what-you-use.exe!clang::Parser::ParseStatementOrDeclaration(llvm::SmallVector & Stmts={...}, clang::Parser::AllowedConstructsKind Allowed=ACK_Any,
clang::SourceLocation * TrailingElseLoc=0x) Line 113 C++
include-what-you-use.exe!clang::Parser::ParseCompoundStatementBody(bool
isStmtExpr=false) Line 996 C++
include-what-you-use.exe!clang::Parser::ParseFunctionStatementBody(clang::Decl
* Decl=0x07e84ce0, clang::Parser::ParseScope & BodyScope={...}) Line 1965
C++
include-what-you-use.exe!clang::Parser::ParseLateTemplatedFuncDef(clang::LateParsedTemplate
& LPT={...}) Line 1418 C++
include-what-you-use.exe!clang::Parser::LateTemplateParserCallback(void
* P=0x0077d438, clang::LateParsedTemplate & LPT={...}) Line 1339C++
include-what-you-use.exe!include_what_you_use::IwyuAstConsumer::ParseFunctionTemplates(clang::TranslationUnitDecl
* decl=0x007c1a00) Line 3558 C++
include-what-you-use.exe!include_what_you_use::IwyuAstConsumer::HandleTranslationUnit(clang::ASTContext
& context={...}) Line 3503 C++
include-what-you-use.exe!clang::ParseAST(clang::Sema & S={...}, bool
PrintStats=false, bool SkipFunctionBodies=false) Line 159 C++
include-what-you-use.exe!clang::ASTFrontendAction::ExecuteAction() Line
611 C++
include-what-you-use.exe!clang::FrontendAction::Execute() Line 512
C++
include-what-you-use.exe!clang::CompilerInstance::ExecuteAction(clang::FrontendAction
& Act={...}) Line 970 C++
This is triggered when we invoke a hacky hook in Sema to force instantiation of
late-parsed function templates:
void
[PATCH] D31697: Check for null before using TUScope
zturner created this revision. To be honest I don't really understand anything about this code. I encountered a situation when running `include-what-you-use` against LLVM where `TUScope` was null here, triggering a segfault. Some surrounding comments and code suggests that this variable is specific to Objective C, but I don't really grok whether null is indicative here of an earlier problem or whether it should actually be handled. There are other places in this file where it is checked for null, so there is at least some precedent for it in Sema. https://reviews.llvm.org/D31697 Files: clang/lib/Sema/SemaDecl.cpp Index: clang/lib/Sema/SemaDecl.cpp === --- clang/lib/Sema/SemaDecl.cpp +++ clang/lib/Sema/SemaDecl.cpp @@ -1906,7 +1906,8 @@ // entirely, but we're not there yet. DeclContext *SavedContext = CurContext; CurContext = Parent; - PushOnScopeChains(New, TUScope); + if (TUScope) +PushOnScopeChains(New, TUScope); CurContext = SavedContext; return New; } Index: clang/lib/Sema/SemaDecl.cpp === --- clang/lib/Sema/SemaDecl.cpp +++ clang/lib/Sema/SemaDecl.cpp @@ -1906,7 +1906,8 @@ // entirely, but we're not there yet. DeclContext *SavedContext = CurContext; CurContext = Parent; - PushOnScopeChains(New, TUScope); + if (TUScope) +PushOnScopeChains(New, TUScope); CurContext = SavedContext; return New; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
