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.

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

ztfm.c fails to meet its requirement to test transforms on all platforms #243

Open
thejayps opened this issue Jun 14, 2023 · 2 comments
Open
Labels
essential Will cause failure to meet customer requirements. Assign resources.

Comments

@thejayps
Copy link
Contributor

ztfm.c includes a test whose intention is to verify that a transform runs to completion in the absence of ambiguous references to objects in the transform. The test setup isn't sufficient to create the desired test environment and at least one compiler under specific conditions appears to store/"spill" ambiguous references onto the stack, invalidating the test. The transform fails to complete, creating a false positive test result. This was observed here (gcc 11.3.0 )

This issue could be fixed by redesigning the testing of transforms (see #242 )

@rptb1
Copy link
Member

rptb1 commented Jun 14, 2023

Temporarily worked around this issue prior to version 1.118 by suppressing the test except on Windows in 9f71e31 as part of #214 .

@rptb1
Copy link
Member

rptb1 commented Jun 14, 2023

Currently, the test declares the stack as an ambiguous root, to ensure that the C code is GC-safe. However, there are other ways of achieving this. (The amcss test does not declare the stack, for example.) In this case, it's not the duty of this test to check the interaction of the incremental GC with the C code. I suggest not declaring the stack, and arranging that all graph-manipulating operations are done while the arena is parked. (Transforms require the arena to be parked anyway.) The test can release the arena to run manual GCs after manipulating the graph, in order to check that Transforms work when GCs have been run.

We could check that this works well by restructuring the existing test code before redesigning it in response to #242 .

@thejayps thejayps added the essential Will cause failure to meet customer requirements. Assign resources. label Jun 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
essential Will cause failure to meet customer requirements. Assign resources.
Projects
None yet
Development

No branches or pull requests

2 participants