linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* objtool crashes with some clang produced .o files
@ 2020-12-03 13:56 Arnd Bergmann
  2020-12-10 23:35 ` Nick Desaulniers
  0 siblings, 1 reply; 11+ messages in thread
From: Arnd Bergmann @ 2020-12-03 13:56 UTC (permalink / raw)
  To: Josh Poimboeuf, Peter Zijlstra, Linux Kernel Mailing List,
	clang-built-linux

[-- Attachment #1: Type: text/plain, Size: 1233 bytes --]

I see occasional randconfig builds failing on x86 with clang-11
and clang-12 when objtool crashes with a segmentation fault.

The simplest test case I managed to create is

$ echo "__SCK__tp_func_cdev_update() { __SCT__tp_func_cdev_update(); }" > file.c
$ clang-12 -c file.c -O2 -fno-asynchronous-unwind-tables
$ ./tools/objtool/objtool orc generate  file.o
Segmentation fault (core dumped)
$ clang-12 -S file.c -O2 -fno-asynchronous-unwind-tables -o-
.text
.file "file.c"
.globl __SCK__tp_func_cdev_update      # -- Begin function
__SCK__tp_func_cdev_update
.p2align 4, 0x90
.type __SCK__tp_func_cdev_update,@function
__SCK__tp_func_cdev_update:             # @__SCK__tp_func_cdev_update
# %bb.0:
xorl %eax, %eax
jmp __SCT__tp_func_cdev_update      # TAILCALL
.Lfunc_end0:
.size __SCK__tp_func_cdev_update, .Lfunc_end0-__SCK__tp_func_cdev_update
                                        # -- End function
.ident "Ubuntu clang version
12.0.0-++20201129052612+ce134da4b18-1~exp1~20201129163253.238"
.section ".note.GNU-stack","",@progbits
.addrsig

The behavior seems to depend on the specific symbol names, and it only happens
for the integrated assembler, not the GNU assembler.

Attaching both .o files for reference.

        Arnd

[-- Attachment #2: integrated-as.o --]
[-- Type: application/x-object, Size: 920 bytes --]

[-- Attachment #3: gnu-as.o --]
[-- Type: application/x-object, Size: 1184 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: objtool crashes with some clang produced .o files
  2020-12-03 13:56 objtool crashes with some clang produced .o files Arnd Bergmann
@ 2020-12-10 23:35 ` Nick Desaulniers
  2020-12-11  8:45   ` Peter Zijlstra
  2020-12-11  9:32   ` Peter Zijlstra
  0 siblings, 2 replies; 11+ messages in thread
From: Nick Desaulniers @ 2020-12-10 23:35 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Josh Poimboeuf, Peter Zijlstra, Linux Kernel Mailing List,
	clang-built-linux

On Thu, Dec 3, 2020 at 5:56 AM Arnd Bergmann <arnd@kernel.org> wrote:
>
> I see occasional randconfig builds failing on x86 with clang-11
> and clang-12 when objtool crashes with a segmentation fault.
>
> The simplest test case I managed to create is
>
> $ echo "__SCK__tp_func_cdev_update() { __SCT__tp_func_cdev_update(); }" > file.c
> $ clang-12 -c file.c -O2 -fno-asynchronous-unwind-tables
> $ ./tools/objtool/objtool orc generate  file.o
> Segmentation fault (core dumped)
> $ clang-12 -S file.c -O2 -fno-asynchronous-unwind-tables -o-
> .text
> .file "file.c"
> .globl __SCK__tp_func_cdev_update      # -- Begin function
> __SCK__tp_func_cdev_update
> .p2align 4, 0x90
> .type __SCK__tp_func_cdev_update,@function
> __SCK__tp_func_cdev_update:             # @__SCK__tp_func_cdev_update
> # %bb.0:
> xorl %eax, %eax
> jmp __SCT__tp_func_cdev_update      # TAILCALL
> .Lfunc_end0:
> .size __SCK__tp_func_cdev_update, .Lfunc_end0-__SCK__tp_func_cdev_update
>                                         # -- End function
> .ident "Ubuntu clang version
> 12.0.0-++20201129052612+ce134da4b18-1~exp1~20201129163253.238"
> .section ".note.GNU-stack","",@progbits
> .addrsig
>
> The behavior seems to depend on the specific symbol names, and it only happens
> for the integrated assembler, not the GNU assembler.
>
> Attaching both .o files for reference.

Thanks for the report.

(gdb) r orc generate file.o
Starting program: /android0/linux-next/tools/objtool/objtool orc generate file.o

Program received signal SIGSEGV, Segmentation fault.
0x000000000040d7d3 in elf_rebuild_rela_reloc_section (sec=0xc24e30,
nr=<optimized out>) at elf.c:880
880                     relocs[idx].r_info =
GELF_R_INFO(reloc->sym->idx, reloc->type);
(gdb) bt
#0  0x000000000040d7d3 in elf_rebuild_rela_reloc_section
(sec=0xc24e30, nr=<optimized out>) at elf.c:880
#1  elf_rebuild_reloc_section (elf=<optimized out>,
sec=sec@entry=0xc24e30) at elf.c:901
#2  0x00000000004049b6 in create_static_call_sections (file=0x41f478
<file>) at check.c:520
#3  check (file=0x41f478 <file>) at check.c:2918
#4  0x000000000040b97c in cmd_orc (argc=<optimized out>,
argv=0x7fffffffda98) at builtin-orc.c:47
#5  0x000000000040dce9 in handle_internal_command (argc=argc@entry=3,
argv=argv@entry=0x7fffffffda90) at objtool.c:128
#6  0x000000000040dbff in main (argc=3, argv=0x7fffffffda90) at objtool.c:151
(gdb) p reloc
$1 = (struct reloc *) 0xc24fd0
(gdb) p *reloc
$2 = {list = {next = 0xc25070, prev = 0xc24eb8}, hash = {next = 0x0,
pprev = 0xc25080}, {rela = {r_offset = 0, r_info = 0, r_addend = 0},
rel = {r_offset = 0, r_info = 0}},
  sec = 0xc24e30, sym = 0x0, offset = 0, type = 2, addend = 2, idx =
0, jump_table_start = false}

