diff mbox series

[bpf-next] tools/resolve_btfids: Fix sections with wrong alignment

Message ID 20200819092342.259004-1-jolsa@kernel.org
State Accepted
Delegated to: BPF Maintainers
Headers show
Series [bpf-next] tools/resolve_btfids: Fix sections with wrong alignment | expand

Commit Message

Jiri Olsa Aug. 19, 2020, 9:23 a.m. UTC
The data of compressed section should be aligned to 4
(for 32bit) or 8 (for 64 bit) bytes.

The binutils ld sets sh_addralign to 1, which makes libelf
fail with misaligned section error during the update as
reported by Jesper:

   FAILED elf_update(WRITE): invalid section alignment

While waiting for ld fix, we can fix compressed sections
sh_addralign value manually.

Adding warning in -vv mode when the fix is triggered:

  $ ./tools/bpf/resolve_btfids/resolve_btfids -vv vmlinux
  ...
  section(36) .comment, size 44, link 0, flags 30, type=1
  section(37) .debug_aranges, size 45684, link 0, flags 800, type=1
   - fixing wrong alignment sh_addralign 16, expected 8
  section(38) .debug_info, size 129104957, link 0, flags 800, type=1
   - fixing wrong alignment sh_addralign 1, expected 8
  section(39) .debug_abbrev, size 1152583, link 0, flags 800, type=1
   - fixing wrong alignment sh_addralign 1, expected 8
  section(40) .debug_line, size 7374522, link 0, flags 800, type=1
   - fixing wrong alignment sh_addralign 1, expected 8
  section(41) .debug_frame, size 702463, link 0, flags 800, type=1
  section(42) .debug_str, size 1017571, link 0, flags 830, type=1
   - fixing wrong alignment sh_addralign 1, expected 8
  section(43) .debug_loc, size 3019453, link 0, flags 800, type=1
   - fixing wrong alignment sh_addralign 1, expected 8
  section(44) .debug_ranges, size 1744583, link 0, flags 800, type=1
   - fixing wrong alignment sh_addralign 16, expected 8
  section(45) .symtab, size 2955888, link 46, flags 0, type=2
  section(46) .strtab, size 2613072, link 0, flags 0, type=3
  ...
  update ok for vmlinux

Another workaround is to disable compressed debug info data
CONFIG_DEBUG_INFO_COMPRESSED kernel option.

Fixes: fbbb68de80a4 ("bpf: Add resolve_btfids tool to resolve BTF IDs in ELF object")
Cc: Mark Wielaard <mjw@redhat.com>
Cc: Nick Clifton <nickc@redhat.com>
Reported-by: Jesper Dangaard Brouer <brouer@redhat.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/bpf/resolve_btfids/main.c | 36 +++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

Comments

Yonghong Song Aug. 19, 2020, 3:31 p.m. UTC | #1
On 8/19/20 2:23 AM, Jiri Olsa wrote:
> The data of compressed section should be aligned to 4
> (for 32bit) or 8 (for 64 bit) bytes.
> 
> The binutils ld sets sh_addralign to 1, which makes libelf
> fail with misaligned section error during the update as
> reported by Jesper:
> 
>     FAILED elf_update(WRITE): invalid section alignment
> 
> While waiting for ld fix, we can fix compressed sections
> sh_addralign value manually.
> 
> Adding warning in -vv mode when the fix is triggered:
> 
>    $ ./tools/bpf/resolve_btfids/resolve_btfids -vv vmlinux
>    ...
>    section(36) .comment, size 44, link 0, flags 30, type=1
>    section(37) .debug_aranges, size 45684, link 0, flags 800, type=1
>     - fixing wrong alignment sh_addralign 16, expected 8
>    section(38) .debug_info, size 129104957, link 0, flags 800, type=1
>     - fixing wrong alignment sh_addralign 1, expected 8
>    section(39) .debug_abbrev, size 1152583, link 0, flags 800, type=1
>     - fixing wrong alignment sh_addralign 1, expected 8
>    section(40) .debug_line, size 7374522, link 0, flags 800, type=1
>     - fixing wrong alignment sh_addralign 1, expected 8
>    section(41) .debug_frame, size 702463, link 0, flags 800, type=1
>    section(42) .debug_str, size 1017571, link 0, flags 830, type=1
>     - fixing wrong alignment sh_addralign 1, expected 8
>    section(43) .debug_loc, size 3019453, link 0, flags 800, type=1
>     - fixing wrong alignment sh_addralign 1, expected 8
>    section(44) .debug_ranges, size 1744583, link 0, flags 800, type=1
>     - fixing wrong alignment sh_addralign 16, expected 8
>    section(45) .symtab, size 2955888, link 46, flags 0, type=2
>    section(46) .strtab, size 2613072, link 0, flags 0, type=3
>    ...
>    update ok for vmlinux
> 
> Another workaround is to disable compressed debug info data
> CONFIG_DEBUG_INFO_COMPRESSED kernel option.

So CONFIG_DEBUG_INFO_COMPRESSED is required to reproduce the bug, right?

I turned on CONFIG_DEBUG_INFO_COMPRESSED in my config and got a bunch of
build failures.

ld: drivers/crypto/virtio/virtio_crypto_algs.o: unable to initialize 
decompress status for section .debug_info
ld: drivers/crypto/virtio/virtio_crypto_algs.o: unable to initialize 
decompress status for section .debug_info
ld: drivers/crypto/virtio/virtio_crypto_algs.o: unable to initialize 
decompress status for section .debug_info
ld: drivers/crypto/virtio/virtio_crypto_algs.o: unable to initialize 
decompress status for section .debug_info
drivers/crypto/virtio/virtio_crypto_algs.o: file not recognized: File 
format not recognized

ld: net/llc/llc_core.o: unable to initialize decompress status for 
section .debug_info
ld: net/llc/llc_core.o: unable to initialize decompress status for 
section .debug_info
ld: net/llc/llc_core.o: unable to initialize decompress status for 
section .debug_info
ld: net/llc/llc_core.o: unable to initialize decompress status for 
section .debug_info
net/llc/llc_core.o: file not recognized: File format not recognized

...

The 'ld' in my system:

$ ld -V
GNU ld version 2.30-74.el8
   Supported emulations:
    elf_x86_64
    elf32_x86_64
    elf_i386
    elf_iamcu
    i386linux
    elf_l1om
    elf_k1om
    i386pep
    i386pe
$

Do you know what is the issue here?

