From patchwork Sun Mar 30 18:38:39 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Linus Torvalds X-Patchwork-Id: 3909991 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 66D859F2B6 for ; Sun, 30 Mar 2014 18:38:43 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 929FF201DE for ; Sun, 30 Mar 2014 18:38:42 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id E07882018E for ; Sun, 30 Mar 2014 18:38:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751668AbaC3Sil (ORCPT ); Sun, 30 Mar 2014 14:38:41 -0400 Received: from mail-vc0-f174.google.com ([209.85.220.174]:32862 "EHLO mail-vc0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751645AbaC3Sik (ORCPT ); Sun, 30 Mar 2014 14:38:40 -0400 Received: by mail-vc0-f174.google.com with SMTP id ld13so7430757vcb.19 for ; Sun, 30 Mar 2014 11:38:39 -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=+uCtyUt2PiRfhl2+HxAfGDfdUHLFSTYhbO3OL+AwlCk=; b=ldLS4+g62AwJERrdjL8uiyqC6KWsH4FQ1npnvy2FKEdX0ocuQi1OTXyEPClt5pQOu3 vDO5kUQbjkIyoO63FNlKs87JSSHyT5qQTmcQO5qJlOXpw3GIz/jJkGZEe5uiowEA9Fv3 K7WAYmSrE4G3DMt7g5sPaki5KL+MXAJHi64ctu1kkZKcLfCBbXIlryzXNcS4gsn9H4oK KP+iShNfRwovd8oLRmjViNvIL6xGcGMeUGwmy9FAPdICTsEN0gt0/CoDdSJbfSP2k2iP aRUOVHrpGgn1UteqXa9FJuYuNBsZkj6SxwCphz7qQ5TRhbGXokCjsCb/ZMT/aWyHEKGF 5Cug== 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=+uCtyUt2PiRfhl2+HxAfGDfdUHLFSTYhbO3OL+AwlCk=; b=WikiBd2T4X2aBZoKB+ppFvoTLo9q5V9x6fINjcMsRNk32y4buJ7kkKhm2w0pUjGQUc Flii3JXYt1bKlmdboV48QuuR06sKcXoBr3fvnf7/ahjHTuDN9lyzg0eGl+Iz4mM7neWF FFv4F5w37PLzEUAM16XDRYMma2pAnmCifxCx8= MIME-Version: 1.0 X-Received: by 10.220.11.208 with SMTP id u16mr71947vcu.19.1396204719866; Sun, 30 Mar 2014 11:38:39 -0700 (PDT) Received: by 10.220.13.2 with HTTP; Sun, 30 Mar 2014 11:38:39 -0700 (PDT) In-Reply-To: References: Date: Sun, 30 Mar 2014 11:38:39 -0700 X-Google-Sender-Auth: My5n0TJXJ2dIgN6rFmSH9a0g05w Message-ID: Subject: Re: [PATCH 2/2] Use any previous initializer to size a symbol From: Linus Torvalds To: Christopher Li , Hans Verkuil Cc: Linux-Sparse 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.3 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 Sun, Mar 30, 2014 at 10:22 AM, Linus Torvalds wrote: > > This seems to be the simplest approach. Chris, what was your concern about > scoping? The "same_symbol" list should already take scoping into account. Ho humm. I found a test-case that still messes up: extern const char *v4l2_type_names[]; const char *v4l2_type_names[] = { "test", "hello" }; static unsigned test(void) { extern const char *v4l2_type_names[]; return sizeof(v4l2_type_names); } and the reason seems to be that the 'same_symbol' list is incomplete. What happen sis that we tie the two "extern" v4l2_type_names[] things together, and the declaration of v4l2_type_names[] with the initializer is also tied to the first 'extern' declaration, but we do *not* tie the second 'extern' in block scope to the one with the initializer. The reason seems to be that our logic in check_declaration[] is broken: it only ties things together based on both symbols having 'extern'. So the actual declaration with an initializer, that lacks the extern (but is in global scope) is missed. The attached patch would seem to fix it. Chris, what is the reason for that MOD_INLINE check? You added it in commit 8cf99394ee4c ("move extern inline function to file scope") and I suspect it messes up the same_symbol logic. But I left it alone, and added a comment. If you can resolve that comment, please apply this patch with my sign-off and some random commit message.. Linus symbol.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/symbol.c b/symbol.c index 4b91abd8021e..c5e4784734a8 100644 --- a/symbol.c +++ b/symbol.c @@ -576,11 +576,15 @@ void check_declaration(struct symbol *sym) sym->same_symbol = next; return; } - if (sym->ctype.modifiers & next->ctype.modifiers & MOD_EXTERN) { - if ((sym->ctype.modifiers ^ next->ctype.modifiers) & MOD_INLINE) - continue; - sym->same_symbol = next; - return; + /* Extern in block level matches a TOPLEVEL non-static symbol */ + if (sym->ctype.modifiers & MOD_EXTERN) { + if ((next->ctype.modifiers & (MOD_TOPLEVEL|MOD_STATIC)) == MOD_TOPLEVEL) { + /* FIXME? Differs in 'inline' only? Why does that matter? */ + if ((sym->ctype.modifiers ^ next->ctype.modifiers) & MOD_INLINE) + continue; + sym->same_symbol = next; + return; + } } if (!Wshadow || warned)