Merged main:e069434afcd21911ad36c55971bb8f754854c09f into amd-gfx:2c74854f7595

Local branch amd-gfx 2c74854 Merged main:139688a699f6db784bd559b147334f1d51314f9c into amd-gfx:cf8fb1be1ea5
Remote branch main e069434 [rtsan][NFC] Remove unncessary namespace specifiers (llvm#110197)

Author: SC llvm team

clang-tools-extra/clang-tidy/bugprone/PosixReturnCheck.cpp

@@ -7,52 +7,58 @@
 //===----------------------------------------------------------------------===//
 
 #include "PosixReturnCheck.h"
-#include "../utils/Matchers.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Lex/Lexer.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang::tidy::bugprone {
 
-static StringRef getFunctionSpelling(const MatchFinder::MatchResult &Result,
-                                     const char *BindingStr) {
-  const CallExpr *MatchedCall = cast<CallExpr>(
-      (Result.Nodes.getNodeAs<BinaryOperator>(BindingStr))->getLHS());
+static StringRef getFunctionSpelling(const MatchFinder::MatchResult &Result) {
+  const auto *MatchedCall = Result.Nodes.getNodeAs<CallExpr>("call");
   const SourceManager &SM = *Result.SourceManager;
   return Lexer::getSourceText(CharSourceRange::getTokenRange(
                                   MatchedCall->getCallee()->getSourceRange()),
                               SM, Result.Context->getLangOpts());
 }
 
 void PosixReturnCheck::registerMatchers(MatchFinder *Finder) {
+  const auto PosixCall =
+      callExpr(callee(functionDecl(
+                   anyOf(matchesName("^::posix_"), matchesName("^::pthread_")),
+                   unless(hasName("::posix_openpt")))))
+          .bind("call");
+  const auto ZeroIntegerLiteral = integerLiteral(equals(0));
+  const auto NegIntegerLiteral =
+      unaryOperator(hasOperatorName("-"), hasUnaryOperand(integerLiteral()));
+
   Finder->addMatcher(
       binaryOperator(
-          hasOperatorName("<"),
-          hasLHS(callExpr(callee(functionDecl(
-              anyOf(matchesName("^::posix_"), matchesName("^::pthread_")),
-              unless(hasName("::posix_openpt")))))),
-          hasRHS(integerLiteral(equals(0))))
+          anyOf(allOf(hasOperatorName("<"), hasLHS(PosixCall),
+                      hasRHS(ZeroIntegerLiteral)),
+                allOf(hasOperatorName(">"), hasLHS(ZeroIntegerLiteral),
+                      hasRHS(PosixCall))))
           .bind("ltzop"),
       this);
   Finder->addMatcher(
       binaryOperator(
-          hasOperatorName(">="),
-          hasLHS(callExpr(callee(functionDecl(
-              anyOf(matchesName("^::posix_"), matchesName("^::pthread_")),
-              unless(hasName("::posix_openpt")))))),
-          hasRHS(integerLiteral(equals(0))))
+          anyOf(allOf(hasOperatorName(">="), hasLHS(PosixCall),
+                      hasRHS(ZeroIntegerLiteral)),
+                allOf(hasOperatorName("<="), hasLHS(ZeroIntegerLiteral),
+                      hasRHS(PosixCall))))
           .bind("atop"),
       this);
+  Finder->addMatcher(binaryOperator(hasAnyOperatorName("==", "!="),
+                                    hasOperands(PosixCall, NegIntegerLiteral))
+                         .bind("binop"),
+                     this);
   Finder->addMatcher(
-      binaryOperator(
-          hasAnyOperatorName("==", "!=", "<=", "<"),
-          hasLHS(callExpr(callee(functionDecl(
-              anyOf(matchesName("^::posix_"), matchesName("^::pthread_")),
-              unless(hasName("::posix_openpt")))))),
-          hasRHS(unaryOperator(hasOperatorName("-"),
-                               hasUnaryOperand(integerLiteral()))))
+      binaryOperator(anyOf(allOf(hasAnyOperatorName("<=", "<"),
+                                 hasLHS(PosixCall), hasRHS(NegIntegerLiteral)),
+                           allOf(hasAnyOperatorName(">", ">="),
+                                 hasLHS(NegIntegerLiteral), hasRHS(PosixCall))))
           .bind("binop"),
       this);
 }
