Skip to content

[AVR] Wrap out-of-bounds relative jumps #118015

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.

Sign up for GitHub

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 1 commit into from
Dec 27, 2024
Merged

[AVR] Wrap out-of-bounds relative jumps #118015

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
50 changes: 31 additions & 19 deletions llvm/lib/Target/AVR/AVRDevices.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,18 @@ def FeatureSmallStack
"The device has an 8-bit "
"stack pointer">;

Copy link

Choose a reason for hiding this comment

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

hey

// The device potentially requires emitting rjmp that wraps across the flash
// boundary.
//
// We enable this for devices that have exactly 8 kB of flash memory and don't
// support the `jmp` instruction - with this feature enabled, we try to convert
// out-of-bounds relative jumps into in-bounds by wrapping the offset, e.g.
// `rjmp +5000` becomes `rjmp -3192`.
def FeatureWrappingRjmp
: SubtargetFeature<"wrappingrjmp", "HasWrappingRjmp", "true",
"The device potentially requires emitting rjmp that "
"wraps across the flash boundary">;

// The device supports the 16-bit GPR pair MOVW instruction.
def FeatureMOVW : SubtargetFeature<"movw", "HasMOVW", "true",
"The device supports the 16-bit MOVW "
Expand Down Expand Up @@ -274,11 +286,11 @@ def : Device<"at86rf401", FamilyAVR2, ELFArchAVR25, [FeatureMOVW, FeatureLPMX]>;
def : Device<"at90s4414", FamilyAVR2, ELFArchAVR2, [FeatureSmallStack]>;
def : Device<"at90s4433", FamilyAVR2, ELFArchAVR2, [FeatureSmallStack]>;
def : Device<"at90s4434", FamilyAVR2, ELFArchAVR2, [FeatureSmallStack]>;
def : Device<"at90s8515", FamilyAVR2, ELFArchAVR2>;
def : Device<"at90c8534", FamilyAVR2, ELFArchAVR2>;
def : Device<"at90s8535", FamilyAVR2, ELFArchAVR2>;
def : Device<"ata5272", FamilyAVR25, ELFArchAVR25>;
def : Device<"ata6616c", FamilyAVR25, ELFArchAVR25>;
def : Device<"at90s8515", FamilyAVR2, ELFArchAVR2, [FeatureWrappingRjmp]>;
def : Device<"at90c8534", FamilyAVR2, ELFArchAVR2, [FeatureWrappingRjmp]>;
def : Device<"at90s8535", FamilyAVR2, ELFArchAVR2, [FeatureWrappingRjmp]>;
def : Device<"ata5272", FamilyAVR25, ELFArchAVR25, [FeatureWrappingRjmp]>;
def : Device<"ata6616c", FamilyAVR25, ELFArchAVR25, [FeatureWrappingRjmp]>;
def : Device<"attiny13", FamilyAVR25, ELFArchAVR25, [FeatureSmallStack]>;
def : Device<"attiny13a", FamilyAVR25, ELFArchAVR25, [FeatureSmallStack]>;
def : Device<"attiny2313", FamilyAVR25, ELFArchAVR25, [FeatureSmallStack]>;
Expand All @@ -288,24 +300,24 @@ def : Device<"attiny24a", FamilyAVR25, ELFArchAVR25, [FeatureSmallStack]>;
def : Device<"attiny4313", FamilyAVR25, ELFArchAVR25>;
def : Device<"attiny44", FamilyAVR25, ELFArchAVR25>;
def : Device<"attiny44a", FamilyAVR25, ELFArchAVR25>;
def : Device<"attiny84", FamilyAVR25, ELFArchAVR25>;
def : Device<"attiny84a", FamilyAVR25, ELFArchAVR25>;
def : Device<"attiny84", FamilyAVR25, ELFArchAVR25, [FeatureWrappingRjmp]>;
def : Device<"attiny84a", FamilyAVR25, ELFArchAVR25, [FeatureWrappingRjmp]>;
def : Device<"attiny25", FamilyAVR25, ELFArchAVR25, [FeatureSmallStack]>;
def : Device<"attiny45", FamilyAVR25, ELFArchAVR25>;
def : Device<"attiny85", FamilyAVR25, ELFArchAVR25>;
def : Device<"attiny85", FamilyAVR25, ELFArchAVR25, [FeatureWrappingRjmp]>;
def : Device<"attiny261", FamilyAVR25, ELFArchAVR25, [FeatureSmallStack]>;
def : Device<"attiny261a", FamilyAVR25, ELFArchAVR25, [FeatureSmallStack]>;
def : Device<"attiny441", FamilyAVR25, ELFArchAVR25>;
def : Device<"attiny461", FamilyAVR25, ELFArchAVR25>;
def : Device<"attiny461a", FamilyAVR25, ELFArchAVR25>;
def : Device<"attiny841", FamilyAVR25, ELFArchAVR25>;
def : Device<"attiny861", FamilyAVR25, ELFArchAVR25>;
def : Device<"attiny861a", FamilyAVR25, ELFArchAVR25>;
def : Device<"attiny87", FamilyAVR25, ELFArchAVR25>;
def : Device<"attiny841", FamilyAVR25, ELFArchAVR25, [FeatureWrappingRjmp]>;
def : Device<"attiny861", FamilyAVR25, ELFArchAVR25, [FeatureWrappingRjmp]>;
def : Device<"attiny861a", FamilyAVR25, ELFArchAVR25, [FeatureWrappingRjmp]>;
def : Device<"attiny87", FamilyAVR25, ELFArchAVR25, [FeatureWrappingRjmp]>;
def : Device<"attiny43u", FamilyAVR25, ELFArchAVR25>;
def : Device<"attiny48", FamilyAVR25, ELFArchAVR25>;
def : Device<"attiny88", FamilyAVR25, ELFArchAVR25>;
def : Device<"attiny828", FamilyAVR25, ELFArchAVR25>;
def : Device<"attiny88", FamilyAVR25, ELFArchAVR25, [FeatureWrappingRjmp]>;
def : Device<"attiny828", FamilyAVR25, ELFArchAVR25, [FeatureWrappingRjmp]>;
def : Device<"at43usb355", FamilyAVR3, ELFArchAVR3>;
def : Device<"at76c711", FamilyAVR3, ELFArchAVR3>;
def : Device<"atmega103", FamilyAVR31, ELFArchAVR31>;
Expand All @@ -321,11 +333,11 @@ def : Device<"atmega16u2", FamilyAVR35, ELFArchAVR35>;
def : Device<"atmega32u2", FamilyAVR35, ELFArchAVR35>;
def : Device<"attiny1634", FamilyAVR35, ELFArchAVR35>;
def : Device<"atmega8", FamilyAVR2, ELFArchAVR4,
[FeatureMultiplication, FeatureMOVW, FeatureLPMX, FeatureSPM]>;
[FeatureMultiplication, FeatureMOVW, FeatureLPMX, FeatureSPM, FeatureWrappingRjmp]>;
def : Device<"ata6289", FamilyAVR4, ELFArchAVR4>;
def : Device<"atmega8a", FamilyAVR2, ELFArchAVR4,
[FeatureMultiplication, FeatureMOVW, FeatureLPMX, FeatureSPM]>;
def : Device<"ata6285", FamilyAVR4, ELFArchAVR4>;
[FeatureMultiplication, FeatureMOVW, FeatureLPMX, FeatureSPM, FeatureWrappingRjmp]>;
def : Device<"ata6285", FamilyAVR4, ELFArchAVR4, [FeatureWrappingRjmp]>;
def : Device<"ata6286", FamilyAVR4, ELFArchAVR4>;
def : Device<"ata6612c", FamilyAVR4, ELFArchAVR4>;
def : Device<"atmega48", FamilyAVR4, ELFArchAVR4>;
Expand All @@ -339,9 +351,9 @@ def : Device<"atmega88p", FamilyAVR4, ELFArchAVR4>;
def : Device<"atmega88pa", FamilyAVR4, ELFArchAVR4>;
def : Device<"atmega88pb", FamilyAVR4, ELFArchAVR4>;
def : Device<"atmega8515", FamilyAVR2, ELFArchAVR4,
[FeatureMultiplication, FeatureMOVW, FeatureLPMX, FeatureSPM]>;
[FeatureMultiplication, FeatureMOVW, FeatureLPMX, FeatureSPM, FeatureWrappingRjmp]>;
def : Device<"atmega8535", FamilyAVR2, ELFArchAVR4,
[FeatureMultiplication, FeatureMOVW, FeatureLPMX, FeatureSPM]>;
[FeatureMultiplication, FeatureMOVW, FeatureLPMX, FeatureSPM, FeatureWrappingRjmp]>;
def : Device<"atmega8hva", FamilyAVR4, ELFArchAVR4>;
def : Device<"at90pwm1", FamilyAVR4, ELFArchAVR4>;
def : Device<"at90pwm2", FamilyAVR4, ELFArchAVR4>;
Expand Down
69 changes: 35 additions & 34 deletions llvm/lib/Target/AVR/MCTargetDesc/AVRAsmBackend.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,13 @@
#include "llvm/Support/MathExtras.h"
#include "llvm/Support/raw_ostream.h"

