[PATCH] objtool: fix build of 64-bit kernel with 32-bit userspace

[PATCH] objtool: fix build of 64-bit kernel with 32-bit userspace

[Mikulas Patocka]

This patch fixes building of 64-bit kernel on 32-bit userspace (I tested 
it on RHEL-6-i686 and Debian-Sid-x32).

There are still some more bugs - when building 64-bit kernel on 32-bit
Debian 7 distribution, objtool corrupts object files so that the linker
doesn't recognize them (ld doesn't recognize the format of the file at all
and gold prints a lot of errors).

Another problem - when building 32-bit kernel, why do you force frame
pointers on and why did you hide CONFIG_GUESS_UNWINDER behind
CONFIG_EXPERT? Frame pointers increase code size and register pressure,
most users don't need precise stacktraces, so CONFIG_GUESS_UNWINDER should
be default for non-expert users just like it was before.

Is there some technical reason why do you want to avoid 
CONFIG_GUESS_UNWINDER?





From: Mikulas Patocka <mpatocka@redhat.com>
Subject: [PATCH] objtool: fix build of 64-bit kernel with 32-bit userspace

The new orc unwinder breaks building 64-bit kernel with 32-bit userspace.
Building the kernel with i386 or x32 userspace fails with this error:

  CC       /usr/src/git/linux-2.6/tools/objtool/orc_dump.o
orc_dump.c: In function 'orc_dump':
orc_dump.c:105:26: error: passing argument 2 of 'elf_getshdrnum' from incompatible pointer type [-Werror=incompatible-pointer-types]
  if (elf_getshdrnum(elf, &nr_sections)) {
                          ^
In file included from /usr/local/include/gelf.h:32:0,
                 from elf.h:22,
                 from warn.h:26,
                 from orc_dump.c:20:
/usr/local/include/libelf.h:304:12: note: expected 'size_t * {aka unsigned int *}' but argument is of type 'long unsigned int *'
 extern int elf_getshdrnum (Elf *__elf, size_t *__dst);
            ^~~~~~~~~~~~~~
orc_dump.c:190:17: error: format '%lx' expects argument of type 'long unsigned int', but argument 3 has type 'Elf64_Sxword {aka long long int}' [-Werror=format=]
    printf("%s+%lx:", name, rela.r_addend);
               ~~^          ~~~~~~~~~~~~~
               %llx
cc1: all warnings being treated as errors

This patch fixes the build failure.

Another problem is that if the user specifies HOSTCC or HOSTLD variables,
they are ignored in the makefile tools/objtool/Makefile. This patch
changes the makefile so that it respects these variables.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

---
 tools/objtool/Makefile   |    5 +++--
 tools/objtool/orc_dump.c |    7 ++++---
 2 files changed, 7 insertions(+), 5 deletions(-)

Index: linux-2.6/tools/objtool/orc_dump.c
===================================================================
--- linux-2.6.orig/tools/objtool/orc_dump.c
+++ linux-2.6/tools/objtool/orc_dump.c
@@ -76,7 +76,8 @@ int orc_dump(const char *_objname)
 	int fd, nr_entries, i, *orc_ip = NULL, orc_size = 0;
 	struct orc_entry *orc = NULL;
 	char *name;
-	unsigned long nr_sections, orc_ip_addr = 0;
+	size_t nr_sections;
+	Elf64_Addr orc_ip_addr = 0;
 	size_t shstrtab_idx;
 	Elf *elf;
 	Elf_Scn *scn;
@@ -187,10 +188,10 @@ int orc_dump(const char *_objname)
 				return -1;
 			}
 
-			printf("%s+%lx:", name, rela.r_addend);
+			printf("%s+%llx:", name, (unsigned long long)rela.r_addend);
 
 		} else {
-			printf("%lx:", orc_ip_addr + (i * sizeof(int)) + orc_ip[i]);
+			printf("%llx:", (unsigned long long)(orc_ip_addr + (i * sizeof(int)) + orc_ip[i]));
 		}
 
 
