linux-kernel - Re: [GIT PULL 2/4] Kbuild updates for v4.21 part2

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 7 Jan 2019 13:32:56 +0900
From:   Masahiro Yamada <yamada.masahiro@...ionext.com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Linux Kbuild mailing list <linux-kbuild@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Changbin Du <changbin.du@...il.com>,
        Arnd Bergmann <arnd@...db.de>
Subject: Re: [GIT PULL 2/4] Kbuild updates for v4.21 part2

(+CC Changbin Du, Arnd Bergmann)



On Sun, Dec 30, 2018 at 6:01 AM Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> On Fri, Dec 28, 2018 at 8:00 AM Masahiro Yamada
> <yamada.masahiro@...ionext.com> wrote:
> >
> > Introduce a new option CONFIG_NO_AUTO_INLINE as well. With this option,
> > only functions explicitly marked with "inline" will be inlined. This
> > will allow the function tracer to trace more functions.
>
> Ugh. This causes new and bogus warnings, because gcc doesn't see some
> of the checks.
>
> For example, the x86 emulate_vsyscall() function now warns that
>
>     warning: ‘syscall_nr’ may be used uninitialized in this function
>
> because addr_to_vsyscall_nr() isn't inlined any more, so gcc doesn't
> see that the return value is always smaller than three (and then it
> doesn't see that we handle all the cases in the switch statement
> later).
>
> So honestly, these "debugging improvements" have a rather nasty side
> effect. I'm not at all convinced that we should do this.  It is *not*
> worth causing extra development pain with a totally pointless option
> that changes code generation radically like this. It's going to cause
> lots of extra churn.



Sorry for late reply.

I know this series was rejected,
but I felt guilty about being completely silent.

So, here is just my two cents.


For addr_to_vsyscall_nr(), we can fix it by marking it as 'inline'
(or __always_inline).


IMHO, it is not so bad to tell the compiler what we want.

In most cases, it is just a matter of 'inline' marker,
although I admit this process is kind of painful...



> This branch already added a couple of extra inline markers just to
> make code work reasonably. How many tens (or hundreds) got missed just
> because the build still works, but the lack of inlining means that we
> generate completely garbage code?


I do not have the exact number, but
my impression was "not so many".

Changbin and Arnd might have better insight.
They were trying to eliminate potential problems beforehand.

For example,
7e17916b
412dd15c
08813e0e




> I'm going to skip this pull request.
>
> We have basically always required a certain level of competence from
> the compiler, and this basically says that the compiler can do stupid
> things and we'd have to fix those stupidities by hand.
>
>              Linus



-- 
Best Regards
Masahiro Yamada