[Mlir-commits] [mlir] [mlir] Fix UB in comparator lambdas in getRegisteredOperationsByDialect (PR #186428)

[llvmlistbot at llvm.org]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir-core

Author: Mehdi Amini (joker-eph)

<details>
<summary>Changes</summary>

The binary search comparators used in `getRegisteredOperationsByDialect` and `RegisteredOperationName::insert` were returning the result of `StringRef::compare()` directly (which returns `int`) instead of a boolean, violating the strict weak ordering requirement of `std::lower_bound`, `std::upper_bound`, and `llvm::lower_bound`/`llvm::upper_bound`.

Since `compare()` returns a negative value for less-than, zero for equal, and a positive value for greater-than, a positive return value (lhs > rhs) was being treated as `true` by the algorithms — incorrectly indicating that lhs < rhs. This could cause binary searches to skip over valid entries, resulting in `getRegisteredOperationsByDialect` returning an empty or incomplete array for a dialect that does have registered operations.

Fix by adding `< 0` to each comparator to return a proper boolean.

Fixes #<!-- -->146940

Assisted-by: Claude Code

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


1 Files Affected:

- (modified) mlir/lib/IR/MLIRContext.cpp (+7-7) 


``````````diff
diff --git a/mlir/lib/IR/MLIRContext.cpp b/mlir/lib/IR/MLIRContext.cpp
index 73219c6917061..8f5f3b3a572e1 100644
--- a/mlir/lib/IR/MLIRContext.cpp
+++ b/mlir/lib/IR/MLIRContext.cpp
@@ -711,18 +711,18 @@ ArrayRef<RegisteredOperationName>
 MLIRContext::getRegisteredOperationsByDialect(StringRef dialectName) {
   auto *lowerBound = llvm::lower_bound(
       impl->sortedRegisteredOperations, dialectName, [](auto &lhs, auto &rhs) {
-        return lhs.getDialect().getNamespace().compare(rhs);
+        return lhs.getDialect().getNamespace().compare(rhs) < 0;
       });
 
   if (lowerBound == impl->sortedRegisteredOperations.end() ||
       lowerBound->getDialect().getNamespace() != dialectName)
     return ArrayRef<RegisteredOperationName>();
 
-  auto *upperBound =
-      std::upper_bound(lowerBound, impl->sortedRegisteredOperations.end(),
-                       dialectName, [](auto &lhs, auto &rhs) {
-                         return lhs.compare(rhs.getDialect().getNamespace());
-                       });
+  auto *upperBound = std::upper_bound(
+      lowerBound, impl->sortedRegisteredOperations.end(), dialectName,
+      [](auto &lhs, auto &rhs) {
+        return lhs.compare(rhs.getDialect().getNamespace()) < 0;
+      });
 
   size_t count = std::distance(lowerBound, upperBound);
   return ArrayRef(&*lowerBound, count);
@@ -999,7 +999,7 @@ void RegisteredOperationName::insert(
       llvm::upper_bound(ctxImpl.sortedRegisteredOperations, value,
                         [](auto &lhs, auto &rhs) {
                           return lhs.getIdentifier().compare(
-                              rhs.getIdentifier());
+                                     rhs.getIdentifier()) < 0;
                         }),
       value);
 }

``````````

</details>


https://github.com/llvm/llvm-project/pull/186428