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:37:17 +0100 Message-ID: <5192683D02000078000D6118@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> <5192666A02000078000D60F8@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <5192666A02000078000D60F8@nat28.tlf.novell.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 16:29, "Jan Beulich" wrote: >>>> 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. Actually, the part about structures without members was wrong - gcc allocates a byte for each instance nevertheless (albeit it also only allocates a single byte for an array of such structures, i.e. the individual array members are indistinguishable from their addresses). Jan