Skip to content

AVR: miscompilation, or stack overflow, when running cryptographic code #109000

@xphoniex

Description

@xphoniex
Contributor

Using rust to compile for Arduino target, I'm see a lot of weird and random behaviors from compiler when making changes to profile and compiler setting. (I'm using avr-hal for uno, here's a sample)

As an example, if I use hmac-sha256 crate:

    let h = hmac_sha256::HMAC::mac(b"hello", b"key"); // hmac for input "hello" with key "key"
    print_hex_arr(" mac", &mut serial, &h);
    let h = hmac_sha256::Hash::hash(b"hello"); // sha256 of "hello"
    print_hex_arr("hash", &mut serial, &h);

I get different outputs depending on opt-level and lto, correct one is:

 mac = 9307b3b915efb5171ff14d8cb55fbcc798c6c0ef1456d66ded1a6aa723a58b7b
hash = 2cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9824

which I'm getting from opt-level=2 with both lto=true and unspecified, however:

// opt-level = "s", lto = true
 mac = d238e536e20f0b1b210644248134891454c20ad29c10f75756218b6ab8f5c17d
hash = 5f23619d4ed28dbf06d25969fa262384869011f4a44469c1eb38c50b15b01c4b

// opt-level = "z", lto = true
 mac = 03942da43b5034c3f87f9652c4d569392f444af18c66a7c587db8065d4c79faf
hash = 5f23619d4ed28dbf06d25969fa262384869011f4a44469c1eb38c50b15b01c4b

// opt-level = "s | z", lto unspecified
 mac = d238e536e20f0b1b210644248134891454c20ad29c10f75756218b6ab8f5c17d
hash = 5f23619d4ed28dbf06d25969fa262384869011f4a44469c1eb38c50b15b01c4b

not only that, even if I run fns from another crate, even with the working opt-level = 2, I get incorrect output:

  other_crate::do_something();
  let h = hmac_sha256::Hash::hash(b"hello");
  print_hex_arr("hash", &mut serial, &h);

has a wrong output and instead this works:

  let h = hmac_sha256::Hash::hash(b"hello");
  print_hex_arr("hash", &mut serial, &h);
  other_crate::do_something();

before I compile a bug report at gcc, I want to make sure the fault is not at rust side. here's the final command that rustc is running to link the final elf:

$ avr-gcc -mmcu=atmega328p -Wl,--as-needed,--print-memory-usage,--detailed-mem-usage -fpack-struct -fshort-enums -Wstack-usage=20 -Wall -Wextra -fstack-usage /tmp/rustcsspEI5/symbols.o /project/target/avr-atmega328p/release/deps/arduino_lib-1e0b86c11d560d13.arduino_lib.9b917980-cgu.0.rcgu.o -Wl,--as-needed -L /project/target/avr-atmega328p/release/deps -L /project/target/release/deps -L . -L /home/usr/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/avr-atmega328p/lib -Wl,-Bstatic /project/target/avr-atmega328p/release/deps/libcompiler_builtins-4dcc5d36d44c3317.rlib -Wl,-Bdynamic -lgcc -Wl,-znoexecstack -L /home/usr/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/avr-atmega328p/lib -o /project/target/avr-atmega328p/release/deps/arduino_lib-1e0b86c11d560d13.elf -Wl,--gc-sections -Wl,-O1 -Wl,--strip-all

as you can see it's using -O1 which I haven't specified anywhere. (See avr-atmega328p.json)

I have tried overriding in my avr-atmega328p.json file inside pre-link-args but that doesn't affect anything. Can someone shed some light on what is going wrong here? If it's the -O1 flag, how do I override that?

Activity

saethlin

saethlin commented on Mar 11, 2023

@saethlin
Member

We have some history with AVR miscompilations, so just glancing at this issue my priors are that it is more likely than not that this is a bug somewhere in rustc/LLVM.

You've included a fair bit of information in here, but there's no reproducer that I can run to observe the bug. Can you provide a program (snippet or link to a repo/example) that demonstrates the bug, and precise instructions on how to compile it and observe the bug? It would be a great help if someone tackling this doesn't need to have any particular hardware on hand in order to observe the bug.

added
O-AVRTarget: AVR processors (ATtiny, ATmega, etc.)
A-LLVMArea: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.
on Mar 11, 2023
workingjubilee

