Skip to content
Draft
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
Binary file added .DS_Store
Binary file not shown.
7 changes: 6 additions & 1 deletion harm-runtime/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,9 @@ publish = false

[dependencies]
harm = { workspace = true }
memmap2 = "0.9.9"
memmap2 = { version = "0.9.9", optional = true }
thiserror = "2.0.18"

[features]
default = ["memmap2"]
memmap2 = ["dep:memmap2"]
84 changes: 83 additions & 1 deletion harm-runtime/src/labels.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,93 @@
* This document is licensed under the BSD 3-clause license.
*/

use harm::reloc::Offset;
use std::collections::HashMap;

use harm::reloc::{LabelId, Offset};

#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub enum LabelInfo {
Forward,
// TODO segment
Offset(Offset),
}

#[derive(Debug, Default)]
pub struct LabelRegistry {
named_labels: HashMap<String, LabelId>,
labels: HashMap<LabelId, LabelInfo>,
next_id: usize,
}

impl LabelRegistry {
#[inline]
pub fn new() -> Self {
Self::default()
}

#[inline]
pub fn forward_named_label(&mut self, name: &str) -> LabelId {
if let Some(id) = self.named_labels.get(name) {
*id
} else {
let id = self.next_label();
self.named_labels.insert(name.to_string(), id);
self.labels.insert(id, LabelInfo::Forward);
id
}
}

#[inline]
pub fn forward_label(&mut self) -> LabelId {
let id = self.next_label();
self.labels.insert(id, LabelInfo::Forward);
id
}

pub fn define_label(&mut self, label_id: LabelId, offset: Offset) {
if let Some(info) = self.labels.get_mut(&label_id) {
match info {
LabelInfo::Forward => {
*info = LabelInfo::Offset(offset);
}
LabelInfo::Offset(_) => {
todo!("Label {label_id:?} is already defined");
}
}
} else {
panic!("Label {label_id:?} is not registered");
}
}

#[inline]
pub fn define_named_label(&mut self, name: &str, offset: Offset) -> LabelId {
if let Some(id) = self.named_labels.get(name).copied() {
self.labels.insert(id, LabelInfo::Offset(offset));
id
} else {
let id = self.next_label();
self.named_labels.insert(name.to_string(), id);
self.labels.insert(id, LabelInfo::Offset(offset));
id
}
}

pub fn name_label(&mut self, id: LabelId, name: &str) {
if self.labels.contains_key(&id) {
self.named_labels.insert(name.to_string(), id);
} else {
panic!("Label {id:?} is not registered");
}
}
Comment on lines +49 to +83
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Avoid panics and silent redefinitions in label definition APIs.

define_label uses todo!/panic!, define_named_label overwrites existing definitions, and name_label can rebind names. For a public API this should return Result with explicit errors to prevent accidental label corruption and runtime panics. This will also require updating Assembler call sites.

Suggested fix (return Result and guard redefinitions)
-    pub fn define_label(&mut self, label_id: LabelId, offset: Offset) {
-        if let Some(info) = self.labels.get_mut(&label_id) {
-            match info {
-                LabelInfo::Forward => {
-                    *info = LabelInfo::Offset(offset);
-                }
-                LabelInfo::Offset(_) => {
-                    todo!("Label {label_id:?} is already defined");
-                }
-            }
-        } else {
-            panic!("Label {label_id:?} is not registered");
-        }
-    }
+    pub fn define_label(
+        &mut self,
+        label_id: LabelId,
+        offset: Offset,
+    ) -> Result<(), LabelRegistryError> {
+        if let Some(info) = self.labels.get_mut(&label_id) {
+            match info {
+                LabelInfo::Forward => {
+                    *info = LabelInfo::Offset(offset);
+                    Ok(())
+                }
+                LabelInfo::Offset(_) => Err(LabelRegistryError::AlreadyDefined(label_id)),
+            }
+        } else {
+            Err(LabelRegistryError::Unregistered(label_id))
+        }
+    }

-    pub fn define_named_label(&mut self, name: &str, offset: Offset) -> LabelId {
+    pub fn define_named_label(
+        &mut self,
+        name: &str,
+        offset: Offset,
+    ) -> Result<LabelId, LabelRegistryError> {
         if let Some(id) = self.named_labels.get(name).copied() {
-            self.labels.insert(id, LabelInfo::Offset(offset));
-            id
+            if matches!(self.labels.get(&id), Some(LabelInfo::Offset(_))) {
+                return Err(LabelRegistryError::AlreadyDefined(id));
+            }
+            self.labels.insert(id, LabelInfo::Offset(offset));
+            Ok(id)
         } else {
             let id = self.next_label();
             self.named_labels.insert(name.to_string(), id);
             self.labels.insert(id, LabelInfo::Offset(offset));
-            id
+            Ok(id)
         }
     }

-    pub fn name_label(&mut self, id: LabelId, name: &str) {
-        if self.labels.contains_key(&id) {
-            self.named_labels.insert(name.to_string(), id);
-        } else {
-            panic!("Label {id:?} is not registered");
-        }
-    }
+    pub fn name_label(&mut self, id: LabelId, name: &str) -> Result<(), LabelRegistryError> {
+        if !self.labels.contains_key(&id) {
+            return Err(LabelRegistryError::Unregistered(id));
+        }
+        if let Some(existing) = self.named_labels.get(name) {
+            if *existing != id {
+                return Err(LabelRegistryError::NameConflict(name.to_string()));
+            }
+        }
+        self.named_labels.insert(name.to_string(), id);
+        Ok(())
+    }
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum LabelRegistryError {
    Unregistered(LabelId),
    AlreadyDefined(LabelId),
    NameConflict(String),
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
pub fn define_label(&mut self, label_id: LabelId, offset: Offset) {
if let Some(info) = self.labels.get_mut(&label_id) {
match info {
LabelInfo::Forward => {
*info = LabelInfo::Offset(offset);
}
LabelInfo::Offset(_) => {
todo!("Label {label_id:?} is already defined");
}
}
} else {
panic!("Label {label_id:?} is not registered");
}
}
#[inline]
pub fn define_named_label(&mut self, name: &str, offset: Offset) -> LabelId {
if let Some(id) = self.named_labels.get(name).copied() {
self.labels.insert(id, LabelInfo::Offset(offset));
id
} else {
let id = self.next_label();
self.named_labels.insert(name.to_string(), id);
self.labels.insert(id, LabelInfo::Offset(offset));
id
}
}
pub fn name_label(&mut self, id: LabelId, name: &str) {
if self.labels.contains_key(&id) {
self.named_labels.insert(name.to_string(), id);
} else {
panic!("Label {id:?} is not registered");
}
}
pub fn define_label(
&mut self,
label_id: LabelId,
offset: Offset,
) -> Result<(), LabelRegistryError> {
if let Some(info) = self.labels.get_mut(&label_id) {
match info {
LabelInfo::Forward => {
*info = LabelInfo::Offset(offset);
Ok(())
}
LabelInfo::Offset(_) => Err(LabelRegistryError::AlreadyDefined(label_id)),
}
} else {
Err(LabelRegistryError::Unregistered(label_id))
}
}
#[inline]
pub fn define_named_label(
&mut self,
name: &str,
offset: Offset,
) -> Result<LabelId, LabelRegistryError> {
if let Some(id) = self.named_labels.get(name).copied() {
if matches!(self.labels.get(&id), Some(LabelInfo::Offset(_))) {
return Err(LabelRegistryError::AlreadyDefined(id));
}
self.labels.insert(id, LabelInfo::Offset(offset));
Ok(id)
} else {
let id = self.next_label();
self.named_labels.insert(name.to_string(), id);
self.labels.insert(id, LabelInfo::Offset(offset));
Ok(id)
}
}
pub fn name_label(&mut self, id: LabelId, name: &str) -> Result<(), LabelRegistryError> {
if !self.labels.contains_key(&id) {
return Err(LabelRegistryError::Unregistered(id));
}
if let Some(existing) = self.named_labels.get(name) {
if *existing != id {
return Err(LabelRegistryError::NameConflict(name.to_string()));
}
}
self.named_labels.insert(name.to_string(), id);
Ok(())
}
🤖 Prompt for AI Agents
In `@harm-runtime/src/labels.rs` around lines 49 - 83, The current label APIs
(define_label, define_named_label, name_label) panic or silently overwrite;
change them to return Result<LabelId, LabelRegistryError> (or Result<(), ...>
for define_label/name_label) and introduce a LabelRegistryError enum with
variants Unregistered(LabelId), AlreadyDefined(LabelId), and
NameConflict(String). Implement define_label to check self.labels: return
Err(Unregistered) if the id isn't registered, Err(AlreadyDefined) if it's
already an Offset, otherwise set LabelInfo::Offset and return Ok. Change
define_named_label to refuse to overwrite an existing Offset or rebind an
existing name: if name exists map to id and that id is AlreadyDefined return
Err(AlreadyDefined); if name absent but id exists check and insert only if not
already defined, else return Err(Unregistered) or NameConflict accordingly;
return Ok(id) on success. Change name_label to return Result and only insert
name if id is registered and the name isn't already bound (return NameConflict
if it is), otherwise return Unregistered; update all Assembler call sites to
propagate or handle these Results.


#[inline]
pub fn label_info(&self, id: LabelId) -> Option<&LabelInfo> {
self.labels.get(&id)
}

fn next_label(&mut self) -> LabelId {
let id = LabelId(self.next_id);
self.next_id += 1;
id
}
}
2 changes: 2 additions & 0 deletions harm-runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,5 @@
*/

pub mod labels;
pub mod memory;
pub mod runtime;
72 changes: 72 additions & 0 deletions harm-runtime/src/memory.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/* Copyright (C) 2026 Ivan Boldyrev
*
* This document is licensed under the BSD 3-clause license.
*/

#[cfg(feature = "memmap2")]
mod memmap2;

#[cfg(feature = "memmap2")]
pub use self::memmap2::{Mmap2Buffer, Mmap2FixedMemory};

pub trait Memory<FM: FixedMemory> {
type ExtendError;
type FixedMemoryError;

/// Current writing position.
fn pos(&self) -> usize;

/// If memory has fixed capacity, return it.
///
/// A `Vec` is not considered a memory of fixed capacity because it can grow indefinitely.
fn capacity(&self) -> Option<usize>;

/// Append data to the memory. Should fail when it reaches memory's capacity.
fn try_extend<I: Iterator<Item = u8>>(&mut self, bytes: I) -> Result<(), Self::ExtendError>;

/// Transform into fixed-location memory.
fn into_fixed_memory(self) -> Result<FM, Self::FixedMemoryError>;

/// Align position.
fn align(&mut self, alignment: usize) -> Result<(), Self::ExtendError> {
let pos = self.pos();
let remn = pos % alignment;
if remn != 0 {
self.try_extend(core::iter::repeat(0).take(alignment - remn))?;
}
Ok(())
Comment on lines +30 to +37
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Guard against alignment == 0 to avoid panic.

pos % alignment will panic when alignment is 0. Consider treating alignment ≤ 1 as a no-op (or returning an error) to keep the API safe.

Suggested fix
     fn align(&mut self, alignment: usize) -> Result<(), Self::ExtendError> {
-        let pos = self.pos();
-        let remn = pos % alignment;
+        if alignment <= 1 {
+            return Ok(());
+        }
+        let pos = self.pos();
+        let remn = pos % alignment;
         if remn != 0 {
             self.try_extend(core::iter::repeat(0).take(alignment - remn))?;
         }
         Ok(())
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/// Align position.
fn align(&mut self, alignment: usize) -> Result<(), Self::ExtendError> {
let pos = self.pos();
let remn = pos % alignment;
if remn != 0 {
self.try_extend(core::iter::repeat(0).take(alignment - remn))?;
}
Ok(())
/// Align position.
fn align(&mut self, alignment: usize) -> Result<(), Self::ExtendError> {
if alignment <= 1 {
return Ok(());
}
let pos = self.pos();
let remn = pos % alignment;
if remn != 0 {
self.try_extend(core::iter::repeat(0).take(alignment - remn))?;
}
Ok(())
}
🤖 Prompt for AI Agents
In `@harm-runtime/src/memory.rs` around lines 30 - 37, The align method can panic
when alignment == 0 due to pos % alignment; modify align (the fn align(&mut
self, alignment: usize) -> Result<(), Self::ExtendError>) to explicitly guard
against alignment == 0 (and optionally treat alignment <= 1 as a no-op) before
computing pos % alignment; if alignment <= 1 simply return Ok(()) (or return an
appropriate error) and otherwise proceed to compute remn from self.pos() and
call self.try_extend(... ) as before to perform padding.

}
}

/// Memory with fixed location that can be transformed to an executable one after relocations are applied.
pub trait FixedMemory: AsMut<[u8]> {
type ExecutableMemory;
type ExecutableMemoryError;

fn into_executable_memory(self) -> Result<Self::ExecutableMemory, Self::ExecutableMemoryError>;
}

#[cfg(test)]
mod tests {
#[cfg(feature = "memmap2")]
#[test]
fn test_align() {
use super::*;

let mut data = Vec::<u8>::new();

Memory::align(&mut data, 8);
assert!(data.is_empty());

data.push(1);
Memory::align(&mut data, 8);
assert_eq!(data.len(), 8);

Memory::align(&mut data, 8);
assert_eq!(data.len(), 8);

data.extend_from_slice(&[1, 2, 3, 4, 5, 6, 7]);
Memory::align(&mut data, 8);
assert_eq!(data.len(), 16);
}
}
Loading