So reloc->sym was NULL.

(gdb) p *sec
$2 = {list = {next = 0x7ffff559e068, prev = 0xc23bf0}, hash = {next =
0x0, pprev = 0x7ffff65d60d8}, name_hash = {next = 0x0, pprev =
0x7ffff6dd6888}, sh = {sh_name = 147, sh_type = 4,
    sh_flags = 64, sh_addr = 0, sh_offset = 0, sh_size = 48, sh_link =
7, sh_info = 8, sh_addralign = 8, sh_entsize = 24}, symbol_tree =
{rb_node = 0x0}, symbol_list = {next = 0xc24ea8,
    prev = 0xc24ea8}, reloc_list = {next = 0xc24fd0, prev = 0xc25070},
base = 0xc23bf0, reloc = 0x0, sym = 0x0, data = 0xc23db0, name =
0xc24f60 ".rela.static_call_sites", idx = 9,
  len = 0, changed = true, text = false, rodata = false, noinstr = false}

So .rela.static_call_sites was the problematic section.

(gdb) b tools/objtool/check.c:475
(gdb) r orc generate file.o
Breakpoint 1, create_static_call_sections (file=0x41f478 <file>) at check.c:475
475                     reloc->sym = insn->sec->sym;
(gdb) p insn->sec->sym
$4 = (struct symbol *) 0x0
(gdb) p insn->sec->name
$5 = 0xc22d26 ".text"

So some instruction in .text that contained a relocation for, we could
not determine a symbol for?  I'm curious why we're even in this loop
though, since we didn't do anything related to static calls...is
`file->static_call_list` not populated correctly? Is the final else
case from `add_jump_destinations` perhaps being hit and adding nodes
to file->static_call_list incorrectly?
-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: objtool crashes with some clang produced .o files
  2020-12-10 23:35 ` Nick Desaulniers
@ 2020-12-11  8:45   ` Peter Zijlstra
  2020-12-11  9:32   ` Peter Zijlstra
  1 sibling, 0 replies; 11+ messages in thread
From: Peter Zijlstra @ 2020-12-11  8:45 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Arnd Bergmann, Josh Poimboeuf, Linux Kernel Mailing List,
	clang-built-linux

On Thu, Dec 10, 2020 at 03:35:45PM -0800, Nick Desaulniers wrote:
> On Thu, Dec 3, 2020 at 5:56 AM Arnd Bergmann <arnd@kernel.org> wrote:
> >
> > I see occasional randconfig builds failing on x86 with clang-11
> > and clang-12 when objtool crashes with a segmentation fault.
> >
> > The simplest test case I managed to create is
> >
> > $ echo "__SCK__tp_func_cdev_update() { __SCT__tp_func_cdev_update(); }" > file.c

> So some instruction in .text that contained a relocation for, we could
> not determine a symbol for?  I'm curious why we're even in this loop
> though, since we didn't do anything related to static calls...


No you did, you called a __SCT*() function, which is a
static-call-trampoline. objtool does indeed assume it then has a symbol
for the matching key, which should be guaranteed by the __ADDRESSABLE()
in __static_call().

From linux/static_call.h:

/*
 * __ADDRESSABLE() is used to ensure the key symbol doesn't get stripped from
 * the symbol table so that objtool can reference it when it generates the
 * .static_call_sites section.
 */
#define __static_call(name)						\
({									\
	__ADDRESSABLE(STATIC_CALL_KEY(name));				\
	&STATIC_CALL_TRAMP(name);					\
})


Let me go find a copy of clang-11..

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: objtool crashes with some clang produced .o files
  2020-12-10 23:35 ` Nick Desaulniers
  2020-12-11  8:45   ` Peter Zijlstra
@ 2020-12-11  9:32   ` Peter Zijlstra
  2020-12-11 16:37     ` Josh Poimboeuf
  1 sibling, 1 reply; 11+ messages in thread
From: Peter Zijlstra @ 2020-12-11  9:32 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Arnd Bergmann, Josh Poimboeuf, Linux Kernel Mailing List,
	clang-built-linux

On Thu, Dec 10, 2020 at 03:35:45PM -0800, Nick Desaulniers wrote:

> > $ echo "__SCK__tp_func_cdev_update() { __SCT__tp_func_cdev_update(); }" > file.c

> > $ clang-12 -S file.c -O2 -fno-asynchronous-unwind-tables -o-
> > .text
> > .file "file.c"
> > .globl __SCK__tp_func_cdev_update      # -- Begin function
> > __SCK__tp_func_cdev_update
> > .p2align 4, 0x90
> > .type __SCK__tp_func_cdev_update,@function
> > __SCK__tp_func_cdev_update:             # @__SCK__tp_func_cdev_update
> > # %bb.0:
> > xorl %eax, %eax
> > jmp __SCT__tp_func_cdev_update      # TAILCALL
> > .Lfunc_end0:
> > .size __SCK__tp_func_cdev_update, .Lfunc_end0-__SCK__tp_func_cdev_update
> >                                         # -- End function
> > .ident "Ubuntu clang version
> > 12.0.0-++20201129052612+ce134da4b18-1~exp1~20201129163253.238"
> > .section ".note.GNU-stack","",@progbits
> > .addrsig

> (gdb) b tools/objtool/check.c:475
> (gdb) r orc generate file.o
> Breakpoint 1, create_static_call_sections (file=0x41f478 <file>) at check.c:475
> 475                     reloc->sym = insn->sec->sym;
> (gdb) p insn->sec->sym
> $4 = (struct symbol *) 0x0
> (gdb) p insn->sec->name
> $5 = 0xc22d26 ".text"
> 
> So some instruction in .text that contained a relocation for, we could
> not determine a symbol for?

Ooh, cute that's weird. So it's trying to write a relocation to the
site that does (IOW fill static_call_site::addr):

  CALL/JMP __SCT*

and that's failing, because the instruction's section doesn't have a
symbol.

Both my GCC and clang-11 get me:

(gdb) p insn->sec->sym
$1 = (struct symbol *) 0x555555d83a70
(gdb) p insn->sec->sym->name
$2 = 0x555555d82f46 ".text"

Looking at elf.c, it seems we're missing an STT_SECTION symbol for
.text.

And indeed, when I add -fno-asynchronous-unwind-tables to clang-11, that
goes missing from the readelf .symtab listing. Help ?!

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: objtool crashes with some clang produced .o files
  2020-12-11  9:32   ` Peter Zijlstra
