Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

newCopyArgs copies too many args #1087

Open
jamshark70 opened this issue Apr 20, 2014 · 3 comments · May be fixed by #6302
Open

newCopyArgs copies too many args #1087

jamshark70 opened this issue Apr 20, 2014 · 3 comments · May be fixed by #6302
Assignees
@JordanHendersonMusic
Labels
bug Issues that relate to unexpected/unwanted behavior. Don't use for PRs. comp: sclang sclang C++ implementation (primitives, etc.). for changes to class lib use "comp: class library"

Comments

@jamshark70
Copy link
Contributor

newCopyArgs seems to have the equivalent of a buffer overrun bug.

TestCopyArgs {
    var <>a, <>b, <>c;

    *new { |a, b| ^super.newCopyArgs(a, b) }
}

Then run:

TestCopyArgs(1, 2, 3).dump

It prints:

Instance of TestCopyArgs {    (0x74ae208, gc=10, fmt=00, flg=00, set=02)
  instance variables [3]
    a : Integer 1
    b : Integer 2
    c : Integer 3
}

The argument 3 has been copied into c -- but the *new method accepts only two arguments, and passes only two arguments to newCopyArgs. The third argument should be discarded.

This happens if newCopyArgs is the last method call. If there is another method call after it, then it behaves as expected -- if *new does { |a, b| ^super.newCopyArgs(a, b).dump }, then it handles only two arguments.
@telephon
Copy link
Member

telephon commented May 4, 2014

The implementation of basicNewCopyArgsToInstanceVars seems to be correct:

int length = sc_min(numArgsPushed-1, newobj->size);
    for (int i=0; i<length; ++i) {
        slotCopy(&newobj->slots[i], &b[i]);
    }

But the reported numArgsPushed is wrong for exactly two arguments.
This works correctly, and has numArgsPushed == 1:

TestCopyArgs {
    var a, b, c;

    *new { |a, b| ^super.newCopyArgs(a) }
}

But this one:

TestCopyArgs {
    var a, b, c;

    *new { |a, b| ^super.newCopyArgs(a, b) }
}

This has numArgsPushed == 3 instead of 2.

@telephon
Copy link
Member

telephon commented May 4, 2014

Somehow I have the suspicion that it has to do with int checkPushAllArgs(PyrParseNode *actualArg, int numArgs) or similar. There is a check for numArgs < 3. But it is strange as we should have noticed something earlier. I leave this note here in case someone else tries to find the source of the bug …

@mossheim mossheim self-assigned this Jan 18, 2017
@mossheim mossheim removed their assignment Apr 17, 2021
@JordanHendersonMusic JordanHendersonMusic self-assigned this May 9, 2024
JordanHendersonMusic pushed a commit to JordanHendersonMusic/supercollider that referenced this issue May 9, 2024
SCLang: Too many args passed to certain 'inline' methods
59f262b 
When the compile/parser see a method with only a single call, or a call to `super.something`, it 'inlines' it.

For these methods, all the users arguments were being passed through, regardless of how many were declared in the function signature.

This commit checks this, and removes any excess args from the stack.

Closes supercollider#1087
@JordanHendersonMusic JordanHendersonMusic linked a pull request May 9, 2024 that will close this issue
Open
4 tasks
JordanHendersonMusic pushed a commit to JordanHendersonMusic/supercollider that referenced this issue May 9, 2024
SCLang: Too many args passed to certain 'inline' methods
ffe7f32 
When the compile/parser see a method with only a single call, or a call to `super.something`, it 'inlines' it.

For these methods, all the users arguments were being passed through, regardless of how many were declared in the function signature.

This commit checks this, and removes any excess args from the stack.

Closes supercollider#1087
@JordanHendersonMusic
Copy link
Contributor 

TestCopyArgs {
    var a, b, c;
    *new { |a| ^super.newCopyArgs(a) }
}

TestCopyArgs(\a, \b, \c).dump

This was being caused by not checking how many args are present for certain 'inlined' methods. I've proposed a PR to fix this.

JordanHendersonMusic pushed a commit to JordanHendersonMusic/supercollider that referenced this issue May 9, 2024
SCLang: Too many args passed to certain 'inline' methods
a89bea0 
When the compile/parser see a method with only a single call, or a call to `super.something`, it 'inlines' it.

For these methods, all the users arguments were being passed through, regardless of how many were declared in the function signature.

This commit checks this, and removes any excess args from the stack.

Closes supercollider#1087
JordanHendersonMusic pushed a commit to JordanHendersonMusic/supercollider that referenced this issue May 10, 2024
SCLang: Too many args passed to certain 'inline' methods
067af08 
When the compile/parser see a method with only a single call, or a call to `super.something`, it 'inlines' it.

For these methods, all the users arguments were being passed through, regardless of how many were declared in the function signature.

This commit checks this, and removes any excess args from the stack.

Closes supercollider#1087
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@JordanHendersonMusic JordanHendersonMusic

Labels
bug Issues that relate to unexpected/unwanted behavior. Don't use for PRs. comp: sclang sclang C++ implementation (primitives, etc.). for changes to class lib use "comp: class library"
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

SCLang: Too many args passed to certain 'inlined' methods JordanHendersonMusic/supercollider
4 participants
@jamshark70 @telephon @mossheim @JordanHendersonMusic