Message ID | 20240626152033.2567309-1-jremus@linux.ibm.com |
---|---|
Headers |
Return-Path: <binutils-bounces+patchwork=sourceware.org@sourceware.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id A698E387542E for <patchwork@sourceware.org>; Wed, 26 Jun 2024 15:21:36 +0000 (GMT) X-Original-To: binutils@sourceware.org Delivered-To: binutils@sourceware.org Received: from mx0b-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) by sourceware.org (Postfix) with ESMTPS id C96193871030 for <binutils@sourceware.org>; Wed, 26 Jun 2024 15:20:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org C96193871030 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=linux.ibm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linux.ibm.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org C96193871030 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=148.163.158.5 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1719415261; cv=none; b=ma2dARLSpXQPjwsGWXLmfJ3XQB3uhO0R12K+bOZ7ASxgLSpNUFSbLyYw4d6+02lVInkpwt3x8b/C+ZCjlTEL3VqG2pyMC5yAfzvytfrtmI6cATvA8LcI0M7mswxSo8uPCmq26KNA2BelllrN53wiIFct3SovZSZhPbkm5RqUvhk= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1719415261; c=relaxed/simple; bh=bbvCGbjW+N2Tq+RLfoD18WKyXvcQ/TqxXH+Cq12EGUI=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=Gzw6JKjOCPnM/yC6r6u0Miphev92DWAzqWP21sl9++4jrQwvwocIYD23pr+0p9TlDEE+NDZACMKqLuHpdTHymWK9NaizFAQF4TwLbeaMTn9Hlt/ZpbUMqzcTxQnOKL+jEEeBtlx4CCuLAWCrusU4rtb/0h/bTvsdMoyCns9n89o= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from pps.filterd (m0353725.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.18.1.2/8.18.1.2) with ESMTP id 45QF5ZVg005104 for <binutils@sourceware.org>; Wed, 26 Jun 2024 15:20:50 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=from :to:cc:subject:date:message-id:mime-version :content-transfer-encoding; s=pp1; bh=mtOdPBAesBqAPzjZLS6/G97aMJ VPksGQp1Oh4jBEEME=; b=mLBvju6SWDwuvQQQOXnOy/Q/3irO1dF2l4JWxwJIRb nx+HO4Ldh0b5ZmYu2c0TnL3EmwZPWdXC7Evi5beLio9QcLEO4ZjSGpe4CFXj8i41 Bty220LFnQ8LZiW/08VysVWCn/txyqtUoOyF0AiGgsU1wyBZp7Q5HZeFDN9lGDjk IcvoNZbm8/M5gPT/TSewwGnJfBCzirMblfCSdg8KGphdlJWCpMojsfdTgkntXw4L Bb74Zv8G1wqhUmUp0zXF9cD4VKvb4ONN26o05CJNkWHbbfEsTmn9i0yG/CCEZL1R g6YCewGbfjwn5i7rXFkmsZcu+iOzdcURyWTQJIc8UxWQ== Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 400n7fr26y-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for <binutils@sourceware.org>; Wed, 26 Jun 2024 15:20:50 +0000 (GMT) Received: from m0353725.ppops.net (m0353725.ppops.net [127.0.0.1]) by pps.reinject (8.18.0.8/8.18.0.8) with ESMTP id 45QFKndf028329 for <binutils@sourceware.org>; Wed, 26 Jun 2024 15:20:49 GMT Received: from ppma11.dal12v.mail.ibm.com (db.9e.1632.ip4.static.sl-reverse.com [50.22.158.219]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 400n7fr26u-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 26 Jun 2024 15:20:49 +0000 (GMT) Received: from pps.filterd (ppma11.dal12v.mail.ibm.com [127.0.0.1]) by ppma11.dal12v.mail.ibm.com (8.17.1.19/8.17.1.19) with ESMTP id 45QE55IQ032376; Wed, 26 Jun 2024 15:20:49 GMT Received: from smtprelay02.fra02v.mail.ibm.com ([9.218.2.226]) by ppma11.dal12v.mail.ibm.com (PPS) with ESMTPS id 3yxbn3cr06-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 26 Jun 2024 15:20:49 +0000 Received: from smtpav05.fra02v.mail.ibm.com (smtpav05.fra02v.mail.ibm.com [10.20.54.104]) by smtprelay02.fra02v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 45QFKhM555706062 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 26 Jun 2024 15:20:45 GMT Received: from smtpav05.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 584C520043; Wed, 26 Jun 2024 15:20:43 +0000 (GMT) Received: from smtpav05.fra02v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 2A18B20040; Wed, 26 Jun 2024 15:20:43 +0000 (GMT) Received: from tuxmaker.boeblingen.de.ibm.com (unknown [9.152.85.9]) by smtpav05.fra02v.mail.ibm.com (Postfix) with ESMTP; Wed, 26 Jun 2024 15:20:43 +0000 (GMT) From: Jens Remus <jremus@linux.ibm.com> To: binutils@sourceware.org, Andreas Krebbel <krebbel@linux.ibm.com> Cc: Jens Remus <jremus@linux.ibm.com> Subject: [PATCH 0/2] s390: Avoid relocation overflows on undefined weak symbols Date: Wed, 26 Jun 2024 17:20:31 +0200 Message-Id: <20240626152033.2567309-1-jremus@linux.ibm.com> X-Mailer: git-send-email 2.40.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-TM-AS-GCONF: 00 X-Proofpoint-GUID: uZgkMxyAChPHbOSxyOiWdS_gUMbQjGB7 X-Proofpoint-ORIG-GUID: anReq-04N9Rc-LRaSVec73j8x58F7otL X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.293,Aquarius:18.0.1039,Hydra:6.0.680,FMLib:17.12.28.16 definitions=2024-06-26_07,2024-06-25_01,2024-05-17_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 suspectscore=0 phishscore=0 spamscore=0 priorityscore=1501 impostorscore=0 malwarescore=0 mlxlogscore=507 adultscore=0 bulkscore=0 clxscore=1015 mlxscore=0 lowpriorityscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.19.0-2406140001 definitions=main-2406260109 X-Spam-Status: No, score=-6.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: binutils@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Binutils mailing list <binutils.sourceware.org> List-Unsubscribe: <https://sourceware.org/mailman/options/binutils>, <mailto:binutils-request@sourceware.org?subject=unsubscribe> List-Archive: <https://sourceware.org/pipermail/binutils/> List-Post: <mailto:binutils@sourceware.org> List-Help: <mailto:binutils-request@sourceware.org?subject=help> List-Subscribe: <https://sourceware.org/mailman/listinfo/binutils>, <mailto:binutils-request@sourceware.org?subject=subscribe> Errors-To: binutils-bounces+patchwork=sourceware.org@sourceware.org |
Series |
s390: Avoid relocation overflows on undefined weak symbols
|
|
Message
Jens Remus
June 26, 2024, 3:20 p.m. UTC
Patch 1 corrects commit 896a639babe2 ("s390: Avoid reloc overflows on undefined weak symbols") not to replace Branch Relative on Count High (brcth) referencing an undefined weak symbol definitively resolving to zero by a trap, as it is not guaranteed that the conditional branch is taken in any case. Patch 2 complements commit 896a639babe2 ("s390: Avoid reloc overflows on undefined weak symbols") by applying similar replacements of instructions referencing undefined weak symbols that definitively resolve to zero. This time for PLT32DBL relocations. Regards, Jens Jens Remus (2): s390: Do not replace brcth referencing undefined weak symbol s390: Avoid reloc overflows on undefined weak symbols (cont) bfd/elf64-s390.c | 42 ++++++++++++++++++++++++++--- ld/testsuite/ld-s390/s390.exp | 5 +++- ld/testsuite/ld-s390/weakundef-1.dd | 6 ++--- ld/testsuite/ld-s390/weakundef-1.s | 1 - ld/testsuite/ld-s390/weakundef-2.dd | 17 ++++++++++++ ld/testsuite/ld-s390/weakundef-2.s | 17 ++++++++++++ 6 files changed, 80 insertions(+), 8 deletions(-) create mode 100644 ld/testsuite/ld-s390/weakundef-2.dd create mode 100644 ld/testsuite/ld-s390/weakundef-2.s
Comments
On Wed, Jun 26, 2024 at 8:20 AM Jens Remus <jremus@linux.ibm.com> wrote: > > Patch 1 corrects commit 896a639babe2 ("s390: Avoid reloc overflows on > undefined weak symbols") not to replace Branch Relative on Count High > (brcth) referencing an undefined weak symbol definitively resolving to > zero by a trap, as it is not guaranteed that the conditional branch is > taken in any case. > > Patch 2 complements commit 896a639babe2 ("s390: Avoid reloc overflows > on undefined weak symbols") by applying similar replacements of > instructions referencing undefined weak symbols that definitively > resolve to zero. This time for PLT32DBL relocations. > > Regards, > Jens > > Jens Remus (2): > s390: Do not replace brcth referencing undefined weak symbol > s390: Avoid reloc overflows on undefined weak symbols (cont) > > bfd/elf64-s390.c | 42 ++++++++++++++++++++++++++--- > ld/testsuite/ld-s390/s390.exp | 5 +++- > ld/testsuite/ld-s390/weakundef-1.dd | 6 ++--- > ld/testsuite/ld-s390/weakundef-1.s | 1 - > ld/testsuite/ld-s390/weakundef-2.dd | 17 ++++++++++++ > ld/testsuite/ld-s390/weakundef-2.s | 17 ++++++++++++ > 6 files changed, 80 insertions(+), 8 deletions(-) > create mode 100644 ld/testsuite/ld-s390/weakundef-2.dd > create mode 100644 ld/testsuite/ld-s390/weakundef-2.s > > -- > 2.40.1 > Other ports have simpler strategies for unresolved undefined weak symbols https://reviews.llvm.org/D103001 aarch64: GNU ld: rewrite the instruction to a NOP; ld.lld: branch to the next instruction mips: GNU ld: branch to the start of the text segment (?); ld.lld: branch to zero ppc32: GNU ld: rewrite the instruction to a NOP; ld.lld: branch to the current instruction ppc64: GNU ld: rewrite the instruction to a NOP; ld.lld: branch to the current instruction riscv: GNU ld: branch to the absolute zero address (with instruction rewriting) i386/x86_64: GNU ld/ld.lld: branch to the link-time zero address I haven't checked the patch closely, but it seems to add quite a few lines.
Hello Fangrui, thank you for the feedback and hints at how the issue is dealt with by other architectures! Am 26.06.2024 um 20:26 schrieb Fangrui Song: > On Wed, Jun 26, 2024 at 8:20 AM Jens Remus <jremus@linux.ibm.com> wrote: >> Patch 1 corrects commit 896a639babe2 ("s390: Avoid reloc overflows on >> undefined weak symbols") not to replace Branch Relative on Count High >> (brcth) referencing an undefined weak symbol definitively resolving to >> zero by a trap, as it is not guaranteed that the conditional branch is >> taken in any case. >> >> Patch 2 complements commit 896a639babe2 ("s390: Avoid reloc overflows >> on undefined weak symbols") by applying similar replacements of >> instructions referencing undefined weak symbols that definitively >> resolve to zero. This time for PLT32DBL relocations. ... >> Jens Remus (2): >> s390: Do not replace brcth referencing undefined weak symbol >> s390: Avoid reloc overflows on undefined weak symbols (cont) >> >> bfd/elf64-s390.c | 42 ++++++++++++++++++++++++++--- >> ld/testsuite/ld-s390/s390.exp | 5 +++- >> ld/testsuite/ld-s390/weakundef-1.dd | 6 ++--- >> ld/testsuite/ld-s390/weakundef-1.s | 1 - >> ld/testsuite/ld-s390/weakundef-2.dd | 17 ++++++++++++ >> ld/testsuite/ld-s390/weakundef-2.s | 17 ++++++++++++ >> 6 files changed, 80 insertions(+), 8 deletions(-) >> create mode 100644 ld/testsuite/ld-s390/weakundef-2.dd >> create mode 100644 ld/testsuite/ld-s390/weakundef-2.s > Other ports have simpler strategies for unresolved undefined weak symbols > https://reviews.llvm.org/D103001 > > aarch64: GNU ld: rewrite the instruction to a NOP; ld.lld: branch to > the next instruction > mips: GNU ld: branch to the start of the text segment (?); ld.lld: > branch to zero > ppc32: GNU ld: rewrite the instruction to a NOP; ld.lld: branch to the > current instruction > ppc64: GNU ld: rewrite the instruction to a NOP; ld.lld: branch to the > current instruction > riscv: GNU ld: branch to the absolute zero address (with instruction rewriting) > i386/x86_64: GNU ld/ld.lld: branch to the link-time zero address > > I haven't checked the patch closely, but it seems to add quite a few lines. I had a quick look at bfd.ld AArch64 and PPC64. As you listed they both replace branches with NOPs. Yet it is unclear to me how/where they deal with (function) pointers resolving to zero. On s390x using a section start address greater than 4GB at link-time causes PC32DBL and PLT32DBL PC-relative relocations for undefined weak symbols that definitively resolve to zero to overflow. So far we have identified three types of PC-relative instructions and decided to rewrite them as follows to not change the general behavior: - PC-relative load address of zero are replaced by a non-PC-relative load address of zero. This ensures correctness of run-time checks for undef weak symbols that resolved to zero. - PC-relative branches to zero are replaced by a trap. A branch to zero would result in an exception at run-time and so does a trap. - PC-relative instructions that can be considered optional, such as PC-relative prefetch of zero, are replaced by a NOP. While technically it might be ok to rewrite subject branches as a NOP, we think that using a trap is better for debugging, as a NOP might go undetected at run-time. Also the complexity of the logic to do so does not change whether to rewrite as NOP or trap. Regards, Jens
On Mon, Jul 01, 2024 at 04:33:20PM +0200, Jens Remus wrote: > I had a quick look at bfd.ld AArch64 and PPC64. As you listed they both > replace branches with NOPs. Yet it is unclear to me how/where they deal with > (function) pointers resolving to zero. ppc64 does nothing special. As with most other targets, you need to write if (foo) foo (); when dealing with a weak function foo that may or may not be defined at runtime. However, if you know the function will be resolved at link time (static linking, or -z nodynamic-undefined-weak) then you can just write foo (); and get the same behaviour as if (foo) foo (); So replacing a branch and link with a nop is only useful in special cases, and of course doesn't apply if you're calling via a function pointer.
Am 03.07.2024 um 02:43 schrieb Alan Modra: > On Mon, Jul 01, 2024 at 04:33:20PM +0200, Jens Remus wrote: >> I had a quick look at bfd.ld AArch64 and PPC64. As you listed they both >> replace branches with NOPs. Yet it is unclear to me how/where they deal with >> (function) pointers resolving to zero. > > ppc64 does nothing special. As with most other targets, you need to > write > if (foo) > foo (); On s390x both the address taking in "if (foo)" and the call of "foo ()" have a PC-relative relocation that may overflow for undefined weak symbols that definitively resolve to zero. So both instructions need to be rewritten. In the ppc64 code I think I found where the call of "foo ()" is apparently rewritten as a NOP in bfd/elf64-ppc.c ppc64_elf_relocate_section (): "NOP out calls to undefined weak functions. ...". I did not find where the address taking in "if (foo)" is rewritten with a load of zero. > when dealing with a weak function foo that may or may not be defined > at runtime. However, if you know the function will be resolved at > link time (static linking, or -z nodynamic-undefined-weak) then you > can just write > foo (); > and get the same behaviour as > if (foo) > foo (); > > So replacing a branch and link with a nop is only useful in special > cases, and of course doesn't apply if you're calling via a function > pointer. Thank you for the explanation! Thanks and regards, Jens
Am 26.06.2024 um 17:20 schrieb Jens Remus: > Patch 1 corrects commit 896a639babe2 ("s390: Avoid reloc overflows on > undefined weak symbols") not to replace Branch Relative on Count High > (brcth) referencing an undefined weak symbol definitively resolving to > zero by a trap, as it is not guaranteed that the conditional branch is > taken in any case. > > Patch 2 complements commit 896a639babe2 ("s390: Avoid reloc overflows > on undefined weak symbols") by applying similar replacements of > instructions referencing undefined weak symbols that definitively > resolve to zero. This time for PLT32DBL relocations. ... > Jens Remus (2): > s390: Do not replace brcth referencing undefined weak symbol > s390: Avoid reloc overflows on undefined weak symbols (cont) ... Committed to mainline with Andreas' approval. Regards, Jens