buildfarm's typedefs list has gone completely nutso

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
19 messages Options
Reply | Threaded
Open this post in threaded view
|

buildfarm's typedefs list has gone completely nutso

Tom Lane-2
The current HEAD typedefs list available from
https://buildfarm.postgresql.org/cgi-bin/typedefs.pl
has the following interesting additions compared to where
things were on July 1:

        2
        ECPGt_bytea
        connection_name
        in_addr
        pg_fprintf
        send_appname

The "2" in particular is causing seriously bad pgindent results for me.
But as far as I can tell, none of these have any justification being
marked as a typedef.

calliphoridae seems to be contributing the "2" and "pg_fprintf".
I didn't track down the rest (but calliphoridae is not to blame).

Was there any change in calliphoridae's toolchain this month?

                        regards, tom lane


Reply | Threaded
Open this post in threaded view
|

Re: buildfarm's typedefs list has gone completely nutso

Andres Freund
Hi,

On 2019-07-10 12:57:08 -0400, Tom Lane wrote:

> The current HEAD typedefs list available from
> https://buildfarm.postgresql.org/cgi-bin/typedefs.pl
> has the following interesting additions compared to where
> things were on July 1:
>
> 2
> ECPGt_bytea
> connection_name
> in_addr
> pg_fprintf
> send_appname

Huh.


> The "2" in particular is causing seriously bad pgindent results for
> me.

I haven't run pgindent, but I certainly can imagine...


> But as far as I can tell, none of these have any justification being
> marked as a typedef.
>
> calliphoridae seems to be contributing the "2" and "pg_fprintf".
> I didn't track down the rest (but calliphoridae is not to blame).

> Was there any change in calliphoridae's toolchain this month?

Hm, it has gotten gcc-9 installed recently, but calliphoridae isn't
using that. So it's probably not the compiler side. But I also see a
binutils upgrade:

2019-07-08 06:22:48 upgrade binutils-multiarch:amd64 2.31.1-16 2.32.51.20190707-1

and corresponding upgrades forall the arch specific packages. I suspect
it might be that.

I can't immediately reproduce that locally though, using the same
version of binutils. It's somewhat annoying that the buildfarm uses a
different form of computing the typedefs than src/tools/find_typedef ...

Greetings,

Andres Freund


Reply | Threaded
Open this post in threaded view
|

Re: buildfarm's typedefs list has gone completely nutso

Tom Lane-2
Andres Freund <[hidden email]> writes:
> On 2019-07-10 12:57:08 -0400, Tom Lane wrote:
>> Was there any change in calliphoridae's toolchain this month?

> Hm, it has gotten gcc-9 installed recently, but calliphoridae isn't
> using that. So it's probably not the compiler side. But I also see a
> binutils upgrade:
> 2019-07-08 06:22:48 upgrade binutils-multiarch:amd64 2.31.1-16 2.32.51.20190707-1
> and corresponding upgrades forall the arch specific packages. I suspect
> it might be that.

Yeah, a plausible theory is that the output format changed enough
to confuse our typedef-symbol-scraping code.

                        regards, tom lane


Reply | Threaded
Open this post in threaded view
|

Re: buildfarm's typedefs list has gone completely nutso

Andrew Dunstan
In reply to this post by Andres Freund


On 7/10/19 1:34 PM, Andres Freund wrote:

>
> Hm, it has gotten gcc-9 installed recently, but calliphoridae isn't
> using that. So it's probably not the compiler side. But I also see a
> binutils upgrade:
>
> 2019-07-08 06:22:48 upgrade binutils-multiarch:amd64 2.31.1-16 2.32.51.20190707-1
>
> and corresponding upgrades forall the arch specific packages. I suspect
> it might be that.
>
> I can't immediately reproduce that locally though, using the same
> version of binutils. It's somewhat annoying that the buildfarm uses a
> different form of computing the typedefs than src/tools/find_typedef ...
>


That script is notably non-portable, and hasn't seen any significant
update in a decade.


If you want to run something like the buildfarm code, see
<https://adpgtech.blogspot.com/2015/05/running-pgindent-on-non-core-code-or.html>
for some clues


I ran the client on a new Fedora 30 and it didn't produce the error.


cheers


andrew





Reply | Threaded
Open this post in threaded view
|

Re: buildfarm's typedefs list has gone completely nutso

Andres Freund
Hi,

On 2019-07-10 16:40:20 -0400, Andrew Dunstan wrote:

> On 7/10/19 1:34 PM, Andres Freund wrote:
> >
> > Hm, it has gotten gcc-9 installed recently, but calliphoridae isn't
> > using that. So it's probably not the compiler side. But I also see a
> > binutils upgrade:
> >
> > 2019-07-08 06:22:48 upgrade binutils-multiarch:amd64 2.31.1-16 2.32.51.20190707-1
> >
> > and corresponding upgrades forall the arch specific packages. I suspect
> > it might be that.
> >
> > I can't immediately reproduce that locally though, using the same
> > version of binutils. It's somewhat annoying that the buildfarm uses a
> > different form of computing the typedefs than src/tools/find_typedef ...

> That script is notably non-portable, and hasn't seen any significant
> update in a decade.

I think that's kinda what I'm complaining about... It seems like a bad
idea to have this in the buildfarm code, rather than our code. IMO the
buildfarm code should invoke an updated src/tools/find_typedef - that
way people at least can create typedefs manually locally.

Not yet sure what's actually going on, but there's something odd with
debug information afaict:

objdump -w spits out warnings for a few files, all static libraries:

../install/lib/libpgcommon.a
objdump: Warning: Location list starting at offset 0x0 is not terminated.
objdump: Warning: Hole and overlap detection requires adjacent view lists and loclists.
objdump: Warning: There are 3411 unused bytes at the end of section .debug_loc

../install/lib/libecpg.a
objdump: Warning: Hole and overlap detection requires adjacent view lists and loclists.
objdump: Warning: There are 8450 unused bytes at the end of section .debug_loc

../install/lib/libpgcommon_shlib.a
objdump: Warning: Location list starting at offset 0x0 is not terminated.
objdump: Warning: Hole and overlap detection requires adjacent view lists and loclists.
objdump: Warning: There are 3411 unused bytes at the end of section .debug_loc

../install/lib/libpgfeutils.a
objdump: Warning: Hole and overlap detection requires adjacent view lists and loclists.
objdump: Warning: There are 3090 unused bytes at the end of section .debug_loc

../install/lib/libpgport.a
objdump: Warning: Location list starting at offset 0x0 is not terminated.
objdump: Warning: Hole and overlap detection requires adjacent view lists and loclists.
objdump: Warning: There are 38 unused bytes at the end of section .debug_loc

../install/lib/libpgtypes.a
objdump: Warning: Location list starting at offset 0x0 is not terminated.
objdump: Warning: Hole and overlap detection requires adjacent view lists and loclists.
objdump: Warning: There are 13199 unused bytes at the end of section .debug_loc

../install/lib/libecpg_compat.a
objdump: Warning: Hole and overlap detection requires adjacent view lists and loclists.
objdump: Warning: There are 15001 unused bytes at the end of section .debug_loc

../install/lib/libpgport_shlib.a
objdump: Warning: Location list starting at offset 0x0 is not terminated.
objdump: Warning: Hole and overlap detection requires adjacent view lists and loclists.
objdump: Warning: There are 38 unused bytes at the end of section .debug_loc

../install/lib/libpq.a
objdump: Warning: Location list starting at offset 0x0 is not terminated.
objdump: Warning: Hole and overlap detection requires adjacent view lists and loclists.
objdump: Warning: There are 5528 unused bytes at the end of section .debug_loc

Not sure if that's related - I don't get that locally (newer compiler,
different compiler options, same binutils).

Interestingly pg_fprintf is only generated by pg_fprintf, as far as I
can tell, and the 1/2 typedefs are from libpq. The relevant entries look
like:

 <1><4b>: Abbrev Number: 4 (DW_TAG_typedef)
    <4c>   DW_AT_name        : (indirect string, offset: 0x0): USE_SSE42_CRC32C_WITH_RUNTIME_CHECK 1
    <50>   DW_AT_decl_file   : 2
    <51>   DW_AT_decl_line   : 216
    <52>   DW_AT_decl_column : 23
    <53>   DW_AT_type        : <0x57>

So I suspect it might be the corruption/misparsing of objdump that's to
blame. Kinda looks like there's an issue with the dwarf stringtable, and
thus the wrong strings (debug information for macros in this case) is
being picked up.

Greetings,

Andres Freund


Reply | Threaded
Open this post in threaded view
|

Re: buildfarm's typedefs list has gone completely nutso

Andrew Dunstan-8

On 7/10/19 8:24 PM, Andres Freund wrote:

