Re: [HACKERS] Broken atomics code on PPC with FreeBSD 10.3

Andres Freund <andres@anarazel.de> writes:
> On 2016-12-30 00:44:33 -0500, Tom Lane wrote:
>> Perhaps it could be argued that there's a FreeBSD compiler bug here,
>> but what I'm wondering is why configure believes that this:
>> 
>> [char lock = 0;
>> __sync_lock_test_and_set(&lock, 1);
>> __sync_lock_release(&lock);])],
>> 
>> is going to draw a hard error on platforms without byte-wide atomics.
>> My sense of C semantics is that the best you could hope for is a
>> warning

> Well, in theory these aren't actual functions but intrinsics with
> special behaviour ("by being overloaded so that they work on multiple
> types."). The compiler is supposed to warn and link to an external
> function if a certain operation is not available:

Yeah, I read that.  But configure tests don't normally notice warnings.
Therefore, even if the compiler is behaving fully per spec (and I think
FreeBSD's might not be), we are going to think that char and word-wide
operations are interchangeable even when one is an efficient, in-line
operation and the other is inefficiently emulated by a function or
kernel trap.  This doesn't really seem good enough, especially for
standard architectures where we know darn well what ought to happen.

There's a reason why we've not converted s_lock.h to rely on compiler
intrinsics everywhere, and this is exactly it: the quality of the
intrinsics implementations are horridly variable.

> So that assumption made in that configure test doesn't seem that
> unreasonable.  What assembler do byte-wide atomics generate on that
> platform?  Which compiler/version is this?

$ gcc -v
Using built-in specs.
Target: powerpc-undermydesk-freebsd
Configured with: FreeBSD/powerpc system compiler
Thread model: posix
gcc version 4.2.1 20070831 patched [FreeBSD]

I tried "gcc -Wall -O0 -g -S bogus.c" on this input:

int
test_tas_char(volatile char *ptr)
{
  return __sync_lock_test_and_set(ptr, 1) == 0;
}

int
test_tas_int(volatile int *ptr)
{
  return __sync_lock_test_and_set(ptr, 1) == 0;
}

and got no warnings and the attached output.  I'm not very good at reading
PPC assembler, but I think what is happening in the "char" case is that
gcc is trying to emulate a byte-wide operation using a word-wide one,
ie an lwarx/stwcx. loop.  Even if that works (and apparently it does not),
we would not want to use it since it implies contention between adjacent
atomic flags.

BTW, I looked around to see why the PPC critters in the buildfarm aren't
failing, and the answer is that they all have compilers that are too old
to have __sync_lock_test_and_set at all, so that the configure probes
just fail outright.  But the probes can't tell the difference between
implementations we want to use and implementations we don't.

            regards, tom lane

    .file    "bogus.c"
    .section    .debug_abbrev,"",@progbits
.Ldebug_abbrev0:
    .section    .debug_info,"",@progbits
.Ldebug_info0:
    .section    .debug_line,"",@progbits
.Ldebug_line0:
    .section    ".text"
.Ltext0:
    .align 2
    .globl test_tas_char
    .type    test_tas_char, @function
test_tas_char:
.LFB2:
    .file 1 "bogus.c"
    .loc 1 3 0
    stwu 1,-32(1)
.LCFI0:
    stw 31,28(1)
.LCFI1:
    mr 31,1
.LCFI2:
    stw 3,8(31)
    .loc 1 4 0
    lwz 7,8(31)
    stw 7,12(31)
    lwz 8,12(31)
    lbz 8,0(8)
    stb 8,17(31)
.L2:
    lbz 9,17(31)
    stb 9,16(31)
    li 11,1
    lwz 10,12(31)
    rlwinm 9,10,3,27,28
    xori 9,9,24
    lbz 0,16(31)
    rlwinm 10,0,0,24,31
    slw 10,10,9
    rlwinm 0,11,0,0xff
    rlwinm 11,0,0,24,31
    slw 11,11,9
    li 0,255
    slw 0,0,9
    lwz 7,12(31)
    rlwinm 9,7,0,0,29
    sync
.L4:
    lwarx 8,0,9
    and 7,8,0
    cmpw 0,7,10
    bne- 0,.L5
    andc 8,8,0
    or 8,8,11
    stwcx. 8,0,9
    bne- 0,.L4
    isync
.L5:
    mr 0,7
    stb 0,17(31)
    lbz 9,17(31)
    lbz 0,16(31)
    cmpw 7,9,0
    bne 7,.L2
    lbz 0,16(31)
    cmpwi 7,0,0
    mfcr 0
    rlwinm 0,0,31,1
    .loc 1 5 0
    mr 3,0
    lwz 11,0(1)
    lwz 31,-4(11)
    mr 1,11
    blr
.LFE2:
    .size    test_tas_char,.-test_tas_char
    .align 2
    .globl test_tas_int
    .type    test_tas_int, @function
test_tas_int:
.LFB3:
    .loc 1 9 0
    stwu 1,-32(1)
.LCFI3:
    stw 31,28(1)
.LCFI4:
    mr 31,1
.LCFI5:
    stw 3,8(31)
    .loc 1 10 0
    lwz 9,8(31)
    li 10,1
    sync
.L8:
    lwarx 0,0,9
    mr 11,10
    stwcx. 11,0,9
    bne- 0,.L8
    isync
    cmpwi 7,0,0
    mfcr 0
    rlwinm 0,0,31,1
    .loc 1 11 0
    mr 3,0
    lwz 11,0(1)
    lwz 31,-4(11)
    mr 1,11
    blr
.LFE3:
    .size    test_tas_int,.-test_tas_int
    .section    .debug_frame,"",@progbits
.Lframe0:
    .4byte    .LECIE0-.LSCIE0
.LSCIE0:
    .4byte    0xffffffff
    .byte    0x1
    .string    ""
    .uleb128 0x1
    .sleb128 -4
    .byte    0x6c
    .byte    0xc
    .uleb128 0x1
    .uleb128 0x0
    .align 2
.LECIE0:
.LSFDE0:
    .4byte    .LEFDE0-.LASFDE0
.LASFDE0:
    .4byte    .Lframe0
    .4byte    .LFB2
    .4byte    .LFE2-.LFB2
    .byte    0x4
    .4byte    .LCFI0-.LFB2
    .byte    0xe
    .uleb128 0x20
    .byte    0x4
    .4byte    .LCFI1-.LCFI0
    .byte    0x9f
    .uleb128 0x1
    .byte    0x4
    .4byte    .LCFI2-.LCFI1
    .byte    0xd
    .uleb128 0x1f
    .align 2
.LEFDE0:
.LSFDE2:
    .4byte    .LEFDE2-.LASFDE2
.LASFDE2:
    .4byte    .Lframe0
    .4byte    .LFB3
    .4byte    .LFE3-.LFB3
    .byte    0x4
    .4byte    .LCFI3-.LFB3
    .byte    0xe
    .uleb128 0x20
    .byte    0x4
    .4byte    .LCFI4-.LCFI3
    .byte    0x9f
    .uleb128 0x1
    .byte    0x4
    .4byte    .LCFI5-.LCFI4
    .byte    0xd
    .uleb128 0x1f
    .align 2
.LEFDE2:
    .section    ".text"
.Letext0:
    .section    .debug_loc,"",@progbits
.Ldebug_loc0:
.LLST0:
    .4byte    .LFB2-.Ltext0
    .4byte    .LCFI0-.Ltext0
    .2byte    0x1
    .byte    0x51
    .4byte    .LCFI0-.Ltext0
    .4byte    .LCFI2-.Ltext0
    .2byte    0x2
    .byte    0x71
    .sleb128 32
    .4byte    .LCFI2-.Ltext0
    .4byte    .LFE2-.Ltext0
    .2byte    0x2
    .byte    0x8f
    .sleb128 32
    .4byte    0x0
    .4byte    0x0
.LLST1:
    .4byte    .LFB3-.Ltext0
    .4byte    .LCFI3-.Ltext0
    .2byte    0x1
    .byte    0x51
    .4byte    .LCFI3-.Ltext0
    .4byte    .LCFI5-.Ltext0
    .2byte    0x2
    .byte    0x71
    .sleb128 32
    .4byte    .LCFI5-.Ltext0
    .4byte    .LFE3-.Ltext0
    .2byte    0x2
    .byte    0x8f
    .sleb128 32
    .4byte    0x0
    .4byte    0x0
    .section    .debug_info
    .4byte    0xdf
    .2byte    0x2
    .4byte    .Ldebug_abbrev0
    .byte    0x4
    .uleb128 0x1
    .string    "GNU C 4.2.1 20070831 patched [FreeBSD]"
    .byte    0x1
    .string    "bogus.c"
    .string    "/home/tgl"
    .4byte    .Ltext0
    .4byte    .Letext0
    .4byte    .Ldebug_line0
    .uleb128 0x2
    .byte    0x1
    .string    "test_tas_char"
    .byte    0x1
    .byte    0x3
    .byte    0x1
    .4byte    0x88
    .4byte    .LFB2
    .4byte    .LFE2
    .4byte    .LLST0
    .4byte    0x88
    .uleb128 0x3
    .string    "ptr"
    .byte    0x1
    .byte    0x2
    .4byte    0x8f
    .byte    0x2
    .byte    0x91
    .sleb128 -24
    .byte    0x0
    .uleb128 0x4
    .byte    0x4
    .byte    0x5
    .string    "int"
    .uleb128 0x5
    .byte    0x4
    .4byte    0x95
    .uleb128 0x6
    .4byte    0x9a
    .uleb128 0x4
    .byte    0x1
    .byte    0x8
    .string    "char"
    .uleb128 0x2
    .byte    0x1
    .string    "test_tas_int"
    .byte    0x1
    .byte    0x9
    .byte    0x1
    .4byte    0x88
    .4byte    .LFB3
    .4byte    .LFE3
    .4byte    .LLST1
    .4byte    0xd7
    .uleb128 0x3
    .string    "ptr"
    .byte    0x1
    .byte    0x8
    .4byte    0xd7
    .byte    0x2
    .byte    0x91
    .sleb128 -24
    .byte    0x0
    .uleb128 0x5
    .byte    0x4
    .4byte    0xdd
    .uleb128 0x6
    .4byte    0x88
    .byte    0x0
    .section    .debug_abbrev
    .uleb128 0x1
    .uleb128 0x11
    .byte    0x1
    .uleb128 0x25
    .uleb128 0x8
    .uleb128 0x13
    .uleb128 0xb
    .uleb128 0x3
    .uleb128 0x8
    .uleb128 0x1b
    .uleb128 0x8
    .uleb128 0x11
    .uleb128 0x1
    .uleb128 0x12
    .uleb128 0x1
    .uleb128 0x10
    .uleb128 0x6
    .byte    0x0
    .byte    0x0
    .uleb128 0x2
    .uleb128 0x2e
    .byte    0x1
    .uleb128 0x3f
    .uleb128 0xc
    .uleb128 0x3
    .uleb128 0x8
    .uleb128 0x3a
    .uleb128 0xb
    .uleb128 0x3b
    .uleb128 0xb
    .uleb128 0x27
    .uleb128 0xc
    .uleb128 0x49
    .uleb128 0x13
    .uleb128 0x11
    .uleb128 0x1
    .uleb128 0x12
    .uleb128 0x1
    .uleb128 0x40
    .uleb128 0x6
    .uleb128 0x1
    .uleb128 0x13
    .byte    0x0
    .byte    0x0
    .uleb128 0x3
    .uleb128 0x5
    .byte    0x0
    .uleb128 0x3
    .uleb128 0x8
    .uleb128 0x3a
    .uleb128 0xb
    .uleb128 0x3b
    .uleb128 0xb
    .uleb128 0x49
    .uleb128 0x13
    .uleb128 0x2
    .uleb128 0xa
    .byte    0x0
    .byte    0x0
    .uleb128 0x4
    .uleb128 0x24
    .byte    0x0
    .uleb128 0xb
    .uleb128 0xb
    .uleb128 0x3e
    .uleb128 0xb
    .uleb128 0x3
    .uleb128 0x8
    .byte    0x0
    .byte    0x0
    .uleb128 0x5
    .uleb128 0xf
    .byte    0x0
    .uleb128 0xb
    .uleb128 0xb
    .uleb128 0x49
    .uleb128 0x13
    .byte    0x0
    .byte    0x0
    .uleb128 0x6
    .uleb128 0x35
    .byte    0x0
    .uleb128 0x49
    .uleb128 0x13
    .byte    0x0
    .byte    0x0
    .byte    0x0
    .section    .debug_pubnames,"",@progbits
    .4byte    0x31
    .2byte    0x2
    .4byte    .Ldebug_info0
    .4byte    0xe3
    .4byte    0x52
    .string    "test_tas_char"
    .4byte    0xa2
    .string    "test_tas_int"
    .4byte    0x0
    .section    .debug_aranges,"",@progbits
    .4byte    0x1c
    .2byte    0x2
    .4byte    .Ldebug_info0
    .byte    0x4
    .byte    0x0
    .2byte    0x0
    .2byte    0x0
    .4byte    .Ltext0
    .4byte    .Letext0-.Ltext0
    .4byte    0x0
    .4byte    0x0
    .ident    "GCC: (GNU) 4.2.1 20070831 patched [FreeBSD]"
    .section    .note.GNU-stack,"",@progbits

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