r195819 - [libclang] Make sure we don't access past the tokens buffer while token annotation.
Alp Toker
alp at nuanti.com
Tue Nov 26 22:57:52 PST 2013
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.
Alp.
> if (Size)
> return CRC.RunSafelyOnThread(Fn, UserData, Size);
> return CRC.RunSafely(Fn, UserData);
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
--
http://www.nuanti.com
the browser experts
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131127/58d40e1d/attachment.html>
More information about the cfe-commits
mailing list