@ 2020-12-11 16:37     ` Josh Poimboeuf
  2020-12-11 16:49       ` Peter Zijlstra
  0 siblings, 1 reply; 11+ messages in thread
From: Josh Poimboeuf @ 2020-12-11 16:37 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Nick Desaulniers, Arnd Bergmann, Linux Kernel Mailing List,
	clang-built-linux

On Fri, Dec 11, 2020 at 10:32:05AM +0100, Peter Zijlstra wrote:
> Looking at elf.c, it seems we're missing an STT_SECTION symbol for
> .text.
> 
> And indeed, when I add -fno-asynchronous-unwind-tables to clang-11, that
> goes missing from the readelf .symtab listing. Help ?!

I had a similar problem with ORC relocations:

  e81e07244325 ("objtool: Support Clang non-section symbols in ORC generation")

If Clang strips the section symbol then we have to find the function
symbol instead.

Does this fix it?

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index c6ab44543c92..9bc18864154f 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -472,8 +472,25 @@ static int create_static_call_sections(struct objtool_file *file)
 			return -1;
 		}
 		memset(reloc, 0, sizeof(*reloc));
-		reloc->sym = insn->sec->sym;
-		reloc->addend = insn->offset;
+
+		if (insn->sec->sym) {
+			reloc->sym = insn->sec->sym;
+			reloc->addend = insn->offset;
+		} else {
+			reloc->sym = find_symbol_containing(insn->sec, insn->offset);
+			if (!reloc->sym) {
+				WARN_FUNC("can't create static call: missing containing symbol",
+					  insn->sec, insn->offset);
+				return -1;
+			}
+
+			reloc->addend = insn->offset - reloc->sym->offset;
+		}
+
 		reloc->type = R_X86_64_PC32;
 		reloc->offset = idx * sizeof(struct static_call_site);
 		reloc->sec = reloc_sec;


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: objtool crashes with some clang produced .o files
  2020-12-11 16:37     ` Josh Poimboeuf
@ 2020-12-11 16:49       ` Peter Zijlstra
  2020-12-11 17:46         ` Josh Poimboeuf
  0 siblings, 1 reply; 11+ messages in thread
From: Peter Zijlstra @ 2020-12-11 16:49 UTC (permalink / raw)
  To: Josh Poimboeuf
  Cc: Nick Desaulniers, Arnd Bergmann, Linux Kernel Mailing List,
	clang-built-linux

On Fri, Dec 11, 2020 at 10:37:48AM -0600, Josh Poimboeuf wrote:
> On Fri, Dec 11, 2020 at 10:32:05AM +0100, Peter Zijlstra wrote:
> > Looking at elf.c, it seems we're missing an STT_SECTION symbol for
> > .text.
> > 
> > And indeed, when I add -fno-asynchronous-unwind-tables to clang-11, that
> > goes missing from the readelf .symtab listing. Help ?!
> 
> I had a similar problem with ORC relocations:
> 
>   e81e07244325 ("objtool: Support Clang non-section symbols in ORC generation")
> 
> If Clang strips the section symbol then we have to find the function
> symbol instead.

Argh, I knew I'd seen something like that before, but I couldn't find it :-/

> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index c6ab44543c92..9bc18864154f 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -472,8 +472,25 @@ static int create_static_call_sections(struct objtool_file *file)
>  			return -1;
>  		}
>  		memset(reloc, 0, sizeof(*reloc));
> -		reloc->sym = insn->sec->sym;
> -		reloc->addend = insn->offset;
> +
> +		if (insn->sec->sym) {
> +			reloc->sym = insn->sec->sym;
> +			reloc->addend = insn->offset;
> +		} else {
> +			reloc->sym = find_symbol_containing(insn->sec, insn->offset);
> +			if (!reloc->sym) {
> +				WARN_FUNC("can't create static call: missing containing symbol",
> +					  insn->sec, insn->offset);
> +				return -1;
> +			}
> +
> +			reloc->addend = insn->offset - reloc->sym->offset;
> +		}
> +

Do we want to capture all that gunk in something like
elf_reloc_to_insn(reloc, insn) instead of duplicating the magic?

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: objtool crashes with some clang produced .o files
  2020-12-11 16:49       ` Peter Zijlstra
