linux-kernel - Re: [PATCH] bug.h: Work around GCC PR82365 in BUG()

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: <CAK8P3a0-_u7_FCj-nH0izBv4ub6krm1uA32bwi2jtBzXJePcnQ@mail.gmail.com>
Date:   Wed, 11 Apr 2018 09:30:56 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     James Hogan <jhogan@...nel.org>
Cc:     "open list:RALINK MIPS ARCHITECTURE" <linux-mips@...ux-mips.org>,
        Paul Burton <paul.burton@...s.com>,
        Maciej Rozycki <macro@...s.com>,
        linux-arch <linux-arch@...r.kernel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Linux Kbuild mailing list <linux-kbuild@...r.kernel.org>,
        Vineet Gupta <vgupta@...opsys.com>,
        Mikael Starvik <starvik@...s.com>,
        Jesper Nilsson <jesper.nilsson@...s.com>,
        Tony Luck <tony.luck@...el.com>,
        Fenghua Yu <fenghua.yu@...el.com>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        "David S. Miller" <davem@...emloft.net>,
        Christopher Li <sparse@...isli.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Peter Zijlstra <peterz@...radead.org>,
        Kees Cook <keescook@...omium.org>,
        Ingo Molnar <mingo@...nel.org>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Will Deacon <will.deacon@....com>,
        "Steven Rostedt (VMware)" <rostedt@...dmis.org>,
        Mark Rutland <mark.rutland@....com>,
        "open list:SYNOPSYS ARC ARCHITECTURE" 
        <linux-snps-arc@...ts.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-cris-kernel@...s.com, linux-ia64@...r.kernel.org,
        linux-m68k@...ts.linux-m68k.org,
        sparclinux <sparclinux@...r.kernel.org>,
        linux-sparse@...r.kernel.org
Subject: Re: [PATCH] bug.h: Work around GCC PR82365 in BUG()

On Wed, Apr 11, 2018 at 12:48 AM, James Hogan <jhogan@...nel.org> wrote:
> Hi Arnd,
>
> On Tue, Dec 19, 2017 at 12:39:33PM +0100, Arnd Bergmann wrote:
>> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
>> index 5d595cfdb2c4..66cfdad68f7e 100644
>> --- a/include/linux/compiler-gcc.h
>> +++ b/include/linux/compiler-gcc.h
>> @@ -205,6 +205,15 @@
>>  #endif
>>
>>  /*
>> + * calling noreturn functions, __builtin_unreachable() and __builtin_trap()
>> + * confuse the stack allocation in gcc, leading to overly large stack
>> + * frames, see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82365
>> + *
>> + * Adding an empty inline assembly before it works around the problem
>> + */
>> +#define barrier_before_unreachable() asm volatile("")
>> +
>> +/*
>>   * Mark a position in code as unreachable.  This can be used to
>>   * suppress control flow warnings after asm blocks that transfer
>>   * control elsewhere.
>> @@ -214,7 +223,11 @@
>>   * unreleased.  Really, we need to have autoconf for the kernel.
>>   */
>>  #define unreachable() \
>> -     do { annotate_unreachable(); __builtin_unreachable(); } while (0)
>> +     do {                                    \
>> +             annotate_unreachable();         \
>> +             barrier_before_unreachable();   \
>> +             __builtin_unreachable();        \
>> +     } while (0)
>
> Unfortunately this breaks microMIPS builds (e.g. MIPS
> micro32r2_defconfig and micro32r2el_defconfig) on gcc 7.2, due to the
> lack of .insn in the asm volatile. Because of the
> __builtin_unreachable() there is no code following it. Without the empty
> asm the compiler will apparently put the .insn there automatically, but
> with the empty asm it doesn't. Therefore the assembler won't treat an
> immediately preceeding label as pointing at 16-bit microMIPS
> instructions which need the ISA bit set, i.e. bit 0 of the address.
> This causes assembler errors since the branch target is treated as a
> different ISA mode:
>
> arch/mips/mm/dma-default.s:3265: Error: branch to a symbol in another ISA mode
> arch/mips/mm/dma-default.s:5027: Error: branch to a symbol in another ISA mode

Ok, I see.

> Due to a compiler bug on gcc 4.9.2 -> somewhere before 7.2, Paul
> submitted these patches a while back:
> https://patchwork.linux-mips.org/patch/13360/
> https://patchwork.linux-mips.org/patch/13361/
>
> Your patch (suitably fixed for microMIPS) would I imagine fix that issue
> too (it certainly fixes the resulting link error on microMIPS builds
> with an old toolchain).
>
> Before I forward port those patches to add .insn for MIPS, is that sort
> of approach (an arch specific asm/compiler-gcc.h to allow MIPS to
> override barrier_before_unreachable()) an acceptable fix?

That sounds fine to me. However, I would suggest making that
asm/compiler.h instead of asm/compiler-gcc.h, so we can also
use the same file to include workarounds for clang if needed.

       Arnd