* [PATCH v6 0/9] Add jump table support for objtool on LoongArch
@ 2024-12-17 1:08 Tiezhu Yang
2024-12-17 1:08 ` [PATCH v6 1/9] objtool: Handle various symbol types of rodata Tiezhu Yang
` (10 more replies)
0 siblings, 11 replies; 37+ messages in thread
From: Tiezhu Yang @ 2024-12-17 1:08 UTC (permalink / raw)
To: Huacai Chen, Josh Poimboeuf, Peter Zijlstra
Cc: loongarch, amd-gfx, linux-kernel
This version is based on tip/tip.git objtool/core branch [1], add some weak
and arch-specific functions to make the generic code more readable, tested
with the latest upstream mainline Binutils, GCC and Clang.
The first 6 patches are preparation for patch #7 to enable jump table for
objtool on LoongArch, the last 2 patches are small enough to fix objtool
warnings "funcA() falls through to next function funcB()", one is under
arch/loongarch and the other is under drm/amd/display.
v6:
-- Add arch_reloc_size() for x86 and ppc.
-- Call arch_reloc_size() directly in add_jump_table().
-- Refine arch_adjust_offset() for LoongArch.
-- Rename arch_adjust_offset() to arch_jump_table_sym_offset().
-- Get each table size of rodata in time for switch table.
-- Update the commit message to make it more clear.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/log/?h=objtool/core
Tiezhu Yang (9):
objtool: Handle various symbol types of rodata
objtool: Handle different entry size of rodata
objtool: Handle PC relative relocation type
objtool: Handle unreachable entry of rodata
objtool/LoongArch: Add support for switch table
objtool/LoongArch: Add support for goto table
LoongArch: Enable jump table for objtool
LoongArch: Convert unreachable() to BUG()
drm/amd/display: Mark dc_fixpt_from_fraction() noinline
arch/loongarch/Kconfig | 3 +
arch/loongarch/Makefile | 6 +-
arch/loongarch/kernel/machine_kexec.c | 4 +-
.../drm/amd/display/dc/basics/fixpt31_32.c | 2 +-
tools/objtool/arch/loongarch/decode.c | 28 ++-
.../objtool/arch/loongarch/include/arch/elf.h | 7 +
tools/objtool/arch/loongarch/special.c | 159 +++++++++++++++++-
tools/objtool/arch/powerpc/decode.c | 15 ++
tools/objtool/arch/x86/decode.c | 13 ++
tools/objtool/check.c | 28 ++-
tools/objtool/include/objtool/arch.h | 3 +
11 files changed, 251 insertions(+), 17 deletions(-)
--
2.42.0
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH v6 1/9] objtool: Handle various symbol types of rodata
2024-12-17 1:08 [PATCH v6 0/9] Add jump table support for objtool on LoongArch Tiezhu Yang
@ 2024-12-17 1:08 ` Tiezhu Yang
2024-12-17 1:08 ` [PATCH v6 2/9] objtool: Handle different entry size " Tiezhu Yang
` (9 subsequent siblings)
10 siblings, 0 replies; 37+ messages in thread
From: Tiezhu Yang @ 2024-12-17 1:08 UTC (permalink / raw)
To: Huacai Chen, Josh Poimboeuf, Peter Zijlstra
Cc: loongarch, amd-gfx, linux-kernel
In the relocation section ".rela.rodata" of each .o file compiled with
LoongArch toolchain, there are various symbol types such as STT_NOTYPE,
STT_OBJECT, STT_FUNC in addition to the usual STT_SECTION, it needs to
use reloc symbol offset instead of reloc addend to find the destination
instruction in find_jump_table() and add_jump_table().
For the most part, an absolute relocation type is used for rodata. In the
case of STT_SECTION, reloc->sym->offset is always zero, and for the other
symbol types, reloc_addend(reloc) is always zero, thus it can use a simple
statement "reloc->sym->offset + reloc_addend(reloc)" to obtain the symbol
offset for various symbol types.
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
tools/objtool/check.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index e92c5564d9ca..f64435ad3514 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1953,6 +1953,7 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,
unsigned int prev_offset = 0;
struct reloc *reloc = table;
struct alternative *alt;
+ unsigned long sym_offset;
/*
* Each @reloc is a switch table relocation which points to the target
@@ -1970,12 +1971,13 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,
if (prev_offset && reloc_offset(reloc) != prev_offset + 8)
break;
+ sym_offset = reloc->sym->offset + reloc_addend(reloc);
+
/* Detect function pointers from contiguous objects: */
- if (reloc->sym->sec == pfunc->sec &&
- reloc_addend(reloc) == pfunc->offset)
+ if (reloc->sym->sec == pfunc->sec && sym_offset == pfunc->offset)
break;
- dest_insn = find_insn(file, reloc->sym->sec, reloc_addend(reloc));
+ dest_insn = find_insn(file, reloc->sym->sec, sym_offset);
if (!dest_insn)
break;
@@ -2013,6 +2015,7 @@ static void find_jump_table(struct objtool_file *file, struct symbol *func,
struct reloc *table_reloc;
struct instruction *dest_insn, *orig_insn = insn;
unsigned long table_size;
+ unsigned long sym_offset;
/*
* Backward search using the @first_jump_src links, these help avoid
@@ -2036,7 +2039,10 @@ static void find_jump_table(struct objtool_file *file, struct symbol *func,
table_reloc = arch_find_switch_table(file, insn, &table_size);
if (!table_reloc)
continue;
- dest_insn = find_insn(file, table_reloc->sym->sec, reloc_addend(table_reloc));
+
+ sym_offset = table_reloc->sym->offset + reloc_addend(table_reloc);
+
+ dest_insn = find_insn(file, table_reloc->sym->sec, sym_offset);
if (!dest_insn || !insn_func(dest_insn) || insn_func(dest_insn)->pfunc != func)
continue;
--
2.42.0
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH v6 2/9] objtool: Handle different entry size of rodata
2024-12-17 1:08 [PATCH v6 0/9] Add jump table support for objtool on LoongArch Tiezhu Yang
2024-12-17 1:08 ` [PATCH v6 1/9] objtool: Handle various symbol types of rodata Tiezhu Yang
@ 2024-12-17 1:08 ` Tiezhu Yang
2025-02-11 14:19 ` [PATCH] objtool: remove duplicate case value R_PPC64_REL32 Kexy Biscuit
2024-12-17 1:08 ` [PATCH v6 3/9] objtool: Handle PC relative relocation type Tiezhu Yang
` (8 subsequent siblings)
10 siblings, 1 reply; 37+ messages in thread
From: Tiezhu Yang @ 2024-12-17 1:08 UTC (permalink / raw)
To: Huacai Chen, Josh Poimboeuf, Peter Zijlstra
Cc: loongarch, amd-gfx, linux-kernel
In the most cases, the entry size of rodata is 8 bytes because the
relocation type is 64 bit. There are also 32 bit relocation types,
the entry size of rodata should be 4 bytes in this case.
Add an arch-specific function arch_reloc_size() to assign the entry
size of rodata for x86, powerpc and LoongArch.
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
tools/objtool/arch/loongarch/decode.c | 11 +++++++++++
tools/objtool/arch/powerpc/decode.c | 15 +++++++++++++++
tools/objtool/arch/x86/decode.c | 13 +++++++++++++
tools/objtool/check.c | 2 +-
tools/objtool/include/objtool/arch.h | 2 ++
5 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/tools/objtool/arch/loongarch/decode.c b/tools/objtool/arch/loongarch/decode.c
index 69b66994f2a1..b64205b89f6b 100644
--- a/tools/objtool/arch/loongarch/decode.c
+++ b/tools/objtool/arch/loongarch/decode.c
@@ -363,3 +363,14 @@ void arch_initial_func_cfi_state(struct cfi_init_state *state)
state->cfa.base = CFI_SP;
state->cfa.offset = 0;
}
+
+unsigned int arch_reloc_size(struct reloc *reloc)
+{
+ switch (reloc_type(reloc)) {
+ case R_LARCH_32:
+ case R_LARCH_32_PCREL:
+ return 4;
+ default:
+ return 8;
+ }
+}
diff --git a/tools/objtool/arch/powerpc/decode.c b/tools/objtool/arch/powerpc/decode.c
index 53b55690f320..3c95dd74fca0 100644
--- a/tools/objtool/arch/powerpc/decode.c
+++ b/tools/objtool/arch/powerpc/decode.c
@@ -106,3 +106,18 @@ void arch_initial_func_cfi_state(struct cfi_init_state *state)
state->regs[CFI_RA].base = CFI_CFA;
state->regs[CFI_RA].offset = 0;
}
+
+unsigned int arch_reloc_size(struct reloc *reloc)
+{
+ switch (reloc_type(reloc)) {
+ case R_PPC_REL32:
+ case R_PPC64_REL32:
+ case R_PPC_ADDR32:
+ case R_PPC_UADDR32:
+ case R_PPC_PLT32:
+ case R_PPC_PLTREL32:
+ return 4;
+ default:
+ return 8;
+ }
+}
diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
index fe1362c34564..fb9691a34d92 100644
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -852,3 +852,16 @@ bool arch_is_embedded_insn(struct symbol *sym)
return !strcmp(sym->name, "retbleed_return_thunk") ||
!strcmp(sym->name, "srso_safe_ret");
}
+
+unsigned int arch_reloc_size(struct reloc *reloc)
+{
+ switch (reloc_type(reloc)) {
+ case R_X86_64_32:
+ case R_X86_64_32S:
+ case R_X86_64_PC32:
+ case R_X86_64_PLT32:
+ return 4;
+ default:
+ return 8;
+ }
+}
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index f64435ad3514..d8668ae0f599 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1968,7 +1968,7 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,
break;
/* Make sure the table entries are consecutive: */
- if (prev_offset && reloc_offset(reloc) != prev_offset + 8)
+ if (prev_offset && reloc_offset(reloc) != prev_offset + arch_reloc_size(reloc))
break;
sym_offset = reloc->sym->offset + reloc_addend(reloc);
diff --git a/tools/objtool/include/objtool/arch.h b/tools/objtool/include/objtool/arch.h
index d63b46a19f39..396f7c6c81c0 100644
--- a/tools/objtool/include/objtool/arch.h
+++ b/tools/objtool/include/objtool/arch.h
@@ -97,4 +97,6 @@ int arch_rewrite_retpolines(struct objtool_file *file);
bool arch_pc_relative_reloc(struct reloc *reloc);
+unsigned int arch_reloc_size(struct reloc *reloc);
+
#endif /* _ARCH_H */
--
2.42.0
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH v6 3/9] objtool: Handle PC relative relocation type
2024-12-17 1:08 [PATCH v6 0/9] Add jump table support for objtool on LoongArch Tiezhu Yang
2024-12-17 1:08 ` [PATCH v6 1/9] objtool: Handle various symbol types of rodata Tiezhu Yang
2024-12-17 1:08 ` [PATCH v6 2/9] objtool: Handle different entry size " Tiezhu Yang
@ 2024-12-17 1:08 ` Tiezhu Yang
2024-12-17 1:09 ` [PATCH v6 4/9] objtool: Handle unreachable entry of rodata Tiezhu Yang
` (7 subsequent siblings)
10 siblings, 0 replies; 37+ messages in thread
From: Tiezhu Yang @ 2024-12-17 1:08 UTC (permalink / raw)
To: Huacai Chen, Josh Poimboeuf, Peter Zijlstra
Cc: loongarch, amd-gfx, linux-kernel
For the most part, an absolute relocation type is used for rodata.
In the case of STT_SECTION, reloc->sym->offset is always zero, for
the other symbol types, reloc_addend(reloc) is always zero, thus it
can use a simple statement "reloc->sym->offset + reloc_addend(reloc)"
to obtain the symbol offset for various symbol types.
When compiling on LoongArch, there exist PC relative relocation types
for rodata, it needs to calculate the symbol offset with "S + A - PC"
according to the spec of "ELF for the LoongArch Architecture".
If there is only one jump table in the rodata, the "PC" is the entry
address which is equal with the value of reloc_offset(reloc), at this
time, reloc_offset(table) is 0.
If there are many jump tables in the rodata, the "PC" is the offset
of the jump table's base address which is equal with the value of
reloc_offset(reloc) - reloc_offset(table).
So for LoongArch, if the relocation type is PC relative, it can use a
statement "reloc_offset(reloc) - reloc_offset(table)" to get the "PC"
value when calculating the symbol offset with "S + A - PC" for one or
many jump tables in the rodata.
Add an arch-specific function arch_jump_table_sym_offset() to assign
the symbol offset, for the most part that is an absolute relocation,
the default value is "reloc->sym->offset + reloc_addend(reloc)" in
the weak definition, it can be overridden by each architecture that
has different requirements.
Link: https://github.com/loongson/la-abi-specs/blob/release/laelf.adoc
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
tools/objtool/arch/loongarch/decode.c | 17 +++++++++++++----
tools/objtool/arch/loongarch/include/arch/elf.h | 7 +++++++
tools/objtool/check.c | 7 ++++++-
tools/objtool/include/objtool/arch.h | 1 +
4 files changed, 27 insertions(+), 5 deletions(-)
diff --git a/tools/objtool/arch/loongarch/decode.c b/tools/objtool/arch/loongarch/decode.c
index b64205b89f6b..02e490555966 100644
--- a/tools/objtool/arch/loongarch/decode.c
+++ b/tools/objtool/arch/loongarch/decode.c
@@ -5,10 +5,7 @@
#include <asm/inst.h>
#include <asm/orc_types.h>
#include <linux/objtool_types.h>
-
-#ifndef EM_LOONGARCH
-#define EM_LOONGARCH 258
-#endif
+#include <arch/elf.h>
int arch_ftrace_match(char *name)
{
@@ -374,3 +371,15 @@ unsigned int arch_reloc_size(struct reloc *reloc)
return 8;
}
}
+
+unsigned long arch_jump_table_sym_offset(struct reloc *reloc, struct reloc *table)
+{
+ switch (reloc_type(reloc)) {
+ case R_LARCH_32_PCREL:
+ case R_LARCH_64_PCREL:
+ return reloc->sym->offset + reloc_addend(reloc) -
+ (reloc_offset(reloc) - reloc_offset(table));
+ default:
+ return reloc->sym->offset + reloc_addend(reloc);
+ }
+}
diff --git a/tools/objtool/arch/loongarch/include/arch/elf.h b/tools/objtool/arch/loongarch/include/arch/elf.h
index 9623d663220e..ec79062c9554 100644
--- a/tools/objtool/arch/loongarch/include/arch/elf.h
+++ b/tools/objtool/arch/loongarch/include/arch/elf.h
@@ -18,6 +18,13 @@
#ifndef R_LARCH_32_PCREL
#define R_LARCH_32_PCREL 99
#endif
+#ifndef R_LARCH_64_PCREL
+#define R_LARCH_64_PCREL 109
+#endif
+
+#ifndef EM_LOONGARCH
+#define EM_LOONGARCH 258
+#endif
#define R_NONE R_LARCH_NONE
#define R_ABS32 R_LARCH_32
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index d8668ae0f599..cff7416b207e 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1943,6 +1943,11 @@ static int add_special_section_alts(struct objtool_file *file)
return ret;
}
+__weak unsigned long arch_jump_table_sym_offset(struct reloc *reloc, struct reloc *table)
+{
+ return reloc->sym->offset + reloc_addend(reloc);
+}
+
static int add_jump_table(struct objtool_file *file, struct instruction *insn,
struct reloc *next_table)
{
@@ -1971,7 +1976,7 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,
if (prev_offset && reloc_offset(reloc) != prev_offset + arch_reloc_size(reloc))
break;
- sym_offset = reloc->sym->offset + reloc_addend(reloc);
+ sym_offset = arch_jump_table_sym_offset(reloc, table);
/* Detect function pointers from contiguous objects: */
if (reloc->sym->sec == pfunc->sec && sym_offset == pfunc->offset)
diff --git a/tools/objtool/include/objtool/arch.h b/tools/objtool/include/objtool/arch.h
index 396f7c6c81c0..089a1acc48a8 100644
--- a/tools/objtool/include/objtool/arch.h
+++ b/tools/objtool/include/objtool/arch.h
@@ -98,5 +98,6 @@ int arch_rewrite_retpolines(struct objtool_file *file);
bool arch_pc_relative_reloc(struct reloc *reloc);
unsigned int arch_reloc_size(struct reloc *reloc);
+unsigned long arch_jump_table_sym_offset(struct reloc *reloc, struct reloc *table);
#endif /* _ARCH_H */
--
2.42.0
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH v6 4/9] objtool: Handle unreachable entry of rodata
2024-12-17 1:08 [PATCH v6 0/9] Add jump table support for objtool on LoongArch Tiezhu Yang
` (2 preceding siblings ...)
2024-12-17 1:08 ` [PATCH v6 3/9] objtool: Handle PC relative relocation type Tiezhu Yang
@ 2024-12-17 1:09 ` Tiezhu Yang
2025-02-10 21:17 ` Josh Poimboeuf
2024-12-17 1:09 ` [PATCH v6 5/9] objtool/LoongArch: Add support for switch table Tiezhu Yang
` (6 subsequent siblings)
10 siblings, 1 reply; 37+ messages in thread
From: Tiezhu Yang @ 2024-12-17 1:09 UTC (permalink / raw)
To: Huacai Chen, Josh Poimboeuf, Peter Zijlstra
Cc: loongarch, amd-gfx, linux-kernel
When compiling with Clang on LoongArch, there exists unreachable entry of
rodata which points to a position after the function return instruction,
this is generated by compiler to fill the non-existent switch case, just
skip the entry when parsing the relocation section of rodata.
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
tools/objtool/check.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index cff7416b207e..654cffcf9512 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1986,9 +1986,10 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,
if (!dest_insn)
break;
- /* Make sure the destination is in the same function: */
- if (!insn_func(dest_insn) || insn_func(dest_insn)->pfunc != pfunc)
- break;
+ if (!insn_func(dest_insn) || insn_func(dest_insn)->pfunc != pfunc) {
+ prev_offset = reloc_offset(reloc);
+ continue;
+ }
alt = malloc(sizeof(*alt));
if (!alt) {
--
2.42.0
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH v6 5/9] objtool/LoongArch: Add support for switch table
2024-12-17 1:08 [PATCH v6 0/9] Add jump table support for objtool on LoongArch Tiezhu Yang
` (3 preceding siblings ...)
2024-12-17 1:09 ` [PATCH v6 4/9] objtool: Handle unreachable entry of rodata Tiezhu Yang
@ 2024-12-17 1:09 ` Tiezhu Yang
2024-12-17 1:09 ` [PATCH v6 6/9] objtool/LoongArch: Add support for goto table Tiezhu Yang
` (5 subsequent siblings)
10 siblings, 0 replies; 37+ messages in thread
From: Tiezhu Yang @ 2024-12-17 1:09 UTC (permalink / raw)
To: Huacai Chen, Josh Poimboeuf, Peter Zijlstra
Cc: loongarch, amd-gfx, linux-kernel
The objtool program need to analysis the control flow of each object file
generated by compiler toolchain, it needs to know all the locations that
a branch instruction may jump into, if a jump table is used, objtool has
to correlate the jump instruction with the table.
On x86 (which is the only port supported by objtool before LoongArch),
there is a relocation type on the jump instruction and directly points
to the table. But on LoongArch, the relocation is on another kind of
instruction prior to the jump instruction, and also with scheduling it
is not very easy to tell the offset of that instruction from the jump
instruction. Furthermore, because LoongArch has -fsection-anchors (often
enabled at -O1 or above) the relocation may actually points to a section
anchor instead of the table itself.
The good news is that after continuous analysis and discussion, at last
a GCC patch "LoongArch: Add support to annotate tablejump" and a Clang
patch "[LoongArch] Add options for annotate tablejump" have been merged
into the upstream mainline, the compiler changes make life much easier
for switch table support of objtool on LoongArch.
By now, there is an additional section ".discard.tablejump_annotate" to
store the jump info as pairs of addresses, each pair contains the address
of jump instruction and the address of jump table.
In order to find switch table, it is easy to parse the relocation section
".rela.discard.tablejump_annotate" to get table_sec and table_offset, the
rest process is somehow like x86.
Additionally, it needs to get each table size. When compiling on LoongArch,
there are unsorted table offsets of rodata if there exist many jump tables,
it will get the wrong table end and find the wrong table jump destination
instructions in add_jump_table().
Sort the rodata table offset by parsing ".rela.discard.tablejump_annotate"
and then get each table size of rodata corresponded with each table jump
instruction, it is used to check the table end and will break the process
when parsing ".rela.rodata" to avoid getting the wrong jump destination
instructions.
Link: https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=0ee028f55640
Link: https://github.com/llvm/llvm-project/commit/4c2c17756739
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
tools/objtool/arch/loongarch/special.c | 131 ++++++++++++++++++++++++-
1 file changed, 130 insertions(+), 1 deletion(-)
diff --git a/tools/objtool/arch/loongarch/special.c b/tools/objtool/arch/loongarch/special.c
index 87230ed570fd..4fa6877d5b2b 100644
--- a/tools/objtool/arch/loongarch/special.c
+++ b/tools/objtool/arch/loongarch/special.c
@@ -1,5 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-or-later
+#include <string.h>
#include <objtool/special.h>
+#include <objtool/warn.h>
bool arch_support_alt_relocation(struct special_alt *special_alt,
struct instruction *insn,
@@ -8,9 +10,136 @@ bool arch_support_alt_relocation(struct special_alt *special_alt,
return false;
}
+struct table_info {
+ struct list_head jump_info;
+ unsigned long insn_offset;
+ unsigned long rodata_offset;
+};
+
+static void get_rodata_table_size_by_table_annotate(struct objtool_file *file,
+ struct instruction *insn,
+ unsigned long *table_size)
+{
+ struct section *rsec;
+ struct reloc *reloc;
+ struct list_head table_list;
+ struct table_info *orig_table;
+ struct table_info *next_table;
+ unsigned long tmp_insn_offset;
+ unsigned long tmp_rodata_offset;
+
+ rsec = find_section_by_name(file->elf, ".rela.discard.tablejump_annotate");
+ if (!rsec)
+ return;
+
+ INIT_LIST_HEAD(&table_list);
+
+ for_each_reloc(rsec, reloc) {
+ orig_table = malloc(sizeof(struct table_info));
+ if (!orig_table) {
+ WARN("malloc failed");
+ return;
+ }
+
+ orig_table->insn_offset = reloc->sym->offset + reloc_addend(reloc);
+ reloc++;
+ orig_table->rodata_offset = reloc->sym->offset + reloc_addend(reloc);
+
+ list_add_tail(&orig_table->jump_info, &table_list);
+
+ if (reloc_idx(reloc) + 1 == sec_num_entries(rsec))
+ break;
+ }
+
+ list_for_each_entry(orig_table, &table_list, jump_info) {
+ next_table = list_next_entry(orig_table, jump_info);
+ list_for_each_entry_from(next_table, &table_list, jump_info) {
+ if (next_table->rodata_offset < orig_table->rodata_offset) {
+ tmp_insn_offset = next_table->insn_offset;
+ tmp_rodata_offset = next_table->rodata_offset;
+ next_table->insn_offset = orig_table->insn_offset;
+ next_table->rodata_offset = orig_table->rodata_offset;
+ orig_table->insn_offset = tmp_insn_offset;
+ orig_table->rodata_offset = tmp_rodata_offset;
+ }
+ }
+ }
+
+ list_for_each_entry(orig_table, &table_list, jump_info) {
+ if (insn->offset == orig_table->insn_offset) {
+ next_table = list_next_entry(orig_table, jump_info);
+ if (&next_table->jump_info == &table_list) {
+ *table_size = 0;
+ return;
+ }
+
+ while (next_table->rodata_offset == orig_table->rodata_offset) {
+ next_table = list_next_entry(next_table, jump_info);
+ if (&next_table->jump_info == &table_list) {
+ *table_size = 0;
+ return;
+ }
+ }
+
+ *table_size = next_table->rodata_offset - orig_table->rodata_offset;
+ }
+ }
+}
+
+static struct reloc *find_reloc_by_table_annotate(struct objtool_file *file,
+ struct instruction *insn,
+ unsigned long *table_size)
+{
+ struct section *rsec;
+ struct reloc *reloc;
+ unsigned long offset;
+
+ rsec = find_section_by_name(file->elf, ".rela.discard.tablejump_annotate");
+ if (!rsec)
+ return NULL;
+
+ for_each_reloc(rsec, reloc) {
+ if (reloc->sym->sec->rodata)
+ continue;
+
+ if (strcmp(insn->sec->name, reloc->sym->sec->name))
+ continue;
+
+ offset = reloc->sym->offset + reloc_addend(reloc);
+ if (insn->offset == offset) {
+ get_rodata_table_size_by_table_annotate(file, insn, table_size);
+ reloc++;
+ return reloc;
+ }
+ }
+
+ return NULL;
+}
+
struct reloc *arch_find_switch_table(struct objtool_file *file,
struct instruction *insn,
unsigned long *table_size)
{
- return NULL;
+ struct reloc *annotate_reloc;
+ struct reloc *rodata_reloc;
+ struct section *table_sec;
+ unsigned long table_offset;
+
+ annotate_reloc = find_reloc_by_table_annotate(file, insn, table_size);
+ if (!annotate_reloc)
+ return NULL;
+
+ table_sec = annotate_reloc->sym->sec;
+ table_offset = annotate_reloc->sym->offset + reloc_addend(annotate_reloc);
+
+ /*
+ * Each table entry has a rela associated with it. The rela
+ * should reference text in the same function as the original
+ * instruction.
+ */
+ rodata_reloc = find_reloc_by_dest(file->elf, table_sec, table_offset);
+ if (!rodata_reloc)
+ return NULL;
+
+ return rodata_reloc;
}
--
2.42.0
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH v6 6/9] objtool/LoongArch: Add support for goto table
2024-12-17 1:08 [PATCH v6 0/9] Add jump table support for objtool on LoongArch Tiezhu Yang
` (4 preceding siblings ...)
2024-12-17 1:09 ` [PATCH v6 5/9] objtool/LoongArch: Add support for switch table Tiezhu Yang
@ 2024-12-17 1:09 ` Tiezhu Yang
2024-12-17 1:09 ` [PATCH v6 7/9] LoongArch: Enable jump table for objtool Tiezhu Yang
` (4 subsequent siblings)
10 siblings, 0 replies; 37+ messages in thread
From: Tiezhu Yang @ 2024-12-17 1:09 UTC (permalink / raw)
To: Huacai Chen, Josh Poimboeuf, Peter Zijlstra
Cc: loongarch, amd-gfx, linux-kernel
The objtool program need to analysis the control flow of each object file
generated by compiler toolchain, it needs to know all the locations that
a branch instruction may jump into, if a jump table is used, objtool has
to correlate the jump instruction with the table.
On x86 (which is the only port supported by objtool before LoongArch),
there is a relocation type on the jump instruction and directly points
to the table. But on LoongArch, the relocation is on another kind of
instruction prior to the jump instruction, and also with scheduling it
is not very easy to tell the offset of that instruction from the jump
instruction. Furthermore, because LoongArch has -fsection-anchors (often
enabled at -O1 or above) the relocation may actually points to a section
anchor instead of the table itself.
For the jump table of switch cases, a GCC patch "LoongArch: Add support
to annotate tablejump" and a Clang patch "[LoongArch] Add options for
annotate tablejump" have been merged into the upstream mainline, it can
parse the additional section ".discard.tablejump_annotate" which stores
the jump info as pairs of addresses, each pair contains the address of
jump instruction and the address of jump table.
For the jump table of computed gotos, it is indeed not easy to implement
in the compiler, especially if there is more than one computed goto in a
function such as ___bpf_prog_run(). objdump kernel/bpf/core.o shows that
there are many table jump instructions in ___bpf_prog_run(), but there are
no relocations on the table jump instructions and to the table directly on
LoongArch.
Without the help of compiler, in order to figure out the address of goto
table for the special case of ___bpf_prog_run(), since the instruction
sequence is relatively single and stable, it makes sense to add a helper
find_reloc_of_rodata_c_jump_table() to find the relocation which points
to the section ".rodata..c_jump_table".
If find_reloc_by_table_annotate() failed, it means there is no relocation
info of switch table address in ".rela.discard.tablejump_annotate", then
objtool may find the relocation info of goto table ".rodata..c_jump_table"
with find_reloc_of_rodata_c_jump_table().
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
tools/objtool/arch/loongarch/special.c | 32 ++++++++++++++++++++++++--
1 file changed, 30 insertions(+), 2 deletions(-)
diff --git a/tools/objtool/arch/loongarch/special.c b/tools/objtool/arch/loongarch/special.c
index 4fa6877d5b2b..27c6473608f3 100644
--- a/tools/objtool/arch/loongarch/special.c
+++ b/tools/objtool/arch/loongarch/special.c
@@ -116,6 +116,30 @@ static struct reloc *find_reloc_by_table_annotate(struct objtool_file *file,
return NULL;
}
+static struct reloc *find_reloc_of_rodata_c_jump_table(struct section *sec,
+ unsigned long offset,
+ unsigned long *table_size)
+{
+ struct section *rsec;
+ struct reloc *reloc;
+
+ rsec = sec->rsec;
+ if (!rsec)
+ return NULL;
+
+ for_each_reloc(rsec, reloc) {
+ if (reloc_offset(reloc) > offset)
+ break;
+
+ if (!strncmp(reloc->sym->sec->name, ".rodata..c_jump_table", 21)) {
+ *table_size = 0;
+ return reloc;
+ }
+ }
+
+ return NULL;
+}
+
struct reloc *arch_find_switch_table(struct objtool_file *file,
struct instruction *insn,
unsigned long *table_size)
@@ -126,8 +150,12 @@ struct reloc *arch_find_switch_table(struct objtool_file *file,
unsigned long table_offset;
annotate_reloc = find_reloc_by_table_annotate(file, insn, table_size);
- if (!annotate_reloc)
- return NULL;
+ if (!annotate_reloc) {
+ annotate_reloc = find_reloc_of_rodata_c_jump_table(
+ insn->sec, insn->offset, table_size);
+ if (!annotate_reloc)
+ return NULL;
+ }
table_sec = annotate_reloc->sym->sec;
table_offset = annotate_reloc->sym->offset + reloc_addend(annotate_reloc);
--
2.42.0
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH v6 7/9] LoongArch: Enable jump table for objtool
2024-12-17 1:08 [PATCH v6 0/9] Add jump table support for objtool on LoongArch Tiezhu Yang
` (5 preceding siblings ...)
2024-12-17 1:09 ` [PATCH v6 6/9] objtool/LoongArch: Add support for goto table Tiezhu Yang
@ 2024-12-17 1:09 ` Tiezhu Yang
2025-03-14 20:03 ` [tip: objtool/core] " tip-bot2 for Tiezhu Yang
2024-12-17 1:09 ` [PATCH v6 8/9] LoongArch: Convert unreachable() to BUG() Tiezhu Yang
` (3 subsequent siblings)
10 siblings, 1 reply; 37+ messages in thread
From: Tiezhu Yang @ 2024-12-17 1:09 UTC (permalink / raw)
To: Huacai Chen, Josh Poimboeuf, Peter Zijlstra
Cc: loongarch, amd-gfx, linux-kernel
For now, it is time to remove -fno-jump-tables to enable jump table for
objtool if the compiler has -mannotate-tablejump, otherwise it is better
to remain -fno-jump-tables to keep compatibility with older compilers.
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
arch/loongarch/Kconfig | 3 +++
arch/loongarch/Makefile | 6 +++++-
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
index dae3a9104ca6..66980d847f4e 100644
--- a/arch/loongarch/Kconfig
+++ b/arch/loongarch/Kconfig
@@ -287,6 +287,9 @@ config AS_HAS_LBT_EXTENSION
config AS_HAS_LVZ_EXTENSION
def_bool $(as-instr,hvcl 0)
+config CC_HAS_ANNOTATE_TABLEJUMP
+ def_bool $(cc-option,-mannotate-tablejump)
+
menu "Kernel type and options"
source "kernel/Kconfig.hz"
diff --git a/arch/loongarch/Makefile b/arch/loongarch/Makefile
index 567bd122a9ee..0304eabbe606 100644
--- a/arch/loongarch/Makefile
+++ b/arch/loongarch/Makefile
@@ -101,7 +101,11 @@ KBUILD_AFLAGS += $(call cc-option,-mthin-add-sub) $(call cc-option,-Wa$(comma)
KBUILD_CFLAGS += $(call cc-option,-mthin-add-sub) $(call cc-option,-Wa$(comma)-mthin-add-sub)
ifdef CONFIG_OBJTOOL
-KBUILD_CFLAGS += -fno-jump-tables
+ifdef CONFIG_CC_HAS_ANNOTATE_TABLEJUMP
+KBUILD_CFLAGS += -mannotate-tablejump
+else
+KBUILD_CFLAGS += -fno-jump-tables # keep compatibility with older compilers
+endif
endif
KBUILD_RUSTFLAGS += --target=loongarch64-unknown-none-softfloat -Ccode-model=small
--
2.42.0
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH v6 8/9] LoongArch: Convert unreachable() to BUG()
2024-12-17 1:08 [PATCH v6 0/9] Add jump table support for objtool on LoongArch Tiezhu Yang
` (6 preceding siblings ...)
2024-12-17 1:09 ` [PATCH v6 7/9] LoongArch: Enable jump table for objtool Tiezhu Yang
@ 2024-12-17 1:09 ` Tiezhu Yang
2024-12-17 1:50 ` [PATCH v6 9/9] drm/amd/display: Mark dc_fixpt_from_fraction() noinline Tiezhu Yang
` (2 subsequent siblings)
10 siblings, 0 replies; 37+ messages in thread
From: Tiezhu Yang @ 2024-12-17 1:09 UTC (permalink / raw)
To: Huacai Chen, Josh Poimboeuf, Peter Zijlstra
Cc: loongarch, amd-gfx, linux-kernel
When compiling on LoongArch, there exists the following objtool
warning in arch/loongarch/kernel/machine_kexec.o:
kexec_reboot() falls through to next function crash_shutdown_secondary()
Avoid unreachable() as it can (and will in the absence of UBSAN)
generate fallthrough code. Use BUG() so we get a "break BRK_BUG"
trap (with unreachable annotation).
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
arch/loongarch/kernel/machine_kexec.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/loongarch/kernel/machine_kexec.c b/arch/loongarch/kernel/machine_kexec.c
index 8ae641dc53bb..f9381800e291 100644
--- a/arch/loongarch/kernel/machine_kexec.c
+++ b/arch/loongarch/kernel/machine_kexec.c
@@ -126,14 +126,14 @@ void kexec_reboot(void)
/* All secondary cpus go to kexec_smp_wait */
if (smp_processor_id() > 0) {
relocated_kexec_smp_wait(NULL);
- unreachable();
+ BUG();
}
#endif
do_kexec = (void *)reboot_code_buffer;
do_kexec(efi_boot, cmdline_ptr, systable_ptr, start_addr, first_ind_entry);
- unreachable();
+ BUG();
}
--
2.42.0
^ permalink raw reply related [flat|nested] 37+ messages in thread
* [PATCH v6 9/9] drm/amd/display: Mark dc_fixpt_from_fraction() noinline
2024-12-17 1:08 [PATCH v6 0/9] Add jump table support for objtool on LoongArch Tiezhu Yang
` (7 preceding siblings ...)
2024-12-17 1:09 ` [PATCH v6 8/9] LoongArch: Convert unreachable() to BUG() Tiezhu Yang
@ 2024-12-17 1:50 ` Tiezhu Yang
2024-12-18 14:36 ` Huacai Chen
2025-01-04 13:58 ` [PATCH v6 0/9] Add jump table support for objtool on LoongArch Huacai Chen
2025-01-11 6:57 ` Tiezhu Yang
10 siblings, 1 reply; 37+ messages in thread
From: Tiezhu Yang @ 2024-12-17 1:50 UTC (permalink / raw)
To: Huacai Chen, Josh Poimboeuf, Peter Zijlstra
Cc: loongarch, amd-gfx, linux-kernel
When compiling with Clang on LoongArch, there exists the following objtool
warning in drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.o:
dc_fixpt_recip() falls through to next function dc_fixpt_sinc()
This is because dc_fixpt_from_fraction() is inlined in dc_fixpt_recip()
by Clang, given dc_fixpt_from_fraction() is not a simple function, just
mark it noinline to avoid the above issue.
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c b/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c
index 88d3f9d7dd55..b40c6a21460d 100644
--- a/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c
+++ b/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c
@@ -68,7 +68,7 @@ static inline unsigned long long complete_integer_division_u64(
#define GET_FRACTIONAL_PART(x) \
(FRACTIONAL_PART_MASK & (x))
-struct fixed31_32 dc_fixpt_from_fraction(long long numerator, long long denominator)
+noinline struct fixed31_32 dc_fixpt_from_fraction(long long numerator, long long denominator)
{
struct fixed31_32 res;
--
2.42.0
^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [PATCH v6 9/9] drm/amd/display: Mark dc_fixpt_from_fraction() noinline
2024-12-17 1:50 ` [PATCH v6 9/9] drm/amd/display: Mark dc_fixpt_from_fraction() noinline Tiezhu Yang
@ 2024-12-18 14:36 ` Huacai Chen
2024-12-18 19:05 ` Josh Poimboeuf
0 siblings, 1 reply; 37+ messages in thread
From: Huacai Chen @ 2024-12-18 14:36 UTC (permalink / raw)
To: Tiezhu Yang
Cc: Josh Poimboeuf, Peter Zijlstra, loongarch, amd-gfx, linux-kernel
Hi, Tiezhu,
On Tue, Dec 17, 2024 at 9:50 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>
> When compiling with Clang on LoongArch, there exists the following objtool
> warning in drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.o:
>
> dc_fixpt_recip() falls through to next function dc_fixpt_sinc()
>
> This is because dc_fixpt_from_fraction() is inlined in dc_fixpt_recip()
> by Clang, given dc_fixpt_from_fraction() is not a simple function, just
> mark it noinline to avoid the above issue.
I don't know whether drm maintainers can accept this, because it looks
like a workaround. Yes, uninline this function "solve" a problem and
seems reasonable in this case because the function is "not simple",
but from another point of view, you may hide a type of bug.
Huacai
>
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> ---
> drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c b/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c
> index 88d3f9d7dd55..b40c6a21460d 100644
> --- a/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c
> +++ b/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c
> @@ -68,7 +68,7 @@ static inline unsigned long long complete_integer_division_u64(
> #define GET_FRACTIONAL_PART(x) \
> (FRACTIONAL_PART_MASK & (x))
>
> -struct fixed31_32 dc_fixpt_from_fraction(long long numerator, long long denominator)
> +noinline struct fixed31_32 dc_fixpt_from_fraction(long long numerator, long long denominator)
> {
> struct fixed31_32 res;
>
> --
> 2.42.0
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v6 9/9] drm/amd/display: Mark dc_fixpt_from_fraction() noinline
2024-12-18 14:36 ` Huacai Chen
@ 2024-12-18 19:05 ` Josh Poimboeuf
2024-12-18 19:22 ` Alex Deucher
0 siblings, 1 reply; 37+ messages in thread
From: Josh Poimboeuf @ 2024-12-18 19:05 UTC (permalink / raw)
To: Huacai Chen; +Cc: Tiezhu Yang, Peter Zijlstra, loongarch, amd-gfx, linux-kernel
On Wed, Dec 18, 2024 at 10:36:00PM +0800, Huacai Chen wrote:
> Hi, Tiezhu,
>
> On Tue, Dec 17, 2024 at 9:50 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
> >
> > When compiling with Clang on LoongArch, there exists the following objtool
> > warning in drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.o:
> >
> > dc_fixpt_recip() falls through to next function dc_fixpt_sinc()
> >
> > This is because dc_fixpt_from_fraction() is inlined in dc_fixpt_recip()
> > by Clang, given dc_fixpt_from_fraction() is not a simple function, just
> > mark it noinline to avoid the above issue.
> I don't know whether drm maintainers can accept this, because it looks
> like a workaround. Yes, uninline this function "solve" a problem and
> seems reasonable in this case because the function is "not simple",
> but from another point of view, you may hide a type of bug.
Agreed, it sounds like there's definitely a bug which this patch is
papering over.
--
Josh
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v6 9/9] drm/amd/display: Mark dc_fixpt_from_fraction() noinline
2024-12-18 19:05 ` Josh Poimboeuf
@ 2024-12-18 19:22 ` Alex Deucher
2024-12-20 5:02 ` Tiezhu Yang
0 siblings, 1 reply; 37+ messages in thread
From: Alex Deucher @ 2024-12-18 19:22 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: Huacai Chen, Tiezhu Yang, Peter Zijlstra, loongarch, amd-gfx,
linux-kernel
On Wed, Dec 18, 2024 at 2:18 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> On Wed, Dec 18, 2024 at 10:36:00PM +0800, Huacai Chen wrote:
> > Hi, Tiezhu,
> >
> > On Tue, Dec 17, 2024 at 9:50 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
> > >
> > > When compiling with Clang on LoongArch, there exists the following objtool
> > > warning in drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.o:
> > >
> > > dc_fixpt_recip() falls through to next function dc_fixpt_sinc()
> > >
> > > This is because dc_fixpt_from_fraction() is inlined in dc_fixpt_recip()
> > > by Clang, given dc_fixpt_from_fraction() is not a simple function, just
> > > mark it noinline to avoid the above issue.
> > I don't know whether drm maintainers can accept this, because it looks
> > like a workaround. Yes, uninline this function "solve" a problem and
> > seems reasonable in this case because the function is "not simple",
> > but from another point of view, you may hide a type of bug.
>
> Agreed, it sounds like there's definitely a bug which this patch is
> papering over.
Yes, agreed.
Alex
>
> --
> Josh
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v6 9/9] drm/amd/display: Mark dc_fixpt_from_fraction() noinline
2024-12-18 19:22 ` Alex Deucher
@ 2024-12-20 5:02 ` Tiezhu Yang
2024-12-20 10:31 ` Peter Zijlstra
0 siblings, 1 reply; 37+ messages in thread
From: Tiezhu Yang @ 2024-12-20 5:02 UTC (permalink / raw)
To: Alex Deucher, Josh Poimboeuf
Cc: Huacai Chen, Peter Zijlstra, loongarch, amd-gfx, linux-kernel
On 12/19/2024 03:22 AM, Alex Deucher wrote:
> On Wed, Dec 18, 2024 at 2:18 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>>
>> On Wed, Dec 18, 2024 at 10:36:00PM +0800, Huacai Chen wrote:
>>> Hi, Tiezhu,
>>>
>>> On Tue, Dec 17, 2024 at 9:50 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>>>>
>>>> When compiling with Clang on LoongArch, there exists the following objtool
>>>> warning in drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.o:
>>>>
>>>> dc_fixpt_recip() falls through to next function dc_fixpt_sinc()
>>>>
>>>> This is because dc_fixpt_from_fraction() is inlined in dc_fixpt_recip()
>>>> by Clang, given dc_fixpt_from_fraction() is not a simple function, just
>>>> mark it noinline to avoid the above issue.
>>> I don't know whether drm maintainers can accept this, because it looks
>>> like a workaround. Yes, uninline this function "solve" a problem and
>>> seems reasonable in this case because the function is "not simple",
>>> but from another point of view, you may hide a type of bug.
>>
>> Agreed, it sounds like there's definitely a bug which this patch is
>> papering over.
>
> Yes, agreed.
Additional Info:
In order to avoid the effect of this series, I tested with kernel
6.13-rc3 without this series again.
--------------------------------------------------------------------------
Here are the test result.
1. For LoongArch
"dc_fixpt_recip() falls through to next function dc_fixpt_sinc()"
objtool warning only appears compiled with the latest mainline LLVM,
there is no this issue compiled with the latest release version such
as LLVM 19.1.6.
(1) objdump info with LLVM release version 19.1.6:
$ clang --version | head -1
clang version 19.1.6
There is an jump instruction "b" at the end of dc_fixpt_recip(), it
maybe jumps to a position and then steps to the return instruction, so
there is no "falls through" objtool warning.
0000000000000200 <dc_fixpt_recip>:
200: 40009480 beqz $a0, 148 # 294
<dc_fixpt_recip+0x94>
204: 0049fc85 srai.d $a1, $a0, 0x3f
208: 00159486 xor $a2, $a0, $a1
20c: 001194c5 sub.d $a1, $a2, $a1
210: 03800007 ori $a3, $zero, 0x0
214: 16000027 lu32i.d $a3, 1
218: 002314e6 div.du $a2, $a3, $a1
21c: 001d94c8 mul.d $a4, $a2, $a1
220: 0011a0e9 sub.d $a5, $a3, $a4
224: 02bf8008 addi.w $a4, $zero, -32
228: 03400000 andi $zero, $zero, 0x0
22c: 03400000 andi $zero, $zero, 0x0
230: 00410529 slli.d $a5, $a5, 0x1
234: 004104c6 slli.d $a2, $a2, 0x1
238: 0012952a sltu $a6, $a5, $a1
23c: 03c0054a xori $a6, $a6, 0x1
240: 001328ab maskeqz $a7, $a1, $a6
244: 0011ad29 sub.d $a5, $a5, $a7
248: 00df0108 bstrpick.d $a4, $a4, 0x1f, 0x0
24c: 02c00508 addi.d $a4, $a4, 1
250: 00149d0b and $a7, $a4, $a3
254: 001528c6 or $a2, $a2, $a6
258: 43ffd97f beqz $a7, -40 # 230
<dc_fixpt_recip+0x30>
25c: 00410527 slli.d $a3, $a5, 0x1
260: 001294e5 sltu $a1, $a3, $a1
264: 03c004a5 xori $a1, $a1, 0x1
268: 02bffc07 addi.w $a3, $zero, -1
26c: 031ffce7 lu52i.d $a3, $a3, 2047
270: 00159ca7 xor $a3, $a1, $a3
274: 680030e6 bltu $a3, $a2, 48 # 2a4
<dc_fixpt_recip+0xa4>
278: 001094c5 add.d $a1, $a2, $a1
27c: 00119406 sub.d $a2, $zero, $a1
280: 02000084 slti $a0, $a0, 0
284: 001390a5 masknez $a1, $a1, $a0
288: 001310c4 maskeqz $a0, $a2, $a0
28c: 00151484 or $a0, $a0, $a1
290: 4c000020 jirl $zero, $ra, 0
294: 00150005 or $a1, $zero, $zero
298: 002a0001 break 0x1
29c: 002a0001 break 0x1
2a0: 53ff73ff b -144 # 210
<dc_fixpt_recip+0x10>
2a4: 002a0001 break 0x1
2a8: 53ffd3ff b -48 # 278
<dc_fixpt_recip+0x78>
2ac: 03400000 andi $zero, $zero, 0x0
2b0: 03400000 andi $zero, $zero, 0x0
2b4: 03400000 andi $zero, $zero, 0x0
2b8: 03400000 andi $zero, $zero, 0x0
2bc: 03400000 andi $zero, $zero, 0x0
00000000000002c0 <dc_fixpt_sinc>:
2c0: 0049fc85 srai.d $a1, $a0, 0x3f
2c4: 00159486 xor $a2, $a0, $a1
2c8: 1490fda7 lu12i.w $a3, 296941
2cc: 039444e7 ori $a3, $a3, 0x511
2d0: 001194c6 sub.d $a2, $a2, $a1
2d4: 001500e5 or $a1, $a3, $zero
2d8: 160000c5 lu32i.d $a1, 6
2dc: 001500c9 or $a5, $a2, $zero
2e0: 00150088 or $a4, $a0, $zero
2e4: 6000a8c5 blt $a2, $a1, 168 # 38c
<dc_fixpt_sinc+0xcc>
(2) objdump info with LLVM mainline version 20.0.0git:
$ clang --version | head -1
clang version 20.0.0git (https://github.com/llvm/llvm-project.git
8daf4f16fa08b5d876e98108721dd1743a360326)
There is "break" instruction at the end of dc_fixpt_recip(), its offset
is "2a0", this "break" instruction is not dead end due to the offset
"2a4" is in the relocation section '.rela.discard.reachable', that is
to say, dc_fixpt_recip() doesn't end with a return instruction or an
unconditional jump, so objtool determined that the function can fall
through into the next function, thus there is "falls through" objtool
warning.
0000000000000200 <dc_fixpt_recip>:
200: 40009c80 beqz $a0, 156 # 29c
<dc_fixpt_recip+0x9c>
204: 0049fc85 srai.d $a1, $a0, 0x3f
208: 00159486 xor $a2, $a0, $a1
20c: 001194c5 sub.d $a1, $a2, $a1
210: 03800007 ori $a3, $zero, 0x0
214: 16000027 lu32i.d $a3, 1
218: 002314e6 div.du $a2, $a3, $a1
21c: 001d94c8 mul.d $a4, $a2, $a1
220: 0011a0e9 sub.d $a5, $a3, $a4
224: 02bf8008 addi.w $a4, $zero, -32
228: 03400000 andi $zero, $zero, 0x0
22c: 03400000 andi $zero, $zero, 0x0
230: 00410529 slli.d $a5, $a5, 0x1
234: 004104c6 slli.d $a2, $a2, 0x1
238: 0012952a sltu $a6, $a5, $a1
23c: 03c0054a xori $a6, $a6, 0x1
240: 001328ab maskeqz $a7, $a1, $a6
244: 0011ad29 sub.d $a5, $a5, $a7
248: 00df0108 bstrpick.d $a4, $a4, 0x1f, 0x0
24c: 02c00508 addi.d $a4, $a4, 1
250: 00149d0b and $a7, $a4, $a3
254: 001528c6 or $a2, $a2, $a6
258: 43ffd97f beqz $a7, -40 # 230
<dc_fixpt_recip+0x30>
25c: 00410527 slli.d $a3, $a5, 0x1
260: 001294e5 sltu $a1, $a3, $a1
264: 03c004a5 xori $a1, $a1, 0x1
268: 02bffc07 addi.w $a3, $zero, -1
26c: 031ffce7 lu52i.d $a3, $a3, 2047
270: 00159ca7 xor $a3, $a1, $a3
274: 680020e6 bltu $a3, $a2, 32 # 294
<dc_fixpt_recip+0x94>
278: 001094c5 add.d $a1, $a2, $a1
27c: 00119406 sub.d $a2, $zero, $a1
280: 02000084 slti $a0, $a0, 0
284: 001390a5 masknez $a1, $a1, $a0
288: 001310c4 maskeqz $a0, $a2, $a0
28c: 00151484 or $a0, $a0, $a1
290: 4c000020 jirl $zero, $ra, 0
294: 002a0001 break 0x1
298: 53ffe3ff b -32 # 278
<dc_fixpt_recip+0x78>
29c: 002a0001 break 0x1
2a0: 002a0001 break 0x1
2a4: 03400000 andi $zero, $zero, 0x0
2a8: 03400000 andi $zero, $zero, 0x0
2ac: 03400000 andi $zero, $zero, 0x0
2b0: 03400000 andi $zero, $zero, 0x0
2b4: 03400000 andi $zero, $zero, 0x0
2b8: 03400000 andi $zero, $zero, 0x0
2bc: 03400000 andi $zero, $zero, 0x0
00000000000002c0 <dc_fixpt_sinc>:
2c0: 0049fc85 srai.d $a1, $a0, 0x3f
2c4: 00159486 xor $a2, $a0, $a1
2c8: 001194c6 sub.d $a2, $a2, $a1
2cc: 1490fda5 lu12i.w $a1, 296941
2d0: 039444a5 ori $a1, $a1, 0x511
2d4: 160000c5 lu32i.d $a1, 6
2d8: 001500c7 or $a3, $a2, $zero
2dc: 00150088 or $a4, $a0, $zero
2e0: 600090c5 blt $a2, $a1, 144 # 370
<dc_fixpt_sinc+0xb0>
2. For x86
I tested with LLVM 19.1.6 and the latest mainline LLVM, the test result
is same with LoongArch.
(1) objdump info with LLVM release version 19.1.6:
$ clang --version | head -1
clang version 19.1.6
There is an jump instruction "jmp" at the end of dc_fixpt_recip(), it
maybe jumps to a position and then steps to the return instruction, so
there is no "falls through" objtool warning.
0000000000000290 <dc_fixpt_recip>:
290: f3 0f 1e fa endbr64
294: e8 00 00 00 00 call 299 <dc_fixpt_recip+0x9>
299: 48 85 ff test %rdi,%rdi
29c: 0f 84 a8 00 00 00 je 34a <dc_fixpt_recip+0xba>
2a2: 48 89 f9 mov %rdi,%rcx
2a5: 48 f7 d9 neg %rcx
2a8: 48 0f 48 cf cmovs %rdi,%rcx
2ac: 53 push %rbx
2ad: 48 b8 00 00 00 00 01 movabs $0x100000000,%rax
2b4: 00 00 00
2b7: 31 d2 xor %edx,%edx
2b9: 48 f7 f1 div %rcx
2bc: be e0 ff ff ff mov $0xffffffe0,%esi
2c1: eb 1b jmp 2de <dc_fixpt_recip+0x4e>
2c3: 45 88 c8 mov %r9b,%r8b
2c6: 4d 01 c0 add %r8,%r8
2c9: 4d 8d 04 80 lea (%r8,%rax,4),%r8
2cd: 48 29 da sub %rbx,%rdx
2d0: 45 88 da mov %r11b,%r10b
2d3: 4c 89 d0 mov %r10,%rax
2d6: 4c 09 c0 or %r8,%rax
2d9: 83 c6 02 add $0x2,%esi
2dc: 74 31 je 30f <dc_fixpt_recip+0x7f>
2de: 48 01 d2 add %rdx,%rdx
2e1: 45 31 c0 xor %r8d,%r8d
2e4: 41 ba 00 00 00 00 mov $0x0,%r10d
2ea: 48 39 ca cmp %rcx,%rdx
2ed: 41 0f 93 c1 setae %r9b
2f1: 72 03 jb 2f6 <dc_fixpt_recip+0x66>
2f3: 49 89 ca mov %rcx,%r10
2f6: 4c 29 d2 sub %r10,%rdx
2f9: 48 01 d2 add %rdx,%rdx
2fc: 45 31 d2 xor %r10d,%r10d
2ff: 48 89 cb mov %rcx,%rbx
302: 48 39 ca cmp %rcx,%rdx
305: 41 0f 93 c3 setae %r11b
309: 73 b8 jae 2c3 <dc_fixpt_recip+0x33>
30b: 31 db xor %ebx,%ebx
30d: eb b4 jmp 2c3 <dc_fixpt_recip+0x33>
30f: 48 01 d2 add %rdx,%rdx
312: 48 be fe ff ff ff ff movabs $0x7ffffffffffffffe,%rsi
319: ff ff 7f
31c: 4c 8d 46 01 lea 0x1(%rsi),%r8
320: 48 39 ca cmp %rcx,%rdx
323: 4c 0f 43 c6 cmovae %rsi,%r8
327: 4c 39 c0 cmp %r8,%rax
32a: 77 29 ja 355 <dc_fixpt_recip+0xc5>
32c: 48 39 ca cmp %rcx,%rdx
32f: 48 83 d8 ff sbb $0xffffffffffffffff,%rax
333: 48 89 c1 mov %rax,%rcx
336: 48 f7 d9 neg %rcx
339: 48 85 ff test %rdi,%rdi
33c: 48 0f 49 c8 cmovns %rax,%rcx
340: 48 89 c8 mov %rcx,%rax
343: 5b pop %rbx
344: 2e e9 00 00 00 00 cs jmp 34a <dc_fixpt_recip+0xba>
34a: 0f 0b ud2
34c: 0f 0b ud2
34e: 31 c9 xor %ecx,%ecx
350: e9 57 ff ff ff jmp 2ac <dc_fixpt_recip+0x1c>
355: 0f 0b ud2
357: eb d3 jmp 32c <dc_fixpt_recip+0x9c>
359: 0f 1f 80 00 00 00 00 nopl 0x0(%rax)
0000000000000360 <.Ltmp40>:
360: 90 nop
361: 90 nop
362: 90 nop
363: 90 nop
364: 90 nop
365: 90 nop
366: 90 nop
367: 90 nop
368: 90 nop
369: 90 nop
36a: 90 nop
36b: 90 nop
36c: 90 nop
36d: 90 nop
36e: 90 nop
36f: 90 nop
0000000000000370 <dc_fixpt_sinc>:
370: f3 0f 1e fa endbr64
374: e8 00 00 00 00 call 379 <dc_fixpt_sinc+0x9>
(2) objdump info with LLVM mainline version 20.0.0git:
$ clang --version | head -1
clang version 20.0.0git (https://github.com/llvm/llvm-project.git
8daf4f16fa08b5d876e98108721dd1743a360326)
There is "ud2" instruction at the end of dc_fixpt_recip(), its offset
is "350", this "ud2" instruction is not dead end due to the offset "352"
is in the relocation section '.rela.discard.reachable', that is to say,
dc_fixpt_recip() doesn't end with a return instruction or an
unconditional jump, so objtool determined that the function can fall
through into the next function, thus there is "falls through" objtool
warning.
0000000000000290 <dc_fixpt_recip>:
290: f3 0f 1e fa endbr64
294: e8 00 00 00 00 call 299 <dc_fixpt_recip+0x9>
299: 48 85 ff test %rdi,%rdi
29c: 0f 84 ac 00 00 00 je 34e <dc_fixpt_recip+0xbe>
2a2: 53 push %rbx
2a3: 48 89 f9 mov %rdi,%rcx
2a6: 48 f7 d9 neg %rcx
2a9: 48 0f 48 cf cmovs %rdi,%rcx
2ad: 48 b8 00 00 00 00 01 movabs $0x100000000,%rax
2b4: 00 00 00
2b7: 31 d2 xor %edx,%edx
2b9: 48 f7 f1 div %rcx
2bc: be e0 ff ff ff mov $0xffffffe0,%esi
2c1: eb 1b jmp 2de <dc_fixpt_recip+0x4e>
2c3: 45 88 c8 mov %r9b,%r8b
2c6: 4d 01 c0 add %r8,%r8
2c9: 4d 8d 04 80 lea (%r8,%rax,4),%r8
2cd: 48 29 da sub %rbx,%rdx
2d0: 45 88 da mov %r11b,%r10b
2d3: 4c 89 d0 mov %r10,%rax
2d6: 4c 09 c0 or %r8,%rax
2d9: 83 c6 02 add $0x2,%esi
2dc: 74 31 je 30f <dc_fixpt_recip+0x7f>
2de: 48 01 d2 add %rdx,%rdx
2e1: 45 31 c0 xor %r8d,%r8d
2e4: 41 ba 00 00 00 00 mov $0x0,%r10d
2ea: 48 39 ca cmp %rcx,%rdx
2ed: 41 0f 93 c1 setae %r9b
2f1: 72 03 jb 2f6 <dc_fixpt_recip+0x66>
2f3: 49 89 ca mov %rcx,%r10
2f6: 4c 29 d2 sub %r10,%rdx
2f9: 48 01 d2 add %rdx,%rdx
2fc: 45 31 d2 xor %r10d,%r10d
2ff: 48 89 cb mov %rcx,%rbx
302: 48 39 ca cmp %rcx,%rdx
305: 41 0f 93 c3 setae %r11b
309: 73 b8 jae 2c3 <dc_fixpt_recip+0x33>
30b: 31 db xor %ebx,%ebx
30d: eb b4 jmp 2c3 <dc_fixpt_recip+0x33>
30f: 48 01 d2 add %rdx,%rdx
312: 48 be fe ff ff ff ff movabs $0x7ffffffffffffffe,%rsi
319: ff ff 7f
31c: 4c 8d 46 01 lea 0x1(%rsi),%r8
320: 48 39 ca cmp %rcx,%rdx
323: 4c 0f 43 c6 cmovae %rsi,%r8
327: 4c 39 c0 cmp %r8,%rax
32a: 77 1e ja 34a <dc_fixpt_recip+0xba>
32c: 48 39 ca cmp %rcx,%rdx
32f: 48 83 d8 ff sbb $0xffffffffffffffff,%rax
333: 48 89 c1 mov %rax,%rcx
336: 48 f7 d9 neg %rcx
339: 48 85 ff test %rdi,%rdi
33c: 48 0f 49 c8 cmovns %rax,%rcx
340: 48 89 c8 mov %rcx,%rax
343: 5b pop %rbx
344: 2e e9 00 00 00 00 cs jmp 34a <dc_fixpt_recip+0xba>
34a: 0f 0b ud2
34c: eb de jmp 32c <dc_fixpt_recip+0x9c>
34e: 0f 0b ud2
350: 0f 0b ud2
352: 66 66 66 66 66 2e 0f data16 data16 data16 data16 cs
nopw 0x0(%rax,%rax,1)
359: 1f 84 00 00 00 00 00
0000000000000360 <.Ltmp40>:
360: 90 nop
361: 90 nop
362: 90 nop
363: 90 nop
364: 90 nop
365: 90 nop
366: 90 nop
367: 90 nop
368: 90 nop
369: 90 nop
36a: 90 nop
36b: 90 nop
36c: 90 nop
36d: 90 nop
36e: 90 nop
36f: 90 nop
0000000000000370 <dc_fixpt_sinc>:
370: f3 0f 1e fa endbr64
374: e8 00 00 00 00 call 379 <dc_fixpt_sinc+0x9>
--------------------------------------------------------------------------
In my opinion, if there is a bug, then it is a generic rather than
arch-specific bug.
The root cause is related with LLVM or Linux kernel? I am not sure
because objtool only checks the object info and reports the warning
"falls through" if the check conditions are true.
tools/objtool/check.c
static int validate_branch(struct objtool_file *file, struct symbol *func,
struct instruction *insn, struct insn_state
state)
{
struct alternative *alt;
struct instruction *next_insn, *prev_insn = NULL;
struct section *sec;
u8 visited;
int ret;
sec = insn->sec;
while (1) {
next_insn = next_insn_to_validate(file, insn);
if (func && insn_func(insn) && func !=
insn_func(insn)->pfunc) {
/* Ignore KCFI type preambles, which always
fall through */
if (!strncmp(func->name, "__cfi_", 6) ||
!strncmp(func->name, "__pfx_", 6))
return 0;
WARN("%s() falls through to next function %s()",
func->name, insn_func(insn)->name);
return 1;
}
...
}
How to silence the warning when compiling with the latest mainline
LLVM with a proper way? Modify LLVM code or tools/objtool/check.c
or drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c?
Anyway, I think this patch can be independent of this series and can
be sent separately after the "real bug" is fixed, please ignore it now.
Thanks,
Tiezhu
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v6 9/9] drm/amd/display: Mark dc_fixpt_from_fraction() noinline
2024-12-20 5:02 ` Tiezhu Yang
@ 2024-12-20 10:31 ` Peter Zijlstra
2024-12-20 22:34 ` Nathan Chancellor
0 siblings, 1 reply; 37+ messages in thread
From: Peter Zijlstra @ 2024-12-20 10:31 UTC (permalink / raw)
To: Tiezhu Yang
Cc: Alex Deucher, Josh Poimboeuf, Huacai Chen, loongarch, amd-gfx,
linux-kernel, Nick Desaulniers, nathan, llvm
On Fri, Dec 20, 2024 at 01:02:18PM +0800, Tiezhu Yang wrote:
> 2. For x86
>
> I tested with LLVM 19.1.6 and the latest mainline LLVM, the test result
> is same with LoongArch.
Debian's clang-19 is 19.1.5.
> (1) objdump info with LLVM release version 19.1.6:
Please always use -r, that's ever so much more readable.
> $ clang --version | head -1
> clang version 19.1.6
>
> There is an jump instruction "jmp" at the end of dc_fixpt_recip(), it
> maybe jumps to a position and then steps to the return instruction, so
> there is no "falls through" objtool warning.
>
> 0000000000000290 <dc_fixpt_recip>:
> 290: f3 0f 1e fa endbr64
> 294: e8 00 00 00 00 call 299 <dc_fixpt_recip+0x9>
> 299: 48 85 ff test %rdi,%rdi
> 29c: 0f 84 a8 00 00 00 je 34a <dc_fixpt_recip+0xba>
> 2a2: 48 89 f9 mov %rdi,%rcx
> 2a5: 48 f7 d9 neg %rcx
> 2a8: 48 0f 48 cf cmovs %rdi,%rcx
> 2ac: 53 push %rbx
> 2ad: 48 b8 00 00 00 00 01 movabs $0x100000000,%rax
> 2b4: 00 00 00
> 2b7: 31 d2 xor %edx,%edx
> 2b9: 48 f7 f1 div %rcx
> 2bc: be e0 ff ff ff mov $0xffffffe0,%esi
> 2c1: eb 1b jmp 2de <dc_fixpt_recip+0x4e>
> 2c3: 45 88 c8 mov %r9b,%r8b
> 2c6: 4d 01 c0 add %r8,%r8
> 2c9: 4d 8d 04 80 lea (%r8,%rax,4),%r8
> 2cd: 48 29 da sub %rbx,%rdx
> 2d0: 45 88 da mov %r11b,%r10b
> 2d3: 4c 89 d0 mov %r10,%rax
> 2d6: 4c 09 c0 or %r8,%rax
> 2d9: 83 c6 02 add $0x2,%esi
> 2dc: 74 31 je 30f <dc_fixpt_recip+0x7f>
> 2de: 48 01 d2 add %rdx,%rdx
> 2e1: 45 31 c0 xor %r8d,%r8d
> 2e4: 41 ba 00 00 00 00 mov $0x0,%r10d
> 2ea: 48 39 ca cmp %rcx,%rdx
> 2ed: 41 0f 93 c1 setae %r9b
> 2f1: 72 03 jb 2f6 <dc_fixpt_recip+0x66>
> 2f3: 49 89 ca mov %rcx,%r10
> 2f6: 4c 29 d2 sub %r10,%rdx
> 2f9: 48 01 d2 add %rdx,%rdx
> 2fc: 45 31 d2 xor %r10d,%r10d
> 2ff: 48 89 cb mov %rcx,%rbx
> 302: 48 39 ca cmp %rcx,%rdx
> 305: 41 0f 93 c3 setae %r11b
> 309: 73 b8 jae 2c3 <dc_fixpt_recip+0x33>
> 30b: 31 db xor %ebx,%ebx
> 30d: eb b4 jmp 2c3 <dc_fixpt_recip+0x33>
> 30f: 48 01 d2 add %rdx,%rdx
> 312: 48 be fe ff ff ff ff movabs $0x7ffffffffffffffe,%rsi
> 319: ff ff 7f
> 31c: 4c 8d 46 01 lea 0x1(%rsi),%r8
> 320: 48 39 ca cmp %rcx,%rdx
> 323: 4c 0f 43 c6 cmovae %rsi,%r8
> 327: 4c 39 c0 cmp %r8,%rax
> 32a: 77 29 ja 355 <dc_fixpt_recip+0xc5>
> 32c: 48 39 ca cmp %rcx,%rdx
> 32f: 48 83 d8 ff sbb $0xffffffffffffffff,%rax
> 333: 48 89 c1 mov %rax,%rcx
> 336: 48 f7 d9 neg %rcx
> 339: 48 85 ff test %rdi,%rdi
> 33c: 48 0f 49 c8 cmovns %rax,%rcx
> 340: 48 89 c8 mov %rcx,%rax
> 343: 5b pop %rbx
> 344: 2e e9 00 00 00 00 cs jmp 34a <dc_fixpt_recip+0xba>
> 34a: 0f 0b ud2
> 34c: 0f 0b ud2
> 34e: 31 c9 xor %ecx,%ecx
> 350: e9 57 ff ff ff jmp 2ac <dc_fixpt_recip+0x1c>
> 355: 0f 0b ud2
> 357: eb d3 jmp 32c <dc_fixpt_recip+0x9c>
> 359: 0f 1f 80 00 00 00 00 nopl 0x0(%rax)
I had to puzzle a bit to get that double ud2 -- not all configs do that.
Also, curse the DRM Makefiles, you can't do:
make drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.s
:-(
They are the two ASSERT()s on line 217 and 54 respectively. They end up
asserting the same value twice, so that makes sense.
> $ clang --version | head -1
> clang version 20.0.0git (https://github.com/llvm/llvm-project.git
> 8daf4f16fa08b5d876e98108721dd1743a360326)
So I didn't have a recent build at hand.. so I've not validated the
below.
> There is "ud2" instruction at the end of dc_fixpt_recip(), its offset
> is "350", this "ud2" instruction is not dead end due to the offset "352"
> is in the relocation section '.rela.discard.reachable', that is to say,
> dc_fixpt_recip() doesn't end with a return instruction or an
> unconditional jump, so objtool determined that the function can fall
> through into the next function, thus there is "falls through" objtool
> warning.
>
> 0000000000000290 <dc_fixpt_recip>:
> 290: f3 0f 1e fa endbr64
> 294: e8 00 00 00 00 call 299 <dc_fixpt_recip+0x9>
> 299: 48 85 ff test %rdi,%rdi
> 29c: 0f 84 ac 00 00 00 je 34e <dc_fixpt_recip+0xbe>
> 2a2: 53 push %rbx
> 2a3: 48 89 f9 mov %rdi,%rcx
> 2a6: 48 f7 d9 neg %rcx
> 2a9: 48 0f 48 cf cmovs %rdi,%rcx
> 2ad: 48 b8 00 00 00 00 01 movabs $0x100000000,%rax
> 2b4: 00 00 00
> 2b7: 31 d2 xor %edx,%edx
> 2b9: 48 f7 f1 div %rcx
> 2bc: be e0 ff ff ff mov $0xffffffe0,%esi
> 2c1: eb 1b jmp 2de <dc_fixpt_recip+0x4e>
> 2c3: 45 88 c8 mov %r9b,%r8b
> 2c6: 4d 01 c0 add %r8,%r8
> 2c9: 4d 8d 04 80 lea (%r8,%rax,4),%r8
> 2cd: 48 29 da sub %rbx,%rdx
> 2d0: 45 88 da mov %r11b,%r10b
> 2d3: 4c 89 d0 mov %r10,%rax
> 2d6: 4c 09 c0 or %r8,%rax
> 2d9: 83 c6 02 add $0x2,%esi
> 2dc: 74 31 je 30f <dc_fixpt_recip+0x7f>
> 2de: 48 01 d2 add %rdx,%rdx
> 2e1: 45 31 c0 xor %r8d,%r8d
> 2e4: 41 ba 00 00 00 00 mov $0x0,%r10d
> 2ea: 48 39 ca cmp %rcx,%rdx
> 2ed: 41 0f 93 c1 setae %r9b
> 2f1: 72 03 jb 2f6 <dc_fixpt_recip+0x66>
> 2f3: 49 89 ca mov %rcx,%r10
> 2f6: 4c 29 d2 sub %r10,%rdx
> 2f9: 48 01 d2 add %rdx,%rdx
> 2fc: 45 31 d2 xor %r10d,%r10d
> 2ff: 48 89 cb mov %rcx,%rbx
> 302: 48 39 ca cmp %rcx,%rdx
> 305: 41 0f 93 c3 setae %r11b
> 309: 73 b8 jae 2c3 <dc_fixpt_recip+0x33>
> 30b: 31 db xor %ebx,%ebx
> 30d: eb b4 jmp 2c3 <dc_fixpt_recip+0x33>
> 30f: 48 01 d2 add %rdx,%rdx
> 312: 48 be fe ff ff ff ff movabs $0x7ffffffffffffffe,%rsi
> 319: ff ff 7f
> 31c: 4c 8d 46 01 lea 0x1(%rsi),%r8
> 320: 48 39 ca cmp %rcx,%rdx
> 323: 4c 0f 43 c6 cmovae %rsi,%r8
> 327: 4c 39 c0 cmp %r8,%rax
> 32a: 77 1e ja 34a <dc_fixpt_recip+0xba>
> 32c: 48 39 ca cmp %rcx,%rdx
> 32f: 48 83 d8 ff sbb $0xffffffffffffffff,%rax
> 333: 48 89 c1 mov %rax,%rcx
> 336: 48 f7 d9 neg %rcx
> 339: 48 85 ff test %rdi,%rdi
> 33c: 48 0f 49 c8 cmovns %rax,%rcx
> 340: 48 89 c8 mov %rcx,%rax
> 343: 5b pop %rbx
> 344: 2e e9 00 00 00 00 cs jmp 34a <dc_fixpt_recip+0xba>
> 34a: 0f 0b ud2
> 34c: eb de jmp 32c <dc_fixpt_recip+0x9c>
> 34e: 0f 0b ud2
> 350: 0f 0b ud2
> 352: 66 66 66 66 66 2e 0f data16 data16 data16 data16 cs nopw
> 0x0(%rax,%rax,1)
> 359: 1f 84 00 00 00 00 00
If you put them size-by-side, you'll see it's more or less the same
code-gen (trivial differences), but now it just stops code-gen, where
previously it would continue.
So this really is a compiler problem, this needs no annotation, it's
straight up broken.
Now, the thing is, these ASSERT()s are checking for divide-by-zero, I
suspect clang figured that out and invokes UB on us and just stops
code-gen.
Nathan, Nick, don't we have a compiler flag that forces __builtin_trap()
whenever clang pulls something like this? I think UBSAN does this, but
we really shouldn't pull in the whole of that for sanity.
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v6 9/9] drm/amd/display: Mark dc_fixpt_from_fraction() noinline
2024-12-20 10:31 ` Peter Zijlstra
@ 2024-12-20 22:34 ` Nathan Chancellor
2024-12-21 7:40 ` Xi Ruoyao
0 siblings, 1 reply; 37+ messages in thread
From: Nathan Chancellor @ 2024-12-20 22:34 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Tiezhu Yang, Alex Deucher, Josh Poimboeuf, Huacai Chen, loongarch,
amd-gfx, linux-kernel, Nick Desaulniers, llvm
On Fri, Dec 20, 2024 at 11:31:00AM +0100, Peter Zijlstra wrote:
> Also, curse the DRM Makefiles, you can't do:
>
> make drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.s
Small tip: You can get the path of the target by building
drivers/gpu/drm/amd/amdgpu/ and finding it in the output. In this case,
it'd be
$ make drivers/gpu/drm/amd/amdgpu/../display/dc/basics/fixpt31_32.s
Not excusing that it does not work as it should but sometimes you have
to work with what you can *shrug*
> > $ clang --version | head -1
> > clang version 20.0.0git (https://github.com/llvm/llvm-project.git
> > 8daf4f16fa08b5d876e98108721dd1743a360326)
>
> So I didn't have a recent build at hand.. so I've not validated the
> below.
...
> If you put them size-by-side, you'll see it's more or less the same
> code-gen (trivial differences), but now it just stops code-gen, where
> previously it would continue.
>
> So this really is a compiler problem, this needs no annotation, it's
> straight up broken.
>
> Now, the thing is, these ASSERT()s are checking for divide-by-zero, I
> suspect clang figured that out and invokes UB on us and just stops
> code-gen.
Yeah, I think your analysis is spot on, as this was introduced by a
change in clang from a few months ago according to my bisect:
https://github.com/llvm/llvm-project/commit/37932643abab699e8bb1def08b7eb4eae7ff1448
Since the ASSERT does not do anything to prevent the divide by zero (it
just flags it with WARN_ON) and the rest of the code doesn't either, I
assume that the codegen stops as soon as it encounters the unreachable
that change created from the path where divide by zero would occur via
dc_fixpt_recip() ->
dc_fixpt_from_fraction() ->
complete_integer_division_u64() ->
div64_u64_rem()
Shouldn't callers of division functions harden them against dividing by
zero?
> Nathan, Nick, don't we have a compiler flag that forces __builtin_trap()
> whenever clang pulls something like this? I think UBSAN does this, but
> we really shouldn't pull in the whole of that for sanity.
Right, I think that LLVM has a hidden flag for this:
-mllvm -trap-unreachable
That makes this particular warning disappear.
It isn't the greatest because '-mllvm' flags need to be passed along to
the linker for LTO but that's easy enough to deal with. I know we have
talked about enabling that flag in the past but I cannot remember why we
decided against it (maybe code size concerns and other optimization
restrictions)? It looks like GCC has a similar flag,
-funreachable-traps.
Cheers,
Nathan
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v6 9/9] drm/amd/display: Mark dc_fixpt_from_fraction() noinline
2024-12-20 22:34 ` Nathan Chancellor
@ 2024-12-21 7:40 ` Xi Ruoyao
2024-12-22 4:27 ` Tiezhu Yang
0 siblings, 1 reply; 37+ messages in thread
From: Xi Ruoyao @ 2024-12-21 7:40 UTC (permalink / raw)
To: Nathan Chancellor, Peter Zijlstra
Cc: Tiezhu Yang, Alex Deucher, Josh Poimboeuf, Huacai Chen, loongarch,
amd-gfx, linux-kernel, Nick Desaulniers, llvm
On Fri, 2024-12-20 at 15:34 -0700, Nathan Chancellor wrote:
> > Now, the thing is, these ASSERT()s are checking for divide-by-zero, I
> > suspect clang figured that out and invokes UB on us and just stops
> > code-gen.
>
> Yeah, I think your analysis is spot on, as this was introduced by a
> change in clang from a few months ago according to my bisect:
>
> https://github.com/llvm/llvm-project/commit/37932643abab699e8bb1def08b7eb4eae7ff1448
>
> Since the ASSERT does not do anything to prevent the divide by zero (it
> just flags it with WARN_ON) and the rest of the code doesn't either, I
> assume that the codegen stops as soon as it encounters the unreachable
> that change created from the path where divide by zero would occur via
>
> dc_fixpt_recip() ->
> dc_fixpt_from_fraction() ->
> complete_integer_division_u64() ->
> div64_u64_rem()
>
> Shouldn't callers of division functions harden them against dividing by
> zero?
Yes I think it'd be the correct solution.
--
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v6 9/9] drm/amd/display: Mark dc_fixpt_from_fraction() noinline
2024-12-21 7:40 ` Xi Ruoyao
@ 2024-12-22 4:27 ` Tiezhu Yang
2024-12-23 21:46 ` Nathan Chancellor
0 siblings, 1 reply; 37+ messages in thread
From: Tiezhu Yang @ 2024-12-22 4:27 UTC (permalink / raw)
To: Xi Ruoyao, Nathan Chancellor, Peter Zijlstra
Cc: Alex Deucher, Josh Poimboeuf, Huacai Chen, loongarch, amd-gfx,
linux-kernel, Nick Desaulniers, llvm
On 12/21/2024 03:40 PM, Xi Ruoyao wrote:
> On Fri, 2024-12-20 at 15:34 -0700, Nathan Chancellor wrote:
>>> Now, the thing is, these ASSERT()s are checking for divide-by-zero, I
>>> suspect clang figured that out and invokes UB on us and just stops
>>> code-gen.
>>
>> Yeah, I think your analysis is spot on, as this was introduced by a
>> change in clang from a few months ago according to my bisect:
>>
>> https://github.com/llvm/llvm-project/commit/37932643abab699e8bb1def08b7eb4eae7ff1448
>>
>> Since the ASSERT does not do anything to prevent the divide by zero (it
>> just flags it with WARN_ON) and the rest of the code doesn't either, I
>> assume that the codegen stops as soon as it encounters the unreachable
>> that change created from the path where divide by zero would occur via
>>
>> dc_fixpt_recip() ->
>> dc_fixpt_from_fraction() ->
>> complete_integer_division_u64() ->
>> div64_u64_rem()
>>
>> Shouldn't callers of division functions harden them against dividing by
>> zero?
>
> Yes I think it'd be the correct solution.
Thank you all. Do you mean like this?
--- >8 ---
diff --git a/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c
b/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c
index 88d3f9d7dd55..848d8e67304a 100644
--- a/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c
+++ b/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c
@@ -79,11 +79,13 @@ struct fixed31_32 dc_fixpt_from_fraction(long long
numerator, long long denomina
unsigned long long arg2_value = arg2_negative ? -denominator :
denominator;
unsigned long long remainder;
+ unsigned long long res_value;
/* determine integer part */
- unsigned long long res_value = complete_integer_division_u64(
- arg1_value, arg2_value, &remainder);
+ ASSERT(arg2_value);
+
+ res_value = complete_integer_division_u64(arg1_value,
arg2_value, &remainder);
ASSERT(res_value <= LONG_MAX);
@@ -214,8 +216,6 @@ struct fixed31_32 dc_fixpt_recip(struct fixed31_32 arg)
* Good idea to use Newton's method
*/
- ASSERT(arg.value);
-
return dc_fixpt_from_fraction(
dc_fixpt_one.value,
arg.value);
With the above changes, there is no "falls through" objtool warning
compiled with both clang 19 and the latest mainline clang 20.
If you are OK with it, I will send a separate formal patch to handle
this issue after doing some more testing.
Thanks,
Tiezhu
^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [PATCH v6 9/9] drm/amd/display: Mark dc_fixpt_from_fraction() noinline
2024-12-22 4:27 ` Tiezhu Yang
@ 2024-12-23 21:46 ` Nathan Chancellor
2024-12-25 9:43 ` Tiezhu Yang
0 siblings, 1 reply; 37+ messages in thread
From: Nathan Chancellor @ 2024-12-23 21:46 UTC (permalink / raw)
To: Tiezhu Yang
Cc: Xi Ruoyao, Peter Zijlstra, Alex Deucher, Josh Poimboeuf,
Huacai Chen, loongarch, amd-gfx, linux-kernel, Nick Desaulniers,
llvm
On Sun, Dec 22, 2024 at 12:27:47PM +0800, Tiezhu Yang wrote:
> On 12/21/2024 03:40 PM, Xi Ruoyao wrote:
> > On Fri, 2024-12-20 at 15:34 -0700, Nathan Chancellor wrote:
> > > > Now, the thing is, these ASSERT()s are checking for divide-by-zero, I
> > > > suspect clang figured that out and invokes UB on us and just stops
> > > > code-gen.
> > >
> > > Yeah, I think your analysis is spot on, as this was introduced by a
> > > change in clang from a few months ago according to my bisect:
> > >
> > > https://github.com/llvm/llvm-project/commit/37932643abab699e8bb1def08b7eb4eae7ff1448
> > >
> > > Since the ASSERT does not do anything to prevent the divide by zero (it
> > > just flags it with WARN_ON) and the rest of the code doesn't either, I
> > > assume that the codegen stops as soon as it encounters the unreachable
> > > that change created from the path where divide by zero would occur via
> > >
> > > dc_fixpt_recip() ->
> > > dc_fixpt_from_fraction() ->
> > > complete_integer_division_u64() ->
> > > div64_u64_rem()
> > >
> > > Shouldn't callers of division functions harden them against dividing by
> > > zero?
> >
> > Yes I think it'd be the correct solution.
>
> Thank you all. Do you mean like this?
>
> --- >8 ---
>
> diff --git a/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c
> b/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c
> index 88d3f9d7dd55..848d8e67304a 100644
> --- a/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c
> +++ b/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c
> @@ -79,11 +79,13 @@ struct fixed31_32 dc_fixpt_from_fraction(long long
> numerator, long long denomina
> unsigned long long arg2_value = arg2_negative ? -denominator :
> denominator;
>
> unsigned long long remainder;
> + unsigned long long res_value;
>
> /* determine integer part */
>
> - unsigned long long res_value = complete_integer_division_u64(
> - arg1_value, arg2_value, &remainder);
> + ASSERT(arg2_value);
> +
> + res_value = complete_integer_division_u64(arg1_value, arg2_value,
> &remainder);
>
> ASSERT(res_value <= LONG_MAX);
>
> @@ -214,8 +216,6 @@ struct fixed31_32 dc_fixpt_recip(struct fixed31_32 arg)
> * Good idea to use Newton's method
> */
>
> - ASSERT(arg.value);
> -
> return dc_fixpt_from_fraction(
> dc_fixpt_one.value,
> arg.value);
>
> With the above changes, there is no "falls through" objtool warning
> compiled with both clang 19 and the latest mainline clang 20.
I am somewhat surprised that changes anything because the ASSERT is not
stopping control flow so I would expect the same problem as before. I
guess it does not happen perhaps due to inlining differences? I looked
at this code briefly when I sent my initial message and I was not sure
where such a check should exist. It does not look like these functions
really do any sort of error handling.
> If you are OK with it, I will send a separate formal patch to handle
> this issue after doing some more testing.
It may still be worth doing this to get some initial thoughts from the
AMD DRM folks.
Cheers,
Nathan
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v6 9/9] drm/amd/display: Mark dc_fixpt_from_fraction() noinline
2024-12-23 21:46 ` Nathan Chancellor
@ 2024-12-25 9:43 ` Tiezhu Yang
0 siblings, 0 replies; 37+ messages in thread
From: Tiezhu Yang @ 2024-12-25 9:43 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Xi Ruoyao, Peter Zijlstra, Alex Deucher, Josh Poimboeuf,
Huacai Chen, loongarch, amd-gfx, linux-kernel, Nick Desaulniers,
llvm
On 12/24/2024 05:46 AM, Nathan Chancellor wrote:
> On Sun, Dec 22, 2024 at 12:27:47PM +0800, Tiezhu Yang wrote:
...
>> With the above changes, there is no "falls through" objtool warning
>> compiled with both clang 19 and the latest mainline clang 20.
>
> I am somewhat surprised that changes anything because the ASSERT is not
> stopping control flow so I would expect the same problem as before. I
> guess it does not happen perhaps due to inlining differences? I looked
It is weird and I think it is not the correct way.
> at this code briefly when I sent my initial message and I was not sure
> where such a check should exist. It does not look like these functions
> really do any sort of error handling.
>
>> If you are OK with it, I will send a separate formal patch to handle
>> this issue after doing some more testing.
>
> It may still be worth doing this to get some initial thoughts from the
> AMD DRM folks.
I think the correct way is:
Keep the current ASSERT for the aim of debugging, just add BUG() to
stop control flow if the divisor is zero.
--- >8 ---
diff --git a/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c
b/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c
index 88d3f9d7dd55..e15391e36b40 100644
--- a/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c
+++ b/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c
@@ -52,6 +52,7 @@ static inline unsigned long long
complete_integer_division_u64(
unsigned long long result;
ASSERT(divisor);
+ BUG_ON(!divisor);
result = div64_u64_rem(dividend, divisor, remainder);
diff --git a/drivers/gpu/drm/amd/display/dc/spl/spl_fixpt31_32.c
b/drivers/gpu/drm/amd/display/dc/spl/spl_fixpt31_32.c
index 131f1e3949d3..ce2036950808 100644
--- a/drivers/gpu/drm/amd/display/dc/spl/spl_fixpt31_32.c
+++ b/drivers/gpu/drm/amd/display/dc/spl/spl_fixpt31_32.c
@@ -30,6 +30,7 @@ static inline unsigned long long
spl_complete_integer_division_u64(
unsigned long long result;
SPL_ASSERT(divisor);
+ BUG_ON(!divisor);
result = spl_div64_u64_rem(dividend, divisor, remainder);
It looks reasonable and works well both on x86 and LoongArch, there are
no the following objtool warnings:
dc_fixpt_recip() falls through to next function dc_fixpt_sinc()
spl_fixpt_recip() falls through to next function spl_fixpt_sinc()
If no more comments, I will send a separate formal patch for your
review in the next week.
Thanks,
Tiezhu
^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [PATCH v6 0/9] Add jump table support for objtool on LoongArch
2024-12-17 1:08 [PATCH v6 0/9] Add jump table support for objtool on LoongArch Tiezhu Yang
` (8 preceding siblings ...)
2024-12-17 1:50 ` [PATCH v6 9/9] drm/amd/display: Mark dc_fixpt_from_fraction() noinline Tiezhu Yang
@ 2025-01-04 13:58 ` Huacai Chen
2025-01-11 6:57 ` Tiezhu Yang
10 siblings, 0 replies; 37+ messages in thread
From: Huacai Chen @ 2025-01-04 13:58 UTC (permalink / raw)
To: Tiezhu Yang
Cc: Josh Poimboeuf, Peter Zijlstra, loongarch, amd-gfx, linux-kernel
Hi, Josh and Peter,
I think this series (except the last patch, but that one can be a
separate one) is good enough now, right? If so, I think there is some
ways to get it upstream:
1) I merge objtool/core from tip.git to the loongarch tree, then apply
this whole series with your acked-by;
2) You apply the first 4 patches to tip.git, and then I apply others
to the loongarch tree (still need to merge objtool/core before apply
to avoid build issues).
I prefer the first method, but I can also accept other ways.
Huacai
On Tue, Dec 17, 2024 at 9:09 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>
> This version is based on tip/tip.git objtool/core branch [1], add some weak
> and arch-specific functions to make the generic code more readable, tested
> with the latest upstream mainline Binutils, GCC and Clang.
>
> The first 6 patches are preparation for patch #7 to enable jump table for
> objtool on LoongArch, the last 2 patches are small enough to fix objtool
> warnings "funcA() falls through to next function funcB()", one is under
> arch/loongarch and the other is under drm/amd/display.
>
> v6:
> -- Add arch_reloc_size() for x86 and ppc.
> -- Call arch_reloc_size() directly in add_jump_table().
> -- Refine arch_adjust_offset() for LoongArch.
> -- Rename arch_adjust_offset() to arch_jump_table_sym_offset().
> -- Get each table size of rodata in time for switch table.
> -- Update the commit message to make it more clear.
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/log/?h=objtool/core
>
> Tiezhu Yang (9):
> objtool: Handle various symbol types of rodata
> objtool: Handle different entry size of rodata
> objtool: Handle PC relative relocation type
> objtool: Handle unreachable entry of rodata
> objtool/LoongArch: Add support for switch table
> objtool/LoongArch: Add support for goto table
> LoongArch: Enable jump table for objtool
> LoongArch: Convert unreachable() to BUG()
> drm/amd/display: Mark dc_fixpt_from_fraction() noinline
>
> arch/loongarch/Kconfig | 3 +
> arch/loongarch/Makefile | 6 +-
> arch/loongarch/kernel/machine_kexec.c | 4 +-
> .../drm/amd/display/dc/basics/fixpt31_32.c | 2 +-
> tools/objtool/arch/loongarch/decode.c | 28 ++-
> .../objtool/arch/loongarch/include/arch/elf.h | 7 +
> tools/objtool/arch/loongarch/special.c | 159 +++++++++++++++++-
> tools/objtool/arch/powerpc/decode.c | 15 ++
> tools/objtool/arch/x86/decode.c | 13 ++
> tools/objtool/check.c | 28 ++-
> tools/objtool/include/objtool/arch.h | 3 +
> 11 files changed, 251 insertions(+), 17 deletions(-)
>
> --
> 2.42.0
>
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v6 0/9] Add jump table support for objtool on LoongArch
2024-12-17 1:08 [PATCH v6 0/9] Add jump table support for objtool on LoongArch Tiezhu Yang
` (9 preceding siblings ...)
2025-01-04 13:58 ` [PATCH v6 0/9] Add jump table support for objtool on LoongArch Huacai Chen
@ 2025-01-11 6:57 ` Tiezhu Yang
2025-01-15 1:34 ` Josh Poimboeuf
10 siblings, 1 reply; 37+ messages in thread
From: Tiezhu Yang @ 2025-01-11 6:57 UTC (permalink / raw)
To: Huacai Chen, Josh Poimboeuf, Peter Zijlstra
Cc: loongarch, amd-gfx, linux-kernel
Hi Josh and Peter,
On 12/17/2024 09:08 AM, Tiezhu Yang wrote:
> This version is based on tip/tip.git objtool/core branch [1], add some weak
> and arch-specific functions to make the generic code more readable, tested
> with the latest upstream mainline Binutils, GCC and Clang.
...
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/log/?h=objtool/core
>
> Tiezhu Yang (9):
> objtool: Handle various symbol types of rodata
> objtool: Handle different entry size of rodata
> objtool: Handle PC relative relocation type
> objtool: Handle unreachable entry of rodata
> objtool/LoongArch: Add support for switch table
> objtool/LoongArch: Add support for goto table
> LoongArch: Enable jump table for objtool
> LoongArch: Convert unreachable() to BUG()
> drm/amd/display: Mark dc_fixpt_from_fraction() noinline
Are you OK with the first 8 patches?
What's the merge plan for this series?
Thanks,
Tiezhu
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v6 0/9] Add jump table support for objtool on LoongArch
2025-01-11 6:57 ` Tiezhu Yang
@ 2025-01-15 1:34 ` Josh Poimboeuf
2025-02-10 6:07 ` Tiezhu Yang
0 siblings, 1 reply; 37+ messages in thread
From: Josh Poimboeuf @ 2025-01-15 1:34 UTC (permalink / raw)
To: Tiezhu Yang; +Cc: Huacai Chen, Peter Zijlstra, loongarch, amd-gfx, linux-kernel
On Sat, Jan 11, 2025 at 02:57:42PM +0800, Tiezhu Yang wrote:
> Hi Josh and Peter,
>
> On 12/17/2024 09:08 AM, Tiezhu Yang wrote:
> > This version is based on tip/tip.git objtool/core branch [1], add some weak
> > and arch-specific functions to make the generic code more readable, tested
> > with the latest upstream mainline Binutils, GCC and Clang.
>
> ...
>
> > [1] https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/log/?h=objtool/core
> >
> > Tiezhu Yang (9):
> > objtool: Handle various symbol types of rodata
> > objtool: Handle different entry size of rodata
> > objtool: Handle PC relative relocation type
> > objtool: Handle unreachable entry of rodata
> > objtool/LoongArch: Add support for switch table
> > objtool/LoongArch: Add support for goto table
> > LoongArch: Enable jump table for objtool
> > LoongArch: Convert unreachable() to BUG()
> > drm/amd/display: Mark dc_fixpt_from_fraction() noinline
>
> Are you OK with the first 8 patches?
> What's the merge plan for this series?
Sorry, my inbox is still reeling from the holidays. I will review this
soon.
--
Josh
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v6 0/9] Add jump table support for objtool on LoongArch
2025-01-15 1:34 ` Josh Poimboeuf
@ 2025-02-10 6:07 ` Tiezhu Yang
2025-02-10 21:26 ` Josh Poimboeuf
0 siblings, 1 reply; 37+ messages in thread
From: Tiezhu Yang @ 2025-02-10 6:07 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: Huacai Chen, Peter Zijlstra, loongarch, amd-gfx, linux-kernel
On 01/15/2025 09:34 AM, Josh Poimboeuf wrote:
> On Sat, Jan 11, 2025 at 02:57:42PM +0800, Tiezhu Yang wrote:
>> Hi Josh and Peter,
>>
>> On 12/17/2024 09:08 AM, Tiezhu Yang wrote:
>>> This version is based on tip/tip.git objtool/core branch [1], add some weak
>>> and arch-specific functions to make the generic code more readable, tested
>>> with the latest upstream mainline Binutils, GCC and Clang.
>>
>> ...
>>
>>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/log/?h=objtool/core
>>>
>>> Tiezhu Yang (9):
>>> objtool: Handle various symbol types of rodata
>>> objtool: Handle different entry size of rodata
>>> objtool: Handle PC relative relocation type
>>> objtool: Handle unreachable entry of rodata
>>> objtool/LoongArch: Add support for switch table
>>> objtool/LoongArch: Add support for goto table
>>> LoongArch: Enable jump table for objtool
>>> LoongArch: Convert unreachable() to BUG()
>>> drm/amd/display: Mark dc_fixpt_from_fraction() noinline
>>
>> Are you OK with the first 8 patches?
>> What's the merge plan for this series?
>
> Sorry, my inbox is still reeling from the holidays. I will review this
> soon.
What are the status of the first 8 patches?
What is the next step? Is there anything else to be done here?
I would really appreciate any advice on how to get this merged.
Thanks,
Tiezhu
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v6 4/9] objtool: Handle unreachable entry of rodata
2024-12-17 1:09 ` [PATCH v6 4/9] objtool: Handle unreachable entry of rodata Tiezhu Yang
@ 2025-02-10 21:17 ` Josh Poimboeuf
0 siblings, 0 replies; 37+ messages in thread
From: Josh Poimboeuf @ 2025-02-10 21:17 UTC (permalink / raw)
To: Tiezhu Yang; +Cc: Huacai Chen, Peter Zijlstra, loongarch, amd-gfx, linux-kernel
On Tue, Dec 17, 2024 at 09:09:00AM +0800, Tiezhu Yang wrote:
> When compiling with Clang on LoongArch, there exists unreachable entry of
> rodata which points to a position after the function return instruction,
> this is generated by compiler to fill the non-existent switch case, just
> skip the entry when parsing the relocation section of rodata.
>
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> ---
> tools/objtool/check.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index cff7416b207e..654cffcf9512 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -1986,9 +1986,10 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,
> if (!dest_insn)
> break;
>
> - /* Make sure the destination is in the same function: */
> - if (!insn_func(dest_insn) || insn_func(dest_insn)->pfunc != pfunc)
> - break;
> + if (!insn_func(dest_insn) || insn_func(dest_insn)->pfunc != pfunc) {
> + prev_offset = reloc_offset(reloc);
> + continue;
> + }
This patch can be dropped, this is already fixed in the -tip tree:
3724062ca2b1 ("objtool: Ignore dangling jump table entries")
--
Josh
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v6 0/9] Add jump table support for objtool on LoongArch
2025-02-10 6:07 ` Tiezhu Yang
@ 2025-02-10 21:26 ` Josh Poimboeuf
2025-02-11 11:59 ` Huacai Chen
0 siblings, 1 reply; 37+ messages in thread
From: Josh Poimboeuf @ 2025-02-10 21:26 UTC (permalink / raw)
To: Tiezhu Yang; +Cc: Huacai Chen, Peter Zijlstra, loongarch, amd-gfx, linux-kernel
On Mon, Feb 10, 2025 at 02:07:43PM +0800, Tiezhu Yang wrote:
> On 01/15/2025 09:34 AM, Josh Poimboeuf wrote:
> > On Sat, Jan 11, 2025 at 02:57:42PM +0800, Tiezhu Yang wrote:
> > > Hi Josh and Peter,
> > >
> > > On 12/17/2024 09:08 AM, Tiezhu Yang wrote:
> > > > This version is based on tip/tip.git objtool/core branch [1], add some weak
> > > > and arch-specific functions to make the generic code more readable, tested
> > > > with the latest upstream mainline Binutils, GCC and Clang.
> > >
> > > ...
> > >
> > > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/log/?h=objtool/core
> > > >
> > > > Tiezhu Yang (9):
> > > > objtool: Handle various symbol types of rodata
> > > > objtool: Handle different entry size of rodata
> > > > objtool: Handle PC relative relocation type
> > > > objtool: Handle unreachable entry of rodata
> > > > objtool/LoongArch: Add support for switch table
> > > > objtool/LoongArch: Add support for goto table
> > > > LoongArch: Enable jump table for objtool
> > > > LoongArch: Convert unreachable() to BUG()
> > > > drm/amd/display: Mark dc_fixpt_from_fraction() noinline
> > >
> > > Are you OK with the first 8 patches?
> > > What's the merge plan for this series?
> >
> > Sorry, my inbox is still reeling from the holidays. I will review this
> > soon.
>
> What are the status of the first 8 patches?
> What is the next step? Is there anything else to be done here?
> I would really appreciate any advice on how to get this merged.
Please post a new revision rebased on tip/master, with patch 4 dropped,
and I'll queue them up for -tip. Thanks for your patience.
--
Josh
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v6 0/9] Add jump table support for objtool on LoongArch
2025-02-10 21:26 ` Josh Poimboeuf
@ 2025-02-11 11:59 ` Huacai Chen
2025-02-11 23:30 ` Josh Poimboeuf
0 siblings, 1 reply; 37+ messages in thread
From: Huacai Chen @ 2025-02-11 11:59 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: Tiezhu Yang, Peter Zijlstra, loongarch, amd-gfx, linux-kernel
Hi, Josh,
On Tue, Feb 11, 2025 at 5:26 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> On Mon, Feb 10, 2025 at 02:07:43PM +0800, Tiezhu Yang wrote:
> > On 01/15/2025 09:34 AM, Josh Poimboeuf wrote:
> > > On Sat, Jan 11, 2025 at 02:57:42PM +0800, Tiezhu Yang wrote:
> > > > Hi Josh and Peter,
> > > >
> > > > On 12/17/2024 09:08 AM, Tiezhu Yang wrote:
> > > > > This version is based on tip/tip.git objtool/core branch [1], add some weak
> > > > > and arch-specific functions to make the generic code more readable, tested
> > > > > with the latest upstream mainline Binutils, GCC and Clang.
> > > >
> > > > ...
> > > >
> > > > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/log/?h=objtool/core
> > > > >
> > > > > Tiezhu Yang (9):
> > > > > objtool: Handle various symbol types of rodata
> > > > > objtool: Handle different entry size of rodata
> > > > > objtool: Handle PC relative relocation type
> > > > > objtool: Handle unreachable entry of rodata
> > > > > objtool/LoongArch: Add support for switch table
> > > > > objtool/LoongArch: Add support for goto table
> > > > > LoongArch: Enable jump table for objtool
> > > > > LoongArch: Convert unreachable() to BUG()
> > > > > drm/amd/display: Mark dc_fixpt_from_fraction() noinline
> > > >
> > > > Are you OK with the first 8 patches?
> > > > What's the merge plan for this series?
> > >
> > > Sorry, my inbox is still reeling from the holidays. I will review this
> > > soon.
> >
> > What are the status of the first 8 patches?
> > What is the next step? Is there anything else to be done here?
> > I would really appreciate any advice on how to get this merged.
>
> Please post a new revision rebased on tip/master, with patch 4 dropped,
> and I'll queue them up for -tip. Thanks for your patience.
You will queue the whole series, or the first 5 patches, or the first 3 patches?
Huacai
>
> --
> Josh
^ permalink raw reply [flat|nested] 37+ messages in thread
* [PATCH] objtool: remove duplicate case value R_PPC64_REL32
2024-12-17 1:08 ` [PATCH v6 2/9] objtool: Handle different entry size " Tiezhu Yang
@ 2025-02-11 14:19 ` Kexy Biscuit
2025-02-11 23:34 ` Josh Poimboeuf
0 siblings, 1 reply; 37+ messages in thread
From: Kexy Biscuit @ 2025-02-11 14:19 UTC (permalink / raw)
To: yangtiezhu
Cc: amd-gfx, chenhuacai, jpoimboe, linux-kernel, loongarch, peterz,
Kexy Biscuit
In arch/powerpc/include/uapi/asm/elf.h, R_PPC64_REL32 is defined as a
macro to R_PPC_REL32, makes the case value here being duplicate and
creates the following error...
arch/powerpc/decode.c: In function ‘arch_reloc_size’:
arch/powerpc/decode.c:114:9: error: duplicate case value
114 | case R_PPC64_REL32:
| ^~~~
arch/powerpc/decode.c:113:9: note: previously used here
113 | case R_PPC_REL32:
| ^~~~
Remove the duplicate case value to fix the error.
Fixes: "FROMLIST: objtool: Handle different entry size of rodata"
Signed-off-by: Kexy Biscuit <kexybiscuit@aosc.io>
---
This patch is required for this series to build on powerpc, however I'm
not sure if it's the preferred way... Please advise.
tools/objtool/arch/powerpc/decode.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/tools/objtool/arch/powerpc/decode.c b/tools/objtool/arch/powerpc/decode.c
index 3c95dd74fca0..7c0bf2429067 100644
--- a/tools/objtool/arch/powerpc/decode.c
+++ b/tools/objtool/arch/powerpc/decode.c
@@ -111,7 +111,6 @@ unsigned int arch_reloc_size(struct reloc *reloc)
{
switch (reloc_type(reloc)) {
case R_PPC_REL32:
- case R_PPC64_REL32:
case R_PPC_ADDR32:
case R_PPC_UADDR32:
case R_PPC_PLT32:
--
2.48.1
^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: [PATCH v6 0/9] Add jump table support for objtool on LoongArch
2025-02-11 11:59 ` Huacai Chen
@ 2025-02-11 23:30 ` Josh Poimboeuf
2025-02-12 7:22 ` Huacai Chen
0 siblings, 1 reply; 37+ messages in thread
From: Josh Poimboeuf @ 2025-02-11 23:30 UTC (permalink / raw)
To: Huacai Chen; +Cc: Tiezhu Yang, Peter Zijlstra, loongarch, amd-gfx, linux-kernel
On Tue, Feb 11, 2025 at 07:59:57PM +0800, Huacai Chen wrote:
> Hi, Josh,
>
> On Tue, Feb 11, 2025 at 5:26 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> >
> > On Mon, Feb 10, 2025 at 02:07:43PM +0800, Tiezhu Yang wrote:
> > > On 01/15/2025 09:34 AM, Josh Poimboeuf wrote:
> > > > On Sat, Jan 11, 2025 at 02:57:42PM +0800, Tiezhu Yang wrote:
> > > > > Hi Josh and Peter,
> > > > >
> > > > > On 12/17/2024 09:08 AM, Tiezhu Yang wrote:
> > > > > > This version is based on tip/tip.git objtool/core branch [1], add some weak
> > > > > > and arch-specific functions to make the generic code more readable, tested
> > > > > > with the latest upstream mainline Binutils, GCC and Clang.
> > > > >
> > > > > ...
> > > > >
> > > > > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/log/?h=objtool/core
> > > > > >
> > > > > > Tiezhu Yang (9):
> > > > > > objtool: Handle various symbol types of rodata
> > > > > > objtool: Handle different entry size of rodata
> > > > > > objtool: Handle PC relative relocation type
> > > > > > objtool: Handle unreachable entry of rodata
> > > > > > objtool/LoongArch: Add support for switch table
> > > > > > objtool/LoongArch: Add support for goto table
> > > > > > LoongArch: Enable jump table for objtool
> > > > > > LoongArch: Convert unreachable() to BUG()
> > > > > > drm/amd/display: Mark dc_fixpt_from_fraction() noinline
> > > > >
> > > > > Are you OK with the first 8 patches?
> > > > > What's the merge plan for this series?
> > > >
> > > > Sorry, my inbox is still reeling from the holidays. I will review this
> > > > soon.
> > >
> > > What are the status of the first 8 patches?
> > > What is the next step? Is there anything else to be done here?
> > > I would really appreciate any advice on how to get this merged.
> >
> > Please post a new revision rebased on tip/master, with patch 4 dropped,
> > and I'll queue them up for -tip. Thanks for your patience.
> You will queue the whole series, or the first 5 patches, or the first 3 patches?
The new series now has 7 patches:
Tiezhu Yang (7):
objtool: Handle various symbol types of rodata
objtool: Handle different entry size of rodata
objtool: Handle PC relative relocation type
objtool/LoongArch: Add support for switch table
objtool/LoongArch: Add support for goto table
LoongArch: Enable jump table for objtool
LoongArch: Convert unreachable() to BUG()
I was planning on queueing all 7.
In particular, patch 6 should stay with the objtool patches since
they're directly related.
But I was also just going to grab 7 as well.
Please let me know if you disagree.
--
Josh
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH] objtool: remove duplicate case value R_PPC64_REL32
2025-02-11 14:19 ` [PATCH] objtool: remove duplicate case value R_PPC64_REL32 Kexy Biscuit
@ 2025-02-11 23:34 ` Josh Poimboeuf
0 siblings, 0 replies; 37+ messages in thread
From: Josh Poimboeuf @ 2025-02-11 23:34 UTC (permalink / raw)
To: Kexy Biscuit
Cc: yangtiezhu, amd-gfx, chenhuacai, linux-kernel, loongarch, peterz
On Tue, Feb 11, 2025 at 10:19:57PM +0800, Kexy Biscuit wrote:
> In arch/powerpc/include/uapi/asm/elf.h, R_PPC64_REL32 is defined as a
> macro to R_PPC_REL32, makes the case value here being duplicate and
> creates the following error...
>
> arch/powerpc/decode.c: In function ‘arch_reloc_size’:
> arch/powerpc/decode.c:114:9: error: duplicate case value
> 114 | case R_PPC64_REL32:
> | ^~~~
> arch/powerpc/decode.c:113:9: note: previously used here
> 113 | case R_PPC_REL32:
> | ^~~~
>
> Remove the duplicate case value to fix the error.
>
> Fixes: "FROMLIST: objtool: Handle different entry size of rodata"
> Signed-off-by: Kexy Biscuit <kexybiscuit@aosc.io>
> ---
> This patch is required for this series to build on powerpc, however I'm
> not sure if it's the preferred way... Please advise.
If there are no objections, I'll squash this into the original patch to
avoid breaking bisection.
--
Josh
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v6 0/9] Add jump table support for objtool on LoongArch
2025-02-11 23:30 ` Josh Poimboeuf
@ 2025-02-12 7:22 ` Huacai Chen
2025-02-13 2:51 ` Josh Poimboeuf
0 siblings, 1 reply; 37+ messages in thread
From: Huacai Chen @ 2025-02-12 7:22 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: Tiezhu Yang, Peter Zijlstra, loongarch, amd-gfx, linux-kernel
On Wed, Feb 12, 2025 at 7:30 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> On Tue, Feb 11, 2025 at 07:59:57PM +0800, Huacai Chen wrote:
> > Hi, Josh,
> >
> > On Tue, Feb 11, 2025 at 5:26 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> > >
> > > On Mon, Feb 10, 2025 at 02:07:43PM +0800, Tiezhu Yang wrote:
> > > > On 01/15/2025 09:34 AM, Josh Poimboeuf wrote:
> > > > > On Sat, Jan 11, 2025 at 02:57:42PM +0800, Tiezhu Yang wrote:
> > > > > > Hi Josh and Peter,
> > > > > >
> > > > > > On 12/17/2024 09:08 AM, Tiezhu Yang wrote:
> > > > > > > This version is based on tip/tip.git objtool/core branch [1], add some weak
> > > > > > > and arch-specific functions to make the generic code more readable, tested
> > > > > > > with the latest upstream mainline Binutils, GCC and Clang.
> > > > > >
> > > > > > ...
> > > > > >
> > > > > > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/log/?h=objtool/core
> > > > > > >
> > > > > > > Tiezhu Yang (9):
> > > > > > > objtool: Handle various symbol types of rodata
> > > > > > > objtool: Handle different entry size of rodata
> > > > > > > objtool: Handle PC relative relocation type
> > > > > > > objtool: Handle unreachable entry of rodata
> > > > > > > objtool/LoongArch: Add support for switch table
> > > > > > > objtool/LoongArch: Add support for goto table
> > > > > > > LoongArch: Enable jump table for objtool
> > > > > > > LoongArch: Convert unreachable() to BUG()
> > > > > > > drm/amd/display: Mark dc_fixpt_from_fraction() noinline
> > > > > >
> > > > > > Are you OK with the first 8 patches?
> > > > > > What's the merge plan for this series?
> > > > >
> > > > > Sorry, my inbox is still reeling from the holidays. I will review this
> > > > > soon.
> > > >
> > > > What are the status of the first 8 patches?
> > > > What is the next step? Is there anything else to be done here?
> > > > I would really appreciate any advice on how to get this merged.
> > >
> > > Please post a new revision rebased on tip/master, with patch 4 dropped,
> > > and I'll queue them up for -tip. Thanks for your patience.
> > You will queue the whole series, or the first 5 patches, or the first 3 patches?
>
> The new series now has 7 patches:
>
> Tiezhu Yang (7):
> objtool: Handle various symbol types of rodata
> objtool: Handle different entry size of rodata
> objtool: Handle PC relative relocation type
> objtool/LoongArch: Add support for switch table
> objtool/LoongArch: Add support for goto table
> LoongArch: Enable jump table for objtool
> LoongArch: Convert unreachable() to BUG()
>
> I was planning on queueing all 7.
>
> In particular, patch 6 should stay with the objtool patches since
> they're directly related.
>
> But I was also just going to grab 7 as well.
>
> Please let me know if you disagree.
What about you merge the first 5 patches, and then I merge the last 2
to the loongarch tree? (I prefer to merge the whole series to the
loongarch tree with your acked-by, but that may be inconvenient to
you).
Huacai
>
> --
> Josh
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v6 0/9] Add jump table support for objtool on LoongArch
2025-02-12 7:22 ` Huacai Chen
@ 2025-02-13 2:51 ` Josh Poimboeuf
2025-02-17 3:13 ` Huacai Chen
0 siblings, 1 reply; 37+ messages in thread
From: Josh Poimboeuf @ 2025-02-13 2:51 UTC (permalink / raw)
To: Huacai Chen; +Cc: Tiezhu Yang, Peter Zijlstra, loongarch, amd-gfx, linux-kernel
On Wed, Feb 12, 2025 at 03:22:45PM +0800, Huacai Chen wrote:
> > The new series now has 7 patches:
> >
> > Tiezhu Yang (7):
> > objtool: Handle various symbol types of rodata
> > objtool: Handle different entry size of rodata
> > objtool: Handle PC relative relocation type
> > objtool/LoongArch: Add support for switch table
> > objtool/LoongArch: Add support for goto table
> > LoongArch: Enable jump table for objtool
> > LoongArch: Convert unreachable() to BUG()
> >
> > I was planning on queueing all 7.
> >
> > In particular, patch 6 should stay with the objtool patches since
> > they're directly related.
> >
> > But I was also just going to grab 7 as well.
> >
> > Please let me know if you disagree.
> What about you merge the first 5 patches, and then I merge the last 2
> to the loongarch tree? (I prefer to merge the whole series to the
> loongarch tree with your acked-by, but that may be inconvenient to
> you).
I want the first 5 patches to go through the -tip tree because we'll
have other patches depending on them.
I'll go ahead and take the first 5.
If you take in patches 6 & 7 separately, that might introduce a lot of
warnings. But it's up to you.
For patches 6 & 7:
Acked-by: Josh Poimboeuf <jpoimboe@kernel.org>
--
Josh
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v6 0/9] Add jump table support for objtool on LoongArch
2025-02-13 2:51 ` Josh Poimboeuf
@ 2025-02-17 3:13 ` Huacai Chen
2025-02-18 17:46 ` Josh Poimboeuf
0 siblings, 1 reply; 37+ messages in thread
From: Huacai Chen @ 2025-02-17 3:13 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: Tiezhu Yang, Peter Zijlstra, loongarch, amd-gfx, linux-kernel
On Thu, Feb 13, 2025 at 10:51 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> On Wed, Feb 12, 2025 at 03:22:45PM +0800, Huacai Chen wrote:
> > > The new series now has 7 patches:
> > >
> > > Tiezhu Yang (7):
> > > objtool: Handle various symbol types of rodata
> > > objtool: Handle different entry size of rodata
> > > objtool: Handle PC relative relocation type
> > > objtool/LoongArch: Add support for switch table
> > > objtool/LoongArch: Add support for goto table
> > > LoongArch: Enable jump table for objtool
> > > LoongArch: Convert unreachable() to BUG()
> > >
> > > I was planning on queueing all 7.
> > >
> > > In particular, patch 6 should stay with the objtool patches since
> > > they're directly related.
> > >
> > > But I was also just going to grab 7 as well.
> > >
> > > Please let me know if you disagree.
> > What about you merge the first 5 patches, and then I merge the last 2
> > to the loongarch tree? (I prefer to merge the whole series to the
> > loongarch tree with your acked-by, but that may be inconvenient to
> > you).
>
> I want the first 5 patches to go through the -tip tree because we'll
> have other patches depending on them.
>
> I'll go ahead and take the first 5.
>
> If you take in patches 6 & 7 separately, that might introduce a lot of
> warnings. But it's up to you.
>
> For patches 6 & 7:
>
> Acked-by: Josh Poimboeuf <jpoimboe@kernel.org>
OK, please take the first 5 patches, I will merge your objtool/core to
the loongarch tree and then apply the last 2 to avoid build warnings.
Huacai
>
> --
> Josh
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v6 0/9] Add jump table support for objtool on LoongArch
2025-02-17 3:13 ` Huacai Chen
@ 2025-02-18 17:46 ` Josh Poimboeuf
2025-02-19 9:49 ` Huacai Chen
0 siblings, 1 reply; 37+ messages in thread
From: Josh Poimboeuf @ 2025-02-18 17:46 UTC (permalink / raw)
To: Huacai Chen; +Cc: Tiezhu Yang, Peter Zijlstra, loongarch, amd-gfx, linux-kernel
On Mon, Feb 17, 2025 at 11:13:43AM +0800, Huacai Chen wrote:
> On Thu, Feb 13, 2025 at 10:51 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> >
> > On Wed, Feb 12, 2025 at 03:22:45PM +0800, Huacai Chen wrote:
> > > > The new series now has 7 patches:
> > > >
> > > > Tiezhu Yang (7):
> > > > objtool: Handle various symbol types of rodata
> > > > objtool: Handle different entry size of rodata
> > > > objtool: Handle PC relative relocation type
> > > > objtool/LoongArch: Add support for switch table
> > > > objtool/LoongArch: Add support for goto table
> > > > LoongArch: Enable jump table for objtool
> > > > LoongArch: Convert unreachable() to BUG()
> > > >
> > > > I was planning on queueing all 7.
> > > >
> > > > In particular, patch 6 should stay with the objtool patches since
> > > > they're directly related.
> > > >
> > > > But I was also just going to grab 7 as well.
> > > >
> > > > Please let me know if you disagree.
> > > What about you merge the first 5 patches, and then I merge the last 2
> > > to the loongarch tree? (I prefer to merge the whole series to the
> > > loongarch tree with your acked-by, but that may be inconvenient to
> > > you).
> >
> > I want the first 5 patches to go through the -tip tree because we'll
> > have other patches depending on them.
> >
> > I'll go ahead and take the first 5.
> >
> > If you take in patches 6 & 7 separately, that might introduce a lot of
> > warnings. But it's up to you.
> >
> > For patches 6 & 7:
> >
> > Acked-by: Josh Poimboeuf <jpoimboe@kernel.org>
> OK, please take the first 5 patches, I will merge your objtool/core to
> the loongarch tree and then apply the last 2 to avoid build warnings.
Looks like that's not going to work. Without patch 7 I'm getting a
warning (upgraded to a build error with a pending change to upgrade
objtool warnings to errors):
arch/loongarch/kernel/machine_kexec.o: error: objtool: kexec_reboot() falls through to next function crash_shutdown_secondary()
arch/loongarch/kernel/machine_kexec.o: error: objtool: kexec_reboot+0x1c: (branch)
arch/loongarch/kernel/machine_kexec.o: error: objtool: kexec_reboot+0x0: <=== (sym)
arch/loongarch/kernel/machine_kexec.o: error: objtool: 1 warning(s) upgraded to errors
And that would break bisection anyway, so that really needs to come
before the others.
--
Josh
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v6 0/9] Add jump table support for objtool on LoongArch
2025-02-18 17:46 ` Josh Poimboeuf
@ 2025-02-19 9:49 ` Huacai Chen
2025-02-20 21:33 ` Josh Poimboeuf
0 siblings, 1 reply; 37+ messages in thread
From: Huacai Chen @ 2025-02-19 9:49 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: Tiezhu Yang, Peter Zijlstra, loongarch, amd-gfx, linux-kernel
On Wed, Feb 19, 2025 at 1:46 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> On Mon, Feb 17, 2025 at 11:13:43AM +0800, Huacai Chen wrote:
> > On Thu, Feb 13, 2025 at 10:51 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> > >
> > > On Wed, Feb 12, 2025 at 03:22:45PM +0800, Huacai Chen wrote:
> > > > > The new series now has 7 patches:
> > > > >
> > > > > Tiezhu Yang (7):
> > > > > objtool: Handle various symbol types of rodata
> > > > > objtool: Handle different entry size of rodata
> > > > > objtool: Handle PC relative relocation type
> > > > > objtool/LoongArch: Add support for switch table
> > > > > objtool/LoongArch: Add support for goto table
> > > > > LoongArch: Enable jump table for objtool
> > > > > LoongArch: Convert unreachable() to BUG()
> > > > >
> > > > > I was planning on queueing all 7.
> > > > >
> > > > > In particular, patch 6 should stay with the objtool patches since
> > > > > they're directly related.
> > > > >
> > > > > But I was also just going to grab 7 as well.
> > > > >
> > > > > Please let me know if you disagree.
> > > > What about you merge the first 5 patches, and then I merge the last 2
> > > > to the loongarch tree? (I prefer to merge the whole series to the
> > > > loongarch tree with your acked-by, but that may be inconvenient to
> > > > you).
> > >
> > > I want the first 5 patches to go through the -tip tree because we'll
> > > have other patches depending on them.
> > >
> > > I'll go ahead and take the first 5.
> > >
> > > If you take in patches 6 & 7 separately, that might introduce a lot of
> > > warnings. But it's up to you.
> > >
> > > For patches 6 & 7:
> > >
> > > Acked-by: Josh Poimboeuf <jpoimboe@kernel.org>
> > OK, please take the first 5 patches, I will merge your objtool/core to
> > the loongarch tree and then apply the last 2 to avoid build warnings.
>
> Looks like that's not going to work. Without patch 7 I'm getting a
> warning (upgraded to a build error with a pending change to upgrade
> objtool warnings to errors):
>
> arch/loongarch/kernel/machine_kexec.o: error: objtool: kexec_reboot() falls through to next function crash_shutdown_secondary()
> arch/loongarch/kernel/machine_kexec.o: error: objtool: kexec_reboot+0x1c: (branch)
> arch/loongarch/kernel/machine_kexec.o: error: objtool: kexec_reboot+0x0: <=== (sym)
> arch/loongarch/kernel/machine_kexec.o: error: objtool: 1 warning(s) upgraded to errors
>
> And that would break bisection anyway, so that really needs to come
> before the others.
OK, then please take the whole series of V7, but please exchange the
order of Patch-6 and Patch-7 (I think the enablement should be the
last for bisection).
For V7: Acked-by: Huacai Chen <chenhuacai@loongson.cn>
Huacai
>
> --
> Josh
^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: [PATCH v6 0/9] Add jump table support for objtool on LoongArch
2025-02-19 9:49 ` Huacai Chen
@ 2025-02-20 21:33 ` Josh Poimboeuf
0 siblings, 0 replies; 37+ messages in thread
From: Josh Poimboeuf @ 2025-02-20 21:33 UTC (permalink / raw)
To: Huacai Chen; +Cc: Tiezhu Yang, Peter Zijlstra, loongarch, amd-gfx, linux-kernel
On Wed, Feb 19, 2025 at 05:49:01PM +0800, Huacai Chen wrote:
> > Looks like that's not going to work. Without patch 7 I'm getting a
> > warning (upgraded to a build error with a pending change to upgrade
> > objtool warnings to errors):
> >
> > arch/loongarch/kernel/machine_kexec.o: error: objtool: kexec_reboot() falls through to next function crash_shutdown_secondary()
> > arch/loongarch/kernel/machine_kexec.o: error: objtool: kexec_reboot+0x1c: (branch)
> > arch/loongarch/kernel/machine_kexec.o: error: objtool: kexec_reboot+0x0: <=== (sym)
> > arch/loongarch/kernel/machine_kexec.o: error: objtool: 1 warning(s) upgraded to errors
> >
> > And that would break bisection anyway, so that really needs to come
> > before the others.
> OK, then please take the whole series of V7, but please exchange the
> order of Patch-6 and Patch-7 (I think the enablement should be the
> last for bisection).
> For V7: Acked-by: Huacai Chen <chenhuacai@loongson.cn>
Ok, but to avoid the warning, patch 7 needs to come *before* the objtool
patches, so it will be the following order:
LoongArch: Convert unreachable() to BUG()
objtool: Handle various symbol types of rodata
objtool: Handle different entry size of rodata
objtool: Handle PC relative relocation type
objtool/LoongArch: Add support for switch table
objtool/LoongArch: Add support for goto table
LoongArch: Enable jump table for objtool
--
Josh
^ permalink raw reply [flat|nested] 37+ messages in thread
* [tip: objtool/core] LoongArch: Enable jump table for objtool
2024-12-17 1:09 ` [PATCH v6 7/9] LoongArch: Enable jump table for objtool Tiezhu Yang
@ 2025-03-14 20:03 ` tip-bot2 for Tiezhu Yang
0 siblings, 0 replies; 37+ messages in thread
From: tip-bot2 for Tiezhu Yang @ 2025-03-14 20:03 UTC (permalink / raw)
To: linux-tip-commits
Cc: Tiezhu Yang, Huacai Chen, Josh Poimboeuf, x86, linux-kernel
The following commit has been merged into the objtool/core branch of tip:
Commit-ID: e20ab7d454ee8d1e0e8b9ff73a7c87e84c666b2f
Gitweb: https://git.kernel.org/tip/e20ab7d454ee8d1e0e8b9ff73a7c87e84c666b2f
Author: Tiezhu Yang <yangtiezhu@loongson.cn>
AuthorDate: Tue, 17 Dec 2024 09:09:03 +08:00
Committer: Josh Poimboeuf <jpoimboe@kernel.org>
CommitterDate: Wed, 12 Mar 2025 15:43:39 -07:00
LoongArch: Enable jump table for objtool
For now, it is time to remove -fno-jump-tables to enable jump table for
objtool if the compiler has -mannotate-tablejump, otherwise it is better
to remain -fno-jump-tables to keep compatibility with older compilers.
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
Link: https://lore.kernel.org/r/20241217010905.13054-8-yangtiezhu@loongson.cn
Acked-by: Huacai Chen <chenhuacai@loongson.cn>
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
arch/loongarch/Kconfig | 3 +++
arch/loongarch/Makefile | 6 +++++-
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
index 2b8bd27..15aaa2e 100644
--- a/arch/loongarch/Kconfig
+++ b/arch/loongarch/Kconfig
@@ -291,6 +291,9 @@ config AS_HAS_LBT_EXTENSION
config AS_HAS_LVZ_EXTENSION
def_bool $(as-instr,hvcl 0)
+config CC_HAS_ANNOTATE_TABLEJUMP
+ def_bool $(cc-option,-mannotate-tablejump)
+
menu "Kernel type and options"
source "kernel/Kconfig.hz"
diff --git a/arch/loongarch/Makefile b/arch/loongarch/Makefile
index 567bd12..0304eab 100644
--- a/arch/loongarch/Makefile
+++ b/arch/loongarch/Makefile
@@ -101,7 +101,11 @@ KBUILD_AFLAGS += $(call cc-option,-mthin-add-sub) $(call cc-option,-Wa$(comma)
KBUILD_CFLAGS += $(call cc-option,-mthin-add-sub) $(call cc-option,-Wa$(comma)-mthin-add-sub)
ifdef CONFIG_OBJTOOL
-KBUILD_CFLAGS += -fno-jump-tables
+ifdef CONFIG_CC_HAS_ANNOTATE_TABLEJUMP
+KBUILD_CFLAGS += -mannotate-tablejump
+else
+KBUILD_CFLAGS += -fno-jump-tables # keep compatibility with older compilers
+endif
endif
KBUILD_RUSTFLAGS += --target=loongarch64-unknown-none-softfloat -Ccode-model=small
^ permalink raw reply related [flat|nested] 37+ messages in thread
end of thread, other threads:[~2025-03-14 20:04 UTC | newest]
Thread overview: 37+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-17 1:08 [PATCH v6 0/9] Add jump table support for objtool on LoongArch Tiezhu Yang
2024-12-17 1:08 ` [PATCH v6 1/9] objtool: Handle various symbol types of rodata Tiezhu Yang
2024-12-17 1:08 ` [PATCH v6 2/9] objtool: Handle different entry size " Tiezhu Yang
2025-02-11 14:19 ` [PATCH] objtool: remove duplicate case value R_PPC64_REL32 Kexy Biscuit
2025-02-11 23:34 ` Josh Poimboeuf
2024-12-17 1:08 ` [PATCH v6 3/9] objtool: Handle PC relative relocation type Tiezhu Yang
2024-12-17 1:09 ` [PATCH v6 4/9] objtool: Handle unreachable entry of rodata Tiezhu Yang
2025-02-10 21:17 ` Josh Poimboeuf
2024-12-17 1:09 ` [PATCH v6 5/9] objtool/LoongArch: Add support for switch table Tiezhu Yang
2024-12-17 1:09 ` [PATCH v6 6/9] objtool/LoongArch: Add support for goto table Tiezhu Yang
2024-12-17 1:09 ` [PATCH v6 7/9] LoongArch: Enable jump table for objtool Tiezhu Yang
2025-03-14 20:03 ` [tip: objtool/core] " tip-bot2 for Tiezhu Yang
2024-12-17 1:09 ` [PATCH v6 8/9] LoongArch: Convert unreachable() to BUG() Tiezhu Yang
2024-12-17 1:50 ` [PATCH v6 9/9] drm/amd/display: Mark dc_fixpt_from_fraction() noinline Tiezhu Yang
2024-12-18 14:36 ` Huacai Chen
2024-12-18 19:05 ` Josh Poimboeuf
2024-12-18 19:22 ` Alex Deucher
2024-12-20 5:02 ` Tiezhu Yang
2024-12-20 10:31 ` Peter Zijlstra
2024-12-20 22:34 ` Nathan Chancellor
2024-12-21 7:40 ` Xi Ruoyao
2024-12-22 4:27 ` Tiezhu Yang
2024-12-23 21:46 ` Nathan Chancellor
2024-12-25 9:43 ` Tiezhu Yang
2025-01-04 13:58 ` [PATCH v6 0/9] Add jump table support for objtool on LoongArch Huacai Chen
2025-01-11 6:57 ` Tiezhu Yang
2025-01-15 1:34 ` Josh Poimboeuf
2025-02-10 6:07 ` Tiezhu Yang
2025-02-10 21:26 ` Josh Poimboeuf
2025-02-11 11:59 ` Huacai Chen
2025-02-11 23:30 ` Josh Poimboeuf
2025-02-12 7:22 ` Huacai Chen
2025-02-13 2:51 ` Josh Poimboeuf
2025-02-17 3:13 ` Huacai Chen
2025-02-18 17:46 ` Josh Poimboeuf
2025-02-19 9:49 ` Huacai Chen
2025-02-20 21:33 ` Josh Poimboeuf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox