2020-10-07 16:25:35

by tip-bot2 for Alexey Makhalov

Subject: [tip: objtool/core] x86/insn: Support big endian cross-compiles

The following commit has been merged into the objtool/core branch of tip:

Commit-ID: 2a522b53c47051d3bf98748418f4f8e5f20d2c04
Gitweb: https://git.kernel.org/tip/2a522b53c47051d3bf98748418f4f8e5f20d2c04
Author: Martin Schwidefsky <[email protected]>
AuthorDate: Mon, 05 Oct 2020 17:50:31 +02:00
Committer: Josh Poimboeuf <[email protected]>
CommitterDate: Tue, 06 Oct 2020 09:32:29 -05:00

x86/insn: Support big endian cross-compiles

x86 instruction decoder code is shared across the kernel source and the
tools. Currently objtool seems to be the only tool from build tools needed
which breaks x86 cross compilation on big endian systems. Make the x86
instruction decoder build host endianness agnostic to support x86 cross
compilation and enable objtool to implement endianness awareness for
big endian architectures support.

Signed-off-by: Martin Schwidefsky <[email protected]>
Co-developed-by: Vasily Gorbik <[email protected]>
Signed-off-by: Vasily Gorbik <[email protected]>
Acked-by: Masami Hiramatsu <[email protected]>
Signed-off-by: Josh Poimboeuf <[email protected]>
---
arch/x86/include/asm/insn.h | 33 +++++++++-
arch/x86/lib/insn.c | 101 +++++++++++++----------------
tools/arch/x86/include/asm/insn.h | 33 +++++++++-
tools/arch/x86/lib/insn.c | 101 +++++++++++++----------------
4 files changed, 160 insertions(+), 108 deletions(-)

diff --git a/arch/x86/include/asm/insn.h b/arch/x86/include/asm/insn.h
index 5c1ae3e..004e27b 100644
--- a/arch/x86/include/asm/insn.h
+++ b/arch/x86/include/asm/insn.h
@@ -7,9 +7,12 @@
* Copyright (C) IBM Corporation, 2009
*/

+#include <asm/byteorder.h>
/* insn_attr_t is defined in inat.h */
#include <asm/inat.h>

