Hi all,
After merging the tip tree, today's linux-next build (powerpc
allyesconfig) failed like this:
drivers/video/fbdev/aty/atyfb_base.c: In function 'atyfb_setup_generic':
drivers/video/fbdev/aty/atyfb_base.c:3447:2: error: implicit declaration of function 'ioremap_uc' [-Werror=implicit-function-declaration]
par->ati_regbase = ioremap_uc(info->fix.mmio_start, 0x1000);
^
drivers/video/fbdev/aty/atyfb_base.c:3447:19: warning: assignment makes pointer from integer without a cast
par->ati_regbase = ioremap_uc(info->fix.mmio_start, 0x1000);
^
Caused by commits
3cc2dac5be3f ("drivers/video/fbdev/atyfb: Replace MTRR UC hole with strong UC")
8c7ea50c010b ("x86/mm, asm-generic: Add IOMMU ioremap_uc() variant default")
The latter defines ioremap_uc() for x86 and those architectures that
use asm-generic/io.h - which is not all of them :-( . The former commit
then uses ioremap_uc().
I have reverted commit 3cc2dac5be3f (and 7d89a3cb159a that follows it)
for today.
--
Cheers,
Stephen Rothwell [email protected]
On Tue, Jul 28, 2015 at 03:33:03PM +1000, Stephen Rothwell wrote:
> Hi all,
>
> After merging the tip tree, today's linux-next build (powerpc
> allyesconfig) failed like this:
>
> drivers/video/fbdev/aty/atyfb_base.c: In function 'atyfb_setup_generic':
> drivers/video/fbdev/aty/atyfb_base.c:3447:2: error: implicit declaration of function 'ioremap_uc' [-Werror=implicit-function-declaration]
> par->ati_regbase = ioremap_uc(info->fix.mmio_start, 0x1000);
> ^
> drivers/video/fbdev/aty/atyfb_base.c:3447:19: warning: assignment makes pointer from integer without a cast
> par->ati_regbase = ioremap_uc(info->fix.mmio_start, 0x1000);
> ^
>
> Caused by commits
>
> 3cc2dac5be3f ("drivers/video/fbdev/atyfb: Replace MTRR UC hole with strong UC")
> 8c7ea50c010b ("x86/mm, asm-generic: Add IOMMU ioremap_uc() variant default")
>
> The latter defines ioremap_uc() for x86 and those architectures that
> use asm-generic/io.h - which is not all of them :-( . The former commit
> then uses ioremap_uc().
>
> I have reverted commit 3cc2dac5be3f (and 7d89a3cb159a that follows it)
> for today.
This should be fixed by:
http://git.kernel.org/tip/8c7ea50c010b2f1e006ad37c43f98202a31de2cb
The way it was defined was to return NULL if an arch does not have it,
*but* if the asm-generic io.h header is not included on some archs it will still
fail, which leaves us no option but to then poke and define its implementaiton
for archs which opt-out of asm-generic io.h
Benh, in this case I believe its OK to to just map it to ioremap(), let me know
what you think.
diff --git a/arch/powerpc/include/asm/io.h b/arch/powerpc/include/asm/io.h
index a8d2ef30d473..91db9153cd44 100644
--- a/arch/powerpc/include/asm/io.h
+++ b/arch/powerpc/include/asm/io.h
@@ -721,6 +721,7 @@ extern void __iomem *ioremap_prot(phys_addr_t address, unsigned long size,
unsigned long flags);
extern void __iomem *ioremap_wc(phys_addr_t address, unsigned long size);
#define ioremap_nocache(addr, size) ioremap((addr), (size))
+#define ioremap_uc(addr, size) ioremap((addr), (size))
extern void iounmap(volatile void __iomem *addr);
Luis
On Tue, Jul 28, 2015 at 06:34:19PM +0200, Luis R. Rodriguez wrote:
> On Tue, Jul 28, 2015 at 03:33:03PM +1000, Stephen Rothwell wrote:
> > Hi all,
> >
> > After merging the tip tree, today's linux-next build (powerpc
> > allyesconfig) failed like this:
> >
> > drivers/video/fbdev/aty/atyfb_base.c: In function 'atyfb_setup_generic':
> > drivers/video/fbdev/aty/atyfb_base.c:3447:2: error: implicit declaration of function 'ioremap_uc' [-Werror=implicit-function-declaration]
> > par->ati_regbase = ioremap_uc(info->fix.mmio_start, 0x1000);
> > ^
> > drivers/video/fbdev/aty/atyfb_base.c:3447:19: warning: assignment makes pointer from integer without a cast
> > par->ati_regbase = ioremap_uc(info->fix.mmio_start, 0x1000);
> > ^
> >
> > Caused by commits
> >
> > 3cc2dac5be3f ("drivers/video/fbdev/atyfb: Replace MTRR UC hole with strong UC")
> > 8c7ea50c010b ("x86/mm, asm-generic: Add IOMMU ioremap_uc() variant default")
> >
> > The latter defines ioremap_uc() for x86 and those architectures that
> > use asm-generic/io.h - which is not all of them :-( . The former commit
> > then uses ioremap_uc().
> >
> > I have reverted commit 3cc2dac5be3f (and 7d89a3cb159a that follows it)
> > for today.
>
> This should be fixed by:
>
> http://git.kernel.org/tip/8c7ea50c010b2f1e006ad37c43f98202a31de2cb
>
> The way it was defined was to return NULL if an arch does not have it,
> *but* if the asm-generic io.h header is not included on some archs it will still
> fail, which leaves us no option but to then poke and define its implementaiton
> for archs which opt-out of asm-generic io.h
>
> Benh, in this case I believe its OK to to just map it to ioremap(), let me know
> what you think.
>
I checked and there are other architectures that do not include asm-generic/io.h,
so here is the full patch fix, which I'll post next.
From: "Luis R. Rodriguez" <[email protected]>
Subject: [PATCH] arch/*/io.h: Add ioremap_uc() to all architectures
This adds ioremap_uc() only for architectures that do not
include asm-generic.h/io.h as that already provides a default
definition for them for both cases where you have CONFIG_MMU
and you do not, and because of this, the number of architectures
this patch address is less than the architectures that the
ioremap_wt() patch addressed, "arch/*/io.h: Add ioremap_wt() to all
architectures").
In order to reduce the number of architectures we have to
modify by adding new architecture IO APIs we'll have to review
the architectures in this patch, see why they can't add
asm-generic.h/io.h or issues that would be created by doing
so and then spread a consistent inclusion of this header
towards the end of their own header. For instance arch/metag
includes the asm-generic/io.h *before* the ioremap*()
definitions, this should be the other way around but only
once we have guard wrappers for the non-MMU case also for
asm-generic/io.h.
Signed-off-by: Luis R. Rodriguez <[email protected]>
---
arch/avr32/include/asm/io.h | 1 +
arch/frv/include/asm/io.h | 1 +
arch/m32r/include/asm/io.h | 1 +
arch/m68k/include/asm/io_mm.h | 1 +
arch/mn10300/include/asm/io.h | 1 +
arch/powerpc/include/asm/io.h | 1 +
arch/sh/include/asm/io.h | 1 +
arch/tile/include/asm/io.h | 1 +
8 files changed, 8 insertions(+)
diff --git a/arch/avr32/include/asm/io.h b/arch/avr32/include/asm/io.h
index e998ff5d8e1a..f855646e0db7 100644
--- a/arch/avr32/include/asm/io.h
+++ b/arch/avr32/include/asm/io.h
@@ -297,6 +297,7 @@ extern void __iounmap(void __iomem *addr);
#define ioremap_wc ioremap_nocache
#define ioremap_wt ioremap_nocache
+#define ioremap_uc ioremap_nocache
#define cached(addr) P1SEGADDR(addr)
#define uncached(addr) P2SEGADDR(addr)
diff --git a/arch/frv/include/asm/io.h b/arch/frv/include/asm/io.h
index a31b63ec4930..70dfbea8c8d7 100644
--- a/arch/frv/include/asm/io.h
+++ b/arch/frv/include/asm/io.h
@@ -278,6 +278,7 @@ static inline void __iomem *ioremap_fullcache(unsigned long physaddr, unsigned l
}
#define ioremap_wc ioremap_nocache
+#define ioremap_uc ioremap_nocache
extern void iounmap(void volatile __iomem *addr);
diff --git a/arch/m32r/include/asm/io.h b/arch/m32r/include/asm/io.h
index f8de767ce2bc..61b8931bc192 100644
--- a/arch/m32r/include/asm/io.h
+++ b/arch/m32r/include/asm/io.h
@@ -69,6 +69,7 @@ extern void iounmap(volatile void __iomem *addr);
#define ioremap_nocache(off,size) ioremap(off,size)
#define ioremap_wc ioremap_nocache
#define ioremap_wt ioremap_nocache
+#define ioremap_uc ioremap_nocache
/*
* IO bus memory addresses are also 1:1 with the physical address
diff --git a/arch/m68k/include/asm/io_mm.h b/arch/m68k/include/asm/io_mm.h
index f55cad529400..c98ac81582ac 100644
--- a/arch/m68k/include/asm/io_mm.h
+++ b/arch/m68k/include/asm/io_mm.h
@@ -468,6 +468,7 @@ static inline void __iomem *ioremap_nocache(unsigned long physaddr, unsigned lon
{
return __ioremap(physaddr, size, IOMAP_NOCACHE_SER);
}
+#define ioremap_uc ioremap_nocache
static inline void __iomem *ioremap_wt(unsigned long physaddr,
unsigned long size)
{
diff --git a/arch/mn10300/include/asm/io.h b/arch/mn10300/include/asm/io.h
index 07c5b4a3903b..62189353d2f6 100644
--- a/arch/mn10300/include/asm/io.h
+++ b/arch/mn10300/include/asm/io.h
@@ -283,6 +283,7 @@ static inline void __iomem *ioremap_nocache(unsigned long offset, unsigned long
#define ioremap_wc ioremap_nocache
#define ioremap_wt ioremap_nocache
+#define ioremap_uc ioremap_nocache
static inline void iounmap(void __iomem *addr)
{
diff --git a/arch/powerpc/include/asm/io.h b/arch/powerpc/include/asm/io.h
index a8d2ef30d473..5879fde56f3c 100644
--- a/arch/powerpc/include/asm/io.h
+++ b/arch/powerpc/include/asm/io.h
@@ -721,6 +721,7 @@ extern void __iomem *ioremap_prot(phys_addr_t address, unsigned long size,
unsigned long flags);
extern void __iomem *ioremap_wc(phys_addr_t address, unsigned long size);
#define ioremap_nocache(addr, size) ioremap((addr), (size))
+#define ioremap_uc(addr, size) ioremap((addr), (size))
extern void iounmap(volatile void __iomem *addr);
diff --git a/arch/sh/include/asm/io.h b/arch/sh/include/asm/io.h
index 728c4c571f40..93ec9066dbef 100644
--- a/arch/sh/include/asm/io.h
+++ b/arch/sh/include/asm/io.h
@@ -368,6 +368,7 @@ static inline int iounmap_fixed(void __iomem *addr) { return -EINVAL; }
#endif
#define ioremap_nocache ioremap
+#define ioremap_uc ioremap
#define iounmap __iounmap
/*
diff --git a/arch/tile/include/asm/io.h b/arch/tile/include/asm/io.h
index dc61de15c1f9..322b5fe94781 100644
--- a/arch/tile/include/asm/io.h
+++ b/arch/tile/include/asm/io.h
@@ -55,6 +55,7 @@ extern void iounmap(volatile void __iomem *addr);
#define ioremap_nocache(physaddr, size) ioremap(physaddr, size)
#define ioremap_wc(physaddr, size) ioremap(physaddr, size)
#define ioremap_wt(physaddr, size) ioremap(physaddr, size)
+#define ioremap_uc(physaddr, size) ioremap(physaddr, size)
#define ioremap_fullcache(physaddr, size) ioremap(physaddr, size)
#define mmiowb()
--
2.3.2.209.gd67f9d5.dirty
Commit-ID: 4c73e8926623ca0f64c2c6111289ab8096fa647a
Gitweb: http://git.kernel.org/tip/4c73e8926623ca0f64c2c6111289ab8096fa647a
Author: Luis R. Rodriguez <[email protected]>
AuthorDate: Tue, 28 Jul 2015 20:17:13 +0200
Committer: Ingo Molnar <[email protected]>
CommitDate: Wed, 29 Jul 2015 10:02:36 +0200
arch/*/io.h: Add ioremap_uc() to all architectures
This adds ioremap_uc() only for architectures that do not
include asm-generic.h/io.h as that already provides a default
definition for them for both cases where you have CONFIG_MMU
and you do not, and because of this, the number of architectures
this patch address is less than the architectures that the
ioremap_wt() patch addressed, "arch/*/io.h: Add ioremap_wt() to
all architectures").
In order to reduce the number of architectures we have to
modify by adding new architecture IO APIs we'll have to review
the architectures in this patch, see why they can't add
asm-generic.h/io.h or issues that would be created by doing
so and then spread a consistent inclusion of this header
towards the end of their own header. For instance arch/metag
includes the asm-generic/io.h *before* the ioremap*()
definitions, this should be the other way around but only
once we have guard wrappers for the non-MMU case also for
asm-generic/io.h.
Reported-by: Stephen Rothwell <[email protected]>
Signed-off-by: Luis R. Rodriguez <[email protected]>
Cc: Abhilash Kesavan <[email protected]>
Cc: Benjamin Herrenschmidt <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Chris Metcalf <[email protected]>
Cc: David Howells <[email protected]>
Cc: Fengguang Wu <[email protected]>
Cc: Geert Uytterhoeven <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Greg Ungerer <[email protected]>
Cc: Guenter Roeck <[email protected]>
Cc: Haavard Skinnemoen <[email protected]>
Cc: Hans-Christian Egtvedt <[email protected]>
Cc: Koichi Yasutake <[email protected]>
Cc: Kyle McMartin <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Hurley <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Toshi Kani <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/avr32/include/asm/io.h | 1 +
arch/frv/include/asm/io.h | 1 +
arch/m32r/include/asm/io.h | 1 +
arch/m68k/include/asm/io_mm.h | 1 +
arch/mn10300/include/asm/io.h | 1 +
arch/powerpc/include/asm/io.h | 1 +
arch/sh/include/asm/io.h | 1 +
arch/tile/include/asm/io.h | 1 +
8 files changed, 8 insertions(+)
diff --git a/arch/avr32/include/asm/io.h b/arch/avr32/include/asm/io.h
index e998ff5..f855646 100644
--- a/arch/avr32/include/asm/io.h
+++ b/arch/avr32/include/asm/io.h
@@ -297,6 +297,7 @@ extern void __iounmap(void __iomem *addr);
#define ioremap_wc ioremap_nocache
#define ioremap_wt ioremap_nocache
+#define ioremap_uc ioremap_nocache
#define cached(addr) P1SEGADDR(addr)
#define uncached(addr) P2SEGADDR(addr)
diff --git a/arch/frv/include/asm/io.h b/arch/frv/include/asm/io.h
index a31b63e..70dfbea 100644
--- a/arch/frv/include/asm/io.h
+++ b/arch/frv/include/asm/io.h
@@ -278,6 +278,7 @@ static inline void __iomem *ioremap_fullcache(unsigned long physaddr, unsigned l
}
#define ioremap_wc ioremap_nocache
+#define ioremap_uc ioremap_nocache
extern void iounmap(void volatile __iomem *addr);
diff --git a/arch/m32r/include/asm/io.h b/arch/m32r/include/asm/io.h
index 0c3f25e..4c4982a 100644
--- a/arch/m32r/include/asm/io.h
+++ b/arch/m32r/include/asm/io.h
@@ -69,6 +69,7 @@ extern void iounmap(volatile void __iomem *addr);
#define ioremap_nocache(off,size) ioremap(off,size)
#define ioremap_wc ioremap_nocache
#define ioremap_wt ioremap_nocache
+#define ioremap_uc ioremap_nocache
/*
* IO bus memory addresses are also 1:1 with the physical address
diff --git a/arch/m68k/include/asm/io_mm.h b/arch/m68k/include/asm/io_mm.h
index 618c85d3..a0fce34 100644
--- a/arch/m68k/include/asm/io_mm.h
+++ b/arch/m68k/include/asm/io_mm.h
@@ -467,6 +467,7 @@ static inline void __iomem *ioremap_nocache(unsigned long physaddr, unsigned lon
{
return __ioremap(physaddr, size, IOMAP_NOCACHE_SER);
}
+#define ioremap_uc ioremap_nocache
static inline void __iomem *ioremap_wt(unsigned long physaddr,
unsigned long size)
{
diff --git a/arch/mn10300/include/asm/io.h b/arch/mn10300/include/asm/io.h
index 07c5b4a..6218935 100644
--- a/arch/mn10300/include/asm/io.h
+++ b/arch/mn10300/include/asm/io.h
@@ -283,6 +283,7 @@ static inline void __iomem *ioremap_nocache(unsigned long offset, unsigned long
#define ioremap_wc ioremap_nocache
#define ioremap_wt ioremap_nocache
+#define ioremap_uc ioremap_nocache
static inline void iounmap(void __iomem *addr)
{
diff --git a/arch/powerpc/include/asm/io.h b/arch/powerpc/include/asm/io.h
index a8d2ef3..5879fde 100644
--- a/arch/powerpc/include/asm/io.h
+++ b/arch/powerpc/include/asm/io.h
@@ -721,6 +721,7 @@ extern void __iomem *ioremap_prot(phys_addr_t address, unsigned long size,
unsigned long flags);
extern void __iomem *ioremap_wc(phys_addr_t address, unsigned long size);
#define ioremap_nocache(addr, size) ioremap((addr), (size))
+#define ioremap_uc(addr, size) ioremap((addr), (size))
extern void iounmap(volatile void __iomem *addr);
diff --git a/arch/sh/include/asm/io.h b/arch/sh/include/asm/io.h
index 728c4c5..93ec906 100644
--- a/arch/sh/include/asm/io.h
+++ b/arch/sh/include/asm/io.h
@@ -368,6 +368,7 @@ static inline int iounmap_fixed(void __iomem *addr) { return -EINVAL; }
#endif
#define ioremap_nocache ioremap
+#define ioremap_uc ioremap
#define iounmap __iounmap
/*
diff --git a/arch/tile/include/asm/io.h b/arch/tile/include/asm/io.h
index dc61de1..322b5fe 100644
--- a/arch/tile/include/asm/io.h
+++ b/arch/tile/include/asm/io.h
@@ -55,6 +55,7 @@ extern void iounmap(volatile void __iomem *addr);
#define ioremap_nocache(physaddr, size) ioremap(physaddr, size)
#define ioremap_wc(physaddr, size) ioremap(physaddr, size)
#define ioremap_wt(physaddr, size) ioremap(physaddr, size)
+#define ioremap_uc(physaddr, size) ioremap(physaddr, size)
#define ioremap_fullcache(physaddr, size) ioremap(physaddr, size)
#define mmiowb()
Around Wed 29 Jul 2015 01:15:56 -0700 or thereabout, tip-bot for Luis R. Rodriguez wrote:
> Commit-ID: 4c73e8926623ca0f64c2c6111289ab8096fa647a
> Gitweb: http://git.kernel.org/tip/4c73e8926623ca0f64c2c6111289ab8096fa647a
> Author: Luis R. Rodriguez <[email protected]>
> AuthorDate: Tue, 28 Jul 2015 20:17:13 +0200
> Committer: Ingo Molnar <[email protected]>
> CommitDate: Wed, 29 Jul 2015 10:02:36 +0200
>
> arch/*/io.h: Add ioremap_uc() to all architectures
>
> This adds ioremap_uc() only for architectures that do not
> include asm-generic.h/io.h as that already provides a default
> definition for them for both cases where you have CONFIG_MMU
> and you do not, and because of this, the number of architectures
> this patch address is less than the architectures that the
> ioremap_wt() patch addressed, "arch/*/io.h: Add ioremap_wt() to
> all architectures").
>
> In order to reduce the number of architectures we have to
> modify by adding new architecture IO APIs we'll have to review
> the architectures in this patch, see why they can't add
> asm-generic.h/io.h or issues that would be created by doing
> so and then spread a consistent inclusion of this header
> towards the end of their own header. For instance arch/metag
> includes the asm-generic/io.h *before* the ioremap*()
> definitions, this should be the other way around but only
> once we have guard wrappers for the non-MMU case also for
> asm-generic/io.h.
>
> Reported-by: Stephen Rothwell <[email protected]>
> Signed-off-by: Luis R. Rodriguez <[email protected]>
> Cc: Abhilash Kesavan <[email protected]>
> Cc: Benjamin Herrenschmidt <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: Chris Metcalf <[email protected]>
> Cc: David Howells <[email protected]>
> Cc: Fengguang Wu <[email protected]>
> Cc: Geert Uytterhoeven <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Greg Ungerer <[email protected]>
> Cc: Guenter Roeck <[email protected]>
> Cc: Haavard Skinnemoen <[email protected]>
> Cc: Hans-Christian Egtvedt <[email protected]>
> Cc: Koichi Yasutake <[email protected]>
> Cc: Kyle McMartin <[email protected]>
> Cc: Linus Torvalds <[email protected]>
> Cc: Michael Ellerman <[email protected]>
> Cc: Paul Mackerras <[email protected]>
> Cc: Peter Hurley <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Toshi Kani <[email protected]>
> Cc: Will Deacon <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Link: http://lkml.kernel.org/r/[email protected]
> Signed-off-by: Ingo Molnar <[email protected]>
> ---
> arch/avr32/include/asm/io.h | 1 +
For the avr32 part
Acked-by: Hans-Christian Egtvedt <[email protected]>
> arch/frv/include/asm/io.h | 1 +
> arch/m32r/include/asm/io.h | 1 +
> arch/m68k/include/asm/io_mm.h | 1 +
> arch/mn10300/include/asm/io.h | 1 +
> arch/powerpc/include/asm/io.h | 1 +
> arch/sh/include/asm/io.h | 1 +
> arch/tile/include/asm/io.h | 1 +
> 8 files changed, 8 insertions(+)
<snipp diff>
--
HcE
* Stephen Rothwell <[email protected]> wrote:
> Hi all,
>
> After merging the tip tree, today's linux-next build (x86_64 allmodconfig)
> failed like this:
>
> DESCEND objtool
> CC /home/sfr/next/x86_64_allmodconfig/tools/objtool/builtin-check.o
> CC /home/sfr/next/x86_64_allmodconfig/tools/objtool/special.o
> CC /home/sfr/next/x86_64_allmodconfig/tools/objtool/elf.o
> CC /home/sfr/next/x86_64_allmodconfig/tools/objtool/objtool.o
> MKDIR /home/sfr/next/x86_64_allmodconfig/tools/objtool/arch/x86/insn/
> CC /home/sfr/next/x86_64_allmodconfig/tools/objtool/libstring.o
> elf.c:22:23: fatal error: sys/types.h: No such file or directory
> compilation terminated.
> CC /home/sfr/next/x86_64_allmodconfig/tools/objtool/exec-cmd.o
> CC /home/sfr/next/x86_64_allmodconfig/tools/objtool/help.o
> builtin-check.c:28:20: fatal error: string.h: No such file or directory
> compilation terminated.
> objtool.c:28:19: fatal error: stdio.h: No such file or directory
> compilation terminated.
>
> and further errors ...
>
> This build is done with a PowerPC hosted cross compiler with no glibc.
Ugh, what a rare and weird way to build an x86 kernel, and you made linux-next
dependent on it?
> I assume that some things here need to be built with HOSTCC?
I suspect that's the culprit. Do you now mandate people to have PowerPC systems as
a requirement to merge to linux-next? How are people supposed to be able to test
that rare type of build method which does not matter to 99.99% of our kernel
testers and users?
Thanks,
Ingo
On Tue, Mar 1, 2016 at 8:07 AM, Ingo Molnar <[email protected]> wrote:
>
> * Stephen Rothwell <[email protected]> wrote:
>
>> Hi all,
>>
>> After merging the tip tree, today's linux-next build (x86_64 allmodconfig)
>> failed like this:
>>
>> DESCEND objtool
>> CC /home/sfr/next/x86_64_allmodconfig/tools/objtool/builtin-check.o
>> CC /home/sfr/next/x86_64_allmodconfig/tools/objtool/special.o
>> CC /home/sfr/next/x86_64_allmodconfig/tools/objtool/elf.o
>> CC /home/sfr/next/x86_64_allmodconfig/tools/objtool/objtool.o
>> MKDIR /home/sfr/next/x86_64_allmodconfig/tools/objtool/arch/x86/insn/
>> CC /home/sfr/next/x86_64_allmodconfig/tools/objtool/libstring.o
>> elf.c:22:23: fatal error: sys/types.h: No such file or directory
>> compilation terminated.
>> CC /home/sfr/next/x86_64_allmodconfig/tools/objtool/exec-cmd.o
>> CC /home/sfr/next/x86_64_allmodconfig/tools/objtool/help.o
>> builtin-check.c:28:20: fatal error: string.h: No such file or directory
>> compilation terminated.
>> objtool.c:28:19: fatal error: stdio.h: No such file or directory
>> compilation terminated.
>>
>> and further errors ...
>>
>> This build is done with a PowerPC hosted cross compiler with no glibc.
>
> Ugh, what a rare and weird way to build an x86 kernel, and you made linux-next
> dependent on it?
>
>> I assume that some things here need to be built with HOSTCC?
>
> I suspect that's the culprit. Do you now mandate people to have PowerPC systems as
> a requirement to merge to linux-next? How are people supposed to be able to test
> that rare type of build method which does not matter to 99.99% of our kernel
> testers and users?
>
>From my experience in using different $COMPILER you should always pass
CC and HOSTCC in your make-line when building a Linux-kernel or
playing with the Kconfig-system.
When building my llvm-toolchain with make/autoconf I had also to pass
CC and CXX to my configure-line otherwise you got misconfiguration.
[ EXAMPLE: Linux Kconfig-system ]
$ MAKE="make V=1" ; COMPILER="mycompiler" ; MAKE_OPTS="CC=$COMPILER
HOSTCC=$COMPILER"
$ yes "" | $MAKE $MAKE_OPTS oldconfig && $MAKE $MAKE_OPTS
silentoldconfig < /dev/null
- Sedat -
On February 29, 2016 11:28:22 PM PST, Sedat Dilek <[email protected]> wrote:
>On Tue, Mar 1, 2016 at 8:07 AM, Ingo Molnar <[email protected]> wrote:
>>
>> * Stephen Rothwell <[email protected]> wrote:
>>
>>> Hi all,
>>>
>>> After merging the tip tree, today's linux-next build (x86_64
>allmodconfig)
>>> failed like this:
>>>
>>> DESCEND objtool
>>> CC
>/home/sfr/next/x86_64_allmodconfig/tools/objtool/builtin-check.o
>>> CC
>/home/sfr/next/x86_64_allmodconfig/tools/objtool/special.o
>>> CC /home/sfr/next/x86_64_allmodconfig/tools/objtool/elf.o
>>> CC
>/home/sfr/next/x86_64_allmodconfig/tools/objtool/objtool.o
>>> MKDIR
>/home/sfr/next/x86_64_allmodconfig/tools/objtool/arch/x86/insn/
>>> CC
>/home/sfr/next/x86_64_allmodconfig/tools/objtool/libstring.o
>>> elf.c:22:23: fatal error: sys/types.h: No such file or directory
>>> compilation terminated.
>>> CC
>/home/sfr/next/x86_64_allmodconfig/tools/objtool/exec-cmd.o
>>> CC /home/sfr/next/x86_64_allmodconfig/tools/objtool/help.o
>>> builtin-check.c:28:20: fatal error: string.h: No such file or
>directory
>>> compilation terminated.
>>> objtool.c:28:19: fatal error: stdio.h: No such file or directory
>>> compilation terminated.
>>>
>>> and further errors ...
>>>
>>> This build is done with a PowerPC hosted cross compiler with no
>glibc.
>>
>> Ugh, what a rare and weird way to build an x86 kernel, and you made
>linux-next
>> dependent on it?
>>
>>> I assume that some things here need to be built with HOSTCC?
>>
>> I suspect that's the culprit. Do you now mandate people to have
>PowerPC systems as
>> a requirement to merge to linux-next? How are people supposed to be
>able to test
>> that rare type of build method which does not matter to 99.99% of our
>kernel
>> testers and users?
>>
>
>From my experience in using different $COMPILER you should always pass
>CC and HOSTCC in your make-line when building a Linux-kernel or
>playing with the Kconfig-system.
>
>When building my llvm-toolchain with make/autoconf I had also to pass
>CC and CXX to my configure-line otherwise you got misconfiguration.
>
>[ EXAMPLE: Linux Kconfig-system ]
>
>$ MAKE="make V=1" ; COMPILER="mycompiler" ; MAKE_OPTS="CC=$COMPILER
>HOSTCC=$COMPILER"
>
>$ yes "" | $MAKE $MAKE_OPTS oldconfig && $MAKE $MAKE_OPTS
>silentoldconfig < /dev/null
>
>- Sedat -
That is not the issue here. The problem is clearly that objtool is a host program and not compiled with host cc. So it is a good thing to test even though it is weird, because it affects real use cases.
As x86 is far and beyond the most widely used host architecture, cross-compiling x86 is more likely to involve compiler differences than actually using another host, or possibly compiling 32 bits on a system without the 32-bit libc installed, but it should still work.
--
Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.
On Tue, Mar 1, 2016 at 8:39 AM, H. Peter Anvin <[email protected]> wrote:
> On February 29, 2016 11:28:22 PM PST, Sedat Dilek <[email protected]> wrote:
>>On Tue, Mar 1, 2016 at 8:07 AM, Ingo Molnar <[email protected]> wrote:
>>>
>>> * Stephen Rothwell <[email protected]> wrote:
>>>
>>>> Hi all,
>>>>
>>>> After merging the tip tree, today's linux-next build (x86_64
>>allmodconfig)
>>>> failed like this:
>>>>
>>>> DESCEND objtool
>>>> CC
>>/home/sfr/next/x86_64_allmodconfig/tools/objtool/builtin-check.o
>>>> CC
>>/home/sfr/next/x86_64_allmodconfig/tools/objtool/special.o
>>>> CC /home/sfr/next/x86_64_allmodconfig/tools/objtool/elf.o
>>>> CC
>>/home/sfr/next/x86_64_allmodconfig/tools/objtool/objtool.o
>>>> MKDIR
>>/home/sfr/next/x86_64_allmodconfig/tools/objtool/arch/x86/insn/
>>>> CC
>>/home/sfr/next/x86_64_allmodconfig/tools/objtool/libstring.o
>>>> elf.c:22:23: fatal error: sys/types.h: No such file or directory
>>>> compilation terminated.
>>>> CC
>>/home/sfr/next/x86_64_allmodconfig/tools/objtool/exec-cmd.o
>>>> CC /home/sfr/next/x86_64_allmodconfig/tools/objtool/help.o
>>>> builtin-check.c:28:20: fatal error: string.h: No such file or
>>directory
>>>> compilation terminated.
>>>> objtool.c:28:19: fatal error: stdio.h: No such file or directory
>>>> compilation terminated.
>>>>
>>>> and further errors ...
>>>>
>>>> This build is done with a PowerPC hosted cross compiler with no
>>glibc.
>>>
>>> Ugh, what a rare and weird way to build an x86 kernel, and you made
>>linux-next
>>> dependent on it?
>>>
>>>> I assume that some things here need to be built with HOSTCC?
>>>
>>> I suspect that's the culprit. Do you now mandate people to have
>>PowerPC systems as
>>> a requirement to merge to linux-next? How are people supposed to be
>>able to test
>>> that rare type of build method which does not matter to 99.99% of our
>>kernel
>>> testers and users?
>>>
>>
> >From my experience in using different $COMPILER you should always pass
>>CC and HOSTCC in your make-line when building a Linux-kernel or
>>playing with the Kconfig-system.
>>
>>When building my llvm-toolchain with make/autoconf I had also to pass
>>CC and CXX to my configure-line otherwise you got misconfiguration.
>>
>>[ EXAMPLE: Linux Kconfig-system ]
>>
>>$ MAKE="make V=1" ; COMPILER="mycompiler" ; MAKE_OPTS="CC=$COMPILER
>>HOSTCC=$COMPILER"
>>
>>$ yes "" | $MAKE $MAKE_OPTS oldconfig && $MAKE $MAKE_OPTS
>>silentoldconfig < /dev/null
>>
>>- Sedat -
>
> That is not the issue here. The problem is clearly that objtool is a host program and not compiled with host cc. So it is a good thing to test even though it is weird, because it affects real use cases.
>
> As x86 is far and beyond the most widely used host architecture, cross-compiling x86 is more likely to involve compiler differences than actually using another host, or possibly compiling 32 bits on a system without the 32-bit libc installed, but it should still work.
>
Thanks for the clarification.
I talked about my experiences in software building in general.
When this is a "tools/objtool problem" then someone should address
this to "tools/objtool maintainers/folks" or whoever else is
responsible.
- Sedat -
Hi Ingo,
On Tue, 1 Mar 2016 08:07:50 +0100 Ingo Molnar <[email protected]> wrote:
>
> > This build is done with a PowerPC hosted cross compiler with no glibc.
>
> Ugh, what a rare and weird way to build an x86 kernel, and you made linux-next
> dependent on it?
It is just the fastest hardware I currently have access to (you remember
who I work for, right? ;-)). I have always done at least part of the
linux-next building (daily, or overnight) on PowerPC hardware and this
is only the 2nd or third time in over 8 years that it has found an
issue like this.
> > I assume that some things here need to be built with HOSTCC?
>
> I suspect that's the culprit.
Good, hopefully it is not too hard to fix.
--
Cheers,
Stephen Rothwell
* H. Peter Anvin <[email protected]> wrote:
> That is not the issue here. The problem is clearly that objtool is a host
> program and not compiled with host cc. So it is a good thing to test even
> though it is weird, because it affects real use cases.
Absolutely, it's a real bug that should be fixed. What is counterproductive is
that this rare and hard to replicate build modus is now a must-have test for
linux-next inclusion.
I already cross-build to 20+ weird, rarely used architectures, slowing down the
linux-next merge integration test immensely and wasting quite a bit of
electricity. linux-next making it even harder to test for is a step backwards.
Thanks,
Ingo
On Tue, Mar 01, 2016 at 08:40:01PM +1100, Stephen Rothwell wrote:
> Hi Ingo,
>
> On Tue, 1 Mar 2016 08:07:50 +0100 Ingo Molnar <[email protected]> wrote:
> >
> > > This build is done with a PowerPC hosted cross compiler with no glibc.
> >
> > Ugh, what a rare and weird way to build an x86 kernel, and you made linux-next
> > dependent on it?
>
> It is just the fastest hardware I currently have access to (you remember
> who I work for, right? ;-)). I have always done at least part of the
> linux-next building (daily, or overnight) on PowerPC hardware and this
> is only the 2nd or third time in over 8 years that it has found an
> issue like this.
>
> > > I assume that some things here need to be built with HOSTCC?
> >
> > I suspect that's the culprit.
>
> Good, hopefully it is not too hard to fix.
Changing it to use the host compiler would probably be an easy fix, but
that would expose a harder bug related to endianness.
objtool uses the kernel x86 decoder (arch/x86/lib/insn.c) which is
endian-ignorant. If it tries to analyze reverse endian binaries it gets
confused. And since CONFIG_STACK_VALIDATION causes objtool to analyze
every .o file in the build, it'll spit out a ton of false warnings.
How about the below workaround patch to disable objtool and warn when
CROSS_COMPILE is used? If anybody complains about lack of cross-compile
support later, we could try to fix it then.
>From a3c65947011a420743f308b698171c4209105d3f Mon Sep 17 00:00:00 2001
Message-Id: <a3c65947011a420743f308b698171c4209105d3f.1456868910.git.jpoimboe@redhat.com>
From: Josh Poimboeuf <[email protected]>
Date: Tue, 1 Mar 2016 13:35:51 -0600
Subject: [PATCH] objtool: Disable stack validation when CROSS_COMPILE is used
When building with CONFIG_STACK_VALIDATION on a PowerPC host using an
x86 cross-compiler, Stephen Rothwell saw the following objtool build
errors:
DESCEND objtool
CC /home/sfr/next/x86_64_allmodconfig/tools/objtool/builtin-check.o
CC /home/sfr/next/x86_64_allmodconfig/tools/objtool/special.o
CC /home/sfr/next/x86_64_allmodconfig/tools/objtool/elf.o
CC /home/sfr/next/x86_64_allmodconfig/tools/objtool/objtool.o
MKDIR /home/sfr/next/x86_64_allmodconfig/tools/objtool/arch/x86/insn/
CC /home/sfr/next/x86_64_allmodconfig/tools/objtool/libstring.o
elf.c:22:23: fatal error: sys/types.h: No such file or directory
compilation terminated.
CC /home/sfr/next/x86_64_allmodconfig/tools/objtool/exec-cmd.o
CC /home/sfr/next/x86_64_allmodconfig/tools/objtool/help.o
builtin-check.c:28:20: fatal error: string.h: No such file or directory
compilation terminated.
objtool.c:28:19: fatal error: stdio.h: No such file or directory
compilation terminated.
The problem is that objtool was compiled with the cross compiler instead
of the host compiler. But even if that were fixed, we'd still have a
problem with endianness. objtool's x86 decoder (copied from
arch/x86/lib/insn.c) assumes that the endianness of the host and target
are the same. When analyzing a little-endian x86 binary on a big-endian
host, it will get confused and spit out a bunch of false positive
warnings during the build.
Eventually we may want to add endian awareness to x86 objtool, but
that's generally a rare edge case and it's not clear if anybody would
use it. For now, when a cross-compiler is used, just disable
CONFIG_STACK_VALIDATION and emit a warning.
Reported-by: Stephen Rothwell <[email protected]>
Signed-off-by: Josh Poimboeuf <[email protected]>
---
Makefile | 12 +++++++++++-
scripts/Makefile.build | 4 +++-
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index 8cc926f..b59007c 100644
--- a/Makefile
+++ b/Makefile
@@ -998,8 +998,18 @@ prepare0: archprepare FORCE
# All the preparing..
prepare: prepare0 prepare-objtool
+
+ifdef CONFIG_STACK_VALIDATION
+ ifeq ($(CROSS_COMPILE),)
+ objtool_target := tools/objtool FORCE
+ else
+ $(warning Stack validation is not supported with cross-compilation. Disabling CONFIG_STACK_VALIDATION!)
+ endif
+endif
+
PHONY += prepare-objtool
-prepare-objtool: $(if $(CONFIG_STACK_VALIDATION), tools/objtool FORCE)
+prepare-objtool: $(objtool_target)
+
# Generate some files
# ---------------------------------------------------------------------------
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 130a452..973bb26 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -242,6 +242,7 @@ cmd_record_mcount = \
endif
ifdef CONFIG_STACK_VALIDATION
+ifeq ($(CROSS_COMPILE),)
__objtool_obj := $(objtree)/tools/objtool/objtool
@@ -260,13 +261,14 @@ objtool_obj = $(if $(patsubst y%,, \
$(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n), \
$(__objtool_obj))
+endif # CROSS_COMPILE
endif # CONFIG_STACK_VALIDATION
define rule_cc_o_c
$(call echo-cmd,checksrc) $(cmd_checksrc) \
$(call echo-cmd,cc_o_c) $(cmd_cc_o_c); \
$(cmd_modversions) \
- $(cmd_objtool) \
+ $(cmd_objtool) \
$(call echo-cmd,record_mcount) \
$(cmd_record_mcount) \
scripts/basic/fixdep $(depfile) $@ '$(call make-cmd,cc_o_c)' > \
--
2.4.3
Hi Josh,
On Tue, 1 Mar 2016 15:54:51 -0600 Josh Poimboeuf <[email protected]> wrote:
>
> Changing it to use the host compiler would probably be an easy fix, but
> that would expose a harder bug related to endianness.
Just by luck, my PowerPC host is little endian :-)
> How about the below workaround patch to disable objtool and warn when
> CROSS_COMPILE is used? If anybody complains about lack of cross-compile
> support later, we could try to fix it then.
This seems reasonable.
> From a3c65947011a420743f308b698171c4209105d3f Mon Sep 17 00:00:00 2001
> Message-Id: <a3c65947011a420743f308b698171c4209105d3f.1456868910.git.jpoimboe@redhat.com>
> From: Josh Poimboeuf <[email protected]>
> Date: Tue, 1 Mar 2016 13:35:51 -0600
> Subject: [PATCH] objtool: Disable stack validation when CROSS_COMPILE is used
I have applied this to the merge of the tip tree in linux-next today
and it compiles fine for me. I will continue applying it until
something better comes along or it is applied to the tip tree.
Thanks for that.
--
Cheers,
Stephen Rothwell
On Wed, Mar 02, 2016 at 01:27:35PM +1100, Stephen Rothwell wrote:
> Hi Josh,
>
> On Tue, 1 Mar 2016 15:54:51 -0600 Josh Poimboeuf <[email protected]> wrote:
> >
> > Changing it to use the host compiler would probably be an easy fix, but
> > that would expose a harder bug related to endianness.
>
> Just by luck, my PowerPC host is little endian :-)
Aha! In that case, I think I'll take a different approach and try to
add support for CROSS_COMPILE.
We'll need it anyway, later on, when we port objtool to more
architectures.
> > How about the below workaround patch to disable objtool and warn when
> > CROSS_COMPILE is used? If anybody complains about lack of cross-compile
> > support later, we could try to fix it then.
>
> This seems reasonable.
>
> > From a3c65947011a420743f308b698171c4209105d3f Mon Sep 17 00:00:00 2001
> > Message-Id: <a3c65947011a420743f308b698171c4209105d3f.1456868910.git.jpoimboe@redhat.com>
> > From: Josh Poimboeuf <[email protected]>
> > Date: Tue, 1 Mar 2016 13:35:51 -0600
> > Subject: [PATCH] objtool: Disable stack validation when CROSS_COMPILE is used
>
> I have applied this to the merge of the tip tree in linux-next today
> and it compiles fine for me. I will continue applying it until
> something better comes along or it is applied to the tip tree.
Thanks. I should have the patch(es) soon. If/when they get merged,
I'll let you know, and then you can drop this one.
--
Josh
Hi Josh,
On Wed, 2 Mar 2016 15:17:26 -0600 Josh Poimboeuf <[email protected]> wrote:
>
> On Wed, Mar 02, 2016 at 01:27:35PM +1100, Stephen Rothwell wrote:
> >
> > On Tue, 1 Mar 2016 15:54:51 -0600 Josh Poimboeuf <[email protected]> wrote:
> > >
> > > Changing it to use the host compiler would probably be an easy fix, but
> > > that would expose a harder bug related to endianness.
> >
> > Just by luck, my PowerPC host is little endian :-)
>
> Aha! In that case, I think I'll take a different approach and try to
> add support for CROSS_COMPILE.
>
> We'll need it anyway, later on, when we port objtool to more
> architectures.
That would be great. I was discussing this briefly with the powerpc
maintainer and he expressed some interest.
> Thanks. I should have the patch(es) soon. If/when they get merged,
> I'll let you know, and then you can drop this one.
OK, I will keep an eye out - I usually notice when stuff get fixed, but
a heads up is always helpful.
--
Cheers,
Stephen Rothwell
When running objtool on a ppc64le host to analyze x86 binaries, it
reports a lot of false warnings like:
ipc/compat_mq.o: warning: objtool: compat_SyS_mq_open()+0x91: can't find jump dest instruction at .text+0x3a5
The warnings are caused by the x86 instruction decoder setting the wrong
value for the jump instruction's immediate field because it assumes that
"char == signed char", which isn't true for all architectures. When
converting char to int, gcc sign-extends on x86 but doesn't sign-extend
on ppc64le.
According to the gcc man page, that's a feature, not a bug:
> Each kind of machine has a default for what "char" should be. It is
> either like "unsigned char" by default or like "signed char" by
> default.
>
> Ideally, a portable program should always use "signed char" or
> "unsigned char" when it depends on the signedness of an object.
Conform to the "standards" by changing the "char" casts to "signed
char". This results in no actual changes to the object code on x86.
Note: the x86 decoder now lives in three different locations in the
kernel tree, which are all kept in sync via makefile checks and
warnings: in-kernel, perf, and objtool. This fixes all three locations.
Eventually we should probably try to at least converge the two separate
"tools" locations into a single shared location.
Signed-off-by: Josh Poimboeuf <[email protected]>
---
arch/x86/lib/insn.c | 6 +++---
tools/objtool/arch/x86/insn/insn.c | 6 +++---
tools/perf/util/intel-pt-decoder/insn.c | 6 +++---
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/arch/x86/lib/insn.c b/arch/x86/lib/insn.c
index 8f72b33..1a41693 100644
--- a/arch/x86/lib/insn.c
+++ b/arch/x86/lib/insn.c
@@ -374,7 +374,7 @@ void insn_get_displacement(struct insn *insn)
if (mod == 3)
goto out;
if (mod == 1) {
- insn->displacement.value = get_next(char, insn);
+ insn->displacement.value = get_next(signed char, insn);
insn->displacement.nbytes = 1;
} else if (insn->addr_bytes == 2) {
if ((mod == 0 && rm == 6) || mod == 2) {
@@ -532,7 +532,7 @@ void insn_get_immediate(struct insn *insn)
switch (inat_immediate_size(insn->attr)) {
case INAT_IMM_BYTE:
- insn->immediate.value = get_next(char, insn);
+ insn->immediate.value = get_next(signed char, insn);
insn->immediate.nbytes = 1;
break;
case INAT_IMM_WORD:
@@ -566,7 +566,7 @@ void insn_get_immediate(struct insn *insn)
goto err_out;
}
if (inat_has_second_immediate(insn->attr)) {
- insn->immediate2.value = get_next(char, insn);
+ insn->immediate2.value = get_next(signed char, insn);
insn->immediate2.nbytes = 1;
}
done:
diff --git a/tools/objtool/arch/x86/insn/insn.c b/tools/objtool/arch/x86/insn/insn.c
index 47314a6..9f26eae 100644
--- a/tools/objtool/arch/x86/insn/insn.c
+++ b/tools/objtool/arch/x86/insn/insn.c
@@ -374,7 +374,7 @@ void insn_get_displacement(struct insn *insn)
if (mod == 3)
goto out;
if (mod == 1) {
- insn->displacement.value = get_next(char, insn);
+ insn->displacement.value = get_next(signed char, insn);
insn->displacement.nbytes = 1;
} else if (insn->addr_bytes == 2) {
if ((mod == 0 && rm == 6) || mod == 2) {
@@ -532,7 +532,7 @@ void insn_get_immediate(struct insn *insn)
switch (inat_immediate_size(insn->attr)) {
case INAT_IMM_BYTE:
- insn->immediate.value = get_next(char, insn);
+ insn->immediate.value = get_next(signed char, insn);
insn->immediate.nbytes = 1;
break;
case INAT_IMM_WORD:
@@ -566,7 +566,7 @@ void insn_get_immediate(struct insn *insn)
goto err_out;
}
if (inat_has_second_immediate(insn->attr)) {
- insn->immediate2.value = get_next(char, insn);
+ insn->immediate2.value = get_next(signed char, insn);
insn->immediate2.nbytes = 1;
}
done:
diff --git a/tools/perf/util/intel-pt-decoder/insn.c b/tools/perf/util/intel-pt-decoder/insn.c
index 47314a6..9f26eae 100644
--- a/tools/perf/util/intel-pt-decoder/insn.c
+++ b/tools/perf/util/intel-pt-decoder/insn.c
@@ -374,7 +374,7 @@ void insn_get_displacement(struct insn *insn)
if (mod == 3)
goto out;
if (mod == 1) {
- insn->displacement.value = get_next(char, insn);
+ insn->displacement.value = get_next(signed char, insn);
insn->displacement.nbytes = 1;
} else if (insn->addr_bytes == 2) {
if ((mod == 0 && rm == 6) || mod == 2) {
@@ -532,7 +532,7 @@ void insn_get_immediate(struct insn *insn)
switch (inat_immediate_size(insn->attr)) {
case INAT_IMM_BYTE:
- insn->immediate.value = get_next(char, insn);
+ insn->immediate.value = get_next(signed char, insn);
insn->immediate.nbytes = 1;
break;
case INAT_IMM_WORD:
@@ -566,7 +566,7 @@ void insn_get_immediate(struct insn *insn)
goto err_out;
}
if (inat_has_second_immediate(insn->attr)) {
- insn->immediate2.value = get_next(char, insn);
+ insn->immediate2.value = get_next(signed char, insn);
insn->immediate2.nbytes = 1;
}
done:
--
2.4.3
When building with CONFIG_STACK_VALIDATION on a ppc64le host with an x86
cross-compiler, Stephen Rothwell saw the following objtool build errors:
DESCEND objtool
CC /home/sfr/next/x86_64_allmodconfig/tools/objtool/builtin-check.o
CC /home/sfr/next/x86_64_allmodconfig/tools/objtool/special.o
CC /home/sfr/next/x86_64_allmodconfig/tools/objtool/elf.o
CC /home/sfr/next/x86_64_allmodconfig/tools/objtool/objtool.o
MKDIR /home/sfr/next/x86_64_allmodconfig/tools/objtool/arch/x86/insn/
CC /home/sfr/next/x86_64_allmodconfig/tools/objtool/libstring.o
elf.c:22:23: fatal error: sys/types.h: No such file or directory
compilation terminated.
CC /home/sfr/next/x86_64_allmodconfig/tools/objtool/exec-cmd.o
CC /home/sfr/next/x86_64_allmodconfig/tools/objtool/help.o
builtin-check.c:28:20: fatal error: string.h: No such file or directory
compilation terminated.
objtool.c:28:19: fatal error: stdio.h: No such file or directory
compilation terminated.
It fails to build because it tries to compile objtool with the
cross-compiler instead of the host compiler.
Ensure that it always uses the host compiler by ignoring CROSS_COMPILE.
In order to do that properly, the libsubcmd.a library needs to be built
in tools/objtool/ rather than tools/lib/subcmd/. The latter directory
contains the cross-compiled version which is needed for perf and
possibly other tools.
Note that cross-compiling for x86 on a _big_ endian system would result
in a bunch of false positive objtool warnings during the kernel build
because it isn't endian-aware. But that's generally a rare edge case
and there haven't been any reports of anybody needing that.
Reported-by: Stephen Rothwell <[email protected]>
Signed-off-by: Josh Poimboeuf <[email protected]>
---
tools/lib/subcmd/Makefile | 6 ++++--
tools/objtool/Makefile | 17 ++++++++++-------
2 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/tools/lib/subcmd/Makefile b/tools/lib/subcmd/Makefile
index 629cf8c..1faecb8 100644
--- a/tools/lib/subcmd/Makefile
+++ b/tools/lib/subcmd/Makefile
@@ -8,8 +8,10 @@ srctree := $(patsubst %/,%,$(dir $(srctree)))
#$(info Determined 'srctree' to be $(srctree))
endif
-CC = $(CROSS_COMPILE)gcc
-AR = $(CROSS_COMPILE)ar
+CC ?= $(CROSS_COMPILE)gcc
+LD ?= $(CROSS_COMPILE)ld
+AR ?= $(CROSS_COMPILE)ar
+
RM = rm -f
MAKEFLAGS += --no-print-directory
diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
index c4f0713..e4a6bd5 100644
--- a/tools/objtool/Makefile
+++ b/tools/objtool/Makefile
@@ -7,13 +7,19 @@ ARCH := x86
endif
endif
+# always use the host compiler
+CC = gcc
+LD = ld
+AR = ar
+
ifeq ($(srctree),)
srctree := $(patsubst %/,%,$(dir $(shell pwd)))
srctree := $(patsubst %/,%,$(dir $(srctree)))
endif
-SUBCMD_SRCDIR = $(srctree)/tools/lib/subcmd/
-LIBSUBCMD = $(if $(OUTPUT),$(OUTPUT),$(SUBCMD_SRCDIR))libsubcmd.a
+SUBCMD_SRCDIR = $(srctree)/tools/lib/subcmd/
+LIBSUBCMD_OUTPUT = $(if $(OUTPUT),$(OUTPUT),$(PWD)/)
+LIBSUBCMD = $(LIBSUBCMD_OUTPUT)libsubcmd.a
OBJTOOL := $(OUTPUT)objtool
OBJTOOL_IN := $(OBJTOOL)-in.o
@@ -45,12 +51,9 @@ $(OBJTOOL): $(LIBSUBCMD) $(OBJTOOL_IN)
$(LIBSUBCMD): fixdep FORCE
- $(Q)$(MAKE) -C $(SUBCMD_SRCDIR)
-
-$(LIBSUBCMD)-clean:
- $(Q)$(MAKE) -C $(SUBCMD_SRCDIR) clean > /dev/null
+ $(Q)$(MAKE) -C $(SUBCMD_SRCDIR) OUTPUT=$(LIBSUBCMD_OUTPUT)
-clean: $(LIBSUBCMD)-clean
+clean:
$(call QUIET_CLEAN, objtool) $(RM) $(OBJTOOL)
$(Q)find $(OUTPUT) -name '*.o' -delete -o -name '\.*.cmd' -delete -o -name '\.*.d' -delete
$(Q)$(RM) $(OUTPUT)arch/x86/insn/inat-tables.c $(OUTPUT)fixdep
--
2.4.3
This adds the ability for CONFIG_STACK_VALIDATION to work in a
cross-compiled environment against an x86 kernel. Based on tip/master.
Josh Poimboeuf (2):
x86/asm/decoder: Use explicitly signed chars
objtool: Support CROSS_COMPILE
arch/x86/lib/insn.c | 6 +++---
tools/lib/subcmd/Makefile | 6 ++++--
tools/objtool/Makefile | 17 ++++++++++-------
tools/objtool/arch/x86/insn/insn.c | 6 +++---
tools/perf/util/intel-pt-decoder/insn.c | 6 +++---
5 files changed, 23 insertions(+), 18 deletions(-)
--
2.4.3
Hi Josh,
Just a couple of quick comments ...
On Wed, 2 Mar 2016 18:39:37 -0600 Josh Poimboeuf <[email protected]> wrote:
>
> diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
> index c4f0713..e4a6bd5 100644
> --- a/tools/objtool/Makefile
> +++ b/tools/objtool/Makefile
I was wondering if this would be more appropriate in scripts/objtool
since it is used during the building of the kernel. Or does it have a
wider use?
> @@ -7,13 +7,19 @@ ARCH := x86
> endif
> endif
>
> +# always use the host compiler
> +CC = gcc
We have HOSTCC with its associated HOSTCFLAGS etc ... I am not sure if
that is more appropriate (but it does take care of people using clang).
--
Cheers,
Stephen Rothwell
On Thu, Mar 03, 2016 at 01:43:14PM +1100, Stephen Rothwell wrote:
> Hi Josh,
>
> Just a couple of quick comments ...
>
> On Wed, 2 Mar 2016 18:39:37 -0600 Josh Poimboeuf <[email protected]> wrote:
> >
> > diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
> > index c4f0713..e4a6bd5 100644
> > --- a/tools/objtool/Makefile
> > +++ b/tools/objtool/Makefile
>
> I was wondering if this would be more appropriate in scripts/objtool
> since it is used during the building of the kernel. Or does it have a
> wider use?
Yeah, it was actually in the scripts/ dir in earlier revisions of the
patch set, for that very reason. However, Ingo pointed out that it
could be useful beyond the kernel, so we graduated it to a "tool".
> > @@ -7,13 +7,19 @@ ARCH := x86
> > endif
> > endif
> >
> > +# always use the host compiler
> > +CC = gcc
>
> We have HOSTCC with its associated HOSTCFLAGS etc ... I am not sure if
> that is more appropriate (but it does take care of people using clang).
The "tools" are almost completely separate from the rest of the kernel.
They have their own scaled-down version of kbuild, which doesn't have
HOSTCC.
But yeah, we might eventually need to copy some of the host compilation
infrastructure from scripts/Makefile.host over to the tools/ side.
--
Josh
Hi Josh,
On Wed, 2 Mar 2016 21:20:58 -0600 Josh Poimboeuf <[email protected]> wrote:
>
> On Thu, Mar 03, 2016 at 01:43:14PM +1100, Stephen Rothwell wrote:
> >
> > I was wondering if this would be more appropriate in scripts/objtool
> > since it is used during the building of the kernel. Or does it have a
> > wider use?
>
> Yeah, it was actually in the scripts/ dir in earlier revisions of the
> patch set, for that very reason. However, Ingo pointed out that it
> could be useful beyond the kernel, so we graduated it to a "tool".
>
> >
> > We have HOSTCC with its associated HOSTCFLAGS etc ... I am not sure if
> > that is more appropriate (but it does take care of people using clang).
>
> The "tools" are almost completely separate from the rest of the kernel.
> They have their own scaled-down version of kbuild, which doesn't have
> HOSTCC.
>
> But yeah, we might eventually need to copy some of the host compilation
> infrastructure from scripts/Makefile.host over to the tools/ side.
That all sounds sane, thanks.
I did not add this to linux-next today, but may tomorrow if people
think it is sensible to do so (for testing on a powerpcle host). If I
do, I will just back out to the previous patch if it all goes south (so
it won't impact on the rest of the tip tree's testing).
--
Cheers,
Stephen Rothwell
On Thu, Mar 03, 2016 at 02:38:43PM +1100, Stephen Rothwell wrote:
> Hi Josh,
>
> On Wed, 2 Mar 2016 21:20:58 -0600 Josh Poimboeuf <[email protected]> wrote:
> >
> > On Thu, Mar 03, 2016 at 01:43:14PM +1100, Stephen Rothwell wrote:
> > >
> > > I was wondering if this would be more appropriate in scripts/objtool
> > > since it is used during the building of the kernel. Or does it have a
> > > wider use?
> >
> > Yeah, it was actually in the scripts/ dir in earlier revisions of the
> > patch set, for that very reason. However, Ingo pointed out that it
> > could be useful beyond the kernel, so we graduated it to a "tool".
> >
> > >
> > > We have HOSTCC with its associated HOSTCFLAGS etc ... I am not sure if
> > > that is more appropriate (but it does take care of people using clang).
> >
> > The "tools" are almost completely separate from the rest of the kernel.
> > They have their own scaled-down version of kbuild, which doesn't have
> > HOSTCC.
> >
> > But yeah, we might eventually need to copy some of the host compilation
> > infrastructure from scripts/Makefile.host over to the tools/ side.
>
> That all sounds sane, thanks.
>
> I did not add this to linux-next today, but may tomorrow if people
> think it is sensible to do so (for testing on a powerpcle host). If I
> do, I will just back out to the previous patch if it all goes south (so
> it won't impact on the rest of the tip tree's testing).
Sounds good, thanks!
FWIW, I did test it on a ppc64le host with an x86 cross-compiler and it
worked fine (but more testing is certainly welcome).
--
Josh
On 3/2/16, Stephen Rothwell <[email protected]> wrote:
> Hi Josh,
>
> On Tue, 1 Mar 2016 15:54:51 -0600 Josh Poimboeuf <[email protected]>
> wrote:
>>
>> Changing it to use the host compiler would probably be an easy fix, but
>> that would expose a harder bug related to endianness.
>
> Just by luck, my PowerPC host is little endian :-)
>
>> How about the below workaround patch to disable objtool and warn when
>> CROSS_COMPILE is used? If anybody complains about lack of cross-compile
>> support later, we could try to fix it then.
>
> This seems reasonable.
>
>> From a3c65947011a420743f308b698171c4209105d3f Mon Sep 17 00:00:00 2001
>> Message-Id:
>> <a3c65947011a420743f308b698171c4209105d3f.1456868910.git.jpoimboe@redhat.com>
>> From: Josh Poimboeuf <[email protected]>
>> Date: Tue, 1 Mar 2016 13:35:51 -0600
>> Subject: [PATCH] objtool: Disable stack validation when CROSS_COMPILE is
>> used
>
> I have applied this to the merge of the tip tree in linux-next today
> and it compiles fine for me. I will continue applying it until
> something better comes along or it is applied to the tip tree.
>
Does Linux next-20160303 has this patch?
On a quick view I could not find it.
- Sedat -
> Thanks for that.
> --
> Cheers,
> Stephen Rothwell
> --
> To unsubscribe from this list: send the line "unsubscribe linux-next" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Hi Sedat,
On Thu, 3 Mar 2016 08:31:57 +0100 Sedat Dilek <[email protected]> wrote:
>
> Does Linux next-20160303 has this patch?
> On a quick view I could not find it.
It is applied as part of the merge commit that merges the tip tree, so
there is not a separate commit for it.
--
Cheers,
Stephen Rothwell
* Stephen Rothwell <[email protected]> wrote:
> Hi Josh,
>
> On Wed, 2 Mar 2016 21:20:58 -0600 Josh Poimboeuf <[email protected]> wrote:
> >
> > On Thu, Mar 03, 2016 at 01:43:14PM +1100, Stephen Rothwell wrote:
> > >
> > > I was wondering if this would be more appropriate in scripts/objtool
> > > since it is used during the building of the kernel. Or does it have a
> > > wider use?
> >
> > Yeah, it was actually in the scripts/ dir in earlier revisions of the
> > patch set, for that very reason. However, Ingo pointed out that it
> > could be useful beyond the kernel, so we graduated it to a "tool".
> >
> > >
> > > We have HOSTCC with its associated HOSTCFLAGS etc ... I am not sure if
> > > that is more appropriate (but it does take care of people using clang).
> >
> > The "tools" are almost completely separate from the rest of the kernel.
> > They have their own scaled-down version of kbuild, which doesn't have
> > HOSTCC.
> >
> > But yeah, we might eventually need to copy some of the host compilation
> > infrastructure from scripts/Makefile.host over to the tools/ side.
>
> That all sounds sane, thanks.
>
> I did not add this to linux-next today, but may tomorrow if people
> think it is sensible to do so (for testing on a powerpcle host). If I
> do, I will just back out to the previous patch if it all goes south (so
> it won't impact on the rest of the tip tree's testing).
I'll add Josh's fixes to -tip ASAP as well, so hopefully soon you can drop all
linux-next specific patches related to this and it will all work Just Fine (tm).
Sorry about the breakage!
Thanks,
Ingo
On March 2, 2016 6:43:14 PM PST, Stephen Rothwell <[email protected]> wrote:
>Hi Josh,
>
>Just a couple of quick comments ...
>
>On Wed, 2 Mar 2016 18:39:37 -0600 Josh Poimboeuf <[email protected]>
>wrote:
>>
>> diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
>> index c4f0713..e4a6bd5 100644
>> --- a/tools/objtool/Makefile
>> +++ b/tools/objtool/Makefile
>
>I was wondering if this would be more appropriate in scripts/objtool
>since it is used during the building of the kernel. Or does it have a
>wider use?
>
>> @@ -7,13 +7,19 @@ ARCH := x86
>> endif
>> endif
>>
>> +# always use the host compiler
>> +CC = gcc
>
>We have HOSTCC with its associated HOSTCFLAGS etc ... I am not sure if
>that is more appropriate (but it does take care of people using clang).
This is what HOSTCC is for. And yes, this belongs in scripts.
--
Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.
Commit-ID: 19072f23d1d785c093b7f81cb1fb161e7a13ecc0
Gitweb: http://git.kernel.org/tip/19072f23d1d785c093b7f81cb1fb161e7a13ecc0
Author: Josh Poimboeuf <[email protected]>
AuthorDate: Wed, 2 Mar 2016 18:39:36 -0600
Committer: Ingo Molnar <[email protected]>
CommitDate: Thu, 3 Mar 2016 16:13:00 +0100
x86/asm/decoder: Use explicitly signed chars
When running objtool on a ppc64le host to analyze x86 binaries, it
reports a lot of false warnings like:
ipc/compat_mq.o: warning: objtool: compat_SyS_mq_open()+0x91: can't find jump dest instruction at .text+0x3a5
The warnings are caused by the x86 instruction decoder setting the wrong
value for the jump instruction's immediate field because it assumes that
"char == signed char", which isn't true for all architectures. When
converting char to int, gcc sign-extends on x86 but doesn't sign-extend
on ppc64le.
According to the gcc man page, that's a feature, not a bug:
> Each kind of machine has a default for what "char" should be. It is
> either like "unsigned char" by default or like "signed char" by
> default.
>
> Ideally, a portable program should always use "signed char" or
> "unsigned char" when it depends on the signedness of an object.
Conform to the "standards" by changing the "char" casts to "signed
char". This results in no actual changes to the object code on x86.
Note: the x86 decoder now lives in three different locations in the
kernel tree, which are all kept in sync via makefile checks and
warnings: in-kernel, perf, and objtool. This fixes all three locations.
Eventually we should probably try to at least converge the two separate
"tools" locations into a single shared location.
Signed-off-by: Josh Poimboeuf <[email protected]>
Cc: Adrian Hunter <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Masami Hiramatsu <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephen Rothwell <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Link: http://lkml.kernel.org/r/9dd4161719b20e6def9564646d68bfbe498c549f.1456962210.git.jpoimboe@redhat.com
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/lib/insn.c | 6 +++---
tools/objtool/arch/x86/insn/insn.c | 6 +++---
tools/perf/util/intel-pt-decoder/insn.c | 6 +++---
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/arch/x86/lib/insn.c b/arch/x86/lib/insn.c
index 8f72b33..1a41693 100644
--- a/arch/x86/lib/insn.c
+++ b/arch/x86/lib/insn.c
@@ -374,7 +374,7 @@ void insn_get_displacement(struct insn *insn)
if (mod == 3)
goto out;
if (mod == 1) {
- insn->displacement.value = get_next(char, insn);
+ insn->displacement.value = get_next(signed char, insn);
insn->displacement.nbytes = 1;
} else if (insn->addr_bytes == 2) {
if ((mod == 0 && rm == 6) || mod == 2) {
@@ -532,7 +532,7 @@ void insn_get_immediate(struct insn *insn)
switch (inat_immediate_size(insn->attr)) {
case INAT_IMM_BYTE:
- insn->immediate.value = get_next(char, insn);
+ insn->immediate.value = get_next(signed char, insn);
insn->immediate.nbytes = 1;
break;
case INAT_IMM_WORD:
@@ -566,7 +566,7 @@ void insn_get_immediate(struct insn *insn)
goto err_out;
}
if (inat_has_second_immediate(insn->attr)) {
- insn->immediate2.value = get_next(char, insn);
+ insn->immediate2.value = get_next(signed char, insn);
insn->immediate2.nbytes = 1;
}
done:
diff --git a/tools/objtool/arch/x86/insn/insn.c b/tools/objtool/arch/x86/insn/insn.c
index 47314a6..9f26eae 100644
--- a/tools/objtool/arch/x86/insn/insn.c
+++ b/tools/objtool/arch/x86/insn/insn.c
@@ -374,7 +374,7 @@ void insn_get_displacement(struct insn *insn)
if (mod == 3)
goto out;
if (mod == 1) {
- insn->displacement.value = get_next(char, insn);
+ insn->displacement.value = get_next(signed char, insn);
insn->displacement.nbytes = 1;
} else if (insn->addr_bytes == 2) {
if ((mod == 0 && rm == 6) || mod == 2) {
@@ -532,7 +532,7 @@ void insn_get_immediate(struct insn *insn)
switch (inat_immediate_size(insn->attr)) {
case INAT_IMM_BYTE:
- insn->immediate.value = get_next(char, insn);
+ insn->immediate.value = get_next(signed char, insn);
insn->immediate.nbytes = 1;
break;
case INAT_IMM_WORD:
@@ -566,7 +566,7 @@ void insn_get_immediate(struct insn *insn)
goto err_out;
}
if (inat_has_second_immediate(insn->attr)) {
- insn->immediate2.value = get_next(char, insn);
+ insn->immediate2.value = get_next(signed char, insn);
insn->immediate2.nbytes = 1;
}
done:
diff --git a/tools/perf/util/intel-pt-decoder/insn.c b/tools/perf/util/intel-pt-decoder/insn.c
index 47314a6..9f26eae 100644
--- a/tools/perf/util/intel-pt-decoder/insn.c
+++ b/tools/perf/util/intel-pt-decoder/insn.c
@@ -374,7 +374,7 @@ void insn_get_displacement(struct insn *insn)
if (mod == 3)
goto out;
if (mod == 1) {
- insn->displacement.value = get_next(char, insn);
+ insn->displacement.value = get_next(signed char, insn);
insn->displacement.nbytes = 1;
} else if (insn->addr_bytes == 2) {
if ((mod == 0 && rm == 6) || mod == 2) {
@@ -532,7 +532,7 @@ void insn_get_immediate(struct insn *insn)
switch (inat_immediate_size(insn->attr)) {
case INAT_IMM_BYTE:
- insn->immediate.value = get_next(char, insn);
+ insn->immediate.value = get_next(signed char, insn);
insn->immediate.nbytes = 1;
break;
case INAT_IMM_WORD:
@@ -566,7 +566,7 @@ void insn_get_immediate(struct insn *insn)
goto err_out;
}
if (inat_has_second_immediate(insn->attr)) {
- insn->immediate2.value = get_next(char, insn);
+ insn->immediate2.value = get_next(signed char, insn);
insn->immediate2.nbytes = 1;
}
done:
Commit-ID: c1d45c3abd49b5bf9447e435099c1b000dcde752
Gitweb: http://git.kernel.org/tip/c1d45c3abd49b5bf9447e435099c1b000dcde752
Author: Josh Poimboeuf <[email protected]>
AuthorDate: Wed, 2 Mar 2016 18:39:37 -0600
Committer: Ingo Molnar <[email protected]>
CommitDate: Thu, 3 Mar 2016 16:13:00 +0100
objtool: Support CROSS_COMPILE
When building with CONFIG_STACK_VALIDATION on a ppc64le host with an x86
cross-compiler, Stephen Rothwell saw the following objtool build errors:
DESCEND objtool
CC /home/sfr/next/x86_64_allmodconfig/tools/objtool/builtin-check.o
CC /home/sfr/next/x86_64_allmodconfig/tools/objtool/special.o
CC /home/sfr/next/x86_64_allmodconfig/tools/objtool/elf.o
CC /home/sfr/next/x86_64_allmodconfig/tools/objtool/objtool.o
MKDIR /home/sfr/next/x86_64_allmodconfig/tools/objtool/arch/x86/insn/
CC /home/sfr/next/x86_64_allmodconfig/tools/objtool/libstring.o
elf.c:22:23: fatal error: sys/types.h: No such file or directory
compilation terminated.
CC /home/sfr/next/x86_64_allmodconfig/tools/objtool/exec-cmd.o
CC /home/sfr/next/x86_64_allmodconfig/tools/objtool/help.o
builtin-check.c:28:20: fatal error: string.h: No such file or directory
compilation terminated.
objtool.c:28:19: fatal error: stdio.h: No such file or directory
compilation terminated.
It fails to build because it tries to compile objtool with the
cross-compiler instead of the host compiler.
Ensure that it always uses the host compiler by ignoring CROSS_COMPILE.
In order to do that properly, the libsubcmd.a library needs to be built
in tools/objtool/ rather than tools/lib/subcmd/. The latter directory
contains the cross-compiled version which is needed for perf and
possibly other tools.
Note that cross-compiling for x86 on a _big_ endian system would result
in a bunch of false positive objtool warnings during the kernel build
because it isn't endian-aware. But that's generally a rare edge case
and there haven't been any reports of anybody needing that.
Reported-by: Stephen Rothwell <[email protected]>
Signed-off-by: Josh Poimboeuf <[email protected]>
Cc: Adrian Hunter <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Masami Hiramatsu <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Link: http://lkml.kernel.org/r/55b63eefc347f1bb28573f972d8d1adbf1f1c31d.1456962210.git.jpoimboe@redhat.com
Signed-off-by: Ingo Molnar <[email protected]>
---
tools/lib/subcmd/Makefile | 6 ++++--
tools/objtool/Makefile | 17 ++++++++++-------
2 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/tools/lib/subcmd/Makefile b/tools/lib/subcmd/Makefile
index 629cf8c..1faecb8 100644
--- a/tools/lib/subcmd/Makefile
+++ b/tools/lib/subcmd/Makefile
@@ -8,8 +8,10 @@ srctree := $(patsubst %/,%,$(dir $(srctree)))
#$(info Determined 'srctree' to be $(srctree))
endif
-CC = $(CROSS_COMPILE)gcc
-AR = $(CROSS_COMPILE)ar
+CC ?= $(CROSS_COMPILE)gcc
+LD ?= $(CROSS_COMPILE)ld
+AR ?= $(CROSS_COMPILE)ar
+
RM = rm -f
MAKEFLAGS += --no-print-directory
diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
index c4f0713..e4a6bd5 100644
--- a/tools/objtool/Makefile
+++ b/tools/objtool/Makefile
@@ -7,13 +7,19 @@ ARCH := x86
endif
endif
+# always use the host compiler
+CC = gcc
+LD = ld
+AR = ar
+
ifeq ($(srctree),)
srctree := $(patsubst %/,%,$(dir $(shell pwd)))
srctree := $(patsubst %/,%,$(dir $(srctree)))
endif
-SUBCMD_SRCDIR = $(srctree)/tools/lib/subcmd/
-LIBSUBCMD = $(if $(OUTPUT),$(OUTPUT),$(SUBCMD_SRCDIR))libsubcmd.a
+SUBCMD_SRCDIR = $(srctree)/tools/lib/subcmd/
+LIBSUBCMD_OUTPUT = $(if $(OUTPUT),$(OUTPUT),$(PWD)/)
+LIBSUBCMD = $(LIBSUBCMD_OUTPUT)libsubcmd.a
OBJTOOL := $(OUTPUT)objtool
OBJTOOL_IN := $(OBJTOOL)-in.o
@@ -45,12 +51,9 @@ $(OBJTOOL): $(LIBSUBCMD) $(OBJTOOL_IN)
$(LIBSUBCMD): fixdep FORCE
- $(Q)$(MAKE) -C $(SUBCMD_SRCDIR)
-
-$(LIBSUBCMD)-clean:
- $(Q)$(MAKE) -C $(SUBCMD_SRCDIR) clean > /dev/null
+ $(Q)$(MAKE) -C $(SUBCMD_SRCDIR) OUTPUT=$(LIBSUBCMD_OUTPUT)
-clean: $(LIBSUBCMD)-clean
+clean:
$(call QUIET_CLEAN, objtool) $(RM) $(OBJTOOL)
$(Q)find $(OUTPUT) -name '*.o' -delete -o -name '\.*.cmd' -delete -o -name '\.*.d' -delete
$(Q)$(RM) $(OUTPUT)arch/x86/insn/inat-tables.c $(OUTPUT)fixdep
On March 3, 2016 8:51:39 AM PST, tip-bot for Josh Poimboeuf <[email protected]> wrote:
>Commit-ID: 19072f23d1d785c093b7f81cb1fb161e7a13ecc0
>Gitweb:
>http://git.kernel.org/tip/19072f23d1d785c093b7f81cb1fb161e7a13ecc0
>Author: Josh Poimboeuf <[email protected]>
>AuthorDate: Wed, 2 Mar 2016 18:39:36 -0600
>Committer: Ingo Molnar <[email protected]>
>CommitDate: Thu, 3 Mar 2016 16:13:00 +0100
>
>x86/asm/decoder: Use explicitly signed chars
>
>When running objtool on a ppc64le host to analyze x86 binaries, it
>reports a lot of false warnings like:
>
>ipc/compat_mq.o: warning: objtool: compat_SyS_mq_open()+0x91: can't
>find jump dest instruction at .text+0x3a5
>
>The warnings are caused by the x86 instruction decoder setting the
>wrong
>value for the jump instruction's immediate field because it assumes
>that
>"char == signed char", which isn't true for all architectures. When
>converting char to int, gcc sign-extends on x86 but doesn't sign-extend
>on ppc64le.
>
>According to the gcc man page, that's a feature, not a bug:
>
> > Each kind of machine has a default for what "char" should be. It is
> > either like "unsigned char" by default or like "signed char" by
> > default.
> >
> > Ideally, a portable program should always use "signed char" or
> > "unsigned char" when it depends on the signedness of an object.
>
>Conform to the "standards" by changing the "char" casts to "signed
>char". This results in no actual changes to the object code on x86.
>
>Note: the x86 decoder now lives in three different locations in the
>kernel tree, which are all kept in sync via makefile checks and
>warnings: in-kernel, perf, and objtool. This fixes all three
>locations.
>Eventually we should probably try to at least converge the two separate
>"tools" locations into a single shared location.
>
>Signed-off-by: Josh Poimboeuf <[email protected]>
>Cc: Adrian Hunter <[email protected]>
>Cc: Linus Torvalds <[email protected]>
>Cc: Masami Hiramatsu <[email protected]>
>Cc: Michael Ellerman <[email protected]>
>Cc: Peter Zijlstra <[email protected]>
>Cc: Stephen Rothwell <[email protected]>
>Cc: Thomas Gleixner <[email protected]>
>Link:
>http://lkml.kernel.org/r/9dd4161719b20e6def9564646d68bfbe498c549f.1456962210.git.jpoimboe@redhat.com
>Signed-off-by: Ingo Molnar <[email protected]>
>---
> arch/x86/lib/insn.c | 6 +++---
> tools/objtool/arch/x86/insn/insn.c | 6 +++---
> tools/perf/util/intel-pt-decoder/insn.c | 6 +++---
> 3 files changed, 9 insertions(+), 9 deletions(-)
>
>diff --git a/arch/x86/lib/insn.c b/arch/x86/lib/insn.c
>index 8f72b33..1a41693 100644
>--- a/arch/x86/lib/insn.c
>+++ b/arch/x86/lib/insn.c
>@@ -374,7 +374,7 @@ void insn_get_displacement(struct insn *insn)
> if (mod == 3)
> goto out;
> if (mod == 1) {
>- insn->displacement.value = get_next(char, insn);
>+ insn->displacement.value = get_next(signed char, insn);
> insn->displacement.nbytes = 1;
> } else if (insn->addr_bytes == 2) {
> if ((mod == 0 && rm == 6) || mod == 2) {
>@@ -532,7 +532,7 @@ void insn_get_immediate(struct insn *insn)
>
> switch (inat_immediate_size(insn->attr)) {
> case INAT_IMM_BYTE:
>- insn->immediate.value = get_next(char, insn);
>+ insn->immediate.value = get_next(signed char, insn);
> insn->immediate.nbytes = 1;
> break;
> case INAT_IMM_WORD:
>@@ -566,7 +566,7 @@ void insn_get_immediate(struct insn *insn)
> goto err_out;
> }
> if (inat_has_second_immediate(insn->attr)) {
>- insn->immediate2.value = get_next(char, insn);
>+ insn->immediate2.value = get_next(signed char, insn);
> insn->immediate2.nbytes = 1;
> }
> done:
>diff --git a/tools/objtool/arch/x86/insn/insn.c
>b/tools/objtool/arch/x86/insn/insn.c
>index 47314a6..9f26eae 100644
>--- a/tools/objtool/arch/x86/insn/insn.c
>+++ b/tools/objtool/arch/x86/insn/insn.c
>@@ -374,7 +374,7 @@ void insn_get_displacement(struct insn *insn)
> if (mod == 3)
> goto out;
> if (mod == 1) {
>- insn->displacement.value = get_next(char, insn);
>+ insn->displacement.value = get_next(signed char, insn);
> insn->displacement.nbytes = 1;
> } else if (insn->addr_bytes == 2) {
> if ((mod == 0 && rm == 6) || mod == 2) {
>@@ -532,7 +532,7 @@ void insn_get_immediate(struct insn *insn)
>
> switch (inat_immediate_size(insn->attr)) {
> case INAT_IMM_BYTE:
>- insn->immediate.value = get_next(char, insn);
>+ insn->immediate.value = get_next(signed char, insn);
> insn->immediate.nbytes = 1;
> break;
> case INAT_IMM_WORD:
>@@ -566,7 +566,7 @@ void insn_get_immediate(struct insn *insn)
> goto err_out;
> }
> if (inat_has_second_immediate(insn->attr)) {
>- insn->immediate2.value = get_next(char, insn);
>+ insn->immediate2.value = get_next(signed char, insn);
> insn->immediate2.nbytes = 1;
> }
> done:
>diff --git a/tools/perf/util/intel-pt-decoder/insn.c
>b/tools/perf/util/intel-pt-decoder/insn.c
>index 47314a6..9f26eae 100644
>--- a/tools/perf/util/intel-pt-decoder/insn.c
>+++ b/tools/perf/util/intel-pt-decoder/insn.c
>@@ -374,7 +374,7 @@ void insn_get_displacement(struct insn *insn)
> if (mod == 3)
> goto out;
> if (mod == 1) {
>- insn->displacement.value = get_next(char, insn);
>+ insn->displacement.value = get_next(signed char, insn);
> insn->displacement.nbytes = 1;
> } else if (insn->addr_bytes == 2) {
> if ((mod == 0 && rm == 6) || mod == 2) {
>@@ -532,7 +532,7 @@ void insn_get_immediate(struct insn *insn)
>
> switch (inat_immediate_size(insn->attr)) {
> case INAT_IMM_BYTE:
>- insn->immediate.value = get_next(char, insn);
>+ insn->immediate.value = get_next(signed char, insn);
> insn->immediate.nbytes = 1;
> break;
> case INAT_IMM_WORD:
>@@ -566,7 +566,7 @@ void insn_get_immediate(struct insn *insn)
> goto err_out;
> }
> if (inat_has_second_immediate(insn->attr)) {
>- insn->immediate2.value = get_next(char, insn);
>+ insn->immediate2.value = get_next(signed char, insn);
> insn->immediate2.nbytes = 1;
> }
> done:
It ought to be made specific as __s8 (or int8_t) really...
--
Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.
Hi Ingo,
On Thu, 3 Mar 2016 16:10:21 +0100 Ingo Molnar <[email protected]> wrote:
>
> I'll add Josh's fixes to -tip ASAP as well, so hopefully soon you can drop all
> linux-next specific patches related to this and it will all work Just Fine (tm).
Thanks for that. I have now dropped these patches.
> Sorry about the breakage!
Hey, it happens ..
--
Cheers,
Stephen Rothwell