All of lore.kernel.org
 help / color / mirror / Atom feed
From: will.deacon@arm.com (Will Deacon)
To: linux-arm-kernel@lists.infradead.org
Subject: Build failure with v4.9-rc1 and GCC trunk -- compiler weirdness
Date: Mon, 17 Oct 2016 19:38:07 +0100	[thread overview]
Message-ID: <20161017183806.GG5601@arm.com> (raw)

Hi all,

I'm seeing an arm64 build failure with -rc1 and GCC trunk, although I
believe that the new compiler behaviour at the heart of the problem
has the potential to affect other architectures and other pieces of
kernel code relying on dead-code elimination to remove deliberately
undefined functions.

The failure looks like:

  | drivers/built-in.o: In function `armada_3700_add_composite_clk':
  |
  | linux/drivers/clk/mvebu/armada-37xx-periph.c:351:
  | undefined reference to `____ilog2_NaN'
  |
  | linux/drivers/clk/mvebu/armada-37xx-periph.c:351:(.text+0xc72e0):
  | relocation truncated to fit: R_AARCH64_CALL26 against undefined symbol
  | `____ilog2_NaN'
  |
  | make: *** [vmlinux] Error 1

and if we look at the source for armada_3700_add_composite_clk, we see
that this is caused by:

  int table_size = 0;

  rate->reg = reg + (u64)rate->reg;
  for (clkt = rate->table; clkt->div; clkt++)
	 table_size++;
  rate->width = order_base_2(table_size);

order_base_2 calls ilog2, which has the ____ilog2_NaN call:

#define ilog2(n)				\
(						\
	__builtin_constant_p(n) ? (		\
		(n) < 1 ? ____ilog2_NaN() :	\

This is because we're in a curious case where GCC has emitted a
special-cased version of armada_3700_add_composite_clk, with table_size
effectively constant-folded as 0. Whilst we shouldn't see this in a
non-buggy kernel (hence the deliberate call to the undefined function
____ilog2_NaN), it means that the final link fails because we have a
____ilog2_NaN in the code, with a runtime check on table_size.

In other words, __builtin_constant_p appears to be weaker than we've
been assuming. Talking to the compiler guys here, this is due to the
"jump-threading" optimisation pass, so the patch below disables that.

A simpler example is:

int foo();
int bar();

int count(int *argc)
{
	int table_size = 0;

	for (; *argc; argc++)
		table_size++;

	if (__builtin_constant_p(table_size))
		return table_size == 0 ? foo() : bar();

	return bar();
}

which compiles to:

count:
	ldr	w0, [x0]
	cbz	w0, .L4
	b	bar
	.p2align 3
.L4:
	b	foo

and, with the "optimisation" disabled:

count:
	b	bar

Thoughts? It feels awfully fragile disabling passes like this, but with
GCC transforming the code like this, I can't immediately think of a way
to preserve the intended behaviour of the code.

Will

--->8

diff --git a/Makefile b/Makefile
index 512e47a53e9a..750873d6d11e 100644
--- a/Makefile
+++ b/Makefile
@@ -641,6 +641,11 @@ endif
 # Tell gcc to never replace conditional load with a non-conditional one
 KBUILD_CFLAGS	+= $(call cc-option,--param=allow-store-data-races=0)
 
+# Stop gcc from converting switches into a form that defeats dead code
+# elimination and can subsequently lead to calls to intentionally
+# undefined functions appearing in the final link.
+KBUILD_CFLAGS	+= $(call cc-option,--param=max-fsm-thread-path-insns=1)
+
 include scripts/Makefile.gcc-plugins
 
 ifdef CONFIG_READABLE_ASM

WARNING: multiple messages have this Message-ID (diff)
From: Will Deacon <will.deacon@arm.com>
To: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org, torvalds@linux-foundation.org,
	peterz@infradead.org, mingo@kernel.org,
	ard.biesheuvel@linaro.org, james.greenhalgh@arm.com,
	gregory.clement@free-electrons.com, sboyd@codeaurora.org
Subject: Build failure with v4.9-rc1 and GCC trunk -- compiler weirdness
Date: Mon, 17 Oct 2016 19:38:07 +0100	[thread overview]
Message-ID: <20161017183806.GG5601@arm.com> (raw)

Hi all,

I'm seeing an arm64 build failure with -rc1 and GCC trunk, although I
believe that the new compiler behaviour at the heart of the problem
has the potential to affect other architectures and other pieces of
kernel code relying on dead-code elimination to remove deliberately
undefined functions.

The failure looks like:

  | drivers/built-in.o: In function `armada_3700_add_composite_clk':
  |
  | linux/drivers/clk/mvebu/armada-37xx-periph.c:351:
  | undefined reference to `____ilog2_NaN'
  |
  | linux/drivers/clk/mvebu/armada-37xx-periph.c:351:(.text+0xc72e0):
  | relocation truncated to fit: R_AARCH64_CALL26 against undefined symbol
  | `____ilog2_NaN'
  |
  | make: *** [vmlinux] Error 1

and if we look at the source for armada_3700_add_composite_clk, we see
that this is caused by:

  int table_size = 0;

  rate->reg = reg + (u64)rate->reg;
  for (clkt = rate->table; clkt->div; clkt++)
	 table_size++;
  rate->width = order_base_2(table_size);

order_base_2 calls ilog2, which has the ____ilog2_NaN call:

