Skip to content

stack frame header struct#12

Open
michcat wants to merge 2 commits into
masterfrom
michcat/stackframe-struct
Open

stack frame header struct#12
michcat wants to merge 2 commits into
masterfrom
michcat/stackframe-struct

Conversation

@michcat
Copy link
Copy Markdown

@michcat michcat commented May 29, 2026

StackFrameHeader struct to make pushing onto stack a bit nicer and to avoid the raw indexing

ran these to test

cargo test
cargo brimstone-test -- -s integration
cargo brimstone-test -- -s test262 --ignore-unimplemented

Comment thread src/js/runtime/bytecode/stack_frame.rs Outdated
let num_args_with_receiver = num_parameters.max(header.argc) + 1;

let receiver_ptr = self.fp.add(RECEIVER_SLOT_INDEX) as *mut Value;
let receiver_ptr = self.fp.add(StackFrameHeader::RECEIVER_SLOT) as *mut Value;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

For this, is it not possible to simply read the receiver?

Copy link
Copy Markdown
Author

@michcat michcat May 29, 2026

Choose a reason for hiding this comment

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

this was reading both the receiver and the args below it (for gc), which is why we couldn't just read receiver -- but that is awkward I agree, so I changed now to just have this function return args and then we visit the receiver separately

this commit:
visit reciever separately for gc

(this function still calls from_raw_parts_mut to get the args and is scary -- I think we can potentially do something about that too...)

#[inline]
fn push_value<T>(&mut self, value: T) {
// inside const block evaluates at compile-time instead of at runtime
const {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice!

// Patch the address of the return value in the stack frame
let mut return_value = Value::undefined();
stack_frame.set_return_value_address((&mut return_value) as *mut Value);
stack_frame.header_mut().return_value_address = (&mut return_value) as *mut Value;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Similar to comment above, can we do this unsafety in one function? e.g. rather than header_mut you have set_return_value_addr which takes &mut return_value and does the unsafe stuff inside.

/// not read or write directly — use [`StackFrame::return_address`],
/// [`StackFrame::set_return_address`], [`StackFrame::is_rust_caller`].
return_address: usize,
pub return_value_address: *mut Value,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

can we encapsulate all of these and just have getters and setters? This stuff is super unsafe so I think it is actually worth having methods for getting / setting these.

@@ -205,24 +203,24 @@ impl<W: Width> Register<W> {
/// Construct a register referencing the current `this` value.
#[inline]
pub fn this() -> Self {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Can you explain exactly what's going on here? I'm a bit by confused. This holds the offset of the receiver field relative to the base address of the StackFrameHeader's base addr?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants