2020-10-10 23:09:32

by Ian Rogers

[permalink] [raw]
Subject: Re: [PATCH] Only add -fno-var-tracking-assignments workaround for old GCC versions.

On Sat, Oct 10, 2020 at 3:08 PM Mark Wielaard <[email protected]> wrote:
>
> Some old GCC versions between 4.5.0 and 4.9.1 might miscompile code
> with -fvar-tracking-assingments (which is enabled by default with -g -O2).
> commit 2062afb4f added -fno-var-tracking-assignments unconditionally to
> work around this. But newer versions of GCC no longer have this bug, so
> only add it for versions of GCC before 5.0.
>
> Signed-off-by: Mark Wielaard <[email protected]>

Acked-by: Ian Rogers <[email protected]>

Thanks,
Ian

> ---
> Makefile | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index f84d7e4ca0be..4f4a9416a87a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -813,7 +813,9 @@ KBUILD_CFLAGS += -ftrivial-auto-var-init=zero
> KBUILD_CFLAGS += -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang
> endif
>
> -DEBUG_CFLAGS := $(call cc-option, -fno-var-tracking-assignments)
> +# Workaround https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61801
> +# for old versions of GCC.
> +DEBUG_CFLAGS := $(call cc-ifversion, -lt, 0500, $(call cc-option, -fno-var-tracking-assignments))
>
> ifdef CONFIG_DEBUG_INFO
> ifdef CONFIG_DEBUG_INFO_SPLIT
> --
> 2.18.4
>


2020-10-12 19:03:08

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH] Only add -fno-var-tracking-assignments workaround for old GCC versions.

On Sat, Oct 10, 2020 at 3:57 PM Ian Rogers <[email protected]> wrote:
>
> On Sat, Oct 10, 2020 at 3:08 PM Mark Wielaard <[email protected]> wrote:
> >
> > Some old GCC versions between 4.5.0 and 4.9.1 might miscompile code
> > with -fvar-tracking-assingments (which is enabled by default with -g -O2).
> > commit 2062afb4f added -fno-var-tracking-assignments unconditionally to
> > work around this. But newer versions of GCC no longer have this bug, so
> > only add it for versions of GCC before 5.0.
> >
> > Signed-off-by: Mark Wielaard <[email protected]>
>
> Acked-by: Ian Rogers <[email protected]>
>
> Thanks,
> Ian
>
> > ---
> > Makefile | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/Makefile b/Makefile
> > index f84d7e4ca0be..4f4a9416a87a 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -813,7 +813,9 @@ KBUILD_CFLAGS += -ftrivial-auto-var-init=zero
> > KBUILD_CFLAGS += -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang
> > endif
> >
> > -DEBUG_CFLAGS := $(call cc-option, -fno-var-tracking-assignments)
> > +# Workaround https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61801
> > +# for old versions of GCC.
> > +DEBUG_CFLAGS := $(call cc-ifversion, -lt, 0500, $(call cc-option, -fno-var-tracking-assignments))

Should this be wrapped in: `ifdef CONFIG_CC_IS_GCC`/`endif`?

> >
> > ifdef CONFIG_DEBUG_INFO
> > ifdef CONFIG_DEBUG_INFO_SPLIT
> > --
> > 2.18.4
> >



--
Thanks,
~Nick Desaulniers

2020-10-12 19:14:04

by Mark Wielaard

[permalink] [raw]
Subject: Re: [PATCH] Only add -fno-var-tracking-assignments workaround for old GCC versions.

Hi,

On Mon, 2020-10-12 at 11:59 -0700, Nick Desaulniers wrote:
> On Sat, Oct 10, 2020 at 3:57 PM Ian Rogers <[email protected]>
> wrote:
> > On Sat, Oct 10, 2020 at 3:08 PM Mark Wielaard <[email protected]>
> > wrote:
> > > -DEBUG_CFLAGS := $(call cc-option, -fno-var-tracking-
> > > assignments)
> > > +# Workaround https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61801
> > > +# for old versions of GCC.
> > > +DEBUG_CFLAGS := $(call cc-ifversion, -lt, 0500, $(call cc-
> > > option, -fno-var-tracking-assignments))
>
> Should this be wrapped in: `ifdef CONFIG_CC_IS_GCC`/`endif`?