> 
> Fixes: fbbb68de80a4 ("bpf: Add resolve_btfids tool to resolve BTF IDs in ELF object")
> Cc: Mark Wielaard <mjw@redhat.com>
> Cc: Nick Clifton <nickc@redhat.com>
> Reported-by: Jesper Dangaard Brouer <brouer@redhat.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>   tools/bpf/resolve_btfids/main.c | 36 +++++++++++++++++++++++++++++++++
>   1 file changed, 36 insertions(+)
> 
> diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c
> index 4d9ecb975862..0def0bb1f783 100644
> --- a/tools/bpf/resolve_btfids/main.c
> +++ b/tools/bpf/resolve_btfids/main.c
> @@ -233,6 +233,39 @@ static struct btf_id *add_symbol(struct rb_root *root, char *name, size_t size)
>   	return btf_id__add(root, id, false);
>   }
>   
> +/*
> + * The data of compressed section should be aligned to 4
> + * (for 32bit) or 8 (for 64 bit) bytes. The binutils ld
> + * sets sh_addralign to 1, which makes libelf fail with
> + * misaligned section error during the update:
> + *    FAILED elf_update(WRITE): invalid section alignment
> + *
> + * While waiting for ld fix, we fix the compressed sections
> + * sh_addralign value manualy.
> + */
> +static int compressed_section_fix(Elf *elf, Elf_Scn *scn, GElf_Shdr *sh)
> +{
> +	int expected = gelf_getclass(elf) == ELFCLASS32 ? 4 : 8;
> +
> +	if (!(sh->sh_flags & SHF_COMPRESSED))
> +		return 0;
> +
> +	if (sh->sh_addralign == expected)
> +		return 0;
> +
> +	pr_debug2(" - fixing wrong alignment sh_addralign %u, expected %u\n",
> +		  sh->sh_addralign, expected);
> +
> +	sh->sh_addralign = expected;
> +
> +	if (gelf_update_shdr(scn, sh) == 0) {
> +		printf("FAILED cannot update section header: %s\n",
> +			elf_errmsg(-1));
> +		return -1;
> +	}
> +	return 0;
> +}
> +
>   static int elf_collect(struct object *obj)
>   {
>   	Elf_Scn *scn = NULL;
> @@ -309,6 +342,9 @@ static int elf_collect(struct object *obj)
>   			obj->efile.idlist_shndx = idx;
>   			obj->efile.idlist_addr  = sh.sh_addr;
>   		}
> +
> +		if (compressed_section_fix(elf, scn, &sh))
> +			return -1;
>   	}
>   
>   	return 0;
>
Jesper Dangaard Brouer Aug. 19, 2020, 5:02 p.m. UTC | #2
On Wed, 19 Aug 2020 11:23:42 +0200 Jiri Olsa <jolsa@kernel.org> wrote:

> The data of compressed section should be aligned to 4
> (for 32bit) or 8 (for 64 bit) bytes.
> 
> The binutils ld sets sh_addralign to 1, which makes libelf
> fail with misaligned section error during the update as
> reported by Jesper:
> 
>    FAILED elf_update(WRITE): invalid section alignment
> 
> While waiting for ld fix, we can fix compressed sections
> sh_addralign value manually.
> 
> Adding warning in -vv mode when the fix is triggered:
> 
>   $ ./tools/bpf/resolve_btfids/resolve_btfids -vv vmlinux
>   ...
>   section(36) .comment, size 44, link 0, flags 30, type=1
>   section(37) .debug_aranges, size 45684, link 0, flags 800, type=1
>    - fixing wrong alignment sh_addralign 16, expected 8
>   section(38) .debug_info, size 129104957, link 0, flags 800, type=1
>    - fixing wrong alignment sh_addralign 1, expected 8
>   section(39) .debug_abbrev, size 1152583, link 0, flags 800, type=1
>    - fixing wrong alignment sh_addralign 1, expected 8
>   section(40) .debug_line, size 7374522, link 0, flags 800, type=1
>    - fixing wrong alignment sh_addralign 1, expected 8
>   section(41) .debug_frame, size 702463, link 0, flags 800, type=1
>   section(42) .debug_str, size 1017571, link 0, flags 830, type=1
>    - fixing wrong alignment sh_addralign 1, expected 8
>   section(43) .debug_loc, size 3019453, link 0, flags 800, type=1
>    - fixing wrong alignment sh_addralign 1, expected 8
>   section(44) .debug_ranges, size 1744583, link 0, flags 800, type=1
>    - fixing wrong alignment sh_addralign 16, expected 8
>   section(45) .symtab, size 2955888, link 46, flags 0, type=2
>   section(46) .strtab, size 2613072, link 0, flags 0, type=3
>   ...
>   update ok for vmlinux

I also works for me:

[...]
section(37) .debug_aranges, size 45684, link 0, flags 800, type=1
 - fixing wrong alignment sh_addralign 16, expected 8
section(38) .debug_info, size 129098181, link 0, flags 800, type=1
 - fixing wrong alignment sh_addralign 1, expected 8
section(39) .debug_abbrev, size 1152583, link 0, flags 800, type=1
 - fixing wrong alignment sh_addralign 1, expected 8
section(40) .debug_line, size 7374522, link 0, flags 800, type=1
 - fixing wrong alignment sh_addralign 1, expected 8
section(41) .debug_frame, size 702463, link 0, flags 800, type=1
section(42) .debug_str, size 1017606, link 0, flags 830, type=1
 - fixing wrong alignment sh_addralign 1, expected 8
section(43) .debug_loc, size 3019453, link 0, flags 800, type=1
 - fixing wrong alignment sh_addralign 1, expected 8
section(44) .debug_ranges, size 1744583, link 0, flags 800, type=1
 - fixing wrong alignment sh_addralign 16, expected 8
section(45) .symtab, size 2955888, link 46, flags 0, type=2
section(46) .strtab, size 2613072, link 0, flags 0, type=3
section(47) .shstrtab, size 525, link 0, flags 0, type=3
[...]
update ok for vmlinux.err.bak


> 
> Another workaround is to disable compressed debug info data
> CONFIG_DEBUG_INFO_COMPRESSED kernel option.
> 
> Fixes: fbbb68de80a4 ("bpf: Add resolve_btfids tool to resolve BTF IDs in ELF object")
> Cc: Mark Wielaard <mjw@redhat.com>
> Cc: Nick Clifton <nickc@redhat.com>
> Reported-by: Jesper Dangaard Brouer <brouer@redhat.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---

Thanks for fixing this!

Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
Jiri Olsa Aug. 19, 2020, 5:36 p.m. UTC | #3
On Wed, Aug 19, 2020 at 08:31:51AM -0700, Yonghong Song wrote:
> 
> 
> On 8/19/20 2:23 AM, Jiri Olsa wrote:
> > The data of compressed section should be aligned to 4
> > (for 32bit) or 8 (for 64 bit) bytes.
> > 
> > The binutils ld sets sh_addralign to 1, which makes libelf
> > fail with misaligned section error during the update as
> > reported by Jesper:
> > 
> >     FAILED elf_update(WRITE): invalid section alignment
> > 
> > While waiting for ld fix, we can fix compressed sections
> > sh_addralign value manually.
> > 
> > Adding warning in -vv mode when the fix is triggered:
> > 
> >    $ ./tools/bpf/resolve_btfids/resolve_btfids -vv vmlinux
> >    ...
> >    section(36) .comment, size 44, link 0, flags 30, type=1
> >    section(37) .debug_aranges, size 45684, link 0, flags 800, type=1
> >     - fixing wrong alignment sh_addralign 16, expected 8
> >    section(38) .debug_info, size 129104957, link 0, flags 800, type=1
> >     - fixing wrong alignment sh_addralign 1, expected 8
> >    section(39) .debug_abbrev, size 1152583, link 0, flags 800, type=1
> >     - fixing wrong alignment sh_addralign 1, expected 8
> >    section(40) .debug_line, size 7374522, link 0, flags 800, type=1
> >     - fixing wrong alignment sh_addralign 1, expected 8
> >    section(41) .debug_frame, size 702463, link 0, flags 800, type=1
> >    section(42) .debug_str, size 1017571, link 0, flags 830, type=1
> >     - fixing wrong alignment sh_addralign 1, expected 8
> >    section(43) .debug_loc, size 3019453, link 0, flags 800, type=1
> >     - fixing wrong alignment sh_addralign 1, expected 8
> >    section(44) .debug_ranges, size 1744583, link 0, flags 800, type=1
> >     - fixing wrong alignment sh_addralign 16, expected 8
> >    section(45) .symtab, size 2955888, link 46, flags 0, type=2
> >    section(46) .strtab, size 2613072, link 0, flags 0, type=3
> >    ...
> >    update ok for vmlinux
> > 
> > Another workaround is to disable compressed debug info data
> > CONFIG_DEBUG_INFO_COMPRESSED kernel option.
> 
> So CONFIG_DEBUG_INFO_COMPRESSED is required to reproduce the bug, right?

