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

Fixing deep checking and shield checking #209

Draft
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

rptb1
Copy link
Member

@rptb1 rptb1 commented Mar 30, 2023

Fixing false positive assertions found by deep checking so that deep checking is repaired as a useful too. See #208 .

Adding deep checking to CI so that we will find out promptly if it is broken again. See #208 .

Changing shield functions to take a shield, rather than an arena, so that the shield is naturally checked often. Fixes #205 .

Fixing #206 as described in the issue.

@rptb1
Copy link
Member Author

rptb1 commented Mar 30, 2023

@rptb1
Copy link
Member Author

rptb1 commented Mar 30, 2023

#208 says:

3. Deep checking has an infinite recursion in ArenaCheck via ShieldCheck, which checks some segments, which check their addresses via ChunkOfAddr, which checks the arena with ArenaCheck. I haven't discovered when this was introduced yet.

Fixed in 54b9351

@rptb1
Copy link
Member Author

rptb1 commented Mar 30, 2023

Changelist 191812 does not appear to fix job004026, as this assertion fires in many tests

mps/code/shield.c

Lines 88 to 90 in 179341b

/* Every unsynced segment should be on the queue, because we have to
remember to sync it before we return to the mutator. */
CHECKL(shield->limit + shield->queuePending >= shield->unsynced);

The comment on that assertion is false. I think it used to be true, but various tweaks to the shield have rendered it ineffective.

Fixed in 0d56581 by keeping counts of the number of synced segments in the queue, and the number of unsynced segments not in the queue, allowing exact checking of the queue length.

@rptb1 rptb1 self-assigned this Mar 30, 2023
@rptb1 rptb1 linked an issue Mar 31, 2023 that may be closed by this pull request
@rptb1 rptb1 added the build Problems with builds and build automation label Mar 31, 2023
… the shield gets checked. Also makes the shield code smaller and more natural. Fixes GitHub issue #205 <#205>.
…ld queue, so that shieldDebugCheck can find and check it. Fixes GitHub issue #206 <#206>.
@rptb1 rptb1 changed the title Fixing deep checking Fixing deep checking and shield checking Mar 31, 2023
@rptb1 rptb1 requested review from UNAA008 and thejayps March 31, 2023 07:08
@thejayps thejayps added the essential Will cause failure to meet customer requirements. Assign resources. label Apr 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Problems with builds and build automation essential Will cause failure to meet customer requirements. Assign resources.
Projects
None yet
2 participants