BPF List
 help / color / mirror / Atom feed
From: Eduard Zingerman <eddyz87@gmail.com>
To: Alan Maguire <alan.maguire@oracle.com>,
	acme@kernel.org, yhs@fb.com, ast@kernel.org, olsajiri@gmail.com,
	timo@incline.eu
Cc: daniel@iogearbox.net, andrii@kernel.org, songliubraving@fb.com,
	john.fastabend@gmail.com, kpsingh@chromium.org, sdf@google.com,
	haoluo@google.com, martin.lau@kernel.org, bpf@vger.kernel.org
Subject: Re: [PATCH dwarves 1/5] dwarves: help dwarf loader spot functions with optimized-out parameters
Date: Thu, 26 Jan 2023 01:42:41 +0200	[thread overview]
Message-ID: <bc50242da1ea8b3b3eafb62e880ed4a278492d2c.camel@gmail.com> (raw)
In-Reply-To: <530ea13a-5229-82a8-d976-b0bc141c3448@oracle.com>

On Wed, 2023-01-25 at 22:52 +0000, Alan Maguire wrote:
[...]
> 
> Thanks for this - I tried it, and we spot the optimization once we update
> die__create_new_parameter() as follows:
> 
> diff --git a/dwarf_loader.c b/dwarf_loader.c
> index f96b6ff..605ad45 100644
> --- a/dwarf_loader.c
> +++ b/dwarf_loader.c
> @@ -1529,6 +1530,8 @@ static struct tag *die__create_new_parameter(Dwarf_Die *di
>  
>         if (ftype != NULL) {
>                 ftype__add_parameter(ftype, parm);
> +               if (parm->optimized)
> +                       ftype->optimized_parms = 1;
>                 if (param_idx >= 0) {
>                         if (add_child_llvm_annotations(die, param_idx, conf, &(t
>                                 return NULL;
> 

Great, looks good.

> With that change, I see:
> 
> $ pahole --verbose --btf_encode_detached=test.btf test.o
> btf_encoder__new: 'test.o' doesn't have '.data..percpu' section
> Found 0 per-CPU variables!
> Found 2 functions!
> File test.o:
> [1] INT int size=4 nr_bits=32 encoding=SIGNED
> [2] PTR (anon) type_id=3
> [3] PTR (anon) type_id=4
> [4] INT char size=1 nr_bits=8 encoding=SIGNED
> [5] FUNC_PROTO (anon) return=1 args=(1 argc, 2 argv)
> [6] FUNC main type_id=5
> added local function 'f', optimized-out params
> skipping addition of 'f' due to optimized-out parameters

Sorry, I have one more silly program.

I talked to Yonghong today and we discussed if compiler can change a
type of a function parameter as a result of some optimization.
Consider the following example:

    $ cat test.c
    struct st {
      int a;
      int b;
    };
    
    __attribute__((noinline))
    static int f(struct st *s) {
      return s->a + s->b;
    }
    
    int main(int argc, char *argv[]) {
      struct st s = {
        .a = (long)argv[0],
        .b = (long)argv[1]
      };
      return f(&s);
    }

When compiled by `clang` with -O3 the prototype of `f` is changed from
`int f(struct *st)` to `int f(int, int)`:

    $ clang -O3 -g -c test.c -o test.o && llvm-objdump -d test.o
    ...
    0000000000000000 <main>:
           0: 8b 3e                        	movl	(%rsi), %edi
           2: 8b 76 08                     	movl	0x8(%rsi), %esi
           5: eb 09                        	jmp	0x10 <f>
           7: 66 0f 1f 84 00 00 00 00 00   	nopw	(%rax,%rax)
    
    0000000000000010 <f>:
          10: 8d 04 37                     	leal	(%rdi,%rsi), %eax
          13: c3                           	retq
    
But generated DWARF hides this information:

    $ llvm-dwarfdump test.o
    ...
    0x0000005c:   DW_TAG_subprogram
                    DW_AT_low_pc	(0x0000000000000010)
                    DW_AT_high_pc	(0x0000000000000014)
                    DW_AT_frame_base	(DW_OP_reg7 RSP)
                    DW_AT_call_all_calls	(true)
                    DW_AT_name	("f")
                    DW_AT_decl_file	("/home/eddy/work/tmp/test.c")
                    DW_AT_decl_line	(7)
                    DW_AT_prototyped	(true)
                    DW_AT_type	(0x00000074 "int")
    
    0x0000006b:     DW_TAG_formal_parameter
                      DW_AT_name	("s")
                      DW_AT_decl_file	("/home/eddy/work/tmp/test.c")
                      DW_AT_decl_line	(7)
                      DW_AT_type	(0x0000009e "st *")
    
    0x00000073:     NULL
    ...

Is this important?
(gcc does not do this for the particular example, but I don't know if
 it could be tricked to under some conditions).

Thanks,
Eduard

[...]

  reply	other threads:[~2023-01-25 23:43 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-24 13:45 [PATCH dwarves 0/5] dwarves: support encoding of optimized-out parameters, removal of inconsistent static functions Alan Maguire
2023-01-24 13:45 ` [PATCH dwarves 1/5] dwarves: help dwarf loader spot functions with optimized-out parameters Alan Maguire
2023-01-25 16:53   ` Jiri Olsa
2023-01-25 17:47   ` Eduard Zingerman
2023-01-25 18:28     ` Alan Maguire
2023-01-25 21:34       ` Eduard Zingerman
2023-01-25 22:52         ` Alan Maguire
2023-01-25 23:42           ` Eduard Zingerman [this message]
2023-01-26  0:20             ` Eduard Zingerman
2023-01-26 14:02               ` Alan Maguire
2023-01-26 15:02                 ` Eduard Zingerman
2023-01-24 13:45 ` [PATCH dwarves 2/5] btf_encoder: refactor function addition into dedicated btf_encoder__add_func Alan Maguire
2023-01-24 13:45 ` [PATCH dwarves 3/5] btf_encoder: child encoders should have a reference to parent encoder Alan Maguire
2023-01-24 13:45 ` [PATCH dwarves 4/5] btf_encoder: represent "."-suffixed optimized functions (".isra.0") in BTF Alan Maguire
2023-01-25 17:54   ` Kui-Feng Lee
2023-01-25 18:56     ` Arnaldo Carvalho de Melo
2023-01-26 18:37       ` Kui-Feng Lee
2023-01-25 18:59     ` Alan Maguire
2023-01-26 17:43       ` Kui-Feng Lee
2023-01-24 13:45 ` [PATCH dwarves 5/5] btf_encoder: skip BTF encoding of static functions with inconsistent prototypes Alan Maguire
2023-01-25 13:39   ` Jiri Olsa
2023-01-25 14:18     ` Alan Maguire
2023-01-25 16:53   ` Jiri Olsa
2023-01-26 14:12     ` Alan Maguire
2023-01-24 15:14 ` [PATCH dwarves 0/5] dwarves: support encoding of optimized-out parameters, removal of inconsistent static functions Jiri Olsa
2023-01-24 16:11   ` Alan Maguire
2023-01-25 13:59     ` Jiri Olsa

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=bc50242da1ea8b3b3eafb62e880ed4a278492d2c.camel@gmail.com \
    --to=eddyz87@gmail.com \
    --cc=acme@kernel.org \
    --cc=alan.maguire@oracle.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=haoluo@google.com \
    --cc=john.fastabend@gmail.com \
    --cc=kpsingh@chromium.org \
    --cc=martin.lau@kernel.org \
    --cc=olsajiri@gmail.com \
    --cc=sdf@google.com \
    --cc=songliubraving@fb.com \
    --cc=timo@incline.eu \
    --cc=yhs@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