correct

> 
> I turned on CONFIG_DEBUG_INFO_COMPRESSED in my config and got a bunch of
> build failures.
> 
> ld: drivers/crypto/virtio/virtio_crypto_algs.o: unable to initialize
> decompress status for section .debug_info
> ld: drivers/crypto/virtio/virtio_crypto_algs.o: unable to initialize
> decompress status for section .debug_info
> ld: drivers/crypto/virtio/virtio_crypto_algs.o: unable to initialize
> decompress status for section .debug_info
> ld: drivers/crypto/virtio/virtio_crypto_algs.o: unable to initialize
> decompress status for section .debug_info
> drivers/crypto/virtio/virtio_crypto_algs.o: file not recognized: File format
> not recognized
> 
> ld: net/llc/llc_core.o: unable to initialize decompress status for section
> .debug_info
> ld: net/llc/llc_core.o: unable to initialize decompress status for section
> .debug_info
> ld: net/llc/llc_core.o: unable to initialize decompress status for section
> .debug_info
> ld: net/llc/llc_core.o: unable to initialize decompress status for section
> .debug_info
> net/llc/llc_core.o: file not recognized: File format not recognized
> 
> ...
> 
> The 'ld' in my system:
> 
> $ ld -V
> GNU ld version 2.30-74.el8
>   Supported emulations:
>    elf_x86_64
>    elf32_x86_64
>    elf_i386
>    elf_iamcu
>    i386linux
>    elf_l1om
>    elf_k1om
>    i386pep
>    i386pe
> $
> 
> Do you know what is the issue here?

mine's: GNU ld version 2.32-31.fc31

there's version info in commit:
  10e68b02c861 Makefile: support compressed debug info

  Compress the debug information using zlib.  Requires GCC 5.0+ or Clang
  5.0+, binutils 2.26+, and zlib.

cc-ing Nick Desaulniers, author of that patch.. any idea about the error above?

thanks,
jirka
Nick Desaulniers Aug. 19, 2020, 6:16 p.m. UTC | #4
On Wed, Aug 19, 2020 at 10:36 AM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Wed, Aug 19, 2020 at 08:31:51AM -0700, Yonghong Song wrote:
> >
> >
> > On 8/19/20 2:23 AM, Jiri Olsa wrote:
> > > The data of compressed section should be aligned to 4
> > > (for 32bit) or 8 (for 64 bit) bytes.
> > >
> > > The binutils ld sets sh_addralign to 1, which makes libelf
> > > fail with misaligned section error during the update as
> > > reported by Jesper:
> > >
> > >     FAILED elf_update(WRITE): invalid section alignment
> > >
> > > While waiting for ld fix, we can fix compressed sections
> > > sh_addralign value manually.

Is there a bug filed against GNU ld? Link?

> > >
> > > Adding warning in -vv mode when the fix is triggered:
> > >
> > >    $ ./tools/bpf/resolve_btfids/resolve_btfids -vv vmlinux
> > >    ...
> > >    section(36) .comment, size 44, link 0, flags 30, type=1
> > >    section(37) .debug_aranges, size 45684, link 0, flags 800, type=1
> > >     - fixing wrong alignment sh_addralign 16, expected 8
> > >    section(38) .debug_info, size 129104957, link 0, flags 800, type=1
> > >     - fixing wrong alignment sh_addralign 1, expected 8
> > >    section(39) .debug_abbrev, size 1152583, link 0, flags 800, type=1
> > >     - fixing wrong alignment sh_addralign 1, expected 8
> > >    section(40) .debug_line, size 7374522, link 0, flags 800, type=1
> > >     - fixing wrong alignment sh_addralign 1, expected 8
> > >    section(41) .debug_frame, size 702463, link 0, flags 800, type=1
> > >    section(42) .debug_str, size 1017571, link 0, flags 830, type=1
> > >     - fixing wrong alignment sh_addralign 1, expected 8
> > >    section(43) .debug_loc, size 3019453, link 0, flags 800, type=1
> > >     - fixing wrong alignment sh_addralign 1, expected 8
> > >    section(44) .debug_ranges, size 1744583, link 0, flags 800, type=1
> > >     - fixing wrong alignment sh_addralign 16, expected 8
> > >    section(45) .symtab, size 2955888, link 46, flags 0, type=2
> > >    section(46) .strtab, size 2613072, link 0, flags 0, type=3
> > >    ...
> > >    update ok for vmlinux
> > >
> > > Another workaround is to disable compressed debug info data
> > > CONFIG_DEBUG_INFO_COMPRESSED kernel option.
> >
> > So CONFIG_DEBUG_INFO_COMPRESSED is required to reproduce the bug, right?
>
> correct
>
> >
> > I turned on CONFIG_DEBUG_INFO_COMPRESSED in my config and got a bunch of
> > build failures.
> >
> > ld: drivers/crypto/virtio/virtio_crypto_algs.o: unable to initialize
> > decompress status for section .debug_info
> > ld: drivers/crypto/virtio/virtio_crypto_algs.o: unable to initialize
> > decompress status for section .debug_info
> > ld: drivers/crypto/virtio/virtio_crypto_algs.o: unable to initialize
> > decompress status for section .debug_info
> > ld: drivers/crypto/virtio/virtio_crypto_algs.o: unable to initialize
> > decompress status for section .debug_info
> > drivers/crypto/virtio/virtio_crypto_algs.o: file not recognized: File format
> > not recognized
> >
> > ld: net/llc/llc_core.o: unable to initialize decompress status for section
> > .debug_info
> > ld: net/llc/llc_core.o: unable to initialize decompress status for section
> > .debug_info
> > ld: net/llc/llc_core.o: unable to initialize decompress status for section
> > .debug_info
> > ld: net/llc/llc_core.o: unable to initialize decompress status for section
> > .debug_info
> > net/llc/llc_core.o: file not recognized: File format not recognized
> >
> > ...
> >
> > The 'ld' in my system:
> >
> > $ ld -V
> > GNU ld version 2.30-74.el8
> >   Supported emulations:
> >    elf_x86_64
> >    elf32_x86_64
> >    elf_i386
> >    elf_iamcu
> >    i386linux
> >    elf_l1om
> >    elf_k1om
> >    i386pep
> >    i386pe

According to Documentation/process/changes.rst, the minimum supported
version of GNU binutils for the kernels is 2.23.  Can you upgrade to
that and confirm that you still observe the issue?  I don't want to
spend time chasing bugs in old, unsupported versions of GNU binutils,
especially as Jiri notes, 2.26 is required for
CONFIG_DEBUG_INFO_COMPRESSED.  We can always strengthen the Kconfig
check for it.  Otherwise, I'm not familiar with the observed error
message.

