[PATCH dwarves] btf_encoder: Keep the source prototype of kfuncs

[Yonghong Song <yonghong.song@linux.dev>]

The location based checks can drop kfuncs from BTF even though their ABI
did not change. With the arm64 KCSAN setup in the report, resolve_btfids
fails with:

  WARN: resolve_btfids: no BTF func for kfunc bpf_tcp_ops_set_rcvlowat in bpf_tcp_ops_set_rcvlowat_kfunc_set
  WARN: resolve_btfids: unresolved symbol bpf_tcp_ops_set_rcvlowat

The location list of 'rcvlowat' starts after the function entry, in a
callee-saved register, so the function is marked unexpected_reg and
skipped. scx_bpf_tid_to_task() hits the same skip through a
different DWARF pattern.

Kfuncs are defined with __bpf_kfunc ("__used __retain __noclone
noinline"), so the compiler keeps the calling convention of the C
prototype. Odd parameter locations of a kfunc are debug info artifacts,
not ABI changes.

For kfuncs, clear the location derived flags when saving the state,
keep all source parameters, and skip the location based checks, also in
btf_encoder__add_saved_funcs() where saved_functions_combine() can merge
the flags back in. The inconsistent prototype and ambiguous address
checks still apply.

Kfuncs were only collected with decl_tag_kfuncs, which the kernel no
longer passes since commit 692393104909 ("resolve_btfids: Emit bpf_kfunc
and bpf_fastcall decl tags"). Collect them whenever BTF is encoded; the
decl tags are still only emitted with decl_tag_kfuncs.

With this change, bpf_tcp_ops_set_rcvlowat() and scx_bpf_tid_to_task()
are encoded again, and no other function in vmlinux BTF changes.

Reported-by: Breno Leitao <leitao@debian.org>
Closes: https://lore.kernel.org/all/au27Y9m78_3EH2v1@gmail.com/
Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
 btf_encoder.c | 49 +++++++++++++++++++++++++++++++++++--------------
 1 file changed, 35 insertions(+), 14 deletions(-)

diff --git a/btf_encoder.c b/btf_encoder.c
index 05ffe82..4269467 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -1421,16 +1421,30 @@ static int32_t btf_encoder__save_func(struct btf_encoder *encoder, struct functi
 			}
 		}
 	}
+
+	/*
+	 * A kfunc keeps the ABI of its C prototype, so reset the DWARF
+	 * location based flags: otherwise the kfunc may be treated as
+	 * optimized and skipped, or lose parameters in BTF.
+	 */
+	if (!loc && is_kfunc_state(state)) {
+		state->unexpected_reg = 0;
+		state->optimized_parms = 0;
+		state->uncertain_parm_loc = 0;
+		state->reordered_parm = 0;
+		state->signature_changed = 0;
+	}
+
 	ftype__for_each_parameter(ftype, param) {
 		const char *name;
 		char *final_name = NULL;
 
 		/* No location info/optimized + reordered means optimized out. */
-		if (!loc && ftype->reordered_parm && (!param->has_loc || param->optimized)) {
+		if (!loc && state->reordered_parm && (!param->has_loc || param->optimized)) {
 			state->nr_parms--;
 			continue;
 		}
-		if (!loc && encoder->true_signature && ftype->signature_changed &&
+		if (!loc && encoder->true_signature && state->signature_changed &&
 		    param->optimized) {
 			state->nr_parms--;
 			continue;
@@ -1797,17 +1811,20 @@ static int btf_encoder__add_saved_funcs(struct btf_encoder *encoder, bool skip_e
 		 * signature; true_signature can rewrite those signatures.
 		 * Also exclude functions with unexpected register use, multiple
 		 * inconsistent prototypes or uncertain parameter locations.
+		 * The location based checks do not apply to kfuncs.
 		 */
-		if (state->unexpected_reg)
-			skip_reason = "unexpected register usage for parameter\n";
+		if (!is_kfunc_state(state)) {
+			if (state->unexpected_reg)
+				skip_reason = "unexpected register usage for parameter\n";
+			if (state->uncertain_parm_loc)
+				skip_reason = "uncertain parameter location\n";
+			if (state->reordered_parm)
+				skip_reason = "reordered parameters\n";
+			if (btf_encoder_func_state__skip_optimized_parms(encoder, canonical_state))
+				skip_reason = "optimized parameters\n";
+		}
 		if (skip_encoding_inconsistent_proto && state->inconsistent_proto)
 			skip_reason = "inconsistet prototype\n";
-		if (state->uncertain_parm_loc)
-			skip_reason = "uncertain parameter location\n";
-		if (state->reordered_parm)
-			skip_reason = "reordered parameters\n";
-		if (btf_encoder_func_state__skip_optimized_parms(encoder, canonical_state))
-			skip_reason = "optimized parameters\n";
 		if (state->elf->ambiguous_addr)
 			skip_reason = "ambiguous address\n";
 
@@ -3056,10 +3073,14 @@ struct btf_encoder *btf_encoder__new(struct cu *cu, const char *detached_filenam
 		if (!found_percpu && encoder->verbose)
 			printf("%s: '%s' doesn't have '%s' section\n", __func__, cu->filename, PERCPU_SECTION);
 
-		if (encoder->tag_kfuncs) {
-			if (btf_encoder__collect_kfuncs(encoder))
-				goto out_delete;
-		}
+		/*
+		 * Mark kfuncs before any function state is saved, so
+		 * btf_encoder__save_func() keeps their source prototypes.
+		 * Needed even without decl_tag_kfuncs, which only controls
+		 * the decl tags.
+		 */
+		if (btf_encoder__collect_kfuncs(encoder))
+			goto out_delete;
 
 		if (encoder->verbose)
 			printf("File %s:\n", cu->filename);
-- 
2.53.0-Meta