From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH 4/4] xen/arm: correctly handle an empty array of platform descs. Date: Tue, 14 May 2013 11:21:20 +0100 Message-ID: <1368526880.14264.29.camel@zakaz.uk.xensource.com> References: <1368462160.537.125.camel@zakaz.uk.xensource.com> <1368462182-10317-4-git-send-email-ian.campbell@citrix.com> <1368523975.14264.22.camel@zakaz.uk.xensource.com> <5192248402000078000D5E7F@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <5192248402000078000D5E7F@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: "Keir (Xen.org)" , Stefano Stabellini , Ian Jackson , "Tim (Xen.org)" , "xen-devel@lists.xen.org" , Julien Grall List-Id: xen-devel@lists.xenproject.org On Tue, 2013-05-14 at 10:48 +0100, Jan Beulich wrote: > >>> On 14.05.13 at 11:32, Ian Campbell wrote: > > Sadly this turns out to be a bit more complicated :-( CCing Jan and Keir > > since this was wider implications (this construct is used elsewhere) and > > Ian J because he pointed this out. > > > > TL;DR: There is probably a problem here, but I'm not sure it is one > > worth worrying about, at least not yet/for 4.3. > > > > The problem is that the compiler is allowed to assume that: > > extern const struct platform_desc _splatform[], _eplatform[]; > > Defines two *distinct* arrays, which therefore can never be equal. Hence > > it optimises > > for ( platform = _splatform; platform != _eplatform; platform++ ) > > with the assumption that there must always be at least one iteration. > > Did you actually observe gcc doing this? Yes, leading to an infinite loop. With my "fixed" version (with the <) it just did one needless iteration of whatever happened to be at _splatform =_eplatform, which luckily was accessible data. It happens to be the device data (which likely suffers the same issue). I've also semi-confirmed by staring at the disassembly... > I agree that the abstract > C specification allows for such an assumption, but with the gcc > itself providing ways to create symbols aliases I would be pretty > surprised if it actually then produces broken code here. > > > FWIW this seems to work: > > for ( platform = &_splatform[0]; platform < &_eplatform[0]; platform++ ) > > But I'm not sure it is strictly any less undefined than the original > > code and it might just be obscuring things enough that today's optimiser > > misses a trick. > > This surely is as (un)defined as the original code. As I suspected! Ian.