> > $
> >
> > Do you know what is the issue here?
>
> mine's: GNU ld version 2.32-31.fc31
>
> there's version info in commit:
>   10e68b02c861 Makefile: support compressed debug info
>
>   Compress the debug information using zlib.  Requires GCC 5.0+ or Clang
>   5.0+, binutils 2.26+, and zlib.
>
> cc-ing Nick Desaulniers, author of that patch.. any idea about the error above?
>
> thanks,
> jirka
>
Yonghong Song Aug. 19, 2020, 9:30 p.m. UTC | #5
On 8/19/20 11:16 AM, Nick Desaulniers wrote:
> On Wed, Aug 19, 2020 at 10:36 AM Jiri Olsa <jolsa@redhat.com> wrote:
>>
>> On Wed, Aug 19, 2020 at 08:31:51AM -0700, Yonghong Song wrote:
>>>
>>>
>>> On 8/19/20 2:23 AM, Jiri Olsa wrote:
>>>> The data of compressed section should be aligned to 4
>>>> (for 32bit) or 8 (for 64 bit) bytes.
>>>>
>>>> The binutils ld sets sh_addralign to 1, which makes libelf
>>>> fail with misaligned section error during the update as
>>>> reported by Jesper:
>>>>
>>>>      FAILED elf_update(WRITE): invalid section alignment
>>>>
>>>> While waiting for ld fix, we can fix compressed sections
>>>> sh_addralign value manually.
> 
> Is there a bug filed against GNU ld? Link?
> 
>>>>
>>>> Adding warning in -vv mode when the fix is triggered:
>>>>
>>>>     $ ./tools/bpf/resolve_btfids/resolve_btfids -vv vmlinux
>>>>     ...
>>>>     section(36) .comment, size 44, link 0, flags 30, type=1
>>>>     section(37) .debug_aranges, size 45684, link 0, flags 800, type=1
>>>>      - fixing wrong alignment sh_addralign 16, expected 8
>>>>     section(38) .debug_info, size 129104957, link 0, flags 800, type=1
>>>>      - fixing wrong alignment sh_addralign 1, expected 8
>>>>     section(39) .debug_abbrev, size 1152583, link 0, flags 800, type=1
>>>>      - fixing wrong alignment sh_addralign 1, expected 8
>>>>     section(40) .debug_line, size 7374522, link 0, flags 800, type=1
>>>>      - fixing wrong alignment sh_addralign 1, expected 8
>>>>     section(41) .debug_frame, size 702463, link 0, flags 800, type=1
>>>>     section(42) .debug_str, size 1017571, link 0, flags 830, type=1
>>>>      - fixing wrong alignment sh_addralign 1, expected 8
>>>>     section(43) .debug_loc, size 3019453, link 0, flags 800, type=1
>>>>      - fixing wrong alignment sh_addralign 1, expected 8
>>>>     section(44) .debug_ranges, size 1744583, link 0, flags 800, type=1
>>>>      - fixing wrong alignment sh_addralign 16, expected 8
>>>>     section(45) .symtab, size 2955888, link 46, flags 0, type=2
>>>>     section(46) .strtab, size 2613072, link 0, flags 0, type=3
>>>>     ...
>>>>     update ok for vmlinux
>>>>
>>>> Another workaround is to disable compressed debug info data
>>>> CONFIG_DEBUG_INFO_COMPRESSED kernel option.
>>>
>>> So CONFIG_DEBUG_INFO_COMPRESSED is required to reproduce the bug, right?
>>
>> correct
>>
>>>
>>> I turned on CONFIG_DEBUG_INFO_COMPRESSED in my config and got a bunch of
>>> build failures.
>>>
>>> ld: drivers/crypto/virtio/virtio_crypto_algs.o: unable to initialize
>>> decompress status for section .debug_info
>>> ld: drivers/crypto/virtio/virtio_crypto_algs.o: unable to initialize
>>> decompress status for section .debug_info
>>> ld: drivers/crypto/virtio/virtio_crypto_algs.o: unable to initialize
>>> decompress status for section .debug_info
>>> ld: drivers/crypto/virtio/virtio_crypto_algs.o: unable to initialize
>>> decompress status for section .debug_info
>>> drivers/crypto/virtio/virtio_crypto_algs.o: file not recognized: File format
>>> not recognized
>>>
>>> ld: net/llc/llc_core.o: unable to initialize decompress status for section
>>> .debug_info
>>> ld: net/llc/llc_core.o: unable to initialize decompress status for section
>>> .debug_info
>>> ld: net/llc/llc_core.o: unable to initialize decompress status for section
>>> .debug_info
>>> ld: net/llc/llc_core.o: unable to initialize decompress status for section
>>> .debug_info
>>> net/llc/llc_core.o: file not recognized: File format not recognized
>>>
>>> ...
>>>
>>> The 'ld' in my system:
>>>
>>> $ ld -V
>>> GNU ld version 2.30-74.el8
>>>    Supported emulations:
>>>     elf_x86_64
>>>     elf32_x86_64
>>>     elf_i386
>>>     elf_iamcu
>>>     i386linux
>>>     elf_l1om
>>>     elf_k1om
>>>     i386pep
>>>     i386pe
> 
> According to Documentation/process/changes.rst, the minimum supported
> version of GNU binutils for the kernels is 2.23.  Can you upgrade to
> that and confirm that you still observe the issue?  I don't want to
> spend time chasing bugs in old, unsupported versions of GNU binutils,
> especially as Jiri notes, 2.26 is required for
> CONFIG_DEBUG_INFO_COMPRESSED.  We can always strengthen the Kconfig
> check for it.  Otherwise, I'm not familiar with the observed error
> message.

I built a "ld" with latest binutils-gdb repo and I can reproduced
the issue. Indeed applying the patch here fixed the issue. So
I think there is no need to investigate since upstream exhibits
the exact issue described here.

> 
>>> $
>>>
>>> Do you know what is the issue here?
>>
>> mine's: GNU ld version 2.32-31.fc31
>>
>> there's version info in commit:
>>    10e68b02c861 Makefile: support compressed debug info
>>
>>    Compress the debug information using zlib.  Requires GCC 5.0+ or Clang
>>    5.0+, binutils 2.26+, and zlib.
>>
>> cc-ing Nick Desaulniers, author of that patch.. any idea about the error above?
>>
>> thanks,
>> jirka
>>
> 
>
Yonghong Song Aug. 19, 2020, 9:32 p.m. UTC | #6
On 8/19/20 2:23 AM, Jiri Olsa wrote:
> The data of compressed section should be aligned to 4
> (for 32bit) or 8 (for 64 bit) bytes.
> 
> The binutils ld sets sh_addralign to 1, which makes libelf
> fail with misaligned section error during the update as
> reported by Jesper:
> 
>     FAILED elf_update(WRITE): invalid section alignment
> 
> While waiting for ld fix, we can fix compressed sections
> sh_addralign value manually.
> 
> Adding warning in -vv mode when the fix is triggered:
> 
>    $ ./tools/bpf/resolve_btfids/resolve_btfids -vv vmlinux
>    ...
>    section(36) .comment, size 44, link 0, flags 30, type=1
>    section(37) .debug_aranges, size 45684, link 0, flags 800, type=1
>     - fixing wrong alignment sh_addralign 16, expected 8
>    section(38) .debug_info, size 129104957, link 0, flags 800, type=1
>     - fixing wrong alignment sh_addralign 1, expected 8
>    section(39) .debug_abbrev, size 1152583, link 0, flags 800, type=1
>     - fixing wrong alignment sh_addralign 1, expected 8
>    section(40) .debug_line, size 7374522, link 0, flags 800, type=1
>     - fixing wrong alignment sh_addralign 1, expected 8
>    section(41) .debug_frame, size 702463, link 0, flags 800, type=1
>    section(42) .debug_str, size 1017571, link 0, flags 830, type=1
>     - fixing wrong alignment sh_addralign 1, expected 8
>    section(43) .debug_loc, size 3019453, link 0, flags 800, type=1
>     - fixing wrong alignment sh_addralign 1, expected 8
>    section(44) .debug_ranges, size 1744583, link 0, flags 800, type=1
>     - fixing wrong alignment sh_addralign 16, expected 8
>    section(45) .symtab, size 2955888, link 46, flags 0, type=2
>    section(46) .strtab, size 2613072, link 0, flags 0, type=3
>    ...
>    update ok for vmlinux
> 
> Another workaround is to disable compressed debug info data
> CONFIG_DEBUG_INFO_COMPRESSED kernel option.
> 
> Fixes: fbbb68de80a4 ("bpf: Add resolve_btfids tool to resolve BTF IDs in ELF object")
> Cc: Mark Wielaard <mjw@redhat.com>
> Cc: Nick Clifton <nickc@redhat.com>
> Reported-by: Jesper Dangaard Brouer <brouer@redhat.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>

