|
| 1 | +From 73919b285fab9d52983a4585de60104e05dddb30 Mon Sep 17 00:00:00 2001 |
| 2 | +In-Reply-To: <20250502070318.1561924-1-tony.ambardar@gmail.com> |
| 3 | +References: <20250502070318.1561924-1-tony.ambardar@gmail.com> |
| 4 | +From: Tony Ambardar <tony.ambardar@gmail.com> |
| 5 | +Date: Tue, 20 May 2025 19:13:54 -0700 |
| 6 | +Subject: [PATCH dwarves v3] dwarf_loader: Fix skipped encoding of function BTF |
| 7 | + on 32-bit systems |
| 8 | +MIME-Version: 1.0 |
| 9 | +Content-Type: text/plain; charset=UTF-8 |
| 10 | +Content-Transfer-Encoding: 8bit |
| 11 | +To: dwarves@vger.kernel.org, |
| 12 | + bpf@vger.kernel.org |
| 13 | +Cc: Alan Maguire <alan.maguire@oracle.com>, |
| 14 | + Arnaldo Carvalho de Melo <acme@kernel.org>, |
| 15 | + Andrii Nakryiko <andrii@kernel.org>, |
| 16 | + Alexei Starovoitov <ast@kernel.org>, |
| 17 | + Daniel Borkmann <daniel@iogearbox.net> |
| 18 | + |
| 19 | +I encountered an issue building BTF kernels for 32-bit armhf, where many |
| 20 | +functions are missing in BTF data: |
| 21 | + |
| 22 | + LD vmlinux |
| 23 | + BTFIDS vmlinux |
| 24 | +WARN: resolve_btfids: unresolved symbol vfs_truncate |
| 25 | +WARN: resolve_btfids: unresolved symbol vfs_fallocate |
| 26 | +WARN: resolve_btfids: unresolved symbol scx_bpf_select_cpu_dfl |
| 27 | +WARN: resolve_btfids: unresolved symbol scx_bpf_pick_idle_cpu_node |
| 28 | +WARN: resolve_btfids: unresolved symbol scx_bpf_pick_idle_cpu |
| 29 | +WARN: resolve_btfids: unresolved symbol scx_bpf_pick_any_cpu_node |
| 30 | +WARN: resolve_btfids: unresolved symbol scx_bpf_pick_any_cpu |
| 31 | +WARN: resolve_btfids: unresolved symbol scx_bpf_kick_cpu |
| 32 | +WARN: resolve_btfids: unresolved symbol scx_bpf_exit_bstr |
| 33 | +WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_nr_queued |
| 34 | +WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_move_vtime |
| 35 | +WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_move_to_local |
| 36 | +WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_move |
| 37 | +WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_insert_vtime |
| 38 | +WARN: resolve_btfids: unresolved symbol scx_bpf_dsq_insert |
| 39 | +WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_vtime_from_dsq |
| 40 | +WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_vtime |
| 41 | +WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_from_dsq_set_vtime |
| 42 | +WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_from_dsq_set_slice |
| 43 | +WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch_from_dsq |
| 44 | +WARN: resolve_btfids: unresolved symbol scx_bpf_dispatch |
| 45 | +WARN: resolve_btfids: unresolved symbol scx_bpf_destroy_dsq |
| 46 | +WARN: resolve_btfids: unresolved symbol scx_bpf_create_dsq |
| 47 | +WARN: resolve_btfids: unresolved symbol scx_bpf_consume |
| 48 | +WARN: resolve_btfids: unresolved symbol bpf_throw |
| 49 | +WARN: resolve_btfids: unresolved symbol bpf_sock_ops_enable_tx_tstamp |
| 50 | +WARN: resolve_btfids: unresolved symbol bpf_percpu_obj_new_impl |
| 51 | +WARN: resolve_btfids: unresolved symbol bpf_obj_new_impl |
| 52 | +WARN: resolve_btfids: unresolved symbol bpf_lookup_user_key |
| 53 | +WARN: resolve_btfids: unresolved symbol bpf_lookup_system_key |
| 54 | +WARN: resolve_btfids: unresolved symbol bpf_iter_task_vma_new |
| 55 | +WARN: resolve_btfids: unresolved symbol bpf_iter_scx_dsq_new |
| 56 | +WARN: resolve_btfids: unresolved symbol bpf_get_kmem_cache |
| 57 | +WARN: resolve_btfids: unresolved symbol bpf_dynptr_from_xdp |
| 58 | +WARN: resolve_btfids: unresolved symbol bpf_dynptr_from_skb |
| 59 | +WARN: resolve_btfids: unresolved symbol bpf_cgroup_from_id |
| 60 | + NM System.map |
| 61 | + |
| 62 | +After further debugging this can be reproduced more simply: |
| 63 | + |
| 64 | +$ pahole -J -j --btf_features=decl_tag,consistent_func,decl_tag_kfuncs .tmp_vmlinux_armhf |
| 65 | +btf_encoder__tag_kfunc: failed to find kfunc 'scx_bpf_select_cpu_dfl' in BTF |
| 66 | +btf_encoder__tag_kfuncs: failed to tag kfunc 'scx_bpf_select_cpu_dfl' |
| 67 | + |
| 68 | +$ pfunct -Fbtf -E -f scx_bpf_select_cpu_dfl .tmp_vmlinux_armhf |
| 69 | +<nothing> |
| 70 | + |
| 71 | +$ pfunct -Fdwarf -E -f scx_bpf_select_cpu_dfl .tmp_vmlinux_armhf |
| 72 | +s32 scx_bpf_select_cpu_dfl(struct task_struct * p, s32 prev_cpu, u64 wake_flags, bool * is_idle); |
| 73 | + |
| 74 | +$ pahole -J -j --btf_features=decl_tag,decl_tag_kfuncs .tmp_vmlinux_armhf |
| 75 | + |
| 76 | +$ pfunct -Fbtf -E -f scx_bpf_select_cpu_dfl .tmp_vmlinux_armhf |
| 77 | +bpf_kfunc s32 scx_bpf_select_cpu_dfl(struct task_struct * p, s32 prev_cpu, u64 wake_flags, bool * is_idle); |
| 78 | + |
| 79 | +The key things to note are the pahole 'consistent_func' feature and the u64 |
| 80 | +'wake_flags' parameter vs. arm 32-bit registers. These point to existing |
| 81 | +code handling arguments larger than register-size, allowing them to be |
| 82 | +BTF encoded but only if structs. |
| 83 | + |
| 84 | +Generalize the code for any argument type larger than register size (i.e. |
| 85 | +size > cu->addr_size). This should work for integral or aggregate types, |
| 86 | +and also avoids a bug in the current code where a register-sized struct |
| 87 | +could be mistaken for larger. Note that zero-sized arguments will still |
| 88 | +be marked as inconsistent and not encoded. |
| 89 | + |
| 90 | +Fixes: a53c58158b76 ("dwarf_loader: Mark functions that do not use expected registers for params") |
| 91 | +Tested-by: Alexis Lothoré <alexis.lothore@bootlin.com> |
| 92 | +Tested-by: Alan Maguire <alan.maguire@oracle.com> |
| 93 | +Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com> |
| 94 | +--- |
| 95 | +v2 -> v3: |
| 96 | + - Added Tested-by: from Alexis and Alan. |
| 97 | + - Revert support for encoding 0-sized structs (as v1) after discussion: |
| 98 | + https://lore.kernel.org/dwarves/9a41b21f-c0ae-4298-bf95-09d0cdc3f3ab@oracle.com/ |
| 99 | + - Inline param__is_wide() and clarify some naming/wording. |
| 100 | + |
| 101 | +v1 -> v2: |
| 102 | + - Update to preserve existing behaviour where zero-sized struct params |
| 103 | + still permit the function to be encoded, as noted by Alan. |
| 104 | + |
| 105 | +--- |
| 106 | + dwarf_loader.c | 37 ++++++++++++------------------------- |
| 107 | + 1 file changed, 12 insertions(+), 25 deletions(-) |
| 108 | + |
| 109 | +diff --git a/dwarf_loader.c b/dwarf_loader.c |
| 110 | +index e1ba7bc..134a76b 100644 |
| 111 | +--- a/dwarf_loader.c |
| 112 | ++++ b/dwarf_loader.c |
| 113 | +@@ -2914,23 +2914,9 @@ out: |
| 114 | + return 0; |
| 115 | + } |
| 116 | + |
| 117 | +-static bool param__is_struct(struct cu *cu, struct tag *tag) |
| 118 | ++static inline bool param__is_wide(struct cu *cu, struct tag *tag) |
| 119 | + { |
| 120 | +- struct tag *type = cu__type(cu, tag->type); |
| 121 | +- |
| 122 | +- if (!type) |
| 123 | +- return false; |
| 124 | +- |
| 125 | +- switch (type->tag) { |
| 126 | +- case DW_TAG_structure_type: |
| 127 | +- return true; |
| 128 | +- case DW_TAG_const_type: |
| 129 | +- case DW_TAG_typedef: |
| 130 | +- /* handle "typedef struct", const parameter */ |
| 131 | +- return param__is_struct(cu, type); |
| 132 | +- default: |
| 133 | +- return false; |
| 134 | +- } |
| 135 | ++ return tag__size(tag, cu) > cu->addr_size; |
| 136 | + } |
| 137 | + |
| 138 | + static int cu__resolve_func_ret_types_optimized(struct cu *cu) |
| 139 | +@@ -2942,9 +2928,9 @@ static int cu__resolve_func_ret_types_optimized(struct cu *cu) |
| 140 | + struct tag *tag = pt->entries[i]; |
| 141 | + struct parameter *pos; |
| 142 | + struct function *fn = tag__function(tag); |
| 143 | +- bool has_unexpected_reg = false, has_struct_param = false; |
| 144 | ++ bool has_unexpected_reg = false, has_wide_param = false; |
| 145 | + |
| 146 | +- /* mark function as optimized if parameter is, or |
| 147 | ++ /* Mark function as optimized if parameter is, or |
| 148 | + * if parameter does not have a location; at this |
| 149 | + * point location presence has been marked in |
| 150 | + * abstract origins for cases where a parameter |
| 151 | +@@ -2953,10 +2939,11 @@ static int cu__resolve_func_ret_types_optimized(struct cu *cu) |
| 152 | + * |
| 153 | + * Also mark functions which, due to optimization, |
| 154 | + * use an unexpected register for a parameter. |
| 155 | +- * Exception is functions which have a struct |
| 156 | +- * as a parameter, as multiple registers may |
| 157 | +- * be used to represent it, throwing off register |
| 158 | +- * to parameter mapping. |
| 159 | ++ * Exception is functions with a wide parameter, |
| 160 | ++ * as single register won't be used to represent |
| 161 | ++ * it, throwing off register to parameter mapping. |
| 162 | ++ * Examples include large structs or 64-bit types |
| 163 | ++ * on a 32-bit arch. |
| 164 | + */ |
| 165 | + ftype__for_each_parameter(&fn->proto, pos) { |
| 166 | + if (pos->optimized || !pos->has_loc) |
| 167 | +@@ -2967,11 +2954,11 @@ static int cu__resolve_func_ret_types_optimized(struct cu *cu) |
| 168 | + } |
| 169 | + if (has_unexpected_reg) { |
| 170 | + ftype__for_each_parameter(&fn->proto, pos) { |
| 171 | +- has_struct_param = param__is_struct(cu, &pos->tag); |
| 172 | +- if (has_struct_param) |
| 173 | ++ has_wide_param = param__is_wide(cu, &pos->tag); |
| 174 | ++ if (has_wide_param) |
| 175 | + break; |
| 176 | + } |
| 177 | +- if (!has_struct_param) |
| 178 | ++ if (!has_wide_param) |
| 179 | + fn->proto.unexpected_reg = 1; |
| 180 | + } |
| 181 | + |
| 182 | +-- |
| 183 | +2.34.1 |
| 184 | + |
0 commit comments