Translating RISC-V programs directly from ELF files.

cli-rs/src/main.rs

@@ -12,8 +12,8 @@ use powdr_pipeline::Pipeline;
 use powdr_riscv_executor::ProfilerOptions;
 
 use std::ffi::OsStr;
-use std::io;
 use std::{borrow::Cow, io::Write, path::Path};
+use std::{fs, io};
 use strum::{Display, EnumString, EnumVariantNames};
 
 #[derive(Clone, EnumString, EnumVariantNames, Display)]
@@ -63,9 +63,9 @@ enum Commands {
         coprocessors: Option<String>,
 
         /// Convert from the executable ELF file instead of the assembly.
-        #[arg(short = 'e', long)]
+        #[arg(short, long)]
         #[arg(default_value_t = false)]
-        via_elf: bool,
+        elf: bool,
 
         /// Run a long execution in chunks (Experimental and not sound!)
         #[arg(short, long)]
@@ -224,14 +224,14 @@ fn run_command(command: Commands) {
             field,
             output_directory,
             coprocessors,
-            via_elf,
+            elf,
             continuations,
         } => {
             call_with_field!(compile_rust::<field>(
                 &file,
                 Path::new(&output_directory),
                 coprocessors,
-                via_elf,
+                elf,
                 continuations
             ))
         }
@@ -359,9 +359,14 @@ fn compile_riscv_asm<F: FieldElement>(
         None => powdr_riscv::Runtime::base(),
     };
 
-    powdr_riscv::compile_riscv_asm::<F>(
+    powdr_riscv::compile_riscv_asm_bundle::<F>(
         original_file_name,
-        file_names,
+        file_names
+            .map(|name| {
+                let contents = fs::read_to_string(&name).unwrap();
+                (name, contents)
+            })
+            .collect(),
         output_dir,
         true,
         &runtime,

riscv/Cargo.toml

@@ -31,6 +31,8 @@ lalrpop-util = { version = "^0.19", features = ["lexer"] }
 log = "0.4.17"
 mktemp = "0.5.0"
 num-traits = "0.2.15"
+# Use the patched version of raki until the fix is merged.
+# Fixes the name of "mulhsu" instruction.
 raki = { git = "https://github.com/powdr-labs/raki.git", branch = "patch-1" }
 serde_json = "1.0"
 # This is only here to work around https://github.com/lalrpop/lalrpop/issues/750

riscv/src/asm/mod.rs

@@ -110,9 +110,8 @@ impl InstructionArgs for &[Argument] {
 
     fn rr(&self) -> Result<(Register, Register), &'static str> {
         match self {
-            [Argument::Register(r1), Argument::Register(r2) | Argument::RegOffset(None, r2)] => {
-                Ok((*r1, *r2))
-            }
+            [Argument::Register(r1), Argument::Register(r2)
+            | Argument::RegOffset(None | Some(Expression::Number(0)), r2)] => Ok((*r1, *r2)),
             _ => Err("Expected: register, register"),
         }
     }

riscv/src/elf.rs

@@ -10,10 +10,10 @@ use goblin::{
     elf::sym::STT_OBJECT,
     elf::{
         header::{EI_CLASS, EI_DATA, ELFCLASS32, ELFDATA2LSB, EM_RISCV, ET_DYN},
-        program_header,
+        program_header::PT_LOAD,
         reloc::{R_RISCV_32, R_RISCV_HI20, R_RISCV_RELATIVE},
         sym::STT_FUNC,
-        Elf,
+        Elf, ProgramHeader,
     },
 };
 use itertools::{Either, Itertools};
@@ -30,16 +30,8 @@ use crate::{
     Runtime,
 };
 
-struct ElfProgram {
-    symbol_table: SymbolTable,
-    data_map: BTreeMap<u32, Data>,
-    text_labels: BTreeSet<u32>,
-    instructions: Vec<HighLevelInsn>,
-    entry_point: u32,
-}
-
 /// Generates a Powdr Assembly program from a RISC-V 32 executable ELF file.
-pub fn elf_translate<F: FieldElement>(
+pub fn translate<F: FieldElement>(
     file_name: &Path,
     runtime: &Runtime,
     with_bootloader: bool,
@@ -48,6 +40,14 @@ pub fn elf_translate<F: FieldElement>(
     code_gen::translate_program::<F>(elf_program, runtime, with_bootloader)
 }
 
+struct ElfProgram {
+    symbol_table: SymbolTable,
+    data_map: BTreeMap<u32, Data>,
+    text_labels: BTreeSet<u32>,
+    instructions: Vec<HighLevelInsn>,
+    entry_point: u32,
+}
+
 fn load_elf(file_name: &Path) -> ElfProgram {
     log::info!("Loading ELF file: {}", file_name.display());
     let file_buffer = fs::read(file_name).unwrap();
@@ -85,7 +85,7 @@ fn load_elf(file_name: &Path) -> ElfProgram {
     let address_map = AddressMap(
         elf.program_headers
             .iter()
-            .filter(|p| p.p_type == program_header::PT_LOAD)
+            .filter(|p| p.p_type == PT_LOAD)
             .map(|p| (p.p_vaddr as u32, p))
             .collect(),
     );
@@ -108,8 +108,7 @@ fn load_elf(file_name: &Path) -> ElfProgram {
     for (&addr, &p) in address_map.0.iter() {
         let section_data = &file_buffer[p.p_offset as usize..(p.p_offset + p.p_filesz) as usize];
 
-        // Test if executable
-        if p.p_flags & 1 == 1 {
+        if p.is_executable() {
             search_text_addrs(
                 addr,
                 section_data,
@@ -137,7 +136,7 @@ fn load_elf(file_name: &Path) -> ElfProgram {
 
     // Load all the text sections.
     let mut lifted_text_sections = Vec::new();
-    for (&addr, &p) in address_map.0.iter().filter(|(_, p)| p.p_flags & 1 == 1) {
+    for (&addr, &p) in address_map.0.iter().filter(|(_, p)| p.is_executable()) {
         let section_data = &file_buffer[p.p_offset as usize..(p.p_offset + p.p_filesz) as usize];
         let insns = lift_instructions(
             addr,
@@ -241,9 +240,10 @@ struct SymbolTable(HashMap<u32, String>);
 
 impl SymbolTable {
     fn new(elf: &Elf) -> SymbolTable {
-        // TODO: read the symbols from the debug information to be more comprehensive.
         let mut deduplicator = HashMap::new();
         for sym in elf.syms.iter() {
+            // We only care about global symbols that have string names, and are
+            // either functions or variables.
             if sym.st_name == 0 || (sym.st_type() != STT_OBJECT && sym.st_type() != STT_FUNC) {
                 continue;
             }
@@ -270,11 +270,6 @@ impl SymbolTable {
             .map(|name| Cow::Borrowed(name.as_str()))
             .unwrap_or_else(|| Cow::Owned(format!("L{addr:08x}")))
     }
-
-    /// Get the symbol or a default label formed from the address value.
-    fn get_as_string(&self, addr: u32) -> String {
-        self.get(addr).into_owned()
-    }
 }
 
 impl RiscVProgram for ElfProgram {
@@ -287,7 +282,7 @@ impl RiscVProgram for ElfProgram {
         self.data_map.iter().map(|(addr, data)| {
             let value = match data {
                 Data::TextLabel(label) => {
-                    SingleDataValue::LabelReference(self.symbol_table.get_as_string(*label))
+                    SingleDataValue::LabelReference(self.symbol_table.get(*label).into())
                 }
                 Data::Value(value) => SingleDataValue::Value(*value),
             };
@@ -348,7 +343,7 @@ impl<'a> InstructionArgs for WrappedArgs<'a> {
                 rd: None,
                 rs1: None,
                 rs2: None,
-            } => Ok(self.symbol_table.get_as_string(*addr)),
+            } => Ok(self.symbol_table.get(*addr).into()),
             _ => Err(format!("Expected: label, got {:?}", self.args)),
         }
     }
@@ -447,7 +442,7 @@ impl<'a> InstructionArgs for WrappedArgs<'a> {
             } => Ok((
                 Register::new(*rs1 as u8),
                 Register::new(*rs2 as u8),
-                self.symbol_table.get_as_string(*addr),
+                self.symbol_table.get(*addr).into(),
             )),
             _ => Err(format!("Expected: rs1, rs2, label, got {:?}", self.args)),
         }
@@ -462,7 +457,7 @@ impl<'a> InstructionArgs for WrappedArgs<'a> {
                 rs2: None,
             } => Ok((
                 Register::new(*rs1 as u8),
-                self.symbol_table.get_as_string(*addr),
+                self.symbol_table.get(*addr).into(),
             )),
             HighLevelArgs {
                 imm: HighLevelImmediate::CodeLabel(addr),
@@ -471,7 +466,7 @@ impl<'a> InstructionArgs for WrappedArgs<'a> {
                 rs2: None,
             } => Ok((
                 Register::new(*rd as u8),
-                self.symbol_table.get_as_string(*addr),
+                self.symbol_table.get(*addr).into(),
             )),
             _ => Err(format!("Expected: {{rs1|rd}}, label, got {:?}", self.args)),
         }
@@ -522,26 +517,20 @@ impl<'a> InstructionArgs for WrappedArgs<'a> {
 /// Indexes the program sections by their virtual address.
 ///
 /// Allows for querying if an address is in a data or text section.
-struct AddressMap<'a>(BTreeMap<u32, &'a program_header::ProgramHeader>);
+struct AddressMap<'a>(BTreeMap<u32, &'a ProgramHeader>);
 
 impl AddressMap<'_> {
     fn is_in_data_section(&self, addr: u32) -> bool {
-        if let Some(section) = self.get_section_of_addr(addr) {
-            section.p_flags & 1 != 1
-        } else {
-            false
-        }
+        self.get_section_of_addr(addr)
+            .map_or(false, |section| !section.is_executable())
     }
 
     fn is_in_text_section(&self, addr: u32) -> bool {
-        if let Some(section) = self.get_section_of_addr(addr) {
-            section.p_flags & 1 == 1
-        } else {
-            false
-        }
+        self.get_section_of_addr(addr)
+            .map_or(false, ProgramHeader::is_executable)
     }
 
-    fn get_section_of_addr(&self, addr: u32) -> Option<&program_header::ProgramHeader> {
+    fn get_section_of_addr(&self, addr: u32) -> Option<&ProgramHeader> {
         // Get the latest section that starts before the address.
         let section = self
             .0
@@ -710,8 +699,8 @@ impl TwoOrOneMapper<MaybeInstruction, HighLevelInsn> for InstructionLifter<'_> {
             ) if rd_lui == rs1_addi => {
                 // Sometimes, in non-PIE code, this pair of instructions is used
                 // to load an address into a register. We must check if this is
-                // the case, and if the address points to the text section, so
-                // we must load it from a label.
+                // the case, and if the address points to a text section, we
+                // must load it from a label.
                 let is_address = self.rellocs_set.contains(&original_address);
                 let (op, args) =
                     self.composed_immediate(*hi, *lo, *rd_lui, *rd_addi, insn2_addr, is_address)?;
@@ -723,7 +712,7 @@ impl TwoOrOneMapper<MaybeInstruction, HighLevelInsn> for InstructionLifter<'_> {
                 }
             }
             (
-                // All other double instructions we can lift starts with auipc.
+                // All other double instructions we can lift start with auipc.
                 Ins {
                     opc: Op::AUIPC,
                     rd: Some(rd_auipc),
@@ -865,7 +854,7 @@ impl TwoOrOneMapper<MaybeInstruction, HighLevelInsn> for InstructionLifter<'_> {
         };
 
         let mut imm = match insn.opc {
-            // All jump instructions that have the immediate as an address
+            // All jump instructions that have an address as immediate
             Op::JAL | Op::BEQ | Op::BNE | Op::BLT | Op::BGE | Op::BLTU | Op::BGEU => {
                 let addr = (insn.imm.unwrap() + original_address as i32) as u32;
                 if let ReadOrWrite::Write(refs) = &mut self.referenced_text_addrs {
@@ -909,9 +898,9 @@ impl TwoOrOneMapper<MaybeInstruction, HighLevelInsn> for InstructionLifter<'_> {
             },
         };
 
-        // For some reason, atomic instructions comes with the immediate set to
-        // zero instead of None (maybe to mimic assembly syntax? Who knows). Fix
-        // this.
+        // For some reason, atomic instructions come with the immediate set to
+        // zero instead of None (maybe to mimic assembly syntax? Who knows). We
+        // must fix this:
         if let Extensions::A = insn.extension {
             assert!(matches!(imm, HighLevelImmediate::Value(0)));
             imm = HighLevelImmediate::None;
@@ -1010,7 +999,7 @@ impl Iterator for RiscVInstructionIterator<'_> {
             .decode(Isa::Rv32)
             .unwrap_or_else(|_| {
                 // TODO: maybe instead of failing we should just emit `unimp`.
-                // This way we would support the garbage default GNU binutils
+                // This way we would support the default GNU binutils
                 // linker script, that places the ELF header in the text
                 // section.
                 panic!(
@@ -1042,7 +1031,7 @@ impl Iterator for RiscVInstructionIterator<'_> {
                         // Otherwise, there is something more fishy going on, and we
                         // panic.
 
-                        // TODO: we should just emit `unimp` for every unknown.
+                        // TODO: maybe we should just emit `unimp` for every unknown.
                         assert_eq!(
                             bin_instruction, 0,
                             "Failed to decode 16-bit instruction at {:08x}",