@ 2020-12-11 17:46         ` Josh Poimboeuf
  2020-12-11 20:57           ` Nick Desaulniers
  0 siblings, 1 reply; 11+ messages in thread
From: Josh Poimboeuf @ 2020-12-11 17:46 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Nick Desaulniers, Arnd Bergmann, Linux Kernel Mailing List,
	clang-built-linux

On Fri, Dec 11, 2020 at 05:49:15PM +0100, Peter Zijlstra wrote:
> Do we want to capture all that gunk in something like
> elf_reloc_to_insn(reloc, insn) instead of duplicating the magic?

Yup, here's an actual patch

From: Josh Poimboeuf <jpoimboe@redhat.com>
Subject: [PATCH] objtool: Support Clang non-section symbols in static call generation

The Clang assembler likes to strip section symbols, which means you
can't reference some text code by its section.  This confuses objtool
greatly, causing it to seg fault.

The fix is similar to what was done before, for ORC reloc generation:

  e81e07244325 ("objtool: Support Clang non-section symbols in ORC generation")

Factor out that code into a common helper and use it for static call
reloc generation as well.

Reported-by: Arnd Bergmann <arnd@kernel.org>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 tools/objtool/check.c   | 11 +++++++++--
 tools/objtool/elf.c     | 26 ++++++++++++++++++++++++++
 tools/objtool/elf.h     |  2 ++
 tools/objtool/orc_gen.c | 29 +++++------------------------
 4 files changed, 42 insertions(+), 26 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index c6ab44543c92..5f8d3eed78a1 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -467,13 +467,20 @@ static int create_static_call_sections(struct objtool_file *file)
 
 		/* populate reloc for 'addr' */
 		reloc = malloc(sizeof(*reloc));
+
 		if (!reloc) {
 			perror("malloc");
 			return -1;
 		}
 		memset(reloc, 0, sizeof(*reloc));
-		reloc->sym = insn->sec->sym;
-		reloc->addend = insn->offset;
+
+		insn_to_reloc_sym_addend(insn->sec, insn->offset, reloc);
+		if (!reloc->sym) {
+			WARN_FUNC("static call tramp: missing containing symbol",
+				  insn->sec, insn->offset);
+			return -1;
+		}
+
 		reloc->type = R_X86_64_PC32;
 		reloc->offset = idx * sizeof(struct static_call_site);
 		reloc->sec = reloc_sec;
diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
index 4e1d7460574b..be89c741ba9a 100644
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -262,6 +262,32 @@ struct reloc *find_reloc_by_dest(const struct elf *elf, struct section *sec, uns
 	return find_reloc_by_dest_range(elf, sec, offset, 1);
 }
 
