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

Fix for #2204 #7983

Merged
merged 1 commit into from
May 16, 2023
Merged

Fix for #2204 #7983

merged 1 commit into from
May 16, 2023

Conversation

furbrain
Copy link

@furbrain furbrain commented May 15, 2023

This PR provides a fix for issue #2204. It fixes a moderate bug which results in hard segfaults when you repeatedly initialise a display. The original context for this is a display which is powered down prior to the processor going in to light sleep

@tannewt
Copy link
Member

tannewt commented May 15, 2023

Please explain what the bug is. It isn't immediately obvious to me.

@furbrain
Copy link
Author

I will use line numbers from the original source code
gc_never_free will crash the fourth time it is called.

  • First time it is called the permanent_pointers item of mp_state is set to null.
    The while block on line 1017 is skipped and next_block is allocated - a set of four pointers
    The if statement on line 1029 evaluates as true and line 1030 executes, correctly storing the address of next_block in permanent_pointers
    the pointer passed into the function is stored in index [1] of current_reference_block
  • second and third time around it loads the previously allocated block from permanent_pointers into current_reference_block correctly just places the pointer into current_reference_block[2] and [3]
  • Fourth time around current_reference_block looks like this:
    [0] = NULL
    [1] = ptr1 (first pointer added)
    [2] = ptr2
    [3] = ptr 3
    As there is nowhere to store the new pointer current_reference_block is set to current_reference_block[0] - which is NULL - indicating that we need to create a new block
    However, on line 1032 we set current_reference_block[0] to the new block.
    But at this point current_reference_block is NULL
    So line 1032 tries to set the memory at 0x00000000 to the address of next_block - which segfaults

@furbrain
Copy link
Author

My fix keeps a record of the previous current_record_block so we can assign the next block into it correctly.

@akoebbe
Copy link

akoebbe commented May 16, 2023

@furbrain Thanks for this change. Just wanted to confirm here that this change resolves #7974 . The only possible side effect is that auto-reload crashes, though pressing reset does bring it back. Again, I can't confirm that this fix has anything to do with that, but just something to keep an eye out for.

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix!

@tannewt
Copy link
Member

tannewt commented May 16, 2023

(and the thorough explanation.)

@tannewt tannewt merged commit ae699ba into adafruit:main May 16, 2023
@dhalbert
Copy link
Collaborator

dhalbert commented May 16, 2023

@furbrain I am closing all the issues you think are related. Let us know if you think not.

@dhalbert
Copy link
Collaborator

@furbrain - I marked them as "needs re-test" instead.

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.

4 participants