Skip to content

Fix R_AVR_7_PCREL and R_AVR_13_PCREL range checking #92695

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions lld/ELF/Arch/AVR.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -231,14 +231,13 @@ void AVR::relocate(uint8_t *loc, const Relocation &rel, uint64_t val) const {

// Since every jump destination is word aligned we gain an extra bit
case R_AVR_7_PCREL: {
checkInt(loc, val - 2, 7, rel);
checkInt(loc, val - 2, 8, rel);
checkAlignment(loc, val, 2, rel);
const uint16_t target = (val - 2) >> 1;
write16le(loc, (read16le(loc) & 0xfc07) | ((target & 0x7f) << 3));
break;
}
case R_AVR_13_PCREL: {
checkInt(loc, val - 2, 13, rel);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not catch your point at here, indeed RJMP/RCALL have a range limit of [-4096, 4094], why we can not check it for attiny85 ? And How can it jump over 4KB with RJMP on attiny85?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there an example, that RJMP can exceeds 4094 bytes on attiny85, which is allowed by avr-ld ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like these were older comments?

checkAlignment(loc, val, 2, rel);
const uint16_t target = (val - 2) >> 1;
write16le(loc, (read16le(loc) & 0xf000) | (target & 0xfff));
Expand Down
5 changes: 2 additions & 3 deletions lld/test/ELF/avr-reloc-error.s
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
# RUN: rm -rf %t && split-file %s %t && cd %t

# RUN: llvm-mc -filetype=obj -triple=avr -mcpu=atmega328 avr-pcrel-7.s -o avr-pcrel-7.o
# RUN: not ld.lld avr-pcrel-7.o -o /dev/null -Ttext=0x1000 --defsym=callee0=0x1040 --defsym=callee1=0x1044 --defsym=callee2=0x100f 2>&1 | \
# RUN: not ld.lld avr-pcrel-7.o -o /dev/null -Ttext=0x1000 --defsym=callee0=0x1040 --defsym=callee1=0x1084 --defsym=callee2=0x100f 2>&1 | \
# RUN: FileCheck %s --check-prefix=PCREL7
# RUN: llvm-mc -filetype=obj -triple=avr -mcpu=atmega328 avr-pcrel-13.s -o avr-pcrel-13.o
# RUN: not ld.lld avr-pcrel-13.o -o /dev/null -Ttext=0x1000 --defsym=callee0=0x2000 --defsym=callee1=0x2004 --defsym=callee2=0x100f 2>&1 | \
Expand All @@ -20,7 +20,7 @@
__start:

# PCREL7-NOT: callee0
# PCREL7: error: {{.*}} relocation R_AVR_7_PCREL out of range: {{.*}} is not in [-64, 63]; references 'callee1'
# PCREL7: error: {{.*}} relocation R_AVR_7_PCREL out of range: {{.*}} is not in [-128, 127]; references 'callee1'
# PCREL7: error: {{.*}} improper alignment for relocation R_AVR_7_PCREL: {{.*}} is not aligned to 2 bytes
brne callee0
breq callee1
Expand All @@ -34,7 +34,6 @@ brlt callee2
__start:

# PCREL13-NOT: callee0
# PCREL13: error: {{.*}} relocation R_AVR_13_PCREL out of range: {{.*}} is not in [-4096, 4095]; references 'callee1'
# PCREL13: error: {{.*}} improper alignment for relocation R_AVR_13_PCREL: {{.*}} is not aligned to 2 bytes
rjmp callee0
rcall callee1
Expand Down
12 changes: 12 additions & 0 deletions lld/test/ELF/avr-reloc.s
Original file line number Diff line number Diff line change
Expand Up @@ -82,13 +82,25 @@ sbic b, 1 ; R_AVR_PORT5
; CHECK-NEXT: rjmp .-36
; CHECK-NEXT: breq .+26
; CHECK-NEXT: breq .-40
; CHECK-NEXT: rjmp .-4096
; CHECK-NEXT: rjmp .+4094
; CHECK-NEXT: rjmp .+4094
; CHECK-NEXT: rjmp .-4096
; CHECK-NEXT: breq .-128
; CHECK-NEXT: breq .+126
; HEX-LABEL: section .PCREL:
; HEX-NEXT: 0fc0eecf 69f061f3
foo:
rjmp foo + 32 ; R_AVR_13_PCREL
rjmp foo - 32 ; R_AVR_13_PCREL
breq foo + 32 ; R_AVR_7_PCREL
breq foo - 32 ; R_AVR_7_PCREL
rjmp 1f - 4096 $ 1: ; R_AVR_13_PCREL
rjmp 1f + 4094 $ 1: ; R_AVR_13_PCREL
rjmp 1f - 4098 $ 1: ; R_AVR_13_PCREL (overflow)
rjmp 1f + 4096 $ 1: ; R_AVR_13_PCREL (overflow)
breq 1f - 128 $ 1: ; R_AVR_7_PCREL
breq 1f + 126 $ 1: ; R_AVR_7_PCREL
Comment on lines +98 to +103
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For those not familiar with this syntax, the $ symbol works as a newline.
So this code:

rjmp 1f - 4096  $ 1:

Is equivalent to the following:

rjmp 1f - 4096
1:

(where 1f refers to the next anonymous label numbered "1")


.section .LDSSTS,"ax",@progbits
; CHECK-LABEL: section .LDSSTS:
Expand Down
Loading