Thread: relocation truncated to fit: citus build failure on s390x
[Cc'ing -hackers] I said: > That said, there's a build failure on s390x: > > https://buildd.debian.org/status/fetch.php?pkg=citus&arch=s390x&ver=5.0.1-1&stamp=1461670278 > > gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute > -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -g -g -O2 -fstack-protector-strong -Wformat > -Werror=format-security -I/usr/include/mit-krb5 -fPIC -pie -fpic -g -O2 -fstack-protector-strong -Wformat > -Werror=format-security -Wall -Wextra -Wno-unused-parameter -Wno-sign-compare -Wno-missing-field-initializers > -Wno-clobbered -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wmissing-declarations > -Wmissing-prototypes -shared -o citus.so ./shared_library_init.o commands/create_distributed_table.o > commands/transmit.o executor/multi_utility.o executor/multi_router_executor.o executor/multi_server_executor.o > executor/multi_real_time_executor.o executor/multi_executor.o executor/multi_task_tracker_executor.o > executor/multi_client_executor.o master/master_stage_protocol.o master/master_metadata_utility.o > master/master_delete_protocol.o master/master_repair_shards.o master/master_create_shards.o > master/worker_node_manager.o master/master_node_protocol.o planner/multi_physical_planner.o > planner/multi_master_planner.o planner/multi_explain.o planner/multi_logical_planner.o planner/multi_planner.o > planner/multi_join_order.o planner/modify_planner.o planner/multi_logical_optimizer.o relay/relay_event_utility.o > test/generate_ddl_commands.o test/connection_utils.o test/prune_shard_list.o test/distribution_metadata.o > test/test_helper_functions.o test/connection_cache.o test/fake_fdw.o test/create_shards.o utils/listutils.o > utils/citus_readfuncs_94.o utils/ruleutils_94.o utils/multi_resowner.o utils/resource_lock.o utils/citus_outfuncs.o > utils/metadata_cache.o utils/connection_cache.o utils/citus_read.o utils/citus_ruleutils.o utils/citus_nodefuncs.o > utils/citus_readfuncs_95.o utils/ruleutils_95.o worker/worker_partition_protocol.o worker/task_tracker_protocol.o > worker/task_tracker.o worker/worker_file_access_protocol.o worker/worker_data_fetch_protocol.o > worker/worker_merge_protocol.o -L/usr/lib/s390x-linux-gnu -Wl,-z,relro -Wl,-z,now -Wl,--as-needed -L/usr/lib/mit-krb5 > -L/usr/lib/s390x-linux-gnu/mit-krb5 -Wl,--as-needed -Wl,-z,relro -L/usr/lib/s390x-linux-gnu -lpq > utils/ruleutils_95.o: In function `simple_quote_literal': > /«PKGBUILDDIR»/src/backend/distributed/utils/ruleutils_95.c:6114:(.text+0x596): relocation truncated to fit: > R_390_GOT12 against undefined symbol `standard_conforming_strings' > > I've had that problem before, but forgot most of it. I think the > problem is that "-fPIC -pie -fpic" isn't consistent and should > probably be "-fPIC -PIE", but I might be totally wrong. (I'll try to > dig up some docs.) Re: Andres Freund 2016-04-28 <20160427230516.44fuzdxlifch2wwp@alap3.anarazel.de> > > Just talked to a s390x porter, the flags used should be -fPIC -pie", > > i.e. without the lower case version. > > Afaics, that's citus independent, and coming from postgres code. > Makefile.port: > ifeq "$(findstring sparc,$(host_cpu))" "sparc" > CFLAGS_SL = -fPIC > else > CFLAGS_SL = -fpic > endif > > I'm not clear why citus triggers this, when other extensions don't? Maybe it's simply because citus.so is bigger than all the other extension .so files: -fpic Generate position-independent code (PIC) suitable for use in a shared library, if supported for the target machine. Such code accesses all constant addresses through a global offset table (GOT). The dynamic loader resolves the GOT entries when the program starts (the dynamic loader is not part of GCC; it is part of the operating system). If the GOT size for the linked executable exceeds a machine- specific maximum size, you get an error message from the linker indicating that -fpic does not work; in that case, recompile with -fPIC instead. (These maximums are 8k on the SPARC and 32k on the m68k and RS/6000. The 386 has no such limit.) Position-independent code requires special support, and therefore works only on certain machines. For the 386, GCC supports PIC for System V but not for the Sun 386i. Code generated for the IBM RS/6000 is always position-independent. When this flag is set, the macros "__pic__" and "__PIC__" are defined to 1. -fPIC If supported for the target machine, emit position-independent code, suitable for dynamic linking and avoiding any limit on the size of the global offset table. This option makes a difference on the m68k, PowerPC and SPARC. Position-independent code requires special support, and therefore works only on certain machines. When this flag is set, the macros "__pic__" and "__PIC__" are defined to 2. This doesn't mention s390(x), but citus.so 382952 bytes (on amd64) is well beyond the 8k/32k limits mentioned above. PostgreSQL itself links correctly on s390x: ... -I/usr/include/mit-krb5 -fPIC -pie -I../../../../src/include I'm not an expert in compiler flags, but it seems to me CFLAGS_SL is wrong on s390(x) in Makefile.port, it should use -fPIC like sparc. (The m68k build has yet to happen, I'd guess it would exhibit the same problem.) Christoph
Re: To Andres Freund 2016-04-28 <20160428080824.GA22412@msg.df7cb.de> > I'm not an expert in compiler flags, but it seems to me CFLAGS_SL is > wrong on s390(x) in Makefile.port, it should use -fPIC like sparc. > > (The m68k build has yet to happen, I'd guess it would exhibit the same > problem.) Fwiw, the m68k build finished successfully with -fpic: https://buildd.debian.org/status/fetch.php?pkg=citus&arch=m68k&ver=5.0.1-1&stamp=1461978369 Christoph
Re: [HACKERS] [PATCH] relocation truncated to fit: citus buildfailure on s390x
From
Christoph Berg
Date:
Re: To Andres Freund 2016-04-28 <20160428080824.GA22412@msg.df7cb.de> > > I'm not clear why citus triggers this, when other extensions don't? > > Maybe it's simply because citus.so is bigger than all the other > extension .so files: > > -fpic > Generate position-independent code (PIC) suitable for use > in a shared library, if supported for the target machine. > Such code accesses all constant addresses through a global > offset table (GOT). The dynamic loader resolves the GOT > entries when the program starts (the dynamic loader is not > part of GCC; it is part of the operating system). If the > GOT size for the linked executable exceeds a machine- > specific maximum size, you get an error message from the > linker indicating that -fpic does not work; in that case, > recompile with -fPIC instead. (These maximums are 8k on > the SPARC and 32k on the m68k and RS/6000. The 386 has no > such limit.) > > Position-independent code requires special support, and > therefore works only on certain machines. For the 386, GCC > supports PIC for System V but not for the Sun 386i. Code > generated for the IBM RS/6000 is always > position-independent. > > When this flag is set, the macros "__pic__" and "__PIC__" > are defined to 1. > > -fPIC > If supported for the target machine, emit > position-independent code, suitable for dynamic linking and > avoiding any limit on the size of the global offset table. > This option makes a difference on the m68k, PowerPC and > SPARC. > > Position-independent code requires special support, and > therefore works only on certain machines. > > When this flag is set, the macros "__pic__" and "__PIC__" > are defined to 2. > > This doesn't mention s390(x), but citus.so 382952 bytes (on amd64) is > well beyond the 8k/32k limits mentioned above. > > PostgreSQL itself links correctly on s390x: > ... -I/usr/include/mit-krb5 -fPIC -pie -I../../../../src/include > > I'm not an expert in compiler flags, but it seems to me CFLAGS_SL is > wrong on s390(x) in Makefile.port, it should use -fPIC like sparc. After talking to a s390x Debian porter, -fPIC is the correct flag to use. The quoted patch made the previously failing builds for citus and pglogical (which have larger-than-average .so files) on s390x succeed (and the sparc64 case still works): --- a/src/makefiles/Makefile.linux +++ b/src/makefiles/Makefile.linux @@ -5,7 +5,8 @@ export_dynamic = -Wl,-E rpath = -Wl,-rpath,'$(rpathdir)',--enable-new-dtags DLSUFFIX = .so -ifeq "$(findstring sparc,$(host_cpu))" "sparc" +# Enable -fPIC to avoid "relocation truncated to fit" linker errors +ifneq "$(filter sparc% s390%,$(host_cpu))" "" CFLAGS_SL = -fPIC else CFLAGS_SL = -fpic The patch was made against 9.6; I'd opt to include it in master and the back branches. https://buildd.debian.org/status/logs.php?pkg=citus&arch=s390x https://buildd.debian.org/status/logs.php?pkg=pglogical&arch=s390x Christoph
Christoph Berg <myon@debian.org> writes: > Re: To Andres Freund 2016-04-28 <20160428080824.GA22412@msg.df7cb.de> >>> I'm not clear why citus triggers this, when other extensions don't? >> Maybe it's simply because citus.so is bigger than all the other >> extension .so files: I wonder what the overhead is of using -fPIC when -fpic would be sufficient. Whatever it is, the proposed patch imposes it on every shlib or extension, to accommodate one single extension that isn't even one we ship. Maybe this is small enough to not be something we need to worry about, but I'm wondering if we should ask citus and other large .so's to set some additional make flag that would cue usage of -fPIC over -fpic. regards, tom lane
Re: [HACKERS] [PATCH] relocation truncated to fit: citus buildfailure on s390x
From
Robert Haas
Date:
On Mon, May 29, 2017 at 3:45 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > I wonder what the overhead is of using -fPIC when -fpic would be > sufficient. Whatever it is, the proposed patch imposes it on every > shlib or extension, to accommodate one single extension that isn't > even one we ship. > > Maybe this is small enough to not be something we need to worry about, > but I'm wondering if we should ask citus and other large .so's to set > some additional make flag that would cue usage of -fPIC over -fpic. Do we have an idea how to measure the increased overhead? Just from reading the description, I'm guessing that the increased cost would happen when the extension calls back into core, but maybe that doesn't happen often enough to worry about? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes: > On Mon, May 29, 2017 at 3:45 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> I wonder what the overhead is of using -fPIC when -fpic would be >> sufficient. > Do we have an idea how to measure the increased overhead? Just from > reading the description, I'm guessing that the increased cost would > happen when the extension calls back into core, but maybe that doesn't > happen often enough to worry about? My gut feeling is that it'd be a pretty distributed cost, because every internal cross-reference in the .so (for instance, loading the address of a string literal) would involve a bit more overhead to support a wider offset field. An easy thing to look at would be how much the code expands by. That might or might not be a good proxy for the runtime slowdown percentage, but it seems like it ought to serve as a zero-order approximation. regards, tom lane
Re: [HACKERS] [PATCH] relocation truncated to fit: citus buildfailure on s390x
From
Andres Freund
Date:
On 2017-05-29 15:45:11 -0400, Tom Lane wrote: > Christoph Berg <myon@debian.org> writes: > > Re: To Andres Freund 2016-04-28 <20160428080824.GA22412@msg.df7cb.de> > >>> I'm not clear why citus triggers this, when other extensions don't? > > >> Maybe it's simply because citus.so is bigger than all the other > >> extension .so files: > > I wonder what the overhead is of using -fPIC when -fpic would be > sufficient. Whatever it is, the proposed patch imposes it on every > shlib or extension, to accommodate one single extension that isn't > even one we ship. > Maybe this is small enough to not be something we need to worry about, > but I'm wondering if we should ask citus and other large .so's to set > some additional make flag that would cue usage of -fPIC over -fpic. I think we can fix this easily enough in Citus, postgis, and whatever. But it's not a particularly good user/developer experience, and presumably is going to become more and more common. On x86 there shouldn't be a difference in efficiency between the two, but some others will see some. Given that most distributions switched to building the main executables with -fPIE anyway, to allow for ASLR, it seems unlikely that the intra extension overhead is going to be very meaningful in comparison. Greetings, Andres Freund
Andres Freund <andres@anarazel.de> writes: > On 2017-05-29 15:45:11 -0400, Tom Lane wrote: >> Maybe this is small enough to not be something we need to worry about, >> but I'm wondering if we should ask citus and other large .so's to set >> some additional make flag that would cue usage of -fPIC over -fpic. > I think we can fix this easily enough in Citus, postgis, and whatever. > But it's not a particularly good user/developer experience, and > presumably is going to become more and more common. On x86 there > shouldn't be a difference in efficiency between the two, but some others > will see some. Given that most distributions switched to building the > main executables with -fPIE anyway, to allow for ASLR, it seems unlikely > that the intra extension overhead is going to be very meaningful in > comparison. Very possibly true, but I wish we had some hard facts and not just guesses. regards, tom lane
Re: [HACKERS] [PATCH] relocation truncated to fit: citus buildfailure on s390x
From
Christoph Berg
Date:
Re: Andres Freund 2017-05-30 <20170530161541.koj5xbvvovrrtxtd@alap3.anarazel.de> > I think we can fix this easily enough in Citus, postgis, and whatever. > But it's not a particularly good user/developer experience, and > presumably is going to become more and more common. On x86 there > shouldn't be a difference in efficiency between the two, but some others > will see some. Given that most distributions switched to building the > main executables with -fPIE anyway, to allow for ASLR, it seems unlikely > that the intra extension overhead is going to be very meaningful in > comparison. My main point here would be that we are already setting this for all extensions for sparc and sparc64, so s390(x) would just follow suit. -fPIC is the default in Debian now, see the discussion starting at https://lists.debian.org/debian-devel/2016/05/msg00028.html including the Fedora: https://lists.debian.org/debian-devel/2016/05/msg00219.html and Ubuntu: https://lists.debian.org/debian-devel/2016/05/msg00225.html situation, which all do that. Christoph
Christoph Berg <myon@debian.org> writes: > My main point here would be that we are already setting this for all > extensions for sparc and sparc64, so s390(x) would just follow suit. For some values of "we", sure ;-). But I think what is really under discussion here is whether to change -fpic to -fPIC for all platforms. It looks like Makefile.netbsd and Makefile.openbsd would be affected along with Makefile.linux. Some other platforms such as freebsd are already in the fPIC-always camp. regards, tom lane
Re: [HACKERS] [PATCH] relocation truncated to fit: citus buildfailure on s390x
From
Christoph Berg
Date:
Re: Tom Lane 2017-05-30 <25131.1496163846@sss.pgh.pa.us> > Christoph Berg <myon@debian.org> writes: > > My main point here would be that we are already setting this for all > > extensions for sparc and sparc64, so s390(x) would just follow suit. > > For some values of "we", sure ;-). Afaict for all values of "we": ifeq "$(findstring sparc,$(host_cpu))" "sparc" CFLAGS_SL = -fPIC else CFLAGS_SL = -fpic endif https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/makefiles/Makefile.linux Christoph
I wrote: > Very possibly true, but I wish we had some hard facts and not just > guesses. As a simple but on-point test, I compared sizes of postgres_fdw.so built with -fpic and -fPIC. I no longer have access to a wide variety of weird architectures, but on what I do have in my office: x86_64, RHEL6: -fpic: text data bss dec hex filename 85467 2632 64 88163 15863 postgres_fdw.so -fPIC: text data bss dec hex filename 85467 2632 64 88163 15863 postgres_fdw.so This seems to confirm Andres' opinion that it makes no difference on x86_64. PPC, FreeBSD 10.3: -fpic: text data bss dec hex filename 86638 420 32 87090 15432 postgres_fdw.so -fPIC: text data bss dec hex filename 86474 1860 32 88366 1592e postgres_fdw.so that's a 1.47% penalty PPC, NetBSD 5.1.5: -fpic: text data bss dec hex filename 81880 420 56 82356 141b4 postgres_fdw.so -fPIC: text data bss dec hex filename 81688 2044 56 83788 1474c postgres_fdw.so that's a 1.74% penalty HPPA, HPUX 10.20: -fpic: text data bss dec hex filename 97253 17296 8 114557 1bf7d postgres_fdw.sl -fPIC: text data bss dec hex filename 102629 17320 8 119957 1d495 postgres_fdw.sl that's a 4.7% penalty It's somewhat noteworthy that the PPC builds show a large increase in data segment size. That likely corresponds to relocatable pointer fields that have to be massaged by the dynamic linker during shlib load. Thus one could speculate that shlib load might be noticeably slower with -fPIC, at least on PPC. But people rarely complain about the speed of that anyway, so it's unlikely to be worth worrying about. It'd be interesting if people could gather similar numbers on other platforms of more real-world relevance, such as ppc64. But based on this small sample, I wouldn't object to just going to -fPIC across the board. regards, tom lane
Re: [HACKERS] [PATCH] relocation truncated to fit: citus buildfailure on s390x
From
Robert Haas
Date:
On Tue, May 30, 2017 at 4:38 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > It'd be interesting if people could gather similar numbers on other > platforms of more real-world relevance, such as ppc64. But based on > this small sample, I wouldn't object to just going to -fPIC across > the board. That seems pretty sensible to me. I think we should aim for a configuration that works by default. If we use -fPIC where -fpic would have been better, the result is that on some platforms there might be a speed penalty, probably small. In the reverse situation, the build fails. I believe that the average PostgreSQL extension developer will prefer the first situation. If somebody has got an extension which is small enough to use -fpic and that person cares about minimizing the performance penalty on s390 or other platforms where they have this problem, then they can arrange an override in the opposite direction. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: [HACKERS] [PATCH] relocation truncated to fit: citus buildfailure on s390x
From
Christoph Berg
Date:
Re: Tom Lane 2017-05-30 <1564.1496176732@sss.pgh.pa.us> > It'd be interesting if people could gather similar numbers on other > platforms of more real-world relevance, such as ppc64. But based on > this small sample, I wouldn't object to just going to -fPIC across > the board. ppc64el, Debian unstable: text data bss dec hex filename -fpic: 79520 928 1768 82216 14128 postgres_fdw.so -fPIC: 79520 928 1768 82216 14128 postgres_fdw.so -> no change s390x, Debian unstable: text data bss dec hex filename -fpic: 80735 2552 48 83335 14587 postgres_fdw.so -fPIC: 81247 2552 48 83847 14787 postgres_fdw.so -> +0.61% arm64, Debian unstable: text data bss dec hex filename -fpic: 64130 2600 48 66778 104da postgres_fdw.so -fPIC: 64274 2600 48 66922 1056a postgres_fdw.so -> +0.22% sparc64, Debian unstable: text data bss dec hex filename -fpic: 75804 3296 48 79148 1352c postgres_fdw.so -fPIC: 72748 920 48 73716 11ff4 postgres_fdw.so -> 6.9% decrease (!) 9.6.3, gcc (Debian 6.3.0-18) 6.3.0 20170516, -O2, all objects unstripped (sparc64 is gcc (Debian 6.3.0-17) 6.3.0 20170510) Christoph
Christoph Berg <myon@debian.org> writes: > Re: Tom Lane 2017-05-30 <1564.1496176732@sss.pgh.pa.us> >> It'd be interesting if people could gather similar numbers on other >> platforms of more real-world relevance, such as ppc64. But based on >> this small sample, I wouldn't object to just going to -fPIC across >> the board. > [ more numbers ] OK, this looks good to me. Just to make sure everyone's on the same page, what I propose to do is simplify all our platform-specific Makefiles that use either -fpic or -fPIC to use -fPIC unconditionally. This affects the netbsd, linux, and openbsd ports. Looks like we should also change the example link commands in dfunc.sgml. Next question: should we back-patch this change, or just do it in HEAD? regards, tom lane
Re: [HACKERS] [PATCH] relocation truncated to fit: citus buildfailure on s390x
From
Christoph Berg
Date:
Re: Tom Lane 2017-05-31 <28752.1496238931@sss.pgh.pa.us> > OK, this looks good to me. Just to make sure everyone's on the > same page, what I propose to do is simplify all our platform-specific > Makefiles that use either -fpic or -fPIC to use -fPIC unconditionally. > This affects the netbsd, linux, and openbsd ports. Looks like we should > also change the example link commands in dfunc.sgml. Ack. > Next question: should we back-patch this change, or just do it in HEAD? Debian "needs" it for 9.6, but I've already pushed the s390x patch in the original posting, so I could just live with it being just in head. But of course it would be nice to have everything in sync. Christoph
Re: [HACKERS] [PATCH] relocation truncated to fit: citus buildfailure on s390x
From
Alvaro Herrera
Date:
Christoph Berg wrote: > Re: Tom Lane 2017-05-31 <28752.1496238931@sss.pgh.pa.us> > > Next question: should we back-patch this change, or just do it in HEAD? > > Debian "needs" it for 9.6, but I've already pushed the s390x patch in > the original posting, so I could just live with it being just in head. > But of course it would be nice to have everything in sync. I think it's only a problem for you in 9.6-only because you've not tried pglogical or some other large-shlib extension with earlier branches; in other words, eventually this is going to bite somebody using the old branches as well, unless we believe that the platforms are dead enough that nobody really cares other than for academic purposes. My vote would be to backpatch it all the way. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Alvaro Herrera <alvherre@2ndquadrant.com> writes: > My vote would be to backpatch it all the way. That's my thought too. Otherwise it'll be five years before extension authors can stop worrying about this issue. regards, tom lane
Re: [HACKERS] [PATCH] relocation truncated to fit: citus buildfailure on s390x
From
Andres Freund
Date:
On 2017-05-31 11:57:16 -0400, Alvaro Herrera wrote: > My vote would be to backpatch it all the way. +1
Andres Freund <andres@anarazel.de> writes: > On 2017-05-31 11:57:16 -0400, Alvaro Herrera wrote: >> My vote would be to backpatch it all the way. > +1 Done, buildfarm seems happy. regards, tom lane