I don't think so. It wasn't before. And call cc-option makes sure to
only add the flag if the compiler supports it (clang doesn't and it
also has a much higher version).

Cheers,

Mark

2020-10-14 16:01:48

by Mark Wielaard

[permalink] [raw]
Subject: [PATCH] Only add -fno-var-tracking-assignments workaround for old GCC versions.

Some old GCC versions between 4.5.0 and 4.9.1 might miscompile code
with -fvar-tracking-assingments (which is enabled by default with -g -O2).
commit 2062afb4f added -fno-var-tracking-assignments unconditionally to
work around this. But newer versions of GCC no longer have this bug, so
only add it for versions of GCC before 5.0.

Signed-off-by: Mark Wielaard <[email protected]>
Acked-by: Ian Rogers <[email protected]>
Cc: [email protected]
Cc: Andi Kleen <[email protected]>
Cc: Nick Desaulniers <[email protected]>
Cc: Segher Boessenkool <[email protected]>
Cc: Florian Weimer <[email protected]>
---
Makefile | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 51540b291738..8477fee5f309 100644
--- a/Makefile
+++ b/Makefile
@@ -813,7 +813,9 @@ KBUILD_CFLAGS += -ftrivial-auto-var-init=zero
KBUILD_CFLAGS += -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang
endif

-DEBUG_CFLAGS := $(call cc-option, -fno-var-tracking-assignments)
+# Workaround https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61801
+# for old versions of GCC.
+DEBUG_CFLAGS := $(call cc-ifversion, -lt, 0500, $(call cc-option, -fno-var-tracking-assignments))

ifdef CONFIG_DEBUG_INFO
ifdef CONFIG_DEBUG_INFO_SPLIT
--
2.18.4

2020-10-14 17:39:11

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] Only add -fno-var-tracking-assignments workaround for old GCC versions.

On Wed, Oct 14, 2020 at 01:01:32PM +0200, Mark Wielaard wrote:
> Some old GCC versions between 4.5.0 and 4.9.1 might miscompile code
> with -fvar-tracking-assingments (which is enabled by default with -g -O2).
> commit 2062afb4f added -fno-var-tracking-assignments unconditionally to
> work around this. But newer versions of GCC no longer have this bug, so
> only add it for versions of GCC before 5.0.

Add

... This allows various tools such as a perf probe or gdb debuggers
or systemtap to resolve variable locations using dwarf locations in
more code.
>
> Signed-off-by: Mark Wielaard <[email protected]>
> Acked-by: Ian Rogers <[email protected]>
> Cc: [email protected]
> Cc: Andi Kleen <[email protected]>
> Cc: Nick Desaulniers <[email protected]>
> Cc: Segher Boessenkool <[email protected]>
> Cc: Florian Weimer <[email protected]>

Reviewed-by: Andi Kleen <[email protected]>

-Andi

2020-10-14 17:39:57

by Sedat Dilek

[permalink] [raw]
Subject: Re: [PATCH] Only add -fno-var-tracking-assignments workaround for old GCC versions.

On Mon, Oct 12, 2020 at 9:12 PM Mark Wielaard <[email protected]> wrote:
>
> Hi,
>
> On Mon, 2020-10-12 at 11:59 -0700, Nick Desaulniers wrote:
> > On Sat, Oct 10, 2020 at 3:57 PM Ian Rogers <[email protected]>
> > wrote:
> > > On Sat, Oct 10, 2020 at 3:08 PM Mark Wielaard <[email protected]>
> > > wrote:
> > > > -DEBUG_CFLAGS := $(call cc-option, -fno-var-tracking-
> > > > assignments)
> > > > +# Workaround https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61801
> > > > +# for old versions of GCC.
> > > > +DEBUG_CFLAGS := $(call cc-ifversion, -lt, 0500, $(call cc-
> > > > option, -fno-var-tracking-assignments))
> >
> > Should this be wrapped in: `ifdef CONFIG_CC_IS_GCC`/`endif`?
>
> I don't think so. It wasn't before. And call cc-option makes sure to
> only add the flag if the compiler supports it (clang doesn't and it
> also has a much higher version).
>

I am also in favour of `ifdef CONFIG_CC_IS_GCC` to clearly say this is
a GCC bug.

For the comment something like:

# Workaround for GCC version <= 5.0
# GCC Bug: <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61801>

Think of people grepping in the Linux source code for supported or
broken compiler (versions)...
As a reference see ClangBuiltLinux issue #427 "audit use of __GNUC__".
[2] says:
"There's also a ton of __GNUC_MINOR__ checks against unsupported GCC versions."

- Sedat -

[1] https://github.com/ClangBuiltLinux/linux/issues/427
[2] https://github.com/ClangBuiltLinux/linux/issues/427#issuecomment-700935241

2020-10-17 15:56:39

by Mark Wielaard

[permalink] [raw]
Subject: [PATCH V2] Only add -fno-var-tracking-assignments workaround for old GCC versions.

Some old GCC versions between 4.5.0 and 4.9.1 might miscompile code
with -fvar-tracking-assingments (which is enabled by default with -g -O2).
commit 2062afb4f added -fno-var-tracking-assignments unconditionally to
work around this. But newer versions of GCC no longer have this bug, so
only add it for versions of GCC before 5.0. This allows various tools
such as a perf probe or gdb debuggers or systemtap to resolve variable
locations using dwarf locations in more code.

Changes in V2:
- Update commit message explaining purpose.
- Explicitly mention GCC version in comment.
- Wrap workaround in ifdef CONFIG_CC_IS_GCC

Signed-off-by: Mark Wielaard <[email protected]>
Acked-by: Ian Rogers <[email protected]>
Reviewed-by: Andi Kleen <[email protected]>
Cc: [email protected]
Cc: Nick Desaulniers <[email protected]>
Cc: Segher Boessenkool <[email protected]>
Cc: Florian Weimer <[email protected]>
Cc: Sedat Dilek <[email protected]>
---
Makefile | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 51540b291738..964754b4cedf 100644
--- a/Makefile
+++ b/Makefile
@@ -813,7 +813,11 @@ KBUILD_CFLAGS += -ftrivial-auto-var-init=zero
KBUILD_CFLAGS += -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang
endif

-DEBUG_CFLAGS := $(call cc-option, -fno-var-tracking-assignments)
+# Workaround for GCC versions < 5.0
+# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61801
+ifdef CONFIG_CC_IS_GCC
+DEBUG_CFLAGS := $(call cc-ifversion, -lt, 0500, $(call cc-option, -fno-var-tracking-assignments))
+endif

ifdef CONFIG_DEBUG_INFO
ifdef CONFIG_DEBUG_INFO_SPLIT
--
2.18.4

2020-10-20 14:43:49

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH V2] Only add -fno-var-tracking-assignments workaround for old GCC versions.

