Fix vaarg_006/007/008/009 tests: prototype doesn't match actual parameters#167
Open
jorgsowa wants to merge 2 commits into
Open
Fix vaarg_006/007/008/009 tests: prototype doesn't match actual parameters#167jorgsowa wants to merge 2 commits into
jorgsowa wants to merge 2 commits into
Conversation
…eters Each sum() function declared only 1 named int32_t parameter in its signature but its body read 3 (or 9) fixed arguments directly via PARAM before ever calling va_arg. Under SysV this drift is harmless since register/stack placement doesn't depend on the named/variadic boundary, so it went unnoticed for two years. Apple's aarch64 ABI does care: it only register-passes the parameters actually declared in the prototype, forcing everything else onto the stack. Since the prototype under-declared its named parameters, va_start ended up pointing at the wrong stack offset, corrupting every va_arg read on the new macos-15-arm64 CI runner. Correcting each prototype to declare all the parameters the body actually reads (matching what the compiler's own --save re-serialization already believed the real signature to be) resolves the mismatch with no compiler changes.
…type Nothing previously verified that a function's declared prototype (the count of named parameters before "...") actually matched the number of PARAM instructions in its body. That drift is exactly what caused the vaarg_006/007/008/009 test failures: harmless under SysV, but silently wrong on Apple's aarch64 ABI, which places named vs. variadic arguments differently. Reject the mismatch immediately at load time, on any platform, before any codegen happens, instead of relying on a specific ABI to happen to expose it at runtime.
dstogov
reviewed
Jul 13, 2026
dstogov
left a comment
dstogov
left a comment
Owner
There was a problem hiding this comment.
I'm not sure if this is right or not. I'll need to think about this.
dstogov
requested changes
Jul 20, 2026
dstogov
left a comment
dstogov
left a comment
Owner
There was a problem hiding this comment.
I suppose this is right.
Please, answer the question and make the minor change.
| int32_t c_9 = 5; | ||
| int32_t c_10 = 6; | ||
| uintptr_t c_11 = func sum(int32_t, ...): int32_t; | ||
| uintptr_t c_11 = func sum(int32_t, int32_t, int32_t, ...): int32_t; |
Owner
There was a problem hiding this comment.
Could you plase show the difference in generated assembler code. Just in comments.
Owner
There was a problem hiding this comment.
Ignore this. I already understood the difference.
| ir_free(&ctx); | ||
| yy_error_str("parameter count doesn't match function signature", name); | ||
| } | ||
| } |
Owner
There was a problem hiding this comment.
Please move this check into ir_check() (or make a copy).
This way we will able to check this, with big test suites through LLVM and RCC front-ends.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix 4 failing tests for IR for MACOS aarch64.
I'm not sure whether only tests should be fixed, the check should be added (current PR), or the check should be more general.
The exact failures:
https://github.com/dstogov/ir/actions/runs/29170932167/job/86753268624?pr=135#step:5:2317