From: Yonghong Song <yonghong.song@linux.dev>
To: Alan Maguire <alan.maguire@oracle.com>,
Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>,
dwarves@vger.kernel.org
Cc: Alexei Starovoitov <ast@kernel.org>,
Andrii Nakryiko <andrii@kernel.org>,
bpf@vger.kernel.org, kernel-team@fb.com
Subject: Re: [PATCH dwarves v4 09/11] dwarf_loader: Handle expression lists
Date: Sat, 23 May 2026 09:32:47 -0700 [thread overview]
Message-ID: <0adac56b-d6b6-44d6-b531-e1827498cd19@linux.dev> (raw)
In-Reply-To: <d8eec3f5-6d6d-4fb7-8a51-c0732b9d880a@oracle.com>
On 3/31/26 1:04 AM, Alan Maguire wrote:
> On 26/03/2026 01:32, Yonghong Song wrote:
>> Location lists having more than one op's are checked.
>> If the parameter size is less or equal to size of long,
>> the argument should match the corresponding ABI register.
>> For example:
>>
>> 0x0aba0808: DW_TAG_subprogram
>> DW_AT_name ("addrconf_ifdown")
>> DW_AT_calling_convention (DW_CC_nocall)
>> DW_AT_type (0x0ab7d8e9 "int")
>> ...
>>
>> 0x0aba082b: DW_TAG_formal_parameter
>> DW_AT_location (indexed (0x32b) loclist = 0x016eabcd:
>> [0xffffffff83f6fef9, 0xffffffff83f6ff98): DW_OP_reg5 RDI
>> [0xffffffff83f6ff98, 0xffffffff83f70080): DW_OP_reg12 R12
>> [0xffffffff83f70080, 0xffffffff83f70111): DW_OP_breg7 RSP+112
>> [0xffffffff83f70111, 0xffffffff83f7014f): DW_OP_reg12 R12
>> [0xffffffff83f7014f, 0xffffffff83f7123c): DW_OP_breg7 RSP+112
>> [0xffffffff83f7123c, 0xffffffff83f7128c): DW_OP_entry_value(DW_OP_reg5 RDI), DW_OP_stack_value
>> [0xffffffff83f7128c, 0xffffffff83f712a9): DW_OP_reg12 R12
>> [0xffffffff83f712a9, 0xffffffff83f712cd): DW_OP_breg7 RSP+112
>> [0xffffffff83f712cd, 0xffffffff83f712d2): DW_OP_entry_value(DW_OP_reg5 RDI), DW_OP_stack_value
>> [0xffffffff83f712d2, 0xffffffff83f713dd): DW_OP_breg7 RSP+112)
>> DW_AT_name ("dev")
>> DW_AT_type (0x0ab7cb7d "net_device *")
>> ...
>>
>> 0x0aba0836: DW_TAG_formal_parameter
>> DW_AT_location (indexed (0x32c) loclist = 0x016eac39:
>> [0xffffffff83f6fef9, 0xffffffff83f6ff15): DW_OP_breg4 RSI+0, DW_OP_constu 0xffffffff, DW_OP_and, DW_OP_convert (0x0ab7b571) "DW_ATE_unsigned_1", DW_OP_convert (0x0ab7b576) "DW_ATE_unsigned_8", DW_OP_stack_value
>> [0xffffffff83f6ff15, 0xffffffff83f7127c): DW_OP_breg7 RSP+36, DW_OP_deref_size 0x4, DW_OP_convert (0x0ab7b571) "DW_ATE_unsigned_1", DW_OP_convert (0x0ab7b576) "DW_ATE_unsigned_8", DW_OP_stack_value
>> [0xffffffff83f7128c, 0xffffffff83f713dd): DW_OP_breg7 RSP+36, DW_OP_deref_size 0x4, DW_OP_convert (0x0ab7b571) "DW_ATE_unsigned_1", DW_OP_convert (0x0ab7b576) "DW_ATE_unsigned_8", DW_OP_stack_value)
>> DW_AT_name ("unregister")
>> DW_AT_type (0x0ab7c933 "bool")
>> ...
>>
>> The parameter 'unregister' is the second argument which matches ABI register RSI.
>> So the function "addrconf_ifdown" signature is valid.
>>
>> If the parameter size is '2 x size_of_long', more handling is necessary, e.g., below:
>>
>> 0x0a01e174: DW_TAG_subprogram
>> DW_AT_name ("check_zeroed_sockptr")
>> DW_AT_calling_convention (DW_CC_nocall)
>> DW_AT_type (0x09fead35 "int")
>> ...
>>
>> 0x0a01e187: DW_TAG_formal_parameter
>> DW_AT_location (indexed (0x5b6) loclist = 0x0157f03f:
>> [0xffffffff83c941c0, 0xffffffff83c941c4): DW_OP_reg5 RDI, DW_OP_piece 0x8, DW_OP_reg4 RSI, DW_OP_piece 0x1
>> [0xffffffff83c941c4, 0xffffffff83c941cc): DW_OP_piece 0x8, DW_OP_reg4 RSI, DW_OP_piece 0x1
>> [0xffffffff83c941e1, 0xffffffff83c941e4): DW_OP_piece 0x8, DW_OP_reg4 RSI, DW_OP_piece 0x1)
>> DW_AT_name ("src")
>> DW_AT_type (0x09ff832d "sockptr_t")
>> ...
>>
>> 0x0a01e193: DW_TAG_formal_parameter
>> DW_AT_const_value (64)
>> DW_AT_name ("offset")
>> DW_AT_type (0x09fee984 "size_t")
>> ...
>>
>> 0x0a01e19e: DW_TAG_formal_parameter
>> DW_AT_location (indexed (0x5b7) loclist = 0x0157f06b:
>> [0xffffffff83c941c0, 0xffffffff83c941d1): DW_OP_reg1 RDX
>> [0xffffffff83c941d1, 0xffffffff83c941e1): DW_OP_entry_value(DW_OP_reg1 RDX), DW_OP_stack_value
>> [0xffffffff83c941e1, 0xffffffff83c941e9): DW_OP_reg1 RDX)
>> DW_AT_name ("size")
>> DW_AT_type (0x09fee984 "size_t")
>> ...
>>
>> The first parameter 'src' will take two ABI registers. This patch correctly detects such a pattern
>> to construct the true signature.
>>
>> However, it is possible that only one 'size_of_long' is used from '2 x size_of_long'. For example
>>
>> 0x019520c6: DW_TAG_subprogram
>> DW_AT_name ("map_create")
>> DW_AT_calling_convention (DW_CC_nocall)
>> DW_AT_type (0x01934b29 "int")
>> ...
>>
>> 0x01952111: DW_TAG_formal_parameter
>> DW_AT_location (indexed (0x31b) loclist = 0x0034fa0f:
>> [0xffffffff81892345, 0xffffffff8189237c): DW_OP_reg5 RDI
>> [0xffffffff8189237c, 0xffffffff818923bd): DW_OP_reg3 RBX
>> [0xffffffff818923bd, 0xffffffff818923d4): DW_OP_entry_value(DW_OP_reg5 RDI), DW_OP_stack_value
>> [0xffffffff818923d4, 0xffffffff81892dcb): DW_OP_reg3 RBX
>> [0xffffffff81892df3, 0xffffffff81892e01): DW_OP_entry_value(DW_OP_reg5 RDI), DW_OP_stack_value
>> [0xffffffff81892e01, 0xffffffff818932a9): DW_OP_reg3 RBX)
>> DW_AT_name ("attr")
>> DW_AT_type (0x01934d17 "bpf_attr *")
>> ...
>>
>> 0x0195211d: DW_TAG_formal_parameter
>> DW_AT_location (indexed (0x31a) loclist = 0x0034f9dc:
>> [0xffffffff81892345, 0xffffffff81892357): DW_OP_piece 0x8, DW_OP_reg4 RSI, DW_OP_piece 0x1
>> [0xffffffff81892357, 0xffffffff81892f02): DW_OP_piece 0x8, DW_OP_breg7 RSP+20, DW_OP_deref_size 0x4, DW_OP_stack_value, DW_OP_piece 0x1
>> [0xffffffff81892f07, 0xffffffff818932a9): DW_OP_piece 0x8, DW_OP_breg7 RSP+20, DW_OP_deref_size 0x4, DW_OP_stack_value, DW_OP_piece 0x1)
>> DW_AT_name ("uattr")
>> DW_AT_type (0x019512ab "bpfptr_t")
>> ...
>>
>> For parameter 'uattr', only second half of parameter is used. For such cases,
>> the name and the type is changed in pahole and eventually going to vmlinux btf.
>> [55697] FUNC_PROTO '(anon)' ret_type_id=106780 vlen=2
>> 'attr' type_id=455
>> 'uattr__is_kernel' type_id=82014
>> [82014] TYPEDEF 'bool' type_id=67434
>> [113251] FUNC 'map_create' type_id=55697 linkage=static
>> You can see the new parameter name is 'uattr__is_kernel' and the type is 'bool'.
>>
>> With this patch, the number of invalid true signatures is reduced from 83 to 18.
>>
>> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
>> ---
>> dwarf_loader.c | 239 +++++++++++++++++++++++++++++++++++++++++++++++--
>> dwarves.h | 1 +
>> 2 files changed, 233 insertions(+), 7 deletions(-)
>>
>> diff --git a/dwarf_loader.c b/dwarf_loader.c
>> index d538e97..4e6e042 100644
>> --- a/dwarf_loader.c
>> +++ b/dwarf_loader.c
>> @@ -1204,6 +1204,8 @@ static ptrdiff_t __dwarf_getlocations(Dwarf_Attribute *attr,
>> #define PARM_UNEXPECTED -2
>> #define PARM_OPTIMIZED_OUT -3
>> #define PARM_CONTINUE -4
>> +#define PARM_TWO_ADDR_LEN -5
>> +#define PARM_TO_BE_IMPROVED -6
>>
>> /* Max 20 register parameters, considering some parameters may be optimized out. */
>> #define MAX_PRESCAN_PARAMS 20
>> @@ -1291,7 +1293,47 @@ static int parameter__peek_first_reg(Dwarf_Die *die)
>> return -1;
>> }
>>
[...]
>> /* For DW_AT_location 'attr':
>> * - if first location is DW_OP_regXX with expected number, return the register;
>> * otherwise save the register for later return
>> @@ -1313,15 +1515,18 @@ static int parameter__multi_exprs(Dwarf_Op *expr, int loc_num)
>> * - otherwise if no register was found for locations, return PARM_DEFAULT_FAIL.
>> */
>> static int parameter__reg(Dwarf_Attribute *attr, int expected_reg, struct conf_load *conf,
>> - struct func_info *info)
>> + struct func_info *info, struct cu *cu, Dwarf_Die *die,
>> + struct parameter *parm, int param_idx, int reg_idx)
>> {
>> Dwarf_Addr base, start, end;
>> Dwarf_Op *expr, *entry_ops;
>> Dwarf_Attribute entry_attr;
>> size_t exprlen, entry_len;
>> ptrdiff_t offset = 0;
>> + int byte_size = 0;
>> int loc_num = -1;
>> int ret = PARM_DEFAULT_FAIL;
>> + unsigned long first_half = 0, second_half = 0;
>>
>> /* use libdw__lock as dwarf_getlocation(s) has concurrency issues
>> * when libdw is not compiled with experimental --enable-thread-safety
>> @@ -1341,8 +1546,17 @@ static int parameter__reg(Dwarf_Attribute *attr, int expected_reg, struct conf_l
>> if (!info->signature_changed || !conf->true_signature)
>> continue;
>>
>> + if (!byte_size)
>> + byte_size = get_type_byte_size(die, cu);
>> + /* This should not happen. */
>> + if (!byte_size) {
>> + ret = PARM_UNEXPECTED;
>> + goto out;
>> + }
>> +
>> int res;
>> - res = parameter__multi_exprs(expr, loc_num);
>> + res = parameter__multi_exprs(expr, loc_num, cu, exprlen, die, expected_reg,
>> + byte_size, &first_half, &second_half, &ret);
>> if (res == PARM_CONTINUE)
>> continue;
>> ret = res;
>> @@ -1391,6 +1605,11 @@ static int parameter__reg(Dwarf_Attribute *attr, int expected_reg, struct conf_l
>> break;
>> }
>> }
>> +
>> + ret = parameter__handle_two_addr_len(expected_reg, first_half, second_half,
>> + ret, die, conf, cu, parm, param_idx, reg_idx,
>> + byte_size, info);
>> +
>> out:
>> pthread_mutex_unlock(&libdw__lock);
>> return ret;
>> @@ -1417,8 +1636,6 @@ static struct parameter *parameter__new(Dwarf_Die *die, struct cu *cu,
>> return parm;
>> } else {
>> reg_idx = param_idx - info->skip_idx;
>> - if (reg_idx >= cu->nr_register_params)
>> - return parm;
>> }
>>
>> /* Parameters which use DW_AT_abstract_origin to point at
>> @@ -1459,15 +1676,23 @@ static struct parameter *parameter__new(Dwarf_Die *die, struct cu *cu,
>> true_sig_enabled = conf->true_signature && info->signature_changed;
>>
>> if (parm->has_loc) {
>> + if (reg_idx >= cu->nr_register_params)
> it looks like we only assign reg_idx in the else (signature_changed) branch above;
> we should make sure we don't wind up using an uninitialized value here
You are right. clang works but gcc will have some issues.
The following is the fix:
if (param_idx < 0)
return parm;
if (!info->signature_changed) {
if (cu->producer_clang || param_idx >= cu->nr_register_params)
return parm;
=== change ===: reg_idx = param_idx;
} else {
reg_idx = param_idx - info->skip_idx;
}
>
>
>> + return parm;
>> +
>> int expected_reg = cu->register_params[reg_idx];
>> - int actual_reg = parameter__reg(&attr, expected_reg, conf, info);
>> + int actual_reg = parameter__reg(&attr, expected_reg, conf, info, cu, die,
>> + parm, param_idx, reg_idx);
>>
>> if (actual_reg == PARM_DEFAULT_FAIL) {
>> parm->optimized = 1;
>> } else if (actual_reg == PARM_OPTIMIZED_OUT) {
>> parm->optimized = 1;
>> info->skip_idx++;
>> - } else if (actual_reg == PARM_UNEXPECTED || (expected_reg >= 0 && expected_reg != actual_reg)) {
>> + } else if (actual_reg == PARM_TWO_ADDR_LEN) {
>> + /* account for parameter with two registers */
>> + info->skip_idx--;
>> + } else if (actual_reg == PARM_UNEXPECTED || actual_reg == PARM_TO_BE_IMPROVED ||
>> + (expected_reg >= 0 && expected_reg != actual_reg)) {
>> /* mark parameters that use an unexpected
>> * register to hold a parameter; these will
>> * be problematic for users of BTF as they
>> diff --git a/dwarves.h b/dwarves.h
>> index 164bd3b..7f818d8 100644
>> --- a/dwarves.h
>> +++ b/dwarves.h
>> @@ -944,6 +944,7 @@ size_t lexblock__fprintf(const struct lexblock *lexblock, const struct cu *cu,
>> struct parameter {
>> struct tag tag;
>> const char *name;
>> + const char *true_sig_member_name;
>> uint8_t optimized:1;
>> uint8_t unexpected_reg:1;
>> uint8_t has_loc:1;
next prev parent reply other threads:[~2026-05-23 16:33 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-26 1:31 [PATCH dwarves v4 00/11] pahole: Encode true signatures in kernel BTF Yonghong Song
2026-03-26 1:31 ` [PATCH dwarves v4 01/11] dwarf_loader: Reduce parameter checking with clang DW_AT_calling_convention attr Yonghong Song
2026-03-30 8:31 ` Alan Maguire
2026-05-23 16:22 ` Yonghong Song
2026-03-26 1:31 ` [PATCH dwarves v4 02/11] dwarf_loader: Prescan all parameters with expected registers Yonghong Song
2026-03-26 1:31 ` [PATCH dwarves v4 03/11] dwarf_loader: Handle signatures with dead arguments Yonghong Song
2026-03-30 10:13 ` Alan Maguire
2026-05-23 16:28 ` Yonghong Song
2026-03-26 1:32 ` [PATCH dwarves v4 04/11] dwarf_loader: Refactor initial ret -1 to be macro PARM_DEFAULT_FAIL Yonghong Song
2026-03-26 1:32 ` [PATCH dwarves v4 05/11] dwarf_laoder: Handle locations with DW_OP_fbreg Yonghong Song
2026-03-26 1:32 ` [PATCH dwarves v4 06/11] dwarf_loader: Change exprlen checking condition in parameter__reg() Yonghong Song
2026-03-26 1:32 ` [PATCH dwarves v4 07/11] dwarf_loader: Detect optimized parameters with locations having constant values Yonghong Song
2026-03-26 1:32 ` [PATCH dwarves v4 08/11] dwarf_loader: Check whether two-reg parameter actually use two regs or not Yonghong Song
2026-03-26 1:32 ` [PATCH dwarves v4 09/11] dwarf_loader: Handle expression lists Yonghong Song
2026-03-31 8:04 ` Alan Maguire
2026-05-23 16:32 ` Yonghong Song [this message]
2026-03-26 1:33 ` [PATCH dwarves v4 10/11] btf_encoder: Handle optimized parameter properly Yonghong Song
2026-03-26 1:33 ` [PATCH dwarves v4 11/11] tests: Add a few clang true signature tests Yonghong Song
2026-03-27 16:02 ` [PATCH dwarves v4 00/11] pahole: Encode true signatures in kernel BTF Alan Maguire
2026-03-27 19:38 ` Yonghong Song
2026-03-30 9:56 ` Alan Maguire
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0adac56b-d6b6-44d6-b531-e1827498cd19@linux.dev \
--to=yonghong.song@linux.dev \
--cc=alan.maguire@oracle.com \
--cc=andrii@kernel.org \
--cc=arnaldo.melo@gmail.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=dwarves@vger.kernel.org \
--cc=kernel-team@fb.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.