On Sat, Oct 17, 2020 at 5:02 AM Mark Wielaard <[email protected]> wrote:
>
> Some old GCC versions between 4.5.0 and 4.9.1 might miscompile code
> with -fvar-tracking-assingments (which is enabled by default with -g -O2).
> commit 2062afb4f added -fno-var-tracking-assignments unconditionally to
> work around this. But newer versions of GCC no longer have this bug, so
> only add it for versions of GCC before 5.0. This allows various tools
> such as a perf probe or gdb debuggers or systemtap to resolve variable
> locations using dwarf locations in more code.
>
> Changes in V2:
> - Update commit message explaining purpose.
> - Explicitly mention GCC version in comment.
> - Wrap workaround in ifdef CONFIG_CC_IS_GCC
>
> Signed-off-by: Mark Wielaard <[email protected]>
> Acked-by: Ian Rogers <[email protected]>
> Reviewed-by: Andi Kleen <[email protected]>
> Cc: [email protected]
> Cc: Nick Desaulniers <[email protected]>
> Cc: Segher Boessenkool <[email protected]>
> Cc: Florian Weimer <[email protected]>
> Cc: Sedat Dilek <[email protected]>
> ---
> Makefile | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index 51540b291738..964754b4cedf 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -813,7 +813,11 @@ KBUILD_CFLAGS += -ftrivial-auto-var-init=zero
> KBUILD_CFLAGS += -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang
> endif
>
> -DEBUG_CFLAGS := $(call cc-option, -fno-var-tracking-assignments)
> +# Workaround for GCC versions < 5.0
> +# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61801
> +ifdef CONFIG_CC_IS_GCC
> +DEBUG_CFLAGS := $(call cc-ifversion, -lt, 0500, $(call cc-option, -fno-var-tracking-assignments))

Thanks for adding the comment. That will help us find+remove this when
the kernel's minimum supported version of GCC advances to gcc-5. The
current minimum supported version of GCC according to
Documentation/process/changes.rst is gcc-4.9 (so anything older is
irrelevant, and we drop support for it). If gcc 4.9 supports
`-fno-var-tracking-assignments` (it looks like it does:
https://godbolt.org/z/oa53f5), then we should drop the `cc-option`
call, which will save us a compiler invocation for each invocation of
`make`.

> +endif
>
> ifdef CONFIG_DEBUG_INFO
> ifdef CONFIG_DEBUG_INFO_SPLIT
> --
> 2.18.4
>


--
Thanks,
~Nick Desaulniers

2020-10-21 08:56:11

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH V2] Only add -fno-var-tracking-assignments workaround for old GCC versions.

On Sat, Oct 17, 2020 at 9:02 PM Mark Wielaard <[email protected]> wrote:
>
> Some old GCC versions between 4.5.0 and 4.9.1 might miscompile code
> with -fvar-tracking-assingments (which is enabled by default with -g -O2).
> commit 2062afb4f added -fno-var-tracking-assignments unconditionally to
> work around this. But newer versions of GCC no longer have this bug, so
> only add it for versions of GCC before 5.0. This allows various tools
> such as a perf probe or gdb debuggers or systemtap to resolve variable
> locations using dwarf locations in more code.
>
> Changes in V2:
> - Update commit message explaining purpose.
> - Explicitly mention GCC version in comment.
> - Wrap workaround in ifdef CONFIG_CC_IS_GCC
>
> Signed-off-by: Mark Wielaard <[email protected]>
> Acked-by: Ian Rogers <[email protected]>
> Reviewed-by: Andi Kleen <[email protected]>
> Cc: [email protected]
> Cc: Nick Desaulniers <[email protected]>
> Cc: Segher Boessenkool <[email protected]>
> Cc: Florian Weimer <[email protected]>
> Cc: Sedat Dilek <[email protected]>
> ---

Applied to linux-kbuild. Thanks.



> Makefile | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index 51540b291738..964754b4cedf 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -813,7 +813,11 @@ KBUILD_CFLAGS += -ftrivial-auto-var-init=zero
> KBUILD_CFLAGS += -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang
> endif
>
> -DEBUG_CFLAGS := $(call cc-option, -fno-var-tracking-assignments)
> +# Workaround for GCC versions < 5.0
> +# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61801
> +ifdef CONFIG_CC_IS_GCC
> +DEBUG_CFLAGS := $(call cc-ifversion, -lt, 0500, $(call cc-option, -fno-var-tracking-assignments))
> +endif
>
> ifdef CONFIG_DEBUG_INFO
> ifdef CONFIG_DEBUG_INFO_SPLIT
> --
> 2.18.4
>


--
Best Regards
Masahiro Yamada