public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Vladislav Khmelevsky <och95@yandex.ru>
To: binutils@sourceware.org, ccoutant@gmail.com, schwab@linux-m68k.org
Cc: Vladislav Khmelevsky <och95@yandex.ru>
Subject: [PATCH] gold/aarch64: Fix adrp distance check
Date: Mon,  8 Aug 2022 22:18:46 +0300	[thread overview]
Message-ID: <20220808191846.3365288-1-och95@yandex.ru> (raw)
In-Reply-To: <CAJimCsE=EYscn5hGT43=VGk=6GFWm=STqCXvbj-iggcQQMpJNQ@mail.gmail.com>

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<size, big_endian>
   aarch64_valid_for_adrp_p(AArch64_address location, AArch64_address dest)
   {
     typedef AArch64_relocate_functions<size, big_endian> 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


  reply	other threads:[~2022-08-08 19:19 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-25 17:52 Vladislav Khmelevsky
2022-07-29 20:14 ` Cary Coutant
2022-07-29 20:44   ` Andreas Schwab
2022-07-29 22:14     ` Cary Coutant
2022-08-08 19:18       ` Vladislav Khmelevsky [this message]
2022-11-07 11:28 Vladislav Khmelevsky
2022-11-11  0:49 ` Cary Coutant

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220808191846.3365288-1-och95@yandex.ru \
    --to=och95@yandex.ru \
    --cc=binutils@sourceware.org \
    --cc=ccoutant@gmail.com \
    --cc=schwab@linux-m68k.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).