From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 126849 invoked by alias); 29 Aug 2018 07:29:38 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 126689 invoked by uid 89); 29 Aug 2018 07:29:37 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-3.0 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-lf1-f65.google.com Received: from mail-lf1-f65.google.com (HELO mail-lf1-f65.google.com) (209.85.167.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 29 Aug 2018 07:29:35 +0000 Received: by mail-lf1-f65.google.com with SMTP id r4-v6so3441944lff.12 for ; Wed, 29 Aug 2018 00:29:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=aTTdRYv9zAeAl2b3lKi/Ir6GHqhYmaRvuieCBoqF/wQ=; b=kfMEH69UWuFuIEf6IGmlvxYPODe4gPoPMgmokHPDP+XMPC1XAefnjRixkcKcEpWUA3 woWZ7gbmoKKFCK37PCKB9NVSjk2orJe0vFyrzQguVtg8w7i+dEUG0DmzVaLWbr/THUHL 9+JAlz0ZAqOr1/0UAmagVaky0X98nGWPK32jyMndn15Wi2FtZuErTPtRI2Coh4FH4Qrr mZ5PzJEgTNr7F1zxehH58Xp47t8ZuCMGb+g4E9s01RYWwc8w3YJEoXt9M05sqkPjMIX2 8vqHPUC0cJEa5wJJKrEika3hZRABBJUHyk9qL4nlXgM/Os0rC0C0pIEQ/t8Q2V559aF4 XpHA== MIME-Version: 1.0 References: <88de1ee3-6ee4-d8d9-3e57-3a42474a4169@redhat.com> <4e613d61-40ad-f05f-9107-5fd7a5c2fdb6@gmail.com> <6aaa70ca-af7f-2b34-43bc-953a02defe03@gmail.com> In-Reply-To: <6aaa70ca-af7f-2b34-43bc-953a02defe03@gmail.com> From: Richard Biener Date: Wed, 29 Aug 2018 07:29:00 -0000 Message-ID: Subject: Re: [PATCH] avoid warning on constant strncpy until next statement is reachable (PR 87028) To: Martin Sebor Cc: Jeff Law , GCC Patches Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2018-08/txt/msg01826.txt.bz2 On Wed, Aug 29, 2018 at 2:12 AM Martin Sebor wrote: > > >> Sadly, dstbase is the PARM_DECL for d. That's where things are going > >> "wrong". Not sure why you're getting the PARM_DECL in that case. I'd > >> debug get_addr_base_and_unit_offset to understand what's going on. > >> Essentially you're getting different results of > >> get_addr_base_and_unit_offset in a case where they arguably should be > >> the same. > > > > Probably get_attr_nonstring_decl has the same "mistake" and returns > > the PARM_DECL instead of the SSA name pointer. So we're comparing > > apples and oranges here. > > Returning the SSA_NAME_VAR from get_attr_nonstring_decl() is > intentional but the function need not (perhaps should not) > also set *REF to it. > > > > > Yeah: > > > > /* If EXPR refers to a character array or pointer declared attribute > > nonstring return a decl for that array or pointer and set *REF to > > the referenced enclosing object or pointer. Otherwise returns > > null. */ > > > > tree > > get_attr_nonstring_decl (tree expr, tree *ref) > > { > > tree decl = expr; > > if (TREE_CODE (decl) == SSA_NAME) > > { > > gimple *def = SSA_NAME_DEF_STMT (decl); > > > > if (is_gimple_assign (def)) > > { > > tree_code code = gimple_assign_rhs_code (def); > > if (code == ADDR_EXPR > > || code == COMPONENT_REF > > || code == VAR_DECL) > > decl = gimple_assign_rhs1 (def); > > } > > else if (tree var = SSA_NAME_VAR (decl)) > > decl = var; > > } > > > > if (TREE_CODE (decl) == ADDR_EXPR) > > decl = TREE_OPERAND (decl, 0); > > > > if (ref) > > *ref = decl; > > > > I see a lot of "magic" here again in the attempt to "propagate" > > a nonstring attribute. > > That's the function's purpose: to look for the attribute. Is > there a better way to do this? Well, the question is what "nonstring" is, semantically. I read it as sth like __restrinct - a pointer with "nonstring" attribute points to a non-string. So I suspect your function either computes "may expr point to a nonstring" or "must expr point to a nonstring" if it gets a pointer argument. If it gets a (string) object it checks whether that object is declared "nonstring" (thus, if you'd built a pointer to expr whether that pointer _must_ point to a nonstring. So I guess the first one is "must". Clearly looking at SSA_NAME_VAR isn't good here, it would be semantically correct only for SSA_NAME_IS_DEFAULT_DEF and SSA_NAME_VAR being a PARM_DECL. I guess it would be nice to clearly separate the pointer vs. object case by documentation in the function - all of the quoted parts above seem to be for the address case so a gcc_assert (POINTER_TYPE_P (TREE_TYPE (decl)) inside the if (TREE_CODE (decl) == SSA_NAME) path should never trigger? > > Note > > > > foo (char *p __attribute__(("nonstring"))) > > { > > p = "bar"; > > strlen (p); // or whatever is necessary to call get_attr_nonstring_decl > > } > > > > is perfectly valid and p as passed to strlen is _not_ nonstring(?). > > I don't know if you're saying that it should get a warning or > shouldn't. Right now it doesn't because the strlen() call is > folded before we check for nonstring. I say it shouldn't because I assign "bar" to p and after that p isn't the original parameter anymore? > I could see an argument for diagnosing it but I suspect you > wouldn't like it because it would mean more warning from > the folder. I could also see an argument against it because, > as you said, it's safe. > > If you take the assignment to p away then a warning is issued, > and that's because p is declared with attribute nonstring. > That's also why get_attr_nonstring_decl looks at SSA_NAME_VAR. > > > I think in your code comparing bases you want to look at the _original_ > > argument to the string function rather than what get_attr_nonstring_decl > > returned as ref. > > I've adjusted get_attr_nonstring_decl() to avoid setting *REF > to SSA_NAME_VAR. That let me remove the GIMPLE_NOP code from > the patch. I've also updated the comment above SSA_NAME_VAR > to clarify its purpose per Jeff's comments. > > Attached is an updated revision with these changes. > > Martin