https://github.com/bjope updated https://github.com/llvm/llvm-project/pull/71162
From 1fb659fa15568ec8256824339860de724853c6e3 Mon Sep 17 00:00:00 2001 From: Bjorn Pettersson <bjorn.a.petters...@ericsson.com> Date: Thu, 2 Nov 2023 22:00:52 +0100 Subject: [PATCH 1/2] [clangd] Check for valid source location in ExtractionContext::exprIsValidOutside If the code has a call to an implicitly declared function, an expression could end up referencing declarations without valid source locations. So when doing the exprIsValidOutside check we could end up calling SourceManager::isPointWithin using invalid source locations, and then a debug build would crash with an assertion failure in SourceManager::isBeforeInTranslationUnit. This patch make sure that we deal with the invalid locations (by considering a ReferencedDecl with invalid location as not being inside the Scope). --- .../clangd/refactor/tweaks/ExtractVariable.cpp | 3 ++- clang-tools-extra/clangd/test/implicit-function.test | 11 +++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 clang-tools-extra/clangd/test/implicit-function.test diff --git a/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp b/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp index 1e4edc6d6b4bb39..79bf962544242bb 100644 --- a/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp @@ -176,7 +176,8 @@ bool ExtractionContext::exprIsValidOutside(const clang::Stmt *Scope) const { SourceLocation ScopeBegin = Scope->getBeginLoc(); SourceLocation ScopeEnd = Scope->getEndLoc(); for (const Decl *ReferencedDecl : ReferencedDecls) { - if (SM.isPointWithin(ReferencedDecl->getBeginLoc(), ScopeBegin, ScopeEnd) && + if (ReferencedDecl->getBeginLoc().isValid() && + SM.isPointWithin(ReferencedDecl->getBeginLoc(), ScopeBegin, ScopeEnd) && SM.isPointWithin(ReferencedDecl->getEndLoc(), ScopeBegin, ScopeEnd)) return false; } diff --git a/clang-tools-extra/clangd/test/implicit-function.test b/clang-tools-extra/clangd/test/implicit-function.test new file mode 100644 index 000000000000000..85731f358b4ae8e --- /dev/null +++ b/clang-tools-extra/clangd/test/implicit-function.test @@ -0,0 +1,11 @@ +// RUN: cp %s %t.c +// RUN: not clangd -enable-config=0 -log=verbose -check=%t.c 2>&1 | FileCheck -strict-whitespace %s + +// Regression test for a situation when we used to hit an assertion failure +// due to missing source location (for the implicit function declaration). + +// CHECK-DAG: [-Wimplicit-function-declaration] Line {{.*}}: call to undeclared function 'foo' +// CHECK-DAG: [init_element_not_constant] Line {{.*}}: initializer element is not a compile-time constant +// CHECK: All checks completed, 2 errors + +int x = foo(); From 58084d9b753c08be76fe0b171297352e84d61f87 Mon Sep 17 00:00:00 2001 From: Bjorn Pettersson <bjorn.a.petters...@ericsson.com> Date: Fri, 10 Nov 2023 11:54:25 +0100 Subject: [PATCH 2/2] TO BE SQUASHED Changed the regression test into using unittest. --- clang-tools-extra/clangd/test/implicit-function.test | 11 ----------- .../clangd/unittests/tweaks/ExtractVariableTests.cpp | 8 ++++++++ 2 files changed, 8 insertions(+), 11 deletions(-) delete mode 100644 clang-tools-extra/clangd/test/implicit-function.test diff --git a/clang-tools-extra/clangd/test/implicit-function.test b/clang-tools-extra/clangd/test/implicit-function.test deleted file mode 100644 index 85731f358b4ae8e..000000000000000 --- a/clang-tools-extra/clangd/test/implicit-function.test +++ /dev/null @@ -1,11 +0,0 @@ -// RUN: cp %s %t.c -// RUN: not clangd -enable-config=0 -log=verbose -check=%t.c 2>&1 | FileCheck -strict-whitespace %s - -// Regression test for a situation when we used to hit an assertion failure -// due to missing source location (for the implicit function declaration). - -// CHECK-DAG: [-Wimplicit-function-declaration] Line {{.*}}: call to undeclared function 'foo' -// CHECK-DAG: [init_element_not_constant] Line {{.*}}: initializer element is not a compile-time constant -// CHECK: All checks completed, 2 errors - -int x = foo(); diff --git a/clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp b/clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp index 3d74a941071f849..eb5b06cfee43166 100644 --- a/clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp +++ b/clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp @@ -64,6 +64,14 @@ TEST_F(ExtractVariableTest, Test) { int x = [[1]]; })cpp"; EXPECT_AVAILABLE(AvailableC); + + ExtraArgs = {"-xc"}; + const char *NoCrashCasesC = R"cpp( + // error-ok: broken code, but shouldn't crash + int x = [[foo()]]; + )cpp"; + EXPECT_UNAVAILABLE(NoCrashCasesC); + ExtraArgs = {"-xobjective-c"}; const char *AvailableObjC = R"cpp( __attribute__((objc_root_class)) _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits