From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x542.google.com (mail-ed1-x542.google.com [IPv6:2a00:1450:4864:20::542]) by sourceware.org (Postfix) with ESMTPS id C616C3951C71 for ; Tue, 23 Jun 2020 07:13:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org C616C3951C71 Received: by mail-ed1-x542.google.com with SMTP id k8so15558544edq.4 for ; Tue, 23 Jun 2020 00:13:02 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=vFbKnftKTQ9fPFAaaXbmPp0TqwO97d+0l2Q3YuK834c=; b=UruLoCFC/zPNF0V2C8CTrP/FlsrsVdiTkH9JTlnzElZpuYfsGivilTnBYzCY56lsm8 jE27i58O9JXvBUjwQjtCdUOKRkio6bG2A79Z015eToGTBulstKySrnzIFkeoQuRWr/8M G0RL9F9fNPfCTdo8xeWCLamVZT9xwB5bDsAQccElU8tBV9sDbH8DlgMkAN23U3ZMC5Oa vc2it9qxuZVoQ8rXcH0PAyoy2E138aji6bisqeBznQCcBIEGYaoL+EwbRSOzrcn+7T/R iMRWcITsEEb19TwzhJvsUe7f9C0mwp4SvcD6ybj56PFoD3XDFzIRisvhYzjfT3JRTdGI 4Fwg== X-Gm-Message-State: AOAM532/cqqVdK7CDOW/s+FJGKN9zI+/qdoFc+nlWOq3hEFMzSgbCWYi NFAnGnul2NN1bhuVMGTrrXYeOpds3+BPHYExinY= X-Google-Smtp-Source: ABdhPJy5rJs1zNvK9Xto8k0bMNLKd/asZfWF+55m6i3vEiXftgtFodgb9QiHptFPs6n8fJ+M202IXGFsuYHDG471YHc= X-Received: by 2002:a50:fd19:: with SMTP id i25mr20309185eds.248.1592896381770; Tue, 23 Jun 2020 00:13:01 -0700 (PDT) MIME-Version: 1.0 References: <29c9b3fa-69a2-dca9-1477-54aac80c8680@gmail.com> In-Reply-To: <29c9b3fa-69a2-dca9-1477-54aac80c8680@gmail.com> From: Richard Biener Date: Tue, 23 Jun 2020 09:12:50 +0200 Message-ID: Subject: Re: [PATCH] handle MEM_REF with void* arguments (PR c++/95768) To: Martin Sebor Cc: Jason Merrill , gcc-patches Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-2.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 23 Jun 2020 07:13:04 -0000 On Tue, Jun 23, 2020 at 12:22 AM Martin Sebor via Gcc-patches wrote: > > On 6/22/20 12:55 PM, Jason Merrill wrote: > > On 6/22/20 1:25 PM, Martin Sebor wrote: > >> The attached fix parallels the one for the equivalent C bug 95580 > >> where the pretty printers don't correctly handle MEM_REF arguments > >> with type void* or other pointers to an incomplete type. > >> > >> The incorrect handling was exposed by the recent change to > >> -Wuninitialized which includes such expressions in diagnostics. > > > >> + if (tree size =3D TYPE_SIZE_UNIT (TREE_TYPE (argtype))) > >> + if (!integer_onep (size)) > >> + { > >> + pp_cxx_left_paren (pp); > >> + dump_type (pp, ptr_type_node, flags); > >> + pp_cxx_right_paren (pp); > >> + } > > > > Don't we want to print the cast if the pointer target type is incomplet= e? > > I suppose, yes, although after some more testing I think what should > be output is the type of the access. The target pointer type isn't > meaningful (at least not in this case). > > Here's what the warning looks like in C for the test case in > gcc.dg/pr95580.c: > > warning: =E2=80=98*((void *)(p)+1)=E2=80=99 may be used uninitialized > > and like this in C++: > > warning: =E2=80=98*(p +1)=E2=80=99 may be used uninitialized > > The +1 is a byte offset, which is correct given that incrementing > a void* in GCC is the same as adding 1 to the byte address, but > dereferencing a void* doesn't correspond to what's going on in > the source. > > Even for a complete type (with size greater than 1), printing > the type of the argument plus a byte offset is wrong. It ends > up with this for the C++ test case from 95768: > > warning: =E2=80=98*((int*) +4)=E2=80=99 is used uninitialized > > when the access is actually =E2=80=98*((int*) +1)=E2=80=99 > > So it seems to me for MEM_REF, to make the output meaningful, > it's the type of the access (i.e., the MEM_REF type) that should > be printed here, and the offset should either be in elements of > the accessed type, i.e., > > warning: =E2=80=98*((int*) +1)=E2=80=99 is used uninitialized > > or, if the access is misaligned, the argument should first be > cast to char*, the offset added, and the result then cast to > the access type, like this: > > warning: =E2=80=98*(T*)((char*) +1)=E2=80=99 is used uninitia= lized > > The attached revised and less than fully tested patch implements > this for C++ only for now. If we agree on this approach I'll see > about making the corresponding change in C. Note that there is no C/C++ way of fully expressing MEM_REF semantics. __MEM ((T *)p + 1) is not actually *(int *)((char *)p + 1) because that does not reflect that the effective type of the lvalue when TBAA is concerned is 'T' rather than 'int'. Note for MEM_REF the offset is always a constant byte offset but it indeed does not have to be a multiple of the MEM_REF type size. I wonder whether printing the MEM_REF in full provides any real diagnostic value in the more "obfuscated" cases. I'd also not print but . Richard. > Martin