workingjubilee commented on Mar 11, 2023

@workingjubilee
Member

To add to saethlin's request for a reproducer:

An entire repo is often fine as a reproducer for AVR issues, even though it is usually not for other targets. Often the problem is either so vague that further minimization is futile (so it can't be minimized) or so glaring that further minimization is pointless (making it apparent at a glance at the Cargo.toml or something like that). In particular, a full repo can be desired because some common ecosystem dependencies have used unsound code recently and you may have included one of them in your dependency graph, so please check in your Cargo.lock when pushing the repo so I can see what exact versions of crates you resolved.

Also, for that repo, please include the exact rustc/cargo build commands you are using to produce this issue, ideally. A file in the repo listing all of them would not be too much detail.

added
A-codegenArea: Code generation
A-LTOArea: Link-time optimization (LTO)
C-bugCategory: This is a bug.
and removed
A-LLVMArea: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.
on Mar 11, 2023
xphoniex

xphoniex commented on Mar 11, 2023

@xphoniex
ContributorAuthor

zipped file: avr-bug.zip

$ tree .
.
├── avr-atmega328p.json
├── .cargo
│   └── config.toml
├── Cargo.lock
├── Cargo.toml
├── rust-toolchain.toml
└── src
    └── main.rs

avr-atmega328p.json

{
  "arch": "avr",
  "atomic-cas": false,
  "cpu": "atmega328p",
  "data-layout": "e-P1-p:16:8-i8:8-i16:8-i32:8-i64:8-f32:8-f64:8-n8-a:8",
  "eh-frame-header": false,
  "exe-suffix": ".elf",
  "executables": true,
  "late-link-args": {
    "gcc": [
      "-lgcc"
    ]
  },
  "linker": "avr-gcc",
  "linker-is-gnu": true,
  "llvm-target": "avr-unknown-unknown",
  "max-atomic-width": 8,
  "no-default-libraries": false,
  "pre-link-args": {
    "gcc": [
      "-mmcu=atmega328p",
      "-Wl,--as-needed,--print-memory-usage,--detailed-mem-usage",
      "-fpack-struct",
	  "-fshort-enums",
	  "-Wstack-usage=20",
	  "-Wall","-Wextra","-fstack-usage"
    ]
  },
  "target-c-int-width": "16",
  "target-pointer-width": "16"
}

.cargo/config.toml

rustflags = ["-Z emit-stack-sizes"]

[target.avr-atmega328p]
runner = "qemu-system-avr -M uno -nographic -serial tcp::5678,server=on -bios"

[build]
target = "avr-atmega328p.json"

[unstable]
build-std = ["core"]

Cargo.toml

[package]
name = "avr-bug"
version = "0.1.0"
edition = "2021"

[dependencies]
panic-halt = "0.2.0"
ufmt = "0.1.0"
noble-secp256k1 = { git = "https://github.com/xphoniex/noble-secp256k1-rs", default-features = false, features = ["8-bit"] }
hmac-sha256 = { version = "1.1.6", default-features = false, features = ["opt_size"] }

[dependencies.arduino-hal]
git = "https://github.com/rahix/avr-hal"
rev = "4c9c44c314eb061ee20556ef10d45dea36e75ee4"
features = ["arduino-uno"]

[dependencies.avr-device]
version = "0.5.0"

[profile.dev]
panic = "abort"
debug = true
lto = true
opt-level = "s"

[profile.release]
panic = "abort"
codegen-units = 1
debug = true
lto = true
#opt-level = "s"
#opt-level = "z"
opt-level = 2
strip = true

[profile.dev.package.compiler_builtins]
overflow-checks = false

rust-toolchain.toml

[toolchain]
channel = "nightly"
components = [ "rust-src" ]
profile = "minimal"

src/main.rs

#![no_std]
#![no_main]

use panic_halt as _;

use arduino_hal::prelude::*;

use core::fmt::Debug;
use ufmt::uWrite;

fn print_hex_arr<S>(tag: &str, serial: &mut S, arr: &[u8])
where
    S: uWrite,
    <S as uWrite>::Error: Debug,
{
    ufmt::uwrite!(serial, "{} = ", tag).unwrap();
    for e in arr.iter() {
        ufmt::uwrite!(serial, "{:02x}", *e).unwrap();
    }
    ufmt::uwrite!(serial, "\r\n").unwrap();
}

fn print_hex_arr_rev<S>(tag: &str, serial: &mut S, arr: &[u8])
where
    S: uWrite,
    <S as uWrite>::Error: Debug,
{
    ufmt::uwrite!(serial, "{} = ", tag).unwrap();
    for e in arr.iter().rev() {
        ufmt::uwrite!(serial, "{:02x}", *e).unwrap();
    }
    ufmt::uwrite!(serial, "\r\n").unwrap();
}

#[arduino_hal::entry]
fn main() -> ! {
    let dp = arduino_hal::Peripherals::take().unwrap();
    let pins = arduino_hal::pins!(dp);
    let mut serial = arduino_hal::default_serial!(dp, pins, 57600);

    /*
    // Uncommenting this part changes the output of hmacs
    use noble_secp256k1::awint::{cc, inlawi, inlawi_ty, Bits, InlAwi};
    use noble_secp256k1::{BigNum, Curve};

    let mut private_key = inlawi!(0x02_u512);
    let curve = Curve::secp256k1();
    let public_key = curve.multiply_simple(&mut private_key);

    let mut buf = [0; 32];
    public_key.x.to_u8_slice(&mut buf);
    print_hex_arr_rev("x", &mut serial, &buf);
    public_key.y.to_u8_slice(&mut buf);
    print_hex_arr_rev("y", &mut serial, &buf);
    */

    let h = hmac_sha256::HMAC::mac(b"hello", b"key");
    print_hex_arr(" mac", &mut serial, &h);
    let h = hmac_sha256::Hash::hash(b"hello");
    print_hex_arr("hash", &mut serial, &h);

    loop {}
}
$ rustc -vV
rustc 1.69.0-nightly (ef934d9b6 2023-02-08)
binary: rustc
commit-hash: ef934d9b632b8ac00276558824664c104b92b5f0
commit-date: 2023-02-08
host: x86_64-unknown-linux-gnu
release: 1.69.0-nightly
LLVM version: 15.0.7
xphoniex

xphoniex commented on Mar 11, 2023

@xphoniex
ContributorAuthor

In order to run this you need both avr toolchain such as avr-gcc and qemu-system-avr. In a terminal run cargo run -r and in another:

$ telnet localhost 5678
Trying 127.0.0.1...
Connected to localhost.
Escape character is '^]'.
 mac = 9307b3b915efb5171ff14d8cb55fbcc798c6c0ef1456d66ded1a6aa723a58b7b
hash = 2cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9824

then try changing opt-level, lto, and uncomment the other crate in the main.rs, and the move code around. you should be able to re-produce the bugs.

workingjubilee

workingjubilee commented on Mar 11, 2023

@workingjubilee
Member

For future reference, creating and pushing a repo to GitHub or another host that I can clone with a single command would have been preferred over something that requires me to manually reconstruct the files inside a repository.

xphoniex

xphoniex commented on Mar 11, 2023

@xphoniex
ContributorAuthor

For future reference, creating and pushing a repo to GitHub or another host that I can clone with a single command would have been preferred over something that requires me to manually reconstruct the files inside a repository.

You're right, added the zip file.

workingjubilee

workingjubilee commented on Mar 11, 2023

@workingjubilee
Member

Hmm. I don't know, in that case! Fair enough?!

Thank you for the clear instructions, I will try to reproduce this soon.

Also you will want to update to a version of rustc of about February 19 or later, as an ABI fix for AVR targets was merged about then. It doesn't affect your visible source but it may affect your dependencies.

added
T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.
on Apr 5, 2023
xphoniex

xphoniex commented on May 30, 2023

@xphoniex
ContributorAuthor

polite bump. any updates on this?

39 remaining items

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-LTOArea: Link-time optimization (LTO)A-codegenArea: Code generationC-bugCategory: This is a bug.I-miscompileIssue: Correct Rust code lowers to incorrect machine codeI-unsoundIssue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/SoundnessO-AVRTarget: AVR processors (ATtiny, ATmega, etc.)P-lowLow priorityT-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @oli-obk@Patryk27@fvilante@apiraino@saethlin

        Issue actions

          AVR: miscompilation, or stack overflow, when running cryptographic code · Issue #109000 · rust-lang/rust