On 02/27/2017 06:42 PM, Andres Freund wrote:
On 2017-02-27 12:27:48 -0500, Tom Lane wrote:
Andres Freund <and...@anarazel.de> writes:
The best theory I have so far that I have is that slab.c's idea of
StandardChunkHeader's size doesn't match what mcxt.c think it is
(because slab.c simply embeds StandardChunkHeader, but mcxt uses
MAXALIGN(sizeof(StandardChunkHeader))).  That's not good, but I don't
quite see how that'd cause the issue, since StandardChunkHeader's size
should always be properly sized.


Uh, wrong.  On a 32-bit machine with debug enabled, StandardChunkHeader
will contain 3 4-byte fields.  However, there are some such machines on
which MAXALIGN is 8.  For example, looking at termite's configure
output:

checking size of void *... 4
checking size of size_t... 4
checking size of long... 4
checking alignment of short... 2
checking alignment of int... 4
checking alignment of long... 4
checking alignment of long long int... 8
checking alignment of double... 8

axolotl's output looks similar.  I expect my old HPPA dinosaur
will show the failure as well.

Yea, I hadn't yet realized when writing that that termite actually,
despite running on ppc64, compiles a 32bit postgres. Will thus
duplicate StandardChunkHeader's contents in to slab.c :( - I don't
see an easy way around that...


I've tried this - essentially copying the StandardChunkHeader's contents into SlabChunk, but that does not seem to do the trick, sadly. Per pahole, the structures then (at least on armv7l) look like this:

    struct SlabChunk {
        void *                     block;            /*  0   4 */
        MemoryContext              context;          /*  4   4 */
        Size                       size;             /*  8   4 */
        Size                       requested_size;   /* 12   4 */

        /* size: 16, cachelines: 1, members: 4 */
        /* last cacheline: 16 bytes */
    };

    struct StandardChunkHeader {
        MemoryContext              context;          /*  0   4 */
        Size                       size;             /*  4   4 */
        Size                       requested_size;   /*  8   4 */

        /* size: 12, cachelines: 1, members: 3 */
        /* last cacheline: 12 bytes */
    };

So SlabChunk happens to be perfectly aligned (MAXIMUM_ALIGNOF=8), and so pfree() grabs the block pointer but thinks it's the context :-(

Not sure what to do about this - the only thing I can think about is splitting SlabChunk into two separate structures, and align them independently.

The attached patch does that - it probably needs a bit more work on the comments to make it commit-ready, but it fixes the test_deconding tests on the rpi3 board I'm using for testing.

regards

--
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachment: slab-fix.patch
Description: binary/octet-stream

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to