xiaoxiang781216 commented on code in PR #11322:
URL: https://github.com/apache/nuttx/pull/11322#discussion_r1422393716
##########
include/nuttx/elf.h:
##########
@@ -142,9 +142,9 @@ bool up_checkarch(FAR const Elf_Ehdr *hdr);
#ifdef CONFIG_LIBC_ARCH_ELF
int up_relocate(FAR const Elf_Rel *rel, FAR const Elf_Sym *sym,
- uintptr_t addr);
-int up_relocateadd(FAR const Elf_Rela *rel,
- FAR const Elf_Sym *sym, uintptr_t addr);
+ uintptr_t addr, FAR void *arch_data);
Review Comment:
```suggestion
uintptr_t addr, FAR void *arch_data);
```
##########
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));
- if ((_get_val((uint16_t *)(addr + 4)) & 0x7f) == OPCODE_SW)
- {
- /* Adjust imm for SW : S-type */
+ /* Add the hi20 value to the cache */
- uint32_t val =
- (((int32_t)imm_lo >> 5) << 25) +
- (((int32_t)imm_lo & 0x1f) << 7);
+ _add_hi20(arch_data, addr, offset);
+ }
+ break;
- binfo("imm_lo=%ld (%lx), val=%" PRIx32 "\n",
- imm_lo, imm_lo, val);
+ case R_RISCV_CALL:
+ case R_RISCV_CALL_PLT:
+ {
+ 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));
Review Comment:
```suggestion
_add_val((uint16_t *)addr, imm_hi << 12);
```
##########
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));
Review Comment:
```suggestion
_add_val((uint16_t *)addr, (int32_t)imm_lo << 20);
```
##########
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;
Review Comment:
should we use long
##########
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);
Review Comment:
PRIx32 -> lx
##########
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));
- if ((_get_val((uint16_t *)(addr + 4)) & 0x7f) == OPCODE_SW)
- {
- /* Adjust imm for SW : S-type */
+ /* Add the hi20 value to the cache */
- uint32_t val =
- (((int32_t)imm_lo >> 5) << 25) +
- (((int32_t)imm_lo & 0x1f) << 7);
+ _add_hi20(arch_data, addr, offset);
+ }
+ break;
- binfo("imm_lo=%ld (%lx), val=%" PRIx32 "\n",
- imm_lo, imm_lo, val);
+ case R_RISCV_CALL:
+ case R_RISCV_CALL_PLT:
+ {
+ 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 + 4), val);
- }
- else
- {
- /* Adjust imm for MV(ADDI)/JALR : I-type */
+ /* Adjust imm for CALL (JALR) : I-type */
- _add_val((uint16_t *)(addr + 4), ((int32_t)imm_lo << 20));
- }
+ _add_val((uint16_t *)(addr + 4), ((int32_t)imm_lo << 20));
Review Comment:
```suggestion
_add_val((uint16_t *)(addr + 4), (int32_t)imm_lo << 20);
```
--
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
