linux-kernel - Re: objtool warnings for kernel/trace/trace_selftest

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]

Message-ID: <ri6h8fbqbdf.fsf@suse.cz>
Date:   Tue, 18 Dec 2018 13:15:40 +0100
From:   Martin Jambor <mjambor@...e.cz>
To:     Miroslav Benes <mbenes@...e.cz>,
        Josh Poimboeuf <jpoimboe@...hat.com>
Cc:     Andi Kleen <ak@...ux.intel.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Arnd Bergmann <arnd@...db.de>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "the arch\/x86 maintainers" <x86@...nel.org>
Subject: Re: objtool warnings for kernel/trace/trace_selftest_dynamic.o

Hi,

On Tue, Dec 18 2018, Miroslav Benes wrote:
>> Sorry for suggesting this prematurely, my email client stopped syncing
>> and I missed your later replies to Peter about this.
>> 
>> > > Should it be reverted, or just remove the noclone, and keep the
>> > > noinline?
>> > 
>> > It should not be touched for now, until it is properly debugged.
>> > 
>> > IMHO Josh's explanation doesn't make much sense and there
>> > was a lot of handwaving 
>> > 
>> > And just fixing one case isn't good enough because there are other
>> > noclone functions in the tree.
>> > 
>> > It the problem is the plugin the plugin needs to be fixed.
>> > 
>> > If the problem is gcc we need a gcc test case and bug, with 
>> > some analysis, and then based on that select the proper workaround.
>> 
>> The plugin is only used for older versions of GCC.  Newer versions have
>> the same functionality builtin with -fsanitize-coverage=trace-pc.
>> 
>> So the problem is GCC.

That's a little bit harsh :-)

>> We're using a function attribute which at least
>> oneGCC developer doesn't recommend.  If you want to keep the LTO support
>> then '__used' seems like a much better choice.
>
> Martin added to CC.
>
> Martin, the thread starts here 
> http://lkml.kernel.org/r/CAK8P3a2K1K21ePBFbApaTKPCk+=Bqj0LyWoK1MdFb1s9ZwjfPg@mail.gmail.com
>
> Can you explain the background of noclone vs. used attributes, please? 
> We discussed it yesterday and I understood that maybe we should not rely 
> on noclone that much. However it is used in the kernel. Should we avoid 
> it in general and replace it with something else (used)?

OK, I have read through it and with the caveats that I don't quite
understand what the failure is, that also believe attribute noclone
should not affect frame pointer generation, and that I don't quite get
how LTO comes into play, my comments are the following:

I am the developer who introduced attribute noclone to GCC and also the
one who advises against using it :-) ...at least without also using the
noinline attribute, the combination means "I want only one or zero
copies of this function in the compiled assembly" which you might need
if you do fancy stuff in inline assembly, for example.

I believe that when people use noclone on its own, in 99 out 100 cases
they actually want something else.  Usually there is something that
references the function from code (such as assembly) or a tool that the
compiler does know about and then they should use the "used" attribute.
That is what I understood to be the use case here and therefore I
recommended it.  In other cases people want all inter-procedural (IPA)
analyses to stay away from a function and then they should use attribute
noipa (or in older GCCs, the combination of noinline and noclone).

The attribute was introduced because it is useful in GCC testsuite, and
although I think occasions when it is useful on its own elsewhere are
very rare and quite obscure, they can happen.  But it really only means
you want only one or zero *non-inlined* copies of the function.  For
example if you have an asm in it that must appear in the compiled
assembly only once but you are confident it will be optimized out in
inlined instances.  Or if you play games with __builtin_return_address()
and somehow have a very clear idea what the return values should be.

I'm afraid I cannot give an opinion what you should do in this case
without understanding the problem better.  If you can isolate the case
where noclone behaves weirdly into a self-contained testcase, I'd be
happy to have a look at it.

Martin