Re: Add LogicalTapeSetExtend() to logtape.c

From: Jeff Davis <pgsql(at)j-davis(dot)com>
To: Adam Lee <ali(at)pivotal(dot)io>
Cc: pgsql-hackers(at)postgresql(dot)org, Peter Geoghegan <pg(at)bowt(dot)ie>, Heikki Linnakangas <heikki(dot)linnakangas(at)iki(dot)fi>
Subject: Re: Add LogicalTapeSetExtend() to logtape.c
Date: 2020-03-07 21:00:55
Message-ID: 91ca648cfd1f99bf07981487a7d81a1ec926caad.camel@j-davis.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2020-03-07 21:06:32 Re: range_agg
Previous Message Julien Rouhaud 2020-03-07 20:53:28 Re: Add an optional timeout clause to isolationtester step.