A few things stand out.
### 1. `indirect_return` attribute handling looks wrong/incomplete
You register it as:
```c
{"indirect_return", 0, 0, false, true, true, true, NULL, NULL},
```
and then check it via:
```c
tree fntype = TREE_TYPE (decl);
lookup_attribute ("indirect_return", TYPE_ATTRIBUTES (fntype))
```
That means this is treated as a **type** attribute, not a decl
attribute. That may be OK if intentional, but then:
- is that really the desired user model?
- does it propagate correctly through function pointer types,
redeclarations, builtins, etc?
- should calls through function pointers also be covered?
On 6/9/2026 9:55 PM, Monk Chiang wrote:
> Add call-site LPAD insertion for two cases:
>
> 1. setjmp / __attribute__((returns_twice)) calls, which may return a
> second time via longjmp.
> 2. A new "indirect_return" attribute for functions that may return to
> an unexpected address.
>
> Detection uses riscv_call_needs_lpad_p() at expand time. When needed,
> call_internal_cfi / call_value_internal_cfi emit .p2align 2,
> .option push/norelax/norvc, the call, .option pop, and lpad 0 as a
> single insn. This prevents the assembler (c.jal) or linker (jal
> relaxation) from shifting the return address off the lpad.
>
> gcc/ChangeLog:
>
> * config/riscv/riscv-protos.h (riscv_call_needs_lpad_p): Declare.
> * config/riscv/riscv.cc (riscv_gnu_attributes): Register new
> indirect_return attribute for function types.
> (riscv_call_needs_lpad_p): New function.
> * config/riscv/riscv.md (call_internal_cfi): New insn pattern.
> (call_value_internal_cfi): Likewise for call-with-return-value.
> (define_expand "call"): Emit call_internal_cfi when
> riscv_call_needs_lpad_p returns true.
> (define_expand "call_value"): Likewise.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/riscv/zicfilp-indirect-return.c: New test.
> * gcc.target/riscv/zicfilp-setjmp.c: New test.
>
>
> diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
> index 2055e5c4a9d..cb0153ab0af 100644
> --- a/gcc/config/riscv/riscv.md
> +++ b/gcc/config/riscv/riscv.md
>
> @@ -4206,6 +4210,36 @@
> call\t%0@plt"
> [(set_attr "type" "call")])
>
> +;; Zicfilp-protected call: .option push/pop guards prevent c.jal compression
> +;; and jal linker relaxation from moving the return address off the lpad.
> +;; .p2align 2 ensures the lpad is 4-byte aligned.
> +(define_insn "call_internal_cfi"
> + [(call (mem:SI (match_operand 0 "call_insn_operand" "l,S,U"))
> + (match_operand 1 "" ""))
> + (clobber (reg:SI RETURN_ADDR_REGNUM))]
> + "TARGET_ZICFILP"
> + {
> + output_asm_insn (".p2align\t2", operands);
> + output_asm_insn (".option push", operands);
> + output_asm_insn (".option norelax", operands);
> + output_asm_insn (".option norvc", operands);
> + switch (which_alternative)
> + {
> + case 0:
> + output_asm_insn ("jalr\t%0", operands);
> + break;
> + case 1:
> + output_asm_insn ("call\t%0", operands);
> + break;
> + default:
> + output_asm_insn ("call\t%0@plt", operands);
> + break;
> + }
> + output_asm_insn (".option pop", operands);
> + return "lpad\t0";
> + }
> + [(set_attr "type" "call")])
Given this outputs multiple instructions I think you need an attribute
specifying the length (in bytes) of the sequence.
Thanks for putting a comment on this stuff; one of the things that I've
found problematical with the kcfi work from Kees & friends is they
haven't ever explained why they need the call sequence to be output as
an atomic blob. Your comment makes it clear.
>
> @@ -4230,6 +4269,34 @@
> call\t%1@plt"
> [(set_attr "type" "call")])
>
> +(define_insn "call_value_internal_cfi"
> + [(set (match_operand 0 "" "")
> + (call (mem:SI (match_operand 1 "call_insn_operand" "l,S,U"))
> + (match_operand 2 "" "")))
> + (clobber (reg:SI RETURN_ADDR_REGNUM))]
> + "TARGET_ZICFILP"
You should likely add the same comment to this define_insn. This also
needs a length attribute.
At a high level, if we have a pointer to a function and the signature
includes the returns-twice type attribute, it would seem like it won't
be handled properly. Of course there's probably other things that break
if we call a returns-twice function indirectly. Mostly I wanted to
confirm that you didn't really expect/want to cover that scenario?
Generally OK. Mostly want to get the length attributes added, comment
copied and probably just a "yea, not handling indirect calls to returns
twice functions" and we're good to go.
jeff
