Skip to content

Conversation

alexrp
Copy link
Member

@alexrp alexrp commented Apr 16, 2025

Let's see what CI has to say.

@alexrp alexrp marked this pull request as draft April 16, 2025 18:36
@alexrp alexrp force-pushed the no-memory-accessor branch from f1e9ac9 to ede2083 Compare April 16, 2025 19:36
@alexrp
Copy link
Member Author

alexrp commented Apr 20, 2025

Will revisit this if I figure out a way to repro the aarch64-linux failures locally...

@alexrp alexrp closed this Apr 20, 2025
@alexrp alexrp deleted the no-memory-accessor branch May 29, 2025 17:34
@andrewrk
Copy link
Member

what was the failure and why delete the branch?

Comment on lines -1441 to -1446
const num_restored_pairs: usize =
@popCount(@as(u5, @bitCast(encoding.value.arm64.frame.x_reg_pairs))) +
@popCount(@as(u4, @bitCast(encoding.value.arm64.frame.d_reg_pairs)));
const min_reg_addr = fp - num_restored_pairs * 2 * @sizeOf(usize);

if (ma.load(usize, new_sp) == null or ma.load(usize, min_reg_addr) == null) return error.InvalidUnwindInfo;
Copy link
Member

@andrewrk andrewrk Aug 23, 2025

Choose a reason for hiding this comment

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

what does this change have to do with MemoryAccessor?

edit: looks like they were temporaries only needed for the ma.load expression

Copy link
Member Author

Choose a reason for hiding this comment

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

MemoryAccessor.load() only returns null if the pointer is invalid, i.e. a regular load would have segfaulted. This is a defensive check that can only exist because of MemoryAccessor.

@alexrp
Copy link
Member Author

alexrp commented Aug 23, 2025

what was the failure

I think it was some unwind issue that manifested as a segfault on native aarch64-linux, and I don't have a machine available to debug that.

Of course GitHub helpfully deleted the logs: https://github.com/ziglang/zig/actions/runs/14501073544

and why delete the branch?

I wasn't expecting to get around to this anytime soon due to the above. The changes are just the result of deleting MemoryAccessor and fixing the resulting compile errors. I can resurrect the branch if you want to take a stab at it?

@andrewrk
Copy link
Member

=> #24960

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