+#if defined(__BYTE_ORDER) ? __BYTE_ORDER == __LITTLE_ENDIAN : defined(__LITTLE_ENDIAN)
+
struct insn_field {
union {
insn_value_t value;
@@ -20,6 +23,36 @@ struct insn_field {
unsigned char nbytes;
};

+static inline void insn_field_set(struct insn_field *p, insn_value_t v,
+ unsigned char n)
+{
+ p->value = v;
+ p->nbytes = n;
+}
+
+#else
+
+struct insn_field {
+ insn_value_t value;
+ union {
+ insn_value_t little;
+ insn_byte_t bytes[4];
+ };
+ /* !0 if we've run insn_get_xxx() for this field */
+ unsigned char got;
+ unsigned char nbytes;
+};
+
+static inline void insn_field_set(struct insn_field *p, insn_value_t v,
+ unsigned char n)
+{
+ p->value = v;
+ p->little = __cpu_to_le32(v);
+ p->nbytes = n;
+}
+
+#endif
+
struct insn {
struct insn_field prefixes; /*
* Prefixes
diff --git a/arch/x86/lib/insn.c b/arch/x86/lib/insn.c
index 4042795..520b31f 100644
--- a/arch/x86/lib/insn.c
+++ b/arch/x86/lib/insn.c
@@ -5,6 +5,7 @@
* Copyright (C) IBM Corporation, 2002, 2004, 2009
*/

+#include <linux/kernel.h>
#ifdef __KERNEL__
#include <linux/string.h>
#else
@@ -15,15 +16,28 @@

#include <asm/emulate_prefix.h>

+#define leXX_to_cpu(t, r) \
+({ \
+ __typeof__(t) v; \
+ switch (sizeof(t)) { \
+ case 4: v = le32_to_cpu(r); break; \
+ case 2: v = le16_to_cpu(r); break; \
+ case 1: v = r; break; \
+ default: \
+ BUILD_BUG(); break; \
+ } \
+ v; \
+})
+
/* Verify next sizeof(t) bytes can be on the same instruction */
#define validate_next(t, insn, n) \
((insn)->next_byte + sizeof(t) + n <= (insn)->end_kaddr)

#define __get_next(t, insn) \
- ({ t r = *(t*)insn->next_byte; insn->next_byte += sizeof(t); r; })
+ ({ t r = *(t*)insn->next_byte; insn->next_byte += sizeof(t); leXX_to_cpu(t, r); })

#define __peek_nbyte_next(t, insn, n) \
- ({ t r = *(t*)((insn)->next_byte + n); r; })
+ ({ t r = *(t*)((insn)->next_byte + n); leXX_to_cpu(t, r); })

#define get_next(t, insn) \
({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
@@ -157,8 +171,7 @@ found:
b = peek_next(insn_byte_t, insn);
attr = inat_get_opcode_attribute(b);
if (inat_is_rex_prefix(attr)) {
- insn->rex_prefix.value = b;
- insn->rex_prefix.nbytes = 1;
+ insn_field_set(&insn->rex_prefix, b, 1);
insn->next_byte++;
if (X86_REX_W(b))
/* REX.W overrides opnd_size */
@@ -295,8 +308,7 @@ void insn_get_modrm(struct insn *insn)

if (inat_has_modrm(insn->attr)) {
mod = get_next(insn_byte_t, insn);
- modrm->value = mod;
- modrm->nbytes = 1;
+ insn_field_set(modrm, mod, 1);
if (inat_is_group(insn->attr)) {
pfx_id = insn_last_prefix_id(insn);
insn->attr = inat_get_group_attribute(mod, pfx_id,
@@ -334,7 +346,7 @@ int insn_rip_relative(struct insn *insn)
* For rip-relative instructions, the mod field (top 2 bits)
* is zero and the r/m field (bottom 3 bits) is 0x5.
*/
- return (modrm->nbytes && (modrm->value & 0xc7) == 0x5);
+ return (modrm->nbytes && (modrm->bytes[0] & 0xc7) == 0x5);
}

/**
@@ -353,11 +365,11 @@ void insn_get_sib(struct insn *insn)
if (!insn->modrm.got)
insn_get_modrm(insn);
if (insn->modrm.nbytes) {
- modrm = (insn_byte_t)insn->modrm.value;
+ modrm = insn->modrm.bytes[0];
if (insn->addr_bytes != 2 &&
X86_MODRM_MOD(modrm) != 3 && X86_MODRM_RM(modrm) == 4) {
- insn->sib.value = get_next(insn_byte_t, insn);
- insn->sib.nbytes = 1;
+ insn_field_set(&insn->sib,
+ get_next(insn_byte_t, insn), 1);
}
}
insn->sib.got = 1;
@@ -407,19 +419,18 @@ void insn_get_displacement(struct insn *insn)
if (mod == 3)
goto out;
if (mod == 1) {
- insn->displacement.value = get_next(signed char, insn);
- insn->displacement.nbytes = 1;
+ insn_field_set(&insn->displacement,
+ get_next(signed char, insn), 1);
} else if (insn->addr_bytes == 2) {
if ((mod == 0 && rm == 6) || mod == 2) {
- insn->displacement.value =
- get_next(short, insn);
- insn->displacement.nbytes = 2;
+ insn_field_set(&insn->displacement,
+ get_next(short, insn), 2);
}
} else {
if ((mod == 0 && rm == 5) || mod == 2 ||
(mod == 0 && base == 5)) {
- insn->displacement.value = get_next(int, insn);
- insn->displacement.nbytes = 4;
+ insn_field_set(&insn->displacement,
+ get_next(int, insn), 4);
}
}
}
@@ -435,18 +446,14 @@ static int __get_moffset(struct insn *insn)
{
switch (insn->addr_bytes) {
case 2:
- insn->moffset1.value = get_next(short, insn);
- insn->moffset1.nbytes = 2;
+ insn_field_set(&insn->moffset1, get_next(short, insn), 2);
break;
case 4:
- insn->moffset1.value = get_next(int, insn);
- insn->moffset1.nbytes = 4;
+ insn_field_set(&insn->moffset1, get_next(int, insn), 4);
break;
case 8:
- insn->moffset1.value = get_next(int, insn);
- insn->moffset1.nbytes = 4;
- insn->moffset2.value = get_next(int, insn);
- insn->moffset2.nbytes = 4;
+ insn_field_set(&insn->moffset1, get_next(int, insn), 4);
+ insn_field_set(&insn->moffset2, get_next(int, insn), 4);
break;
default: /* opnd_bytes must be modified manually */
goto err_out;
@@ -464,13 +471,11 @@ static int __get_immv32(struct insn *insn)
{
switch (insn->opnd_bytes) {
case 2:
- insn->immediate.value = get_next(short, insn);
- insn->immediate.nbytes = 2;
+ insn_field_set(&insn->immediate, get_next(short, insn), 2);
break;
case 4:
case 8:
- insn->immediate.value = get_next(int, insn);
- insn->immediate.nbytes = 4;
+ insn_field_set(&insn->immediate, get_next(int, insn), 4);
break;
default: /* opnd_bytes must be modified manually */
goto err_out;
@@ -487,18 +492,15 @@ static int __get_immv(struct insn *insn)
{
switch (insn->opnd_bytes) {
case 2:
- insn->immediate1.value = get_next(short, insn);
- insn->immediate1.nbytes = 2;
+ insn_field_set(&insn->immediate1, get_next(short, insn), 2);
break;
case 4:
- insn->immediate1.value = get_next(int, insn);
+ insn_field_set(&insn->immediate1, get_next(int, insn), 4);
insn->immediate1.nbytes = 4;
break;
case 8:
- insn->immediate1.value = get_next(int, insn);
- insn->immediate1.nbytes = 4;
- insn->immediate2.value = get_next(int, insn);
- insn->immediate2.nbytes = 4;
+ insn_field_set(&insn->immediate1, get_next(int, insn), 4);
+ insn_field_set(&insn->immediate2, get_next(int, insn), 4);
break;
default: /* opnd_bytes must be modified manually */
goto err_out;
@@ -515,12 +517,10 @@ static int __get_immptr(struct insn *insn)
{
switch (insn->opnd_bytes) {
case 2:
- insn->immediate1.value = get_next(short, insn);
- insn->immediate1.nbytes = 2;
+ insn_field_set(&insn->immediate1, get_next(short, insn), 2);
break;
case 4:
- insn->immediate1.value = get_next(int, insn);
- insn->immediate1.nbytes = 4;
+ insn_field_set(&insn->immediate1, get_next(int, insn), 4);
break;
case 8:
/* ptr16:64 is not exist (no segment) */
@@ -528,8 +528,7 @@ static int __get_immptr(struct insn *insn)
default: /* opnd_bytes must be modified manually */
goto err_out;
}
- insn->immediate2.value = get_next(unsigned short, insn);
- insn->immediate2.nbytes = 2;
+ insn_field_set(&insn->immediate2, get_next(unsigned short, insn), 2);
insn->immediate1.got = insn->immediate2.got = 1;

return 1;
@@ -565,22 +564,17 @@ void insn_get_immediate(struct insn *insn)

switch (inat_immediate_size(insn->attr)) {
case INAT_IMM_BYTE:
- insn->immediate.value = get_next(signed char, insn);
- insn->immediate.nbytes = 1;
+ insn_field_set(&insn->immediate, get_next(signed char, insn), 1);
break;
case INAT_IMM_WORD:
- insn->immediate.value = get_next(short, insn);
- insn->immediate.nbytes = 2;
+ insn_field_set(&insn->immediate, get_next(short, insn), 2);
break;
case INAT_IMM_DWORD:
- insn->immediate.value = get_next(int, insn);
- insn->immediate.nbytes = 4;
+ insn_field_set(&insn->immediate, get_next(int, insn), 4);
break;
case INAT_IMM_QWORD:
- insn->immediate1.value = get_next(int, insn);
- insn->immediate1.nbytes = 4;
- insn->immediate2.value = get_next(int, insn);
- insn->immediate2.nbytes = 4;
+ insn_field_set(&insn->immediate1, get_next(int, insn), 4);
+ insn_field_set(&insn->immediate2, get_next(int, insn), 4);
break;
case INAT_IMM_PTR:
if (!__get_immptr(insn))
@@ -599,8 +593,7 @@ void insn_get_immediate(struct insn *insn)
goto err_out;
}
if (inat_has_second_immediate(insn->attr)) {
- insn->immediate2.value = get_next(signed char, insn);
- insn->immediate2.nbytes = 1;
+ insn_field_set(&insn->immediate2, get_next(signed char, insn), 1);
}
done:
insn->immediate.got = 1;
diff --git a/tools/arch/x86/include/asm/insn.h b/tools/arch/x86/include/asm/insn.h
index 568854b..b9b6928 100644
--- a/tools/arch/x86/include/asm/insn.h
+++ b/tools/arch/x86/include/asm/insn.h
@@ -7,9 +7,12 @@
* Copyright (C) IBM Corporation, 2009
*/

+#include <asm/byteorder.h>
/* insn_attr_t is defined in inat.h */
#include "inat.h"

+#if defined(__BYTE_ORDER) ? __BYTE_ORDER == __LITTLE_ENDIAN : defined(__LITTLE_ENDIAN)
+
struct insn_field {
union {
insn_value_t value;
@@ -20,6 +23,36 @@ struct insn_field {
unsigned char nbytes;
};

+static inline void insn_field_set(struct insn_field *p, insn_value_t v,
+ unsigned char n)
+{
+ p->value = v;
+ p->nbytes = n;
+}
+
+#else
+
+struct insn_field {
+ insn_value_t value;
+ union {
+ insn_value_t little;
+ insn_byte_t bytes[4];
+ };
+ /* !0 if we've run insn_get_xxx() for this field */
+ unsigned char got;
+ unsigned char nbytes;
+};
+
+static inline void insn_field_set(struct insn_field *p, insn_value_t v,
+ unsigned char n)
+{
+ p->value = v;
+ p->little = __cpu_to_le32(v);
+ p->nbytes = n;
+}
+
+#endif
+
struct insn {
struct insn_field prefixes; /*
* Prefixes
diff --git a/tools/arch/x86/lib/insn.c b/tools/arch/x86/lib/insn.c
index 0151dfc..77e92aa 100644
--- a/tools/arch/x86/lib/insn.c
+++ b/tools/arch/x86/lib/insn.c
@@ -5,6 +5,7 @@
* Copyright (C) IBM Corporation, 2002, 2004, 2009
*/

+#include <linux/kernel.h>
#ifdef __KERNEL__
#include <linux/string.h>
#else
@@ -15,15 +16,28 @@

#include "../include/asm/emulate_prefix.h"

+#define leXX_to_cpu(t, r) \
+({ \
+ __typeof__(t) v; \
+ switch (sizeof(t)) { \
+ case 4: v = le32_to_cpu(r); break; \
+ case 2: v = le16_to_cpu(r); break; \
+ case 1: v = r; break; \
+ default: \
+ BUILD_BUG(); break; \
+ } \
+ v; \
+})
+
/* Verify next sizeof(t) bytes can be on the same instruction */
#define validate_next(t, insn, n) \
((insn)->next_byte + sizeof(t) + n <= (insn)->end_kaddr)

#define __get_next(t, insn) \
- ({ t r = *(t*)insn->next_byte; insn->next_byte += sizeof(t); r; })
+ ({ t r = *(t*)insn->next_byte; insn->next_byte += sizeof(t); leXX_to_cpu(t, r); })

#define __peek_nbyte_next(t, insn, n) \
- ({ t r = *(t*)((insn)->next_byte + n); r; })
+ ({ t r = *(t*)((insn)->next_byte + n); leXX_to_cpu(t, r); })

#define get_next(t, insn) \
({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
@@ -157,8 +171,7 @@ found:
b = peek_next(insn_byte_t, insn);
attr = inat_get_opcode_attribute(b);
if (inat_is_rex_prefix(attr)) {
- insn->rex_prefix.value = b;
- insn->rex_prefix.nbytes = 1;
+ insn_field_set(&insn->rex_prefix, b, 1);
insn->next_byte++;
if (X86_REX_W(b))
/* REX.W overrides opnd_size */
@@ -295,8 +308,7 @@ void insn_get_modrm(struct insn *insn)

if (inat_has_modrm(insn->attr)) {
mod = get_next(insn_byte_t, insn);
- modrm->value = mod;
- modrm->nbytes = 1;
+ insn_field_set(modrm, mod, 1);
if (inat_is_group(insn->attr)) {
pfx_id = insn_last_prefix_id(insn);
insn->attr = inat_get_group_attribute(mod, pfx_id,
@@ -334,7 +346,7 @@ int insn_rip_relative(struct insn *insn)
* For rip-relative instructions, the mod field (top 2 bits)
* is zero and the r/m field (bottom 3 bits) is 0x5.
*/
- return (modrm->nbytes && (modrm->value & 0xc7) == 0x5);
+ return (modrm->nbytes && (modrm->bytes[0] & 0xc7) == 0x5);
}

/**
@@ -353,11 +365,11 @@ void insn_get_sib(struct insn *insn)
if (!insn->modrm.got)
insn_get_modrm(insn);
if (insn->modrm.nbytes) {
- modrm = (insn_byte_t)insn->modrm.value;
+ modrm = insn->modrm.bytes[0];
if (insn->addr_bytes != 2 &&
X86_MODRM_MOD(modrm) != 3 && X86_MODRM_RM(modrm) == 4) {
- insn->sib.value = get_next(insn_byte_t, insn);
- insn->sib.nbytes = 1;
+ insn_field_set(&insn->sib,
+ get_next(insn_byte_t, insn), 1);
}
}
insn->sib.got = 1;
@@ -407,19 +419,18 @@ void insn_get_displacement(struct insn *insn)
if (mod == 3)
goto out;
if (mod == 1) {
- insn->displacement.value = get_next(signed char, insn);
- insn->displacement.nbytes = 1;
+ insn_field_set(&insn->displacement,
+ get_next(signed char, insn), 1);
} else if (insn->addr_bytes == 2) {
if ((mod == 0 && rm == 6) || mod == 2) {
- insn->displacement.value =
- get_next(short, insn);
- insn->displacement.nbytes = 2;
+ insn_field_set(&insn->displacement,
+ get_next(short, insn), 2);
}
} else {
if ((mod == 0 && rm == 5) || mod == 2 ||
(mod == 0 && base == 5)) {
- insn->displacement.value = get_next(int, insn);
- insn->displacement.nbytes = 4;
+ insn_field_set(&insn->displacement,
+ get_next(int, insn), 4);
}
}
}
@@ -435,18 +446,14 @@ static int __get_moffset(struct insn *insn)
{
switch (insn->addr_bytes) {
case 2:
- insn->moffset1.value = get_next(short, insn);
- insn->moffset1.nbytes = 2;
+ insn_field_set(&insn->moffset1, get_next(short, insn), 2);
break;
case 4:
- insn->moffset1.value = get_next(int, insn);
- insn->moffset1.nbytes = 4;
+ insn_field_set(&insn->moffset1, get_next(int, insn), 4);
break;
case 8:
- insn->moffset1.value = get_next(int, insn);
- insn->moffset1.nbytes = 4;
- insn->moffset2.value = get_next(int, insn);
- insn->moffset2.nbytes = 4;
+ insn_field_set(&insn->moffset1, get_next(int, insn), 4);
+ insn_field_set(&insn->moffset2, get_next(int, insn), 4);
break;
default: /* opnd_bytes must be modified manually */
goto err_out;
@@ -464,13 +471,11 @@ static int __get_immv32(struct insn *insn)
{
switch (insn->opnd_bytes) {
case 2:
- insn->immediate.value = get_next(short, insn);
- insn->immediate.nbytes = 2;
+ insn_field_set(&insn->immediate, get_next(short, insn), 2);
break;
case 4:
case 8:
- insn->immediate.value = get_next(int, insn);
- insn->immediate.nbytes = 4;
+ insn_field_set(&insn->immediate, get_next(int, insn), 4);
break;
default: /* opnd_bytes must be modified manually */
goto err_out;
@@ -487,18 +492,15 @@ static int __get_immv(struct insn *insn)
{
switch (insn->opnd_bytes) {
case 2:
- insn->immediate1.value = get_next(short, insn);
- insn->immediate1.nbytes = 2;
+ insn_field_set(&insn->immediate1, get_next(short, insn), 2);
break;
case 4:
- insn->immediate1.value = get_next(int, insn);
+ insn_field_set(&insn->immediate1, get_next(int, insn), 4);
insn->immediate1.nbytes = 4;
break;
case 8:
- insn->immediate1.value = get_next(int, insn);
- insn->immediate1.nbytes = 4;
- insn->immediate2.value = get_next(int, insn);
- insn->immediate2.nbytes = 4;
+ insn_field_set(&insn->immediate1, get_next(int, insn), 4);
+ insn_field_set(&insn->immediate2, get_next(int, insn), 4);
break;
default: /* opnd_bytes must be modified manually */
goto err_out;
@@ -515,12 +517,10 @@ static int __get_immptr(struct insn *insn)
{
switch (insn->opnd_bytes) {
case 2:
- insn->immediate1.value = get_next(short, insn);
- insn->immediate1.nbytes = 2;
+ insn_field_set(&insn->immediate1, get_next(short, insn), 2);
break;
case 4:
- insn->immediate1.value = get_next(int, insn);
- insn->immediate1.nbytes = 4;
+ insn_field_set(&insn->immediate1, get_next(int, insn), 4);
break;
case 8:
/* ptr16:64 is not exist (no segment) */
@@ -528,8 +528,7 @@ static int __get_immptr(struct insn *insn)
default: /* opnd_bytes must be modified manually */
goto err_out;
}
- insn->immediate2.value = get_next(unsigned short, insn);
- insn->immediate2.nbytes = 2;
+ insn_field_set(&insn->immediate2, get_next(unsigned short, insn), 2);
insn->immediate1.got = insn->immediate2.got = 1;

return 1;
@@ -565,22 +564,17 @@ void insn_get_immediate(struct insn *insn)

switch (inat_immediate_size(insn->attr)) {
case INAT_IMM_BYTE:
- insn->immediate.value = get_next(signed char, insn);
- insn->immediate.nbytes = 1;
+ insn_field_set(&insn->immediate, get_next(signed char, insn), 1);
break;
case INAT_IMM_WORD:
- insn->immediate.value = get_next(short, insn);
- insn->immediate.nbytes = 2;
+ insn_field_set(&insn->immediate, get_next(short, insn), 2);
break;
case INAT_IMM_DWORD:
- insn->immediate.value = get_next(int, insn);
- insn->immediate.nbytes = 4;
+ insn_field_set(&insn->immediate, get_next(int, insn), 4);
break;
case INAT_IMM_QWORD:
- insn->immediate1.value = get_next(int, insn);
- insn->immediate1.nbytes = 4;
- insn->immediate2.value = get_next(int, insn);
- insn->immediate2.nbytes = 4;
+ insn_field_set(&insn->immediate1, get_next(int, insn), 4);
+ insn_field_set(&insn->immediate2, get_next(int, insn), 4);
break;
case INAT_IMM_PTR:
if (!__get_immptr(insn))
@@ -599,8 +593,7 @@ void insn_get_immediate(struct insn *insn)
goto err_out;
}
if (inat_has_second_immediate(insn->attr)) {
- insn->immediate2.value = get_next(signed char, insn);
- insn->immediate2.nbytes = 1;
+ insn_field_set(&insn->immediate2, get_next(signed char, insn), 1);
}
done:
insn->immediate.got = 1;

2020-10-10 03:59:22

by Peter Zijlstra

Subject: Re: [tip: objtool/core] x86/insn: Support big endian cross-compiles

On Wed, Oct 07, 2020 at 04:20:19PM -0000, tip-bot2 for Martin Schwidefsky wrote:
> The following commit has been merged into the objtool/core branch of tip:
>
> Commit-ID: 2a522b53c47051d3bf98748418f4f8e5f20d2c04
> Gitweb: https://git.kernel.org/tip/2a522b53c47051d3bf98748418f4f8e5f20d2c04
> Author: Martin Schwidefsky <[email protected]>
> AuthorDate: Mon, 05 Oct 2020 17:50:31 +02:00
> Committer: Josh Poimboeuf <[email protected]>
> CommitterDate: Tue, 06 Oct 2020 09:32:29 -05:00
>
> x86/insn: Support big endian cross-compiles
>
> x86 instruction decoder code is shared across the kernel source and the
> tools. Currently objtool seems to be the only tool from build tools needed
> which breaks x86 cross compilation on big endian systems. Make the x86
> instruction decoder build host endianness agnostic to support x86 cross
> compilation and enable objtool to implement endianness awareness for
> big endian architectures support.
>
> Signed-off-by: Martin Schwidefsky <[email protected]>
> Co-developed-by: Vasily Gorbik <[email protected]>
> Signed-off-by: Vasily Gorbik <[email protected]>
> Acked-by: Masami Hiramatsu <[email protected]>
> Signed-off-by: Josh Poimboeuf <[email protected]>

This commit breaks the x86 build with CONFIG_X86_DECODER_SELFTEST=y.

I've asked Boris to truncate tip/objtool/core.

2020-10-10 04:09:20

by Borislav Petkov

Subject: Re: [tip: objtool/core] x86/insn: Support big endian cross-compiles

On Fri, Oct 09, 2020 at 10:38:22PM +0200, Peter Zijlstra wrote:
> On Wed, Oct 07, 2020 at 04:20:19PM -0000, tip-bot2 for Martin Schwidefsky wrote:
> > The following commit has been merged into the objtool/core branch of tip:
> >
> > Commit-ID: 2a522b53c47051d3bf98748418f4f8e5f20d2c04
> > Gitweb: https://git.kernel.org/tip/2a522b53c47051d3bf98748418f4f8e5f20d2c04
> > Author: Martin Schwidefsky <[email protected]>
> > AuthorDate: Mon, 05 Oct 2020 17:50:31 +02:00
> > Committer: Josh Poimboeuf <[email protected]>
> > CommitterDate: Tue, 06 Oct 2020 09:32:29 -05:00
> >
> > x86/insn: Support big endian cross-compiles
> >
> > x86 instruction decoder code is shared across the kernel source and the
> > tools. Currently objtool seems to be the only tool from build tools needed
> > which breaks x86 cross compilation on big endian systems. Make the x86
> > instruction decoder build host endianness agnostic to support x86 cross
> > compilation and enable objtool to implement endianness awareness for
> > big endian architectures support.
> >
> > Signed-off-by: Martin Schwidefsky <[email protected]>
> > Co-developed-by: Vasily Gorbik <[email protected]>
> > Signed-off-by: Vasily Gorbik <[email protected]>
> > Acked-by: Masami Hiramatsu <[email protected]>
> > Signed-off-by: Josh Poimboeuf <[email protected]>
>
> This commit breaks the x86 build with CONFIG_X86_DECODER_SELFTEST=y.
>
> I've asked Boris to truncate tip/objtool/core.

Yeah, top 4 are gone until this is resolved.

What I would suggest is to have a look at how tools/ headers are kept
separate from kernel proper ones, see tools/include/ and how those
headers there are full of dummy definitions just so it builds.

And then including a global one like linux/kernel.h is just looking for
trouble:

In file included from ./include/uapi/linux/byteorder/little_endian.h:12,
from ./include/linux/byteorder/little_endian.h:5,
from /usr/include/x86_64-linux-gnu/asm/byteorder.h:5,
from ./arch/x86/include/asm/insn.h:10,
from arch/x86/tools/insn_sanity.c:21:
./tools/include/linux/types.h:30:18: error: conflicting types for ‘u64’
30 | typedef uint64_t u64;
| ^~~
In file included from /usr/include/asm-generic/types.h:7,
from /usr/include/x86_64-linux-gnu/asm/types.h:1,
from ./tools/include/linux/types.h:10,
from ./include/uapi/linux/byteorder/little_endian.h:12,
from ./include/linux/byteorder/little_endian.h:5,
from /usr/include/x86_64-linux-gnu/asm/byteorder.h:5,
from ./arch/x86/include/asm/insn.h:10,
from arch/x86/tools/insn_sanity.c:21:
./include/asm-generic/int-ll64.h:23:15: note: previous declaration of ‘u64’ was here
23 | typedef __u64 u64;
| ^~~
In file included from ./include/uapi/linux/byteorder/little_endian.h:12,
from ./include/linux/byteorder/little_endian.h:5,
from /usr/include/x86_64-linux-gnu/asm/byteorder.h:5,
from ./arch/x86/include/asm/insn.h:10,
from arch/x86/tools/insn_sanity.c:21:
./tools/include/linux/types.h:31:17: error: conflicting types for ‘s64’
31 | typedef int64_t s64;
| ^~~
In file included from /usr/include/asm-generic/types.h:7,
from /usr/include/x86_64-linux-gnu/asm/types.h:1,
from ./tools/include/linux/types.h:10,
from ./include/uapi/linux/byteorder/little_endian.h:12,
from ./include/linux/byteorder/little_endian.h:5,
from /usr/include/x86_64-linux-gnu/asm/byteorder.h:5,
from ./arch/x86/include/asm/insn.h:10,
from arch/x86/tools/insn_sanity.c:21:
./include/asm-generic/int-ll64.h:22:15: note: previous declaration of ‘s64’ was here
22 | typedef __s64 s64;
| ^~~
In file included from ./arch/x86/lib/insn.c:8,
from arch/x86/tools/insn_sanity.c:23:
./tools/include/linux/kernel.h:87: warning: "cpu_to_le16" redefined
87 | #define cpu_to_le16
|
In file included from ./include/linux/byteorder/little_endian.h:11,
from /usr/include/x86_64-linux-gnu/asm/byteorder.h:5,
from ./arch/x86/include/asm/insn.h:10,
from arch/x86/tools/insn_sanity.c:21:
./include/linux/byteorder/generic.h:90: note: this is the location of the previous definition
90 | #define cpu_to_le16 __cpu_to_le16
|
In file included from ./arch/x86/lib/insn.c:8,
from arch/x86/tools/insn_sanity.c:23:
./tools/include/linux/kernel.h:88: warning: "cpu_to_le32" redefined
88 | #define cpu_to_le32
|
In file included from ./include/linux/byteorder/little_endian.h:11,
from /usr/include/x86_64-linux-gnu/asm/byteorder.h:5,
from ./arch/x86/include/asm/insn.h:10,
from arch/x86/tools/insn_sanity.c:21:
./include/linux/byteorder/generic.h:88: note: this is the location of the previous definition
88 | #define cpu_to_le32 __cpu_to_le32
|
In file included from ./arch/x86/lib/insn.c:8,
from arch/x86/tools/insn_sanity.c:23:
./tools/include/linux/kernel.h:89: warning: "cpu_to_le64" redefined
89 | #define cpu_to_le64
|
In file included from ./include/linux/byteorder/little_endian.h:11,
from /usr/include/x86_64-linux-gnu/asm/byteorder.h:5,
from ./arch/x86/include/asm/insn.h:10,
from arch/x86/tools/insn_sanity.c:21:
./include/linux/byteorder/generic.h:86: note: this is the location of the previous definition
86 | #define cpu_to_le64 __cpu_to_le64
|
In file included from ./arch/x86/lib/insn.c:8,
from arch/x86/tools/insn_sanity.c:23:
./tools/include/linux/kernel.h:90: warning: "le16_to_cpu" redefined
90 | #define le16_to_cpu
|
In file included from ./include/linux/byteorder/little_endian.h:11,
from /usr/include/x86_64-linux-gnu/asm/byteorder.h:5,
from ./arch/x86/include/asm/insn.h:10,
from arch/x86/tools/insn_sanity.c:21:
./include/linux/byteorder/generic.h:91: note: this is the location of the previous definition
91 | #define le16_to_cpu __le16_to_cpu
|
In file included from ./arch/x86/lib/insn.c:8,
from arch/x86/tools/insn_sanity.c:23:
./tools/include/linux/kernel.h:91: warning: "le32_to_cpu" redefined
91 | #define le32_to_cpu
|
In file included from ./include/linux/byteorder/little_endian.h:11,
from /usr/include/x86_64-linux-gnu/asm/byteorder.h:5,
from ./arch/x86/include/asm/insn.h:10,
from arch/x86/tools/insn_sanity.c:21:
./include/linux/byteorder/generic.h:89: note: this is the location of the previous definition
89 | #define le32_to_cpu __le32_to_cpu
|
In file included from ./arch/x86/lib/insn.c:8,
from arch/x86/tools/insn_sanity.c:23:
./tools/include/linux/kernel.h:92: warning: "le64_to_cpu" redefined
92 | #define le64_to_cpu
|
In file included from ./include/linux/byteorder/little_endian.h:11,
from /usr/include/x86_64-linux-gnu/asm/byteorder.h:5,
from ./arch/x86/include/asm/insn.h:10,
from arch/x86/tools/insn_sanity.c:21:
./include/linux/byteorder/generic.h:87: note: this is the location of the previous definition
87 | #define le64_to_cpu __le64_to_cpu
|
In file included from ./arch/x86/lib/insn.c:8,
from arch/x86/tools/insn_sanity.c:23:
./tools/include/linux/kernel.h:93: warning: "cpu_to_be16" redefined
93 | #define cpu_to_be16 bswap_16
|
In file included from ./include/linux/byteorder/little_endian.h:11,
from /usr/include/x86_64-linux-gnu/asm/byteorder.h:5,
from ./arch/x86/include/asm/insn.h:10,
from arch/x86/tools/insn_sanity.c:21:
./include/linux/byteorder/generic.h:96: note: this is the location of the previous definition
96 | #define cpu_to_be16 __cpu_to_be16
|
In file included from ./arch/x86/lib/insn.c:8,
from arch/x86/tools/insn_sanity.c:23:
./tools/include/linux/kernel.h:94: warning: "cpu_to_be32" redefined
94 | #define cpu_to_be32 bswap_32
|
In file included from ./include/linux/byteorder/little_endian.h:11,
from /usr/include/x86_64-linux-gnu/asm/byteorder.h:5,
from ./arch/x86/include/asm/insn.h:10,
from arch/x86/tools/insn_sanity.c:21:
./include/linux/byteorder/generic.h:94: note: this is the location of the previous definition
94 | #define cpu_to_be32 __cpu_to_be32
|
In file included from ./arch/x86/lib/insn.c:8,
from arch/x86/tools/insn_sanity.c:23:
./tools/include/linux/kernel.h:95: warning: "cpu_to_be64" redefined
95 | #define cpu_to_be64 bswap_64
|
In file included from ./include/linux/byteorder/little_endian.h:11,
from /usr/include/x86_64-linux-gnu/asm/byteorder.h:5,
from ./arch/x86/include/asm/insn.h:10,
from arch/x86/tools/insn_sanity.c:21:
./include/linux/byteorder/generic.h:92: note: this is the location of the previous definition
92 | #define cpu_to_be64 __cpu_to_be64
|
In file included from ./arch/x86/lib/insn.c:8,
from arch/x86/tools/insn_sanity.c:23:
./tools/include/linux/kernel.h:96: warning: "be16_to_cpu" redefined
96 | #define be16_to_cpu bswap_16
|
In file included from ./include/linux/byteorder/little_endian.h:11,
from /usr/include/x86_64-linux-gnu/asm/byteorder.h:5,
from ./arch/x86/include/asm/insn.h:10,
from arch/x86/tools/insn_sanity.c:21:
./include/linux/byteorder/generic.h:97: note: this is the location of the previous definition
97 | #define be16_to_cpu __be16_to_cpu
|
In file included from ./arch/x86/lib/insn.c:8,
from arch/x86/tools/insn_sanity.c:23:
./tools/include/linux/kernel.h:97: warning: "be32_to_cpu" redefined
97 | #define be32_to_cpu bswap_32
|
In file included from ./include/linux/byteorder/little_endian.h:11,
from /usr/include/x86_64-linux-gnu/asm/byteorder.h:5,
from ./arch/x86/include/asm/insn.h:10,
from arch/x86/tools/insn_sanity.c:21:
./include/linux/byteorder/generic.h:95: note: this is the location of the previous definition
95 | #define be32_to_cpu __be32_to_cpu
|
In file included from ./arch/x86/lib/insn.c:8,
from arch/x86/tools/insn_sanity.c:23:
./tools/include/linux/kernel.h:98: warning: "be64_to_cpu" redefined
98 | #define be64_to_cpu bswap_64
|
In file included from ./include/linux/byteorder/little_endian.h:11,
from /usr/include/x86_64-linux-gnu/asm/byteorder.h:5,
from ./arch/x86/include/asm/insn.h:10,
from arch/x86/tools/insn_sanity.c:21:
./include/linux/byteorder/generic.h:93: note: this is the location of the previous definition
93 | #define be64_to_cpu __be64_to_cpu
|
In file included from ./arch/x86/lib/insn.c:8,
from arch/x86/tools/insn_sanity.c:23:
./tools/include/linux/kernel.h:105: warning: "ARRAY_SIZE" redefined
105 | #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
|
arch/x86/tools/insn_sanity.c:19: note: this is the location of the previous definition
19 | #define ARRAY_SIZE(a) (sizeof(a)/sizeof(a[0]))
|
make[1]: *** [scripts/Makefile.host:95: arch/x86/tools/insn_sanity] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [arch/x86/Makefile:267: bzImage] Error 2
make: *** Waiting for unfinished jobs....

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2020-10-10 23:13:20

by Vasily Gorbik

Subject: Re: [tip: objtool/core] x86/insn: Support big endian cross-compiles

On Fri, Oct 09, 2020 at 10:49:21PM +0200, Borislav Petkov wrote:
> On Fri, Oct 09, 2020 at 10:38:22PM +0200, Peter Zijlstra wrote:
> > On Wed, Oct 07, 2020 at 04:20:19PM -0000, tip-bot2 for Martin Schwidefsky wrote:
> > > The following commit has been merged into the objtool/core branch of tip:
> > >
> > > Commit-ID: 2a522b53c47051d3bf98748418f4f8e5f20d2c04
> > > Gitweb: https://git.kernel.org/tip/2a522b53c47051d3bf98748418f4f8e5f20d2c04
> > >
> > > x86/insn: Support big endian cross-compiles
> >
> > This commit breaks the x86 build with CONFIG_X86_DECODER_SELFTEST=y.
> >
> > I've asked Boris to truncate tip/objtool/core.
>
> Yeah, top 4 are gone until this is resolved.
>
> What I would suggest is to have a look at how tools/ headers are kept
> separate from kernel proper ones, see tools/include/ and how those
> headers there are full of dummy definitions just so it builds.
>
> And then including a global one like linux/kernel.h is just looking for
> trouble:
>
> In file included from ./include/uapi/linux/byteorder/little_endian.h:12,
> from ./include/linux/byteorder/little_endian.h:5,
> from /usr/include/x86_64-linux-gnu/asm/byteorder.h:5,
> from ./arch/x86/include/asm/insn.h:10,
> from arch/x86/tools/insn_sanity.c:21:
> ./tools/include/linux/types.h:30:18: error: conflicting types for ‘u64’
> 30 | typedef uint64_t u64;

Sigh... I have not realized there are more usages of insn.c which are
conditionally compiled. It's not like you grep *.c files to find who
includes them regularity.

Looks like there is no way to find common byte swapping helpers for
the kernel and tools then. Even though tools provide quite a bunch of
them in tools/include/. So, completely avoiding mixing "kernel" and
"userspace" headers would look like the following (delta to commit
mentioned above):
---

diff --git a/arch/x86/include/asm/insn.h b/arch/x86/include/asm/insn.h
index 004e27bdf121..68197fe18a11 100644
--- a/arch/x86/include/asm/insn.h
+++ b/arch/x86/include/asm/insn.h
@@ -7,7 +7,13 @@
* Copyright (C) IBM Corporation, 2009
*/

+#ifdef __KERNEL__
#include <asm/byteorder.h>
+#define insn_cpu_to_le32 cpu_to_le32
+#else
+#include <endian.h>
+#define insn_cpu_to_le32 htole32
+#endif
/* insn_attr_t is defined in inat.h */
#include <asm/inat.h>

@@ -47,7 +53,7 @@ static inline void insn_field_set(struct insn_field *p, insn_value_t v,
unsigned char n)
{
p->value = v;
- p->little = __cpu_to_le32(v);
+ p->little = insn_cpu_to_le32(v);
p->nbytes = n;
}

diff --git a/arch/x86/lib/insn.c b/arch/x86/lib/insn.c
index 520b31fc1f1a..003f32ff7798 100644
--- a/arch/x86/lib/insn.c
+++ b/arch/x86/lib/insn.c
@@ -5,7 +5,6 @@
* Copyright (C) IBM Corporation, 2002, 2004, 2009
*/

-#include <linux/kernel.h>
#ifdef __KERNEL__
#include <linux/string.h>
#else
@@ -16,15 +15,23 @@

#include <asm/emulate_prefix.h>

+#ifdef __KERNEL__
+#define insn_le32_to_cpu le32_to_cpu
+#define insn_le16_to_cpu le16_to_cpu
+#else
+#define insn_le32_to_cpu le32toh
+#define insn_le16_to_cpu le16toh
+#endif
+
#define leXX_to_cpu(t, r) \
({ \
__typeof__(t) v; \
switch (sizeof(t)) { \
- case 4: v = le32_to_cpu(r); break; \
- case 2: v = le16_to_cpu(r); break; \
+ case 4: v = insn_le32_to_cpu(r); break; \
+ case 2: v = insn_le16_to_cpu(r); break; \
case 1: v = r; break; \
- default: \
- BUILD_BUG(); break; \
+ default: /* relying on -Wuninitialized to report this */ \
+ break; \
} \
v; \
})
--
And the same for the tools/*
No linux/kernel.h means no BUILD_BUG(), but -Wuninitialized actually
does a decent job in this case:
arch/x86/../../../arch/x86/lib/insn.c:605:37: error: variable 'v' is
uninitialized when used here [-Werror,-Wuninitialized]
insn_field_set(&insn->immediate2, get_next(long, insn), 1);
^~~~~~~~~~~~~~~~~~~~

Masami, Josh,
would that be acceptable?

Should I resent the entire patch series again with these changes squashed?
Or just as a separate commit which would go on top?

2020-10-11 11:09:55

by Josh Poimboeuf

Subject: Re: [tip: objtool/core] x86/insn: Support big endian cross-compiles

On Fri, Oct 09, 2020 at 10:49:21PM +0200, Borislav Petkov wrote:
> On Fri, Oct 09, 2020 at 10:38:22PM +0200, Peter Zijlstra wrote:
> > On Wed, Oct 07, 2020 at 04:20:19PM -0000, tip-bot2 for Martin Schwidefsky wrote:
> > > The following commit has been merged into the objtool/core branch of tip:
> > >
> > > Commit-ID: 2a522b53c47051d3bf98748418f4f8e5f20d2c04
> > > Gitweb: https://git.kernel.org/tip/2a522b53c47051d3bf98748418f4f8e5f20d2c04
> > > Author: Martin Schwidefsky <[email protected]>
> > > AuthorDate: Mon, 05 Oct 2020 17:50:31 +02:00
> > > Committer: Josh Poimboeuf <[email protected]>
> > > CommitterDate: Tue, 06 Oct 2020 09:32:29 -05:00
> > >
> > > x86/insn: Support big endian cross-compiles
> > >
> > > x86 instruction decoder code is shared across the kernel source and the
> > > tools. Currently objtool seems to be the only tool from build tools needed
> > > which breaks x86 cross compilation on big endian systems. Make the x86
> > > instruction decoder build host endianness agnostic to support x86 cross
> > > compilation and enable objtool to implement endianness awareness for
> > > big endian architectures support.
> > >
> > > Signed-off-by: Martin Schwidefsky <[email protected]>
> > > Co-developed-by: Vasily Gorbik <[email protected]>
> > > Signed-off-by: Vasily Gorbik <[email protected]>
> > > Acked-by: Masami Hiramatsu <[email protected]>
> > > Signed-off-by: Josh Poimboeuf <[email protected]>
> >
> > This commit breaks the x86 build with CONFIG_X86_DECODER_SELFTEST=y.
> >
> > I've asked Boris to truncate tip/objtool/core.
>
> Yeah, top 4 are gone until this is resolved.

Masami, I wonder if we even need these selftests anymore? Objtool
already decodes the entire kernel.

--
Josh

2020-10-11 18:08:25

by Borislav Petkov

Subject: Re: [tip: objtool/core] x86/insn: Support big endian cross-compiles

On Sat, Oct 10, 2020 at 04:02:10PM +0200, Vasily Gorbik wrote:
> Should I resent the entire patch series again with these changes
> squashed? Or just as a separate commit which would go on top?

You can wait at least a week (merge window starts tomorrow) and then
resend the entire series once the final approach is agreed upon.

Thx.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2020-10-12 02:22:29

by Masami Hiramatsu

Subject: Re: [tip: objtool/core] x86/insn: Support big endian cross-compiles

On Sat, 10 Oct 2020 16:02:10 +0200
Vasily Gorbik <[email protected]> wrote:

> On Fri, Oct 09, 2020 at 10:49:21PM +0200, Borislav Petkov wrote:
> > On Fri, Oct 09, 2020 at 10:38:22PM +0200, Peter Zijlstra wrote:
> > > On Wed, Oct 07, 2020 at 04:20:19PM -0000, tip-bot2 for Martin Schwidefsky wrote:
> > > > The following commit has been merged into the objtool/core branch of tip:
> > > >
> > > > Commit-ID: 2a522b53c47051d3bf98748418f4f8e5f20d2c04
> > > > Gitweb: https://git.kernel.org/tip/2a522b53c47051d3bf98748418f4f8e5f20d2c04
> > > >
> > > > x86/insn: Support big endian cross-compiles
> > >
> > > This commit breaks the x86 build with CONFIG_X86_DECODER_SELFTEST=y.
> > >
> > > I've asked Boris to truncate tip/objtool/core.
> >
> > Yeah, top 4 are gone until this is resolved.
> >
> > What I would suggest is to have a look at how tools/ headers are kept
> > separate from kernel proper ones, see tools/include/ and how those
> > headers there are full of dummy definitions just so it builds.
> >
> > And then including a global one like linux/kernel.h is just looking for
> > trouble:
> >
> > In file included from ./include/uapi/linux/byteorder/little_endian.h:12,
> > from ./include/linux/byteorder/little_endian.h:5,
> > from /usr/include/x86_64-linux-gnu/asm/byteorder.h:5,
> > from ./arch/x86/include/asm/insn.h:10,
> > from arch/x86/tools/insn_sanity.c:21:
> > ./tools/include/linux/types.h:30:18: error: conflicting types for ‘u64’
> > 30 | typedef uint64_t u64;
>
> Sigh... I have not realized there are more usages of insn.c which are
> conditionally compiled. It's not like you grep *.c files to find who
> includes them regularity.

Yes, x86 insn library code is used for the sanity check tool too.

>
> Looks like there is no way to find common byte swapping helpers for
> the kernel and tools then. Even though tools provide quite a bunch of
> them in tools/include/. So, completely avoiding mixing "kernel" and
> "userspace" headers would look like the following (delta to commit
> mentioned above):
> ---
>
> diff --git a/arch/x86/include/asm/insn.h b/arch/x86/include/asm/insn.h
> index 004e27bdf121..68197fe18a11 100644
> --- a/arch/x86/include/asm/insn.h
> +++ b/arch/x86/include/asm/insn.h
> @@ -7,7 +7,13 @@
> * Copyright (C) IBM Corporation, 2009
> */
>
> +#ifdef __KERNEL__
> #include <asm/byteorder.h>
> +#define insn_cpu_to_le32 cpu_to_le32
> +#else
> +#include <endian.h>
> +#define insn_cpu_to_le32 htole32
> +#endif
> /* insn_attr_t is defined in inat.h */
> #include <asm/inat.h>
>
> @@ -47,7 +53,7 @@ static inline void insn_field_set(struct insn_field *p, insn_value_t v,
> unsigned char n)
> {
> p->value = v;
> - p->little = __cpu_to_le32(v);
> + p->little = insn_cpu_to_le32(v);
> p->nbytes = n;
> }
>
> diff --git a/arch/x86/lib/insn.c b/arch/x86/lib/insn.c
> index 520b31fc1f1a..003f32ff7798 100644
> --- a/arch/x86/lib/insn.c
> +++ b/arch/x86/lib/insn.c
> @@ -5,7 +5,6 @@
> * Copyright (C) IBM Corporation, 2002, 2004, 2009
> */
>
> -#include <linux/kernel.h>
> #ifdef __KERNEL__
> #include <linux/string.h>
> #else
> @@ -16,15 +15,23 @@
>
> #include <asm/emulate_prefix.h>
>
> +#ifdef __KERNEL__
> +#define insn_le32_to_cpu le32_to_cpu
> +#define insn_le16_to_cpu le16_to_cpu
> +#else
> +#define insn_le32_to_cpu le32toh
> +#define insn_le16_to_cpu le16toh
> +#endif
> +
> #define leXX_to_cpu(t, r) \
> ({ \
> __typeof__(t) v; \
> switch (sizeof(t)) { \
> - case 4: v = le32_to_cpu(r); break; \
> - case 2: v = le16_to_cpu(r); break; \
> + case 4: v = insn_le32_to_cpu(r); break; \
> + case 2: v = insn_le16_to_cpu(r); break; \
> case 1: v = r; break; \
> - default: \
> - BUILD_BUG(); break; \
> + default: /* relying on -Wuninitialized to report this */ \
> + break; \
> } \
> v; \
> })
> --
> And the same for the tools/*
> No linux/kernel.h means no BUILD_BUG(), but -Wuninitialized actually
> does a decent job in this case:
> arch/x86/../../../arch/x86/lib/insn.c:605:37: error: variable 'v' is
> uninitialized when used here [-Werror,-Wuninitialized]
> insn_field_set(&insn->immediate2, get_next(long, insn), 1);
> ^~~~~~~~~~~~~~~~~~~~

Can you initialize v with 0 ? Anyway it will be optimized out while
compiling the code.

>
> Masami, Josh,
> would that be acceptable?

Yes.

Thank you,


--
Masami Hiramatsu <[email protected]>

2020-10-12 02:23:33

by Masami Hiramatsu

Subject: Re: [tip: objtool/core] x86/insn: Support big endian cross-compiles

On Sat, 10 Oct 2020 12:44:15 -0500
Josh Poimboeuf <[email protected]> wrote:

> On Fri, Oct 09, 2020 at 10:49:21PM +0200, Borislav Petkov wrote:
> > On Fri, Oct 09, 2020 at 10:38:22PM +0200, Peter Zijlstra wrote:
> > > On Wed, Oct 07, 2020 at 04:20:19PM -0000, tip-bot2 for Martin Schwidefsky wrote:
> > > > The following commit has been merged into the objtool/core branch of tip:
> > > >
> > > > Commit-ID: 2a522b53c47051d3bf98748418f4f8e5f20d2c04
> > > > Gitweb: https://git.kernel.org/tip/2a522b53c47051d3bf98748418f4f8e5f20d2c04
> > > > Author: Martin Schwidefsky <[email protected]>
> > > > AuthorDate: Mon, 05 Oct 2020 17:50:31 +02:00
> > > > Committer: Josh Poimboeuf <[email protected]>
> > > > CommitterDate: Tue, 06 Oct 2020 09:32:29 -05:00
> > > >
> > > > x86/insn: Support big endian cross-compiles
> > > >
> > > > x86 instruction decoder code is shared across the kernel source and the
> > > > tools. Currently objtool seems to be the only tool from build tools needed
> > > > which breaks x86 cross compilation on big endian systems. Make the x86
> > > > instruction decoder build host endianness agnostic to support x86 cross
> > > > compilation and enable objtool to implement endianness awareness for
> > > > big endian architectures support.
> > > >
> > > > Signed-off-by: Martin Schwidefsky <[email protected]>
> > > > Co-developed-by: Vasily Gorbik <[email protected]>
> > > > Signed-off-by: Vasily Gorbik <[email protected]>
> > > > Acked-by: Masami Hiramatsu <[email protected]>
> > > > Signed-off-by: Josh Poimboeuf <[email protected]>
> > >
> > > This commit breaks the x86 build with CONFIG_X86_DECODER_SELFTEST=y.
> > >
> > > I've asked Boris to truncate tip/objtool/core.
> >
> > Yeah, top 4 are gone until this is resolved.
>
> Masami, I wonder if we even need these selftests anymore? Objtool
> already decodes the entire kernel.

No, they have different roles. The selftest checks if the decoder
works correctly by comparing with the output of objdump.

As far as I can see, the objtool relies on the sanity of the decoder
(it trusts the output of the decoder).

Thank you,

--
Masami Hiramatsu <[email protected]>

2020-10-12 15:45:44

by Josh Poimboeuf

Subject: Re: [tip: objtool/core] x86/insn: Support big endian cross-compiles

On Mon, Oct 12, 2020 at 09:12:36AM +0900, Masami Hiramatsu wrote:
> On Sat, 10 Oct 2020 12:44:15 -0500
> Josh Poimboeuf <[email protected]> wrote:
>
> > On Fri, Oct 09, 2020 at 10:49:21PM +0200, Borislav Petkov wrote:
> > > On Fri, Oct 09, 2020 at 10:38:22PM +0200, Peter Zijlstra wrote:
> > > > On Wed, Oct 07, 2020 at 04:20:19PM -0000, tip-bot2 for Martin Schwidefsky wrote:
> > > > > The following commit has been merged into the objtool/core branch of tip:
> > > > >
> > > > > Commit-ID: 2a522b53c47051d3bf98748418f4f8e5f20d2c04
> > > > > Gitweb: https://git.kernel.org/tip/2a522b53c47051d3bf98748418f4f8e5f20d2c04
> > > > > Author: Martin Schwidefsky <[email protected]>
> > > > > AuthorDate: Mon, 05 Oct 2020 17:50:31 +02:00
> > > > > Committer: Josh Poimboeuf <[email protected]>
> > > > > CommitterDate: Tue, 06 Oct 2020 09:32:29 -05:00
> > > > >
> > > > > x86/insn: Support big endian cross-compiles
> > > > >
> > > > > x86 instruction decoder code is shared across the kernel source and the
> > > > > tools. Currently objtool seems to be the only tool from build tools needed
> > > > > which breaks x86 cross compilation on big endian systems. Make the x86
> > > > > instruction decoder build host endianness agnostic to support x86 cross
> > > > > compilation and enable objtool to implement endianness awareness for
> > > > > big endian architectures support.
> > > > >
> > > > > Signed-off-by: Martin Schwidefsky <[email protected]>
> > > > > Co-developed-by: Vasily Gorbik <[email protected]>
> > > > > Signed-off-by: Vasily Gorbik <[email protected]>
> > > > > Acked-by: Masami Hiramatsu <[email protected]>
> > > > > Signed-off-by: Josh Poimboeuf <[email protected]>
> > > >
> > > > This commit breaks the x86 build with CONFIG_X86_DECODER_SELFTEST=y.
> > > >
> > > > I've asked Boris to truncate tip/objtool/core.
> > >
> > > Yeah, top 4 are gone until this is resolved.
> >
> > Masami, I wonder if we even need these selftests anymore? Objtool
> > already decodes the entire kernel.
>
> No, they have different roles. The selftest checks if the decoder
> works correctly by comparing with the output of objdump.
>
> As far as I can see, the objtool relies on the sanity of the decoder
> (it trusts the output of the decoder).

Ok. I wonder if we should move the decoder selftest to the 'tools'
subdirectory.

--
Josh

2020-10-14 13:25:42

by Masami Hiramatsu

Subject: Re: [tip: objtool/core] x86/insn: Support big endian cross-compiles

On Mon, 12 Oct 2020 10:39:49 -0500
Josh Poimboeuf <[email protected]> wrote:

> On Mon, Oct 12, 2020 at 09:12:36AM +0900, Masami Hiramatsu wrote:
> > On Sat, 10 Oct 2020 12:44:15 -0500
> > Josh Poimboeuf <[email protected]> wrote:
> >
> > > On Fri, Oct 09, 2020 at 10:49:21PM +0200, Borislav Petkov wrote:
> > > > On Fri, Oct 09, 2020 at 10:38:22PM +0200, Peter Zijlstra wrote:
> > > > > On Wed, Oct 07, 2020 at 04:20:19PM -0000, tip-bot2 for Martin Schwidefsky wrote:
> > > > > > The following commit has been merged into the objtool/core branch of tip:
> > > > > >
> > > > > > Commit-ID: 2a522b53c47051d3bf98748418f4f8e5f20d2c04
> > > > > > Gitweb: https://git.kernel.org/tip/2a522b53c47051d3bf98748418f4f8e5f20d2c04
> > > > > > Author: Martin Schwidefsky <[email protected]>
> > > > > > AuthorDate: Mon, 05 Oct 2020 17:50:31 +02:00
> > > > > > Committer: Josh Poimboeuf <[email protected]>
> > > > > > CommitterDate: Tue, 06 Oct 2020 09:32:29 -05:00
> > > > > >
> > > > > > x86/insn: Support big endian cross-compiles
> > > > > >
> > > > > > x86 instruction decoder code is shared across the kernel source and the
> > > > > > tools. Currently objtool seems to be the only tool from build tools needed
> > > > > > which breaks x86 cross compilation on big endian systems. Make the x86
> > > > > > instruction decoder build host endianness agnostic to support x86 cross
> > > > > > compilation and enable objtool to implement endianness awareness for
> > > > > > big endian architectures support.
> > > > > >
> > > > > > Signed-off-by: Martin Schwidefsky <[email protected]>
> > > > > > Co-developed-by: Vasily Gorbik <[email protected]>
> > > > > > Signed-off-by: Vasily Gorbik <[email protected]>
> > > > > > Acked-by: Masami Hiramatsu <[email protected]>
> > > > > > Signed-off-by: Josh Poimboeuf <[email protected]>
> > > > >
> > > > > This commit breaks the x86 build with CONFIG_X86_DECODER_SELFTEST=y.
> > > > >
> > > > > I've asked Boris to truncate tip/objtool/core.
> > > >
> > > > Yeah, top 4 are gone until this is resolved.
> > >
> > > Masami, I wonder if we even need these selftests anymore? Objtool
> > > already decodes the entire kernel.
> >
> > No, they have different roles. The selftest checks if the decoder
> > works correctly by comparing with the output of objdump.
> >
> > As far as I can see, the objtool relies on the sanity of the decoder
> > (it trusts the output of the decoder).
>
> Ok. I wonder if we should move the decoder selftest to the 'tools'
> subdirectory.

It is in the arch/x86/tools, so it is already in a kind of tools :)
But yeah, it was considered to be used only on x86. But if someone
start trying to run it on non-x86, cross compiling, we need to
reconsider that.

Thank you,

--
Masami Hiramatsu <[email protected]>

2020-10-15 17:43:28

by Ian Rogers

Subject: Re: [tip: objtool/core] x86/insn: Support big endian cross-compiles

On Wed, Oct 14, 2020 at 12:29 AM Masami Hiramatsu <[email protected]> wrote:
>
> On Mon, 12 Oct 2020 10:39:49 -0500
> Josh Poimboeuf <[email protected]> wrote:
>
> > On Mon, Oct 12, 2020 at 09:12:36AM +0900, Masami Hiramatsu wrote:
> > > On Sat, 10 Oct 2020 12:44:15 -0500
> > > Josh Poimboeuf <[email protected]> wrote:
> > >
> > > > On Fri, Oct 09, 2020 at 10:49:21PM +0200, Borislav Petkov wrote:
> > > > > On Fri, Oct 09, 2020 at 10:38:22PM +0200, Peter Zijlstra wrote:
> > > > > > On Wed, Oct 07, 2020 at 04:20:19PM -0000, tip-bot2 for Martin Schwidefsky wrote:
> > > > > > > The following commit has been merged into the objtool/core branch of tip:
> > > > > > >
> > > > > > > Commit-ID: 2a522b53c47051d3bf98748418f4f8e5f20d2c04
> > > > > > > Gitweb: https://git.kernel.org/tip/2a522b53c47051d3bf98748418f4f8e5f20d2c04
> > > > > > > Author: Martin Schwidefsky <[email protected]>
> > > > > > > AuthorDate: Mon, 05 Oct 2020 17:50:31 +02:00
> > > > > > > Committer: Josh Poimboeuf <[email protected]>
> > > > > > > CommitterDate: Tue, 06 Oct 2020 09:32:29 -05:00
> > > > > > >
> > > > > > > x86/insn: Support big endian cross-compiles
> > > > > > >
> > > > > > > x86 instruction decoder code is shared across the kernel source and the
> > > > > > > tools. Currently objtool seems to be the only tool from build tools needed
> > > > > > > which breaks x86 cross compilation on big endian systems. Make the x86
> > > > > > > instruction decoder build host endianness agnostic to support x86 cross
> > > > > > > compilation and enable objtool to implement endianness awareness for
> > > > > > > big endian architectures support.
> > > > > > >
> > > > > > > Signed-off-by: Martin Schwidefsky <[email protected]>
> > > > > > > Co-developed-by: Vasily Gorbik <[email protected]>
> > > > > > > Signed-off-by: Vasily Gorbik <[email protected]>
> > > > > > > Acked-by: Masami Hiramatsu <[email protected]>
> > > > > > > Signed-off-by: Josh Poimboeuf <[email protected]>
> > > > > >
> > > > > > This commit breaks the x86 build with CONFIG_X86_DECODER_SELFTEST=y.
> > > > > >
> > > > > > I've asked Boris to truncate tip/objtool/core.
> > > > >
> > > > > Yeah, top 4 are gone until this is resolved.
> > > >
> > > > Masami, I wonder if we even need these selftests anymore? Objtool
> > > > already decodes the entire kernel.
> > >
> > > No, they have different roles. The selftest checks if the decoder
> > > works correctly by comparing with the output of objdump.
> > >
> > > As far as I can see, the objtool relies on the sanity of the decoder
> > > (it trusts the output of the decoder).
> >
> > Ok. I wonder if we should move the decoder selftest to the 'tools'
> > subdirectory.
>
> It is in the arch/x86/tools, so it is already in a kind of tools :)
> But yeah, it was considered to be used only on x86. But if someone
> start trying to run it on non-x86, cross compiling, we need to
> reconsider that.
>
> Thank you,
>
> --
> Masami Hiramatsu <[email protected]>

There is undefined behavior that is present in the x86 insn.c code as
described in:
https://lore.kernel.org/lkml/[email protected]/

I resent this patch fixing other potential undefined behavior:
https://lore.kernel.org/lkml/[email protected]/T/#t

The misaligned loads will likely break on anything non-x86.

Thanks,
Ian

2020-11-03 19:37:16

by Vasily Gorbik

Subject: Re: [tip: objtool/core] x86/insn: Support big endian cross-compiles

On Wed, Oct 14, 2020 at 04:28:59PM +0900, Masami Hiramatsu wrote:
> On Mon, 12 Oct 2020 10:39:49 -0500
> Josh Poimboeuf <[email protected]> wrote:
>
> > On Mon, Oct 12, 2020 at 09:12:36AM +0900, Masami Hiramatsu wrote:
> > > On Sat, 10 Oct 2020 12:44:15 -0500
> > > Josh Poimboeuf <[email protected]> wrote:
> > >
> > > > On Fri, Oct 09, 2020 at 10:49:21PM +0200, Borislav Petkov wrote:
> > > > > On Fri, Oct 09, 2020 at 10:38:22PM +0200, Peter Zijlstra wrote:
> > > > > > On Wed, Oct 07, 2020 at 04:20:19PM -0000, tip-bot2 for Martin Schwidefsky wrote:
> > > > > > > The following commit has been merged into the objtool/core branch of tip:
> > > > > > >
> > > > > > > Commit-ID: 2a522b53c47051d3bf98748418f4f8e5f20d2c04
> > > > > > > Gitweb: https://git.kernel.org/tip/2a522b53c47051d3bf98748418f4f8e5f20d2c04
> > > > > > > Author: Martin Schwidefsky <[email protected]>
> > > > > > > AuthorDate: Mon, 05 Oct 2020 17:50:31 +02:00
> > > > > > > Committer: Josh Poimboeuf <[email protected]>
> > > > > > > CommitterDate: Tue, 06 Oct 2020 09:32:29 -05:00
> > > > > > >
> > > > > > > x86/insn: Support big endian cross-compiles
> > > > > > >
> > > > > > > x86 instruction decoder code is shared across the kernel source and the
> > > > > > > tools. Currently objtool seems to be the only tool from build tools needed
> > > > > > > which breaks x86 cross compilation on big endian systems. Make the x86
> > > > > > > instruction decoder build host endianness agnostic to support x86 cross
> > > > > > > compilation and enable objtool to implement endianness awareness for
> > > > > > > big endian architectures support.
> > > > > > >
> > > > > > > Signed-off-by: Martin Schwidefsky <[email protected]>
> > > > > > > Co-developed-by: Vasily Gorbik <[email protected]>
> > > > > > > Signed-off-by: Vasily Gorbik <[email protected]>
> > > > > > > Acked-by: Masami Hiramatsu <[email protected]>
> > > > > > > Signed-off-by: Josh Poimboeuf <[email protected]>
> > > > > >
> > > > > > This commit breaks the x86 build with CONFIG_X86_DECODER_SELFTEST=y.
> > > > > >
> > > > > > I've asked Boris to truncate tip/objtool/core.
> > > > >
> > > > > Yeah, top 4 are gone until this is resolved.
> > > >
> > > > Masami, I wonder if we even need these selftests anymore? Objtool
> > > > already decodes the entire kernel.
> > >
> > > No, they have different roles. The selftest checks if the decoder
> > > works correctly by comparing with the output of objdump.
> > >
> > > As far as I can see, the objtool relies on the sanity of the decoder
> > > (it trusts the output of the decoder).
> >
> > Ok. I wonder if we should move the decoder selftest to the 'tools'
> > subdirectory.
>
> It is in the arch/x86/tools, so it is already in a kind of tools :)
> But yeah, it was considered to be used only on x86. But if someone
> start trying to run it on non-x86, cross compiling, we need to
> reconsider that.

I actually tried to move it to tools/testing/selftests and encountered
several problems with kselftest build in general:
- out of source build is broken if path is relative,
- out of source build headers partially installed in
$(srcdir)arch/x86/include/generated/ instead of $(objdir), when
kselftests are called from the kbuild,
- out of source test runs is broken,
- kernel headers are installed unconditionally.

These things impede moving decoder selftests to kselftests.

On the other hand making the decoder selftest work "in place" seems
trivial. The following fix on top of jpoimboe/objtool/core fixes the
build, as well as cross-compilation. With that I can cross-compile
x86 kernel on s390 with CONFIG_X86_DECODER_SELFTEST=y and posttest runs
just fine.

Vasily Gorbik (1):
x86/tools: Use tools headers for instruction decoder selftests

arch/x86/tools/Makefile | 8 ++++----
arch/x86/tools/insn_sanity.c | 4 ----
2 files changed, 4 insertions(+), 8 deletions(-)

--
2.25.4

2020-11-03 19:39:17

by Vasily Gorbik

Subject: [PATCH 1/1] x86/tools: Use tools headers for instruction decoder selftests

Currently x86 instruction decoder is used from:
- the kernel itself
- from tools like objtool and perf
- within x86 tools, i.e. instruction decoder selftests

The first two cases are similar, because tools headers try to mimic
kernel headers.

Instruction decoder selftests include some of the kernel headers
directly, including uapi headers. This works until headers dependencies
are kept to minimum and tools are not cross-compiled. Since the goal of
the x86 instruction decoder selftests is not to verify uapi headers move
it to using tools headers, like this is already done for vdso2c tool,
mkpiggy and other tools in arch/x86/boot/.

This effectively fixes x86 kernel cross-compilation with
CONFIG_X86_DECODER_SELFTEST=y. And posttests are run successfully at
least on s390.

Fixes: 2a522b53c470 ("x86/insn: Support big endian cross-compiles")
Signed-off-by: Vasily Gorbik <[email protected]>
---
Based on jpoimboe/objtool/core

arch/x86/tools/Makefile | 8 ++++----
arch/x86/tools/insn_sanity.c | 4 ----
2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/arch/x86/tools/Makefile b/arch/x86/tools/Makefile
index 55b1ab378974..bddfc9a46645 100644
--- a/arch/x86/tools/Makefile
+++ b/arch/x86/tools/Makefile
@@ -29,14 +29,14 @@ posttest: $(obj)/insn_decoder_test vmlinux $(obj)/insn_sanity
hostprogs += insn_decoder_test insn_sanity

# -I needed for generated C source and C source which in the kernel tree.
-HOSTCFLAGS_insn_decoder_test.o := -Wall -I$(objtree)/arch/x86/lib/ -I$(srctree)/arch/x86/include/uapi/ -I$(srctree)/arch/x86/include/ -I$(srctree)/arch/x86/lib/ -I$(srctree)/include/uapi/
+HOSTCFLAGS_insn_decoder_test.o := -Wall -I$(srctree)/tools/arch/x86/lib/ -I$(srctree)/tools/arch/x86/include/ -I$(objtree)/arch/x86/lib/

-HOSTCFLAGS_insn_sanity.o := -Wall -I$(objtree)/arch/x86/lib/ -I$(srctree)/arch/x86/include/ -I$(srctree)/arch/x86/lib/ -I$(srctree)/include/
+HOSTCFLAGS_insn_sanity.o := -Wall -I$(srctree)/tools/arch/x86/lib/ -I$(srctree)/tools/arch/x86/include/ -I$(objtree)/arch/x86/lib/

# Dependencies are also needed.
-$(obj)/insn_decoder_test.o: $(srctree)/arch/x86/lib/insn.c $(srctree)/arch/x86/lib/inat.c $(srctree)/arch/x86/include/asm/inat_types.h $(srctree)/arch/x86/include/asm/inat.h $(srctree)/arch/x86/include/asm/insn.h $(objtree)/arch/x86/lib/inat-tables.c
+$(obj)/insn_decoder_test.o: $(srctree)/tools/arch/x86/lib/insn.c $(srctree)/tools/arch/x86/lib/inat.c $(srctree)/tools/arch/x86/include/asm/inat_types.h $(srctree)/tools/arch/x86/include/asm/inat.h $(srctree)/tools/arch/x86/include/asm/insn.h $(objtree)/arch/x86/lib/inat-tables.c

-$(obj)/insn_sanity.o: $(srctree)/arch/x86/lib/insn.c $(srctree)/arch/x86/lib/inat.c $(srctree)/arch/x86/include/asm/inat_types.h $(srctree)/arch/x86/include/asm/inat.h $(srctree)/arch/x86/include/asm/insn.h $(objtree)/arch/x86/lib/inat-tables.c
+$(obj)/insn_sanity.o: $(srctree)/tools/arch/x86/lib/insn.c $(srctree)/tools/arch/x86/lib/inat.c $(srctree)/tools/arch/x86/include/asm/inat_types.h $(srctree)/tools/arch/x86/include/asm/inat.h $(srctree)/tools/arch/x86/include/asm/insn.h $(objtree)/arch/x86/lib/inat-tables.c

HOST_EXTRACFLAGS += -I$(srctree)/tools/include
hostprogs += relocs
diff --git a/arch/x86/tools/insn_sanity.c b/arch/x86/tools/insn_sanity.c
index 185ceba9d289..c6a0000ae635 100644
--- a/arch/x86/tools/insn_sanity.c
+++ b/arch/x86/tools/insn_sanity.c
@@ -14,10 +14,6 @@
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
-
-#define unlikely(cond) (cond)
-#define ARRAY_SIZE(a) (sizeof(a)/sizeof(a[0]))
-
#include <asm/insn.h>
#include <inat.c>
#include <insn.c>
--
2.25.4

2020-11-04 09:14:02

by kernel test robot

Subject: Re: [PATCH 1/1] x86/tools: Use tools headers for instruction decoder selftests

Hi Vasily,

I love your patch! Yet something to improve:

[auto build test ERROR on tip/x86/core]
[also build test ERROR on v5.10-rc2 next-20201103]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/0day-ci/linux/commits/Vasily-Gorbik/x86-tools-Use-tools-headers-for-instruction-decoder-selftests/20201104-043600
base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 238c91115cd05c71447ea071624a4c9fe661f970
config: x86_64-randconfig-a005-20201104 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 1fcd5d5655e29f85e12b402e32974f207cfedf32)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# https://github.com/0day-ci/linux/commit/ab4952becdfae8a76a6f0e0fb4ec7d078e80d5d6
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Vasily-Gorbik/x86-tools-Use-tools-headers-for-instruction-decoder-selftests/20201104-043600
git checkout ab4952becdfae8a76a6f0e0fb4ec7d078e80d5d6
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All error/warnings (new ones prefixed by >>):

In file included from arch/x86/tools/insn_sanity.c:19:
>> tools/arch/x86/lib/insn.c:72:7: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
if (peek_nbyte_next(insn_byte_t, insn, i) != prefix[i])
^
tools/arch/x86/lib/insn.c:32:9: note: expanded from macro 'peek_nbyte_next'
({ if (unlikely(!validate_next(t, insn, n))) goto err_out; __peek_nbyte_next(t, insn, n); })
^
tools/arch/x86/lib/insn.c:115:6: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
b = peek_next(insn_byte_t, insn);
^
tools/arch/x86/lib/insn.c:34:28: note: expanded from macro 'peek_next'
#define peek_next(t, insn) peek_nbyte_next(t, insn, 0)
^
tools/arch/x86/lib/insn.c:32:9: note: expanded from macro 'peek_nbyte_next'
({ if (unlikely(!validate_next(t, insn, n))) goto err_out; __peek_nbyte_next(t, insn, n); })
^
tools/arch/x86/lib/insn.c:140:7: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
b = peek_next(insn_byte_t, insn);
^
tools/arch/x86/lib/insn.c:34:28: note: expanded from macro 'peek_next'
#define peek_next(t, insn) peek_nbyte_next(t, insn, 0)
^
tools/arch/x86/lib/insn.c:32:9: note: expanded from macro 'peek_nbyte_next'
({ if (unlikely(!validate_next(t, insn, n))) goto err_out; __peek_nbyte_next(t, insn, n); })
^
tools/arch/x86/lib/insn.c:145:7: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
if (unlikely(insn->prefixes.bytes[3])) {
^
tools/arch/x86/lib/insn.c:157:7: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
b = peek_next(insn_byte_t, insn);
^
tools/arch/x86/lib/insn.c:34:28: note: expanded from macro 'peek_next'
#define peek_next(t, insn) peek_nbyte_next(t, insn, 0)
^
tools/arch/x86/lib/insn.c:32:9: note: expanded from macro 'peek_nbyte_next'
({ if (unlikely(!validate_next(t, insn, n))) goto err_out; __peek_nbyte_next(t, insn, n); })
^
tools/arch/x86/lib/insn.c:171:6: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
b = peek_next(insn_byte_t, insn);
^
tools/arch/x86/lib/insn.c:34:28: note: expanded from macro 'peek_next'
#define peek_next(t, insn) peek_nbyte_next(t, insn, 0)
^
tools/arch/x86/lib/insn.c:32:9: note: expanded from macro 'peek_nbyte_next'
({ if (unlikely(!validate_next(t, insn, n))) goto err_out; __peek_nbyte_next(t, insn, n); })
^
tools/arch/x86/lib/insn.c:174:20: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
insn_byte_t b2 = peek_nbyte_next(insn_byte_t, insn, 1);
^
tools/arch/x86/lib/insn.c:32:9: note: expanded from macro 'peek_nbyte_next'
({ if (unlikely(!validate_next(t, insn, n))) goto err_out; __peek_nbyte_next(t, insn, n); })
^
tools/arch/x86/lib/insn.c:187:9: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
b2 = peek_nbyte_next(insn_byte_t, insn, 2);
^
tools/arch/x86/lib/insn.c:32:9: note: expanded from macro 'peek_nbyte_next'
({ if (unlikely(!validate_next(t, insn, n))) goto err_out; __peek_nbyte_next(t, insn, n); })
^
tools/arch/x86/lib/insn.c:189:9: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
b2 = peek_nbyte_next(insn_byte_t, insn, 3);
^
tools/arch/x86/lib/insn.c:32:9: note: expanded from macro 'peek_nbyte_next'
({ if (unlikely(!validate_next(t, insn, n))) goto err_out; __peek_nbyte_next(t, insn, n); })
^
tools/arch/x86/lib/insn.c:197:9: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
b2 = peek_nbyte_next(insn_byte_t, insn, 2);
^
tools/arch/x86/lib/insn.c:32:9: note: expanded from macro 'peek_nbyte_next'
({ if (unlikely(!validate_next(t, insn, n))) goto err_out; __peek_nbyte_next(t, insn, n); })
^
tools/arch/x86/lib/insn.c:245:7: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
op = get_next(insn_byte_t, insn);
^
tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
^
tools/arch/x86/lib/insn.c:265:8: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
op = get_next(insn_byte_t, insn);
^
tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
^
tools/arch/x86/lib/insn.c:297:9: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
mod = get_next(insn_byte_t, insn);
^
tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
^
tools/arch/x86/lib/insn.c:359:22: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
insn->sib.value = get_next(insn_byte_t, insn);
^
tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
^
tools/arch/x86/lib/insn.c:410:31: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
insn->displacement.value = get_next(signed char, insn);
^
tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
^
tools/arch/x86/lib/insn.c:415:7: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
get_next(short, insn);
--
^
tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
^
tools/arch/x86/lib/insn.c:448:26: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
insn->moffset2.value = get_next(int, insn);
^
tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
^
tools/arch/x86/lib/insn.c:467:27: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
insn->immediate.value = get_next(short, insn);
^
tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
^
tools/arch/x86/lib/insn.c:472:27: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
insn->immediate.value = get_next(int, insn);
^
tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
^
tools/arch/x86/lib/insn.c:490:28: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
insn->immediate1.value = get_next(short, insn);
^
tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
^
tools/arch/x86/lib/insn.c:494:28: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
insn->immediate1.value = get_next(int, insn);
^
tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
^
tools/arch/x86/lib/insn.c:498:28: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
insn->immediate1.value = get_next(int, insn);
^
tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
^
tools/arch/x86/lib/insn.c:500:28: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
insn->immediate2.value = get_next(int, insn);
^
tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
^
tools/arch/x86/lib/insn.c:518:28: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
insn->immediate1.value = get_next(short, insn);
^
tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
^
tools/arch/x86/lib/insn.c:522:28: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
insn->immediate1.value = get_next(int, insn);
^
tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
^
tools/arch/x86/lib/insn.c:531:27: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
insn->immediate2.value = get_next(unsigned short, insn);
^
tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
^
tools/arch/x86/lib/insn.c:568:27: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
insn->immediate.value = get_next(signed char, insn);
^
tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
^
tools/arch/x86/lib/insn.c:572:27: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
insn->immediate.value = get_next(short, insn);
^
tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
^
tools/arch/x86/lib/insn.c:576:27: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
insn->immediate.value = get_next(int, insn);
^
tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
^
tools/arch/x86/lib/insn.c:580:28: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
insn->immediate1.value = get_next(int, insn);
^
tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
^
tools/arch/x86/lib/insn.c:582:28: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
insn->immediate2.value = get_next(int, insn);
^
tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
^
tools/arch/x86/lib/insn.c:602:28: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
insn->immediate2.value = get_next(signed char, insn);
^
tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
^
>> arch/x86/tools/insn_sanity.c:128:19: warning: implicit declaration of function 'ARRAY_SIZE' [-Wimplicit-function-declaration]
tmp = fgets(buf, ARRAY_SIZE(buf), input_file);
^
37 warnings generated.
/usr/bin/ld: /tmp/insn_sanity-8655a9.o: in function `insn_get_prefixes':
>> insn_sanity.c:(.text+0x1bd): undefined reference to `unlikely'
>> /usr/bin/ld: insn_sanity.c:(.text+0x203): undefined reference to `unlikely'
/usr/bin/ld: insn_sanity.c:(.text+0x24d): undefined reference to `unlikely'
/usr/bin/ld: insn_sanity.c:(.text+0x30f): undefined reference to `unlikely'
/usr/bin/ld: insn_sanity.c:(.text+0x353): undefined reference to `unlikely'
/usr/bin/ld: /tmp/insn_sanity-8655a9.o:insn_sanity.c:(.text+0x38e): more undefined references to `unlikely' follow
/usr/bin/ld: /tmp/insn_sanity-8655a9.o: in function `main':
>> insn_sanity.c:(.text+0x13cf): undefined reference to `ARRAY_SIZE'
/usr/bin/ld: /tmp/insn_sanity-8655a9.o: in function `__insn_get_emulate_prefix':
insn_sanity.c:(.text+0x1cc1): undefined reference to `unlikely'
/usr/bin/ld: insn_sanity.c:(.text+0x1cef): undefined reference to `unlikely'
/usr/bin/ld: insn_sanity.c:(.text+0x1d1f): undefined reference to `unlikely'
/usr/bin/ld: insn_sanity.c:(.text+0x1d47): undefined reference to `unlikely'
/usr/bin/ld: insn_sanity.c:(.text+0x1d6f): undefined reference to `unlikely'
clang-12: error: linker command failed with exit code 1 (use -v to see invocation)

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]

Attachments:

2020-11-04 09:21:40

by Vasily Gorbik

Subject: Re: [PATCH 1/1] x86/tools: Use tools headers for instruction decoder selftests

On Wed, Nov 04, 2020 at 05:11:28PM +0800, kernel test robot wrote:
> Hi Vasily,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on tip/x86/core]
> [also build test ERROR on v5.10-rc2 next-20201103]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url: https://github.com/0day-ci/linux/commits/Vasily-Gorbik/x86-tools-Use-tools-headers-for-instruction-decoder-selftests/20201104-043600
> base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 238c91115cd05c71447ea071624a4c9fe661f970
> config: x86_64-randconfig-a005-20201104 (attached as .config)
> compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 1fcd5d5655e29f85e12b402e32974f207cfedf32)
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # install x86_64 cross compiling tool for clang build
> # apt-get install binutils-x86-64-linux-gnu
> # https://github.com/0day-ci/linux/commit/ab4952becdfae8a76a6f0e0fb4ec7d078e80d5d6
> git remote add linux-review https://github.com/0day-ci/linux
> git fetch --no-tags linux-review Vasily-Gorbik/x86-tools-Use-tools-headers-for-instruction-decoder-selftests/20201104-043600
> git checkout ab4952becdfae8a76a6f0e0fb4ec7d078e80d5d6
> # save the attached .config to linux build tree
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <[email protected]>
>
> All error/warnings (new ones prefixed by >>):
>
> In file included from arch/x86/tools/insn_sanity.c:19:
> >> tools/arch/x86/lib/insn.c:72:7: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> if (peek_nbyte_next(insn_byte_t, insn, i) != prefix[i])
> ^
> tools/arch/x86/lib/insn.c:32:9: note: expanded from macro 'peek_nbyte_next'
> ({ if (unlikely(!validate_next(t, insn, n))) goto err_out; __peek_nbyte_next(t, insn, n); })
> ^
> tools/arch/x86/lib/insn.c:115:6: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> b = peek_next(insn_byte_t, insn);
> ^
> tools/arch/x86/lib/insn.c:34:28: note: expanded from macro 'peek_next'
> #define peek_next(t, insn) peek_nbyte_next(t, insn, 0)
> ^
> tools/arch/x86/lib/insn.c:32:9: note: expanded from macro 'peek_nbyte_next'
> ({ if (unlikely(!validate_next(t, insn, n))) goto err_out; __peek_nbyte_next(t, insn, n); })
> ^
> tools/arch/x86/lib/insn.c:140:7: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> b = peek_next(insn_byte_t, insn);
> ^
> tools/arch/x86/lib/insn.c:34:28: note: expanded from macro 'peek_next'
> #define peek_next(t, insn) peek_nbyte_next(t, insn, 0)
> ^
> tools/arch/x86/lib/insn.c:32:9: note: expanded from macro 'peek_nbyte_next'
> ({ if (unlikely(!validate_next(t, insn, n))) goto err_out; __peek_nbyte_next(t, insn, n); })
> ^
> tools/arch/x86/lib/insn.c:145:7: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> if (unlikely(insn->prefixes.bytes[3])) {
> ^
> tools/arch/x86/lib/insn.c:157:7: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> b = peek_next(insn_byte_t, insn);
> ^
> tools/arch/x86/lib/insn.c:34:28: note: expanded from macro 'peek_next'
> #define peek_next(t, insn) peek_nbyte_next(t, insn, 0)
> ^
> tools/arch/x86/lib/insn.c:32:9: note: expanded from macro 'peek_nbyte_next'
> ({ if (unlikely(!validate_next(t, insn, n))) goto err_out; __peek_nbyte_next(t, insn, n); })
> ^
> tools/arch/x86/lib/insn.c:171:6: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> b = peek_next(insn_byte_t, insn);
> ^
> tools/arch/x86/lib/insn.c:34:28: note: expanded from macro 'peek_next'
> #define peek_next(t, insn) peek_nbyte_next(t, insn, 0)
> ^
> tools/arch/x86/lib/insn.c:32:9: note: expanded from macro 'peek_nbyte_next'
> ({ if (unlikely(!validate_next(t, insn, n))) goto err_out; __peek_nbyte_next(t, insn, n); })
> ^
> tools/arch/x86/lib/insn.c:174:20: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> insn_byte_t b2 = peek_nbyte_next(insn_byte_t, insn, 1);
> ^
> tools/arch/x86/lib/insn.c:32:9: note: expanded from macro 'peek_nbyte_next'
> ({ if (unlikely(!validate_next(t, insn, n))) goto err_out; __peek_nbyte_next(t, insn, n); })
> ^
> tools/arch/x86/lib/insn.c:187:9: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> b2 = peek_nbyte_next(insn_byte_t, insn, 2);
> ^
> tools/arch/x86/lib/insn.c:32:9: note: expanded from macro 'peek_nbyte_next'
> ({ if (unlikely(!validate_next(t, insn, n))) goto err_out; __peek_nbyte_next(t, insn, n); })
> ^
> tools/arch/x86/lib/insn.c:189:9: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> b2 = peek_nbyte_next(insn_byte_t, insn, 3);
> ^
> tools/arch/x86/lib/insn.c:32:9: note: expanded from macro 'peek_nbyte_next'
> ({ if (unlikely(!validate_next(t, insn, n))) goto err_out; __peek_nbyte_next(t, insn, n); })
> ^
> tools/arch/x86/lib/insn.c:197:9: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> b2 = peek_nbyte_next(insn_byte_t, insn, 2);
> ^
> tools/arch/x86/lib/insn.c:32:9: note: expanded from macro 'peek_nbyte_next'
> ({ if (unlikely(!validate_next(t, insn, n))) goto err_out; __peek_nbyte_next(t, insn, n); })
> ^
> tools/arch/x86/lib/insn.c:245:7: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> op = get_next(insn_byte_t, insn);
> ^
> tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> ^
> tools/arch/x86/lib/insn.c:265:8: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> op = get_next(insn_byte_t, insn);
> ^
> tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> ^
> tools/arch/x86/lib/insn.c:297:9: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> mod = get_next(insn_byte_t, insn);
> ^
> tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> ^
> tools/arch/x86/lib/insn.c:359:22: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> insn->sib.value = get_next(insn_byte_t, insn);
> ^
> tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> ^
> tools/arch/x86/lib/insn.c:410:31: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> insn->displacement.value = get_next(signed char, insn);
> ^
> tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> ^
> tools/arch/x86/lib/insn.c:415:7: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> get_next(short, insn);
> --
> ^
> tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> ^
> tools/arch/x86/lib/insn.c:448:26: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> insn->moffset2.value = get_next(int, insn);
> ^
> tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> ^
> tools/arch/x86/lib/insn.c:467:27: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> insn->immediate.value = get_next(short, insn);
> ^
> tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> ^
> tools/arch/x86/lib/insn.c:472:27: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> insn->immediate.value = get_next(int, insn);
> ^
> tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> ^
> tools/arch/x86/lib/insn.c:490:28: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> insn->immediate1.value = get_next(short, insn);
> ^
> tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> ^
> tools/arch/x86/lib/insn.c:494:28: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> insn->immediate1.value = get_next(int, insn);
> ^
> tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> ^
> tools/arch/x86/lib/insn.c:498:28: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> insn->immediate1.value = get_next(int, insn);
> ^
> tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> ^
> tools/arch/x86/lib/insn.c:500:28: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> insn->immediate2.value = get_next(int, insn);
> ^
> tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> ^
> tools/arch/x86/lib/insn.c:518:28: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> insn->immediate1.value = get_next(short, insn);
> ^
> tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> ^
> tools/arch/x86/lib/insn.c:522:28: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> insn->immediate1.value = get_next(int, insn);
> ^
> tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> ^
> tools/arch/x86/lib/insn.c:531:27: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> insn->immediate2.value = get_next(unsigned short, insn);
> ^
> tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> ^
> tools/arch/x86/lib/insn.c:568:27: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> insn->immediate.value = get_next(signed char, insn);
> ^
> tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> ^
> tools/arch/x86/lib/insn.c:572:27: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> insn->immediate.value = get_next(short, insn);
> ^
> tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> ^
> tools/arch/x86/lib/insn.c:576:27: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> insn->immediate.value = get_next(int, insn);
> ^
> tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> ^
> tools/arch/x86/lib/insn.c:580:28: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> insn->immediate1.value = get_next(int, insn);
> ^
> tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> ^
> tools/arch/x86/lib/insn.c:582:28: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> insn->immediate2.value = get_next(int, insn);
> ^
> tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> ^
> tools/arch/x86/lib/insn.c:602:28: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> insn->immediate2.value = get_next(signed char, insn);
> ^
> tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> ^
> >> arch/x86/tools/insn_sanity.c:128:19: warning: implicit declaration of function 'ARRAY_SIZE' [-Wimplicit-function-declaration]
> tmp = fgets(buf, ARRAY_SIZE(buf), input_file);
> ^
> 37 warnings generated.
> /usr/bin/ld: /tmp/insn_sanity-8655a9.o: in function `insn_get_prefixes':
> >> insn_sanity.c:(.text+0x1bd): undefined reference to `unlikely'
> >> /usr/bin/ld: insn_sanity.c:(.text+0x203): undefined reference to `unlikely'
> /usr/bin/ld: insn_sanity.c:(.text+0x24d): undefined reference to `unlikely'
> /usr/bin/ld: insn_sanity.c:(.text+0x30f): undefined reference to `unlikely'
> /usr/bin/ld: insn_sanity.c:(.text+0x353): undefined reference to `unlikely'
> /usr/bin/ld: /tmp/insn_sanity-8655a9.o:insn_sanity.c:(.text+0x38e): more undefined references to `unlikely' follow
> /usr/bin/ld: /tmp/insn_sanity-8655a9.o: in function `main':
> >> insn_sanity.c:(.text+0x13cf): undefined reference to `ARRAY_SIZE'
> /usr/bin/ld: /tmp/insn_sanity-8655a9.o: in function `__insn_get_emulate_prefix':
> insn_sanity.c:(.text+0x1cc1): undefined reference to `unlikely'
> /usr/bin/ld: insn_sanity.c:(.text+0x1cef): undefined reference to `unlikely'
> /usr/bin/ld: insn_sanity.c:(.text+0x1d1f): undefined reference to `unlikely'
> /usr/bin/ld: insn_sanity.c:(.text+0x1d47): undefined reference to `unlikely'
> /usr/bin/ld: insn_sanity.c:(.text+0x1d6f): undefined reference to `unlikely'
> clang-12: error: linker command failed with exit code 1 (use -v to see invocation)

Right, this is expected. The patch is based on jpoimboe/objtool/core,
which has extra commits.

2020-11-04 11:58:08

by kernel test robot

Subject: Re: [PATCH 1/1] x86/tools: Use tools headers for instruction decoder selftests

Hi Vasily,

I love your patch! Yet something to improve:

[auto build test ERROR on tip/x86/core]
[also build test ERROR on v5.10-rc2 next-20201103]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/0day-ci/linux/commits/Vasily-Gorbik/x86-tools-Use-tools-headers-for-instruction-decoder-selftests/20201104-043600
base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 238c91115cd05c71447ea071624a4c9fe661f970
config: x86_64-randconfig-s022-20201103 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.3-76-gf680124b-dirty
# https://github.com/0day-ci/linux/commit/ab4952becdfae8a76a6f0e0fb4ec7d078e80d5d6
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Vasily-Gorbik/x86-tools-Use-tools-headers-for-instruction-decoder-selftests/20201104-043600
git checkout ab4952becdfae8a76a6f0e0fb4ec7d078e80d5d6
# save the attached .config to linux build tree
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All error/warnings (new ones prefixed by >>):

In file included from arch/x86/tools/insn_sanity.c:19:
tools/arch/x86/lib/insn.c: In function '__insn_get_emulate_prefix':
>> tools/arch/x86/lib/insn.c:32:9: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
32 | ({ if (unlikely(!validate_next(t, insn, n))) goto err_out; __peek_nbyte_next(t, insn, n); })
| ^~~~~~~~
tools/arch/x86/lib/insn.c:72:7: note: in expansion of macro 'peek_nbyte_next'
72 | if (peek_nbyte_next(insn_byte_t, insn, i) != prefix[i])
| ^~~~~~~~~~~~~~~
arch/x86/tools/insn_sanity.c: In function 'read_next_insn':
>> arch/x86/tools/insn_sanity.c:128:19: warning: implicit declaration of function 'ARRAY_SIZE' [-Wimplicit-function-declaration]
128 | tmp = fgets(buf, ARRAY_SIZE(buf), input_file);
| ^~~~~~~~~~
/usr/bin/ld: /tmp/ccO69OOl.o: in function `__insn_get_emulate_prefix.constprop.0':
>> insn_sanity.c:(.text+0x417): undefined reference to `unlikely'
/usr/bin/ld: /tmp/ccO69OOl.o: in function `insn_get_prefixes':
insn_sanity.c:(.text+0x647): undefined reference to `unlikely'
>> /usr/bin/ld: insn_sanity.c:(.text+0x6b7): undefined reference to `unlikely'
/usr/bin/ld: insn_sanity.c:(.text+0x744): undefined reference to `unlikely'
/usr/bin/ld: insn_sanity.c:(.text+0x77f): undefined reference to `unlikely'
/usr/bin/ld: /tmp/ccO69OOl.o:insn_sanity.c:(.text+0x817): more undefined references to `unlikely' follow
/usr/bin/ld: /tmp/ccO69OOl.o: in function `main':
>> insn_sanity.c:(.text.startup+0x48a): undefined reference to `ARRAY_SIZE'
collect2: error: ld returned 1 exit status

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]

Attachments:

2020-11-06 02:27:06

by Masami Hiramatsu

Subject: Re: [PATCH 1/1] x86/tools: Use tools headers for instruction decoder selftests

On Wed, 4 Nov 2020 10:18:43 +0100
Vasily Gorbik <[email protected]> wrote:

> On Wed, Nov 04, 2020 at 05:11:28PM +0800, kernel test robot wrote:
> > Hi Vasily,
> >
> > I love your patch! Yet something to improve:
> >
> > [auto build test ERROR on tip/x86/core]
> > [also build test ERROR on v5.10-rc2 next-20201103]
> > [If your patch is applied to the wrong git tree, kindly drop us a note.
> > And when submitting patch, we suggest to use '--base' as documented in
> > https://git-scm.com/docs/git-format-patch]
> >
> > url: https://github.com/0day-ci/linux/commits/Vasily-Gorbik/x86-tools-Use-tools-headers-for-instruction-decoder-selftests/20201104-043600
> > base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 238c91115cd05c71447ea071624a4c9fe661f970
> > config: x86_64-randconfig-a005-20201104 (attached as .config)
> > compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 1fcd5d5655e29f85e12b402e32974f207cfedf32)
> > reproduce (this is a W=1 build):
> > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> > chmod +x ~/bin/make.cross
> > # install x86_64 cross compiling tool for clang build
> > # apt-get install binutils-x86-64-linux-gnu
> > # https://github.com/0day-ci/linux/commit/ab4952becdfae8a76a6f0e0fb4ec7d078e80d5d6
> > git remote add linux-review https://github.com/0day-ci/linux
> > git fetch --no-tags linux-review Vasily-Gorbik/x86-tools-Use-tools-headers-for-instruction-decoder-selftests/20201104-043600
> > git checkout ab4952becdfae8a76a6f0e0fb4ec7d078e80d5d6
> > # save the attached .config to linux build tree
> > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
> >
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <[email protected]>
> >
> > All error/warnings (new ones prefixed by >>):
> >
> > In file included from arch/x86/tools/insn_sanity.c:19:
> > >> tools/arch/x86/lib/insn.c:72:7: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> > if (peek_nbyte_next(insn_byte_t, insn, i) != prefix[i])
> > ^
> > tools/arch/x86/lib/insn.c:32:9: note: expanded from macro 'peek_nbyte_next'
> > ({ if (unlikely(!validate_next(t, insn, n))) goto err_out; __peek_nbyte_next(t, insn, n); })
> > ^
> > tools/arch/x86/lib/insn.c:115:6: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> > b = peek_next(insn_byte_t, insn);
> > ^
> > tools/arch/x86/lib/insn.c:34:28: note: expanded from macro 'peek_next'
> > #define peek_next(t, insn) peek_nbyte_next(t, insn, 0)
> > ^
> > tools/arch/x86/lib/insn.c:32:9: note: expanded from macro 'peek_nbyte_next'
> > ({ if (unlikely(!validate_next(t, insn, n))) goto err_out; __peek_nbyte_next(t, insn, n); })
> > ^
> > tools/arch/x86/lib/insn.c:140:7: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> > b = peek_next(insn_byte_t, insn);
> > ^
> > tools/arch/x86/lib/insn.c:34:28: note: expanded from macro 'peek_next'
> > #define peek_next(t, insn) peek_nbyte_next(t, insn, 0)
> > ^
> > tools/arch/x86/lib/insn.c:32:9: note: expanded from macro 'peek_nbyte_next'
> > ({ if (unlikely(!validate_next(t, insn, n))) goto err_out; __peek_nbyte_next(t, insn, n); })
> > ^
> > tools/arch/x86/lib/insn.c:145:7: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> > if (unlikely(insn->prefixes.bytes[3])) {
> > ^
> > tools/arch/x86/lib/insn.c:157:7: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> > b = peek_next(insn_byte_t, insn);
> > ^
> > tools/arch/x86/lib/insn.c:34:28: note: expanded from macro 'peek_next'
> > #define peek_next(t, insn) peek_nbyte_next(t, insn, 0)
> > ^
> > tools/arch/x86/lib/insn.c:32:9: note: expanded from macro 'peek_nbyte_next'
> > ({ if (unlikely(!validate_next(t, insn, n))) goto err_out; __peek_nbyte_next(t, insn, n); })
> > ^
> > tools/arch/x86/lib/insn.c:171:6: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> > b = peek_next(insn_byte_t, insn);
> > ^
> > tools/arch/x86/lib/insn.c:34:28: note: expanded from macro 'peek_next'
> > #define peek_next(t, insn) peek_nbyte_next(t, insn, 0)
> > ^
> > tools/arch/x86/lib/insn.c:32:9: note: expanded from macro 'peek_nbyte_next'
> > ({ if (unlikely(!validate_next(t, insn, n))) goto err_out; __peek_nbyte_next(t, insn, n); })
> > ^
> > tools/arch/x86/lib/insn.c:174:20: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> > insn_byte_t b2 = peek_nbyte_next(insn_byte_t, insn, 1);
> > ^
> > tools/arch/x86/lib/insn.c:32:9: note: expanded from macro 'peek_nbyte_next'
> > ({ if (unlikely(!validate_next(t, insn, n))) goto err_out; __peek_nbyte_next(t, insn, n); })
> > ^
> > tools/arch/x86/lib/insn.c:187:9: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> > b2 = peek_nbyte_next(insn_byte_t, insn, 2);
> > ^
> > tools/arch/x86/lib/insn.c:32:9: note: expanded from macro 'peek_nbyte_next'
> > ({ if (unlikely(!validate_next(t, insn, n))) goto err_out; __peek_nbyte_next(t, insn, n); })
> > ^
> > tools/arch/x86/lib/insn.c:189:9: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> > b2 = peek_nbyte_next(insn_byte_t, insn, 3);
> > ^
> > tools/arch/x86/lib/insn.c:32:9: note: expanded from macro 'peek_nbyte_next'
> > ({ if (unlikely(!validate_next(t, insn, n))) goto err_out; __peek_nbyte_next(t, insn, n); })
> > ^
> > tools/arch/x86/lib/insn.c:197:9: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> > b2 = peek_nbyte_next(insn_byte_t, insn, 2);
> > ^
> > tools/arch/x86/lib/insn.c:32:9: note: expanded from macro 'peek_nbyte_next'
> > ({ if (unlikely(!validate_next(t, insn, n))) goto err_out; __peek_nbyte_next(t, insn, n); })
> > ^
> > tools/arch/x86/lib/insn.c:245:7: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> > op = get_next(insn_byte_t, insn);
> > ^
> > tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> > ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> > ^
> > tools/arch/x86/lib/insn.c:265:8: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> > op = get_next(insn_byte_t, insn);
> > ^
> > tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> > ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> > ^
> > tools/arch/x86/lib/insn.c:297:9: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> > mod = get_next(insn_byte_t, insn);
> > ^
> > tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> > ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> > ^
> > tools/arch/x86/lib/insn.c:359:22: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> > insn->sib.value = get_next(insn_byte_t, insn);
> > ^
> > tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> > ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> > ^
> > tools/arch/x86/lib/insn.c:410:31: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> > insn->displacement.value = get_next(signed char, insn);
> > ^
> > tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> > ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> > ^
> > tools/arch/x86/lib/insn.c:415:7: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> > get_next(short, insn);
> > --
> > ^
> > tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> > ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> > ^
> > tools/arch/x86/lib/insn.c:448:26: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> > insn->moffset2.value = get_next(int, insn);
> > ^
> > tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> > ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> > ^
> > tools/arch/x86/lib/insn.c:467:27: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> > insn->immediate.value = get_next(short, insn);
> > ^
> > tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> > ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> > ^
> > tools/arch/x86/lib/insn.c:472:27: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> > insn->immediate.value = get_next(int, insn);
> > ^
> > tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> > ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> > ^
> > tools/arch/x86/lib/insn.c:490:28: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> > insn->immediate1.value = get_next(short, insn);
> > ^
> > tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> > ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> > ^
> > tools/arch/x86/lib/insn.c:494:28: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> > insn->immediate1.value = get_next(int, insn);
> > ^
> > tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> > ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> > ^
> > tools/arch/x86/lib/insn.c:498:28: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> > insn->immediate1.value = get_next(int, insn);
> > ^
> > tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> > ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> > ^
> > tools/arch/x86/lib/insn.c:500:28: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> > insn->immediate2.value = get_next(int, insn);
> > ^
> > tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> > ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> > ^
> > tools/arch/x86/lib/insn.c:518:28: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> > insn->immediate1.value = get_next(short, insn);
> > ^
> > tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> > ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> > ^
> > tools/arch/x86/lib/insn.c:522:28: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> > insn->immediate1.value = get_next(int, insn);
> > ^
> > tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> > ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> > ^
> > tools/arch/x86/lib/insn.c:531:27: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> > insn->immediate2.value = get_next(unsigned short, insn);
> > ^
> > tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> > ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> > ^
> > tools/arch/x86/lib/insn.c:568:27: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> > insn->immediate.value = get_next(signed char, insn);
> > ^
> > tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> > ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> > ^
> > tools/arch/x86/lib/insn.c:572:27: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> > insn->immediate.value = get_next(short, insn);
> > ^
> > tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> > ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> > ^
> > tools/arch/x86/lib/insn.c:576:27: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> > insn->immediate.value = get_next(int, insn);
> > ^
> > tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> > ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> > ^
> > tools/arch/x86/lib/insn.c:580:28: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> > insn->immediate1.value = get_next(int, insn);
> > ^
> > tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> > ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> > ^
> > tools/arch/x86/lib/insn.c:582:28: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> > insn->immediate2.value = get_next(int, insn);
> > ^
> > tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> > ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> > ^
> > tools/arch/x86/lib/insn.c:602:28: warning: implicit declaration of function 'unlikely' [-Wimplicit-function-declaration]
> > insn->immediate2.value = get_next(signed char, insn);
> > ^
> > tools/arch/x86/lib/insn.c:29:9: note: expanded from macro 'get_next'
> > ({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; __get_next(t, insn); })
> > ^
> > >> arch/x86/tools/insn_sanity.c:128:19: warning: implicit declaration of function 'ARRAY_SIZE' [-Wimplicit-function-declaration]
> > tmp = fgets(buf, ARRAY_SIZE(buf), input_file);
> > ^
> > 37 warnings generated.
> > /usr/bin/ld: /tmp/insn_sanity-8655a9.o: in function `insn_get_prefixes':
> > >> insn_sanity.c:(.text+0x1bd): undefined reference to `unlikely'
> > >> /usr/bin/ld: insn_sanity.c:(.text+0x203): undefined reference to `unlikely'
> > /usr/bin/ld: insn_sanity.c:(.text+0x24d): undefined reference to `unlikely'
> > /usr/bin/ld: insn_sanity.c:(.text+0x30f): undefined reference to `unlikely'
> > /usr/bin/ld: insn_sanity.c:(.text+0x353): undefined reference to `unlikely'
> > /usr/bin/ld: /tmp/insn_sanity-8655a9.o:insn_sanity.c:(.text+0x38e): more undefined references to `unlikely' follow
> > /usr/bin/ld: /tmp/insn_sanity-8655a9.o: in function `main':
> > >> insn_sanity.c:(.text+0x13cf): undefined reference to `ARRAY_SIZE'
> > /usr/bin/ld: /tmp/insn_sanity-8655a9.o: in function `__insn_get_emulate_prefix':
> > insn_sanity.c:(.text+0x1cc1): undefined reference to `unlikely'
> > /usr/bin/ld: insn_sanity.c:(.text+0x1cef): undefined reference to `unlikely'
> > /usr/bin/ld: insn_sanity.c:(.text+0x1d1f): undefined reference to `unlikely'
> > /usr/bin/ld: insn_sanity.c:(.text+0x1d47): undefined reference to `unlikely'
> > /usr/bin/ld: insn_sanity.c:(.text+0x1d6f): undefined reference to `unlikely'
> > clang-12: error: linker command failed with exit code 1 (use -v to see invocation)
>
> Right, this is expected. The patch is based on jpoimboe/objtool/core,
> which has extra commits.

Has that series already submitted to LKML? I need to look at the series too.
Or, Josh, can you review it and if it is OK, please pick it to your series
and send it.

Thank you,

--
Masami Hiramatsu <[email protected]>

2020-11-06 17:54:46

by Josh Poimboeuf

Subject: Re: [PATCH 1/1] x86/tools: Use tools headers for instruction decoder selftests

On Fri, Nov 06, 2020 at 11:24:13AM +0900, Masami Hiramatsu wrote:
> > Right, this is expected. The patch is based on jpoimboe/objtool/core,
> > which has extra commits.
>
> Has that series already submitted to LKML? I need to look at the series too.
> Or, Josh, can you review it and if it is OK, please pick it to your series
> and send it.

I believe those patches were dropped from -tip because of a build issue.

Vasily, can you repost fixed versions of those patches, based on
tip/objtool/core, along with this new patch?

--
Josh