@@ -61,23 +67,26 @@ void PosixReturnCheck::check(const MatchFinder::MatchResult &Result) {
   if (const auto *LessThanZeroOp =
           Result.Nodes.getNodeAs<BinaryOperator>("ltzop")) {
     SourceLocation OperatorLoc = LessThanZeroOp->getOperatorLoc();
+    StringRef NewBinOp =
+        LessThanZeroOp->getOpcode() == BinaryOperator::Opcode::BO_LT ? ">"
+                                                                     : "<";
     diag(OperatorLoc, "the comparison always evaluates to false because %0 "
                       "always returns non-negative values")
-        << getFunctionSpelling(Result, "ltzop")
-        << FixItHint::CreateReplacement(OperatorLoc, Twine(">").str());
+        << getFunctionSpelling(Result)
+        << FixItHint::CreateReplacement(OperatorLoc, NewBinOp);
     return;
   }
   if (const auto *AlwaysTrueOp =
           Result.Nodes.getNodeAs<BinaryOperator>("atop")) {
     diag(AlwaysTrueOp->getOperatorLoc(),
          "the comparison always evaluates to true because %0 always returns "
          "non-negative values")
-        << getFunctionSpelling(Result, "atop");
+        << getFunctionSpelling(Result);
     return;
   }
   const auto *BinOp = Result.Nodes.getNodeAs<BinaryOperator>("binop");
   diag(BinOp->getOperatorLoc(), "%0 only returns non-negative values")
-      << getFunctionSpelling(Result, "binop");
+      << getFunctionSpelling(Result);
 }
 
 } // namespace clang::tidy::bugprone

clang-tools-extra/docs/ReleaseNotes.rst

