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.
>
> > > > + 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.
>
> > > > [...]
> > > > + /* Emit .type directive. */
> > > > + ASM_OUTPUT_TYPE_DIRECTIVE (asm_file, cfi_symbol_name.c_str (),
> > > > "function");
> > > > + ASM_OUTPUT_LABEL (asm_file, cfi_symbol_name.c_str ());
> > > > +
> > > > + /* Emit any needed alignment padding NOPs using target's NOP
> > > > template. */
> > > > + for (int i = 0; i < kcfi_alignment_padding_nops; i++)
> > > > + output_asm_insn (kcfi_nop, NULL);
> > > I think you need to calculate kcfi_nop each time you call this
> > > function. Instead of caching it. For GC reasons.
> >
> > Uh, do I have to? This seems wasteful: the nop is generated frequently.
> > Should kcfi_nop gain GTY instead? I will take a closer look at this.
>
> So, while I could go full GTY, I opted for xstrdup instead. I've
> rearranged things here now, and indeed, LTO created some issues.
> Upstream Linux doesn't have GCC LTO support (there are trees but no one
> has been pushing for it), so I hadn't tested this area at all. I've
> added some limited LTO testing now.
I am less worried about LTO and more worried about PCH when it comes
GTY markers really.
>
> -Kees
>
> --
> Kees Cook
