On Nov 26, 2013, at 10:57 PM, Alp Toker <[email protected]> wrote: > > On 27/11/2013 05:50, Argyrios Kyrtzidis wrote: >> Author: akirtzidis >> Date: Tue Nov 26 23:50:55 2013 >> New Revision: 195819 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=195819&view=rev >> Log: >> [libclang] Make sure we don't access past the tokens buffer while token >> annotation. >> >> Also disable crash recovery using 'LIBCLANG_DISABLE_CRASH_RECOVERY' >> environment variable. >> >> Modified: >> cfe/trunk/test/Index/annotate-tokens.cpp >> cfe/trunk/test/Index/crash-recovery.c >> cfe/trunk/tools/libclang/CIndex.cpp >> >> Modified: cfe/trunk/test/Index/annotate-tokens.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/annotate-tokens.cpp?rev=195819&r1=195818&r2=195819&view=diff >> ============================================================================== >> --- cfe/trunk/test/Index/annotate-tokens.cpp (original) >> +++ cfe/trunk/test/Index/annotate-tokens.cpp Tue Nov 26 23:50:55 2013 >> @@ -28,6 +28,11 @@ struct TS { >> template <bool (*tfn)(X*)> >> void TS<tfn>::foo() {} >> >> +void test4() { >> + if (int p = 0) >> + return; >> +} >> + >> // RUN: c-index-test -test-annotate-tokens=%s:1:1:30:1 %s >> -fno-delayed-template-parsing | FileCheck %s >> // CHECK: Keyword: "struct" [1:1 - 1:7] StructDecl=bonk:1:8 (Definition) >> // CHECK: Identifier: "bonk" [1:8 - 1:12] StructDecl=bonk:1:8 (Definition) >> @@ -173,3 +178,10 @@ void TS<tfn>::foo() {} >> // CHECK: Punctuation: ")" [29:19 - 29:20] CXXMethod=foo:29:15 (Definition) >> // CHECK: Punctuation: "{" [29:21 - 29:22] CompoundStmt= >> // CHECK: Punctuation: "}" [29:22 - 29:23] CompoundStmt= >> + >> +// RUN: env LIBCLANG_DISABLE_CRASH_RECOVERY=1 c-index-test >> -test-annotate-tokens=%s:32:1:32:13 %s | FileCheck %s -check-prefix=CHECK2 >> +// CHECK2: Keyword: "if" [32:3 - 32:5] IfStmt= >> +// CHECK2: Punctuation: "(" [32:6 - 32:7] IfStmt= >> +// CHECK2: Keyword: "int" [32:7 - 32:10] VarDecl=p:32:11 (Definition) >> +// CHECK2: Identifier: "p" [32:11 - 32:12] VarDecl=p:32:11 (Definition) >> +// CHECK2: Punctuation: "=" [32:13 - 32:14] VarDecl=p:32:11 (Definition) >> >> Modified: cfe/trunk/test/Index/crash-recovery.c >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/crash-recovery.c?rev=195819&r1=195818&r2=195819&view=diff >> ============================================================================== >> --- cfe/trunk/test/Index/crash-recovery.c (original) >> +++ cfe/trunk/test/Index/crash-recovery.c Tue Nov 26 23:50:55 2013 >> @@ -1,6 +1,7 @@ >> // RUN: not c-index-test -test-load-source all %s 2> %t.err >> // RUN: FileCheck < %t.err -check-prefix=CHECK-LOAD-SOURCE-CRASH %s >> // CHECK-LOAD-SOURCE-CRASH: Unable to load translation unit >> +// RUN: env LIBCLANG_DISABLE_CRASH_RECOVERY=1 not --crash c-index-test >> -test-load-source all %s >> // >> // REQUIRES: crash-recovery >> >> >> Modified: cfe/trunk/tools/libclang/CIndex.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CIndex.cpp?rev=195819&r1=195818&r2=195819&view=diff >> ============================================================================== >> --- cfe/trunk/tools/libclang/CIndex.cpp (original) >> +++ cfe/trunk/tools/libclang/CIndex.cpp Tue Nov 26 23:50:55 2013 >> @@ -5075,18 +5075,26 @@ class AnnotateTokensWorker { >> unsigned BeforeChildrenTokenIdx; >> }; >> SmallVector<PostChildrenInfo, 8> PostChildrenInfos; >> - >> + >> + CXToken &getTok(unsigned Idx) { >> + assert(Idx < NumTokens); >> + return Tokens[Idx]; >> + } >> + const CXToken &getTok(unsigned Idx) const { >> + assert(Idx < NumTokens); >> + return Tokens[Idx]; >> + } >> bool MoreTokens() const { return TokIdx < NumTokens; } >> unsigned NextToken() const { return TokIdx; } >> void AdvanceToken() { ++TokIdx; } >> SourceLocation GetTokenLoc(unsigned tokI) { >> - return SourceLocation::getFromRawEncoding(Tokens[tokI].int_data[1]); >> + return SourceLocation::getFromRawEncoding(getTok(tokI).int_data[1]); >> } >> bool isFunctionMacroToken(unsigned tokI) const { >> - return Tokens[tokI].int_data[3] != 0; >> + return getTok(tokI).int_data[3] != 0; >> } >> SourceLocation getFunctionMacroTokenLoc(unsigned tokI) const { >> - return SourceLocation::getFromRawEncoding(Tokens[tokI].int_data[3]); >> + return SourceLocation::getFromRawEncoding(getTok(tokI).int_data[3]); >> } >> >> void annotateAndAdvanceTokens(CXCursor, RangeComparisonResult, >> SourceRange); >> @@ -5333,7 +5341,7 @@ AnnotateTokensWorker::Visit(CXCursor cur >> // This can happen for C++ constructor expressions whose range generally >> // include the variable declaration, e.g.: >> // MyCXXClass foo; // Make sure we don't annotate 'foo' as a CallExpr >> cursor. >> - if (clang_isExpression(cursorK)) { >> + if (clang_isExpression(cursorK) && MoreTokens()) { >> const Expr *E = getCursorExpr(cursor); >> if (const Decl *D = getCursorParentDecl(cursor)) { >> const unsigned I = NextToken(); >> @@ -5455,14 +5463,23 @@ public: >> } >> >> private: >> + CXToken &getTok(unsigned Idx) { >> + assert(Idx < NumTokens); >> + return Tokens[Idx]; >> + } >> + const CXToken &getTok(unsigned Idx) const { >> + assert(Idx < NumTokens); >> + return Tokens[Idx]; >> + } >> + >> SourceLocation getTokenLoc(unsigned tokI) { >> - return SourceLocation::getFromRawEncoding(Tokens[tokI].int_data[1]); >> + return SourceLocation::getFromRawEncoding(getTok(tokI).int_data[1]); >> } >> >> void setFunctionMacroTokenLoc(unsigned tokI, SourceLocation loc) { >> // The third field is reserved and currently not used. Use it here >> // to mark macro arg expanded tokens with their expanded locations. >> - Tokens[tokI].int_data[3] = loc.getRawEncoding(); >> + getTok(tokI).int_data[3] = loc.getRawEncoding(); >> } >> }; >> >> @@ -6498,6 +6515,11 @@ bool RunSafely(llvm::CrashRecoveryContex >> unsigned Size) { >> if (!Size) >> Size = GetSafetyThreadStackSize(); >> + if (getenv("LIBCLANG_DISABLE_CRASH_RECOVERY")) { >> + // Don't use crash recovery. >> + llvm::llvm_execute_on_thread(Fn, UserData, Size); >> + return true; >> + } > > Hi Argyrios, > > I don't think this change does what you want. It'll force threaded execution > even when not requested. > > How about just: > > --- a/tools/libclang/CIndex.cpp > +++ b/tools/libclang/CIndex.cpp > @@ -2544,7 +2544,8 @@ CXIndex clang_createIndex(int > excludeDeclarationsFromPCH, > int displayDiagnostics) { > // We use crash recovery to make some of our APIs more reliable, implicitly > // enable it. > - llvm::CrashRecoveryContext::Enable(); > + if (!getenv("LIBCLANG_DISABLE_CRASH_RECOVERY")) > + llvm::CrashRecoveryContext::Enable(); > > // Enable support for multithreading in LLVM. > { > > Given that RunSafely() is documented to "execute the given code safely" I > think this would also be more self-documenting. >
That's much better; in r195829, thanks! > Alp. > > >> if (Size) >> return CRC.RunSafelyOnThread(Fn, UserData, Size); >> return CRC.RunSafely(Fn, UserData); >> >> >> _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > > -- > http://www.nuanti.com > the browser experts
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
