Successfully identified regression in *llvm* in CI configuration tcwg_bmk_llvm_apm/llvm-master-aarch64-spec2k6-Oz_LTO. So far, this commit has regressed CI configurations: - tcwg_bmk_llvm_apm/llvm-master-aarch64-spec2k6-Oz_LTO
Culprit: <cut> commit 4aafd5f00c2a772337ec065d4542ef158453a343 Author: Jan Svoboda jan_svoboda@apple.com Date: Fri Aug 6 14:46:41 2021 +0200
[clang] Remove misleading assertion in FullSourceLoc
D31709 added an assertion was added to `FullSourceLoc::hasManager()` that ensured a valid `SourceLocation` is always paired with a `SourceManager`, and missing `SourceManager` is always paired with an invalid `SourceLocation`.
This appears to be incorrect, since clients never cared about constructing `FullSourceLoc` to uphold that invariant, or always checking `isValid()` before calling `hasManager()`.
The assertion started failing when serializing diagnostics pointing into an explicit module. Explicit modules don't have valid `SourceLocation` for the `import` statement, since they are "imported" from the command-line argument `-fmodule-name=x.pcm`.
This patch removes the assertion, since `FullSourceLoc` was never intended to uphold any kind of invariants between the validity of `SourceLocation` and presence of `SourceManager`.
Reviewed By: arphaman
Differential Revision: https://reviews.llvm.org/D106862 </cut>
Results regressed to (for first_bad == 4aafd5f00c2a772337ec065d4542ef158453a343) # reset_artifacts: -10 # build_abe binutils: -9 # build_abe stage1 -- --set gcc_override_configure=--disable-libsanitizer: -8 # build_abe linux: -7 # build_abe glibc: -6 # build_abe stage2 -- --set gcc_override_configure=--disable-libsanitizer: -5 # build_llvm true: -3 # true: 0 # benchmark -- -Oz_LTO artifacts/build-4aafd5f00c2a772337ec065d4542ef158453a343/results_id: 1 # 470.lbm,lbm_base.default regressed by 104
from (for last_good == 3709822d2602b8b7db2d9bcc0e856f676582f25d) # reset_artifacts: -10 # build_abe binutils: -9 # build_abe stage1 -- --set gcc_override_configure=--disable-libsanitizer: -8 # build_abe linux: -7 # build_abe glibc: -6 # build_abe stage2 -- --set gcc_override_configure=--disable-libsanitizer: -5 # build_llvm true: -3 # true: 0 # benchmark -- -Oz_LTO artifacts/build-3709822d2602b8b7db2d9bcc0e856f676582f25d/results_id: 1
Artifacts of last_good build: https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_apm-llvm-master-a... Results ID of last_good: apm_64/tcwg_bmk_llvm_apm/bisect-llvm-master-aarch64-spec2k6-Oz_LTO/3746 Artifacts of first_bad build: https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_apm-llvm-master-a... Results ID of first_bad: apm_64/tcwg_bmk_llvm_apm/bisect-llvm-master-aarch64-spec2k6-Oz_LTO/3725 Build top page/logs: https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_apm-llvm-master-a...
Configuration details:
Reproduce builds: <cut> mkdir investigate-llvm-4aafd5f00c2a772337ec065d4542ef158453a343 cd investigate-llvm-4aafd5f00c2a772337ec065d4542ef158453a343
git clone https://git.linaro.org/toolchain/jenkins-scripts
mkdir -p artifacts/manifests curl -o artifacts/manifests/build-baseline.sh https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_apm-llvm-master-a... --fail curl -o artifacts/manifests/build-parameters.sh https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_apm-llvm-master-a... --fail curl -o artifacts/test.sh https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_apm-llvm-master-a... --fail chmod +x artifacts/test.sh
# Reproduce the baseline build (build all pre-requisites) ./jenkins-scripts/tcwg_bmk-build.sh @@ artifacts/manifests/build-baseline.sh
# Save baseline build state (which is then restored in artifacts/test.sh) mkdir -p ./bisect rsync -a --del --delete-excluded --exclude /bisect/ --exclude /artifacts/ --exclude /llvm/ ./ ./bisect/baseline/
cd llvm
# Reproduce first_bad build git checkout --detach 4aafd5f00c2a772337ec065d4542ef158453a343 ../artifacts/test.sh
# Reproduce last_good build git checkout --detach 3709822d2602b8b7db2d9bcc0e856f676582f25d ../artifacts/test.sh
cd .. </cut>
History of pending regressions and results: https://git.linaro.org/toolchain/ci/base-artifacts.git/log/?h=linaro-local/c...
Artifacts: https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_apm-llvm-master-a... Build log: https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_apm-llvm-master-a...
Full commit (up to 1000 lines): <cut> commit 4aafd5f00c2a772337ec065d4542ef158453a343 Author: Jan Svoboda jan_svoboda@apple.com Date: Fri Aug 6 14:46:41 2021 +0200
[clang] Remove misleading assertion in FullSourceLoc
D31709 added an assertion was added to `FullSourceLoc::hasManager()` that ensured a valid `SourceLocation` is always paired with a `SourceManager`, and missing `SourceManager` is always paired with an invalid `SourceLocation`.
This appears to be incorrect, since clients never cared about constructing `FullSourceLoc` to uphold that invariant, or always checking `isValid()` before calling `hasManager()`.
The assertion started failing when serializing diagnostics pointing into an explicit module. Explicit modules don't have valid `SourceLocation` for the `import` statement, since they are "imported" from the command-line argument `-fmodule-name=x.pcm`.
This patch removes the assertion, since `FullSourceLoc` was never intended to uphold any kind of invariants between the validity of `SourceLocation` and presence of `SourceManager`.
Reviewed By: arphaman
Differential Revision: https://reviews.llvm.org/D106862 --- clang/include/clang/Basic/SourceLocation.h | 13 +++++++------ clang/test/Modules/Inputs/explicit-build-diags/a.h | 1 + .../Modules/Inputs/explicit-build-diags/module.modulemap | 1 + clang/test/Modules/explicit-build-diags.cpp | 8 ++++++++ 4 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/clang/include/clang/Basic/SourceLocation.h b/clang/include/clang/Basic/SourceLocation.h index 540de23b9f55..ba2e9156a2b1 100644 --- a/clang/include/clang/Basic/SourceLocation.h +++ b/clang/include/clang/Basic/SourceLocation.h @@ -363,6 +363,10 @@ class FileEntry; /// A SourceLocation and its associated SourceManager. /// /// This is useful for argument passing to functions that expect both objects. +/// +/// This class does not guarantee the presence of either the SourceManager or +/// a valid SourceLocation. Clients should use `isValid()` and `hasManager()` +/// before calling the member functions. class FullSourceLoc : public SourceLocation { const SourceManager *SrcMgr = nullptr;
@@ -373,13 +377,10 @@ public: explicit FullSourceLoc(SourceLocation Loc, const SourceManager &SM) : SourceLocation(Loc), SrcMgr(&SM) {}
- bool hasManager() const { - bool hasSrcMgr = SrcMgr != nullptr; - assert(hasSrcMgr == isValid() && "FullSourceLoc has location but no manager"); - return hasSrcMgr; - } + /// Checks whether the SourceManager is present. + bool hasManager() const { return SrcMgr != nullptr; }
- /// \pre This FullSourceLoc has an associated SourceManager. + /// \pre hasManager() const SourceManager &getManager() const { assert(SrcMgr && "SourceManager is NULL."); return *SrcMgr; diff --git a/clang/test/Modules/Inputs/explicit-build-diags/a.h b/clang/test/Modules/Inputs/explicit-build-diags/a.h new file mode 100644 index 000000000000..486941dde83b --- /dev/null +++ b/clang/test/Modules/Inputs/explicit-build-diags/a.h @@ -0,0 +1 @@ +void a() __attribute__((deprecated)); diff --git a/clang/test/Modules/Inputs/explicit-build-diags/module.modulemap b/clang/test/Modules/Inputs/explicit-build-diags/module.modulemap new file mode 100644 index 000000000000..bb00c840ce39 --- /dev/null +++ b/clang/test/Modules/Inputs/explicit-build-diags/module.modulemap @@ -0,0 +1 @@ +module a { header "a.h" } diff --git a/clang/test/Modules/explicit-build-diags.cpp b/clang/test/Modules/explicit-build-diags.cpp new file mode 100644 index 000000000000..4a37dc108a68 --- /dev/null +++ b/clang/test/Modules/explicit-build-diags.cpp @@ -0,0 +1,8 @@ +// RUN: rm -rf %t && mkdir %t +// RUN: %clang_cc1 -fmodules -x c++ %S/Inputs/explicit-build-diags/module.modulemap -fmodule-name=a -emit-module -o %t/a.pcm +// RUN: %clang_cc1 -fmodules -Wdeprecated-declarations -fdiagnostics-show-note-include-stack -serialize-diagnostic-file %t/tu.dia \ +// RUN: -I %S/Inputs/explicit-build-diags -fmodule-file=%t/a.pcm -fsyntax-only %s + +#include "a.h" + +void foo() { a(); } </cut>