All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Alexis Lothoré" <alexis.lothore@bootlin.com>
To: "Ihor Solodrai" <ihor.solodrai@linux.dev>, <dwarves@vger.kernel.org>
Cc: <bpf@vger.kernel.org>, "Alan Maguire" <alan.maguire@oracle.com>,
	"Arnaldo Carvalho de Melo" <acme@kernel.org>,
	"Alexei Starovoitov" <ast@fb.com>,
	"Thomas Petazzoni" <thomas.petazzoni@bootlin.com>,
	"Bastien Curutchet" <bastien.curutchet@bootlin.com>,
	<ebpf@linuxfoundation.org>
Subject: Re: [PATCH v2 1/3] btf_encoder: skip functions consuming packed structs passed by value on stack
Date: Fri, 04 Jul 2025 23:10:07 +0200	[thread overview]
Message-ID: <DB3KUSI90IPO.1AR35OTXH2Y8M@bootlin.com> (raw)
In-Reply-To: <a9a3cc94-7ce2-4993-96ab-500f250e6e25@linux.dev>

On Fri Jul 4, 2025 at 9:59 PM CEST, Ihor Solodrai wrote:
> On 7/4/25 2:01 AM, Alexis Lothoré wrote:
>> Hello Ihor,
>> 
>> thanks for the prompt feedback and testing !
>> 
>> On Thu Jul 3, 2025 at 8:17 PM CEST, Ihor Solodrai wrote:
>>> On 7/3/25 2:02 AM, Alexis Lothoré (eBPF Foundation) wrote:
>> 
>> [...]
>> 
>>>>    		/* do not exclude functions with optimized-out parameters; they
>>>>    		 * may still be _called_ with the right parameter values, they
>>>>    		 * just do not _use_ them.  Only exclude functions with
>>>> -		 * unexpected register use or multiple inconsistent prototypes.
>>>> +		 * unexpected register use, multiple inconsistent prototypes or
>>>> +		 * uncertain parameters location
>>>>    		 */
>>>> -		add_to_btf |= !state->unexpected_reg && !state->inconsistent_proto;
>>>> +		add_to_btf |= !state->unexpected_reg && !state->inconsistent_proto && !state->uncertain_parm_loc;
>>>
>>>
>>> Is it possible for a function to have uncertain_parm_loc in one CU,
>>> but not in another?
>>>
>>> If yes, we still don't want the function in BTF, right?
>> 
>> TBH, my understanding about those discrepancies between CUs about the same
>> functions and how pahole handle them is still a bit fragile. Have you got
>> any example about how it could be the case ?
>
> I would hope stuff like this doesn't happen often in the real
> world, but in principle you could have the following situation:
>
> #ifdef ENABLE_PACKING
> struct some_data {
>      char header;
>      int payload;
>      short footer;
> } __attribute__((packed));
> #else
> struct some_data {
>      char header;
>      int payload;
>      short footer;
> };
> #endif
>
> void read_data(/* lots of args */, struct some_data data) { ... }
>
> And then one user has #define ENABLE_PACKING and the other doesn't,
> for example:
>
> #define ENABLE_PACKING // or not
> #include "some_data.h"
>
> void user() {
>       struct some_data = get_some_data();
>       ...
>       read_data(/* args */, some_data);
> }
>
> And then you compile a binary with both users:
>
> $ gcc -g -O0 user1.c user2.c
>
> DWARF will contain both packed and not packed instances of struct
> some_data and two corresponding read_data() funcs.

Got it, thanks for the clarification !

  reply	other threads:[~2025-07-04 21:10 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-03  9:02 [PATCH v2 0/3] btf_encoder: do not encode functions consuming packed structs on stack Alexis Lothoré (eBPF Foundation)
2025-07-03  9:02 ` [PATCH v2 1/3] btf_encoder: skip functions consuming packed structs passed by value " Alexis Lothoré (eBPF Foundation)
2025-07-03 18:17   ` Ihor Solodrai
2025-07-04  9:01     ` Alexis Lothoré
2025-07-04 19:59       ` Ihor Solodrai
2025-07-04 21:10         ` Alexis Lothoré [this message]
2025-07-04 20:05   ` Ihor Solodrai
2025-07-04 21:12     ` Alexis Lothoré
2025-07-03  9:02 ` [PATCH v2 2/3] tests: add some tests validating skipped functions due to uncertain arg location Alexis Lothoré (eBPF Foundation)
2025-07-03 18:31   ` Ihor Solodrai
2025-07-04  9:06     ` Alexis Lothoré
2025-07-03  9:02 ` [PATCH v2 3/3] gitignore: ignore all the test kmod build-related files Alexis Lothoré (eBPF Foundation)

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=DB3KUSI90IPO.1AR35OTXH2Y8M@bootlin.com \
    --to=alexis.lothore@bootlin.com \
    --cc=acme@kernel.org \
    --cc=alan.maguire@oracle.com \
    --cc=ast@fb.com \
    --cc=bastien.curutchet@bootlin.com \
    --cc=bpf@vger.kernel.org \
    --cc=dwarves@vger.kernel.org \
    --cc=ebpf@linuxfoundation.org \
    --cc=ihor.solodrai@linux.dev \
    --cc=thomas.petazzoni@bootlin.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.