================
@@ -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
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