> Hi,
>
> On 2019-07-10 16:40:20 -0400, Andrew Dunstan wrote:
>> On 7/10/19 1:34 PM, Andres Freund wrote:
>>> Hm, it has gotten gcc-9 installed recently, but calliphoridae isn't
>>> using that. So it's probably not the compiler side. But I also see a
>>> binutils upgrade:
>>>
>>> 2019-07-08 06:22:48 upgrade binutils-multiarch:amd64 2.31.1-16 2.32.51.20190707-1
>>>
>>> and corresponding upgrades forall the arch specific packages. I suspect
>>> it might be that.
>>>
>>> I can't immediately reproduce that locally though, using the same
>>> version of binutils. It's somewhat annoying that the buildfarm uses a
>>> different form of computing the typedefs than src/tools/find_typedef ...
>> That script is notably non-portable, and hasn't seen any significant
>> update in a decade.
> I think that's kinda what I'm complaining about... It seems like a bad
> idea to have this in the buildfarm code, rather than our code. IMO the
> buildfarm code should invoke an updated src/tools/find_typedef - that
> way people at least can create typedefs manually locally.




OK, I don't have a problem with that. I think the sane way to go would
be to replace it with what the buildfarm is using, more or less.



>
> Not yet sure what's actually going on, but there's something odd with
> debug information afaict:
>
> objdump -w spits out warnings for a few files, all static libraries:



I assume you mean -W rather than -w


>
> ../install/lib/libpgcommon.a
> objdump: Warning: Location list starting at offset 0x0 is not terminated.
> objdump: Warning: Hole and overlap detection requires adjacent view lists and loclists.
> objdump: Warning: There are 3411 unused bytes at the end of section .debug_loc
>


That seems new. I didn't get this in my attempt to recreate the setup of
your animal. That was on debian/buster which has binutils 2.31.1-16




> Not sure if that's related - I don't get that locally (newer compiler,
> different compiler options, same binutils).
>
> Interestingly pg_fprintf is only generated by pg_fprintf, as far as I
> can tell, and the 1/2 typedefs are from libpq. The relevant entries look
> like:
>
>  <1><4b>: Abbrev Number: 4 (DW_TAG_typedef)
>     <4c>   DW_AT_name        : (indirect string, offset: 0x0): USE_SSE42_CRC32C_WITH_RUNTIME_CHECK 1
>     <50>   DW_AT_decl_file   : 2
>     <51>   DW_AT_decl_line   : 216
>     <52>   DW_AT_decl_column : 23
>     <53>   DW_AT_type        : <0x57>
>
> So I suspect it might be the corruption/misparsing of objdump that's to
> blame. Kinda looks like there's an issue with the dwarf stringtable, and
> thus the wrong strings (debug information for macros in this case) is
> being picked up.
>

This looks like a bug in the version of objdump unless I'm reading it
wrong. Why would this be tagged as a typedef?


I would tentatively suggest that you revert to the previous version of
binutils if possible, and consider reporting a bug in the bleeding edge
of objdump.


cheers


andrew


--
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



Reply | Threaded
Open this post in threaded view
|

Re: buildfarm's typedefs list has gone completely nutso

Tom Lane-2
Andrew Dunstan <[hidden email]> writes:
> On 7/10/19 8:24 PM, Andres Freund wrote:
>> I think that's kinda what I'm complaining about... It seems like a bad
>> idea to have this in the buildfarm code, rather than our code. IMO the
>> buildfarm code should invoke an updated src/tools/find_typedef - that
>> way people at least can create typedefs manually locally.

> OK, I don't have a problem with that. I think the sane way to go would
> be to replace it with what the buildfarm is using, more or less.

+1 for the idea --- I've not studied the code, but surely the buildfarm's
version of this code is more bulletproof.

> This looks like a bug in the version of objdump unless I'm reading it
> wrong. Why would this be tagged as a typedef?

Maybe.  We still need to explain the other non-typedef symbols that have
just appeared and are not being reported by calliphoridae.

                        regards, tom lane


Reply | Threaded
Open this post in threaded view
|

Re: buildfarm's typedefs list has gone completely nutso

Andrew Dunstan-8

On 7/11/19 12:50 PM, Tom Lane wrote:
>
>> This looks like a bug in the version of objdump unless I'm reading it
>> wrong. Why would this be tagged as a typedef?
> Maybe.  We still need to explain the other non-typedef symbols that have
> just appeared and are not being reported by calliphoridae.
>