Acked-by: Yonghong Song <yhs@fb.com>
Fangrui Song Aug. 20, 2020, 2:27 a.m. UTC | #7
> > >    section(36) .comment, size 44, link 0, flags 30, type=1
> > >    section(37) .debug_aranges, size 45684, link 0, flags 800, type=1
> > >     - fixing wrong alignment sh_addralign 16, expected 8
> > >    section(38) .debug_info, size 129104957, link 0, flags 800, type=1
> > >     - fixing wrong alignment sh_addralign 1, expected 8
> > >    section(39) .debug_abbrev, size 1152583, link 0, flags 800, type=1
> > >     - fixing wrong alignment sh_addralign 1, expected 8
> > >    section(40) .debug_line, size 7374522, link 0, flags 800, type=1
> > >     - fixing wrong alignment sh_addralign 1, expected 8
> > >    section(41) .debug_frame, size 702463, link 0, flags 800, type=1
> > >    section(42) .debug_str, size 1017571, link 0, flags 830, type=1
> > >     - fixing wrong alignment sh_addralign 1, expected 8
> > >    section(43) .debug_loc, size 3019453, link 0, flags 800, type=1
> > >     - fixing wrong alignment sh_addralign 1, expected 8
> > >    section(44) .debug_ranges, size 1744583, link 0, flags 800, type=1
> > >     - fixing wrong alignment sh_addralign 16, expected 8
> > >    section(45) .symtab, size 2955888, link 46, flags 0, type=2
> > >    section(46) .strtab, size 2613072, link 0, flags 0, type=3

I think this is resolve_btfids's bug. GNU ld and LLD are innocent.
These .debug_* sections work fine if their sh_addralign is 1.
When the section flag SHF_COMPRESSED is set, the meaningful alignment
is Elf64_Chdr::ch_addralign, after the header is uncompressed.

On Wed, Aug 19, 2020 at 2:30 PM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 8/19/20 11:16 AM, Nick Desaulniers wrote:
> > On Wed, Aug 19, 2020 at 10:36 AM Jiri Olsa <jolsa@redhat.com> wrote:
> >>
> >> On Wed, Aug 19, 2020 at 08:31:51AM -0700, Yonghong Song wrote:
> >>>
> >>>
> >>> On 8/19/20 2:23 AM, Jiri Olsa wrote:
> >>>> The data of compressed section should be aligned to 4
> >>>> (for 32bit) or 8 (for 64 bit) bytes.
> >>>>
> >>>> The binutils ld sets sh_addralign to 1, which makes libelf
> >>>> fail with misaligned section error during the update as
> >>>> reported by Jesper:
> >>>>
> >>>>      FAILED elf_update(WRITE): invalid section alignment
> >>>>
> >>>> While waiting for ld fix, we can fix compressed sections
> >>>> sh_addralign value manually.
> >
> > Is there a bug filed against GNU ld? Link?
> >
> >>>>
> >>>> Adding warning in -vv mode when the fix is triggered:
> >>>>
> >>>>     $ ./tools/bpf/resolve_btfids/resolve_btfids -vv vmlinux
> >>>>     ...
> >>>>     section(36) .comment, size 44, link 0, flags 30, type=1
> >>>>     section(37) .debug_aranges, size 45684, link 0, flags 800, type=1
> >>>>      - fixing wrong alignment sh_addralign 16, expected 8
> >>>>     section(38) .debug_info, size 129104957, link 0, flags 800, type=1
> >>>>      - fixing wrong alignment sh_addralign 1, expected 8
> >>>>     section(39) .debug_abbrev, size 1152583, link 0, flags 800, type=1
> >>>>      - fixing wrong alignment sh_addralign 1, expected 8
> >>>>     section(40) .debug_line, size 7374522, link 0, flags 800, type=1
> >>>>      - fixing wrong alignment sh_addralign 1, expected 8
> >>>>     section(41) .debug_frame, size 702463, link 0, flags 800, type=1
> >>>>     section(42) .debug_str, size 1017571, link 0, flags 830, type=1
> >>>>      - fixing wrong alignment sh_addralign 1, expected 8
> >>>>     section(43) .debug_loc, size 3019453, link 0, flags 800, type=1
> >>>>      - fixing wrong alignment sh_addralign 1, expected 8
> >>>>     section(44) .debug_ranges, size 1744583, link 0, flags 800, type=1
> >>>>      - fixing wrong alignment sh_addralign 16, expected 8
> >>>>     section(45) .symtab, size 2955888, link 46, flags 0, type=2
> >>>>     section(46) .strtab, size 2613072, link 0, flags 0, type=3
> >>>>     ...
> >>>>     update ok for vmlinux
> >>>>
> >>>> Another workaround is to disable compressed debug info data
> >>>> CONFIG_DEBUG_INFO_COMPRESSED kernel option.
> >>>
> >>> So CONFIG_DEBUG_INFO_COMPRESSED is required to reproduce the bug, right?
> >>
> >> correct
> >>
> >>>
> >>> I turned on CONFIG_DEBUG_INFO_COMPRESSED in my config and got a bunch of
> >>> build failures.
> >>>
> >>> ld: drivers/crypto/virtio/virtio_crypto_algs.o: unable to initialize
> >>> decompress status for section .debug_info
> >>> ld: drivers/crypto/virtio/virtio_crypto_algs.o: unable to initialize
> >>> decompress status for section .debug_info
> >>> ld: drivers/crypto/virtio/virtio_crypto_algs.o: unable to initialize
> >>> decompress status for section .debug_info
> >>> ld: drivers/crypto/virtio/virtio_crypto_algs.o: unable to initialize
> >>> decompress status for section .debug_info
> >>> drivers/crypto/virtio/virtio_crypto_algs.o: file not recognized: File format
> >>> not recognized
> >>>
> >>> ld: net/llc/llc_core.o: unable to initialize decompress status for section
> >>> .debug_info
> >>> ld: net/llc/llc_core.o: unable to initialize decompress status for section
> >>> .debug_info
> >>> ld: net/llc/llc_core.o: unable to initialize decompress status for section
> >>> .debug_info
> >>> ld: net/llc/llc_core.o: unable to initialize decompress status for section
> >>> .debug_info
> >>> net/llc/llc_core.o: file not recognized: File format not recognized
> >>>
> >>> ...
> >>>
> >>> The 'ld' in my system:
> >>>
> >>> $ ld -V
> >>> GNU ld version 2.30-74.el8
> >>>    Supported emulations:
> >>>     elf_x86_64
> >>>     elf32_x86_64
> >>>     elf_i386
> >>>     elf_iamcu
> >>>     i386linux
> >>>     elf_l1om
> >>>     elf_k1om
> >>>     i386pep
> >>>     i386pe
> >
> > According to Documentation/process/changes.rst, the minimum supported
> > version of GNU binutils for the kernels is 2.23.  Can you upgrade to
> > that and confirm that you still observe the issue?  I don't want to
> > spend time chasing bugs in old, unsupported versions of GNU binutils,
> > especially as Jiri notes, 2.26 is required for
> > CONFIG_DEBUG_INFO_COMPRESSED.  We can always strengthen the Kconfig
> > check for it.  Otherwise, I'm not familiar with the observed error
> > message.
>
> I built a "ld" with latest binutils-gdb repo and I can reproduced
> the issue. Indeed applying the patch here fixed the issue. So
> I think there is no need to investigate since upstream exhibits
> the exact issue described here.
>
> >
> >>> $
> >>>
> >>> Do you know what is the issue here?
> >>
> >> mine's: GNU ld version 2.32-31.fc31
> >>
> >> there's version info in commit:
> >>    10e68b02c861 Makefile: support compressed debug info
> >>
> >>    Compress the debug information using zlib.  Requires GCC 5.0+ or Clang
> >>    5.0+, binutils 2.26+, and zlib.
> >>
> >> cc-ing Nick Desaulniers, author of that patch.. any idea about the error above?
> >>
> >> thanks,
> >> jirka
> >>
> >
> >
Yonghong Song Aug. 20, 2020, 3:23 a.m. UTC | #8
On 8/19/20 7:27 PM, Fāng-ruì Sòng wrote:
>>>>     section(36) .comment, size 44, link 0, flags 30, type=1
>>>>     section(37) .debug_aranges, size 45684, link 0, flags 800, type=1
>>>>      - fixing wrong alignment sh_addralign 16, expected 8
>>>>     section(38) .debug_info, size 129104957, link 0, flags 800, type=1
>>>>      - fixing wrong alignment sh_addralign 1, expected 8
>>>>     section(39) .debug_abbrev, size 1152583, link 0, flags 800, type=1
>>>>      - fixing wrong alignment sh_addralign 1, expected 8
>>>>     section(40) .debug_line, size 7374522, link 0, flags 800, type=1
>>>>      - fixing wrong alignment sh_addralign 1, expected 8
>>>>     section(41) .debug_frame, size 702463, link 0, flags 800, type=1
>>>>     section(42) .debug_str, size 1017571, link 0, flags 830, type=1
>>>>      - fixing wrong alignment sh_addralign 1, expected 8
>>>>     section(43) .debug_loc, size 3019453, link 0, flags 800, type=1
>>>>      - fixing wrong alignment sh_addralign 1, expected 8
>>>>     section(44) .debug_ranges, size 1744583, link 0, flags 800, type=1
>>>>      - fixing wrong alignment sh_addralign 16, expected 8
>>>>     section(45) .symtab, size 2955888, link 46, flags 0, type=2
>>>>     section(46) .strtab, size 2613072, link 0, flags 0, type=3
> 
> I think this is resolve_btfids's bug. GNU ld and LLD are innocent.
> These .debug_* sections work fine if their sh_addralign is 1.
> When the section flag SHF_COMPRESSED is set, the meaningful alignment
> is Elf64_Chdr::ch_addralign, after the header is uncompressed.
> 
> On Wed, Aug 19, 2020 at 2:30 PM Yonghong Song <yhs@fb.com> wrote:
>>
>>
>>
>> On 8/19/20 11:16 AM, Nick Desaulniers wrote:
>>> On Wed, Aug 19, 2020 at 10:36 AM Jiri Olsa <jolsa@redhat.com> wrote:
>>>>
>>>> On Wed, Aug 19, 2020 at 08:31:51AM -0700, Yonghong Song wrote:
>>>>>
>>>>>
>>>>> On 8/19/20 2:23 AM, Jiri Olsa wrote:
>>>>>> The data of compressed section should be aligned to 4
>>>>>> (for 32bit) or 8 (for 64 bit) bytes.
>>>>>>
>>>>>> The binutils ld sets sh_addralign to 1, which makes libelf
>>>>>> fail with misaligned section error during the update as
>>>>>> reported by Jesper:
>>>>>>
>>>>>>       FAILED elf_update(WRITE): invalid section alignment

