This patch abstracts the few architecture dependent tests that are
perform when handling special section and switch tables. It enables any
architecture to ignore a particular CPU feature or not to handle switch
tables.
Signed-off-by: Raphael Gault <[email protected]>
---
tools/objtool/arch/arm64/Build | 1 +
tools/objtool/arch/arm64/arch_special.c | 22 +++++++++++++++
.../objtool/arch/arm64/include/arch_special.h | 10 +++++--
tools/objtool/arch/x86/Build | 1 +
tools/objtool/arch/x86/arch_special.c | 28 +++++++++++++++++++
tools/objtool/arch/x86/include/arch_special.h | 9 ++++++
tools/objtool/check.c | 24 ++++++++++++++--
tools/objtool/special.c | 9 ++----
tools/objtool/special.h | 3 ++
9 files changed, 96 insertions(+), 11 deletions(-)
create mode 100644 tools/objtool/arch/arm64/arch_special.c
create mode 100644 tools/objtool/arch/x86/arch_special.c
diff --git a/tools/objtool/arch/arm64/Build b/tools/objtool/arch/arm64/Build
index bf7a32c2b9e9..3d09be745a84 100644
--- a/tools/objtool/arch/arm64/Build
+++ b/tools/objtool/arch/arm64/Build
@@ -1,3 +1,4 @@
+objtool-y += arch_special.o
objtool-y += decode.o
objtool-y += orc_dump.o
objtool-y += orc_gen.o
diff --git a/tools/objtool/arch/arm64/arch_special.c b/tools/objtool/arch/arm64/arch_special.c
new file mode 100644
index 000000000000..a21d28876317
--- /dev/null
+++ b/tools/objtool/arch/arm64/arch_special.c
@@ -0,0 +1,22 @@
+/*
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+#include "../../special.h"
+#include "arch_special.h"
+
+void arch_force_alt_path(unsigned short feature,
+ bool uaccess,
+ struct special_alt *alt)
+{
+}
diff --git a/tools/objtool/arch/arm64/include/arch_special.h b/tools/objtool/arch/arm64/include/arch_special.h
index 63da775d0581..185103be8a51 100644
--- a/tools/objtool/arch/arm64/include/arch_special.h
+++ b/tools/objtool/arch/arm64/include/arch_special.h
@@ -30,7 +30,13 @@
#define ALT_ORIG_LEN_OFFSET 10
#define ALT_NEW_LEN_OFFSET 11
-#define X86_FEATURE_POPCNT (4 * 32 + 23)
-#define X86_FEATURE_SMAP (9 * 32 + 20)
+static inline bool arch_should_ignore_feature(unsigned short feature)
+{
+ return false;
+}
+static inline bool arch_support_switch_table(void)
+{
+ return false;
+}
#endif /* _ARM64_ARCH_SPECIAL_H */
diff --git a/tools/objtool/arch/x86/Build b/tools/objtool/arch/x86/Build
index 1f11b45999d0..63e167775bc8 100644
--- a/tools/objtool/arch/x86/Build
+++ b/tools/objtool/arch/x86/Build
@@ -1,3 +1,4 @@
+objtool-y += arch_special.o
objtool-y += decode.o
objtool-y += orc_dump.o
objtool-y += orc_gen.o
diff --git a/tools/objtool/arch/x86/arch_special.c b/tools/objtool/arch/x86/arch_special.c
new file mode 100644
index 000000000000..6583a1770bb2
--- /dev/null
+++ b/tools/objtool/arch/x86/arch_special.c
@@ -0,0 +1,28 @@
+/*
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+#include "../../special.h"
+#include "arch_special.h"
+
+void arch_force_alt_path(unsigned short feature,
+ bool uaccess,
+ struct special_alt *alt)
+{
+ if (feature == X86_FEATURE_SMAP) {
+ if (uaccess)
+ alt->skip_orig = true;
+ else
+ alt->skip_alt = true;
+ }
+}
diff --git a/tools/objtool/arch/x86/include/arch_special.h b/tools/objtool/arch/x86/include/arch_special.h
index 424ce47013e3..fce2b1193194 100644
--- a/tools/objtool/arch/x86/include/arch_special.h
+++ b/tools/objtool/arch/x86/include/arch_special.h
@@ -33,4 +33,13 @@
#define X86_FEATURE_POPCNT (4 * 32 + 23)
#define X86_FEATURE_SMAP (9 * 32 + 20)
+static inline bool arch_should_ignore_feature(unsigned short feature)
+{
+ return feature == X86_FEATURE_POPCNT;
+}
+
+static inline bool arch_support_switch_table(void)
+{
+ return true;
+}
#endif /* _X86_ARCH_SPECIAL_H */
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 30e147391dcb..4af6422d3428 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -729,7 +729,7 @@ static int handle_group_alt(struct objtool_file *file,
last_orig_insn = insn;
}
- if (next_insn_same_sec(file, last_orig_insn)) {
+ if (last_orig_insn && next_insn_same_sec(file, last_orig_insn)) {
fake_jump = malloc(sizeof(*fake_jump));
if (!fake_jump) {
WARN("malloc failed");
@@ -1061,6 +1061,26 @@ static struct rela *find_jump_table(struct objtool_file *file,
table_rela = find_rela_by_dest(table_sec, table_offset);
if (!table_rela)
continue;
+ /*
+ * If we are on arm64 architecture, we now that we
+ * are in presence of a switch table thanks to
+ * the `br <Xn>` insn. but we can't retrieve it yet.
+ * So we just ignore unreachable for this file.
+ */
+ if (!arch_support_switch_table()) {
+ file->ignore_unreachables = true;
+ return NULL;
+ }
+
+ rodata_rela = find_rela_by_dest(rodata_sec, table_offset);
+ if (rodata_rela) {
+ /*
+ * Use of RIP-relative switch jumps is quite rare, and
+ * indicates a rare GCC quirk/bug which can leave dead
+ * code behind.
+ */
+ if (text_rela->type == R_X86_64_PC32)
+ file->ignore_unreachables = true;
/*
* Use of RIP-relative switch jumps is quite rare, and
@@ -1864,7 +1884,7 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
insn = first;
sec = insn->sec;
- if (insn->alt_group && list_empty(&insn->alts)) {
+ if (!insn->visited && insn->alt_group && list_empty(&insn->alts)) {
WARN_FUNC("don't know how to handle branch to middle of alternative instruction group",
sec, insn->offset);
return 1;
diff --git a/tools/objtool/special.c b/tools/objtool/special.c
index b8ccee1b5382..7a0092d6e5b3 100644
--- a/tools/objtool/special.c
+++ b/tools/objtool/special.c
@@ -81,7 +81,7 @@ static int get_alt_entry(struct elf *elf, struct special_entry *entry,
* feature path which is a "very very small percentage of
* machines".
*/
- if (feature == X86_FEATURE_POPCNT)
+ if (arch_should_ignore_feature(feature))
alt->skip_orig = true;
/*
@@ -93,12 +93,7 @@ static int get_alt_entry(struct elf *elf, struct special_entry *entry,
* find paths that see the STAC but take the NOP instead of
* CLAC and the other way around.
*/
- if (feature == X86_FEATURE_SMAP) {
- if (uaccess)
- alt->skip_orig = true;
- else
- alt->skip_alt = true;
- }
+ arch_force_alt_path(feature, uaccess, alt);
}
orig_rela = find_rela_by_dest(sec, offset + entry->orig);
diff --git a/tools/objtool/special.h b/tools/objtool/special.h
index 35061530e46e..90626a7e41cf 100644
--- a/tools/objtool/special.h
+++ b/tools/objtool/special.h
@@ -27,5 +27,8 @@ struct special_alt {
};
int special_get_alts(struct elf *elf, struct list_head *alts);
+void arch_force_alt_path(unsigned short feature,
+ bool uaccess,
+ struct special_alt *alt);
#endif /* _SPECIAL_H */
--
2.17.1
On Fri, Aug 16, 2019 at 01:23:50PM +0100, Raphael Gault wrote:
> This patch abstracts the few architecture dependent tests that are
The patch description shouldn't talk about the patch specifically, but
should rather describe what it does, in imperative language.
> perform when handling special section and switch tables. It enables any
"performed"
> architecture to ignore a particular CPU feature or not to handle switch
> tables.
>
> Signed-off-by: Raphael Gault <[email protected]>
> ---
> tools/objtool/arch/arm64/Build | 1 +
> tools/objtool/arch/arm64/arch_special.c | 22 +++++++++++++++
> .../objtool/arch/arm64/include/arch_special.h | 10 +++++--
> tools/objtool/arch/x86/Build | 1 +
> tools/objtool/arch/x86/arch_special.c | 28 +++++++++++++++++++
> tools/objtool/arch/x86/include/arch_special.h | 9 ++++++
> tools/objtool/check.c | 24 ++++++++++++++--
> tools/objtool/special.c | 9 ++----
> tools/objtool/special.h | 3 ++
> 9 files changed, 96 insertions(+), 11 deletions(-)
> create mode 100644 tools/objtool/arch/arm64/arch_special.c
> create mode 100644 tools/objtool/arch/x86/arch_special.c
>
> diff --git a/tools/objtool/arch/arm64/Build b/tools/objtool/arch/arm64/Build
> index bf7a32c2b9e9..3d09be745a84 100644
> --- a/tools/objtool/arch/arm64/Build
> +++ b/tools/objtool/arch/arm64/Build
> @@ -1,3 +1,4 @@
> +objtool-y += arch_special.o
> objtool-y += decode.o
> objtool-y += orc_dump.o
> objtool-y += orc_gen.o
> diff --git a/tools/objtool/arch/arm64/arch_special.c b/tools/objtool/arch/arm64/arch_special.c
> new file mode 100644
> index 000000000000..a21d28876317
> --- /dev/null
> +++ b/tools/objtool/arch/arm64/arch_special.c
> @@ -0,0 +1,22 @@
> +/*
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +#include "../../special.h"
> +#include "arch_special.h"
> +
> +void arch_force_alt_path(unsigned short feature,
> + bool uaccess,
> + struct special_alt *alt)
> +{
> +}
Instead of these dedicated files with empty .c functions -- including
the arm64 orc .c files -- I'd rather just have them be empty static
inline functions in header files, like the kernel does.
> diff --git a/tools/objtool/arch/arm64/include/arch_special.h b/tools/objtool/arch/arm64/include/arch_special.h
> index 63da775d0581..185103be8a51 100644
> --- a/tools/objtool/arch/arm64/include/arch_special.h
> +++ b/tools/objtool/arch/arm64/include/arch_special.h
> @@ -30,7 +30,13 @@
> #define ALT_ORIG_LEN_OFFSET 10
> #define ALT_NEW_LEN_OFFSET 11
>
> -#define X86_FEATURE_POPCNT (4 * 32 + 23)
> -#define X86_FEATURE_SMAP (9 * 32 + 20)
> +static inline bool arch_should_ignore_feature(unsigned short feature)
> +{
> + return false;
> +}
>
> +static inline bool arch_support_switch_table(void)
> +{
> + return false;
> +}
> #endif /* _ARM64_ARCH_SPECIAL_H */
> diff --git a/tools/objtool/arch/x86/Build b/tools/objtool/arch/x86/Build
> index 1f11b45999d0..63e167775bc8 100644
> --- a/tools/objtool/arch/x86/Build
> +++ b/tools/objtool/arch/x86/Build
> @@ -1,3 +1,4 @@
> +objtool-y += arch_special.o
> objtool-y += decode.o
> objtool-y += orc_dump.o
> objtool-y += orc_gen.o
> diff --git a/tools/objtool/arch/x86/arch_special.c b/tools/objtool/arch/x86/arch_special.c
> new file mode 100644
> index 000000000000..6583a1770bb2
> --- /dev/null
> +++ b/tools/objtool/arch/x86/arch_special.c
The "arch_" is redundant, these files can just be named "special.c".
> @@ -0,0 +1,28 @@
> +/*
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +#include "../../special.h"
> +#include "arch_special.h"
> +
> +void arch_force_alt_path(unsigned short feature,
> + bool uaccess,
> + struct special_alt *alt)
> +{
> + if (feature == X86_FEATURE_SMAP) {
> + if (uaccess)
> + alt->skip_orig = true;
> + else
> + alt->skip_alt = true;
> + }
> +}
Bad indention.
> diff --git a/tools/objtool/arch/x86/include/arch_special.h b/tools/objtool/arch/x86/include/arch_special.h
> index 424ce47013e3..fce2b1193194 100644
> --- a/tools/objtool/arch/x86/include/arch_special.h
> +++ b/tools/objtool/arch/x86/include/arch_special.h
> @@ -33,4 +33,13 @@
> #define X86_FEATURE_POPCNT (4 * 32 + 23)
> #define X86_FEATURE_SMAP (9 * 32 + 20)
>
> +static inline bool arch_should_ignore_feature(unsigned short feature)
> +{
> + return feature == X86_FEATURE_POPCNT;
> +}
> +
> +static inline bool arch_support_switch_table(void)
> +{
> + return true;
> +}
> #endif /* _X86_ARCH_SPECIAL_H */
> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index 30e147391dcb..4af6422d3428 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -729,7 +729,7 @@ static int handle_group_alt(struct objtool_file *file,
> last_orig_insn = insn;
> }
>
> - if (next_insn_same_sec(file, last_orig_insn)) {
> + if (last_orig_insn && next_insn_same_sec(file, last_orig_insn)) {
Even if the reason for the change is trivial, these types of changes
(and the insn->visited change below) should each be in an earlier
separate patch which explains the reasoning. Putting each logical
change in its own patch helps with review, and also future bisections,
debugging and archaeology.
> fake_jump = malloc(sizeof(*fake_jump));
> if (!fake_jump) {
> WARN("malloc failed");
> @@ -1061,6 +1061,26 @@ static struct rela *find_jump_table(struct objtool_file *file,
> table_rela = find_rela_by_dest(table_sec, table_offset);
> if (!table_rela)
> continue;
> + /*
> + * If we are on arm64 architecture, we now that we
"know"
> + * are in presence of a switch table thanks to
> + * the `br <Xn>` insn. but we can't retrieve it yet.
> + * So we just ignore unreachable for this file.
> + */
> + if (!arch_support_switch_table()) {
> + file->ignore_unreachables = true;
> + return NULL;
> + }
All arches need to support switch tables, otherwise it's a major gap in
functionality.
I think you did it this way because the switch table support comes in a
later patch, right? If so, the patches should be reordered so that you
don't need to add arch_support_switch_table() in the middle.
> +
> + rodata_rela = find_rela_by_dest(rodata_sec, table_offset);
> + if (rodata_rela) {
> + /*
> + * Use of RIP-relative switch jumps is quite rare, and
> + * indicates a rare GCC quirk/bug which can leave dead
> + * code behind.
> + */
> + if (text_rela->type == R_X86_64_PC32)
> + file->ignore_unreachables = true;
>
> /*
> * Use of RIP-relative switch jumps is quite rare, and
This repeats code which already exists right below it?
> @@ -1864,7 +1884,7 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
> insn = first;
> sec = insn->sec;
>
> - if (insn->alt_group && list_empty(&insn->alts)) {
> + if (!insn->visited && insn->alt_group && list_empty(&insn->alts)) {
> WARN_FUNC("don't know how to handle branch to middle of alternative instruction group",
> sec, insn->offset);
> return 1;
> diff --git a/tools/objtool/special.c b/tools/objtool/special.c
> index b8ccee1b5382..7a0092d6e5b3 100644
> --- a/tools/objtool/special.c
> +++ b/tools/objtool/special.c
> @@ -81,7 +81,7 @@ static int get_alt_entry(struct elf *elf, struct special_entry *entry,
> * feature path which is a "very very small percentage of
> * machines".
> */
> - if (feature == X86_FEATURE_POPCNT)
> + if (arch_should_ignore_feature(feature))
> alt->skip_orig = true;
>
> /*
> @@ -93,12 +93,7 @@ static int get_alt_entry(struct elf *elf, struct special_entry *entry,
> * find paths that see the STAC but take the NOP instead of
> * CLAC and the other way around.
> */
> - if (feature == X86_FEATURE_SMAP) {
> - if (uaccess)
> - alt->skip_orig = true;
> - else
> - alt->skip_alt = true;
> - }
> + arch_force_alt_path(feature, uaccess, alt);
Instead of arch_force_alt_path(), maybe it could be something like:
if (arch_is_uaccess_feature(alt))
if (uaccess)
alt->skip_orig = true;
else
alt->skip_alt = true;
That helps keep the common bits common, and even better it makes the
code clearer.
> }
>
> orig_rela = find_rela_by_dest(sec, offset + entry->orig);
> diff --git a/tools/objtool/special.h b/tools/objtool/special.h
> index 35061530e46e..90626a7e41cf 100644
> --- a/tools/objtool/special.h
> +++ b/tools/objtool/special.h
> @@ -27,5 +27,8 @@ struct special_alt {
> };
>
> int special_get_alts(struct elf *elf, struct list_head *alts);
> +void arch_force_alt_path(unsigned short feature,
> + bool uaccess,
> + struct special_alt *alt);
>
> #endif /* _SPECIAL_H */
> --
> 2.17.1
>
--
Josh
Hi Raphaël,
On 16/08/19 13:23, Raphael Gault wrote:
> This patch abstracts the few architecture dependent tests that are
> perform when handling special section and switch tables. It enables any
> architecture to ignore a particular CPU feature or not to handle switch
> tables.
I think it would be better if this patch only focused on CPU features
and leave dealing with switch table to subsequent patches.
>
> Signed-off-by: Raphael Gault <[email protected]>
> ---
> tools/objtool/arch/arm64/Build | 1 +
> tools/objtool/arch/arm64/arch_special.c | 22 +++++++++++++++
> .../objtool/arch/arm64/include/arch_special.h | 10 +++++--
> tools/objtool/arch/x86/Build | 1 +
> tools/objtool/arch/x86/arch_special.c | 28 +++++++++++++++++++
> tools/objtool/arch/x86/include/arch_special.h | 9 ++++++
> tools/objtool/check.c | 24 ++++++++++++++--
> tools/objtool/special.c | 9 ++----
> tools/objtool/special.h | 3 ++
> 9 files changed, 96 insertions(+), 11 deletions(-)
> create mode 100644 tools/objtool/arch/arm64/arch_special.c
> create mode 100644 tools/objtool/arch/x86/arch_special.c
>
> diff --git a/tools/objtool/arch/arm64/Build b/tools/objtool/arch/arm64/Build
> index bf7a32c2b9e9..3d09be745a84 100644
> --- a/tools/objtool/arch/arm64/Build
> +++ b/tools/objtool/arch/arm64/Build
> @@ -1,3 +1,4 @@
> +objtool-y += arch_special.o
> objtool-y += decode.o
> objtool-y += orc_dump.o
> objtool-y += orc_gen.o
> diff --git a/tools/objtool/arch/arm64/arch_special.c b/tools/objtool/arch/arm64/arch_special.c
> new file mode 100644
> index 000000000000..a21d28876317
> --- /dev/null
> +++ b/tools/objtool/arch/arm64/arch_special.c
> @@ -0,0 +1,22 @@
> +/*
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +#include "../../special.h"
> +#include "arch_special.h"
> +
> +void arch_force_alt_path(unsigned short feature,
> + bool uaccess,
> + struct special_alt *alt)
> +{
> +}
> diff --git a/tools/objtool/arch/arm64/include/arch_special.h b/tools/objtool/arch/arm64/include/arch_special.h
> index 63da775d0581..185103be8a51 100644
> --- a/tools/objtool/arch/arm64/include/arch_special.h
> +++ b/tools/objtool/arch/arm64/include/arch_special.h
> @@ -30,7 +30,13 @@
> #define ALT_ORIG_LEN_OFFSET 10
> #define ALT_NEW_LEN_OFFSET 11
>
> -#define X86_FEATURE_POPCNT (4 * 32 + 23)
> -#define X86_FEATURE_SMAP (9 * 32 + 20)
> +static inline bool arch_should_ignore_feature(unsigned short feature)
> +{
> + return false;
> +}
>
> +static inline bool arch_support_switch_table(void)
> +{
> + return false;
If I understand correctly , this gets later (patch 8) replaced by
arch_find_switch_table() which can return NULL if unable to find a
switch table.
So, is it necessary to introduce this function at this stage of the
patchset? Can't we just have directly arch_find_switch_table() ?
Also, I believe that in the end you keep the two
arch_support_switch_table() implementations (x86 and arm64) despite
getting rid of the only caller (in patch 8).
> +}
> #endif /* _ARM64_ARCH_SPECIAL_H */
> diff --git a/tools/objtool/arch/x86/Build b/tools/objtool/arch/x86/Build
> index 1f11b45999d0..63e167775bc8 100644
> --- a/tools/objtool/arch/x86/Build
> +++ b/tools/objtool/arch/x86/Build
> @@ -1,3 +1,4 @@
> +objtool-y += arch_special.o
> objtool-y += decode.o
> objtool-y += orc_dump.o
> objtool-y += orc_gen.o
> diff --git a/tools/objtool/arch/x86/arch_special.c b/tools/objtool/arch/x86/arch_special.c
> new file mode 100644
> index 000000000000..6583a1770bb2
> --- /dev/null
> +++ b/tools/objtool/arch/x86/arch_special.c
> @@ -0,0 +1,28 @@
> +/*
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +#include "../../special.h"
> +#include "arch_special.h"
> +
> +void arch_force_alt_path(unsigned short feature,
> + bool uaccess,
Maybe you could have something like:
void arch_select_alt_path(unsigned short feature, struct special_alt *alt);
or arch_process_alt_inst(...);
This way you just let the arch decide what to do for alternative
instructions associated with their features, and call it in the "if
(entry->feature)" branch of get_alt_entry().
You don't need to pass the "uaccess" as that info is globally accessible
from builtin.h.
> + struct special_alt *alt)
> +{
> + if (feature == X86_FEATURE_SMAP) {
> + if (uaccess)
> + alt->skip_orig = true;
> + else
> + alt->skip_alt = true;
> + }
> +}
> diff --git a/tools/objtool/arch/x86/include/arch_special.h b/tools/objtool/arch/x86/include/arch_special.h
> index 424ce47013e3..fce2b1193194 100644
> --- a/tools/objtool/arch/x86/include/arch_special.h
> +++ b/tools/objtool/arch/x86/include/arch_special.h
> @@ -33,4 +33,13 @@
> #define X86_FEATURE_POPCNT (4 * 32 + 23)
> #define X86_FEATURE_SMAP (9 * 32 + 20)
>
> +static inline bool arch_should_ignore_feature(unsigned short feature)
> +{
> + return feature == X86_FEATURE_POPCNT;
> +}
With my above suggestion you shouldn't need that and just use the arch
specific alternative handling set the alt->skip_orig.
> +
> +static inline bool arch_support_switch_table(void)
> +{
> + return true;
> +}
> #endif /* _X86_ARCH_SPECIAL_H */
> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index 30e147391dcb..4af6422d3428 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -729,7 +729,7 @@ static int handle_group_alt(struct objtool_file *file,
> last_orig_insn = insn;
> }
>
> - if (next_insn_same_sec(file, last_orig_insn)) {
> + if (last_orig_insn && next_insn_same_sec(file, last_orig_insn)) {
> fake_jump = malloc(sizeof(*fake_jump));
> if (!fake_jump) {
> WARN("malloc failed");
> @@ -1061,6 +1061,26 @@ static struct rela *find_jump_table(struct objtool_file *file,
> table_rela = find_rela_by_dest(table_sec, table_offset);
> if (!table_rela)
> continue;
> + /*
> + * If we are on arm64 architecture, we now that we
> + * are in presence of a switch table thanks to
> + * the `br <Xn>` insn. but we can't retrieve it yet.
> + * So we just ignore unreachable for this file.
> + */
> + if (!arch_support_switch_table()) {
> + file->ignore_unreachables = true;
> + return NULL;
> + }
> +
> + rodata_rela = find_rela_by_dest(rodata_sec, table_offset);
> + if (rodata_rela) {
> + /*
> + * Use of RIP-relative switch jumps is quite rare, and
> + * indicates a rare GCC quirk/bug which can leave dead
> + * code behind.
> + */
> + if (text_rela->type == R_X86_64_PC32)
> + file->ignore_unreachables = true;
>
> /*
> * Use of RIP-relative switch jumps is quite rare, and
> @@ -1864,7 +1884,7 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
> insn = first;
> sec = insn->sec;
>
> - if (insn->alt_group && list_empty(&insn->alts)) {
> + if (!insn->visited && insn->alt_group && list_empty(&insn->alts)) {
> WARN_FUNC("don't know how to handle branch to middle of alternative instruction group",
> sec, insn->offset);
> return 1;
> diff --git a/tools/objtool/special.c b/tools/objtool/special.c
> index b8ccee1b5382..7a0092d6e5b3 100644
> --- a/tools/objtool/special.c
> +++ b/tools/objtool/special.c
> @@ -81,7 +81,7 @@ static int get_alt_entry(struct elf *elf, struct special_entry *entry,
> * feature path which is a "very very small percentage of
> * machines".
> */
> - if (feature == X86_FEATURE_POPCNT)
> + if (arch_should_ignore_feature(feature))
> alt->skip_orig = true;
>
> /*
> @@ -93,12 +93,7 @@ static int get_alt_entry(struct elf *elf, struct special_entry *entry,
> * find paths that see the STAC but take the NOP instead of
> * CLAC and the other way around.
> */
> - if (feature == X86_FEATURE_SMAP) {
> - if (uaccess)
> - alt->skip_orig = true;
> - else
> - alt->skip_alt = true;
> - }
> + arch_force_alt_path(feature, uaccess, alt);
> }
>
> orig_rela = find_rela_by_dest(sec, offset + entry->orig);
> diff --git a/tools/objtool/special.h b/tools/objtool/special.h
> index 35061530e46e..90626a7e41cf 100644
> --- a/tools/objtool/special.h
> +++ b/tools/objtool/special.h
> @@ -27,5 +27,8 @@ struct special_alt {
> };
>
> int special_get_alts(struct elf *elf, struct list_head *alts);
> +void arch_force_alt_path(unsigned short feature,
> + bool uaccess,
> + struct special_alt *alt);
>
> #endif /* _SPECIAL_H */
>
CHeers,
--
Julien Thierry