linux-kbuild.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sedat Dilek <sedat.dilek@gmail.com>
To: Nick Desaulniers <ndesaulniers@google.com>
Cc: Masahiro Yamada <masahiroy@kernel.org>,
	Nathan Chancellor <natechancellor@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel@vger.kernel.org,
	Clang-Built-Linux ML <clang-built-linux@googlegroups.com>,
	linux-kbuild@vger.kernel.org, linux-arch@vger.kernel.org,
	Jakub Jelinek <jakub@redhat.com>,
	Fangrui Song <maskray@google.com>,
	Caroline Tice <cmtice@google.com>,
	Nick Clifton <nickc@redhat.com>, Yonghong Song <yhs@fb.com>,
	Jiri Olsa <jolsa@kernel.org>, Andrii Nakryiko <andrii@kernel.org>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Arvind Sankar <nivedita@alum.mit.edu>,
	Chris Murphy <bugzilla@colorremedies.com>,
	Mark Wielaard <mark@klomp.org>
Subject: Re: [PATCH v9 0/3] Kbuild: DWARF v5 support
Date: Fri, 5 Feb 2021 22:00:39 +0100	[thread overview]
Message-ID: <CA+icZUW3sg_PkbmKSFMs6EqwQV7=hvKuAgZSsbg=Qr6gTs7RbQ@mail.gmail.com> (raw)
In-Reply-To: <20210205202220.2748551-1-ndesaulniers@google.com>

On Fri, Feb 5, 2021 at 9:22 PM Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> DWARF v5 is the latest standard of the DWARF debug info format.
>
> DWARF5 wins significantly in terms of size and especially so when mixed
> with compression (CONFIG_DEBUG_INFO_COMPRESSED).
>
> Link: http://www.dwarfstd.org/doc/DWARF5.pdf
>
> Patch 1 places the DWARF v5 sections explicitly in the kernel linker
> script.
> Patch 2 modifies Kconfig for DEBUG_INFO_DWARF4 to be used as a fallback.
> Patch 3 adds an explicit Kconfig for DWARF v5 for clang and older GCC
> where the implicit default DWARF version is not 5.
>
> Changes from v8:
> * Separate out the linker script changes (from v7 0002). Put those
>   first. Carry Reviewed by and tested by tags.  Least contentious part
>   of the series. Tagged for stable; otherwise users upgrading to GCC 11
>   may find orphan section warnings from the implicit default DWARF
>   version changing and generating the new debug info sections.
> * Add CONFIG_DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT in 0002, make it the
>   default rather than CONFIG_DEBUG_INFO_DWARF4, as per Mark, Jakub,
>   Arvind.
> * Drop reviewed by and tested by tags for 0002 and 0003; sorry
>   reviewers/testers, but I view that as a big change. I will buy you
>   beers if you're fatigued, AND for the help so far. I appreciate you.

All 3 patches NACKed - I drink no beer.

- sed@ -