Index: linux-2.6/tools/objtool/Makefile
===================================================================
--- linux-2.6.orig/tools/objtool/Makefile
+++ linux-2.6/tools/objtool/Makefile
@@ -7,8 +7,9 @@ ARCH := x86
 endif
 
 # always use the host compiler
-CC = gcc
-LD = ld
+CC = $(HOSTCC)
+HOSTLD ?= ld
+LD = $(HOSTLD)
 AR = ar
 
 ifeq ($(srctree),)

Re: [PATCH] objtool: fix build of 64-bit kernel with 32-bit userspace

[Josh Poimboeuf]

[ Adding Ingo to cc because I believe it was his suggestion to hide the
  guess unwinder behind CONFIG_EXPERT. ]

On Mon, Nov 06, 2017 at 06:27:53PM -0500, Mikulas Patocka wrote:
> This patch fixes building of 64-bit kernel on 32-bit userspace (I tested 
> it on RHEL-6-i686 and Debian-Sid-x32).

Thanks, I'll review the patch.

> There are still some more bugs - when building 64-bit kernel on 32-bit
> Debian 7 distribution, objtool corrupts object files so that the linker
> doesn't recognize them (ld doesn't recognize the format of the file at all
> and gold prints a lot of errors).

Hm, we should figure out what's going on there...

> Another problem - when building 32-bit kernel, why do you force frame
> pointers on and why did you hide CONFIG_GUESS_UNWINDER behind
> CONFIG_EXPERT? Frame pointers increase code size and register pressure,
> most users don't need precise stacktraces, so CONFIG_GUESS_UNWINDER should
> be default for non-expert users just like it was before.
> 
> Is there some technical reason why do you want to avoid 
> CONFIG_GUESS_UNWINDER?

The technical reason for avoiding the guess unwinder is that it's
sketchy: it gives false positive results.  Not only for oopses, but for
all the other users of the unwinder: /proc/<pid>/stack, perf, lockdep,
etc.  So it's a correctness issue.

I agree with you that the frame pointer unwinder has drawbacks, but if
somebody cares about those drawbacks, I would consider that person an
"expert" ;-)

-- 
Josh

Re: [PATCH] objtool: fix build of 64-bit kernel with 32-bit userspace

[Mikulas Patocka]

On Tue, 7 Nov 2017, Josh Poimboeuf wrote:

> > Is there some technical reason why do you want to avoid 
> > CONFIG_GUESS_UNWINDER?
> 
> The technical reason for avoiding the guess unwinder is that it's
> sketchy: it gives false positive results.

I've always used kernels without frame pointer and I don't see any problem 
with decoding stack traces with some phantom entries that were left in the 
stack - it's easy to find out which functions could call which functions 
and discard the phantom entries.

> Not only for oopses, but for all the other users of the unwinder: 
> /proc/<pid>/stack, perf, lockdep, etc.  So it's a correctness issue.

Experts need these features, but casual users don't.

> I agree with you that the frame pointer unwinder has drawbacks, but if
> somebody cares about those drawbacks, I would consider that person an
> "expert" ;-)

The Kconfig entry says that frame pointers degrade performance by 5-10% - 
so almost any user would care about it, not just experts.

Mikulas

Re: [PATCH] objtool: fix build of 64-bit kernel with 32-bit userspace

[Peter Zijlstra]

On Tue, Nov 07, 2017 at 04:25:10PM -0500, Mikulas Patocka wrote:
> > The technical reason for avoiding the guess unwinder is that it's
> > sketchy: it gives false positive results.
> 
> I've always used kernels without frame pointer and I don't see any problem 
> with decoding stack traces with some phantom entries that were left in the 
> stack - it's easy to find out which functions could call which functions 
> and discard the phantom entries.
> 
> > Not only for oopses, but for all the other users of the unwinder: 
> > /proc/<pid>/stack, perf, lockdep, etc.  So it's a correctness issue.
> 
> Experts need these features, but casual users don't.
> 
> > I agree with you that the frame pointer unwinder has drawbacks, but if
> > somebody cares about those drawbacks, I would consider that person an
> > "expert" ;-)
> 
> The Kconfig entry says that frame pointers degrade performance by 5-10% - 
> so almost any user would care about it, not just experts.

