Skip to content

Commit e07aef9

Browse files
authored
[clang][Sema] close IsStandardConversion hole when adding cfi_unchecked_callee (#164592)
Commit b194cf1 changed this function for the case where attribute `cfi_unchecked_callee` is added in a function conversion. But this introduces a hole (issue #162798), and it seems the change was unnecessary: the preceding `TryFunctionConversion` will already allow adding the `cfi_unchecked_callee` attribute, and will update `FromType` if it succeeds. So we revert the changes to `IsStandardConversion`. We also remove the helper function `AddingCFIUncheckedCallee` which is no longer needed, and simplify the corresponding `DiscardingCFIUncheckedCallee`. Fixes: #162798
1 parent b6e6a4d commit e07aef9

File tree

4 files changed

+10
-37
lines changed

4 files changed

+10
-37
lines changed

clang/include/clang/Sema/Sema.h

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2681,11 +2681,6 @@ class Sema final : public SemaBase {
26812681
/// function without this attribute.
26822682
bool DiscardingCFIUncheckedCallee(QualType From, QualType To) const;
26832683

2684-
/// Returns true if `From` is a function or pointer to a function without the
2685-
/// `cfi_unchecked_callee` attribute but `To` is a function or pointer to
2686-
/// function with this attribute.
2687-
bool AddingCFIUncheckedCallee(QualType From, QualType To) const;
2688-
26892684
/// This function calls Action when it determines that E designates a
26902685
/// misaligned member due to the packed attribute. This is used to emit
26912686
/// local diagnostics like in reference binding.

clang/lib/Sema/SemaChecking.cpp

Lines changed: 5 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -12373,14 +12373,9 @@ static void DiagnoseMixedUnicodeImplicitConversion(Sema &S, const Type *Source,
1237312373
}
1237412374
}
1237512375

12376-
enum CFIUncheckedCalleeChange {
12377-
None,
12378-
Adding,
12379-
Discarding,
12380-
};
12381-
12382-
static CFIUncheckedCalleeChange AdjustingCFIUncheckedCallee(QualType From,
12383-
QualType To) {
12376+
bool Sema::DiscardingCFIUncheckedCallee(QualType From, QualType To) const {
12377+
From = Context.getCanonicalType(From);
12378+
To = Context.getCanonicalType(To);
1238412379
QualType MaybePointee = From->getPointeeType();
1238512380
if (!MaybePointee.isNull() && MaybePointee->getAs<FunctionType>())
1238612381
From = MaybePointee;
@@ -12392,25 +12387,10 @@ static CFIUncheckedCalleeChange AdjustingCFIUncheckedCallee(QualType From,
1239212387
if (const auto *ToFn = To->getAs<FunctionType>()) {
1239312388
if (FromFn->getCFIUncheckedCalleeAttr() &&
1239412389
!ToFn->getCFIUncheckedCalleeAttr())
12395-
return Discarding;
12396-
if (!FromFn->getCFIUncheckedCalleeAttr() &&
12397-
ToFn->getCFIUncheckedCalleeAttr())
12398-
return Adding;
12390+
return true;
1239912391
}
1240012392
}
12401-
return None;
12402-
}
12403-
12404-
bool Sema::DiscardingCFIUncheckedCallee(QualType From, QualType To) const {
12405-
From = Context.getCanonicalType(From);
12406-
To = Context.getCanonicalType(To);
12407-
return ::AdjustingCFIUncheckedCallee(From, To) == Discarding;
12408-
}
12409-
12410-
bool Sema::AddingCFIUncheckedCallee(QualType From, QualType To) const {
12411-
From = Context.getCanonicalType(From);
12412-
To = Context.getCanonicalType(To);
12413-
return ::AdjustingCFIUncheckedCallee(From, To) == Adding;
12393+
return false;
1241412394
}
1241512395

1241612396
void Sema::CheckImplicitConversion(Expr *E, QualType T, SourceLocation CC,

clang/lib/Sema/SemaOverload.cpp

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2532,15 +2532,12 @@ static bool IsStandardConversion(Sema &S, Expr* From, QualType ToType,
25322532

25332533
SCS.setToType(2, FromType);
25342534

2535-
// If we have not converted the argument type to the parameter type,
2536-
// this is a bad conversion sequence, unless we're resolving an overload in C.
2537-
//
2538-
// Permit conversions from a function without `cfi_unchecked_callee` to a
2539-
// function with `cfi_unchecked_callee`.
2540-
if (CanonFrom == CanonTo || S.AddingCFIUncheckedCallee(CanonFrom, CanonTo))
2535+
if (CanonFrom == CanonTo)
25412536
return true;
25422537

2543-
if ((S.getLangOpts().CPlusPlus || !InOverloadResolution))
2538+
// If we have not converted the argument type to the parameter type,
2539+
// this is a bad conversion sequence, unless we're resolving an overload in C.
2540+
if (S.getLangOpts().CPlusPlus || !InOverloadResolution)
25442541
return false;
25452542

25462543
ExprResult ER = ExprResult{From};

clang/test/Frontend/cfi-unchecked-callee-attribute.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ void (*checked_ptr)(void) = unchecked; // expected-warning{{implicit conversion
99
void (CFI_UNCHECKED_CALLEE *unchecked_ptr)(void) = unchecked;
1010
void (CFI_UNCHECKED_CALLEE *from_normal)(void) = checked;
1111
void (CFI_UNCHECKED_CALLEE *c_no_function_decay)(void) = &unchecked;
12+
void (CFI_UNCHECKED_CALLEE __attribute__((noreturn)) *other_conflict)(void) = &checked; // expected-error{{cannot initialize a variable of type 'void (*)() __attribute__((noreturn)) __attribute__((cfi_unchecked_callee))' with an rvalue of type 'void (*)()'}}
1213
void (CFI_UNCHECKED_CALLEE *arr[10])(void);
1314
void (*cfi_elem)(void) = arr[1]; // expected-warning{{implicit conversion from 'void (*)() __attribute__((cfi_unchecked_callee))' to 'void (*)()' discards 'cfi_unchecked_callee' attribute}}
1415
void (CFI_UNCHECKED_CALLEE *cfi_unchecked_elem)(void) = arr[1];

0 commit comments

Comments
 (0)