@@ -125,6 +125,10 @@ Changes in existing checks
   <clang-tidy/checks/bugprone/forwarding-reference-overload>` check by fixing
   a crash when determining if an ``enable_if[_t]`` was found.
 
+- Improved :doc:`bugprone-posix-return
+  <clang-tidy/checks/bugprone/posix-return>` check to support integer literals
+  as LHS and posix call as RHS of comparison.
+
 - Improved :doc:`bugprone-sizeof-expression
   <clang-tidy/checks/bugprone/sizeof-expression>` check to find suspicious
   usages of ``sizeof()``, ``alignof()``, and ``offsetof()`` when adding or

clang-tools-extra/test/clang-tidy/checkers/bugprone/posix-return.cpp

@@ -74,6 +74,9 @@ void warningLessThanZero() {
   if (pthread_yield() < 0) {}
   // CHECK-MESSAGES: :[[@LINE-1]]:23: warning:
   // CHECK-FIXES: pthread_yield() > 0
+  if (0 > pthread_yield() ) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning:
+  // CHECK-FIXES: 0 < pthread_yield()
 
 }
 
@@ -90,7 +93,8 @@ void warningAlwaysTrue() {
   // CHECK-MESSAGES: :[[@LINE-1]]:31: warning:
   if (pthread_yield() >= 0) {}
   // CHECK-MESSAGES: :[[@LINE-1]]:23: warning:
-
+  if (0 <= pthread_yield()) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning:
 }
 
 void warningEqualsNegative() {
@@ -120,7 +124,14 @@ void warningEqualsNegative() {
   // CHECK-MESSAGES: :[[@LINE-1]]:46: warning:
   if (pthread_create(NULL, NULL, NULL, NULL) < -1) {}
   // CHECK-MESSAGES: :[[@LINE-1]]:46: warning:
-
+  if (-1 == pthread_create(NULL, NULL, NULL, NULL)) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning:
+  if (-1 != pthread_create(NULL, NULL, NULL, NULL)) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning:
+  if (-1 >= pthread_create(NULL, NULL, NULL, NULL)) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning:
+  if (-1 > pthread_create(NULL, NULL, NULL, NULL)) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning:
 }
 
 void WarningWithMacro() {
@@ -162,6 +173,16 @@ void noWarning() {
   if (posix_openpt(0) < -1) {}
   if (posix_fadvise(0, 0, 0, 0) <= 0) {}
   if (posix_fadvise(0, 0, 0, 0) == 1) {}
+  if (0 > posix_openpt(0)) {}
+  if (0 >= posix_openpt(0)) {}
+  if (-1 == posix_openpt(0)) {}
+  if (-1 != posix_openpt(0)) {}
+  if (-1 >= posix_openpt(0)) {}
+  if (-1 > posix_openpt(0)) {}
+  if (posix_fadvise(0, 0, 0, 0) <= 0) {}
+  if (posix_fadvise(0, 0, 0, 0) == 1) {}
+  if (0 >= posix_fadvise(0, 0, 0, 0)) {}
+  if (1 == posix_fadvise(0, 0, 0, 0)) {}
 }
 
 namespace i {

clang/docs/ReleaseNotes.rst

@@ -81,6 +81,24 @@ C++ Specific Potentially Breaking Changes
     template <typename T>
     void f();
 
+- During constant evaluation, comparisons between different evaluations of the
+  same string literal are now correctly treated as non-constant, and comparisons
+  between string literals that cannot possibly overlap in memory are now treated
+  as constant. This updates Clang to match the anticipated direction of open core
+  issue `CWG2765 <http://wg21.link/CWG2765>`, but is subject to change once that
+  issue is resolved.
+
+  .. code-block:: c++
+
+    constexpr const char *f() { return "hello"; }
+    constexpr const char *g() { return "world"; }
+    // Used to evaluate to false, now error: non-constant comparison.
+    constexpr bool a = f() == f();
+    // Might evaluate to true or false, as before.
+    bool at_runtime() { return f() == f(); }
+    // Was error, now evaluates to false.
+    constexpr bool b = f() == g();
+
 ABI Changes in This Version
 ---------------------------
 

clang/include/clang/AST/ASTContext.h

@@ -324,6 +324,14 @@ class ASTContext : public RefCountedBase<ASTContext> {
   /// This is lazily created.  This is intentionally not serialized.
   mutable llvm::StringMap<StringLiteral *> StringLiteralCache;
 
+  /// The next string literal "version" to allocate during constant evaluation.
+  /// This is used to distinguish between repeated evaluations of the same
+  /// string literal.
+  ///
+  /// We don't need to serialize this because constants get re-evaluated in the
+  /// current file before they are compared locally.
+  unsigned NextStringLiteralVersion = 0;
+
   /// MD5 hash of CUID. It is calculated when first used and cached by this
   /// data member.
   mutable std::string CUIDHash;
@@ -3300,6 +3308,10 @@ class ASTContext : public RefCountedBase<ASTContext> {
   /// PredefinedExpr to cache evaluated results.
   StringLiteral *getPredefinedStringLiteralFromCache(StringRef Key) const;
 
+  /// Return the next version number to be used for a string literal evaluated
+  /// as part of constant evaluation.
+  unsigned getNextStringLiteralVersion() { return NextStringLiteralVersion++; }
+
   /// Return a declaration for the global GUID object representing the given
   /// GUID value.
   MSGuidDecl *getMSGuidDecl(MSGuidDeclParts Parts) const;

clang/include/clang/Basic/DiagnosticASTKinds.td

@@ -96,6 +96,9 @@ def note_constexpr_pointer_constant_comparison : Note<
   "at runtime">;
 def note_constexpr_literal_comparison : Note<
   "comparison of addresses of literals has unspecified value">;
+def note_constexpr_opaque_call_comparison : Note<
+  "comparison against opaque constant address '%0' can only be performed at "
+  "runtime">;
 def note_constexpr_pointer_weak_comparison : Note<
   "comparison against address of weak declaration '%0' can only be performed "
   "at runtime">;