Jiri,

Since Fangrui mentioned this is not a ld/lld bug, then changing
alighment from 1 to 4 might have some adverse effect for the binary,
I guess.

Do you think we could skip these .debug_* sections somehow in elf 
parsing in resolve_btfids? resolve_btfids does not need to read
these sections. This way, no need to change their alignment either.

Yonghong

>>>>>>
>>>>>> While waiting for ld fix, we can fix compressed sections
>>>>>> sh_addralign value manually.
>>>
>>> Is there a bug filed against GNU ld? Link?
>>>
>>>>>>
>>>>>> Adding warning in -vv mode when the fix is triggered:
>>>>>>
>>>>>>      $ ./tools/bpf/resolve_btfids/resolve_btfids -vv vmlinux
>>>>>>      ...
>>>>>>      section(36) .comment, size 44, link 0, flags 30, type=1
>>>>>>      section(37) .debug_aranges, size 45684, link 0, flags 800, type=1
>>>>>>       - fixing wrong alignment sh_addralign 16, expected 8
>>>>>>      section(38) .debug_info, size 129104957, link 0, flags 800, type=1
>>>>>>       - fixing wrong alignment sh_addralign 1, expected 8
>>>>>>      section(39) .debug_abbrev, size 1152583, link 0, flags 800, type=1
>>>>>>       - fixing wrong alignment sh_addralign 1, expected 8
>>>>>>      section(40) .debug_line, size 7374522, link 0, flags 800, type=1
>>>>>>       - fixing wrong alignment sh_addralign 1, expected 8
>>>>>>      section(41) .debug_frame, size 702463, link 0, flags 800, type=1
>>>>>>      section(42) .debug_str, size 1017571, link 0, flags 830, type=1
>>>>>>       - fixing wrong alignment sh_addralign 1, expected 8
>>>>>>      section(43) .debug_loc, size 3019453, link 0, flags 800, type=1
>>>>>>       - fixing wrong alignment sh_addralign 1, expected 8
>>>>>>      section(44) .debug_ranges, size 1744583, link 0, flags 800, type=1
>>>>>>       - fixing wrong alignment sh_addralign 16, expected 8
>>>>>>      section(45) .symtab, size 2955888, link 46, flags 0, type=2
>>>>>>      section(46) .strtab, size 2613072, link 0, flags 0, type=3
>>>>>>      ...
>>>>>>      update ok for vmlinux
>>>>>>
>>>>>> Another workaround is to disable compressed debug info data
>>>>>> CONFIG_DEBUG_INFO_COMPRESSED kernel option.
>>>>>
>>>>> So CONFIG_DEBUG_INFO_COMPRESSED is required to reproduce the bug, right?
>>>>
>>>> correct
>>>>
>>>>>
>>>>> I turned on CONFIG_DEBUG_INFO_COMPRESSED in my config and got a bunch of
>>>>> build failures.
>>>>>
>>>>> ld: drivers/crypto/virtio/virtio_crypto_algs.o: unable to initialize
>>>>> decompress status for section .debug_info
>>>>> ld: drivers/crypto/virtio/virtio_crypto_algs.o: unable to initialize
>>>>> decompress status for section .debug_info
>>>>> ld: drivers/crypto/virtio/virtio_crypto_algs.o: unable to initialize
>>>>> decompress status for section .debug_info
>>>>> ld: drivers/crypto/virtio/virtio_crypto_algs.o: unable to initialize
>>>>> decompress status for section .debug_info
>>>>> drivers/crypto/virtio/virtio_crypto_algs.o: file not recognized: File format
>>>>> not recognized
>>>>>
>>>>> ld: net/llc/llc_core.o: unable to initialize decompress status for section
>>>>> .debug_info
>>>>> ld: net/llc/llc_core.o: unable to initialize decompress status for section
>>>>> .debug_info
>>>>> ld: net/llc/llc_core.o: unable to initialize decompress status for section
>>>>> .debug_info
>>>>> ld: net/llc/llc_core.o: unable to initialize decompress status for section
>>>>> .debug_info
>>>>> net/llc/llc_core.o: file not recognized: File format not recognized
>>>>>
>>>>> ...
>>>>>
>>>>> The 'ld' in my system:
>>>>>
>>>>> $ ld -V
>>>>> GNU ld version 2.30-74.el8
>>>>>     Supported emulations:
>>>>>      elf_x86_64
>>>>>      elf32_x86_64
>>>>>      elf_i386
>>>>>      elf_iamcu
>>>>>      i386linux
>>>>>      elf_l1om
>>>>>      elf_k1om
>>>>>      i386pep
>>>>>      i386pe
>>>
>>> According to Documentation/process/changes.rst, the minimum supported
>>> version of GNU binutils for the kernels is 2.23.  Can you upgrade to
>>> that and confirm that you still observe the issue?  I don't want to
>>> spend time chasing bugs in old, unsupported versions of GNU binutils,
>>> especially as Jiri notes, 2.26 is required for
>>> CONFIG_DEBUG_INFO_COMPRESSED.  We can always strengthen the Kconfig
>>> check for it.  Otherwise, I'm not familiar with the observed error
>>> message.
>>
>> I built a "ld" with latest binutils-gdb repo and I can reproduced
>> the issue. Indeed applying the patch here fixed the issue. So
>> I think there is no need to investigate since upstream exhibits
>> the exact issue described here.
>>
>>>
>>>>> $
>>>>>
>>>>> Do you know what is the issue here?
>>>>
>>>> mine's: GNU ld version 2.32-31.fc31
>>>>
>>>> there's version info in commit:
>>>>     10e68b02c861 Makefile: support compressed debug info
>>>>
>>>>     Compress the debug information using zlib.  Requires GCC 5.0+ or Clang
>>>>     5.0+, binutils 2.26+, and zlib.
>>>>
>>>> cc-ing Nick Desaulniers, author of that patch.. any idea about the error above?
>>>>
>>>> thanks,
>>>> jirka
>>>>
>>>
>>>
> 
> 
>
Jiri Olsa Aug. 20, 2020, 10:18 a.m. UTC | #9
On Wed, Aug 19, 2020 at 08:23:10PM -0700, Yonghong Song wrote:
> 
> 
> On 8/19/20 7:27 PM, Fāng-ruì Sòng wrote:
> > > > >     section(36) .comment, size 44, link 0, flags 30, type=1
> > > > >     section(37) .debug_aranges, size 45684, link 0, flags 800, type=1
> > > > >      - fixing wrong alignment sh_addralign 16, expected 8
> > > > >     section(38) .debug_info, size 129104957, link 0, flags 800, type=1
> > > > >      - fixing wrong alignment sh_addralign 1, expected 8
> > > > >     section(39) .debug_abbrev, size 1152583, link 0, flags 800, type=1
> > > > >      - fixing wrong alignment sh_addralign 1, expected 8
> > > > >     section(40) .debug_line, size 7374522, link 0, flags 800, type=1
> > > > >      - fixing wrong alignment sh_addralign 1, expected 8
> > > > >     section(41) .debug_frame, size 702463, link 0, flags 800, type=1
> > > > >     section(42) .debug_str, size 1017571, link 0, flags 830, type=1
> > > > >      - fixing wrong alignment sh_addralign 1, expected 8
> > > > >     section(43) .debug_loc, size 3019453, link 0, flags 800, type=1
> > > > >      - fixing wrong alignment sh_addralign 1, expected 8
> > > > >     section(44) .debug_ranges, size 1744583, link 0, flags 800, type=1
> > > > >      - fixing wrong alignment sh_addralign 16, expected 8
> > > > >     section(45) .symtab, size 2955888, link 46, flags 0, type=2
> > > > >     section(46) .strtab, size 2613072, link 0, flags 0, type=3
> > 
> > I think this is resolve_btfids's bug. GNU ld and LLD are innocent.
> > These .debug_* sections work fine if their sh_addralign is 1.
> > When the section flag SHF_COMPRESSED is set, the meaningful alignment
> > is Elf64_Chdr::ch_addralign, after the header is uncompressed.
> > 
> > On Wed, Aug 19, 2020 at 2:30 PM Yonghong Song <yhs@fb.com> wrote:
> > > 
> > > 
> > > 
> > > On 8/19/20 11:16 AM, Nick Desaulniers wrote:
> > > > On Wed, Aug 19, 2020 at 10:36 AM Jiri Olsa <jolsa@redhat.com> wrote:
> > > > > 
> > > > > On Wed, Aug 19, 2020 at 08:31:51AM -0700, Yonghong Song wrote:
> > > > > > 
> > > > > > 
> > > > > > On 8/19/20 2:23 AM, Jiri Olsa wrote:
> > > > > > > The data of compressed section should be aligned to 4
> > > > > > > (for 32bit) or 8 (for 64 bit) bytes.
> > > > > > > 
> > > > > > > The binutils ld sets sh_addralign to 1, which makes libelf
> > > > > > > fail with misaligned section error during the update as
> > > > > > > reported by Jesper:
> > > > > > > 
> > > > > > >       FAILED elf_update(WRITE): invalid section alignment
> 
> Jiri,
> 
> Since Fangrui mentioned this is not a ld/lld bug, then changing
> alighment from 1 to 4 might have some adverse effect for the binary,
> I guess.