#define ilog2(n)				\
(						\
	__builtin_constant_p(n) ? (		\
		(n) < 1 ? ____ilog2_NaN() :	\

This is because we're in a curious case where GCC has emitted a
special-cased version of armada_3700_add_composite_clk, with table_size
effectively constant-folded as 0. Whilst we shouldn't see this in a
non-buggy kernel (hence the deliberate call to the undefined function
____ilog2_NaN), it means that the final link fails because we have a
____ilog2_NaN in the code, with a runtime check on table_size.

In other words, __builtin_constant_p appears to be weaker than we've
been assuming. Talking to the compiler guys here, this is due to the
"jump-threading" optimisation pass, so the patch below disables that.

A simpler example is:

int foo();
int bar();

int count(int *argc)
{
	int table_size = 0;

	for (; *argc; argc++)
		table_size++;

	if (__builtin_constant_p(table_size))
		return table_size == 0 ? foo() : bar();

	return bar();
}

which compiles to:

count:
	ldr	w0, [x0]
	cbz	w0, .L4
	b	bar
	.p2align 3
.L4:
	b	foo

and, with the "optimisation" disabled:

count:
	b	bar

Thoughts? It feels awfully fragile disabling passes like this, but with
GCC transforming the code like this, I can't immediately think of a way
to preserve the intended behaviour of the code.

Will

--->8

diff --git a/Makefile b/Makefile
index 512e47a53e9a..750873d6d11e 100644
--- a/Makefile
+++ b/Makefile
@@ -641,6 +641,11 @@ endif
 # Tell gcc to never replace conditional load with a non-conditional one
 KBUILD_CFLAGS	+= $(call cc-option,--param=allow-store-data-races=0)
 
+# Stop gcc from converting switches into a form that defeats dead code
+# elimination and can subsequently lead to calls to intentionally
+# undefined functions appearing in the final link.
+KBUILD_CFLAGS	+= $(call cc-option,--param=max-fsm-thread-path-insns=1)
+
 include scripts/Makefile.gcc-plugins
 
 ifdef CONFIG_READABLE_ASM

             reply	other threads:[~2016-10-17 18:38 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-17 18:38 Will Deacon [this message]
2016-10-17 18:38 ` Build failure with v4.9-rc1 and GCC trunk -- compiler weirdness Will Deacon
2016-10-17 19:43 ` Ard Biesheuvel
2016-10-17 19:43   ` Ard Biesheuvel
2016-10-19 13:35   ` Will Deacon
2016-10-19 13:35     ` Will Deacon
2016-10-19 14:59     ` Ard Biesheuvel
2016-10-19 14:59       ` Ard Biesheuvel
2016-10-19 15:01       ` Ard Biesheuvel
2016-10-19 15:01         ` Ard Biesheuvel
2016-10-19 15:11         ` Arnd Bergmann
2016-10-19 15:11           ` Arnd Bergmann
2016-10-19 15:27           ` Ard Biesheuvel
2016-10-19 15:27             ` Ard Biesheuvel
2016-10-19 15:44             ` Arnd Bergmann
2016-10-19 15:44               ` Arnd Bergmann
2016-10-19 15:32           ` Gregory CLEMENT
2016-10-19 15:32             ` Gregory CLEMENT
2016-10-19 15:58     ` Russell King - ARM Linux
2016-10-19 15:58       ` Russell King - ARM Linux
2016-10-19 15:37 ` Markus Trippelsdorf
2016-10-19 15:37   ` Markus Trippelsdorf
2016-10-19 15:55   ` Linus Torvalds
2016-10-19 15:55     ` Linus Torvalds
2016-10-19 15:56     ` Markus Trippelsdorf
2016-10-19 15:56       ` Markus Trippelsdorf
2016-10-19 16:00       ` Ard Biesheuvel
2016-10-19 16:00         ` Ard Biesheuvel
2016-10-19 16:01       ` Linus Torvalds
2016-10-19 16:01         ` Linus Torvalds
2016-10-19 16:22         ` Will Deacon
2016-10-19 16:22           ` Will Deacon
2017-02-01 16:58           ` Laura Abbott
2017-02-01 16:58             ` Laura Abbott
2017-02-01 17:36             ` Ard Biesheuvel
2017-02-01 17:36               ` Ard Biesheuvel
2017-02-01 18:19               ` Ard Biesheuvel
2017-02-01 18:19                 ` Ard Biesheuvel
2017-02-01 19:04                 ` Joe Perches
2017-02-01 19:04                   ` Joe Perches
2017-02-01 19:31                   ` Ard Biesheuvel
2017-02-01 19:31                     ` Ard Biesheuvel
2017-02-01 19:49                     ` Joe Perches
2017-02-01 19:49                       ` Joe Perches
2017-02-01 19:53                       ` Ard Biesheuvel
2017-02-01 19:53                         ` Ard Biesheuvel
2017-02-01 20:34                         ` Joe Perches
2017-02-01 20:34                           ` Joe Perches
2017-02-01 21:11                           ` Ard Biesheuvel
2017-02-01 21:11                             ` Ard Biesheuvel
2017-02-02  9:02                   ` Peter Zijlstra
2017-02-02  9:02                     ` Peter Zijlstra
2017-02-01 21:50               ` Laura Abbott
2017-02-01 21:50                 ` Laura Abbott
2017-02-02  9:17                 ` Ard Biesheuvel
2017-02-02  9:17                   ` Ard Biesheuvel
2017-02-02 15:43                   ` Laura Abbott
2017-02-02 15:43                     ` Laura Abbott
2017-02-02 15:45                     ` Ard Biesheuvel
2017-02-02 15:45                       ` Ard Biesheuvel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161017183806.GG5601@arm.com \
    --to=will.deacon@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.