From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from forward106j.mail.yandex.net (forward106j.mail.yandex.net [5.45.198.249]) by sourceware.org (Postfix) with ESMTPS id 50BA5385702D for ; Mon, 8 Aug 2022 19:19:11 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 50BA5385702D Received: from sas1-1f4a002bb12a.qloud-c.yandex.net (sas1-1f4a002bb12a.qloud-c.yandex.net [IPv6:2a02:6b8:c14:3908:0:640:1f4a:2b]) by forward106j.mail.yandex.net (Yandex) with ESMTP id 57EA06BD5E94; Mon, 8 Aug 2022 22:19:09 +0300 (MSK) Received: by sas1-1f4a002bb12a.qloud-c.yandex.net (smtp/Yandex) with ESMTPSA id Q7ZA9GD6a5-J8hmIwEl; Mon, 08 Aug 2022 22:19:08 +0300 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (Client certificate not present) X-Yandex-Fwd: 1 From: Vladislav Khmelevsky To: binutils@sourceware.org, ccoutant@gmail.com, schwab@linux-m68k.org Cc: Vladislav Khmelevsky Subject: [PATCH] gold/aarch64: Fix adrp distance check Date: Mon, 8 Aug 2022 22:18:46 +0300 Message-Id: <20220808191846.3365288-1-och95@yandex.ru> X-Mailer: git-send-email 2.25.1 In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-13.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE 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.29 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 08 Aug 2022 19:19:13 -0000 Hello Cary, Andreas! Thank you for the review. >> Do you have a test case to illustrate the problem? Actually not, since we will need far call and add128+ mb binary to test that the gold properly calculates the distance. Or we might add union test, but AFAIK there are no such tests in binutils. >> adrp_imm >>= 12; As it was already said this is implementation defined, although usually it's true that it would result in arithmetic shift and would work. As for the formatting I've add sign conversion since it might trigger the compiler warning, but if it is not needed I will remove it. As for the extra offset variable it looks equal to me so I don't have any problems with reformatting the patch as you've wrote :) Thanks! --- gold/aarch64.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gold/aarch64.cc b/gold/aarch64.cc index d2b0747ffdc..514fad96789 100644 --- a/gold/aarch64.cc +++ b/gold/aarch64.cc @@ -1182,7 +1182,8 @@ class Reloc_stub : public Stub_base aarch64_valid_for_adrp_p(AArch64_address location, AArch64_address dest) { typedef AArch64_relocate_functions Reloc; - int64_t adrp_imm = (Reloc::Page(dest) - Reloc::Page(location)) >> 12; + int64_t adrp_imm = Reloc::Page (dest) - Reloc::Page (location); + adrp_imm = adrp_imm < 0 ? ~(~adrp_imm >> 12) : adrp_imm >> 12; return adrp_imm >= MIN_ADRP_IMM && adrp_imm <= MAX_ADRP_IMM; } -- 2.25.1