not sure about that.. Mark? ;-)

> 
> Do you think we could skip these .debug_* sections somehow in elf parsing in
> resolve_btfids? resolve_btfids does not need to read
> these sections. This way, no need to change their alignment either.

I'm don't think libelf interface allows for that, will check

jirka
Mark Wielaard Aug. 20, 2020, 10:18 a.m. UTC | #10
Hi,

On Wed, 2020-08-19 at 20:23 -0700, Yonghong Song wrote:
> On 8/19/20 7:27 PM, Fāng-ruì Sòng wrote:
> > > > > 
> > I think this is resolve_btfids's bug. GNU ld and LLD are innocent.
> > These .debug_* sections work fine if their sh_addralign is 1.
> > When the section flag SHF_COMPRESSED is set, the meaningful
> > alignment
> > is Elf64_Chdr::ch_addralign, after the header is uncompressed.
> > 
> > On Wed, Aug 19, 2020 at 2:30 PM Yonghong Song <yhs@fb.com> wrote:
> Since Fangrui mentioned this is not a ld/lld bug, then changing
> alighment from 1 to 4 might have some adverse effect for the binary,
> I guess.

The bug isn't about a wrong ch_addralign, which seems to have been set
correctly. But it is a bug about incorrectly setting the sh_addralign
of the section. The sh_addralign indicates the alignment of the data in
the section, which is the Elf32/64_Chdr plus compressed data, not the
alignment of the uncompressed data. It helps the consumer make sure
they lay out the data so that the ELF data structures can be read
through their natural alignment.

In practice it often isn't a real issue, because consumers, including
libelf, will correct the data alignment before usage anyway. But that
doesn't mean it isn't a bug to set it wrongly.

> Do you think we could skip these .debug_* sections somehow in elf 
> parsing in resolve_btfids? resolve_btfids does not need to read
> these sections. This way, no need to change their alignment either.

The issue is that elfutils libelf will not allow writing out the
section when it notices the sh_addralign field is setup wrongly.

Cheers,

Mark
Yonghong Song Aug. 20, 2020, 3:51 p.m. UTC | #11
On 8/20/20 3:18 AM, Mark Wielaard wrote:
> Hi,
> 
> On Wed, 2020-08-19 at 20:23 -0700, Yonghong Song wrote:
>> On 8/19/20 7:27 PM, Fāng-ruì Sòng wrote:
>>>>>>
>>> I think this is resolve_btfids's bug. GNU ld and LLD are innocent.
>>> These .debug_* sections work fine if their sh_addralign is 1.
>>> When the section flag SHF_COMPRESSED is set, the meaningful
>>> alignment
>>> is Elf64_Chdr::ch_addralign, after the header is uncompressed.
>>>
>>> On Wed, Aug 19, 2020 at 2:30 PM Yonghong Song <yhs@fb.com> wrote:
>> Since Fangrui mentioned this is not a ld/lld bug, then changing
>> alighment from 1 to 4 might have some adverse effect for the binary,
>> I guess.
> 
> The bug isn't about a wrong ch_addralign, which seems to have been set
> correctly. But it is a bug about incorrectly setting the sh_addralign
> of the section. The sh_addralign indicates the alignment of the data in
> the section, which is the Elf32/64_Chdr plus compressed data, not the
> alignment of the uncompressed data. It helps the consumer make sure
> they lay out the data so that the ELF data structures can be read
> through their natural alignment.
> 
> In practice it often isn't a real issue, because consumers, including
> libelf, will correct the data alignment before usage anyway. But that
> doesn't mean it isn't a bug to set it wrongly.
> 
>> Do you think we could skip these .debug_* sections somehow in elf
>> parsing in resolve_btfids? resolve_btfids does not need to read
>> these sections. This way, no need to change their alignment either.
> 
> The issue is that elfutils libelf will not allow writing out the
> section when it notices the sh_addralign field is setup wrongly.

Maybe resolve_btfids can temporarily change sh_addralign to 4/8
before elf manipulation (elf_write) to make libelf happy.
After all elf_write is done, change back to whatever the
original value (1). Does this work?

> 
> Cheers,
> 
> Mark
>
Mark Wielaard Aug. 20, 2020, 5:36 p.m. UTC | #12
Hi

On Thu, 2020-08-20 at 08:51 -0700, Yonghong Song wrote:
> > > Do you think we could skip these .debug_* sections somehow in elf
> > > parsing in resolve_btfids? resolve_btfids does not need to read
> > > these sections. This way, no need to change their alignment
> > > either.
> > 
> > The issue is that elfutils libelf will not allow writing out the
> > section when it notices the sh_addralign field is setup wrongly.
> 
> Maybe resolve_btfids can temporarily change sh_addralign to 4/8
> before elf manipulation (elf_write) to make libelf happy.
> After all elf_write is done, change back to whatever the
> original value (1). Does this work?

Unfortunately no, because there is no elf_write, elf_update is how you
write out the ELF image to disc.

Since the code is using ELF_F_LAYOUT this will not change the actual
layout of the ELF image if that is what you are worried about.

And the workaround to set sh_addralign correctly before calling
elf_update is precisely what the fix in elfutils libelf will do itself
in the next release. Also binutils ld has been fixed to setup
sh_addralign to 4/8 as appropriate now (in git).

Cheers,

Mark
Yonghong Song Aug. 20, 2020, 5:54 p.m. UTC | #13
On 8/20/20 10:36 AM, Mark Wielaard wrote:
> Hi
> 
> On Thu, 2020-08-20 at 08:51 -0700, Yonghong Song wrote:
>>>> Do you think we could skip these .debug_* sections somehow in elf
>>>> parsing in resolve_btfids? resolve_btfids does not need to read
>>>> these sections. This way, no need to change their alignment
>>>> either.
>>>
>>> The issue is that elfutils libelf will not allow writing out the
>>> section when it notices the sh_addralign field is setup wrongly.
>>
>> Maybe resolve_btfids can temporarily change sh_addralign to 4/8
>> before elf manipulation (elf_write) to make libelf happy.
>> After all elf_write is done, change back to whatever the
>> original value (1). Does this work?
> 
> Unfortunately no, because there is no elf_write, elf_update is how you
> write out the ELF image to disc.
> 
> Since the code is using ELF_F_LAYOUT this will not change the actual
> layout of the ELF image if that is what you are worried about.
> 
> And the workaround to set sh_addralign correctly before calling
> elf_update is precisely what the fix in elfutils libelf will do itself
> in the next release. Also binutils ld has been fixed to setup
> sh_addralign to 4/8 as appropriate now (in git).

Sounds good then.
Thanks for fixing the issue in upstream, both libelf and binutils!

> 
> Cheers,
> 
> Mark
>
Alexei Starovoitov Aug. 20, 2020, 9:24 p.m. UTC | #14
On Thu, Aug 20, 2020 at 10:54 AM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 8/20/20 10:36 AM, Mark Wielaard wrote:
> > Hi
> >
> > On Thu, 2020-08-20 at 08:51 -0700, Yonghong Song wrote:
> >>>> Do you think we could skip these .debug_* sections somehow in elf
> >>>> parsing in resolve_btfids? resolve_btfids does not need to read
> >>>> these sections. This way, no need to change their alignment
> >>>> either.
> >>>
> >>> The issue is that elfutils libelf will not allow writing out the
> >>> section when it notices the sh_addralign field is setup wrongly.
> >>
> >> Maybe resolve_btfids can temporarily change sh_addralign to 4/8
> >> before elf manipulation (elf_write) to make libelf happy.
> >> After all elf_write is done, change back to whatever the
> >> original value (1). Does this work?
> >
> > Unfortunately no, because there is no elf_write, elf_update is how you
> > write out the ELF image to disc.
> >
> > Since the code is using ELF_F_LAYOUT this will not change the actual
> > layout of the ELF image if that is what you are worried about.
> >
> > And the workaround to set sh_addralign correctly before calling
> > elf_update is precisely what the fix in elfutils libelf will do itself
> > in the next release. Also binutils ld has been fixed to setup
> > sh_addralign to 4/8 as appropriate now (in git).
>
> Sounds good then.
> Thanks for fixing the issue in upstream, both libelf and binutils!

In the meantime I've applied Jiri's workaround to bpf tree. Thanks!
diff mbox series

Patch

diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c
index 4d9ecb975862..0def0bb1f783 100644
--- a/tools/bpf/resolve_btfids/main.c
+++ b/tools/bpf/resolve_btfids/main.c
@@ -233,6 +233,39 @@  static struct btf_id *add_symbol(struct rb_root *root, char *name, size_t size)
 	return btf_id__add(root, id, false);
 }
 
+/*
+ * The data of compressed section should be aligned to 4
+ * (for 32bit) or 8 (for 64 bit) bytes. The binutils ld
+ * sets sh_addralign to 1, which makes libelf fail with
+ * misaligned section error during the update:
+ *    FAILED elf_update(WRITE): invalid section alignment
+ *
+ * While waiting for ld fix, we fix the compressed sections
+ * sh_addralign value manualy.
+ */
+static int compressed_section_fix(Elf *elf, Elf_Scn *scn, GElf_Shdr *sh)
+{
+	int expected = gelf_getclass(elf) == ELFCLASS32 ? 4 : 8;
+
+	if (!(sh->sh_flags & SHF_COMPRESSED))
+		return 0;
+
+	if (sh->sh_addralign == expected)
+		return 0;
+
+	pr_debug2(" - fixing wrong alignment sh_addralign %u, expected %u\n",
+		  sh->sh_addralign, expected);
+
+	sh->sh_addralign = expected;
+
+	if (gelf_update_shdr(scn, sh) == 0) {
+		printf("FAILED cannot update section header: %s\n",
+			elf_errmsg(-1));
+		return -1;
+	}
+	return 0;
+}
+
 static int elf_collect(struct object *obj)
 {
 	Elf_Scn *scn = NULL;
@@ -309,6 +342,9 @@  static int elf_collect(struct object *obj)
 			obj->efile.idlist_shndx = idx;
 			obj->efile.idlist_addr  = sh.sh_addr;
 		}
+
+		if (compressed_section_fix(elf, scn, &sh))
+			return -1;
 	}
 
 	return 0;