[GIT PULL] compiler-attributes for v5.3-rc8

[GIT PULL] compiler-attributes for v5.3-rc8

[Miguel Ojeda]

Hi Linus,

I was going to send this for 5.4 since it is not that trivial, but since
you are doing an -rc8, and it fixes an oops, please consider pulling it.

Cheers,
Miguel

The following changes since commit a55aa89aab90fae7c815b0551b07be37db359d76:

  Linux 5.3-rc6 (2019-08-25 12:01:23 -0700)

are available in the Git repository at:

  https://github.com/ojeda/linux.git tags/compiler-attributes-for-linus-v5.3-rc8

for you to fetch changes up to c4814af0b75cc6856f60e8a658d829000b156729:

  compiler_attributes.h: add note about __section (2019-08-30 00:56:19 +0200)

----------------------------------------------------------------
__section cleanup that also fixes an Oops (Nick Desaulniers)

    GCC unescapes escaped string section names while Clang does not. Because
    __section uses the `#` stringification operator for the section name, it
    doesn't need to be escaped.

    This fixes an Oops observed in distro's that use systemd and not
    net.core.bpf_jit_enable=1, when their kernels are compiled with Clang.

----------------------------------------------------------------
Nick Desaulniers (13):
      s390/boot: fix section name escaping
      include/linux/compiler.h: prefer __section from compiler_attributes.h
      parisc: prefer __section from compiler_attributes.h
      um: prefer __section from compiler_attributes.h
      ia64: prefer __section from compiler_attributes.h
      arm: prefer __section from compiler_attributes.h
      mips: prefer __section from compiler_attributes.h
      sparc: prefer __section from compiler_attributes.h
      x86: prefer __section, __maybe_unused and __aligned from compiler_attributes.h
      include/asm-generic: prefer __section from compiler_attributes.h
      include/linux: prefer __section and __aligned from compiler_attributes.h
      include/linux/compiler.h: remove unused KENTRY macro
      compiler_attributes.h: add note about __section

 arch/arm/include/asm/cache.h          |  2 +-
 arch/arm/include/asm/mach/arch.h      |  4 ++--
 arch/arm/include/asm/setup.h          |  2 +-
 arch/ia64/include/asm/cache.h         |  2 +-
 arch/mips/include/asm/cache.h         |  2 +-
 arch/parisc/include/asm/cache.h       |  2 +-
 arch/parisc/include/asm/ldcw.h        |  2 +-
 arch/s390/boot/startup.c              |  2 +-
 arch/sparc/include/asm/cache.h        |  2 +-
 arch/sparc/kernel/btext.c             |  2 +-
 arch/um/kernel/um_arch.c              |  6 +++---
 arch/x86/include/asm/cache.h          |  2 +-
 arch/x86/include/asm/intel-mid.h      |  2 +-
 arch/x86/include/asm/iommu_table.h    |  5 ++---
 arch/x86/include/asm/irqflags.h       |  2 +-
 arch/x86/include/asm/mem_encrypt.h    |  2 +-
 arch/x86/kernel/cpu/cpu.h             |  3 +--
 include/asm-generic/error-injection.h |  2 +-
 include/asm-generic/kprobes.h         |  5 ++---
 include/linux/cache.h                 |  6 +++---
 include/linux/compiler.h              | 31 ++++---------------------------
 include/linux/compiler_attributes.h   | 10 ++++++++++
 include/linux/cpu.h                   |  2 +-
 include/linux/export.h                |  2 +-
 include/linux/init_task.h             |  4 ++--
 include/linux/interrupt.h             |  5 ++---
 include/linux/sched/debug.h           |  2 +-
 include/linux/srcutree.h              |  2 +-
 28 files changed, 49 insertions(+), 66 deletions(-)

Re: [GIT PULL] compiler-attributes for v5.3-rc8

[Miguel Ojeda]