// FIXME: we should be doing checks to make sure asm operands
// are not out of bounds.

namespace adjust {

using namespace llvm;

static void signed_width(unsigned Width, uint64_t Value,
std::string Description, const MCFixup &Fixup,
MCContext *Ctx = nullptr) {
MCContext *Ctx) {
if (!isIntN(Width, Value)) {
std::string Diagnostic = "out of range " + Description;

Expand All @@ -46,17 +43,13 @@ static void signed_width(unsigned Width, uint64_t Value,
Diagnostic += " (expected an integer in the range " + std::to_string(Min) +
" to " + std::to_string(Max) + ")";

if (Ctx) {
Ctx->reportError(Fixup.getLoc(), Diagnostic);
} else {
llvm_unreachable(Diagnostic.c_str());
}
Ctx->reportError(Fixup.getLoc(), Diagnostic);
}
}

static void unsigned_width(unsigned Width, uint64_t Value,
std::string Description, const MCFixup &Fixup,
MCContext *Ctx = nullptr) {
MCContext *Ctx) {
if (!isUIntN(Width, Value)) {
std::string Diagnostic = "out of range " + Description;

Expand All @@ -65,17 +58,13 @@ static void unsigned_width(unsigned Width, uint64_t Value,
Diagnostic +=
" (expected an integer in the range 0 to " + std::to_string(Max) + ")";

if (Ctx) {
Ctx->reportError(Fixup.getLoc(), Diagnostic);
} else {
llvm_unreachable(Diagnostic.c_str());
}
Ctx->reportError(Fixup.getLoc(), Diagnostic);
}
}

/// Adjusts the value of a branch target before fixup application.
static void adjustBranch(unsigned Size, const MCFixup &Fixup, uint64_t &Value,
MCContext *Ctx = nullptr) {
MCContext *Ctx) {
// We have one extra bit of precision because the value is rightshifted by
// one.
unsigned_width(Size + 1, Value, std::string("branch target"), Fixup, Ctx);
Expand All @@ -86,13 +75,28 @@ static void adjustBranch(unsigned Size, const MCFixup &Fixup, uint64_t &Value,

/// Adjusts the value of a relative branch target before fixup application.
static void adjustRelativeBranch(unsigned Size, const MCFixup &Fixup,
uint64_t &Value, MCContext *Ctx = nullptr) {
uint64_t &Value, MCContext *Ctx) {
// Jumps are relative to the current instruction.
Value -= 2;

// We have one extra bit of precision because the value is rightshifted by
// one.
signed_width(Size + 1, Value, std::string("branch target"), Fixup, Ctx);
Size += 1;

if (!isIntN(Size, Value) &&
Ctx->getSubtargetInfo()->hasFeature(AVR::FeatureWrappingRjmp)) {
const int32_t FlashSize = 0x2000;
int32_t SignedValue = Value;

uint64_t WrappedValue = SignedValue > 0 ? (uint64_t)(Value - FlashSize)
: (uint64_t)(FlashSize + Value);

if (isIntN(Size, WrappedValue)) {
Value = WrappedValue;
}
}

signed_width(Size, Value, std::string("branch target"), Fixup, Ctx);

// Rightshifts the value by one.
AVR::fixups::adjustBranchTarget(Value);
Expand All @@ -105,7 +109,7 @@ static void adjustRelativeBranch(unsigned Size, const MCFixup &Fixup,
///
/// Offset of 0 (so the result is left shifted by 3 bits before application).
static void fixup_call(unsigned Size, const MCFixup &Fixup, uint64_t &Value,
MCContext *Ctx = nullptr) {
MCContext *Ctx) {
adjustBranch(Size, Fixup, Value, Ctx);

auto top = Value & (0xf00000 << 6); // the top four bits
Expand All @@ -121,7 +125,7 @@ static void fixup_call(unsigned Size, const MCFixup &Fixup, uint64_t &Value,
/// 0000 00kk kkkk k000
/// Offset of 0 (so the result is left shifted by 3 bits before application).
static void fixup_7_pcrel(unsigned Size, const MCFixup &Fixup, uint64_t &Value,
MCContext *Ctx = nullptr) {
MCContext *Ctx) {
adjustRelativeBranch(Size, Fixup, Value, Ctx);

// Because the value may be negative, we must mask out the sign bits
Expand All @@ -135,7 +139,7 @@ static void fixup_7_pcrel(unsigned Size, const MCFixup &Fixup, uint64_t &Value,
/// 0000 kkkk kkkk kkkk
/// Offset of 0 (so the result isn't left-shifted before application).
static void fixup_13_pcrel(unsigned Size, const MCFixup &Fixup, uint64_t &Value,
MCContext *Ctx = nullptr) {
MCContext *Ctx) {
adjustRelativeBranch(Size, Fixup, Value, Ctx);

// Because the value may be negative, we must mask out the sign bits
Expand All @@ -147,8 +151,7 @@ static void fixup_13_pcrel(unsigned Size, const MCFixup &Fixup, uint64_t &Value,
///
/// Resolves to:
/// 10q0 qq10 0000 1qqq
static void fixup_6(const MCFixup &Fixup, uint64_t &Value,
MCContext *Ctx = nullptr) {
static void fixup_6(const MCFixup &Fixup, uint64_t &Value, MCContext *Ctx) {
unsigned_width(6, Value, std::string("immediate"), Fixup, Ctx);

Value = ((Value & 0x20) << 8) | ((Value & 0x18) << 7) | (Value & 0x07);
Expand All @@ -160,7 +163,7 @@ static void fixup_6(const MCFixup &Fixup, uint64_t &Value,
/// Resolves to:
/// 0000 0000 kk00 kkkk
static void fixup_6_adiw(const MCFixup &Fixup, uint64_t &Value,
MCContext *Ctx = nullptr) {
MCContext *Ctx) {
unsigned_width(6, Value, std::string("immediate"), Fixup, Ctx);

Value = ((Value & 0x30) << 2) | (Value & 0x0f);
Expand All @@ -170,8 +173,7 @@ static void fixup_6_adiw(const MCFixup &Fixup, uint64_t &Value,
///
/// Resolves to:
/// 0000 0000 AAAA A000
static void fixup_port5(const MCFixup &Fixup, uint64_t &Value,
MCContext *Ctx = nullptr) {
static void fixup_port5(const MCFixup &Fixup, uint64_t &Value, MCContext *Ctx) {
unsigned_width(5, Value, std::string("port number"), Fixup, Ctx);

Value &= 0x1f;
Expand All @@ -183,8 +185,7 @@ static void fixup_port5(const MCFixup &Fixup, uint64_t &Value,
///
/// Resolves to:
/// 1011 0AAd dddd AAAA
static void fixup_port6(const MCFixup &Fixup, uint64_t &Value,
MCContext *Ctx = nullptr) {
static void fixup_port6(const MCFixup &Fixup, uint64_t &Value, MCContext *Ctx) {
unsigned_width(6, Value, std::string("port number"), Fixup, Ctx);

Value = ((Value & 0x30) << 5) | (Value & 0x0f);
Expand All @@ -195,7 +196,7 @@ static void fixup_port6(const MCFixup &Fixup, uint64_t &Value,
/// Resolves to:
/// 1010 ikkk dddd kkkk
static void fixup_lds_sts_16(const MCFixup &Fixup, uint64_t &Value,
MCContext *Ctx = nullptr) {
MCContext *Ctx) {
unsigned_width(7, Value, std::string("immediate"), Fixup, Ctx);
Value = ((Value & 0x70) << 8) | (Value & 0x0f);
}
Expand All @@ -213,7 +214,7 @@ namespace ldi {
/// 0000 KKKK 0000 KKKK
/// Offset of 0 (so the result isn't left-shifted before application).
static void fixup(unsigned Size, const MCFixup &Fixup, uint64_t &Value,
MCContext *Ctx = nullptr) {
MCContext *Ctx) {
uint64_t upper = Value & 0xf0;
uint64_t lower = Value & 0x0f;

Expand All @@ -223,25 +224,25 @@ static void fixup(unsigned Size, const MCFixup &Fixup, uint64_t &Value,
static void neg(uint64_t &Value) { Value *= -1; }

static void lo8(unsigned Size, const MCFixup &Fixup, uint64_t &Value,
MCContext *Ctx = nullptr) {
MCContext *Ctx) {
Value &= 0xff;
ldi::fixup(Size, Fixup, Value, Ctx);
}

static void hi8(unsigned Size, const MCFixup &Fixup, uint64_t &Value,
MCContext *Ctx = nullptr) {
MCContext *Ctx) {
Value = (Value & 0xff00) >> 8;
ldi::fixup(Size, Fixup, Value, Ctx);
}

static void hh8(unsigned Size, const MCFixup &Fixup, uint64_t &Value,
MCContext *Ctx = nullptr) {
MCContext *Ctx) {
Value = (Value & 0xff0000) >> 16;
ldi::fixup(Size, Fixup, Value, Ctx);
}

static void ms8(unsigned Size, const MCFixup &Fixup, uint64_t &Value,
MCContext *Ctx = nullptr) {
MCContext *Ctx) {
Value = (Value & 0xff000000) >> 24;
ldi::fixup(Size, Fixup, Value, Ctx);
}
Expand Down
Loading
Loading