> * Rework commit one lines, and commit messages somewhat.
> * Remove Kconfig help text about v4 being "bigger."
> * I didn't touch the BTF config from v8, but suggest the BTF folks
>   consider
>   https://lore.kernel.org/bpf/20210111180609.713998-1-natechancellor@gmail.com/
>   that way we can express via Kconfig that older version of pahole are
>   in conflict with other Kconfig options.
>
> Changes from v7:
> (Strictly commit message changes)
> * Pick up Nathan's reviewed by tags for both patches.
> * Add note about only modifying compiler dwarf info, not assembler dwarf
>   info, as per Nathan.
> * Add link to Red Hat bug report and Chris' reported by on patch 2.
> * Add more info from Jakub on patch 2 commit message.
> * Reorder info about validating version, noting the tree is not "clean"
>   in the sense that parts mess up existing CFLAGS, or don't use
>   DEBUG_CFLAGS. I will not be adding such cleanups to this series. They
>   can be done AFTER.
> * Update note about 2.35.2 (rather than include the full text Jakub
>   wrote on it in https://patchwork.kernel.org/project/linux-kbuild/patch/20201022012106.1875129-1-ndesaulniers@google.com/#23727667).
> * Add note that GCC 11 has changed the implicit default version.
>
> Changes from v6:
> * Reorder sections from linker script to match order from BFD's internal
>   linker script.
> * Add .debug_names section, as per Fangrui.
> * Drop CONFIG_DEBUG_INFO_DWARF2. Patch 0001 becomes a menu with 1
>   choice. GCC's implicit default version of DWARF has been DWARF v4
>   since ~4.8.
> * Modify the test script to check for the presence of
>   https://sourceware.org/bugzilla/show_bug.cgi?id=27195.
> * Drop the clang without integrated assembler block in
>   0002. Bumps the version requirement for GAS to 2.35.2, which isn't
>   released yet (but should be released soon).  Folks looking to test
>   with clang but without the integrated assembler should fetch
>   binutils-gdb, build it from source, add a symlink to
>   binutils-gdb/gas/as-new to binutils-gdb/gas/as, then prefix
>   binutils-gdb/gas/as to their $PATH when building the kernel.
>
> Changes from v5:
> * Drop previous patch 1, it has been accepted into kbuild:
>   https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git/commit/?h=kbuild&id=3f4d8ce271c7082be75bacbcbd2048aa78ce2b44
> * Trying to set -Wa,-gdwarf-4 in the earlier patch was the source of
>   additional complexity. Drop it that part of the patch. We can revisit
>   clang without the integrated assembler setting -Wa,-gdwarf-4 later.
>   That is a separate problem from generally supporting DWARF v5.
> * Rework the final patch for clang without the integrated assembler.
>   -Wa,-gdwarf-5 is required for DWARF5 in that case otherwise GAS will
>   not accept the assembler directives clang produces from C code when
>   generating asm.
>
> Changes from v4:
> * drop set -e from script as per Nathan.
> * add dependency on !CONFIG_DEBUG_INFO_BTF for DWARF v5 as per Sedat.
> * Move LLVM_IAS=1 complexity from patch 2 to patch 3 as per Arvind and
>   Masahiro. Sorry it took me a few tries to understand the point (I
>   might still not), but it looks much cleaner this way. Sorry Nathan, I
>   did not carry forward your previous reviews as a result, but I would
>   appreciate if you could look again.
> * Add Nathan's reviewed by tag to patch 1.
> * Reword commit message for patch 3 to mention LLVM_IAS=1 and -gdwarf-5
>   binutils addition later, and BTF issue.
> * I still happen to see a pahole related error spew for the combination
>   of:
>   * LLVM=1
>   * LLVM_IAS=1
>   * CONFIG_DEBUG_INFO_DWARF4
>   * CONFIG_DEBUG_INFO_BTF
>   Though they're non-fatal to the build. I'm not sure yet why removing
>   any one of the above prevents the warning spew. Maybe we'll need a v6.
>
> Changes from v3:
>
> Changes as per Arvind:
> * only add -Wa,-gdwarf-5 for (LLVM=1|CC=clang)+LLVM_IAS=0 builds.
> * add -gdwarf-5 to Kconfig shell script.
> * only run Kconfig shell script for Clang.
>
> Apologies to Sedat and Nathan; I appreciate previous testing/review, but
> I did no carry forward your Tested-by and Reviewed-by tags, as the
> patches have changed too much IMO.
>
> Changes from v2:
> * Drop two of the earlier patches that have been accepted already.
> * Add measurements with GCC 10.2 to commit message.
> * Update help text as per Arvind with help from Caroline.
> * Improve case/wording between DWARF Versions as per Masahiro.
>
> Changes from the RFC:
> * split patch in 3 patch series, include Fangrui's patch, too.
> * prefer `DWARF vX` format, as per Fangrui.
> * use spaces between assignment in Makefile as per Masahiro.
> * simplify setting dwarf-version-y as per Masahiro.
> * indent `prompt` in Kconfig change as per Masahiro.
> * remove explicit default in Kconfig as per Masahiro.
> * add comments to test_dwarf5_support.sh.
> * change echo in test_dwarf5_support.sh as per Masahiro.
> * remove -u from test_dwarf5_support.sh as per Masahiro.
> * add a -gdwarf-5 cc-option check to Kconfig as per Jakub.
>
> Nick Desaulniers (3):
>   vmlinux.lds.h: add DWARF v5 sections
>   Kbuild: make DWARF version a choice
>   Kconfig: allow explicit opt in to DWARF v5
>
>  Makefile                          |  6 ++--
>  include/asm-generic/vmlinux.lds.h |  7 ++++-
>  lib/Kconfig.debug                 | 48 +++++++++++++++++++++++++++----
>  scripts/test_dwarf5_support.sh    |  8 ++++++
>  4 files changed, 61 insertions(+), 8 deletions(-)
>  create mode 100755 scripts/test_dwarf5_support.sh
>
> --
> 2.30.0.365.g02bc693789-goog
>

  parent reply	other threads:[~2021-02-05 21:05 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-05 20:22 [PATCH v9 0/3] Kbuild: DWARF v5 support Nick Desaulniers
2021-02-05 20:22 ` [PATCH v9 1/3] vmlinux.lds.h: add DWARF v5 sections Nick Desaulniers
2021-02-06  0:00   ` Andrew Morton
2021-02-06  0:02     ` Nick Desaulniers
2021-02-06 15:02   ` Mark Wielaard
2021-02-05 20:22 ` [PATCH v9 2/3] Kbuild: make DWARF version a choice Nick Desaulniers
2021-02-05 20:22 ` [PATCH v9 3/3] Kconfig: allow explicit opt in to DWARF v5 Nick Desaulniers
2021-02-05 21:00 ` Sedat Dilek [this message]
2021-02-06 12:04   ` [PATCH v9 0/3] Kbuild: DWARF v5 support Masahiro Yamada
2021-02-06 12:28     ` Sedat Dilek
2021-02-25 19:13 ` Sedat Dilek

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='CA+icZUW3sg_PkbmKSFMs6EqwQV7=hvKuAgZSsbg=Qr6gTs7RbQ@mail.gmail.com' \
    --to=sedat.dilek@gmail.com \
    --cc=acme@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=andrii@kernel.org \
    --cc=bugzilla@colorremedies.com \
    --cc=clang-built-linux@googlegroups.com \
    --cc=cmtice@google.com \
    --cc=jakub@redhat.com \
    --cc=jolsa@kernel.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark@klomp.org \
    --cc=masahiroy@kernel.org \
    --cc=maskray@google.com \
    --cc=natechancellor@gmail.com \
    --cc=ndesaulniers@google.com \
    --cc=nickc@redhat.com \
    --cc=nivedita@alum.mit.edu \
    --cc=yhs@fb.com \
    /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 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).