From patchwork Sat Aug 2 01:16:08 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Linus Torvalds X-Patchwork-Id: 4664981 Return-Path: X-Original-To: patchwork-linux-sparse@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 52E459F36A for ; Sat, 2 Aug 2014 01:16:12 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 958AE2020A for ; Sat, 2 Aug 2014 01:16:11 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id B1E2420200 for ; Sat, 2 Aug 2014 01:16:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751889AbaHBBQK (ORCPT ); Fri, 1 Aug 2014 21:16:10 -0400 Received: from mail-vc0-f170.google.com ([209.85.220.170]:42025 "EHLO mail-vc0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751334AbaHBBQJ (ORCPT ); Fri, 1 Aug 2014 21:16:09 -0400 Received: by mail-vc0-f170.google.com with SMTP id lf12so7979573vcb.1 for ; Fri, 01 Aug 2014 18:16:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=xyM5C1LIjCfI76KppvzRe+xhnnhPSCjpKTzYNeJrrnA=; b=KxY39lsCgP138LWp1iY9mmJHsWNXDlEHSWe4Apzuuax4ZIv6R5n4kY4pdnJBEn6cjf ZNdUQM6Uj/LK5UgsCkfIWWMaZOuPOjYAqbNpfsuW28gjjqBE0Cq0fuD0fs5ms/Ml7MDE 2ljf0yJ/LxSeTycs2rco1N3rxcyqfTBWs+SFB2BJvKKagCW9so9W/Gd8b255TaU7psU+ ZT0Lp4mK+aRkHnj+2xwtZ3xrvb9zSkvrvCy24OV4zXAf/GgWsIVRJteKreVXKhPJY9w+ 1GxotTpxe9O1VgwCoCAQZgTGQzxVyEHjxJ3/hkgq9mdTAeLxi+eZCdioOePChKzaJOpG Ddxg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux-foundation.org; s=google; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=xyM5C1LIjCfI76KppvzRe+xhnnhPSCjpKTzYNeJrrnA=; b=cHp+AjPIM31zWTaO3uMFIB3Q1w9J/cnZVnW7//SEf03jYGIJr8byCA49R5HMuLN1+9 4v+JsksKwyxR97x4PjqomVw6CM5CJptc1vXK2wqBmXXcYDpoegnP5Ep5QLnEKSMflyvI jKknCkldWWQC6SRvSY1zYMpaGSAmWJRJ7bH9M= MIME-Version: 1.0 X-Received: by 10.221.24.135 with SMTP id re7mr10872587vcb.53.1406942168115; Fri, 01 Aug 2014 18:16:08 -0700 (PDT) Received: by 10.221.58.194 with HTTP; Fri, 1 Aug 2014 18:16:08 -0700 (PDT) In-Reply-To: References: <20140731181006.GA13180@cloud> Date: Fri, 1 Aug 2014 18:16:08 -0700 X-Google-Sender-Auth: rUB9jxnE9xs9GCm_7-ET07ieUt4 Message-ID: Subject: Re: Designated initializers for fields in anonymous structs and unions From: Linus Torvalds To: Josh Triplett , Chris Li Cc: Sparse Mailing-list Sender: linux-sparse-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-sparse@vger.kernel.org X-Spam-Status: No, score=-7.4 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,T_DKIM_INVALID,T_TVD_MIME_EPI, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Thu, Jul 31, 2014 at 7:41 PM, Linus Torvalds wrote: > > Hmm. It exposes other problems. > > For example, the fact that we look up the identifier now exposes the > fact that "first_subobject()" creates this dummy "EXPR_IDENTIFIER" > expression that has no identifier. That will then cause > "find_identifier()" to fail, which in turn causes convert_ident() to > fail. Resulting in the parse tree having remaining EXPR_IDENTIFIER > nodes, which will later result in "unknown expression (25,0)" errors. > > That's easy enough to fix with just adding the proper initializer, ie > > new->expr_ident = field->ident; > > to first_subobject(). Actually, sadly, while that worked for my test-cases (and for Josh's case), it turns out to really *not* work in general: the dummy EXPR_IDENTIFIER that we create may end up referring to a unnamed union, which doesn't *have* an ident associated with it. This only really triggers when you mix named initializers with then relying on the whole "initialize the next one without a name", but that does happen. And it fundamentally breaks that whole "use find_identifier()" approach. So convert_ident() really cannot use find_indent(), and really does have to use "sym->offset". But that doesn't work for anonymous union members, because that offset was the offset within the anonymous union, not the parent structure. However, the fix is "simple". We can just say that anonymous union/structure member offsets have to be the offsets within the parent union instead, and fix things up. So how about a patch like this *instead* of the previous patch.. Linus evaluate.c | 2 +- symbol.c | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/evaluate.c b/evaluate.c index 03992d03ae1d..78150f83a19f 100644 --- a/evaluate.c +++ b/evaluate.c @@ -1898,7 +1898,7 @@ static struct symbol *find_identifier(struct ident *ident, struct symbol_list *_ sub = find_identifier(ident, ctype->symbol_list, offset); if (!sub) continue; - *offset += sym->offset; + *offset = sym->offset; return sub; } } diff --git a/symbol.c b/symbol.c index a2731348a011..32af64050a64 100644 --- a/symbol.c +++ b/symbol.c @@ -116,6 +116,18 @@ static int bitfield_base_size(struct symbol *sym) return sym->bit_size; } +static void update_symbol_offsets(struct symbol *sym, unsigned int offset) +{ + struct symbol *field; + + if (sym->type != SYM_STRUCT && sym->type != SYM_UNION) + return; + FOR_EACH_PTR(sym->symbol_list, field) { + field->offset += offset; + update_symbol_offsets(field, offset); + } END_FOR_EACH_PTR(field); +} + /* * Structures are a bit more interesting to lay out */ @@ -174,6 +186,10 @@ static void lay_out_struct(struct symbol *sym, struct struct_union_info *info) bit_size = (bit_size + align_bit_mask) & ~align_bit_mask; sym->offset = bits_to_bytes(bit_size); + /* If it's an unnamed struct or union, update the offsets of the sub-members */ + if (sym->offset && !sym->ident) + update_symbol_offsets(sym, sym->offset); + info->bit_size = bit_size + base_size; // warning (sym->pos, "regular: offset=%d", sym->offset); }