[PATCH RESEND] Kbuild: Add an option to enable GCC VTA

[PATCH RESEND] Kbuild: Add an option to enable GCC VTA

[Josh Stone]

Due to recent codegen issues, gcc -fvar-tracking-assignments was
unconditionally disabled in commit 2062afb4f804a ("Fix gcc-4.9.0
miscompilation of load_balance() in scheduler").  However, this reduces
the debuginfo coverage for variable locations, especially in inline
functions.  VTA is certainly not perfect either in those cases, but it
is much better than without.  With compiler versions that have fixed the
codegen bugs, we would prefer to have the better details for SystemTap,
and surely other debuginfo consumers like perf will benefit as well.

This patch simply makes CONFIG_DEBUG_INFO_VTA an option.  I considered
Frank and Linus's discussion of a cc-option-like -fcompare-debug test,
but I'm convinced that a narrow test of an arch-specific codegen issue
is not really useful.  GCC has their own regression tests for this, so
I'd suggest GCC_COMPARE_DEBUG=-fvar-tracking-assignments-toggle is more
useful for kernel developers to test confidence.

In fact, I ran into a couple more issues when testing for this patch[1],
although neither of those had any codegen impact.
 [1] https://bugzilla.redhat.com/show_bug.cgi?id=1140872

With gcc-4.9.2-1.fc22, I can now build v3.18-rc5 with Fedora's i686 and
x86_64 configs, and this is completely clean with GCC_COMPARE_DEBUG.

Cc: Frank Ch. Eigler <fche@redhat.com>
Cc: Jakub Jelinek <jakub@redhat.com>
Cc: Josh Boyer <jwboyer@fedoraproject.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Markus Trippelsdorf <markus@trippelsdorf.de>
Cc: Michel Dänzer <michel@daenzer.net>
Signed-off-by: Josh Stone <jistone@redhat.com>
---
 Makefile          |  4 ++++
 lib/Kconfig.debug | 18 +++++++++++++++++-
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 00d618bbe8e7..a289efbe7e28 100644
--- a/Makefile
+++ b/Makefile
@@ -704,7 +704,11 @@ KBUILD_CFLAGS	+= -fomit-frame-pointer
 endif
 endif
 
+ifdef CONFIG_DEBUG_INFO_VTA
+KBUILD_CFLAGS   += $(call cc-option, -fvar-tracking-assignments)
+else
 KBUILD_CFLAGS   += $(call cc-option, -fno-var-tracking-assignments)
+endif
 
 ifdef CONFIG_DEBUG_INFO
 ifdef CONFIG_DEBUG_INFO_SPLIT
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 4e35a5d767ed..27410417de1d 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -165,7 +165,23 @@ config DEBUG_INFO_DWARF4
 	  Generate dwarf4 debug info. This requires recent versions
 	  of gcc and gdb. It makes the debug information larger.
 	  But it significantly improves the success of resolving
-	  variables in gdb on optimized code.
+	  variables in gdb on optimized code.  The gcc docs also
+	  recommend enabling -fvar-tracking-assignments for maximum
+	  benefit. (see DEBUG_INFO_VTA)
+
+config DEBUG_INFO_VTA
+	bool "Enable var-tracking-assignments for debuginfo"
+	depends on DEBUG_INFO
+	help
+	  Enable gcc -fvar-tracking-assignments for improved debug
+	  information on variable locations in optimized code.  Per
+	  gcc, DEBUG_INFO_DWARF4 is recommended for best use of VTA.
+
+	  VTA has been implicated in codegen bugs (gcc PR61801,
+	  PR61904), so this may deserve some caution.  One can set
+	  GCC_COMPARE_DEBUG=-fvar-tracking-assignments-toggle in the
+	  environment to automatically compile everything both ways,
+	  generating an error if anything differs.
 
 config ENABLE_WARN_DEPRECATED
 	bool "Enable __deprecated logic"
-- 
2.1.0

Re: [PATCH RESEND] Kbuild: Add an option to enable GCC VTA

[Michal Marek]

Dne 21.11.2014 v 19:40 Josh Stone napsal(a):
> Due to recent codegen issues, gcc -fvar-tracking-assignments was
> unconditionally disabled in commit 2062afb4f804a ("Fix gcc-4.9.0
> miscompilation of load_balance() in scheduler").  However, this reduces
[...]
> With gcc-4.9.2-1.fc22, I can now build v3.18-rc5 with Fedora's i686 and
> x86_64 configs, and this is completely clean with GCC_COMPARE_DEBUG.

According to gcc's bug#61801, this is really fixed in 4.9.2 (commit
556537c4 in the git mirror). So how about checking for this minimal
version instead of a new Kconfig option?

thanks,
Michal

Re: [PATCH RESEND] Kbuild: Add an option to enable GCC VTA

[Josh Stone]

On 11/24/2014 01:46 PM, Michal Marek wrote:
> Dne 21.11.2014 v 19:40 Josh Stone napsal(a):
>> Due to recent codegen issues, gcc -fvar-tracking-assignments was
>> unconditionally disabled in commit 2062afb4f804a ("Fix gcc-4.9.0
>> miscompilation of load_balance() in scheduler").  However, this reduces
> [...]
>> With gcc-4.9.2-1.fc22, I can now build v3.18-rc5 with Fedora's i686 and
>> x86_64 configs, and this is completely clean with GCC_COMPARE_DEBUG.
> 
> According to gcc's bug#61801, this is really fixed in 4.9.2 (commit
> 556537c4 in the git mirror). So how about checking for this minimal
> version instead of a new Kconfig option?

That's possible, if the new Kconfig option is really undesirable.  But
given that there's a similar DWARF4 option, which this pairs well with,
I thought a VTA option was a good choice.

Besides 4.9.2 though, this particular fix has also been backported to
4.8.4, and who knows what various distro maintainers may do.

Re: [PATCH RESEND] Kbuild: Add an option to enable GCC VTA

[Michal Marek]

Dne 25.11.2014 v 00:46 Josh Stone napsal(a):
> On 11/24/2014 01:46 PM, Michal Marek wrote:
>> Dne 21.11.2014 v 19:40 Josh Stone napsal(a):
>>> Due to recent codegen issues, gcc -fvar-tracking-assignments was
>>> unconditionally disabled in commit 2062afb4f804a ("Fix gcc-4.9.0
>>> miscompilation of load_balance() in scheduler").  However, this reduces
>> [...]
>>> With gcc-4.9.2-1.fc22, I can now build v3.18-rc5 with Fedora's i686 and
>>> x86_64 configs, and this is completely clean with GCC_COMPARE_DEBUG.
>>
>> According to gcc's bug#61801, this is really fixed in 4.9.2 (commit
>> 556537c4 in the git mirror). So how about checking for this minimal
>> version instead of a new Kconfig option?
> 
> That's possible, if the new Kconfig option is really undesirable.  But
> given that there's a similar DWARF4 option, which this pairs well with,
> I thought a VTA option was a good choice.
> 
> Besides 4.9.2 though, this particular fix has also been backported to
> 4.8.4, and who knows what various distro maintainers may do.

My idea was that if we switch -fvar-tracking-assignments on with gcc
4.9.2+, then yes, some users with patched compilers will not benefit
from it, but it will eventually work out of the box for "everybody."
With the kconfig option, an uninformed user has no idea whether or not
it is a good idea to switch it on. It should at least mention that it is
safe to enable with 4.9.2+. And BTW

    make KCLFAGS=-fvar-tracking-assignments

works as well, as the user-supplied flags are applied last.

Michal

Re: [PATCH RESEND] Kbuild: Add an option to enable GCC VTA

[Josh Boyer]

On Tue, Nov 25, 2014 at 4:05 PM, Michal Marek <mmarek@suse.cz> wrote:
> Dne 25.11.2014 v 00:46 Josh Stone napsal(a):
>> On 11/24/2014 01:46 PM, Michal Marek wrote:
>>> Dne 21.11.2014 v 19:40 Josh Stone napsal(a):
>>>> Due to recent codegen issues, gcc -fvar-tracking-assignments was
>>>> unconditionally disabled in commit 2062afb4f804a ("Fix gcc-4.9.0
>>>> miscompilation of load_balance() in scheduler").  However, this reduces
>>> [...]
>>>> With gcc-4.9.2-1.fc22, I can now build v3.18-rc5 with Fedora's i686 and
>>>> x86_64 configs, and this is completely clean with GCC_COMPARE_DEBUG.
>>>
>>> According to gcc's bug#61801, this is really fixed in 4.9.2 (commit
>>> 556537c4 in the git mirror). So how about checking for this minimal
>>> version instead of a new Kconfig option?
>>
>> That's possible, if the new Kconfig option is really undesirable.  But
>> given that there's a similar DWARF4 option, which this pairs well with,
>> I thought a VTA option was a good choice.
>>
>> Besides 4.9.2 though, this particular fix has also been backported to
>> 4.8.4, and who knows what various distro maintainers may do.
>
> My idea was that if we switch -fvar-tracking-assignments on with gcc
> 4.9.2+, then yes, some users with patched compilers will not benefit
> from it, but it will eventually work out of the box for "everybody."
> With the kconfig option, an uninformed user has no idea whether or not
> it is a good idea to switch it on. It should at least mention that it is
> safe to enable with 4.9.2+. And BTW
>
>     make KCLFAGS=-fvar-tracking-assignments
>
> works as well, as the user-supplied flags are applied last.

I actually tried this after Kyle McMartin mentioned it, and thought
about doing it in the Fedora kernel spec file instead of taking the
patch.  It's a bit less noticeable when trying to reproduce build
environments though, and with it being a cc-option in the Makefile we
can rely on that to filter out compilers and arches that don't support
the flag.

josh

[PATCH] Kbuild: Add an option to enable GCC VTA

[Frank Ch. Eigler]

From: Josh Stone <jistone@redhat.com>

Due to isolated gcc codegen issues, gcc -fvar-tracking-assignments
was unconditionally disabled in commit 2062afb4f804 ("Fix gcc-4.9.0
miscompilation of load_balance()  in scheduler").

However, this reduces the debuginfo coverage for variable locations,
especially in inline functions.  VTA is certainly not perfect either
in those cases, but it is much better than without.  With compiler
versions that have fixed the codegen bugs, we would prefer to have the
better details for SystemTap, and surely other debuginfo consumers
like perf will benefit as well.

This patch simply makes CONFIG_DEBUG_INFO_VTA an option.  I considered
Frank and Linus's discussion of a cc-option-like -fcompare-debug test,
but I'm convinced that a narrow test of an arch-specific codegen issue
is not really useful.  GCC has their own regression tests for this, so
I'd suggest GCC_COMPARE_DEBUG=-fvar-tracking-assignments-toggle is more
useful for kernel developers to test confidence.

In fact, I ran into a couple more issues when testing for this patch[1],
although neither of those had any codegen impact.
 [1] https://bugzilla.redhat.com/show_bug.cgi?id=1140872

With gcc-4.9.2-1.fc22, I can now build v3.18-rc5 with Fedora's i686 and
x86_64 configs, and this is completely clean with GCC_COMPARE_DEBUG.

Cc: Jakub Jelinek <jakub@redhat.com>
Cc: Josh Boyer <jwboyer@fedoraproject.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Markus Trippelsdorf <markus@trippelsdorf.de>
Cc: Michel Dänzer <michel@daenzer.net>
Signed-off-by: Josh Stone <jistone@redhat.com>
Signed-off-by: Frank Ch. Eigler <fche@redhat.com>
---
 Makefile          |  8 ++++++--
 lib/Kconfig.debug | 21 ++++++++++++++++++++-
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 6cc5b2434224..c8e1fcfdb41a 100644
--- a/Makefile
+++ b/Makefile
@@ -704,8 +704,6 @@ KBUILD_CFLAGS	+= -fomit-frame-pointer
 endif
 endif
 
-KBUILD_CFLAGS   += $(call cc-option, -fno-var-tracking-assignments)
-
 ifdef CONFIG_DEBUG_INFO
 ifdef CONFIG_DEBUG_INFO_SPLIT
 KBUILD_CFLAGS   += $(call cc-option, -gsplit-dwarf, -g)
@@ -718,6 +716,12 @@ ifdef CONFIG_DEBUG_INFO_DWARF4
 KBUILD_CFLAGS	+= $(call cc-option, -gdwarf-4,)
 endif
 
+ifdef CONFIG_DEBUG_INFO_VTA
+KBUILD_CFLAGS   += $(call cc-option, -fvar-tracking-assignments)
+else
+KBUILD_CFLAGS   += $(call cc-option, -fno-var-tracking-assignments)
+endif
+
 ifdef CONFIG_DEBUG_INFO_REDUCED
 KBUILD_CFLAGS 	+= $(call cc-option, -femit-struct-debug-baseonly) \
 		   $(call cc-option,-fno-var-tracking)
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 17670573dda8..e8d072d2b402 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -165,7 +165,26 @@ config DEBUG_INFO_DWARF4
 	  Generate dwarf4 debug info. This requires recent versions
 	  of gcc and gdb. It makes the debug information larger.
 	  But it significantly improves the success of resolving
-	  variables in gdb on optimized code.
+	  variables in gdb on optimized code.  The gcc docs also
+	  recommend enabling -fvar-tracking-assignments for maximum
+	  benefit. (see DEBUG_INFO_VTA)
+
+config DEBUG_INFO_VTA
+	bool "Enable var-tracking-assignments for debuginfo"
+	depends on DEBUG_INFO
+	help
+	  Enable gcc -fvar-tracking-assignments for improved debug
+	  information on variable locations in optimized code.	Per
+	  gcc, DEBUG_INFO_DWARF4 is recommended for best use of VTA,
+	  and allows maximal access to local variables in tracers
+	  and debuggers like perf, systemtap, kgdb, and crash.
+
+	  VTA has been implicated in codegen bugs (gcc PR61801,
+	  PR61904, both fixed in 2014-08), so this flag may be used
+	  to exclude this rare class of problem.  One can also set
+	  GCC_COMPARE_DEBUG=-fvar-tracking-assignments-toggle in the
+	  environment to automatically compile everything both ways,
+	  generating an error if anything differs.
 
 config GDB_SCRIPTS
 	bool "Provide GDB scripts for kernel debugging"
-- 
2.1.0

Re: [PATCH] Kbuild: Add an option to enable GCC VTA

[Josh Boyer]

On Thu, Apr 23, 2015 at 5:25 PM, Frank Ch. Eigler <fche@redhat.com> wrote:
> From: Josh Stone <jistone@redhat.com>
>
> Due to isolated gcc codegen issues, gcc -fvar-tracking-assignments
> was unconditionally disabled in commit 2062afb4f804 ("Fix gcc-4.9.0
> miscompilation of load_balance()  in scheduler").
>
> However, this reduces the debuginfo coverage for variable locations,
> especially in inline functions.  VTA is certainly not perfect either
> in those cases, but it is much better than without.  With compiler
> versions that have fixed the codegen bugs, we would prefer to have the
> better details for SystemTap, and surely other debuginfo consumers
> like perf will benefit as well.
>
> This patch simply makes CONFIG_DEBUG_INFO_VTA an option.  I considered
> Frank and Linus's discussion of a cc-option-like -fcompare-debug test,
> but I'm convinced that a narrow test of an arch-specific codegen issue
> is not really useful.  GCC has their own regression tests for this, so
> I'd suggest GCC_COMPARE_DEBUG=-fvar-tracking-assignments-toggle is more
> useful for kernel developers to test confidence.
>
> In fact, I ran into a couple more issues when testing for this patch[1],
> although neither of those had any codegen impact.
>  [1] https://bugzilla.redhat.com/show_bug.cgi?id=1140872
>
> With gcc-4.9.2-1.fc22, I can now build v3.18-rc5 with Fedora's i686 and
> x86_64 configs, and this is completely clean with GCC_COMPARE_DEBUG.
>
> Cc: Jakub Jelinek <jakub@redhat.com>
> Cc: Josh Boyer <jwboyer@fedoraproject.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Markus Trippelsdorf <markus@trippelsdorf.de>
> Cc: Michel Dänzer <michel@daenzer.net>
> Signed-off-by: Josh Stone <jistone@redhat.com>
> Signed-off-by: Frank Ch. Eigler <fche@redhat.com>

Frank, did you rebase this against some newer tree or something?
Curious why you sent it again.

>  Makefile          |  8 ++++++--
>  lib/Kconfig.debug | 21 ++++++++++++++++++++-
>  2 files changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 6cc5b2434224..c8e1fcfdb41a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -704,8 +704,6 @@ KBUILD_CFLAGS       += -fomit-frame-pointer
>  endif
>  endif
>
> -KBUILD_CFLAGS   += $(call cc-option, -fno-var-tracking-assignments)
> -
>  ifdef CONFIG_DEBUG_INFO
>  ifdef CONFIG_DEBUG_INFO_SPLIT
>  KBUILD_CFLAGS   += $(call cc-option, -gsplit-dwarf, -g)
> @@ -718,6 +716,12 @@ ifdef CONFIG_DEBUG_INFO_DWARF4
>  KBUILD_CFLAGS  += $(call cc-option, -gdwarf-4,)
>  endif
>
> +ifdef CONFIG_DEBUG_INFO_VTA
> +KBUILD_CFLAGS   += $(call cc-option, -fvar-tracking-assignments)
> +else
> +KBUILD_CFLAGS   += $(call cc-option, -fno-var-tracking-assignments)
> +endif
> +

Is there a reason you moved this hunk under the DWARF4 options instead
of modifying it in-place like the original patch did?

josh

Re: [PATCH] Kbuild: Add an option to enable GCC VTA

[Frank Ch. Eigler]

Hi, Josh -

On Fri, Apr 24, 2015 at 08:40:02AM -0400, Josh Boyer wrote:
> [...]
> Frank, did you rebase this against some newer tree or something?

Yes; the lib/Kconfig.debug part didn't apply to current git.

> Curious why you sent it again.

At least as a patch-ping; the poor-debuginfo problems are reported to
affect non-fedora users too.


> > +ifdef CONFIG_DEBUG_INFO_VTA
> > +KBUILD_CFLAGS   += $(call cc-option, -fvar-tracking-assignments)
> > +else
> > +KBUILD_CFLAGS   += $(call cc-option, -fno-var-tracking-assignments)
> > +endif
> > +
> 
> Is there a reason you moved this hunk under the DWARF4 options instead
> of modifying it in-place like the original patch did?

Yes, this version appears a little safer, in the sense that without
CONFIG_DEBUG_INFO, neither setting of CONFIG_DEBUG_INFO_VTA would
affect the CFLAGS.  (In fact, Jakub advises the positive polarity
-fvar-tracking-assignments is redundant with -g, and the negative
polarity one only provides codegen-bug-protection in the
CONFIG_DEBUG_INFO case.)


- FChE