[Bug gas/29655] s390x gas generates PC32DBL instead of PLT32DBL for function call
https://sourceware.org/bugzilla/show_bug.cgi?id=29655 Andreas Krebbel changed: What|Removed |Added CC||krebbel at linux dot ibm.com --- Comment #4 from Andreas Krebbel --- Since GCC 12 we always emit @PLT at brasl symbols already: commit 0990d93dd8a4268bff5bbe48aa26748cf63201c7 Author: Ilya Leoshkevich Date: Mon Jun 7 13:44:15 2021 +0200 IBM Z: Use @PLT symbols for local functions in 64-bit mode This helps with generating code for kernel hotpatches, which contain individual functions and are loaded more than 2G away from vmlinux. This should not create performance regressions for the normal use cases, because for local functions ld replaces @PLT calls with direct calls. That previous GCCs omit this isn't a problem since ld makes sure to jump through PLT also for PC32DBL. -- You are receiving this mail because: You are on the CC list for the bug.
[Bug gas/29655] s390x gas generates PC32DBL instead of PLT32DBL for function call
https://sourceware.org/bugzilla/show_bug.cgi?id=29655 --- Comment #6 from Andreas Krebbel --- (In reply to Rui Ueyama from comment #5) > Ooh, that's why I did see this only on SuSE's builder. I'm glad that that's > already been handled. I would just like to mention that adding the @PLT isn't really a bugfix. We did this to make life easier with hotpatching. Omitting the @PLT for a non-PIC build is also correct I think. At that point it isn't clear whether the jump needs to go through PLT or not. It could very well be resolved locally. So I think it is anyway up to ld to decide whether a PLT stub is needed or not. > mold does not create PLT for R_390_PC32DBL; it does only for R_390_PLT* > relocs. We can change that so that mold would create a PLT for a PC32 reloc > just like GNU ld does, but that would break Qt. So I'll keep the mold's > current behavior as-is. Could you please elaborate what is so special about Qt here. If a function symbol with a PC32DBL reloc could not be resolved locally adding a PLT stub should be the right thing. -- You are receiving this mail because: You are on the CC list for the bug.
[Bug gas/29655] s390x gas generates PC32DBL instead of PLT32DBL for function call
https://sourceware.org/bugzilla/show_bug.cgi?id=29655
--- Comment #12 from Andreas Krebbel ---
So do I understand it correctly that for the Qt case the example would look
more like this:
test2.c:
#include
void alias () __attribute__ ((weak, alias ("bar")));
void bar() { printf("bar=%p alias=%p\n", bar, alias); }
test1.c:
void bar();
// void *get_bar_addr() { return bar; }
int main() { bar(); }
which gives the expected result on x86 (with line 2 commented out):
bar=0x7f1e4cdb4109 alias=0x7f1e4cdb4109
but not on s390x because we always resolve to the PLT slot:
bar=0x10004b8 alias=0x3fffdf00640
On the other hand the behavior Qt is relying on depends on whether the address
of bar is taken in main. So by uncommenting line 2 the problem occurs also on
x86:
bar=0x401030 alias=0x7f63ae84a109
Neither target appears to guarantee that aliases behave the same wrt pointer
equality. On non-s390x targets it only works because Qt happens to trigger a
case where it accidentally matches.
--
You are receiving this mail because:
You are on the CC list for the bug.
[Bug gas/29655] s390x gas generates PC32DBL instead of PLT32DBL for function call
https://sourceware.org/bugzilla/show_bug.cgi?id=29655 --- Comment #16 from Andreas Krebbel --- Created attachment 14386 --> https://sourceware.org/bugzilla/attachment.cgi?id=14386&action=edit Experimental Fix This patch fixes the testcase from comment 14. No testsuite regressions in the Binutils testsuite. -- You are receiving this mail because: You are on the CC list for the bug.
[Bug gas/29655] s390x gas generates PC32DBL instead of PLT32DBL for function call
https://sourceware.org/bugzilla/show_bug.cgi?id=29655 --- Comment #17 from Andreas Krebbel --- I have attached a patch for the testcase in Comment 14. Turns out that we also have to zero out the symbol value in order to avoid function pointer references in the main binary to be wired up to the main binary PLT slot. The patch also helps with my non-pic testcase from Comment 12. However, here it relies on the @PLT marker on the symbol in the function call. I think with that we are circling back to Rui's original question from Comment 2. How should the linker recognize whether a symbol is used only as part of direct function calls or whether its address is taken? All the linker can look at are the relocations. Mold currently relies on symbols in direct function calls to use a PLT32DBL reloc while function pointer references use PC32DBL or R_390_64. With the attached patch the same will apply to ld. Clang always adds the @PLT marker and GCC starts doing this with version 12. So I think we are ok. I would rather not want to backport the GCC patch to older versions. -- You are receiving this mail because: You are on the CC list for the bug.
[Bug gas/29655] s390x gas generates PC32DBL instead of PLT32DBL for function call
https://sourceware.org/bugzilla/show_bug.cgi?id=29655 --- Comment #19 from Andreas Krebbel --- (In reply to Rui Ueyama from comment #18) > Was there any reason to limit it to R_390_64 and R_390_PC32DBL? These were the only relocs which made sense to me as 64 bit pointers. -- You are receiving this mail because: You are on the CC list for the bug.
[Bug gas/29655] s390x gas generates PC32DBL instead of PLT32DBL for function call
https://sourceware.org/bugzilla/show_bug.cgi?id=29655 --- Comment #21 from Andreas Krebbel --- (In reply to Rui Ueyama from comment #20) > GCC 12 seems to always append `@PLT` to a function symbol even if we are not > calling that function. Here is a test case. ... > I think `larl %r1,foo@PLT` should be `larl %r1,foo`. I agree. Ilya is having a look. Thanks! -- You are receiving this mail because: You are on the CC list for the bug.
[Bug ld/31277] building binutils targeting x86_64-linux-gnu on s390x-linux-gnu doesn't enable i686-linux-gnu
https://sourceware.org/bugzilla/show_bug.cgi?id=31277 Andreas Krebbel changed: What|Removed |Added CC||krebbel at linux dot ibm.com --- Comment #2 from Andreas Krebbel --- I've tried various combinations of compiler versions (including current master) and binutils levels but it looks like it is working for me. Regardless of whether I configure binutils with --enable-targets=x86_64-linux-gnu or --enable-targets=i686-linux-gnu in all cases I get support for 32 and 64 bit x86 architectures in binutils: ./ld: supported targets: elf64-s390 elf32-s390 elf64-little elf64-big elf32-little elf32-big elf64-x86-64 elf32-i386 elf32-iamcu elf32-x86-64 pei-i386 pe-x86-64 pei-x86-64 srec symbolsrec verilog tekhex binary ihex plugin trad-core I haven't tried to build a cross GCC with that yet, but I'll give that a try as well. Could you please provide the configure lines of GCC and Binutils you were using in the failing case? -- You are receiving this mail because: You are on the CC list for the bug.
[Bug ld/31277] building binutils targeting x86_64-linux-gnu on s390x-linux-gnu doesn't enable i686-linux-gnu
https://sourceware.org/bugzilla/show_bug.cgi?id=31277 Andreas Krebbel changed: What|Removed |Added CC||jremus at linux dot ibm.com -- You are receiving this mail because: You are on the CC list for the bug.
[Bug ld/31277] building binutils targeting x86_64-linux-gnu on s390x-linux-gnu doesn't enable i686-linux-gnu
https://sourceware.org/bugzilla/show_bug.cgi?id=31277 --- Comment #3 from Andreas Krebbel --- Matthias, is this still a problem? Could you provide steps to reproduce it. -- You are receiving this mail because: You are on the CC list for the bug.
[Bug ld/20113] Partial relro support for s390x ld
https://sourceware.org/bugzilla/show_bug.cgi?id=20113 Andreas Krebbel changed: What|Removed |Added CC||krebbel at linux dot ibm.com --- Comment #12 from Andreas Krebbel --- I've implemented the relro support for Binutils based on the work from Marcin Kościelnicki. Marcin unfortunately didn't finish the bounty so I had to complete it myself. I've decided to do this only for S/390 64 bit since 32 bit binaries do not have a bright future anyway. Unfortunately I forgot to close this bugzilla accordingly. I think the patch posted here is wrong. SEPARATE_GOTPLT must remain as 0 on our platform. Please revert the patch. Here the patches which implement partial relro for IBM Z: commit afca762f598d453c563f244cd315b1a0cb72 Author: Andreas Krebbel Date: Thu Dec 21 13:12:03 2017 +0100 S/390: Improve partial relro support for 64 bit commit a38137289e91fd548fc27fb6566a439233b94d65 Author: Andreas Krebbel Date: Mon Jun 11 13:23:00 2018 +0200 ld: Enable using separate linker script for -z relro With this patch dedicated linker scripts can be generated for partial relro triggered by defining GENERATE_RELRO_SCRIPT in the target specific scripts. This is necessary for e.g. S/390 where usually the .got.plt comes first and prevents the relro segment from being extended across the non-plt GOT entries. The patch started with the work from Marcin taken from the mwk user branches. However, the patch needed substantial changes due to the 'separate code' feature which got committed in the meantime. ld/ChangeLog: 2018-07-18 Andreas Krebbel Marcin Kościelnicki -- You are receiving this mail because: You are on the CC list for the bug. ___ bug-binutils mailing list bug-binutils@gnu.org https://lists.gnu.org/mailman/listinfo/bug-binutils
[Bug ld/20113] Partial relro support for s390x ld
https://sourceware.org/bugzilla/show_bug.cgi?id=20113 --- Comment #13 from Andreas Krebbel --- Unfortunately the patch also went into 2.32 release :( At least for 64 bit the patch doesn't hurt since it is a nop. The code currently looks like: EXTRA_EM_FILE=s390 SEPARATE_GOTPLT=24 <--- the line added with the patch IREL_IN_PLT= SEPARATE_GOTPLT=0 <--- the line added with my patch test -z "$RELRO" && unset SEPARATE_GOTPLT -- You are receiving this mail because: You are on the CC list for the bug. ___ bug-binutils mailing list bug-binutils@gnu.org https://lists.gnu.org/mailman/listinfo/bug-binutils
[Bug gold/20114] Partial relro support for s390x gold
https://sourceware.org/bugzilla/show_bug.cgi?id=20114 Andreas Krebbel changed: What|Removed |Added CC||krebbel at linux dot ibm.com -- You are receiving this mail because: You are on the CC list for the bug. ___ bug-binutils mailing list bug-binutils@gnu.org https://lists.gnu.org/mailman/listinfo/bug-binutils
[Bug ld/20113] Partial relro support for s390x ld
https://sourceware.org/bugzilla/show_bug.cgi?id=20113 --- Comment #14 from Andreas Krebbel --- Maamoun, was your patch ever posted on the Binutils mailing list? I could not find it there. -- You are receiving this mail because: You are on the CC list for the bug. ___ bug-binutils mailing list bug-binutils@gnu.org https://lists.gnu.org/mailman/listinfo/bug-binutils
[Bug ld/20113] Partial relro support for s390x ld
https://sourceware.org/bugzilla/show_bug.cgi?id=20113 --- Comment #19 from Andreas Krebbel --- (In reply to maamountki from comment #18) > *patch > > I just noticed that SEPARATE_GOTPLT is duplicated in the 64 bit script and > partial relro support is already implemented so yes my patch should be > reverted, I'm contacting Bountysource to revert the claim submission too. > Anyway I'm not sure why you think that SEPARATE_GOTPLT must remain as 0 > hence the relro segment doesn't extend across those entries, SEPARATE_GOTPLT > have this argument for the exact reason and architectures such as x86 and > aarch64 take advantage of this feature and there is a reason actually there > are no harm in making those entries non-exploitable. For IBM Z we had to chose a slightly different scheme. Our ABI requires that *all* GOT entries (.got + .got.plt) are accessible via _GLOBAL_OFFSET_TABLE_ symbol and a *positive* displacement. Just setting SEPARATE_GOTPLT makes _GLOBAL_OFFSET_TABLE_ to point at the start of .got.plt which would come after the other .got entries. Hence a negative offset would be needed to address .got entries. More changes to the backend were required to make _GLOBAL_OFFSET_TABLE_ end up at the start of .got. As you say the value of SEPARATE_GOTPLT is used to make the relro segment cover the magic entries assumed to be located at start of .got.plt. With moving _GLOBAL_OFFSET_TABLE_ to start of .got I also had to move the 3 magic entries to the start of .got. So they will be covered by the relro segment anyway. Setting it to 0 is the right value for us then. I apologize for not monitoring the bugzilla and closing it after the relro work was finished. I know you are doing a great job for other of our bounties (e.g. OpenBlas). Please keep up your great work! -- You are receiving this mail because: You are on the CC list for the bug. ___ bug-binutils mailing list bug-binutils@gnu.org https://lists.gnu.org/mailman/listinfo/bug-binutils
[Bug gold/20114] Partial relro support for s390x gold
https://sourceware.org/bugzilla/show_bug.cgi?id=20114 --- Comment #5 from Andreas Krebbel --- (In reply to maamountki from comment #4) Looks good to me. However, I'm not a Gold maintainer. One difference to the ld implementation is that you appear to always use the new got layout. ld uses the new layout only for partial relro to keep impact on existing stuff as small as possible. On the other hand the new layout is supposed to be compatible to the old one. Switching to the new one unconditionally should be ok for Gold I think. It also keeps the code much simpler. Have you checked that all this works with linker scripts as well. I mean linker scripts which have got and got.plt swapped? Will gold do the right thing for these variants with regard to the 3 magic entries, _GLOBAL_OFFSET_TABLE_, and DT_PLTGOT? -- You are receiving this mail because: You are on the CC list for the bug. ___ bug-binutils mailing list bug-binutils@gnu.org https://lists.gnu.org/mailman/listinfo/bug-binutils
[Bug gold/20114] Partial relro support for s390x gold
https://sourceware.org/bugzilla/show_bug.cgi?id=20114 --- Comment #8 from Andreas Krebbel --- (In reply to maamountki from comment #6) ... > Yes I do and it does not. > .got and .got.plt must be set properly in linker srcript to get it work > otherwise a segmentation fault will be produced. I can see the same behavior > on different platform "x86_64". But x86_64 always had the non-plt GOT entries first so for them it isn't a regression. For S/390 it would mean that non of the existing linker scripts would continue to work with Gold after that change. -- You are receiving this mail because: You are on the CC list for the bug. ___ bug-binutils mailing list bug-binutils@gnu.org https://lists.gnu.org/mailman/listinfo/bug-binutils
[Bug gold/20114] Partial relro support for s390x gold
https://sourceware.org/bugzilla/show_bug.cgi?id=20114 --- Comment #9 from Andreas Krebbel --- (In reply to maamountki from comment #7) > Thinking of replacing the new layout with this one > > +--+ > |.got[0]: DYNAMIC | <--- _GLOBAL_OFFSET_TABLE_ .got > +--+ > | | <--- non-plt GOT entries > | | > | | > +--+ > |.got.plt[0]: link_map parm| <--- DT_PLTGOT .gotplt > |.got.plt[1]: &_dl_runtime_resolve | > | | > | | > +--+ > | | <--- PLT GOT entries > | | > | | > | | > +--+ > > However ld and gold layouts will not be on the same track and the code will > not be as simple as the current patch, What do you think? Would this really solve the linker script problem? I also considered this variant but it is some disadvantages: - Our PLT first entry accesses the link_map parm and the dl_runtime_resolve entries via _GLOBAL_OFFSET_TABLE_. Fixing this probably would require adding another symbol to replace _GLOBAL_OFFSET_TABLE_ which can be used in the initial PLT entry. - prelink currently adjusts DT_PLTGOT[0]. The value then will be read by ld.so via _GLOBAL_OFFSET_TABLE_[0]. Having DT_PLTGOT != _GLOBAL_OFFSET_TABLE_ would require to have two copies of that value and keep them in sync somehow and/or change prelink. However, I'm not sure prelink is still a concern anymore?! -- You are receiving this mail because: You are on the CC list for the bug. ___ bug-binutils mailing list bug-binutils@gnu.org https://lists.gnu.org/mailman/listinfo/bug-binutils
[Bug gold/20114] Partial relro support for s390x gold
https://sourceware.org/bugzilla/show_bug.cgi?id=20114 --- Comment #11 from Andreas Krebbel --- (In reply to maamountki from comment #10) > (In reply to Andreas Krebbel from comment #8) > > > For S/390 it would mean that non of the existing linker scripts > > would continue to work with Gold after that change. > > For S/390 gold does not have .got.plt at all, putting this section in linker > script make no sense and will be ignored. Ok. Having a linker script with just a .got continues to work with your patch? -- You are receiving this mail because: You are on the CC list for the bug. ___ bug-binutils mailing list bug-binutils@gnu.org https://lists.gnu.org/mailman/listinfo/bug-binutils