+void insn_to_reloc_sym_addend(struct section *sec, unsigned long offset,
+			      struct reloc *reloc)
+{
+	if (sec->sym) {
+		reloc->sym = sec->sym;
+		reloc->addend = offset;
+		return;
+	}
+
+	/*
+	 * The Clang assembler strips section symbols, so we have to reference
+	 * the function symbol instead:
+	 */
+	reloc->sym = find_symbol_containing(sec, offset);
+	if (!reloc->sym) {
+		/*
+		 * Hack alert.  This happens when we need to reference the NOP
+		 * pad insn immediately after the function.
+		 */
+		reloc->sym = find_symbol_containing(sec, offset - 1);
+	}
+
+	if (reloc->sym)
+		reloc->addend = offset - reloc->sym->offset;
+}
+
 static int read_sections(struct elf *elf)
 {
 	Elf_Scn *s = NULL;
diff --git a/tools/objtool/elf.h b/tools/objtool/elf.h
index 807f8c670097..e6890cc70a25 100644
--- a/tools/objtool/elf.h
+++ b/tools/objtool/elf.h
@@ -140,6 +140,8 @@ struct reloc *find_reloc_by_dest(const struct elf *elf, struct section *sec, uns
 struct reloc *find_reloc_by_dest_range(const struct elf *elf, struct section *sec,
 				     unsigned long offset, unsigned int len);
 struct symbol *find_func_containing(struct section *sec, unsigned long offset);
+void insn_to_reloc_sym_addend(struct section *sec, unsigned long offset,
+			      struct reloc *reloc);
 int elf_rebuild_reloc_section(struct elf *elf, struct section *sec);
 
 #define for_each_sec(file, sec)						\
diff --git a/tools/objtool/orc_gen.c b/tools/objtool/orc_gen.c
index 235663b96adc..9ce68b385a1b 100644
--- a/tools/objtool/orc_gen.c
+++ b/tools/objtool/orc_gen.c
@@ -105,30 +105,11 @@ static int create_orc_entry(struct elf *elf, struct section *u_sec, struct secti
 	}
 	memset(reloc, 0, sizeof(*reloc));
 
-	if (insn_sec->sym) {
-		reloc->sym = insn_sec->sym;
-		reloc->addend = insn_off;
-	} else {
-		/*
-		 * The Clang assembler doesn't produce section symbols, so we
-		 * have to reference the function symbol instead:
-		 */
-		reloc->sym = find_symbol_containing(insn_sec, insn_off);
-		if (!reloc->sym) {
-			/*
-			 * Hack alert.  This happens when we need to reference
-			 * the NOP pad insn immediately after the function.
-			 */
-			reloc->sym = find_symbol_containing(insn_sec,
-							   insn_off - 1);
-		}
-		if (!reloc->sym) {
-			WARN("missing symbol for insn at offset 0x%lx\n",
-			     insn_off);
-			return -1;
-		}
-
-		reloc->addend = insn_off - reloc->sym->offset;
+	insn_to_reloc_sym_addend(insn_sec, insn_off, reloc);
+	if (!reloc->sym) {
+		WARN("missing symbol for insn at offset 0x%lx",
+		     insn_off);
+		return -1;
 	}
 
 	reloc->type = R_X86_64_PC32;
-- 
2.25.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: objtool crashes with some clang produced .o files
  2020-12-11 17:46         ` Josh Poimboeuf
@ 2020-12-11 20:57           ` Nick Desaulniers
  2020-12-12  0:42             ` Nick Desaulniers
  0 siblings, 1 reply; 11+ messages in thread
From: Nick Desaulniers @ 2020-12-11 20:57 UTC (permalink / raw)
  To: Josh Poimboeuf
  Cc: Peter Zijlstra, Arnd Bergmann, Linux Kernel Mailing List,
	clang-built-linux

On Fri, Dec 11, 2020 at 9:46 AM Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>
> On Fri, Dec 11, 2020 at 05:49:15PM +0100, Peter Zijlstra wrote:
> > Do we want to capture all that gunk in something like
> > elf_reloc_to_insn(reloc, insn) instead of duplicating the magic?
>
> Yup, here's an actual patch
>
> From: Josh Poimboeuf <jpoimboe@redhat.com>
> Subject: [PATCH] objtool: Support Clang non-section symbols in static call generation
>
> The Clang assembler likes to strip section symbols, which means you
> can't reference some text code by its section.  This confuses objtool
> greatly, causing it to seg fault.
>
> The fix is similar to what was done before, for ORC reloc generation:
>
>   e81e07244325 ("objtool: Support Clang non-section symbols in ORC generation")
>
> Factor out that code into a common helper and use it for static call
> reloc generation as well.
>
> Reported-by: Arnd Bergmann <arnd@kernel.org>
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>

Thanks for the patch!

Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Link: https://github.com/ClangBuiltLinux/linux/issues/1207

> ---
>  tools/objtool/check.c   | 11 +++++++++--
>  tools/objtool/elf.c     | 26 ++++++++++++++++++++++++++
>  tools/objtool/elf.h     |  2 ++
>  tools/objtool/orc_gen.c | 29 +++++------------------------
>  4 files changed, 42 insertions(+), 26 deletions(-)
>
> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index c6ab44543c92..5f8d3eed78a1 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -467,13 +467,20 @@ static int create_static_call_sections(struct objtool_file *file)
>
>                 /* populate reloc for 'addr' */
>                 reloc = malloc(sizeof(*reloc));
> +
>                 if (!reloc) {
>                         perror("malloc");
>                         return -1;
>                 }
>                 memset(reloc, 0, sizeof(*reloc));
> -               reloc->sym = insn->sec->sym;
> -               reloc->addend = insn->offset;
> +
> +               insn_to_reloc_sym_addend(insn->sec, insn->offset, reloc);
> +               if (!reloc->sym) {
> +                       WARN_FUNC("static call tramp: missing containing symbol",
> +                                 insn->sec, insn->offset);
> +                       return -1;
> +               }
> +
>                 reloc->type = R_X86_64_PC32;
>                 reloc->offset = idx * sizeof(struct static_call_site);
>                 reloc->sec = reloc_sec;
> diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
> index 4e1d7460574b..be89c741ba9a 100644
> --- a/tools/objtool/elf.c
> +++ b/tools/objtool/elf.c
> @@ -262,6 +262,32 @@ struct reloc *find_reloc_by_dest(const struct elf *elf, struct section *sec, uns
>         return find_reloc_by_dest_range(elf, sec, offset, 1);
>  }
>
> +void insn_to_reloc_sym_addend(struct section *sec, unsigned long offset,
> +                             struct reloc *reloc)
> +{
> +       if (sec->sym) {
> +               reloc->sym = sec->sym;
> +               reloc->addend = offset;
> +               return;
> +       }
> +
> +       /*
> +        * The Clang assembler strips section symbols, so we have to reference
> +        * the function symbol instead:
> +        */
> +       reloc->sym = find_symbol_containing(sec, offset);
> +       if (!reloc->sym) {
> +               /*
> +                * Hack alert.  This happens when we need to reference the NOP
> +                * pad insn immediately after the function.
> +                */
> +               reloc->sym = find_symbol_containing(sec, offset - 1);
> +       }
> +
> +       if (reloc->sym)
> +               reloc->addend = offset - reloc->sym->offset;
> +}
> +
>  static int read_sections(struct elf *elf)
>  {
>         Elf_Scn *s = NULL;
> diff --git a/tools/objtool/elf.h b/tools/objtool/elf.h
> index 807f8c670097..e6890cc70a25 100644
> --- a/tools/objtool/elf.h
> +++ b/tools/objtool/elf.h
> @@ -140,6 +140,8 @@ struct reloc *find_reloc_by_dest(const struct elf *elf, struct section *sec, uns
>  struct reloc *find_reloc_by_dest_range(const struct elf *elf, struct section *sec,
>                                      unsigned long offset, unsigned int len);
>  struct symbol *find_func_containing(struct section *sec, unsigned long offset);
> +void insn_to_reloc_sym_addend(struct section *sec, unsigned long offset,
> +                             struct reloc *reloc);
>  int elf_rebuild_reloc_section(struct elf *elf, struct section *sec);
>
>  #define for_each_sec(file, sec)                                                \
> diff --git a/tools/objtool/orc_gen.c b/tools/objtool/orc_gen.c
> index 235663b96adc..9ce68b385a1b 100644
> --- a/tools/objtool/orc_gen.c
> +++ b/tools/objtool/orc_gen.c
> @@ -105,30 +105,11 @@ static int create_orc_entry(struct elf *elf, struct section *u_sec, struct secti
>         }
>         memset(reloc, 0, sizeof(*reloc));
>
> -       if (insn_sec->sym) {
> -               reloc->sym = insn_sec->sym;
> -               reloc->addend = insn_off;
> -       } else {
> -               /*
> -                * The Clang assembler doesn't produce section symbols, so we
> -                * have to reference the function symbol instead:
> -                */
> -               reloc->sym = find_symbol_containing(insn_sec, insn_off);
> -               if (!reloc->sym) {
> -                       /*
> -                        * Hack alert.  This happens when we need to reference
> -                        * the NOP pad insn immediately after the function.
> -                        */
> -                       reloc->sym = find_symbol_containing(insn_sec,
> -                                                          insn_off - 1);
> -               }
> -               if (!reloc->sym) {
> -                       WARN("missing symbol for insn at offset 0x%lx\n",
> -                            insn_off);
> -                       return -1;
> -               }
> -
> -               reloc->addend = insn_off - reloc->sym->offset;
> +       insn_to_reloc_sym_addend(insn_sec, insn_off, reloc);
> +       if (!reloc->sym) {
> +               WARN("missing symbol for insn at offset 0x%lx",
> +                    insn_off);
> +               return -1;
>         }
>
>         reloc->type = R_X86_64_PC32;
> --
> 2.25.4
>
> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20201211174610.2bfprpvrrlg66awd%40treble.



-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: objtool crashes with some clang produced .o files
  2020-12-11 20:57           ` Nick Desaulniers
@ 2020-12-12  0:42             ` Nick Desaulniers
  2020-12-12  0:47               ` Nick Desaulniers
  2020-12-14 12:49               ` Arnd Bergmann
  0 siblings, 2 replies; 11+ messages in thread
From: Nick Desaulniers @ 2020-12-12  0:42 UTC (permalink / raw)
  To: Josh Poimboeuf, Peter Zijlstra
  Cc: Arnd Bergmann, Linux Kernel Mailing List, clang-built-linux

[-- Attachment #1: Type: text/plain, Size: 1307 bytes --]

On Fri, Dec 11, 2020 at 12:57 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> Thanks for the patch!
>
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1207

Arnd reported another objtool warning/error from another randconfig in
https://github.com/ClangBuiltLinux/linux/issues/1209 and CrOS just hit
this as well.

I haven't been able to isolate the configs yet (Arnd has posted the
full config: https://pastebin.com/wwwhUL8L

$ ./tools/objtool/objtool orc generate  --no-fp --no-unreachable
--retpoline arch/x86/entry/thunk_64.o
arch/x86/entry/thunk_64.o: warning: objtool: missing symbol for insn
at offset 0x3e

Is the offset 0x3e referring to the final `ret` instruction in
preempt_schedule_notrace_thunk?  Observing insn_to_reloc_sym_addend()
(with your patch applied), it looks like both calls to
find_symbol_containing() with offset and offset-1 returns NULL.  I'm
curious if there's another quirk going on here, or possibly a config
from the randconfig that's messing up the special case? I don't follow
the comment about:
119        * Hack alert.  This happens when we need to reference
120        * the NOP pad insn immediately after the function.
121        */

Attached the object file FWIW.
-- 
Thanks,
~Nick Desaulniers

[-- Attachment #2: thunk_64.o --]
[-- Type: application/octet-stream, Size: 744 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: objtool crashes with some clang produced .o files
  2020-12-12  0:42             ` Nick Desaulniers
@ 2020-12-12  0:47               ` Nick Desaulniers
  2020-12-14 12:49               ` Arnd Bergmann
  1 sibling, 0 replies; 11+ messages in thread
From: Nick Desaulniers @ 2020-12-12  0:47 UTC (permalink / raw)
  To: Josh Poimboeuf, Peter Zijlstra
  Cc: Arnd Bergmann, Linux Kernel Mailing List, clang-built-linux

[-- Attachment #1: Type: text/plain, Size: 1541 bytes --]

On Fri, Dec 11, 2020 at 4:42 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Fri, Dec 11, 2020 at 12:57 PM Nick Desaulniers
> <ndesaulniers@google.com> wrote:
> >
> > Thanks for the patch!
> >
> > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> > Link: https://github.com/ClangBuiltLinux/linux/issues/1207
>
> Arnd reported another objtool warning/error from another randconfig in
> https://github.com/ClangBuiltLinux/linux/issues/1209 and CrOS just hit
> this as well.
>
> I haven't been able to isolate the configs yet (Arnd has posted the
> full config: https://pastebin.com/wwwhUL8L
>
> $ ./tools/objtool/objtool orc generate  --no-fp --no-unreachable
> --retpoline arch/x86/entry/thunk_64.o
> arch/x86/entry/thunk_64.o: warning: objtool: missing symbol for insn
> at offset 0x3e
>
> Is the offset 0x3e referring to the final `ret` instruction in
> preempt_schedule_notrace_thunk?  Observing insn_to_reloc_sym_addend()
> (with your patch applied), it looks like both calls to
> find_symbol_containing() with offset and offset-1 returns NULL.  I'm
> curious if there's another quirk going on here, or possibly a config
> from the randconfig that's messing up the special case? I don't follow
> the comment about:
> 119        * Hack alert.  This happens when we need to reference
> 120        * the NOP pad insn immediately after the function.
> 121        */
>
> Attached the object file FWIW.

Resending with the attachment renamed; I just got a bounceback from
Josh's mailer daemon.
-- 
Thanks,
~Nick Desaulniers

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: thunk_64.o.txt --]
[-- Type: text/plain; charset="x-binaryenc"; name="thunk_64.o.txt", Size: 744 bytes --]

\x7fELF\x02\x01\x01\0\0\0\0\0\0\0\0\0\x01\0>\0\x01\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0¨\x01\0\0\0\0\0\0\0\0\0\0@\0\0\0\0\0@\0\x05\0\x01\0UH‰åWVRQPAPAQARASè\0\0\0\0ë\x18UH‰åWVRQPAPAQARASè\0\0\0\0ë\0A[AZAYAXXYZ^_]Ã\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0B\0\0\0\x10\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0S\0\0\0\x10\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0#\0\0\0\x12\0\x02\0\x18\0\0\0\0\0\0\0\x18\0\0\0\0\0\0\0\f\0\0\0\x12\0\x02\0\0\0\0\0\0\0\0\0\x18\0\0\0\0\0\0\0\x12\0\0\0\0\0\0\0\x04\0\0\0\x01\0\0\0üÿÿÿÿÿÿÿ*\0\0\0\0\0\0\0\x04\0\0\0\x02\0\0\0üÿÿÿÿÿÿÿ\0.rela.text\0preempt_schedule_thunk\0preempt_schedule_notrace_thunk\0preempt_schedule\0preempt_schedule_notrace\0.strtab\0.symtab\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0l\0\0\0\x03\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0(\x01\0\0\0\0\0\0|\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\x01\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\x06\0\0\0\x01\0\0\0\x06\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0@\0\0\0\0\0\0\0?\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\x04\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\x01\0\0\0\x04\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0ø\0\0\0\0\0\0\00\0\0\0\0\0\0\0\x04\0\0\0\x02\0\0\0\b\0\0\0\0\0\0\0\x18\0\0\0\0\0\0\0t\0\0\0\x02\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0€\0\0\0\0\0\0\0x\0\0\0\0\0\0\0\x01\0\0\0\x01\0\0\0\b\0\0\0\0\0\0\0\x18\0\0\0\0\0\0\0

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: objtool crashes with some clang produced .o files
  2020-12-12  0:42             ` Nick Desaulniers
  2020-12-12  0:47               ` Nick Desaulniers
@ 2020-12-14 12:49               ` Arnd Bergmann
  1 sibling, 0 replies; 11+ messages in thread
From: Arnd Bergmann @ 2020-12-14 12:49 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Josh Poimboeuf, Peter Zijlstra, Linux Kernel Mailing List,
	clang-built-linux

net/xfrm/xfrm_output.o: warning: objtool: xfrm_output_resume()+0xdb4:
unreachable instruction
On Sat, Dec 12, 2020 at 1:42 AM 'Nick Desaulniers' via Clang Built
Linux <clang-built-linux@googlegroups.com> wrote:
>
> On Fri, Dec 11, 2020 at 12:57 PM Nick Desaulniers
> <ndesaulniers@google.com> wrote:
> >
> > Thanks for the patch!
> >
> > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> > Link: https://github.com/ClangBuiltLinux/linux/issues/1207
>
> Arnd reported another objtool warning/error from another randconfig in
> https://github.com/ClangBuiltLinux/linux/issues/1209 and CrOS just hit
> this as well.
>
> I haven't been able to isolate the configs yet (Arnd has posted the
> full config: https://pastebin.com/wwwhUL8L
>
> $ ./tools/objtool/objtool orc generate  --no-fp --no-unreachable
> --retpoline arch/x86/entry/thunk_64.o
> arch/x86/entry/thunk_64.o: warning: objtool: missing symbol for insn
> at offset 0x3e
>
> Is the offset 0x3e referring to the final `ret` instruction in
> preempt_schedule_notrace_thunk?  Observing insn_to_reloc_sym_addend()
> (with your patch applied), it looks like both calls to
> find_symbol_containing() with offset and offset-1 returns NULL.  I'm
> curious if there's another quirk going on here, or possibly a config
> from the randconfig that's messing up the special case? I don't follow
> the comment about:
> 119        * Hack alert.  This happens when we need to reference
> 120        * the NOP pad insn immediately after the function.
> 121        */
>
> Attached the object file FWIW.

For completeness, these are the ones I currently see using clang-11
and clang-12, I believe
I have reported each one in the past:

1. For each file in kernel/trace/
kernel/trace/trace_clock.o: warning: objtool:
__llvm_gcov_writeout()+0x7: call without frame pointer save/setup
kernel/trace/trace_clock.o: warning: objtool: __llvm_gcov_reset()+0x0:
call without frame pointer save/setup
kernel/trace/trace_clock.o: warning: objtool: __llvm_gcov_flush()+0x0:
call without frame pointer save/setup
kernel/trace/trace_clock.o: warning: objtool: __llvm_gcov_init()+0x0:
call without frame pointer save/setup

2) reiserfs_panic()
fs/reiserfs/do_balan.o: warning: objtool: replace_key()+0x3db: stack
state mismatch: cfa1=7+104 cfa2=7+128
fs/reiserfs/do_balan.o: warning: objtool: balance_leaf()+0xd80d: stack
state mismatch: cfa1=7+424 cfa2=7+440
fs/reiserfs/lbalance.o: warning: objtool:
leaf_copy_boundary_item()+0x2bc5: stack state mismatch: cfa1=7+248
cfa2=7+240
fs/reiserfs/lbalance.o: warning: objtool:
leaf_copy_items_entirely()+0xcda: stack state mismatch: cfa1=7+256
cfa2=7+248
fs/reiserfs/ibalance.o: warning: objtool: balance_internal()+0x3448:
stack state mismatch: cfa1=7+328 cfa2=7+336
fs/reiserfs/ibalance.o: warning: objtool:
internal_move_pointers_items()+0x7c1: stack state mismatch: cfa1=7+200
cfa2=7+192

3) unreachable instructions:
arch/x86/entry/entry_64.o: warning: objtool: .entry.text+0xb95:
unreachable instruction
net/xfrm/xfrm_output.o: warning: objtool: xfrm_output_resume()+0xdb4:
unreachable instruction
drivers/hwmon/pmbus/adm1275.o: warning: objtool:
adm1275_probe()+0x622: unreachable instruction
drivers/xen/privcmd.o: warning: objtool: mmap_batch_fn()+0x14d:
unreachable instruction
drivers/xen/privcmd.o: warning: objtool:
privcmd_ioctl_mmap_batch()+0x954: unreachable instruction
lib/string.o: warning: objtool: fortify_panic()+0x3: unreachable instruction
drivers/scsi/smartpqi/smartpqi_init.o: warning: objtool:
pqi_shutdown()+0x244: unreachable instruction

4) jitterentropy built with -O0:
crypto/jitterentropy.o: warning: objtool: tsan.module_ctor()+0x0: call
without frame pointer save/setup

5) unsafe_put_user() misoptimization
arch/x86/kernel/signal.o: warning: objtool: __setup_rt_frame()+0x4c7:
call to memset() with UACCESS enabled
arch/x86/ia32/ia32_signal.o: warning: objtool:
ia32_setup_rt_frame()+0x15b: call to memset() with UACCESS enabled

6) user_access_save()/restore() problem in ftrace
kernel/trace/trace_branch.o: warning: objtool:
ftrace_likely_update()+0x1ed: call to __stack_chk_fail() with UACCESS
enabled

7) sibling calls:
mm/vmscan.o: warning: objtool: shrink_node()+0x540: sibling call from
callable instruction with modified stack frame
drivers/spi/spi-rockchip.o: warning: objtool:
rockchip_spi_transfer_one()+0x2e0: sibling call from callable
instruction with modified stack frame
drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool:
check_relocations()+0x68: return with modified stack frame

8) i915 GEM_BUG_ON() stack state mismatch (same as reiserfs_panic()):
drivers/gpu/drm/i915/gem/i915_gem_execbuffer.o: warning: objtool:
check_relocations()+0x145: stack state mismatch: cfa1=7+48 cfa2=-1+0
drivers/gpu/drm/i915/gem/i915_gem_mman.o: warning: objtool:
__igt_mmap_revoke()+0x180: stack state mismatch: cfa1=7+48 cfa2=-1+0

9) kasan build, for each file:
     arch/x86/entry/vdso/vma.o: warning: objtool:
asan.module_ctor()+0xc: call without frame pointer save/setup
     arch/x86/entry/vdso/vma.o: warning: objtool:
asan.module_dtor()+0xc: call without frame pointer save/setup
     arch/x86/entry/vsyscall/vsyscall_64.o: warning: objtool:
asan.module_ctor()+0xc: call without frame pointer save/setup
     arch/x86/entry/vsyscall/vsyscall_64.o: warning: objtool:
asan.module_dtor()+0xc: call without frame pointer save/setup
     arch/x86/events/amd/core.o: warning: objtool:
asan.module_ctor()+0xc: call without frame pointer save/setup
     arch/x86/events/amd/core.o: warning: objtool:
asan.module_dtor()+0xc: call without frame pointer save/setup
     arch/x86/events/amd/ibs.o: warning: objtool:
asan.module_ctor()+0xc: call without frame pointer save/setup
     arch/x86/events/amd/ibs.o: warning: objtool:
asan.module_dtor()+0xc: call without frame pointer save/setup

        Arnd

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2020-12-14 12:50 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-03 13:56 objtool crashes with some clang produced .o files Arnd Bergmann
2020-12-10 23:35 ` Nick Desaulniers
2020-12-11  8:45   ` Peter Zijlstra
2020-12-11  9:32   ` Peter Zijlstra
2020-12-11 16:37     ` Josh Poimboeuf
2020-12-11 16:49       ` Peter Zijlstra
2020-12-11 17:46         ` Josh Poimboeuf
2020-12-11 20:57           ` Nick Desaulniers
2020-12-12  0:42             ` Nick Desaulniers
2020-12-12  0:47               ` Nick Desaulniers
2020-12-14 12:49               ` Arnd Bergmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).