[mlir] Fix possible null dereference during error logging in EmitC

[apach301]

Fixes #157452

[llvmbot]

@llvm/pr-subscribers-mlir-emitc

Author: Daniel Kuts (apach301)

Changes

Fixes #157452

---

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

1 Files Affected:

• (modified) mlir/lib/Dialect/EmitC/IR/EmitC.cpp (+5-1)

diff --git a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
index a73470cdf76c5..6b88e2c525d07 100644
--- a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
+++ b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
@@ -1320,7 +1320,11 @@ GetGlobalOp::verifySymbolUses(SymbolTableCollection &symbolTable) {
 
   // global has non-array type
   auto lvalueType = dyn_cast<LValueType>(resultType);
-  if (!lvalueType || lvalueType.getValueType() != globalType)
+  if (!lvalueType)
+    return emitOpError("on non-array type result type is not defined "
+                       "for the global @")
+           << getName();
+  if (lvalueType.getValueType() != globalType)
     return emitOpError("on non-array type expects result inner type ")
            << lvalueType.getValueType() << " to match type " << globalType
            << " of the global @" << getName();

[llvmbot]

@llvm/pr-subscribers-mlir

Author: Daniel Kuts (apach301)

Changes

Fixes #157452

---

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

1 Files Affected:

• (modified) mlir/lib/Dialect/EmitC/IR/EmitC.cpp (+5-1)

diff --git a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
index a73470cdf76c5..6b88e2c525d07 100644
--- a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
+++ b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
@@ -1320,7 +1320,11 @@ GetGlobalOp::verifySymbolUses(SymbolTableCollection &symbolTable) {
 
   // global has non-array type
   auto lvalueType = dyn_cast<LValueType>(resultType);
-  if (!lvalueType || lvalueType.getValueType() != globalType)
+  if (!lvalueType)
+    return emitOpError("on non-array type result type is not defined "
+                       "for the global @")
+           << getName();
+  if (lvalueType.getValueType() != globalType)
     return emitOpError("on non-array type expects result inner type ")
            << lvalueType.getValueType() << " to match type " << globalType
            << " of the global @" << getName();

[CoTinker]

Could you please add a test?

[simon-camp]

Thanks you for the fix.

Can you rephrase the Message to something like "... expects result type to be an lvalue type for the global ...". I think that gives a better hint on what is expected for users not familiär with the details of the Operation.

[apach301]

Could you please add a test?

That was a static analyzer report, so I don't know how to run this code or have any input data. But the problem is that code is syntactically inconsistent: there is a check that ensures lvalueType is non-null and is of correct type. And if that check is failed - it tries to dereference lvalueType.

[apach301]

Can you rephrase the Message to something like "... expects result type to be an lvalue type for the global ...". I think that gives a better hint on what is expected for users not familiär with the details of the Operation.

Done