From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <SRS0=pqrr=6U=suse.de=rguenther@sourceware.org>
Received: from smtp-out2.suse.de (smtp-out2.suse.de [IPv6:2001:67c:2178:6::1d])
	by sourceware.org (Postfix) with ESMTPS id A18503858284
	for <gcc-patches@gcc.gnu.org>; Fri, 24 Feb 2023 14:13:20 +0000 (GMT)
DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A18503858284
Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de
Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de
Received: from relay2.suse.de (relay2.suse.de [149.44.160.134])
	by smtp-out2.suse.de (Postfix) with ESMTP id 7AA0860868;
	Fri, 24 Feb 2023 14:13:19 +0000 (UTC)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa;
	t=1677247999; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc:
	 mime-version:mime-version:content-type:content-type:
	 in-reply-to:in-reply-to:references:references;
	bh=dsr+DxkgZbLo0XTueKy5xQniFUFWPWndkDvzGwrOn3I=;
	b=XK/8/U3IV0dTfWwOHSsXR7YEDOGWKnvXqPnHhfV5WnO3pCgqU5QgSNCpBozQSrwKj3v97e
	khuC+rjvyOqltyFQmpLe//44OegnHGeBNwMUb6lA7jo6j2a8hwuxAee2yQQxw+4JzzUtS5
	m9IV2jzFpyeBOKOgWWOf7ynHA62f8UY=
DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de;
	s=susede2_ed25519; t=1677247999;
	h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc:
	 mime-version:mime-version:content-type:content-type:
	 in-reply-to:in-reply-to:references:references;
	bh=dsr+DxkgZbLo0XTueKy5xQniFUFWPWndkDvzGwrOn3I=;
	b=MvW+2GMY93KNT4jxntIKockjFV/04jpGEGnDMuuisYyept8VUmOT1I1mVakPYmIpxLTsz8
	/DcCQ9+Da2hQSMBQ==
Received: from wotan.suse.de (wotan.suse.de [10.160.0.1])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(No client certificate requested)
	by relay2.suse.de (Postfix) with ESMTPS id 572A52C141;
	Fri, 24 Feb 2023 14:13:19 +0000 (UTC)
Date: Fri, 24 Feb 2023 14:13:19 +0000 (UTC)
From: Richard Biener <rguenther@suse.de>
To: Jakub Jelinek <jakub@redhat.com>
cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH 2/2] Avoid default-initializing auto_vec<T, N> storage,
 fix vec<vl_embed>
In-Reply-To: <Y/jE72v6t7+8s0jG@tucnak>
Message-ID: <nycvar.YFH.7.77.849.2302241412110.27913@jbgna.fhfr.qr>
References: <20230224134739.D386F13246@imap2.suse-dmz.suse.de> <Y/jE72v6t7+8s0jG@tucnak>
User-Agent: Alpine 2.22 (LSU 394 2020-01-19)
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
X-Spam-Status: No, score=-5.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org
List-Id: <gcc-patches.gcc.gnu.org>

On Fri, 24 Feb 2023, Jakub Jelinek wrote:

> On Fri, Feb 24, 2023 at 02:47:39PM +0100, Richard Biener wrote:
> > 	* vec.h (vec<T, A, vl_embed>::m_vecdata): Remove.
> > 	(vec<T, A, vl_embed>::m_vecpfx): Align as T to avoid
> > 	changing alignment of vec<T, A, vl_embed> and simplifying
> > 	address.
> > 	(vec<T, A, vl_embed>::address): Compute as this + 1.
> > 	(vec<T, A, vl_embed>::embedded_size): Use sizeof the
> > 	vector instead of the offset of the m_vecdata member.
> > 	(auto_vec<T, N>::m_data): Turn storage into
> > 	uninitialized unsigned char.
> > 	(auto_vec<T, N>::auto_vec): Allow allocation of one
> > 	stack member.  Initialize m_vec in a special way to
> > 	avoid later stringop overflow diagnostics.
> > 	* vec.cc (test_auto_alias): New.
> > 	(vec_cc_tests): Call it.
> > @@ -1559,8 +1560,14 @@ class auto_vec : public vec<T, va_heap>
> >  public:
> >    auto_vec ()
> >    {
> > -    m_auto.embedded_init (MAX (N, 2), 0, 1);
> > -    this->m_vec = &m_auto;
> > +    m_auto.embedded_init (N, 0, 1);
> > +    /* ???  Instead of initializing m_vec from &m_auto directly use an
> > +       expression that avoids refering to a specific member of 'this'
> > +       to derail the -Wstringop-overflow diagnostic code, avoiding
> > +       the impression that data accesses are supposed to be to the
> > +       m_auto memmber storage.  */
> 
> s/memmber/member/
> 
> > +    size_t off = (char *) &m_auto - (char *) this;
> > +    this->m_vec = (vec<T, va_heap, vl_embed> *) ((char *) this + off);
> >    }
> >  
> >    auto_vec (size_t s CXX_MEM_STAT_INFO)
> > @@ -1571,7 +1578,7 @@ public:
> >  	return;
> >        }
> >  
> > -    m_auto.embedded_init (MAX (N, 2), 0, 1);
> > +    m_auto.embedded_init (N, 0, 1);
> >      this->m_vec = &m_auto;
> 
> Don't we need the above 2 lines here as well (perhaps with a shorter comment
> just referencing the earlier comment)?

I've noticed that as well and put it there now, it wasn't necessary
to get bootstrap working.

> Otherwise LGTM, thanks.

Thanks,
Richard.