llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Vlad Serebrennikov (Endilll) <details> <summary>Changes</summary> `SARIFDiagnosticPrinter::HandleDiagnostic` optimistically assumes that its diagnostic renderer (`SARIFDiag`) is always available, but it doesn't hold when driver is parsing arguments and emits unknown argument diagnostic. There is a FIXME to handle diagnostics in this limited mode, but the short-circuit `return` is placed too late, which makes assertion builds crash. See `TextDiagnosticPrinter::handleDiagnostic` for additional context: https://github.com/llvm/llvm-project/blob/42ac09cebaad223f14f4c3052bcd5d6d25be0f3f/clang/lib/Frontend/TextDiagnosticPrinter.cpp#L131-L150 Fixes #<!-- -->233228 --- Full diff: https://github.com/llvm/llvm-project/pull/233241.diff 3 Files Affected: - (modified) clang/lib/Frontend/SARIFDiagnosticPrinter.cpp (+2-1) - (modified) clang/test/Driver/unknown-arg.c (+7) - (modified) clang/test/Frontend/warning-options.cpp (+7) ``````````diff diff --git a/clang/lib/Frontend/SARIFDiagnosticPrinter.cpp b/clang/lib/Frontend/SARIFDiagnosticPrinter.cpp index 72b796f8db7983..edc021f5569656 100644 --- a/clang/lib/Frontend/SARIFDiagnosticPrinter.cpp +++ b/clang/lib/Frontend/SARIFDiagnosticPrinter.cpp @@ -47,7 +47,6 @@ void SARIFDiagnosticPrinter::EndSourceFile() { void SARIFDiagnosticPrinter::HandleDiagnostic(DiagnosticsEngine::Level Level, const Diagnostic &Info) { - assert(SARIFDiag && "SARIFDiagnostic has not been set."); // Default implementation (Warnings/errors count). Keeps track of the // number of errors. DiagnosticConsumer::HandleDiagnostic(Level, Info); @@ -68,6 +67,8 @@ void SARIFDiagnosticPrinter::HandleDiagnostic(DiagnosticsEngine::Level Level, return; } + assert(SARIFDiag && "SARIFDiagnostic has not been set."); + // Assert that the rest of our infrastructure is setup properly. assert(Info.hasSourceManager() && "Unexpected diagnostic with no source manager"); diff --git a/clang/test/Driver/unknown-arg.c b/clang/test/Driver/unknown-arg.c index 26032a407f4131..030952089b4acf 100644 --- a/clang/test/Driver/unknown-arg.c +++ b/clang/test/Driver/unknown-arg.c @@ -65,6 +65,13 @@ // IGNORED: warning: unknown warning option '-Wunknown-to-clang-option' +// RUN %clang -fdiagnostics-format=sarif -Wunknown-to-clang-option + +// FIXME: unlike TextDiagnosticPrinter, SARIFDiagnosticPrinter can't yet emit +// diagnostics in this limited mode without SourceManager, etc. +// At the moment all we can do is to ensure we don't crash with and without +// assertions. + // RUN: %clang -### -offload-arch=sm_70 --offload-arch=sm_70 -offload-device-only -o ffload-device-only \ // RUN: -output -o foo %s 2>&1 | FileCheck --check-prefix=O-WARN %s // O-WARN: warning: joined argument treated as '-o ffload-arch=sm_70'; did you mean '--offload-arch=sm_70'? diff --git a/clang/test/Frontend/warning-options.cpp b/clang/test/Frontend/warning-options.cpp index 444733c8b7f365..e6ddf2e0aad5ea 100644 --- a/clang/test/Frontend/warning-options.cpp +++ b/clang/test/Frontend/warning-options.cpp @@ -6,3 +6,10 @@ // CHECK: unknown warning option '-Wmodule-build'; did you mean '-Wmodule-conflict'? // CHECK-NEXT: unknown -Werror warning specifier: '-Werror-vla' // CHECK: unknown remark option '-Rmodule-built'; did you mean '-Rmodule-build'? + +// RUN: %clang_cc1 -Wmonkey -Wno-monkey -Wno-unused-command-line-arguments \ +// RUN: -Wno-unused-command-line-argument -Wmodule-build -Werror-vla -Rmodule-built -fdiagnostics-format sarif %s 2>&1 +// FIXME: unlike TextDiagnosticPrinter, SARIFDiagnosticPrinter can't yet emit +// diagnostics in this limited mode without SourceManager, etc. +// At the moment all we can do is to ensure we don't crash with and without +// assertions. `````````` </details> https://github.com/llvm/llvm-project/pull/233241 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
