Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Mark Wielaard <mark@klomp.org>
To: Jiri Olsa <jolsa@redhat.com>, Nick Clifton <nickc@redhat.com>
Cc: Jesper Dangaard Brouer <brouer@redhat.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	sdf@google.com, andriin@fb.com
Subject: Re: Kernel build error on BTFIDS vmlinux
Date: Thu, 20 Aug 2020 00:00:09 +0200	[thread overview]
Message-ID: <f03e0fec4b29afe24a7a13c43de23e6db6dfce23.camel@klomp.org> (raw)
In-Reply-To: <20200819171820.GG177896@krava>

[-- Attachment #1: Type: text/plain, Size: 3992 bytes --]

Hi,

On Wed, 2020-08-19 at 19:18 +0200, Jiri Olsa wrote:
> On Wed, Aug 19, 2020 at 04:34:30PM +0100, Nick Clifton wrote:
> > > So it would be nice if binutils ld could also be fixed to write out
> > > compressed sections with the correct alignment.
> > 
> > OK, I will look into doing this.
> > 
> > By any chance is there a small test case that you are using to check
> > this behaviour ?   If so, please may I have a copy for myself ?
> 
> so when I take empty object and compile like:
> 
>   $ echo 'int main(int argc, char **argv) { return 0; }' | gcc -c -o ex.o -g -gz=zlib -x c -
>   $ ld -o ex --compress-debug-sections=zlib ex.o
> 
> then there's .debug_info section that shows sh_addralign = 1

Specifically, if you extend the example code a bit so that it has a
couple more interesting compressed .debug sections (like in an vmlinux
image) you'll see, eu-readelf -Sz:

Section Headers:
[Nr] Name                 Type         Addr             Off      Size     ES Flags Lk Inf Al
     [Compression  Size     Al]

[37] .debug_aranges       PROGBITS     0000000000000000 027ae9f0 0000b274  0 C      0   0 16
     [ELF ZLIB (1) 00028030 16]
[38] .debug_info          PROGBITS     0000000000000000 027b9c64 07b1fc3d  0 C      0   0  1
     [ELF ZLIB (1) 0cb137ad  1]
[39] .debug_abbrev        PROGBITS     0000000000000000 0a2d98a1 00119647  0 C      0   0  1
     [ELF ZLIB (1) 0060811f  1]
[40] .debug_line          PROGBITS     0000000000000000 0a3f2ee8 007086ba  0 C      0   0  1
     [ELF ZLIB (1) 01557659  1]
[41] .debug_frame         PROGBITS     0000000000000000 0aafb5a8 000ab7ff  0 C      0   0  8
     [ELF ZLIB (1) 002a6bf8  8]
[42] .debug_str           PROGBITS     0000000000000000 0aba6da7 000f86e3  1 MSC    0   0  1
     [ELF ZLIB (1) 003a8a8e  1]
[43] .debug_loc           PROGBITS     0000000000000000 0ac9f48a 002e12bd  0 C      0   0  1
     [ELF ZLIB (1) 00e0c448  1]
[44] .debug_ranges        PROGBITS     0000000000000000 0af80750 001a9ec7  0 C      0   0 16
     [ELF ZLIB (1) 00e84b20 16]

Note that the sh_addralign of the sections is set to the same valie as
ch_addralign. That is the alignment of the decompressed data, what
sh_addralign would have been if it wasn't a compressed section.

The sh_addralign of a compressed section however should be equal to
alignment for the datastructure inside it, either 4, for 32bit:

typedef struct
{
  Elf32_Word    ch_type;        /* Compression format.  */
  Elf32_Word    ch_size;        /* Uncompressed data size.  */
  Elf32_Word    ch_addralign;   /* Uncompressed data alignment.  */
} Elf32_Chdr;

or 8, for 64bit:

typedef struct
{
  Elf64_Word    ch_type;        /* Compression format.  */
  Elf64_Word    ch_reserved;
  Elf64_Xword   ch_size;        /* Uncompressed data size.  */
  Elf64_Xword   ch_addralign;   /* Uncompressed data alignment.  */
} Elf64_Chdr;

At least, that is what elfutils libelf expects. And which I believe is
what the ELF spec implies when it says:

   The sh_size and sh_addralign fields of the section header for a
   compressed section reflect the requirements of the compressed
   section.  The ch_size and ch_addralign fields in the compression
   header provide the corresponding values for the uncompressed data,
   thereby supplying the values that sh_size and sh_addralign would
   have had if the section had not been compressed.

> after I open the 'ex' obejct with elf_begin and iterate sections
> 
> according to Mark that should be 8 (on 64 bits)
> 
> when I change it to 8, the elf_update call won't fail for me
> on that elf file

Right, I have a patch that fixes it up in libelf, see attached.
That should make things work without needing a workaround. But of
course I just posted it and it isn't even upstream yet. So for now the
workaround will be needed and it would be nice if binutils ld could
also be fixed to set the sh_addralign field correctly.

Cheers,

Mark

[-- Attachment #2: 0001-libelf-Fixup-SHF_COMPRESSED-sh_addralign-in-elf_upda.patch --]
[-- Type: text/x-patch, Size: 2712 bytes --]

From 55c5c9a568ed707bcea1388bf3a525212d8cf4b8 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mark@klomp.org>
Date: Wed, 19 Aug 2020 23:41:24 +0200
Subject: [PATCH] libelf: Fixup SHF_COMPRESSED sh_addralign in elf_update if
 necessary.

In elf_getdata.c we have the following to compensate for possibly
bad sh_addralign values of compressed sections:

      /* Compressed data has a header, but then compressed data.
         Make sure to set the alignment of the header explicitly,
         don't trust the file alignment for the section, it is
         often wrong.  */
      if ((flags & SHF_COMPRESSED) != 0)
        {
          entsize = 1;
          align = __libelf_type_align (elf->class, ELF_T_CHDR);
        }

Which makes sure the d_data alignment is correct for the Chdr struct
at the start of the compressed section.

But this means that if a user just reads such a compressed section
without changing it, and then tries to write it out again using
elf_update they get an error message about d_align and sh_addralign
being out of sync.

We already correct obviously incorrect sh_entsize fields.
Do the same for the sh_addralign field of a SHF_COMPRESSED section.

Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 libelf/ChangeLog          |  5 +++++
 libelf/elf32_updatenull.c | 12 ++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/libelf/ChangeLog b/libelf/ChangeLog
index 8f6d2d2d..77044c1c 100644
--- a/libelf/ChangeLog
+++ b/libelf/ChangeLog
@@ -1,3 +1,8 @@
+2020-08-19  Mark Wielaard  <mark@klomp.org>
+
+	* elf32_updatenull.c (updatenull_wrlock): Fixup the sh_addralign
+	of an SHF_COMPRESSED section if necessary.
+
 2020-06-04  Mark Wielaard  <mark@klomp.org>
 
 	* elf.h: Update from glibc.
diff --git a/libelf/elf32_updatenull.c b/libelf/elf32_updatenull.c
index 5f3cdbf6..d0d4d1eb 100644
--- a/libelf/elf32_updatenull.c
+++ b/libelf/elf32_updatenull.c
@@ -267,6 +267,18 @@ __elfw2(LIBELFBITS,updatenull_wrlock) (Elf *elf, int *change_bop, size_t shnum)
 	      update_if_changed (shdr->sh_entsize, sh_entsize,
 				 scn->shdr_flags);
 
+	      /* Likewise for the alignment of a compressed section.
+	         For a SHF_COMPRESSED section set the correct
+	         sh_addralign value, which must match the d_align of
+	         the data (see __libelf_set_rawdata in elf_getdata.c).  */
+	      if ((shdr->sh_flags & SHF_COMPRESSED) != 0)
+		{
+		  sh_align = __libelf_type_align (ELFW(ELFCLASS,LIBELFBITS),
+						  ELF_T_CHDR);
+		  update_if_changed (shdr->sh_addralign, sh_align,
+				     scn->shdr_flags);
+		}
+
 	      if (scn->data_read == 0
 		  && __libelf_set_rawdata_wrlock (scn) != 0)
 		/* Something went wrong.  The error value is already set.  */
-- 
2.18.4


  reply	other threads:[~2020-08-19 22:00 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-18  8:55 Jesper Dangaard Brouer
2020-08-18  9:14 ` Jiri Olsa
2020-08-18 10:56   ` Jiri Olsa
2020-08-18 13:45     ` Jiri Olsa
2020-08-18 16:33       ` Jesper Dangaard Brouer
2020-08-18 17:29         ` Mark Wielaard
2020-08-19 15:34           ` Nick Clifton
2020-08-19 17:18             ` Jiri Olsa
2020-08-19 22:00               ` Mark Wielaard [this message]
2020-08-20 12:14                 ` Nick Clifton

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=f03e0fec4b29afe24a7a13c43de23e6db6dfce23.camel@klomp.org \
    --to=mark@klomp.org \
    --cc=andriin@fb.com \
    --cc=brouer@redhat.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=nickc@redhat.com \
    --cc=sdf@google.com \
    --subject='Re: Kernel build error on BTFIDS vmlinux' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).