From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oa1-x2f.google.com (mail-oa1-x2f.google.com [IPv6:2001:4860:4864:20::2f]) by sourceware.org (Postfix) with ESMTPS id E5DC7384F6FA for ; Fri, 18 Nov 2022 08:33:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E5DC7384F6FA Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=rivosinc.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=rivosinc.com Received: by mail-oa1-x2f.google.com with SMTP id 586e51a60fabf-14279410bf4so3388319fac.8 for ; Fri, 18 Nov 2022 00:33:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rivosinc-com.20210112.gappssmtp.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=fVTClgc96R/I0mwCloq1Ax0ivG1g9KvEVLQ62bgS/vE=; b=sTvhiDF/QUHQr+Tg1syaaAtx7DE70Qvcwi4PdOmejTSwGPUcsm+p8VLijGtBUtl9Aj g++MJUVt4+xKI7I+9wwB1rOvd59NUoOEFNmTLSFgiKEBp4xL+fhETv92JE1u1YyoJPA1 jbTOIRTNgE+Lj6s8aIQg+juOjFC6TSMfztvc7j15aPo37wHfw8zBtbomFM2qRj7Ar/Jx MWx7PxnEVZdgc0O28zaEnutZwpd6hMte91t9Kk0p+GLWNNbAK4ghIKhGMPSv17DYizZy UKkQOPZHNbtyaFH+gVKyTMSwEOQHFhKerIyW9iqQmHO1Rp/UAiBbSej0DEYNJpArdofK ubew== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=fVTClgc96R/I0mwCloq1Ax0ivG1g9KvEVLQ62bgS/vE=; b=JETvf76QFB+bxm/hAEKI9O2Cy3bbbVLz4ESzDpvJrccu5c91aouO/sadNKH3U3j8NN YTg6euQDhiJW9xFKzUuIJInmryF88N4U16blhXhGGFWISm4VGHRJQwBo1u1RZ9ZKE4a6 Fi6JM62rrp3WsmsZTFIODYKc1HB5/RyV2ZFlFyXrVdkXSIMDLFFnYolL7kP9Eqg9VjrG yj2I0uCmHT32LkbwAKmHXRbdJCh5BcwQsT0jxgcqCwMnQRhHYx+LK34tDHxZjn+EKlPU KNLMyk3A9pV+moGCrN5hnySWaTNQQzNtBkhQmf8JXZQiEQLYZYj/NwHafiCOlnqPFecw 8RzA== X-Gm-Message-State: ANoB5pmJ3WaSQOzJH7jNWpD93L1OJzsAPQyKBznHzJ8d67VgQQsvOdlV deJjkJtjzzGMJjMZeKZ3ZUOm5tcfNJdoKa+Da6RHWcNGFpyAeyPw X-Google-Smtp-Source: AA0mqf4WH0MF1yFAx4j4v7ij/neqrmd1llCmempPj5KwvX4lc1i9wWh6k2LxOLc0iwRtBgkji7MoVWubECkUTpH77VA= X-Received: by 2002:a05:6870:d582:b0:142:6390:4724 with SMTP id u2-20020a056870d58200b0014263904724mr3067880oao.82.1668760436374; Fri, 18 Nov 2022 00:33:56 -0800 (PST) MIME-Version: 1.0 References: <2bb257a9.2114e.18467527bf0.Coremail.shihua@iscas.ac.cn> In-Reply-To: From: Nelson Chu Date: Fri, 18 Nov 2022 16:33:45 +0800 Message-ID: Subject: Re: [PATCH 1/1] RISC-V: Make R_RISCV_SUB6 conforms to riscv abi standard To: Palmer Dabbelt Cc: shihua@iscas.ac.cn, zengxiao@eswincomputing.com, binutils@sourceware.org Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-8.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,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 List-Id: On Fri, Nov 18, 2022 at 1:00 PM Palmer Dabbelt wrote: > > On Fri, 11 Nov 2022 07:32:49 PST (-0800), shihua@iscas.ac.cn wrote: > > LGTM,and I think it would be better to have a test example. > > > > > > > > > > > >> From: zengxiao > >> > >> This patch makes R_RISCV_SUB6 conforms to riscv abi standard. > >> R_RISCV_SUB6 only the lower 6 bits of the code are valid. > >> The proposed specification which can be found in 8.5. Relocations of, > >> https://github.com/riscv-non-isa/riscv-elf-psabi-doc/releases/download/v1.0-rc4/riscv-abi.pdf > >> > >> bfd/ChangeLog: > >> > >> * elfxx-riscv.c (riscv_elf_add_sub_reloc): Take the lower > >> 6 bits as the significant bit > >> > >> reviewed-by: gaofei@eswincomputing.com > >> jinyanjiang@eswincomputing.com > > Is this trying to say that both of you reviewed it? > > >> Signed-off-by: zengxiao > > > >> --- > >> bfd/elfxx-riscv.c | 7 +++++++ > >> 1 file changed, 7 insertions(+) > >> > >> diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c > >> index f0c91cc97f7..0fbfedd17fe 100644 > >> --- a/bfd/elfxx-riscv.c > >> +++ b/bfd/elfxx-riscv.c > >> @@ -994,6 +994,13 @@ riscv_elf_add_sub_reloc (bfd *abfd, > >> relocation = old_value + relocation; > >> break; > >> case R_RISCV_SUB6: > >> + { > >> + bfd_vma six_bit_valid_value = old_value & howto->dst_mask; > >> + six_bit_valid_value -= relocation; > >> + relocation = (six_bit_valid_value & howto->dst_mask) | > >> + (old_value & ~howto->dst_mask); > >> + } > >> + break; > > Unless I'm missing something here, this just just silently truncates the > relocation to 6 bits because the range check still assumes an 8-bit > relocation range. I'm not sure if there's a way to massage the howto > entry to make bfd_reloc_offset_in_range() understand this is a 6-bit > relocation, if that's not viable then we should just check for the > overflow here and return bfd_reloc_outofrange. Yeah agreed, we shouldn't call bfd_reloc_offset_in_range for R_RISCV_SUB6 since we are assuming it is an 8-bit relocation. That means if the R_RISCV_SUB6 is used to relocate the last 6-bit of the section, then ld will always report bfd_reloc_outofrange since it assumes at least 8-bit is needed, although the case seems minor. As for the overflow of relocation values, we don't have any checks of them, so I think we can just ignore them in the short-term. Thanks Nelson > That also means there should be at least two test cases, on within range > and one outside of it. > > >> case R_RISCV_SUB8: > >> case R_RISCV_SUB16: > >> case R_RISCV_SUB32: > >> -- > >> 2.34.1