On Thu, Jul 23, 2026 at 06:28:16PM -0700, Andrea Pinski wrote:
> On Wed, Jul 15, 2026 at 4:32 PM Kees Cook <[email protected]> wrote:
> >
> > On Wed, Jul 15, 2026 at 03:34:09PM -0700, Kees Cook wrote:
> > > On Sat, Jun 27, 2026 at 03:57:36PM -0700, Andrea Pinski wrote:
> > > > On Thu, Jun 18, 2026 at 1:45 PM Kees Cook <[email protected]> wrote:
> > > > > [...]
> > > > > +/* KCFI label counter, incremented by KCFI insn emission. */
> > > > > +static int kcfi_labelno = 0;
> > > >
> > > > I am not 100% sure if this needs a GTY marker or not. I suspect no
> > > > because we should not have emitted assembly code yet.
> > >
> > > If this moves with kcfi_next_labelno into final, I think it's okay
> > > without GTY?
> >
> > This is an int, so GC shouldn't be an issue.
>
> GTY is used for PCH also and not just GC. This is why I question the
> need for the GTY marker. A GTY marker on an int is to make sure that
> it is restored from the PCH.
> Maybe you can add a couple of PCH testcases to make sure it is working
> correctly.
Ah, gotcha. Yeah, this doesn't appear to be an issue for PCH since it's
used during output only. Regardless, I've added pch tests now as well.
> > > > > + type_id = (uint32_t) TREE_INT_CST_LOW (value);
> > > > > + }
> > > > > + else
> > > > > + {
> > > > > + type_id = compute_kcfi_type_id (fn_type);
> > > > > +
> > > > > + tree type_id_tree = build_int_cst (unsigned_type_node,
> > > > > type_id);
> > > > > + tree attr = build_tree_list (kcfi_type_id_attr, type_id_tree);
> > > > > +
> > > > > + TYPE_ATTRIBUTES (fn_type) = chainon (TYPE_ATTRIBUTES
> > > > > (fn_type), attr);
> > > > > + }
> > > >
> > > > Instead of an attribute there must be a better way of doing this.
> > > > Maybe a hashset instead.
> > >
> > > Perhaps? I will go examine this vs LTO, etc.
> >
> > Tracking this with lifetime tied to the fndecl is going to be more pain
> > from what I can find. The attribute is stable and doesn't cause problems
> > for LTO: I've tested with 2 TUs, and this all appears to happen
> > post-merge? Anyway, if there is something I've missed here, I'm happy to
> > find a new solution, but I can't induce any problems so far.
>
> So maybe we add a field for FUNCTION_TYPE for this instead of an
> attribute. But that requires extra code for streaming the LTO and
> such. But it will reduce the overall overhead in general.
Okay, I've replaced the attribute with a hashset, which you'd suggested
before. This keeps the mapping entirely within kcfi.cc, and doesn't
bloat the FUNCTION_TYPE object with a new field that would only be used
for kcfi.
I'll get v15 sent shortly. :) Thanks!
-Kees
--
Kees Cook
