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

TLSF and the Temple of Corrupted Memory #3653

Draft
wants to merge 16 commits into
base: dev
Choose a base branch
from

Conversation

DrZlo13
Copy link
Member

@DrZlo13 DrZlo13 commented May 16, 2024

What's new

  • [ Describe changes here ]

Verification

  • [ Describe how to verify changes ]

Checklist (For Reviewer)

  • PR has description of feature/bug or link to Confluence/Jira task
  • Description contains actions to verify feature/bugfix
  • I've built this code, uploaded it to the device and verified feature/bugfix

DrZlo13 and others added 2 commits May 16, 2024 19:52
* add tlsf as submodule
* libs: tlsf
* Furi: tlsf as allocator
* Furi: heap walker
* shmal fixshesh
* f18: tlsf
* PVS: ignore tlsf
* I like to moving
* merge upcoming changes
* memmgr: alloc aligned, realloc
* Furi: distinct name for auxiliary memory pool
* Furi: put idle and timer thread to mem2
* Furi: fix smal things in allocator
* Furi: remove aligned_free. Use free instead.
* aligned_malloc -> aligned_alloc
* aligned_alloc, parameters order
* aligned_alloc: check that alignment is correct
* unit test: malloc
* unit tests: realloc and test with memory fragmentation
* unit tests: aligned_alloc
* update api
* updater: properly read large update file

Co-authored-by: Aleksandr Kutuzov <[email protected]>
Copy link

github-actions bot commented May 16, 2024

Compiled f7 firmware for commit 9b15e05e:

@DrZlo13 DrZlo13 marked this pull request as draft May 17, 2024 09:37
@hedger hedger added Sub-GHz Sub-GHz-related Bug Core+Services HAL, furi & core system services labels May 17, 2024
@CookiePLMonster
Copy link
Contributor

CookiePLMonster commented Jun 3, 2024

Regarding the 8-byte alignment, GCC indeed appears to make that assumption, with or without -Os.

This code:

int32_t align_test_app(void* p) {
    UNUSED(p);
    FURI_LOG_I("TEST", "Hello world");
    FURI_LOG_I("TEST", "I'm align_test!");

    void* b = malloc(4);

    uintptr_t valB = (uintptr_t)b;
    return (valB & 7) == 0;
}

compiled into:
image

So it's a very good idea to guarantee proper alignment, at least for malloc and realloc.

Copy link

PVS-Studio report for commit 7d50f4b5:

@DrZlo13
Copy link
Member Author

DrZlo13 commented Jun 11, 2024

I disabled all dolphin animations and ran some tests:

tlsf

: free
Free heap size: 141416
Total heap size: 185560
Minimum heap size: 137696
Maximum heap block: 140328
Aux pool total free: 2752
Aux pool max free block: 2644

: free
Free heap size: 141304
Total heap size: 185560
Minimum heap size: 125768
Maximum heap block: 132512
Aux pool total free: 2752
Aux pool max free block: 2644

heap4

: free
Free heap size: 146200
Total heap size: 185544
Minimum heap size: 142512
Maximum heap block: 145136
Pool free: 1464
Maximum pool block: 1348

: free
Free heap size: 146136
Total heap size: 185544
Minimum heap size: 142440
Maximum heap block: 145296
Pool free: 1464
Maximum pool block: 1348

It looks like in the current version tlsf has no meaning except for the correct implementation of realloc.

@CookiePLMonster
Copy link
Contributor

Suggestion - if this goes through, consider exposing tlsf_memalign_offs as a non-standard aligned_offset_alloc(size_t alignment, size_t size, size_t offset) function, much like MSVC does:
https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/aligned-offset-malloc

It may be handy for cases where a nested object or class field requires alignment, but the rest of the structure does not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Core+Services HAL, furi & core system services Sub-GHz Sub-GHz-related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants