[PATCH] objtool: fix objtool regression on x32 systems

[PATCH] objtool: fix objtool regression on x32 systems

[Mikulas Patocka]

The patch 4abff6d48dbc ("objtool: Fix code relocs vs weak symbols") makes
the kernel unbootable.  The patch c087c6e7b551 ("objtool: Fix type of
reloc::addend") attempts to fix it by replacing 'int' with 'long'.

However, we may be running on a system with x32 ABI and 'long' on x32 is
32-bit, thus the patch c087c6e7b551 doesn't really change anything and we
still end up with miscompiled kernel.  This patch replaces 'long' with
'long long', so that the 64-bit kernel is correctly compiled on a x32
system.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Fixes: 4abff6d48dbc ("objtool: Fix code relocs vs weak symbols")
Fixes: c087c6e7b551 ("objtool: Fix type of reloc::addend")

---
 tools/objtool/check.c               |    8 ++++----
 tools/objtool/elf.c                 |    2 +-
 tools/objtool/include/objtool/elf.h |    4 ++--
 3 files changed, 7 insertions(+), 7 deletions(-)

Index: linux-2.6/tools/objtool/check.c
===================================================================
--- linux-2.6.orig/tools/objtool/check.c	2022-05-16 16:20:49.000000000 +0200
+++ linux-2.6/tools/objtool/check.c	2022-05-16 16:22:11.000000000 +0200
@@ -560,12 +560,12 @@ static int add_dead_ends(struct objtool_
 		else if (reloc->addend == reloc->sym->sec->sh.sh_size) {
 			insn = find_last_insn(file, reloc->sym->sec);
 			if (!insn) {
-				WARN("can't find unreachable insn at %s+0x%lx",
+				WARN("can't find unreachable insn at %s+0x%llx",
 				     reloc->sym->sec->name, reloc->addend);
 				return -1;
 			}
 		} else {
-			WARN("can't find unreachable insn at %s+0x%lx",
+			WARN("can't find unreachable insn at %s+0x%llx",
 			     reloc->sym->sec->name, reloc->addend);
 			return -1;
 		}
@@ -595,12 +595,12 @@ reachable:
 		else if (reloc->addend == reloc->sym->sec->sh.sh_size) {
 			insn = find_last_insn(file, reloc->sym->sec);
 			if (!insn) {
-				WARN("can't find reachable insn at %s+0x%lx",
+				WARN("can't find reachable insn at %s+0x%llx",
 				     reloc->sym->sec->name, reloc->addend);
 				return -1;
 			}
 		} else {
-			WARN("can't find reachable insn at %s+0x%lx",
+			WARN("can't find reachable insn at %s+0x%llx",
 			     reloc->sym->sec->name, reloc->addend);
 			return -1;
 		}
Index: linux-2.6/tools/objtool/elf.c
===================================================================
--- linux-2.6.orig/tools/objtool/elf.c	2022-05-16 16:20:49.000000000 +0200
+++ linux-2.6/tools/objtool/elf.c	2022-05-16 16:22:21.000000000 +0200
@@ -546,7 +546,7 @@ static struct section *elf_create_reloc_
 						int reltype);
 
 int elf_add_reloc(struct elf *elf, struct section *sec, unsigned long offset,
-		  unsigned int type, struct symbol *sym, long addend)
+		  unsigned int type, struct symbol *sym, long long addend)
 {
 	struct reloc *reloc;
 
Index: linux-2.6/tools/objtool/include/objtool/elf.h
===================================================================
--- linux-2.6.orig/tools/objtool/include/objtool/elf.h	2022-05-16 16:07:54.000000000 +0200
+++ linux-2.6/tools/objtool/include/objtool/elf.h	2022-05-16 16:22:45.000000000 +0200
@@ -73,7 +73,7 @@ struct reloc {
 	struct symbol *sym;
 	unsigned long offset;
 	unsigned int type;
-	long addend;
+	long long addend;
 	int idx;
 	bool jump_table_start;
 };
@@ -135,7 +135,7 @@ struct elf *elf_open_read(const char *na
 struct section *elf_create_section(struct elf *elf, const char *name, unsigned int sh_flags, size_t entsize, int nr);
 
 int elf_add_reloc(struct elf *elf, struct section *sec, unsigned long offset,
-		  unsigned int type, struct symbol *sym, long addend);
+		  unsigned int type, struct symbol *sym, long long addend);
 int elf_add_reloc_to_insn(struct elf *elf, struct section *sec,
 			  unsigned long offset, unsigned int type,
 			  struct section *insn_sec, unsigned long insn_off);

Re: [PATCH] objtool: fix objtool regression on x32 systems

[Peter Zijlstra]

On Mon, May 16, 2022 at 11:06:36AM -0400, Mikulas Patocka wrote:
> The patch 4abff6d48dbc ("objtool: Fix code relocs vs weak symbols") makes
> the kernel unbootable.  The patch c087c6e7b551 ("objtool: Fix type of
> reloc::addend") attempts to fix it by replacing 'int' with 'long'.
> 
> However, we may be running on a system with x32 ABI and 'long' on x32 is
> 32-bit, thus the patch c087c6e7b551 doesn't really change anything and we
> still end up with miscompiled kernel.  This patch replaces 'long' with
> 'long long', so that the 64-bit kernel is correctly compiled on a x32
> system.

Hurmph.. you're building a 64bit kernel on a x32 hosted machine? And
this is the *only* thing that goes sideways?

I suspect quite a bit of objtool assumes LP64 and won't quite work right
on ILP32 and we've just been lucky so far.

> Index: linux-2.6/tools/objtool/elf.c
> ===================================================================
> --- linux-2.6.orig/tools/objtool/elf.c	2022-05-16 16:20:49.000000000 +0200
> +++ linux-2.6/tools/objtool/elf.c	2022-05-16 16:22:21.000000000 +0200
> @@ -546,7 +546,7 @@ static struct section *elf_create_reloc_
>  						int reltype);
>  
>  int elf_add_reloc(struct elf *elf, struct section *sec, unsigned long offset,
> -		  unsigned int type, struct symbol *sym, long addend)
> +		  unsigned int type, struct symbol *sym, long long addend)
>  {
>  	struct reloc *reloc;
>  
> Index: linux-2.6/tools/objtool/include/objtool/elf.h
> ===================================================================
> --- linux-2.6.orig/tools/objtool/include/objtool/elf.h	2022-05-16 16:07:54.000000000 +0200
> +++ linux-2.6/tools/objtool/include/objtool/elf.h	2022-05-16 16:22:45.000000000 +0200
> @@ -73,7 +73,7 @@ struct reloc {
>  	struct symbol *sym;
>  	unsigned long offset;
>  	unsigned int type;
> -	long addend;
> +	long long addend;
>  	int idx;
>  	bool jump_table_start;
>  };
> @@ -135,7 +135,7 @@ struct elf *elf_open_read(const char *na
>  struct section *elf_create_section(struct elf *elf, const char *name, unsigned int sh_flags, size_t entsize, int nr);
>  
>  int elf_add_reloc(struct elf *elf, struct section *sec, unsigned long offset,
> -		  unsigned int type, struct symbol *sym, long addend);
> +		  unsigned int type, struct symbol *sym, long long addend);
>  int elf_add_reloc_to_insn(struct elf *elf, struct section *sec,
>  			  unsigned long offset, unsigned int type,
>  			  struct section *insn_sec, unsigned long insn_off);
> 

I think I much prefer s64 there instead of 'long long'. But really I
think all of objtool needs a bit of an audit to see what types need be
used.

Re: [PATCH] objtool: fix objtool regression on x32 systems

[Mikulas Patocka]

On Mon, 16 May 2022, Peter Zijlstra wrote:

> On Mon, May 16, 2022 at 11:06:36AM -0400, Mikulas Patocka wrote:
> > The patch 4abff6d48dbc ("objtool: Fix code relocs vs weak symbols") makes
> > the kernel unbootable.  The patch c087c6e7b551 ("objtool: Fix type of
> > reloc::addend") attempts to fix it by replacing 'int' with 'long'.
> > 
> > However, we may be running on a system with x32 ABI and 'long' on x32 is
> > 32-bit, thus the patch c087c6e7b551 doesn't really change anything and we
> > still end up with miscompiled kernel.  This patch replaces 'long' with
> > 'long long', so that the 64-bit kernel is correctly compiled on a x32
> > system.
> 
> Hurmph.. you're building a 64bit kernel on a x32 hosted machine? And
> this is the *only* thing that goes sideways?

I use the x32 ABI (that is 64-bit mode with addresses truncated to 
32-bit). gcc compiled for the x32 ABI is about 5% to 10% faster than gcc 
compiled for the amd64 ABI. I installed the x32 gcc from Debian Ports.

> I suspect quite a bit of objtool assumes LP64 and won't quite work right
> on ILP32 and we've just been lucky so far.

With this patch, the compiled kernel works. With kernels 5.17 or older, it 
also works. I bisected it and the breakage is caused by the commit 
4abff6d48dbc.

Mikulas

Re: [PATCH] objtool: fix objtool regression on x32 systems

[Peter Zijlstra]

On Mon, May 16, 2022 at 11:56:21AM -0400, Mikulas Patocka wrote:
> 
> 
> On Mon, 16 May 2022, Peter Zijlstra wrote:
> 
> > On Mon, May 16, 2022 at 11:06:36AM -0400, Mikulas Patocka wrote:
> > > The patch 4abff6d48dbc ("objtool: Fix code relocs vs weak symbols") makes
> > > the kernel unbootable.  The patch c087c6e7b551 ("objtool: Fix type of
> > > reloc::addend") attempts to fix it by replacing 'int' with 'long'.
> > > 
> > > However, we may be running on a system with x32 ABI and 'long' on x32 is
> > > 32-bit, thus the patch c087c6e7b551 doesn't really change anything and we
> > > still end up with miscompiled kernel.  This patch replaces 'long' with
> > > 'long long', so that the 64-bit kernel is correctly compiled on a x32
> > > system.
> > 
> > Hurmph.. you're building a 64bit kernel on a x32 hosted machine? And
> > this is the *only* thing that goes sideways?
> 
> I use the x32 ABI (that is 64-bit mode with addresses truncated to 
> 32-bit). gcc compiled for the x32 ABI is about 5% to 10% faster than gcc 
> compiled for the amd64 ABI. I installed the x32 gcc from Debian Ports.
> 
> > I suspect quite a bit of objtool assumes LP64 and won't quite work right
> > on ILP32 and we've just been lucky so far.
> 
> With this patch, the compiled kernel works. With kernels 5.17 or older, it 
> also works. I bisected it and the breakage is caused by the commit 
> 4abff6d48dbc.

Sure; but it works doesn't mean there aren't more latent issues. ILP32
hosted (cross) builds just aren't a thing I've ever considered. If we
really want to go support that then we should at least audit the whole
thing.

A quick look seems to suggest at least all the 'offset' fields should be
u64 or something. The only reason that works is because -mcmodel=kernel
keeps everything in the 2G range to make s32 immediates work. But it
isn't right.

Additionally, for things like LTO/IBT with vmlinux wide objtool runs,
i've seen objtool use up to ~20G of memory, I'm fairly sure x32 won't
cope with that.

RE: [PATCH] objtool: fix objtool regression on x32 systems

[David Laight]

From: Peter Zijlstra
> Sent: 16 May 2022 22:25
...
> A quick look seems to suggest at least all the 'offset' fields should be
> u64 or something. The only reason that works is because -mcmodel=kernel
> keeps everything in the 2G range to make s32 immediates work. But it
> isn't right.

Offsets in instructions are only 8bit or 32bit.
The mod/reg/rm and SiB encodings don't support anything else.
If offsets might be large then they have to be loaded into
registers - which will slow things down.

Immediates can be 64bit (but not if the addressing mode
includes an offset).

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH] objtool: fix objtool regression on x32 systems

[Peter Zijlstra]

On Tue, May 17, 2022 at 08:26:42AM +0000, David Laight wrote:
> From: Peter Zijlstra
> > Sent: 16 May 2022 22:25
> ...
> > A quick look seems to suggest at least all the 'offset' fields should be
> > u64 or something. The only reason that works is because -mcmodel=kernel
> > keeps everything in the 2G range to make s32 immediates work. But it
> > isn't right.
> 
> Offsets in instructions are only 8bit or 32bit.
> The mod/reg/rm and SiB encodings don't support anything else.
> If offsets might be large then they have to be loaded into
> registers - which will slow things down.

Not relevant; these are Elf64_Addr fields, objtool currently uses
'unsigned long' for them, but that goes sideways if ILP32.

[PATCH v2] objtool: fix objtool regression on x32 systems

[Mikulas Patocka]

On Mon, 16 May 2022, Peter Zijlstra wrote:

> On Mon, May 16, 2022 at 11:56:21AM -0400, Mikulas Patocka wrote:
> > 
> > With this patch, the compiled kernel works. With kernels 5.17 or older, it 
> > also works. I bisected it and the breakage is caused by the commit 
> > 4abff6d48dbc.
> 
> Sure; but it works doesn't mean there aren't more latent issues. ILP32
> hosted (cross) builds just aren't a thing I've ever considered. If we
> really want to go support that then we should at least audit the whole
> thing.
> 
> A quick look seems to suggest at least all the 'offset' fields should be
> u64 or something. The only reason that works is because -mcmodel=kernel
> keeps everything in the 2G range to make s32 immediates work. But it
> isn't right.

There are many 'offset' variables and its hard to determine which should 
be 64-bit. Would it be possible to commit this patch, so that kernel 5.18 
can be compiled on x32 distributions? And you can do code refactoring in 
the next merge window.

Mikulas


From: Mikulas Patocka <mpatocka@redhat.com>

The patch 4abff6d48dbc ("objtool: Fix code relocs vs weak symbols") makes
the kernel unbootable.  The patch c087c6e7b551 ("objtool: Fix type of
reloc::addend") attempts to fix it by replacing 'int' with 'long'.

However, we may be running on a system with x32 ABI and 'long' on x32 is
32-bit, thus the patch c087c6e7b551 doesn't really change anything and we
still end up with miscompiled kernel.  This patch replaces 'long' with
's64', so that the 64-bit kernel is correctly compiled on a x32 system.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Fixes: 4abff6d48dbc ("objtool: Fix code relocs vs weak symbols")
Fixes: c087c6e7b551 ("objtool: Fix type of reloc::addend")

---
 tools/objtool/check.c               |    9 +++++----
 tools/objtool/elf.c                 |    2 +-
 tools/objtool/include/objtool/elf.h |    4 ++--
 3 files changed, 8 insertions(+), 7 deletions(-)

Index: linux-2.6/tools/objtool/check.c
===================================================================
--- linux-2.6.orig/tools/objtool/check.c	2022-05-18 13:51:22.000000000 +0200
+++ linux-2.6/tools/objtool/check.c	2022-05-18 13:52:37.000000000 +0200
@@ -6,6 +6,7 @@
 #include <string.h>
 #include <stdlib.h>
 #include <sys/mman.h>
+#include <inttypes.h>
 
 #include <arch/elf.h>
 #include <objtool/builtin.h>
@@ -560,12 +561,12 @@ static int add_dead_ends(struct objtool_
 		else if (reloc->addend == reloc->sym->sec->sh.sh_size) {
 			insn = find_last_insn(file, reloc->sym->sec);
 			if (!insn) {
-				WARN("can't find unreachable insn at %s+0x%lx",
+				WARN("can't find unreachable insn at %s+0x%"PRIx64,
 				     reloc->sym->sec->name, reloc->addend);
 				return -1;
 			}
 		} else {
-			WARN("can't find unreachable insn at %s+0x%lx",
+			WARN("can't find unreachable insn at %s+0x%"PRIx64,
 			     reloc->sym->sec->name, reloc->addend);
 			return -1;
 		}
@@ -595,12 +596,12 @@ reachable:
 		else if (reloc->addend == reloc->sym->sec->sh.sh_size) {
 			insn = find_last_insn(file, reloc->sym->sec);
 			if (!insn) {
-				WARN("can't find reachable insn at %s+0x%lx",
+				WARN("can't find reachable insn at %s+0x%"PRIx64,
 				     reloc->sym->sec->name, reloc->addend);
 				return -1;
 			}
 		} else {
-			WARN("can't find reachable insn at %s+0x%lx",
+			WARN("can't find reachable insn at %s+0x%"PRIx64,
 			     reloc->sym->sec->name, reloc->addend);
 			return -1;
 		}
Index: linux-2.6/tools/objtool/elf.c
===================================================================
--- linux-2.6.orig/tools/objtool/elf.c	2022-05-18 13:51:22.000000000 +0200
+++ linux-2.6/tools/objtool/elf.c	2022-05-18 13:51:22.000000000 +0200
@@ -546,7 +546,7 @@ static struct section *elf_create_reloc_
 						int reltype);
 
 int elf_add_reloc(struct elf *elf, struct section *sec, unsigned long offset,
-		  unsigned int type, struct symbol *sym, long addend)
+		  unsigned int type, struct symbol *sym, s64 addend)
 {
 	struct reloc *reloc;
 
Index: linux-2.6/tools/objtool/include/objtool/elf.h
===================================================================
--- linux-2.6.orig/tools/objtool/include/objtool/elf.h	2022-05-18 13:51:22.000000000 +0200
+++ linux-2.6/tools/objtool/include/objtool/elf.h	2022-05-18 13:51:22.000000000 +0200
@@ -73,7 +73,7 @@ struct reloc {
 	struct symbol *sym;
 	unsigned long offset;
 	unsigned int type;
-	long addend;
+	s64 addend;
 	int idx;
 	bool jump_table_start;
 };
@@ -135,7 +135,7 @@ struct elf *elf_open_read(const char *na
 struct section *elf_create_section(struct elf *elf, const char *name, unsigned int sh_flags, size_t entsize, int nr);
 
 int elf_add_reloc(struct elf *elf, struct section *sec, unsigned long offset,
-		  unsigned int type, struct symbol *sym, long addend);
+		  unsigned int type, struct symbol *sym, s64 addend);
 int elf_add_reloc_to_insn(struct elf *elf, struct section *sec,
 			  unsigned long offset, unsigned int type,
 			  struct section *insn_sec, unsigned long insn_off);

Re: [PATCH v2] objtool: fix objtool regression on x32 systems

[Josh Poimboeuf]

On Wed, May 18, 2022 at 09:28:57AM -0400, Mikulas Patocka wrote:
> 
> 
> On Mon, 16 May 2022, Peter Zijlstra wrote:
> 
> > On Mon, May 16, 2022 at 11:56:21AM -0400, Mikulas Patocka wrote:
> > > 
> > > With this patch, the compiled kernel works. With kernels 5.17 or older, it 
> > > also works. I bisected it and the breakage is caused by the commit 
> > > 4abff6d48dbc.
> > 
> > Sure; but it works doesn't mean there aren't more latent issues. ILP32
> > hosted (cross) builds just aren't a thing I've ever considered. If we
> > really want to go support that then we should at least audit the whole
> > thing.
> > 
> > A quick look seems to suggest at least all the 'offset' fields should be
> > u64 or something. The only reason that works is because -mcmodel=kernel
> > keeps everything in the 2G range to make s32 immediates work. But it
> > isn't right.
> 
> There are many 'offset' variables and its hard to determine which should 
> be 64-bit. Would it be possible to commit this patch, so that kernel 5.18 
> can be compiled on x32 distributions? And you can do code refactoring in 
> the next merge window.

I believe Peter is going ahead with merging this for 5.18, and your v2
looks identical to what he already has here:

  https://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git/log/?h=objtool/urgent

-- 
Josh