The others come from komodoensis (also Andres' machine)


pgbfprod=> select l.sysname, l.snapshot, l.branch from build_status_log
l where snapshot > now() - interval '12 days' and log_stage =
'typedefs.log' and log_text ~
'ECPGt_bytea|in_addr|connection_name|send_appname';
   sysname   |      snapshot       | branch
-------------+---------------------+--------
 komodoensis | 2019-07-08 23:34:01 | HEAD
 komodoensis | 2019-07-10 02:38:01 | HEAD
(2 rows)



cheers


andrew


--
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



Reply | Threaded
Open this post in threaded view
|

Re: buildfarm's typedefs list has gone completely nutso

Andres Freund
In reply to this post by Andres Freund
Hi,

On 2019-07-10 17:24:41 -0700, Andres Freund wrote:
> Not yet sure what's actually going on, but there's something odd with
> debug information afaict:
>
> objdump -W spits out warnings for a few files, all static libraries:
>
> ../install/lib/libpgcommon.a
> objdump: Warning: Location list starting at offset 0x0 is not terminated.
> objdump: Warning: Hole and overlap detection requires adjacent view lists and loclists.
> objdump: Warning: There are 3411 unused bytes at the end of section .debug_loc

...


Interestingly, for the same files, readelf spits out correct
data. E.g. here's a short excerpt from libpq.a:

objdump -W src/interfaces/libpq/libpq.a
...
 <0><b>: Abbrev Number: 1 (DW_TAG_compile_unit)
    <c>   DW_AT_producer    : (indirect string, offset: 0x0): SYNC_FILE_RANGE_WRITE 2
    <10>   DW_AT_language    : 12       (ANSI C99)
    <11>   DW_AT_name        : (indirect string, offset: 0x0): SYNC_FILE_RANGE_WRITE 2
    <15>   DW_AT_comp_dir    : (indirect string, offset: 0x0): SYNC_FILE_RANGE_WRITE 2
...
 <1><31>: Abbrev Number: 2 (DW_TAG_typedef)
    <32>   DW_AT_name        : Oid
    <36>   DW_AT_decl_file   : 30
    <37>   DW_AT_decl_line   : 31
    <38>   DW_AT_decl_column : 22
    <39>   DW_AT_type        : <0x3d>
 <1><3d>: Abbrev Number: 3 (DW_TAG_base_type)
    <3e>   DW_AT_byte_size   : 4
    <3f>   DW_AT_encoding    : 7        (unsigned)
    <40>   DW_AT_name        : (indirect string, offset: 0x0): SYNC_FILE_RANGE_WRITE 2
 <1><44>: Abbrev Number: 3 (DW_TAG_base_type)
    <45>   DW_AT_byte_size   : 8
    <46>   DW_AT_encoding    : 5        (signed)
    <47>   DW_AT_name        : (indirect string, offset: 0x0): SYNC_FILE_RANGE_WRITE 2
 <1><4b>: Abbrev Number: 4 (DW_TAG_typedef)
    <4c>   DW_AT_name        : (indirect string, offset: 0x0): SYNC_FILE_RANGE_WRITE 2
    <50>   DW_AT_decl_file   : 2
    <51>   DW_AT_decl_line   : 216
    <52>   DW_AT_decl_column : 23
    <53>   DW_AT_type        : <0x57>
...

readelf --debug-dump src/interfaces/libpq/libpq.a
...
 <0><b>: Abbrev Number: 1 (DW_TAG_compile_unit)
    <c>   DW_AT_producer    : (indirect string, offset: 0x29268): GNU C17 8.3.0 -march=skylake -mmmx -mno-3dnow -msse -msse2 -m
    <10>   DW_AT_language    : 12       (ANSI C99)
    <11>   DW_AT_name        : (indirect string, offset: 0x28ef3): /home/andres/src/postgresql/src/interfaces/libpq/fe-auth.c
    <15>   DW_AT_comp_dir    : (indirect string, offset: 0xf800): /home/andres/build/postgres/dev-assert/vpath/src/interfaces/l
...
 <1><31>: Abbrev Number: 2 (DW_TAG_typedef)
    <32>   DW_AT_name        : Oid
    <36>   DW_AT_decl_file   : 30
    <37>   DW_AT_decl_line   : 31
    <38>   DW_AT_decl_column : 22
    <39>   DW_AT_type        : <0x3d>
 <1><3d>: Abbrev Number: 3 (DW_TAG_base_type)
    <3e>   DW_AT_byte_size   : 4
    <3f>   DW_AT_encoding    : 7        (unsigned)
    <40>   DW_AT_name        : (indirect string, offset: 0x4f12f): unsigned int
 <1><44>: Abbrev Number: 3 (DW_TAG_base_type)
    <45>   DW_AT_byte_size   : 8
    <46>   DW_AT_encoding    : 5        (signed)
    <47>   DW_AT_name        : (indirect string, offset: 0x57deb): long int
 <1><4b>: Abbrev Number: 4 (DW_TAG_typedef)
    <4c>   DW_AT_name        : (indirect string, offset: 0x26129): size_t
    <50>   DW_AT_decl_file   : 2
    <51>   DW_AT_decl_line   : 216
    <52>   DW_AT_decl_column : 23
    <53>   DW_AT_type        : <0x57>
...

so it seems that objdump mis-parses all indirect strings - which IIRC is
something like a pointer into a "global" string table, assuming the
offset to be 0. That then just returns the first table entry.

It doesn't happen with a slightly older version of binutils. Bisecting
now.

Greetings,

Andres Freund


Reply | Threaded
Open this post in threaded view
|

Re: buildfarm's typedefs list has gone completely nutso

Andres Freund
Hi,

On 2019-07-16 19:35:39 -0700, Andres Freund wrote:

> Hi,
>
> On 2019-07-10 17:24:41 -0700, Andres Freund wrote:
> > Not yet sure what's actually going on, but there's something odd with
> > debug information afaict:
> >
> > objdump -W spits out warnings for a few files, all static libraries:
> >
> > ../install/lib/libpgcommon.a
> > objdump: Warning: Location list starting at offset 0x0 is not terminated.
> > objdump: Warning: Hole and overlap detection requires adjacent view lists and loclists.
> > objdump: Warning: There are 3411 unused bytes at the end of section .debug_loc
>
> ...
>
>
> Interestingly, for the same files, readelf spits out correct
> data. E.g. here's a short excerpt from libpq.a:
>
> objdump -W src/interfaces/libpq/libpq.a
> ...
>  <0><b>: Abbrev Number: 1 (DW_TAG_compile_unit)
>     <c>   DW_AT_producer    : (indirect string, offset: 0x0): SYNC_FILE_RANGE_WRITE 2
>     <10>   DW_AT_language    : 12       (ANSI C99)
>     <11>   DW_AT_name        : (indirect string, offset: 0x0): SYNC_FILE_RANGE_WRITE 2
>     <15>   DW_AT_comp_dir    : (indirect string, offset: 0x0): SYNC_FILE_RANGE_WRITE 2
> ...
>  <1><31>: Abbrev Number: 2 (DW_TAG_typedef)
>     <32>   DW_AT_name        : Oid
>     <36>   DW_AT_decl_file   : 30
>     <37>   DW_AT_decl_line   : 31
>     <38>   DW_AT_decl_column : 22
>     <39>   DW_AT_type        : <0x3d>
>  <1><3d>: Abbrev Number: 3 (DW_TAG_base_type)
>     <3e>   DW_AT_byte_size   : 4
>     <3f>   DW_AT_encoding    : 7        (unsigned)
>     <40>   DW_AT_name        : (indirect string, offset: 0x0): SYNC_FILE_RANGE_WRITE 2
>  <1><44>: Abbrev Number: 3 (DW_TAG_base_type)
>     <45>   DW_AT_byte_size   : 8
>     <46>   DW_AT_encoding    : 5        (signed)
>     <47>   DW_AT_name        : (indirect string, offset: 0x0): SYNC_FILE_RANGE_WRITE 2
>  <1><4b>: Abbrev Number: 4 (DW_TAG_typedef)
>     <4c>   DW_AT_name        : (indirect string, offset: 0x0): SYNC_FILE_RANGE_WRITE 2
>     <50>   DW_AT_decl_file   : 2
>     <51>   DW_AT_decl_line   : 216
>     <52>   DW_AT_decl_column : 23
>     <53>   DW_AT_type        : <0x57>

> so it seems that objdump mis-parses all indirect strings - which IIRC is
> something like a pointer into a "global" string table, assuming the
> offset to be 0. That then just returns the first table entry.
>
> It doesn't happen with a slightly older version of binutils. Bisecting
> now.

It's

commit 39f0547e554df96608dd041d2a7b3c72882fd515 (HEAD, refs/bisect/bad)
Author: Nick Clifton <[hidden email]>
Date:   2019-02-25 12:15:41 +0000

    Extend objdump's --dwarf=follow-links option so that separate debug info files will also be affected by other dump function, and symbol tables from separate debug info files will be used when disassembling the main file.
   
            * objdump.c (sym_ok): New function.
            (find_symbol_for_address): Use new function.
            (disassemble_section): Compare sections by name, not pointer.
            (dump_dwarf): Move code to initialise byte_get pointer and iterate
            over separate debug files from here to ...
            (dump_bfd): ... here.  Add parameter indicating that a separate
            debug info file is being dumped.  For main file, pull in the
            symbol tables from all separate debug info files.
            (display_object): Update call to dump_bfd.
            * doc/binutils.texi: Document extened behaviour of the
            --dwarf=follow-links option.
            * NEWS: Mention this new feature.
            * testsuite/binutils-all/objdump.WK2: Update expected output.
            * testsuite/binutils-all/objdump.exp (test_follow_debuglink): Add
            options and dump file parameters.
            Add extra test.
            * testsuite/binutils-all/objdump.WK3: New file.
            * testsuite/binutils-all/readelf.exp: Change expected output for
            readelf -wKis test.
            * testsuite/binutils-all/readelf.wKis: New file.

luckily that allowed me to find a workaround too. If objdump -W's k and K
parameters (or --dwarf=links,=follow-links) aren't enabled, the dump
comes out correctly:

objdump -WlLiaprmfFsoRtUuTgAckK /tmp/test.o|grep -A5 '(DW_TAG_compile_unit)'
 <0><b>: Abbrev Number: 1 (DW_TAG_compile_unit)
    <c>   DW_AT_producer    : (indirect string, offset: 0x0): /home/andres
    <10>   DW_AT_language    : 12       (ANSI C99)
    <11>   DW_AT_name        : (indirect string, offset: 0x0): /home/andres
    <15>   DW_AT_comp_dir    : (indirect string, offset: 0x0): /home/andres
    <19>   DW_AT_low_pc      : 0x0

objdump -WlLiaprmfFsoRtUuTgAc /tmp/test.o|grep -A5 '(DW_TAG_compile_unit)'
 <0><b>: Abbrev Number: 1 (DW_TAG_compile_unit)
    <c>   DW_AT_producer    : (indirect string, offset: 0x2a): GNU C17 9.1.0 -mtune=generic -march=x86-64 -ggdb -fasynchronous-
    <10>   DW_AT_language    : 12       (ANSI C99)
    <11>   DW_AT_name        : (indirect string, offset: 0x14): /tmp/test.c
    <15>   DW_AT_comp_dir    : (indirect string, offset: 0x0): /home/andres
    <19>   DW_AT_low_pc      : 0x0

(lLiaprmfFsoRtUuTgAckK just is all sub-parts of -W that my objdump knows
about)

looking at the .debug_str section (with -Ws):

  0x00000000 2f686f6d 652f616e 64726573 00646f75 /home/andres.dou
  0x00000010 626c6500 2f746d70 2f746573 742e6300 ble./tmp/test.c.
  0x00000020 72657431 00726574 3200474e 55204331 ret1.ret2.GNU C1
  0x00000030 3720392e 312e3020 2d6d7475 6e653d67 7 9.1.0 -mtune=g
  0x00000040 656e6572 6963202d 6d617263 683d7838 eneric -march=x8
  0x00000050 362d3634 202d6767 6462202d 66617379 6-64 -ggdb -fasy
  0x00000060 6e636872 6f6e6f75 732d756e 77696e64 nchronous-unwind
  0x00000070 2d746162 6c657300 666f6f62 61723100 -tables.foobar1.
  0x00000080 666f6f62 61723200                   foobar2.

it makes sense why in this case all strings are /home/andres, and also
why the precise symbols output into the typedef list appears to be
pretty random - it's just the first string, as the offsets are
mis-computed.  And not all strings are put into the string table, which
explains why the output still has some contents left.

It turns out that -Wi is actually all we need - so I'll probably patch
my animals to use that for now, until the bug is fixed.

It might actually be sensible to always do that - it's a lot cheaper
that way:

$ time objdump -WlLiaprmfFsoRtUuTgAc src/interfaces/libpq/libpq.a|wc
 747866 5190832 48917526

real 0m0.827s
user 0m1.040s
sys 0m0.074s

$ time objdump -Wi src/interfaces/libpq/libpq.a|wc
  78703  378433 3594563

real 0m0.075s
user 0m0.076s
sys 0m0.025s

Greetings,

Andres Freund


Reply | Threaded
Open this post in threaded view
|

Re: buildfarm's typedefs list has gone completely nutso

Andrew Dunstan-8

On 7/17/19 1:21 AM, Andres Freund wrote:

[nice forensics]

>
> It turns out that -Wi is actually all we need - so I'll probably patch
> my animals to use that for now, until the bug is fixed.
>
> It might actually be sensible to always do that - it's a lot cheaper
> that way:
>
> $ time objdump -WlLiaprmfFsoRtUuTgAc src/interfaces/libpq/libpq.a|wc
>  747866 5190832 48917526
>
> real 0m0.827s
> user 0m1.040s
> sys 0m0.074s
>
> $ time objdump -Wi src/interfaces/libpq/libpq.a|wc
>   78703  378433 3594563
>
> real 0m0.075s
> user 0m0.076s
> sys 0m0.025s
>


WFM, I'll put that in the next buildfarm client release, unless we get 
a core script to use instead in the meantime.


cheers


andrew

--
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services




Reply | Threaded
Open this post in threaded view
|

Re: buildfarm's typedefs list has gone completely nutso

Andres Freund
In reply to this post by Andres Freund
Hi,

On 2019-07-16 22:21:34 -0700, Andres Freund wrote:
> It turns out that -Wi is actually all we need - so I'll probably patch
> my animals to use that for now

did that now.


> until the bug is fixed.

Bug report: https://sourceware.org/bugzilla/show_bug.cgi?id=24818

Greetings,

Andres Freund


Reply | Threaded
Open this post in threaded view
|

Re: buildfarm's typedefs list has gone completely nutso

Andres Freund
Hi,

On 2019-07-17 10:33:02 -0700, Andres Freund wrote:
> On 2019-07-16 22:21:34 -0700, Andres Freund wrote:
> > It turns out that -Wi is actually all we need - so I'll probably patch
> > my animals to use that for now
>
> did that now.

Looks like that made the generated typedef lists sane. Any residual
complaints?

Greetings,

Andres Freund


Reply | Threaded
Open this post in threaded view
|

Re: buildfarm's typedefs list has gone completely nutso

Tom Lane-2
Andres Freund <[hidden email]> writes:
> Looks like that made the generated typedef lists sane. Any residual
> complaints?

Before ack'ing this, I've been waiting around for komodoensis to update
its typedefs list, in the hopes that the last couple of bogus entries
would go away.  It still hasn't, and I just realized from looking at its
config that you have it set to do so at most twice a week:

          'optional_steps' => {
                     'find_typedefs' => {
                                'branches' => [
                                        'HEAD'
                                      ],
                                'min_hours_since' => 25,
                                'dow' => [
                                     1,
                                     4
                                     ]
                              }
                     },

That seems a bit, er, miserly.

                        regards, tom lane


Reply | Threaded
Open this post in threaded view
|

Re: buildfarm's typedefs list has gone completely nutso

Tom Lane-2
I wrote:
> ... I just realized from looking at its
> config that you have it set to do so at most twice a week:

>                                 'dow' => [
>                                      1,
>                                      4
>                                      ]

I was still confused, seeing that today is Thursday, as to why
komodoensis didn't update its typedefs list in the run it just
finished.  Looking at the buildfarm script (in sub
check_optional_step), it seems the "dow" filter is implemented
like this:

        return
          if (exists $oconf->{dow}
                && grep { $_ eq $wday } @{ $oconf->{dow} });

I'm the world's worst Perl programmer, but isn't that backwards?
It seems like it will return undef if today matches any entry
of the dow list, making dow a blacklist of weekdays *not* to run
the step on.  That's not what I would have expected it to mean,
although build-farm.conf.sample is surely unclear on the point.

                        regards, tom lane


Reply | Threaded
Open this post in threaded view
|

Re: buildfarm's typedefs list has gone completely nutso

Andres Freund
In reply to this post by Tom Lane-2
Gum

On 2019-07-18 11:27:49 -0400, Tom Lane wrote:
> Andres Freund <[hidden email]> writes:
> > Looks like that made the generated typedef lists sane. Any residual
> > complaints?
>
> Before ack'ing this, I've been waiting around for komodoensis to update
> its typedefs list, in the hopes that the last couple of bogus entries
> would go away.  It still hasn't, and I just realized from looking at its
> config that you have it set to do so at most twice a week:

I've changed that yesterday to be just min_hours_since=48. I can easily set it
to more frequent, especially after using -Wi makes the whole step a lot
faster - I just though that swamping the buildfarm with the additional
data would be unnecessary? But perhaps it doesn't matter in comparison
to the normal stage logs.

Boosted to min_hours_since=24 for now.


>           'optional_steps' => {
>                      'find_typedefs' => {
>                                 'branches' => [
>                                         'HEAD'
>                                       ],
>                                 'min_hours_since' => 25,
>                                 'dow' => [
>                                      1,
>                                      4
>                                      ]
>                               }
>                      },
>
> That seems a bit, er, miserly.

Comes from the default suggestion in build-farm.conf.sample:
                # find_typedefs => { branches => ['HEAD'], dow => [1,4],
                #                                      min_hours_since => 25 },


I'm happy to boost that to something a lot more aggressive.


At least the REL_11_STABLE logs are from after the change to use -Wi
instead of -W, and therefore ought to be correct.

bf@andres-postgres-edb-buildfarm-v1:~/src/pgbuildfarm-client-stock$ TZ=UTC ls -l run_build.pl
-rwxr-xr-x 1 bf bf 63701 Jul 17 17:16 run_build.pl

komodoensis 2019-07-17 03:04:01         HEAD            3308
komodoensis 2019-07-17 17:32:02         REL_12_STABLE   3296
komodoensis 2019-07-17 23:18:24         REL_11_STABLE   3204

Greetings,

Andres Freund


Reply | Threaded
Open this post in threaded view
|

Re: buildfarm's typedefs list has gone completely nutso

Daniel Gustafsson
In reply to this post by Tom Lane-2
> On 18 Jul 2019, at 17:42, Tom Lane <[hidden email]> wrote:

> return
>  if (exists $oconf->{dow}
> && grep { $_ eq $wday } @{ $oconf->{dow} });
>
> I'm the world's worst Perl programmer, but isn't that backwards?
> It seems like it will return undef if today matches any entry
> of the dow list, making dow a blacklist of weekdays *not* to run
> the step on.

As it’s in a scalar context, grep will return the number of times for which the
condition is true against the list entries, so if $oconf->{dow} doesn’t have
duplicates it will return 1 if $wday is found on the list.  This will in turn
make the condition true as the exists expression must be true for grep to at
all execute.

cheers ./daniel

Reply | Threaded
Open this post in threaded view
|

Re: buildfarm's typedefs list has gone completely nutso

Tom Lane-2
In reply to this post by Andres Freund
Andres Freund <[hidden email]> writes:
> On 2019-07-18 11:27:49 -0400, Tom Lane wrote:
>> Before ack'ing this, I've been waiting around for komodoensis to update
>> its typedefs list, in the hopes that the last couple of bogus entries
>> would go away.  It still hasn't, and I just realized from looking at its
>> config that you have it set to do so at most twice a week:

> Boosted to min_hours_since=24 for now.

OK, I see it's updated, and we now have a sane typedefs list again:

$ diff -u src/tools/pgindent/typedefs.list new
--- src/tools/pgindent/typedefs.list    2019-07-02 10:32:57.078918638 -0400
+++ new/typedefs.list   2019-07-18 12:45:22.533451991 -0400
@@ -674,6 +674,12 @@
 FmgrBuiltin
 FmgrHookEventType
 FmgrInfo
+ForBothCellState
+ForBothState
+ForEachState
+ForFiveState
+ForFourState
+ForThreeState
 ForeignDataWrapper
 ForeignKeyCacheInfo
 ForeignKeyOptInfo
@@ -2981,7 +2987,7 @@
 leaf_item
 line_t
 lineno_t
-list_qsort_comparator
+list_sort_comparator
 locale_t
 locate_agg_of_level_context
 locate_var_of_level_context
@@ -3150,6 +3156,7 @@
 pullup_replace_vars_context
 pushdown_safety_info
 qsort_arg_comparator
+qsort_comparator
 query_pathkeys_callback
 radius_attribute
 radius_packet

All of those changes are correct, so we're good.  Thanks!

(I still think the buildfarm's dow filter is wrong though)

                        regards, tom lane


Reply | Threaded
Open this post in threaded view
|

Re: buildfarm's typedefs list has gone completely nutso

Andrew Dunstan-8
In reply to this post by Daniel Gustafsson

On 7/18/19 12:17 PM, Daniel Gustafsson wrote:

>> On 18 Jul 2019, at 17:42, Tom Lane <[hidden email]> wrote:
>> return
>>  if (exists $oconf->{dow}
>> && grep { $_ eq $wday } @{ $oconf->{dow} });
>>
>> I'm the world's worst Perl programmer, but isn't that backwards?
>> It seems like it will return undef if today matches any entry
>> of the dow list, making dow a blacklist of weekdays *not* to run
>> the step on.
> As it’s in a scalar context, grep will return the number of times for which the
> condition is true against the list entries, so if $oconf->{dow} doesn’t have
> duplicates it will return 1 if $wday is found on the list.  This will in turn
> make the condition true as the exists expression must be true for grep to at
> all execute.
>


Yes, it's a bug. Will fix.


cheers


andrew

--
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services