Skip to content

Conversation

@chestnykh
Copy link
Contributor

@chestnykh chestnykh commented Dec 19, 2024

There are a few functions that emit warnings related to positional arguments in format strings. These functions use getLocationOfByte() which has O(n) complexity and may lead to silent hang of compilation in some cases. But such warnings is not widely used and actually don't emit if user didn't pass the appropriate -W... flag, so if the flag is not passed dont make the call to EmitFormatDiagnostic for such diags.

Fix #120462

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Dec 19, 2024
@llvmbot
Copy link
Member

llvmbot commented Dec 19, 2024

@llvm/pr-subscribers-clang

Author: Dmitry Chestnykh (chestnykh)

Changes

There are a few functions that emit warnings related to positional arguments in format strings. These functions use getLocationOfByte() which has O(n) complexity and may lead to silent hang of compilation in some cases. But such warnings is not widely used and actually don't emit if user didn't pass the appropriate -W... flag, so if the flag is not passed dont make the call to EmitFormatDiagnostic for such diags.

Fix #120462


Full diff: https://github.com/llvm/llvm-project/pull/120591.diff

1 Files Affected:

  • (modified) clang/lib/Sema/SemaChecking.cpp (+18-12)
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index be5d3694aec152..a745250988feeb 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -6612,27 +6612,33 @@ void CheckFormatHandler::HandleNonStandardConversionSpecifier(
 
 void CheckFormatHandler::HandlePosition(const char *startPos,
                                         unsigned posLen) {
-  EmitFormatDiagnostic(S.PDiag(diag::warn_format_non_standard_positional_arg),
-                               getLocationOfByte(startPos),
-                               /*IsStringLocation*/true,
-                               getSpecifierRange(startPos, posLen));
+  if (!S.getDiagnostics().isIgnored(diag::warn_format_non_standard_positional_arg, SourceLocation())) {
+    EmitFormatDiagnostic(S.PDiag(diag::warn_format_non_standard_positional_arg),
+                                getLocationOfByte(startPos),
+                                /*IsStringLocation*/true,
+                                getSpecifierRange(startPos, posLen));
+  }
 }
 
 void CheckFormatHandler::HandleInvalidPosition(
     const char *startSpecifier, unsigned specifierLen,
     analyze_format_string::PositionContext p) {
-  EmitFormatDiagnostic(
-      S.PDiag(diag::warn_format_invalid_positional_specifier) << (unsigned)p,
-      getLocationOfByte(startSpecifier), /*IsStringLocation*/ true,
-      getSpecifierRange(startSpecifier, specifierLen));
+  if (!S.getDiagnostics().isIgnored(diag::warn_format_invalid_positional_specifier, SourceLocation())) {
+    EmitFormatDiagnostic(
+        S.PDiag(diag::warn_format_invalid_positional_specifier) << (unsigned)p,
+        getLocationOfByte(startSpecifier), /*IsStringLocation*/ true,
+        getSpecifierRange(startSpecifier, specifierLen));
+  }
 }
 
 void CheckFormatHandler::HandleZeroPosition(const char *startPos,
                                             unsigned posLen) {
-  EmitFormatDiagnostic(S.PDiag(diag::warn_format_zero_positional_specifier),
-                               getLocationOfByte(startPos),
-                               /*IsStringLocation*/true,
-                               getSpecifierRange(startPos, posLen));
+  if (!S.getDiagnostics().isIgnored(diag::warn_format_zero_positional_specifier, SourceLocation())) {
+    EmitFormatDiagnostic(S.PDiag(diag::warn_format_zero_positional_specifier),
+                                getLocationOfByte(startPos),
+                                /*IsStringLocation*/true,
+                                getSpecifierRange(startPos, posLen));
+  }
 }
 
 void CheckFormatHandler::HandleNullChar(const char *nullCharacter) {

@github-actions
Copy link

github-actions bot commented Dec 19, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

There are a few functions that emit warnings
related to positional arguments in format strings.
These functions use `getLocationOfByte()` which has O(n)
complexity and may lead to silent hang of compilation in some cases.
But such warnings is not widely used and actually don't emit
if user didn't pass the appropriate `-W...` flag, so
if the flag is not passed dont make the call
to `EmitFormatDiagnostic` for such diags.

Fix llvm#120462
Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not use braces for if statements with a single child statement
LGTM otherwise

Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@chestnykh chestnykh merged commit 99dddef into llvm:main Dec 23, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

It takes a very long time to compile a glibc test

3 participants