From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: [PATCH 4/4] xen/arm: correctly handle an empty array of platform descs. Date: Tue, 14 May 2013 15:29:30 +0100 Message-ID: <5192666A02000078000D60F8@nat28.tlf.novell.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> <1368526880.14264.29.camel@zakaz.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1368526880.14264.29.camel@zakaz.uk.xensource.com> Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell 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 14.05.13 at 12:21, Ian Campbell wrote: > 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... Just compiled (-O2) this extern unsigned _s[], _e[]; void u(unsigned*); void test(void) { unsigned*p; for(p = _s; p != _e; ++p) u(p); } with gcc 4.7.3 for ix86, x86-64, and arm32, and none of them produced an infinite loop. And as said before, I'd be surprised a non-buggy gcc would, considering that with empty structures (which gcc allows) you can have zero-sized objects, and hence two named objects at the same address. And similarly, they support attributes to create symbol aliases, and for those it would again be necessary to account for the addresses of two distinct symbols to actually resolve to the same address. What compiler version and switches were you using for the case where you spotted bad code to be generated? Jan