On 9/16/25 12:04 AM, Kees Cook wrote:
+/* Apply KCFI wrapping to call pattern if needed. */
+rtx
+riscv_maybe_wrap_call_with_kcfi (rtx pat, rtx addr)
So our coding standards require a bit more for that function comment. What
are PAT and ADDR and how are they used?
Sure, I can document those more fully in the next version.
Thanks.
+riscv_output_kcfi_insn (rtx_insn *insn, rtx *operands)
+{
+ /* Target register. */
+ rtx target_reg = operands[0];
+ gcc_assert (REG_P (target_reg));
+
+ /* Get KCFI type ID. */
+ uint32_t expected_type = (uint32_t) INTVAL (operands[3]);
Do we know operands[3] is a CONST_INT?
Yes, these all come from their respective RTL's:
(match_operand 3 "const_int_operand"))
Perfect. Just wanted to make sure. It's a fairly common goof to try to
extract an integer value from a non-integer node.
ISTM that this will need some kidn of adjustment if someone were to compile
with -ffixed-reg. Maybe all we really need is a sorry() so that if someone
were to try to fix the temporary registers they'd get a loud complaint from
teh compiler.
Yeah, this needs more careful management of the scratch registers. I
have not been able to find a sane way to provide working constraints to
the RTL patterns, but I'd _much_ rather let the register allocator do
all this work.
Usually you end up having to define a register class with the single
register you want. Of course once you do that you also need to start
defining union classes and you also have to audit all kinds of code to
make sure it's doing something sensible. Yea, it's painful.
+
+ /* Load actual type from memory at offset. */
+ temp_operands[0] = gen_rtx_REG (SImode, temp1_regnum);
+ temp_operands[1] = gen_rtx_MEM (SImode,
+ gen_rtx_PLUS (DImode, target_reg,
+ GEN_INT (offset)));
+ output_asm_insn ("lw\t%0, %1", temp_operands);
Rather than using DImode for the PLUS, shouldn't it instead use Pmode so
that it at least tries to work on rv32? Or is this stuff somehow defined as
only working for rv64?
It was designed entirely for rv64. I'm not against making it work with
rv32, but I just haven't tried or tested it there.
ACK. This may never end up being used on rv32. But we should at least
fix the obvious stuff since it's just the right thing to do.
Conceptually any pointer should be using Pmode. If you keep that in
mind, that covers one big blob of issues. It also means that if someone
where to try to light up 32 bit pointers on rv64 that your code is ready
for that (and yes, we've had those kinds of requests, though to date
none of that code has been ready to integrate).
We generally prefer to not generate assembly code like you've done, but
instead prefer to generate actual RTL. Is there some reason why you decided
to use output_asm_insn rather than generating RTL and letting usual
mechanisms for generating assembly code kick in?
Yeah, I covered this a bit in patch #2 in the series which describe the
design requirements. The main issue is that the typeid validation check
cannot be separated from the call, and the instruction pattern needs to
have very close control over the register usage so we don't introduce
any new indirect call gadgets (pop %target, call %target).
So, this is a replacement of the regular CALL rtl pattern. I am totally
open to any other way to do this. I have been bumbling around in here
(and on the other architectures) trying to find ways to make it all
work, but it still feels like a bit of a hack. :)
I didn't really see anything in patch#2 which would indicate we want to
generate blobs of assembly code. It feels like there's something
missing in both our understandings.
Okay, noted. Are there any restrictions on function pointer alignment?
Regardless, I should probably rewrite this language a bit to try to
better say "we don't care about alignment padding since the preamble
typeid contents are a multiple of instruction size" (which would still
be true for 2 byte alignemnt).
Nope. The architecture will require them to be 2 byte aligned if "C" is
enabled or 4 byte aligned if "C" is not enabled. In both cases those
alignments correspond to the minimum instruction size.
Jeff