On Wed, Sep 4, 2019 at 8:18 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> I was going to send this for 5.4 since it is not that trivial, but since
> you are doing an -rc8, and it fixes an oops, please consider pulling it.

By the way, if you choose to pick it for 5.4 instead, I will take the
chance to improve a few things (fix a whitespace/style issue in the
arm patch, improve a few commit messages/typos, maybe get a few extra
Acks for arches that don't have it). However, the Clang team would
like to have this for 5.3 if possible, so I didn't want to delay
sending it.

Cheers,
Miguel

Re: [GIT PULL] compiler-attributes for v5.3-rc8

[Linus Torvalds]

On Wed, Sep 4, 2019 at 11:18 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> I was going to send this for 5.4 since it is not that trivial, but since
> you are doing an -rc8, and it fixes an oops, please consider pulling it.

I looked at this, and while it seems safe, I end up worrying.

Macro stringification isn't entirely obvious, and an unquoted string
could become corrupted if the stringification ends up not happening
immediately.

It does seem safe just because we do

  #define __section(S)   __attribute__((__section__(#S)))

but I had to go _check_ that we do, because it wouldn't have been safe
if there had been another level of macro expansion, because then the
argument in turn could have been expanded before it was stringified.

So sometimes you actually _want_ to pass in a string to be
stringified, because it's safer. I realize it then gets string-quoted,
but this has worked for gcc. Even if I suspect nobody really _thought_
about it.

So I'm not unhappy about the patch, but it's the kind of thing I'd
really prefer not to do at this stage.

Particularly since it seems to do other things too than just fix
double quoting. As far as I can tell, it doesn't just fix double
string quoting, it changes a lot of singly-quoted strings to use the
macro and unquotes them, ie

  - __attribute__((__section__(".arch.info.init"))) = {   \
  + __section(.arch.info.init) = {        \

doesn't actually "fix" anything that I can see, it just uses the simpler form.

               Linus

Re: [GIT PULL] compiler-attributes for v5.3-rc8

[Nick Desaulniers]

On Thu, Sep 5, 2019 at 9:20 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Wed, Sep 4, 2019 at 11:18 AM Miguel Ojeda
> <miguel.ojeda.sandonis@gmail.com> wrote:
> >
> > I was going to send this for 5.4 since it is not that trivial, but since
> > you are doing an -rc8, and it fixes an oops, please consider pulling it.
>
> I looked at this, and while it seems safe, I end up worrying.
>
> Macro stringification isn't entirely obvious, and an unquoted string
> could become corrupted if the stringification ends up not happening
> immediately.
>
> It does seem safe just because we do
>
>   #define __section(S)   __attribute__((__section__(#S)))
>
> but I had to go _check_ that we do, because it wouldn't have been safe
> if there had been another level of macro expansion, because then the
> argument in turn could have been expanded before it was stringified.
>
> So sometimes you actually _want_ to pass in a string to be
> stringified, because it's safer. I realize it then gets string-quoted,
> but this has worked for gcc. Even if I suspect nobody really _thought_
> about it.
>
> So I'm not unhappy about the patch, but it's the kind of thing I'd
> really prefer not to do at this stage.
>
> Particularly since it seems to do other things too than just fix
> double quoting. As far as I can tell, it doesn't just fix double
> string quoting, it changes a lot of singly-quoted strings to use the
> macro and unquotes them, ie
>
>   - __attribute__((__section__(".arch.info.init"))) = {   \
>   + __section(.arch.info.init) = {        \
>
> doesn't actually "fix" anything that I can see, it just uses the simpler form.

Please consider picking up just:
https://github.com/ojeda/linux/commit/c97e82b97f4bba00304905fe7965f923abd2d755
That lone patch is the one that fixes the particularly observed Oops.
The rest are just cleanup; if I made that change in the more important
patch, why not clean up the rest of the instances in the kernel?
-- 
Thanks,
~Nick Desaulniers

Re: [GIT PULL] compiler-attributes for v5.3-rc8

[Linus Torvalds]

On Thu, Sep 5, 2019 at 10:18 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> Please consider picking up just:
> https://github.com/ojeda/linux/commit/c97e82b97f4bba00304905fe7965f923abd2d755
> That lone patch is the one that fixes the particularly observed Oops.
> The rest are just cleanup; if I made that change in the more important
> patch, why not clean up the rest of the instances in the kernel?

"Why not just clean up the rest" is how bugs happen.

If it's not a fix, and it's not marked for stable (or a regression
from the merge window) it shouldn't go in this late in the rc period.

Send me _fixes_. Don't send me stuff that is "fixes plus random
cleanups that were noticed at the same time".

                     Linus

Re: [GIT PULL] compiler-attributes for v5.3-rc8

[Miguel Ojeda]

On Thu, Sep 5, 2019 at 7:22 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> "Why not just clean up the rest" is how bugs happen.
>
> If it's not a fix, and it's not marked for stable (or a regression
> from the merge window) it shouldn't go in this late in the rc period.
>
> Send me _fixes_. Don't send me stuff that is "fixes plus random
> cleanups that were noticed at the same time".

Yeah, I was on the fence about this and I imagined you could also
react this way. I will be more firm next time :-)

On Nick's defense, he was trying to do his best to clean this up for
5.3 but AFAIK he got quite busy meanwhile during rc3-5.

Cheers,
Miguel

Re: [GIT PULL] compiler-attributes for v5.3-rc8

[Miguel Ojeda]

On Thu, Sep 5, 2019 at 6:20 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Macro stringification isn't entirely obvious, and an unquoted string
> could become corrupted if the stringification ends up not happening
> immediately.

Nick, Linus: shouldn't we just simply go for no stringifying at all,
i.e. changing it to:

    #define __section(S)   __attribute__((__section__(S)))

That way we can handle both easy things like:

    __section("foo")

as well as the mentioned:

    __section(".initcall" level ".init")

that we couldn't do before.

Both GCC and Clang give the same result and it is also easier to
reason about it.

Cheers,
Miguel

Re: [GIT PULL] compiler-attributes for v5.3-rc8

[Linus Torvalds]

On Thu, Sep 5, 2019 at 12:41 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> Nick, Linus: shouldn't we just simply go for no stringifying at all,
> i.e. changing it to:
>
>     #define __section(S)   __attribute__((__section__(S)))

That's probably what we should have done originally, avoiding all the
issues with "what if we have multi-part strings" etc.

But it's not what we did, probably because it looked slightly simpler
to do the stringification in the macro for the usual case.

So now we have (according to a quick grep) eight users that have a
constant string, and about one hundred users that use the unquoted
section name and expect the automatic stringification. I say "about",
because I didn't check if any of them might be doing tricks, I really
just did a stupid grep.

And we have that _one_ insane KENTRY thing that was apparently never
actually used.

So I think the minimal fix is to just accept that it's what it is,
remove the unnecessary quotes from the 8 existing users, and _if_
somebody wants to build the string  by hand (like the KENTRY code
did), then just use "__attribute__((section(x)))" for that.

But yeah, we could just remove the stringification and make the users do it.

But for the current late rc (and presumably -stable?), I definitely
want the absolute minimal thing that fixes the oops.

                     Linus

Re: [GIT PULL] compiler-attributes for v5.3-rc8

[Miguel Ojeda]

On Thu, Sep 5, 2019 at 10:53 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> That's probably what we should have done originally, avoiding all the
> issues with "what if we have multi-part strings" etc.
>
> But it's not what we did, probably because it looked slightly simpler
> to do the stringification in the macro for the usual case.
>
> So now we have (according to a quick grep) eight users that have a
> constant string, and about one hundred users that use the unquoted
> section name and expect the automatic stringification. I say "about",
> because I didn't check if any of them might be doing tricks, I really
> just did a stupid grep.
>
> And we have that _one_ insane KENTRY thing that was apparently never
> actually used.
>
> So I think the minimal fix is to just accept that it's what it is,
> remove the unnecessary quotes from the 8 existing users, and _if_
> somebody wants to build the string  by hand (like the KENTRY code
> did), then just use "__attribute__((section(x)))" for that.
>
> But yeah, we could just remove the stringification and make the users do it.
>
> But for the current late rc (and presumably -stable?), I definitely
> want the absolute minimal thing that fixes the oops.

Then I will send a PR with that patch only (Nick, do you know if the
entire patch is needed or we could further reduce it?).

Then for 5.4 I will prepare a new series moving to non-stringification
(unless Nick wants to do it himself).

Cheers,
Miguel

Re: [GIT PULL] compiler-attributes for v5.3-rc8

[Nick Desaulniers]

On Fri, Sep 6, 2019 at 1:11 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Thu, Sep 5, 2019 at 10:53 PM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > That's probably what we should have done originally, avoiding all the
> > issues with "what if we have multi-part strings" etc.
> >
> > But it's not what we did, probably because it looked slightly simpler
> > to do the stringification in the macro for the usual case.
> >
> > So now we have (according to a quick grep) eight users that have a
> > constant string, and about one hundred users that use the unquoted
> > section name and expect the automatic stringification. I say "about",
> > because I didn't check if any of them might be doing tricks, I really
> > just did a stupid grep.
> >
> > And we have that _one_ insane KENTRY thing that was apparently never
> > actually used.
> >
> > So I think the minimal fix is to just accept that it's what it is,
> > remove the unnecessary quotes from the 8 existing users, and _if_
> > somebody wants to build the string  by hand (like the KENTRY code
> > did), then just use "__attribute__((section(x)))" for that.
> >
> > But yeah, we could just remove the stringification and make the users do it.
> >
> > But for the current late rc (and presumably -stable?), I definitely
> > want the absolute minimal thing that fixes the oops.
>
> Then I will send a PR with that patch only (Nick, do you know if the
> entire patch is needed or we could further reduce it?).

Sedat reported (https://github.com/ClangBuiltLinux/linux/issues/619#issuecomment-520042577,
https://github.com/ClangBuiltLinux/linux/issues/619#issuecomment-520065525)
that only the bottom two hunks of that patch
(https://github.com/ojeda/linux/commit/c97e82b97f4bba00304905fe7965f923abd2d755)

>
> Then for 5.4 I will prepare a new series moving to non-stringification
> (unless Nick wants to do it himself).

Technically, it's not a regression, just something that would be nice
to have sooner rather than later.  The whole series can wait for 5.4,
IMO.  I'll look into updating the patchset next week.
-- 
Thanks,
~Nick Desaulniers

Re: [GIT PULL] compiler-attributes for v5.3-rc8

[Linus Torvalds]

On Fri, Sep 6, 2019 at 3:47 PM Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> Sedat reported (https://github.com/ClangBuiltLinux/linux/issues/619#issuecomment-520042577,
> https://github.com/ClangBuiltLinux/linux/issues/619#issuecomment-520065525)
> that only the bottom two hunks of that patch
> (https://github.com/ojeda/linux/commit/c97e82b97f4bba00304905fe7965f923abd2d755)

[ missing "matters" or something here? ]

If fixing two of the __section() uses in that tracing header file is
needed, then just fix all four there. I'm ok with that.

                Linus

Re: [GIT PULL] compiler-attributes for v5.3-rc8

[Nick Desaulniers]

On Fri, Sep 6, 2019 at 4:11 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Fri, Sep 6, 2019 at 3:47 PM Nick Desaulniers <ndesaulniers@google.com> wrote:
> >
> > Sedat reported (https://github.com/ClangBuiltLinux/linux/issues/619#issuecomment-520042577,
> > https://github.com/ClangBuiltLinux/linux/issues/619#issuecomment-520065525)
> > that only the bottom two hunks of that patch
> > (https://github.com/ojeda/linux/commit/c97e82b97f4bba00304905fe7965f923abd2d755)
>
> [ missing "matters" or something here? ]

(Yes; I must have pasted the link over it, or I'm having an episode)

>
> If fixing two of the __section() uses in that tracing header file is
> needed, then just fix all four there. I'm ok with that.

So then Miguel should maybe split off a new branch, rebase to keep
just the relevant patch
(https://github.com/ojeda/linux/commit/c97e82b97f4bba00304905fe7965f923abd2d755),
and send a PR to you for inclusion in 5.3?
-- 
Thanks,
~Nick Desaulniers

Re: [GIT PULL] compiler-attributes for v5.3-rc8

[Linus Torvalds]

On Fri, Sep 6, 2019 at 5:07 PM Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> So then Miguel should maybe split off a new branch, rebase to keep
> just the relevant patch
> (https://github.com/ojeda/linux/commit/c97e82b97f4bba00304905fe7965f923abd2d755),
> and send a PR to you for inclusion in 5.3?

Yes. With the appropriate test cycle, particularly obviously clang
(which I don't test in my own minor tests).

                    Linus

Re: [GIT PULL] compiler-attributes for v5.3-rc8

[Nick Desaulniers]

On Fri, Sep 6, 2019 at 5:08 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Fri, Sep 6, 2019 at 5:07 PM Nick Desaulniers <ndesaulniers@google.com> wrote:
> >
> > So then Miguel should maybe split off a new branch, rebase to keep
> > just the relevant patch
> > (https://github.com/ojeda/linux/commit/c97e82b97f4bba00304905fe7965f923abd2d755),
> > and send a PR to you for inclusion in 5.3?
>
> Yes. With the appropriate test cycle

Sedat reported the issue and already tested/verified the fix.  How
long should it sit in -next before sending a PR for inclusion to 5.3
(as opposed to letting it ride out to 5.4)?

> , particularly obviously clang
> (which I don't test in my own minor tests).

It's the devil's compiler; wouldn't recommend.

In all seriousness, I'm missing giving an update at LPC this year
(have a much more important meeting:
https://ironmaiden.com/tours/legacy-of-the-beast-tour-2019-2019/usa-oakland-ca-oracle-arena-2019-09-10),
but I'm always happy to answer questions related to it or take a look
at bug reports.  Behan Webster is giving a talk about it at LPC
though!  Check it out if you're at LPC.
-- 
Thanks,
~Nick Desaulniers

Re: [GIT PULL] compiler-attributes for v5.3-rc8

[Linus Torvalds]

On Fri, Sep 6, 2019 at 5:45 PM Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> > Yes. With the appropriate test cycle
>
> Sedat reported the issue and already tested/verified the fix.  How
> long should it sit in -next before sending a PR for inclusion to 5.3
> (as opposed to letting it ride out to 5.4)?

If the original patch was already in -next, I wouldn't worry about it,
as long as you do enough local testing that there's nothing stupid
going on.

The -next cycle is a few days, and even with an rc8 we're getting
close enough to release that I'd rather get it earlier than later.  So
I'd rather get a pull request this weekend than then have to deal with
it when traveling next week.

             Linus

Re: [GIT PULL] compiler-attributes for v5.3-rc8

[Sedat Dilek]

On Sat, Sep 7, 2019 at 12:59 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Fri, Sep 6, 2019 at 5:45 PM Nick Desaulniers <ndesaulniers@google.com> wrote:
> >
> > > Yes. With the appropriate test cycle
> >
> > Sedat reported the issue and already tested/verified the fix.  How
> > long should it sit in -next before sending a PR for inclusion to 5.3
> > (as opposed to letting it ride out to 5.4)?
>
> If the original patch was already in -next, I wouldn't worry about it,
> as long as you do enough local testing that there's nothing stupid
> going on.
>

I am the original reporter and tester of the ClangBuiltLinux issue
#619 and highly appreciate to have the single fix in Linux v5.3 final.

The compiler-attribute patchset sit for some weeks in linux-next, so I
have not seen any complains.

What I still prefer is to re-integrate the arm64 and sh arch related
patches which went through maintainer's trees in linux-next.
AFAICS some massage of the commit messages were missing, too.

devil's OMG dileks used compiler and linker: LLVM/Clang and LLD v9.0.0-rc3

> The -next cycle is a few days, and even with an rc8 we're getting
> close enough to release that I'd rather get it earlier than later.  So
> I'd rather get a pull request this weekend than then have to deal with
> it when traveling next week.
>

+1 for ASAPISSIMO

Thanks for taking care.

- Sedat -

Re: [GIT PULL] compiler-attributes for v5.3-rc8

[Miguel Ojeda]

On Sat, Sep 7, 2019 at 7:50 AM Sedat Dilek <sedat.dilek@gmail.com> wrote:
>
> The compiler-attribute patchset sit for some weeks in linux-next, so I
> have not seen any complains.

It has been there only since Monday (cleanly), not weeks.

Cheers,
Miguel

Re: [GIT PULL] compiler-attributes for v5.3-rc8

[Sedat Dilek]

On Sat, Sep 7, 2019 at 1:59 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Sat, Sep 7, 2019 at 7:50 AM Sedat Dilek <sedat.dilek@gmail.com> wrote:
> >
> > The compiler-attribute patchset sit for some weeks in linux-next, so I
> > have not seen any complains.
>
> It has been there only since Monday (cleanly), not weeks.
>


Sorry, I was not precise enough and didn't remember correctly.

I have re-tested with Linux v5.3-rc8. All OK.

- Sedat -

Re: [GIT PULL] compiler-attributes for v5.3-rc8

[Miguel Ojeda]

On Tue, Sep 10, 2019 at 10:58 AM Sedat Dilek <sedat.dilek@gmail.com> wrote:
>
> Sorry, I was not precise enough and didn't remember correctly.
>
> I have re-tested with Linux v5.3-rc8. All OK.

No worries at all! I just wanted to clarify it :)

Thanks a lot for confirming it works.

Cheers,
Miguel

[GIT PULL] compiler-attributes for v5.3-rc8

[Miguel Ojeda]

Hi Linus,

Here it is the Oops-fixing cherry-picked commit for -rc8 from the __section
cleanup series.

Cheers,
Miguel

The following changes since commit 089cf7f6ecb266b6a4164919a2e69bd2f938374a:

  Linux 5.3-rc7 (2019-09-02 09:57:40 -0700)

are available in the Git repository at:

  https://github.com/ojeda/linux.git tags/clang-format-for-linus-v5.3-rc8

for you to fetch changes up to bfafddd8de426d894fcf3e062370b1efaa195ebc:

  include/linux/compiler.h: fix Oops for Clang-compiled kernels (2019-09-08 14:53:58 +0200)

----------------------------------------------------------------
Fix Oops in Clang-compiled kernels (Nick Desaulniers)

----------------------------------------------------------------
Nick Desaulniers (1):
      include/linux/compiler.h: fix Oops for Clang-compiled kernels

 include/linux/compiler.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Re: [GIT PULL] compiler-attributes for v5.3-rc8

[Miguel Ojeda]

On Sun, Sep 8, 2019 at 3:19 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
>   https://github.com/ojeda/linux.git tags/clang-format-for-linus-v5.3-rc8

Typo in the tag name, you can also use the tag:

    https://github.com/ojeda/linux.git
tags/compiler-attributes-for-linus-v5.3-rc8

Cheers,
Miguel

Re: [GIT PULL] compiler-attributes for v5.3-rc8

[pr-tracker-bot]

The pull request you sent on Sun, 8 Sep 2019 15:19:02 +0200:

> https://github.com/ojeda/linux.git tags/clang-format-for-linus-v5.3-rc8

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/983f700eab89c73562f308fc49b1561377d3920e

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker