All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yonghong Song <yonghong.song@linux.dev>
To: Jiri Olsa <olsajiri@gmail.com>
Cc: Alan Maguire <alan.maguire@oracle.com>,
	Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>,
	dwarves@vger.kernel.org, Alexei Starovoitov <ast@kernel.org>,
	Andrii Nakryiko <andrii@kernel.org>,
	bpf@vger.kernel.org, kernel-team@fb.com
Subject: Re: [PATCH dwarves v3 2/9] dwarf_loader: Handle signatures with dead arguments
Date: Sun, 22 Mar 2026 11:03:19 -0700	[thread overview]
Message-ID: <97086279-4796-4753-b718-3206cf524d04@linux.dev> (raw)
In-Reply-To: <ab8lZBk2Z_1sPkZL@krava>



On 3/21/26 4:10 PM, Jiri Olsa wrote:
> On Fri, Mar 20, 2026 at 12:09:27PM -0700, Yonghong Song wrote:
>> For llvm dwarf, the dead argument may be in the middle of
>> DW_TAG_subprogram. So we introduce skip_idx in order to
>> match expected registers properly.
>>
>> For example:
>>    0x00042897:   DW_TAG_subprogram
>>                    DW_AT_name      ("create_dev")
>>                    DW_AT_calling_convention        (DW_CC_nocall)
>>                    DW_AT_type      (0x0002429a "int")
>>                    ...
>>
>>    0x000428ab:     DW_TAG_formal_parameter
>>                      DW_AT_name    ("name")
>>                      DW_AT_type    (0x000242ed "char *")
>>                      ...
>>
>>    0x000428b5:     DW_TAG_formal_parameter
>>                      DW_AT_location        (indexed (0x3f) loclist = 0x000027f8:
>>                         [0xffffffff87681370, 0xffffffff8768137a): DW_OP_reg5 RDI
>>                         [0xffffffff8768137a, 0xffffffff87681392): DW_OP_reg3 RBX
>>                         [0xffffffff87681392, 0xffffffff876813ae): DW_OP_entry_value(DW_OP_reg5 RDI), DW_OP_stack_value)
>>                      DW_AT_name    ("dev")
>>                      DW_AT_type    (0x00026859 "dev_t")
>>                      ...
>>
>> With skip_idx, we can identify that the second original argument
>> 'dev' becomes the first one after optimization.
> hi,
> this change removes for me functions below and some of them (ata_dev_select,
> dmi_save_type, perhaps more) seem to be removed because their last 2 arguments
> are constants IIUC, which does not seem to be what the changelog suggests,
> but I guess it's still correct (it's without +true_signature)

For this patch, without +true_signature, it can only get true signatures
when the dead arguments at the end of argument list. With +true_signature,
it allows dead arguments before non-dead arguments.

>
> jirka
>
>
> ata_dev_select
> __cgroup1_procs_write
> check_buffer_access
> dax_disassociate_entry
> devlink_nl_sb_port_pool_fill
> dmi_format_ids
> dmi_save_type
> dmi_save_uuid
> get_buffer
> get_net_track
> gue_remcsum
> icmpv4_xrlim_allow
> insert_vmap_area_augment
> __intel_pmu_enable_all
> ip6addrlbl_fill
> ip6_dst_store
> kernelmode_fixup_or_oops
> merge_note_headers_elf32
> merge_note_headers_elf64
> mt_destroy_walk
> __mutex_lock
> __netdev_walk_all_lower_dev
> pci_create_device_link
> populate_kprobe_blacklist
> proc_get_long
> rt_mutex_adjust_prio_chain
> rt_set_nexthop
> run_lwt_bpf
> simple_attr_write_xsigned
> __skb_gro_checksum_validate_needed
> tclass_notify
> tg_set_conf
> toggle_bp_slot
> trace_event_get_offsets_aer_event
> trace_event_get_offsets_br_fdb_external_learn_add
> trace_event_get_offsets_br_fdb_update
> trace_event_get_offsets_cache_tag_flush
> trace_event_get_offsets_devlink_health_recover_aborted
> trace_event_get_offsets_iommu_error
> trace_event_get_offsets_prq_report
> virtio_net_hdr_from_skb
> writeback_inodes_wb
> xfs_file_dio_write_aligned
>
>
>> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
>> ---
>>   dwarf_loader.c | 19 +++++++++++++++++--
>>   1 file changed, 17 insertions(+), 2 deletions(-)
>>
>> diff --git a/dwarf_loader.c b/dwarf_loader.c
>> index 21f8bf1..a6e6587 100644
>> --- a/dwarf_loader.c
>> +++ b/dwarf_loader.c
>> @@ -1192,6 +1192,7 @@ static ptrdiff_t __dwarf_getlocations(Dwarf_Attribute *attr,
>>   
>>   struct func_info {
>>   	bool signature_changed;
>> +	int skip_idx;
>>   };
>>   
>>   /* For DW_AT_location 'attr':
>> @@ -1264,6 +1265,7 @@ static struct parameter *parameter__new(Dwarf_Die *die, struct cu *cu,
>>   	if (parm != NULL) {
>>   		bool has_const_value;
>>   		Dwarf_Attribute attr;
>> +		int reg_idx;
>>   
>>   		tag__init(&parm->tag, cu, die);
>>   		parm->name = attr_string(die, DW_AT_name, conf);
>> @@ -1273,7 +1275,18 @@ static struct parameter *parameter__new(Dwarf_Die *die, struct cu *cu,
>>   		if (!info->signature_changed) {
>>   			if (cu->producer_clang || param_idx >= cu->nr_register_params)
>>   				return parm;
>> +		} else {
>> +			/* if true_signature is not enabled, mark parameter as
>> +			 * unexpected_reg since there is a skipped parameter before.
>> +			 */
>> +			if (!conf->true_signature && info->skip_idx) {
>> +				parm->unexpected_reg = 1;
>> +				return parm;
>> +			}
>>   		}
>> +		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
>>   		 * the original parameter definition (with no name in the DIE)
>>   		 * are the result of later DWARF generation during compilation
>> @@ -1311,7 +1324,7 @@ static struct parameter *parameter__new(Dwarf_Die *die, struct cu *cu,
>>   		parm->has_loc = dwarf_attr(die, DW_AT_location, &attr) != NULL;
>>   
>>   		if (parm->has_loc) {
>> -			int expected_reg = cu->register_params[param_idx];
>> +			int expected_reg = cu->register_params[reg_idx];
>>   			int actual_reg = parameter__reg(&attr, expected_reg);
>>   
>>   			if (actual_reg < 0)
>> @@ -1324,8 +1337,10 @@ static struct parameter *parameter__new(Dwarf_Die *die, struct cu *cu,
>>   				 * contents.
>>   				 */
>>   				parm->unexpected_reg = 1;
>> -		} else if (has_const_value) {
>> +		} else if (has_const_value || info->signature_changed) {
>>   			parm->optimized = 1;
>> +			if (info->signature_changed)
>> +				info->skip_idx++;
>>   		}
>>   	}
>>   
>> -- 
>> 2.52.0
>>
>>


  reply	other threads:[~2026-03-22 18:03 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-20 19:09 [PATCH dwarves v3 0/9] pahole: Encode true signatures in kernel BTF Yonghong Song
2026-03-20 19:09 ` [PATCH dwarves v3 1/9] dwarf_loader: Reduce parameter checking with clang DW_AT_calling_convention attr Yonghong Song
2026-03-21 23:10   ` Jiri Olsa
2026-03-22 17:36     ` Yonghong Song
2026-03-23 12:56       ` Alan Maguire
2026-03-23 18:32         ` Yonghong Song
2026-03-20 19:09 ` [PATCH dwarves v3 2/9] dwarf_loader: Handle signatures with dead arguments Yonghong Song
2026-03-21 23:10   ` Jiri Olsa
2026-03-22 18:03     ` Yonghong Song [this message]
2026-03-20 19:09 ` [PATCH dwarves v3 3/9] dwarf_loader: Refactor initial ret -1 to be macro PARM_DEFAULT_FAIL Yonghong Song
2026-03-20 19:09 ` [PATCH dwarves v3 4/9] dwarf_laoder: Handle locations with DW_OP_fbreg Yonghong Song
2026-03-20 19:09 ` [PATCH dwarves v3 5/9] dwarf_loader: Change exprlen checking condition in parameter__reg() Yonghong Song
2026-03-20 19:09 ` [PATCH dwarves v3 6/9] dwarf_loader: Detect optimized parameters with locations having constant values Yonghong Song
2026-03-20 19:09 ` [PATCH dwarves v3 7/9] dwarf_loader: Handle expression lists Yonghong Song
2026-03-21 23:10   ` Jiri Olsa
2026-03-22 18:33     ` Yonghong Song
2026-03-20 19:09 ` [PATCH dwarves v3 8/9] btf_encoder: Handle optimized parameter properly Yonghong Song
2026-03-20 19:10 ` [PATCH dwarves v3 9/9] tests: Add a few clang true signature tests Yonghong Song
2026-03-23 15:41   ` Alan Maguire
2026-03-23 19:58     ` Yonghong Song

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=97086279-4796-4753-b718-3206cf524d04@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 \
    --cc=olsajiri@gmail.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.