You're running a 32bit kernel.... isn't that the same as not caring
about performance in any case?

I suppose the solution you're looking for is making ORC work for it; but
given hardly anybody still cares about 32bit x86 you'll probably have to
do it yourself.

Re: [PATCH] objtool: fix build of 64-bit kernel with 32-bit userspace

[Sven Joachim]

On 2017-11-07 11:55 -0600, Josh Poimboeuf wrote:

> [ Adding Ingo to cc because I believe it was his suggestion to hide the
>   guess unwinder behind CONFIG_EXPERT. ]
>
> On Mon, Nov 06, 2017 at 06:27:53PM -0500, Mikulas Patocka wrote:
>> This patch fixes building of 64-bit kernel on 32-bit userspace (I tested 
>> it on RHEL-6-i686 and Debian-Sid-x32).
>
> Thanks, I'll review the patch.

Any news on that?  After upgrading to 4.15-rc1 and running
"make oldconfig" I found out that the kernel would no longer build
unless I selected CONFIG_UNWINDER_FRAME_POINTER=y.

,----
| $ uname -m
| x86_64
| $ dpkg --print-architecture 
| i386
`----

>> There are still some more bugs - when building 64-bit kernel on 32-bit
>> Debian 7 distribution, objtool corrupts object files so that the linker
>> doesn't recognize them (ld doesn't recognize the format of the file at all
>> and gold prints a lot of errors).
>
> Hm, we should figure out what's going on there...
>
>> Another problem - when building 32-bit kernel, why do you force frame
>> pointers on and why did you hide CONFIG_GUESS_UNWINDER behind
>> CONFIG_EXPERT? Frame pointers increase code size and register pressure,
>> most users don't need precise stacktraces, so CONFIG_GUESS_UNWINDER should
>> be default for non-expert users just like it was before.
>> 
>> Is there some technical reason why do you want to avoid 
>> CONFIG_GUESS_UNWINDER?
>
> The technical reason for avoiding the guess unwinder is that it's
> sketchy: it gives false positive results.  Not only for oopses, but for
> all the other users of the unwinder: /proc/<pid>/stack, perf, lockdep,
> etc.  So it's a correctness issue.
>
> I agree with you that the frame pointer unwinder has drawbacks, but if
> somebody cares about those drawbacks, I would consider that person an
> "expert" ;-)

Cheers,
       Sven

Re: [PATCH] objtool: fix build of 64-bit kernel with 32-bit userspace

[Josh Poimboeuf]

On Mon, Nov 27, 2017 at 06:32:48PM +0100, Sven Joachim wrote:
> On 2017-11-07 11:55 -0600, Josh Poimboeuf wrote:
> 
> > [ Adding Ingo to cc because I believe it was his suggestion to hide the
> >   guess unwinder behind CONFIG_EXPERT. ]
> >
> > On Mon, Nov 06, 2017 at 06:27:53PM -0500, Mikulas Patocka wrote:
> >> This patch fixes building of 64-bit kernel on 32-bit userspace (I tested 
> >> it on RHEL-6-i686 and Debian-Sid-x32).
> >
> > Thanks, I'll review the patch.
> 
> Any news on that?  After upgrading to 4.15-rc1 and running
> "make oldconfig" I found out that the kernel would no longer build
> unless I selected CONFIG_UNWINDER_FRAME_POINTER=y.
> 
> ,----
> | $ uname -m
> | x86_64
> | $ dpkg --print-architecture 
> | i386
> `----

Sorry, I was traveling last week and I'm still getting caught up.  I'll
take a look at it this week.

-- 
Josh