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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox