Message-ID: <91ca648cfd1f99bf07981487a7d81a1ec926caad.camel@j-davis.com>
Subject: Re: Add LogicalTapeSetExtend() to logtape.c
From: Jeff Davis <pgsql@j-davis.com>
To: Adam Lee <ali@pivotal.io>
Cc: pgsql-hackers@postgresql.org, Peter Geoghegan <pg@bowt.ie>, Heikki
	Linnakangas <heikki.linnakangas@iki.fi>
Date: Sat, 07 Mar 2020 13:00:55 -0800
In-Reply-To: <91f9ae9935c715d5638954f52e0e33c4d1dc1388.camel@j-davis.com>
References: <e54bfec11c59689890f277722aaaabd05f78e22c.camel@j-davis.com>
	 <20200228061641.GA883@earth.local>
	 <619e61919fd604c2caa629536ee87615b732e054.camel@j-davis.com>
	 <20200303014935.GA1022@earth.local>
	 <f68211cc9a413c23460f1058967993f8f0b52a2a.camel@j-davis.com>
	 <20200304035719.GB1022@earth.local>
	 <91f9ae9935c715d5638954f52e0e33c4d1dc1388.camel@j-davis.com>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Wed, 2020-03-04 at 12:06 -0800, Jeff Davis wrote:
> I tracked the problem down.

Because of the name _FORTIFY_SOURCE, I got somewhat concerned that this
change (logtape-20200303) was somehow interfering with a safety
mechanism in the compiler.

There's a mechanism in GCC called object size tracking[1]. memcpy() is
replaced by __builtin___memcpy_chk(), which verifies that the amount
being copied doesn't exceed the destination object size -- but of
course this only works if GCC knows the destination object size. If it
doesn't know the object size, or if it can prove at compile time that
it will never be exceeded, then it replaces the checked memcpy with a
call to normal memcpy (at least [1] seems to suggest that it will).

But I did some experiments and GCC is clearly not able to know the
destination object size either before or after the logtape-20200303
change:

 * palloc (and friends) lack the alloc_size function attribute[2],
which is required for GCC to try to track it (aside: maybe we should
add it as an unrelated change?)
 * if I add the alloc_size attribute to palloc, it is able to do very
basic tracking of the object size; but not in more complex cases like
the buffer in logtape.c

Therefore, from [1], I'd expect the call to checked memcpy to be
replaced by a call to normal memcpy() either before or after the
change.

It is replaced by normal memcpy() before the change, but not after.
I conclude that this is arbitrary and not fundamentally related to
object size checking or _FORTIFY_SOURCE.

I don't think I should hold up this change because of an arbitrary
decision by the compiler.

Regards,
	Jeff Davis

[1] https://gcc.gnu.org/onlinedocs/gcc/Object-Size-Checking.html
[2] 
https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-alloc_005fsize-function-attribute