linux-kernel - Re: [GIT PULL] optimize 64-by-32 ddivision for constant divisors on 32-bit machines

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] [day] [month] [year] [list]

Date:	Tue, 24 Nov 2015 11:44:34 -0500 (EST)
From:	Nicolas Pitre <nicolas.pitre@...aro.org>
To:	Arnd Bergmann <arnd@...db.de>
cc:	Russell King - ARM Linux <linux@....linux.org.uk>,
	linux-arch@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [GIT PULL] optimize 64-by-32 ddivision for constant divisors on
 32-bit machines

On Tue, 24 Nov 2015, Arnd Bergmann wrote:

> On Tuesday 24 November 2015 00:37:17 Nicolas Pitre wrote:
> > OK... I produced a minimal test case. I think gcc is screwed. And it is 
> > not a question of __builtin_constant_p being best effort. The resulting 
> > code is plain wrong where a variable is suddenly turned into a constant 
> > of value 0. Any random modification to various part of the code just 
> > makes it disappear but I didn't check the disassembly to see if it is 
> > then correct.
> > 
> > And the good news(tm) is that the same bug is triggered with x86 gcc as 
> > well.
> > 
> > Please try the attached test case.
> 
> I can confirm the behavior you see with gcc-4.9 and later (I only saw
> it on 5.x or later with the kernel code). 

It is really sensitive to code modifications. You simplify it somehow 
and the bug is gone.  So there might be some kind of complexity threshold 
that gcc-4.9 didn't reach with the kernel code.

> It seems to have been
> introduced with
> 
> https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=206941
> 
> 
>         PR tree-optimization/59597
>         * tree-ssa-threadupdate.c (dump_jump_thread_path): Move to earlier
>         in file.  Accept new argument REGISTERING and use it to modify
>         dump output appropriately.
>         (register_jump_thread): Corresponding changes.
>         (mark_threaded_blocks): Reinstate code to cancel unprofitable
>         thread paths involving joiner blocks.  Add code to dump cancelled
>         jump threading paths.
>     
>         PR tree-optimization/59597
>         * gcc.dg/tree-ssa/pr59597.c: New test.
>     
>     git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@...941 138bc75d-0d04-0410-961f-82ee72b054a4
> 
>  gcc/ChangeLog                           |  11 +++++
>  gcc/testsuite/ChangeLog                 |   5 +++
>  gcc/testsuite/gcc.dg/tree-ssa/pr59597.c |  55 +++++++++++++++++++++++++
>  gcc/tree-ssa-threadupdate.c             | 125 +++++++++++++++++++++++++++++++++++++++------------------
>  4 files changed, 157 insertions(+), 39 deletions(-)
> 
> however, in that version, the -DHIDE_THE_BUG variant also fails:
> 
> compilation that works:
> gcc-cross -Wall -I /home/arnd/git/buildall/arm/gcc/gcc/include  -O2 -c -o /home/arnd/git/buildall/arm/gcc_testcase/src.o /home/arnd/git/buildall/arm/gcc_testcase/src.c -DHIDE_THE_BUG
> /home/arnd/git/buildall/arm/gcc_testcase/src.c: In function 'il_send_rxon_timing':
> /home/arnd/git/buildall/arm/gcc_testcase/src.c:39:30: error: call to '____ilog2_NaN' declared with attribute error: gcc is screwed

IMHO the fact that the -DHIDE_THE_BUG case is enough to make the bug go 
away sometimes is even more worrisome given the nature of the 
"workaround".

Given you have a good handle with the bad commits already, are you going 
to bring it up with the gcc people?


Nicolas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/