[clang] [clang] Function type attribute to prevent CFI instrumentation (PR #135836)

[Bruno De Fraine via cfe-commits]

================
@@ -9283,8 +9283,14 @@ static AssignConvertType checkPointerTypesForAssignment(Sema &S,
       return AssignConvertType::IncompatibleFunctionPointer;
     return AssignConvertType::IncompatiblePointer;
   }
-  if (!S.getLangOpts().CPlusPlus && S.IsFunctionConversion(ltrans, rtrans))
-    return AssignConvertType::IncompatibleFunctionPointer;
+  bool DiscardingCFIUncheckedCallee, AddingCFIUncheckedCallee;
+  if (!S.getLangOpts().CPlusPlus &&
+      S.IsFunctionConversion(ltrans, rtrans, &DiscardingCFIUncheckedCallee,
+                             &AddingCFIUncheckedCallee)) {
+    // Allow conversions between CFIUncheckedCallee-ness.
+    if (!DiscardingCFIUncheckedCallee && !AddingCFIUncheckedCallee)
+      return AssignConvertType::IncompatibleFunctionPointer;
+  }
----------------
brunodf-snps wrote:

Hi, I'm late here because we only spotted this while integrating clang-21 in our downstream tree.

I think there is a logic error here. If a function conversion is _also_ discarding or adding CFIUncheckedCallee status, it is suddenly no longer an incompatible function pointer? That does not make sense to me, and it stops diagnosing conversions that we should be diagnosing (and normally are diagnosing), see this example: https://godbolt.org/z/bnYsE63Gc

Granted, the original code has flawed logic, which I reported already back in 2019: https://bugs.llvm.org/show_bug.cgi?id=42120
In short, the code seems to use the _negative_ of the `IsFunctionConversion` condition in the _inverse_ direction of what you would expect. My bug report observes that this appears to work but breaks in more complex cases. I suggested a patch at the time, but this did not get picked up. I would consider submitting a PR, but I would like some clang maintainers to agree with my report and patch (or suggest some other fix), before doing so.

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