Re: master make check fails on Solaris 10 - Mailing list pgsql-hackers

Sorry, diff.patch is attached now.

On 17-01-2018 18:02, Marina Polyakova wrote:
> [I added Victor Wagner as co-researcher of this problem]
> 
> On 13-01-2018 21:10, Tom Lane wrote:
>> In the end this might just be an instance of the old saw about
>> avoiding dot-zero releases.  Have you tried a newer gcc?
>> (Digging in their bugzilla finds quite a number of __int128 bugs
>> fixed in 5.4.x, though none look to be specifically about
>> misaligned data.)
> 
> gcc 5.5.0 (from [1]) did not fix the problem..
> 
> On 16-01-2018 2:41, Tom Lane wrote:
>> Marina Polyakova <m.polyakova@postgrespro.ru> writes:
>>> On 13-01-2018 21:10, Tom Lane wrote:
>>>> I'm not sure there's much we can do about this.  Dropping the use
>>>> of the alignment spec isn't a workable option.  If there were a
>>>> simple way for configure to detect that the compiler generates bad
>>>> code for that, we could have it do so and reject use of __int128,
>>>> but it'd be up to you to come up with a workable test.
>> 
>>> I'll think about it..
>> 
>> Attached is a possible test program.  I can confirm it passes on a
>> machine with working __int128, but I have no idea whether it will
>> detect the problem on yours.  If not, maybe you can tweak it?
> 
> Thank you! Using gcc 5.5.0 it prints that everything is ok. But,
> investigating the regression diffs, we found out that the error occurs
> when we pass int128 as not the first argument to the function (perhaps
> its value is replaced by the value of some address):
> 
> -- Use queries from random.sql
> SELECT count(*) FROM onek; -- Everything is ok
> ...
> SELECT random, count(random) FROM RANDOM_TBL
>   GROUP BY random HAVING count(random) > 3; -- Everything is ok
> 
> postgres=# SELECT * FROM RANDOM_TBL ORDER BY random; -- Print current 
> data
>  random
> --------
>      78
>      86
>      98
>      98
> (4 rows)
> 
> postgres=# SELECT AVG(random) FROM RANDOM_TBL
> postgres-#   HAVING AVG(random) NOT BETWEEN 80 AND 120; -- Oops!
>               avg
> -------------------------------
>  79446934848446476698976780288
> (1 row)
> 
> Debug output from the last query (see attached diff.patch, it is based
> on commit 9c7d06d60680c7f00d931233873dee81fdb311c6 of master):
> 
> makeInt128AggState
> int8_avg_accum val 98
> int8_avg_accum val_int128 as 2 x int64: 0 98
> int8_avg_accum val_int128 bytes: 00000000000000000000000000000062
> int8_avg_accum state 100e648d8
> int8_avg_accum 1007f2e94
> do_int128_accum int128 newval as 2 x int64: 4306826968 0
> do_int128_accum int128 newval bytes: 0000000100B4F6D80000000000000000
> do_int128_accum state 100e648d8
> do_int128_accum 1007f1e30
> int8_avg_accum val 86
> int8_avg_accum val_int128 as 2 x int64: 0 86
> int8_avg_accum val_int128 bytes: 00000000000000000000000000000056
> int8_avg_accum state 100e648d8
> int8_avg_accum 1007f2e94
> do_int128_accum int128 newval as 2 x int64: 4306826968 0
> do_int128_accum int128 newval bytes: 0000000100B4F6D80000000000000000
> do_int128_accum state 100e648d8
> do_int128_accum 1007f1e30
> int8_avg_accum val 98
> int8_avg_accum val_int128 as 2 x int64: 0 98
> int8_avg_accum val_int128 bytes: 00000000000000000000000000000062
> int8_avg_accum state 100e648d8
> int8_avg_accum 1007f2e94
> do_int128_accum int128 newval as 2 x int64: 4306826968 0
> do_int128_accum int128 newval bytes: 0000000100B4F6D80000000000000000
> do_int128_accum state 100e648d8
> do_int128_accum 1007f1e30
> int8_avg_accum val 78
> int8_avg_accum val_int128 as 2 x int64: 0 78
> int8_avg_accum val_int128 bytes: 0000000000000000000000000000004E
> int8_avg_accum state 100e648d8
> int8_avg_accum 1007f2e94
> do_int128_accum int128 newval as 2 x int64: 4306826968 0
> do_int128_accum int128 newval bytes: 0000000100B4F6D80000000000000000
> do_int128_accum state 100e648d8
> do_int128_accum 1007f1e30
> numeric_poly_avg
> int128_to_numericvar
> int128_to_numericvar int128 val as 2 x int64: 17227307872 0
> int128_to_numericvar int128 val bytes: 0000000402D3DB600000000000000000
> 
> (val_int128 in the function int8_avg_accum is correct, but newval in
> the function do_int128_accum is not equal to it. val in the function
> int128_to_numericvar is (4 * 4306826968).)
> 
> Based on this, we modified the test program (see attached). Here is
> its output on Solaris 10 for different alignments requirements for
> int128 (on my machine where make check-world passes everything is OK)
> (ALIGNOF_PG_INT128_TYPE is 16 on Solaris 10):
> 
> $ gcc -D PG_ALIGN_128=16 -m64 -o int128test2 int128test2.c
> $ ./int128test2
> basic aritmetic OK
> pass int 16 OK
> pass uint 16 OK
> pass int 32 OK
> pass int 64 OK
> pass int 128 OK
> $ gcc -D PG_ALIGN_128=8 -m64 -o int128test2 int128test2.c
> $ ./int128test2
> basic aritmetic OK
> pass int 16 FAILED
> pass uint 16 FAILED
> pass int 32 FAILED
> pass int 64 FAILED
> pass int 128 OK
> 
> Maybe some pass test from int128test2.c can be used to test __int128?
> 
> P.S. I suppose, g.b should be 97656250 to get 400000000005:
> 
>> struct glob128
>> {
>>     __int128    start;
>>     char        pad;
>>     int128a        a;
>>     int128a        b;
>>     int128a        c;
>>     int128a        d;
>> } g = {0, 'p', 48828125, 97656255, 0, 0};
>> ...
>> g.b = (g.b << 12) + 5;        /* 400000000005 */
> 
> [1] https://www.opencsw.org

-- 
Marina Polyakova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company