Skip to content

Conversation

veselypeta
Copy link
Collaborator

@veselypeta veselypeta commented Jul 4, 2025

Should fix #8 & #9 & #16

Includes change from #13

@veselypeta veselypeta force-pushed the petr/fix-bounded-var-mem-args branch from d67af1d to ee9fc15 Compare July 4, 2025 13:08
@veselypeta veselypeta force-pushed the petr/fix-bounded-var-mem-args branch from ee9fc15 to dc4404c Compare July 7, 2025 15:20
@dpgao
Copy link

dpgao commented Jul 7, 2025

This doesn't fix the first example in #8. The variadic argument of type Triple should begin at offset 16 of the memory argument buffer. But right now it begins at offset 8.

@dpgao
Copy link

dpgao commented Jul 7, 2025

And this doesn't fix the second example in #8. The call to f in g sets the memory argument buffer to NULL under -Xclang -target-feature -Xclang +cheri-bounded-vararg -Xclang -target-feature -Xclang +cheri-bounded-memarg-caller -Xclang -target-feature -Xclang +cheri-bounded-memarg-callee.

__uint128_t f(void* a, __uint128_t b, __uint128_t c, __uint128_t d, ...);

__uint128_t g() {
    return f((void*)0x1, 0, 0, 123, (__uint128_t)456, (__uint128_t)789);
}

@veselypeta
Copy link
Collaborator Author

Thanks @dpgao. It's proved quite tricky to get this right for RISCV. The issue I've come across is that Triple type is converted to [2 x i64] is llvm. In RISCV these are treated as separate arguments, but I need to align to up a slot boundary. I've made it work now, by picky backing on functionArgumentNeedsConsecutiveRegisters for purecap varargs. I think the layout is correct, now but I'm writing some tests for it now to make sure. Let me know if you still have any issues.
Thanks 🙏

@veselypeta veselypeta force-pushed the petr/fix-bounded-var-mem-args branch from f50166d to d7c5b37 Compare July 8, 2025 15:23
@veselypeta veselypeta force-pushed the petr/fix-bounded-var-mem-args branch from d7c5b37 to 2e12ff6 Compare July 8, 2025 15:44
@dpgao
Copy link

dpgao commented Jul 19, 2025

Thanks! I think once #14 is addressed, the compiler will be able to build CheriBSD with the memory argument ABI successfully.

@dpgao
Copy link

dpgao commented Jul 28, 2025

All LGTM now.

I've only been able to test this PR after rebasing it onto veselypeta/cherillvm@a7d84f5, so do you intend to merge this right now or wait until https://github.com/veselypeta/cherillvm/commits/codasip-rebased/ is first incorporated to the main branch here?

@veselypeta
Copy link
Collaborator Author

wait until https://github.com/veselypeta/cherillvm/commits/codasip-rebased/ is first incorporated to the main branch here?

Yes I'm working on integrating the last changes from both codasip and cambridge into this branch and make this the default branch for cheri alliance before merging this. Is this is a blocker for you, let me know, but it it seems that your rebase is working for your purpose. Hopefully, this will be the default branch by the end of the week. 🤞

@dpgao
Copy link

dpgao commented Jul 30, 2025

wait until https://github.com/veselypeta/cherillvm/commits/codasip-rebased/ is first incorporated to the main branch here?

Yes I'm working on integrating the last changes from both codasip and cambridge into this branch and make this the default branch for cheri alliance before merging this. Is this is a blocker for you, let me know, but it it seems that your rebase is working for your purpose. Hopefully, this will be the default branch by the end of the week. 🤞

Thank you! This is not a blocker for me at the moment but obviously it would be better if it gets merged.

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