pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Jeff Davis <pgsql@j-davis.com>
To: Adam Lee <ali@pivotal.io>
Cc: pgsql-hackers@postgresql.org, Peter Geoghegan <pg@bowt.ie>
Cc: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Subject: Re: Add LogicalTapeSetExtend() to logtape.c
Date: Sat, 07 Mar 2020 13:00:55 -0800
Message-ID: <91ca648cfd1f99bf07981487a7d81a1ec926caad.camel@j-davis.com> (raw)
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>

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-att...








view thread (8+ messages)

Message-ID: <91ca648cfd1f99bf07981487a7d81a1ec926caad.camel@j-davis.com>
Permalink:  ../91ca648cfd1f99bf07981487a7d81a1ec926caad.camel@j-davis.com/
Also on:    postgresql.org/message-id/91ca648cfd1f99bf07981487a7d81a1ec926caad.camel@j-davis.com

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: pgsql@j-davis.com, ali@pivotal.io, pg@bowt.ie, heikki.linnakangas@iki.fi
  Subject: Re: Add LogicalTapeSetExtend() to logtape.c
  In-Reply-To: <91ca648cfd1f99bf07981487a7d81a1ec926caad.camel@j-davis.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox