Re: [PATCH v2 0/1] btf_loader support for subprogram linkage

[Arnaldo Carvalho de Melo <acme@kernel.org>]

On Tue, Aug 27, 2024 at 12:09:07AM -0400, Will Hawkins wrote:
> Thank you for the feedback on my first attempt. I believe that this
> attempt is better. Given the feedback, I
> 
> 0. Removed the incorrect use of linkage_name field;
> 1. Tried to maintain the semantics of DWARF as much as possible;
> 2. Reworked how BTF information is loaded into a new function based on
> my understanding of the semantics of BTF_FUNC_STATIC, BTF_FUNC_GLOBAL,
> and BTF_FUNC_EXTERN; and
> 3. Modified (slightly) the way functions are printed in pfunc to show
> linkage where appropriate (based, in particular, on 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=45153#c6.
> 
> I saw that Arnaldo and Namhyung have made similar changes to the
> printing code so I hope that I am not stepping on their work (sorry if I
> am!).
> 
> Again, I appreciate the feedback and hope that I was able to address it
> with this v2.

So, I broke it down into two patches, the first for the BTF loader and
then a second patch for pfunct to use that info, but:

⬢[acme@toolbox pahole]$ readelf -wi cases/btf_linkages/linkage.o
Contents of the .debug_info section:

  Compilation Unit @ offset 0:
   Length:        0x4e (32-bit)
   Version:       5
   Unit Type:     DW_UT_compile (1)
   Abbrev Offset: 0
   Pointer Size:  8
 <0><c>: Abbrev Number: 1 (DW_TAG_compile_unit)
    <d>   DW_AT_producer    : (indirect string, offset: 0x5): GNU C17 14.1.1 20240701 (Red Hat 14.1.1-7) -mtune=generic -march=x86-64 -gbtf -g -O0
    <11>   DW_AT_language    : 29	(C11)
    <12>   DW_AT_name        : (indirect line string, offset: 0): linkage.c
    <16>   DW_AT_comp_dir    : (indirect line string, offset: 0xa): /home/acme/git/pahole/cases/btf_linkages
    <1a>   DW_AT_low_pc      : 0
    <22>   DW_AT_high_pc     : 0xb
    <2a>   DW_AT_stmt_list   : 0
 <1><2e>: Abbrev Number: 2 (DW_TAG_subprogram)
    <2f>   DW_AT_name        : x
    <31>   DW_AT_decl_file   : 1
    <32>   DW_AT_decl_line   : 6
    <33>   DW_AT_decl_column : 12
    <34>   DW_AT_type        : <0x4a>
    <38>   DW_AT_low_pc      : 0
    <40>   DW_AT_high_pc     : 0xb
    <48>   DW_AT_frame_base  : 1 byte block: 9c 	(DW_OP_call_frame_cfa)
    <4a>   DW_AT_call_all_calls: 1
 <1><4a>: Abbrev Number: 3 (DW_TAG_base_type)
    <4b>   DW_AT_byte_size   : 4
    <4c>   DW_AT_encoding    : 5	(signed)
    <4d>   DW_AT_name        : int
 <1><51>: Abbrev Number: 0

⬢[acme@toolbox pahole]$

In DWARF we don't have that DW_AT_external, so the simplified second
patch:

⬢[acme@toolbox pahole]$ git diff
diff --git a/pfunct.c b/pfunct.c
index bc3026b384284cb4..9cf61713354d178e 100644
--- a/pfunct.c
+++ b/pfunct.c
@@ -377,6 +377,9 @@ static void function__show(struct function *func, struct cu *cu)
        if (fstats && fstats->printed)
                return;
 
+       if (!func->external)
+               fprintf(stdout, "static ");
+
        if (expand_types)
                function__emit_type_definitions(func, cu, stdout);
        tag__fprintf(tag, cu, &conf, stdout);
⬢[acme@toolbox pahole]$

Would print as static, but it isn't...

⬢[acme@toolbox pahole]$ cat cases/btf_linkages/linkage.c
// https://lore.kernel.org/all/20240823212835.1078351-1-hawkinsw@obs.cr/T/#u
// Will Hawkins
// $ gcc -gbtf -g -O0 -c linkage.c
// $ ~/code/pahole/build/pfunct -Fbtf --compile linkage.o

static int x() {
	return 5;
}
⬢[acme@toolbox pahole]$

⬢[acme@toolbox pahole]$ pfunct --prototype -F btf cases/btf_linkages/linkage.o
int x(void);
⬢[acme@toolbox pahole]$


So, if instead I do it at:

⬢[acme@toolbox pahole]$ git diff
diff --git a/dwarves_fprintf.c b/dwarves_fprintf.c
index ffbe6fbd3f5c405a..9a8bb5f15c88125a 100644
--- a/dwarves_fprintf.c
+++ b/dwarves_fprintf.c
@@ -1406,6 +1406,9 @@ static size_t function__fprintf(const struct tag *tag, const struct cu *cu,
        size_t printed = 0;
        bool inlined = !conf->strip_inline && function__declared_inline(func);
 
+       if (!func->external)
+               printed += fprintf(fp, "%s ", "static");
+
        if (tag->attribute)
                printed += fprintf(fp, "%s ", tag->attribute);
 
⬢[acme@toolbox pahole]$

Then we seem to get what we want:

⬢[acme@toolbox pahole]$ pfunct --prototype -F btf cases/btf_linkages/linkage.o
static int x(void);
⬢[acme@toolbox pahole]$ pfunct --compile -F btf cases/btf_linkages/linkage.o
static int x(void)
{
	return 0;
}

⬢[acme@toolbox pahole]$ pfunct --prototype -F dwarf cases/btf_linkages/linkage.o
static int x(void);
⬢[acme@toolbox pahole]$ pfunct --prototype -F btf cases/btf_linkages/linkage.o
static int x(void);
⬢[acme@toolbox pahole]$ 

And if we add a non-static function, it also works for both DWARF and
BTF when generated by gcc:

⬢[acme@toolbox btf_linkages]$ gcc -gbtf -g -O0 -c linkage.c
⬢[acme@toolbox btf_linkages]$ cd -
/home/acme/git/pahole
⬢[acme@toolbox pahole]$ cat cases/btf_linkages/linkage.c
// https://lore.kernel.org/all/20240823212835.1078351-1-hawkinsw@obs.cr/T/#u
// Will Hawkins
// $ gcc -gbtf -g -O0 -c linkage.c
// $ ~/code/pahole/build/pfunct -Fbtf --compile linkage.o

static int x() {
	return 5;
}

int y() {
	return 4;
}
⬢[acme@toolbox pahole]$ pfunct --prototype -F btf cases/btf_linkages/linkage.o
static int x(void);
int y(void);
⬢[acme@toolbox pahole]$ pfunct --prototype -F dwarf cases/btf_linkages/linkage.o
static int x(void);
int y(void);
⬢[acme@toolbox pahole]$ pfunct --compile -F btf cases/btf_linkages/linkage.o
int y(void)
{
	return 0;
}

static int x(void)
{
	return 0;
}

⬢[acme@toolbox pahole]$ pfunct --compile -F dwarf cases/btf_linkages/linkage.o
int y(void)
{
}

static int x(void)
{
}

⬢[acme@toolbox pahole]$

But, for a vmlinux I have laying around it seems we're not encoding that
with pahole:

⬢[acme@toolbox pahole]$ pfunct --prototype -F btf vmlinux  | grep ^static | head
static void clean_cache_range(void * addr, size_t size);
static void arch_wb_cache_pmem(void * addr, size_t size);
static long int __copy_user_flushcache(void * dst, const void  * src, unsigned int size);
static void __memcpy_flushcache(void * _dst, const void  * _src, size_t size);
static long unsigned int copy_from_user_nmi(void * to, const void  * from, long unsigned int n);
static void call_depth_return_thunk(void);
static void entry_untrain_ret(void);
static void retbleed_untrain_ret(void);
static void srso_untrain_ret(void);
static void srso_alias_safe_ret(void);
⬢[acme@toolbox pahole]$ pfunct --prototype -F btf vmlinux  | grep ^static | wc -l
59015
⬢[acme@toolbox pahole]$ pfunct --prototype -F btf vmlinux  | wc -l
59015
⬢[acme@toolbox pahole]$

⬢[acme@toolbox pahole]$ pahole --btf_encode -j vmlinux 
⬢[acme@toolbox pahole]$ pfunct --prototype -F btf vmlinux  | wc -l
59650
⬢[acme@toolbox pahole]$ pfunct --prototype -F btf vmlinux  | grep ^static | wc -l
59650
⬢[acme@toolbox pahole]$ pfunct --prototype -F btf vmlinux  | grep -v ^static
⬢[acme@toolbox pahole]$

So before we change function__fprintf() we need to investigate this
other part: pahole seemingly not encoding:

+       func->declaration = BTF_INFO_VLEN(tp->info) == BTF_FUNC_EXTERN;
+       func->external = BTF_INFO_VLEN(tp->info) == BTF_FUNC_GLOBAL;

The thing is we want to release 1.28, so lets leave this for after that,
ok.

- Arnaldo