On Wed, Oct 22, 2025 at 12:33:18PM -0700, Andrew Pinski wrote: > On Wed, Oct 22, 2025 at 11:27 AM Kees Cook <[email protected]> wrote: > [...] > > @@ -11847,6 +11848,16 @@ aarch64_expand_call (rtx result, rtx mem, rtx > > cookie, bool sibcall) > > > > call = gen_rtx_CALL (VOIDmode, mem, const0_rtx); > > > > + /* Only indirect calls need KCFI instrumentation. */ > > + bool is_direct_call = SYMBOL_REF_P (XEXP (mem, 0)); > > + rtx kcfi_type_rtx = is_direct_call ? NULL_RTX > > + : kcfi_get_type_id_for_expanding_gimple_call (); > > I don't like kcfi_get_type_id_for_expanding_gimple_call call. > Does it make better sense to create a few new optabs for the kfci call > instead and pass this down instead of having this call?
Unless I'm misunderstanding how optabs work, I don't want to use that here. To use an optab, I think I'd need to create a separate "define_expand" RTL pattern for kcfi calls. I found this to be infeasible (I tried it somewhere back in around v2), as the calling conventions for most architectures are extraordinarily complex, and I'd have to duplicate all of that logic for kcfi expansion. Instead, I have KCFI just happen in late expansion, which seems the best fit. Just so I can understand better, why don't you like it? I assume it's the fact that we're basically in RTL and that function ends up reaching back up to GIMPLE? This seemed like a layering violation to me too, but I noticed that it's not uncommon for expansion code to use currently_expanding_gimple_stmt, so as a result it didn't end up seeming unreasonable to also use it for KCFI (it is, as it turns out, exactly what's needed at that moment in the expansion: "give me the kcfi typeid"). Obviously, I could be missing something here, so if you see a way to do this better, I am happy to do so. :) -Kees -- Kees Cook
