From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751040AbdAPQZh (ORCPT ); Mon, 16 Jan 2017 11:25:37 -0500 Received: from merlin.infradead.org ([205.233.59.134]:40058 "EHLO merlin.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750817AbdAPQZf (ORCPT ); Mon, 16 Jan 2017 11:25:35 -0500 Date: Mon, 16 Jan 2017 17:25:04 +0100 From: Peter Zijlstra To: David Laight Cc: "'Anton Blanchard'" , "behanw@converseincode.com" , "ying.huang@intel.com" , "akpm@linux-foundation.org" , "oleg@redhat.com" , Segher Boessenkool , "mingo@elte.hu" , "linuxppc-dev@lists.ozlabs.org" , "linux-kernel@vger.kernel.org" Subject: Re: llist code relies on undefined behaviour, upsets llvm/clang Message-ID: <20170116162504.GA6500@twins.programming.kicks-ass.net> References: <20170116083600.47175073@kryten> <063D6719AE5E284EB5DD2968C1650D6DB0264AA5@AcuExch.aculab.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DB0264AA5@AcuExch.aculab.com> User-Agent: Mutt/1.5.23.1 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Jan 16, 2017 at 02:34:43PM +0000, David Laight wrote: > From: Anton Blanchard > > Sent: 15 January 2017 21:36 > > I was debugging a hang on a ppc64le kernel built with clang, and it > > looks to be undefined behaviour with pointer wrapping in the llist code. > > > > A test case is below. llist_for_each_entry() does container_of() on a > > NULL pointer, which wraps our pointer negative, then adds the same > > offset back in and expects to get back to NULL. Unfortunately clang > > decides that this can never be NULL and optimises it into an infinite > > loop. > ... > > #define llist_for_each_entry(pos, node, member) \ > > for ((pos) = llist_entry((node), typeof(*(pos)), member); \ > > &(pos)->member != NULL; \ > > (pos) = llist_entry((pos)->member.next, typeof(*(pos)), member)) > > Maybe the above could be rewritten as (untested): > for ((pos) = NULL; (!(pos) ? (node) : ((pos)->member.next) || (pos) = 0) && \ > (((pos) = !(pos) ? llist_entry((node), typeof(*(pos)), member) \ > : llist_entry((pos)->member.next, typeof(*(pos)), member)),1); ) > Provided the compiler assumes that the loop body is never executed with 'pos == 0' > it should generate the same code. That's far uglier code and to what point? The compiler should simply not assume silly things.