================
@@ -6350,13 +6351,39 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo
&CallInfo,
else if (const auto *FPT =
Callee.getAbstractInfo().getCalleeFunctionProtoType())
CST = QualType(FPT, 0);
+ else if (const auto *FT =
+ Callee.getAbstractInfo().getCalleeFunctionType())
+ CST = QualType(FT, 0);
else
llvm_unreachable(
"Cannot find the callee type to generate callee_type metadata.");
// Set type identifier metadata of indirect calls for call graph section.
- if (!CST.isNull())
+ if (!CST.isNull()) {
+ if (!CST->isFunctionProtoType()) {
+ // For unprototyped callees, reconstruct a prototype from the
argument
+ // types passed at the call site (after default argument promotion).
----------------
frobtech wrote:
I think it merits some more detailed comments here about the reasoning, and a
mention of possible alternatives / complications.
In particular, the basic rationale of using the types after default argument
promotions is that these are the types that the actual callee's definition must
be using in its prototype-style definition for the call to be thoroughly valid.
However, some further wrinkles are worth noting.
Firstly, if the actual definition is K&R-style, then (_I think_) the canonical
semantics is that the K&R-declared type of the parameter is actually promoted
for the expected type used in the call, and then the callee is implicitly
casting to the declared type. So e.g. `void foo(x) short x; { ... }` actually
expects to be called with an `int` even though the `x` parameter variable
inside the function will be `short`. So by that logic, at a K&R definition
site, the "what is my own signature" computation there ought to actually be
`void(int)` rather than `void(short)` as the same function declared as a
prototype would yield.
The second general class of wrinkles is about the strictness of intent in the
source that you want to hold for your signature matching. We don't have many
options: we're comparing a hash, so the only matching is exact equality. Thus
the only choices to make are in normalization before hashing.
If you decide your program is only "correct" if caller and callee used
precisely the same types, you can be very strict: basically no normalization.
It seems wise to apply default argument promotions at least, since deeming a
caller "incorrect" for passing `(short)x + 1` for a parameter declared `short`
(where the expression is promoted to `int` regardless--and in a
prototype-declared call would technically be re-narrowed in the call!) seems a
bridge too far.
But what beyond default argument promotions? If you want to consider any call
that's technically valid in standard C, then each argument only has to be
[compatible](https://cppreference.com/c/language/type#Compatible_types) and
even gets a few exceptions from that such as for signedness.
With `-fsanitize-cfi-icall-experimental-normalize-integers`, there is
normalization for different integer types with entirely the same ABI (same
bit-width, same signedness). AFAICT that's actually not "compatible" by the C
standard, e.g. `int` vs `long` or `long` vs `long long` for a pair that's the
same actual size.
But that doesn't normalize away signedness, while a mismatch of signedness is
actually an exception beyond "compatible" that's valid in standard C for calls
to functions declared without prototypes (or for a call to a variadic function
vs the type passed to `va_arg`). So if you have a callee that takes an
`unsigned int` and you want callers that say `0` to be considered in its
equivalence set for possible callees, you need to normalize away signedness.
For CFI, it seems very plausible that you wouldn't want to loosen that way and
consider such a callee valid for that caller. But for call graph analysis
perhaps you'd rather err on the side of considering a callee possible. Do we
need a separate switch to decide about that normalization? Does it need to be
separate for CFI vs call-graph?
OTOH, "compatible" admits e.g. any `enum` type vs its underlying type. So do
you want to normalize away to basically all integer types of a given bit-width
being the same, whether signed or not, `enum` or not? Again, it depends on
whether you'd rather err on the side of considering every callee that C rules
actually make it not UB to use for a given function pointer, or err on the side
of excluding things it seems like code that's especially clean would definitely
avoid.
The most fundamental thing to make sure is well-noted in this code is that the
normalization done here (even if just default promotions) must stay mated with
the normalization logic done to choose a self-signature at a K&R-style
definition site.
https://github.com/llvm/llvm-project/pull/210846
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
