pkarashchenko commented on code in PR #11322:
URL: https://github.com/apache/nuttx/pull/11322#discussion_r1424526580


##########
libs/libc/machine/risc-v/arch_elf.c:
##########
@@ -317,58 +396,101 @@ int up_relocateadd(const Elf_Rela *rel, const Elf_Sym 
*sym,
         break;
 
       case R_RISCV_PCREL_LO12_I:
-      case R_RISCV_PCREL_LO12_S:
         {
+          long imm_hi;
+          long imm_lo;
+
           binfo("%s at %08" PRIxPTR " [%08" PRIx32 "] "
                 "to sym=%p st_value=%08lx\n",
                 _get_rname(relotype),
                 addr, _get_val((uint16_t *)addr),
                 sym, sym->st_value);
 
-          /* NOTE: imm value for mv has been adjusted in previous HI20 */
+          offset = _find_hi20(arch_data, sym->st_value);
+
+          /* Adjust imm for MV(ADDI) / JR (JALR) : I-type */
+
+          _calc_imm(offset, &imm_hi, &imm_lo);
+
+          _add_val((uint16_t *)addr, (int32_t)imm_lo << 20);
         }
         break;
 
-      case R_RISCV_PCREL_HI20:
-      case R_RISCV_CALL:
-      case R_RISCV_CALL_PLT:
+      case R_RISCV_PCREL_LO12_S:
         {
+          uint32_t val;
+          long imm_hi;
+          long imm_lo;
+
           binfo("%s at %08" PRIxPTR " [%08" PRIx32 "] "
                 "to sym=%p st_value=%08lx\n",
                 _get_rname(relotype),
                 addr, _get_val((uint16_t *)addr),
                 sym, sym->st_value);
 
-          offset = (long)sym->st_value + (long)rel->r_addend - (long)addr;
+          offset = _find_hi20(arch_data, sym->st_value);
+
+          /* Adjust imm for SW : S-type */
+
+          _calc_imm(offset, &imm_hi, &imm_lo);
+
+          val = (((int32_t)imm_lo >> 5) << 25) +
+                (((int32_t)imm_lo & 0x1f) << 7);
 
+          binfo("imm_lo=%ld (%lx), val=%" PRIx32 "\n", imm_lo, imm_lo, val);
+
+          _add_val((uint16_t *)addr, val);
+        }
+        break;
+
+      case R_RISCV_PCREL_HI20:
+        {
           long imm_hi;
           long imm_lo;
 
+          binfo("%s at %08" PRIxPTR " [%08" PRIx32 "] "
+                "to sym=%p st_value=%08lx\n",
+                _get_rname(relotype),
+                addr, _get_val((uint16_t *)addr),
+                sym, sym->st_value);
+
+          offset = (long)sym->st_value + (long)rel->r_addend - (long)addr;
+
           _calc_imm(offset, &imm_hi, &imm_lo);
 
           /* Adjust auipc (add upper immediate to pc) : 20bit */
 
-          _add_val((uint16_t *)addr, (imm_hi << 12));
+          _add_val((uint16_t *)addr, imm_hi << 12);

Review Comment:
   No. Please move forward with merge.
   @pussuw Both `medlow` 
(https://github.com/riscv-collab/riscv-gnu-toolchain/issues/103) and `medany` 
(https://github.com/riscv-collab/riscv-gnu-toolchain/issues/733) modes are 
affected on RV64. Only RV32 is not affected as there is not place for sign bit 
extension obviously. The `medlow` is affected more compared to `medany` because 
of its nature.
   The https://github.com/riscv-collab/riscv-gnu-toolchain/issues/733 describes 
clearly how this can be detected by linker. From my investigations it is enough 
to have bit 31 set in symbol address to get linker error, so it highly depends 
on the SoC address mapping.
   I would not argue if this is or is not related to this PR, but rather 
spotted things I've met recently, so please feel free to make a merge decision.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to