Re: [PATCH v13 1/7] kcfi: Introduce KCFI typeinfo mangling API

On Sat, Jun 27, 2026 at 03:00:42PM -0700, Andrea Pinski wrote:
> On Thu, Jun 18, 2026 at 1:45 PM Kees Cook <[email protected]> wrote:
> > [...]
> > diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> > index 9f821f048533..2e7b7e4aed89 100644
> > --- a/gcc/doc/extend.texi
> > +++ b/gcc/doc/extend.texi
> > @@ -17967,6 +17967,101 @@ which will cause a @code{NULL} pointer to be used for the unsafe case.
> >
> >  @enddefbuiltin
> >
> > +@defbuiltin{{unsigned int} __builtin_linux_abi_kcfi_hash (@var{type})}
> > +
> > +The built-in function @code{__builtin_linux_abi_kcfi_hash} returns a hash value
> > +for the given type @var{type} (which is a type, not an expression).
> > +The hash is computed using the FNV-1a algorithm on the type's mangled
> > +name representation, which follows a subset of the Itanium C++ ABI
> > +conventions adapted for C types.  (See @code{__builtin_linux_abi_kcfi_name}
> > +for the string representation.)
> 
> I think we should add a link to the Itanium C++ ABI from the documentation.

Do you mean a literal URL?

> The documentation is missing that these builtins are only available
> from the C and Objective-C and not there for C++.
> Unless you are going to add a cp/cp-parser.cc implementation too.

Good point; I don't intend to add this to cp/cp-parser.cc unless there
is some extremely good reason, and if this is Linux-kernel specific, it
really should just be C.

> > [...]
> > +
> > +    printf("\n================================================================\n");
> > +    printf("Passed: %d Failed: %d (%d total tests)\n", pass, fail, pass + fail);
> > +    return fail;
> > +}
> 
> It would be a good idea to add a random generated testing here. See
> testsuite/objc.dg/gnu-encoding and testsuite/gcc.dg/compat for
> examples.

Okay, thanks for the pointer; I'll see what I can come up with. I
generally dislike random testing since they may appear to be "flaky"
when they trip, but I'll give it a shot.

> > [...]
> > --- /dev/null
> > +++ b/gcc/kcfi-typeinfo.h
> > @@ -0,0 +1,32 @@
> > +/* KCFI-compatible type mangling, based on Itanium C++ ABI.
> > +   Copyright (C) 2025 Free Software Foundation, Inc.
> 
> It is 2026 now.

Oops, yes.

> 
> > +
> > +This file is part of GCC.
> > +
> > +GCC is free software; you can redistribute it and/or modify it under
> > +the terms of the GNU General Public License as published by the Free
> > +Software Foundation; either version 3, or (at your option) any later
> > +version.
> > +
> > +GCC is distributed in the hope that it will be useful, but WITHOUT ANY
> > +WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > +FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
> > +for more details.
> > +
> > +You should have received a copy of the GNU General Public License
> > +along with GCC; see the file COPYING3.  If not see
> > +<http://www.gnu.org/licenses/>.  */
> > +
> > +#ifndef GCC_KCFI_TYPEINFO_H
> > +#define GCC_KCFI_TYPEINFO_H
> > +
> > +#include "tree.h"
> > +#include <string>
> 
> Don't include string here. It will break on some host.

Ah! Okay. Do you know which combo trips this? I'd love to be able to
validate I've fixed this correctly.

> > [...]
> > +#include "config.h"
> Add `#define INCLUDE_STRING` here.

> > [...]
> > +/* Forward declaration for recursive type mangling.  */
> > +
> > +static void mangle_type (tree type, std::string *out_str, uint32_t *hash_state);
> 
> Forward declarations should be close to the top of the file.

Gotcha.

> > [...]
> > +  /* Unknown builtin type: this should never happen in a well-formed C.  */
> > +  debug_tree (type);
> > +  internal_error ("mangle: Unknown builtin type - please report this as a bug");
> Maybe fatal_error or sorry. I know we had some disagreement about this
> but I can't remember the out come.
> I am not sure we want internal_error exactly to be user friendly.

We've gone back and forth a few times in other places. My take on this
is that I do want it to be friendly because in at least the place I
tripped over this, it was very confusing for me: first I saw this for
vector types and later for builtins. So, at the very least, I'd like to
keep this as-is because of how hard I found it to be to debug when
something got missed. :P

> Note I also have not doubled check but do you have some testcases
> testing VLAs inside a struct here?
> What about _BitInt testcases?

I can add both; Linux uses neither so I hadn't considered/tripped over
those.

-- 
Kees Cook