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

tests: lib: mem_alloc: migrating mem_alloc tests to new ztest API #50164

Closed
wants to merge 1 commit into from

Conversation

yerabolu
Copy link
Contributor

Migrating mem_alloc tests to new ztest API.

Part of #47002 effort

Signed-off-by: Spoorthy Priya Yerabolu spoorthy.priya.yerabolu@intel.com

@@ -1,3 +1,5 @@
CONFIG_ZTEST=y
CONFIG_ZTEST_NEW_API=y
CONFIG_NO_OPTIMIZATIONS=y
Copy link
Member

Choose a reason for hiding this comment

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

There is a big difference between disabling optimisation globally vs. only for some functions of interest.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, was trying a few things while testing and forgot to remove them.

Migrating mem_alloc tests to new ztest API.

Part of zephyrproject-rtos#47002 effort

Signed-off-by: Spoorthy Priya Yerabolu <spoorthy.priya.yerabolu@intel.com>
@enjiamai
Copy link
Collaborator

Hi, @yerabolu , all looks good for me and I verified them in some boards such as k64f, nrf52840, nsim, etc. It works well.

I just have some concern on removing the __no_optimization, I checked why it had added, then saw in this previous commit: seems like it will affect boards which using the ARC MWDT toolchain.

commit fa41ec052d26e2497fc29e28dc0202317d715f30
Author: Watson Zeng <zhiwei@synopsys.com>
Date:   Mon May 24 16:35:36 2021 +0800

    tests: mem_alloc: workaround aggressive optimization

    As we don't use memory allocated in test_calloc, test_no_mem_malloc,
    and test_no_mem_realloc. malloc call can be optimized away (that really
    happens with ARC MWDT toolchain). That breaks the test. So disable
    optimization for these functions.

    Signed-off-by: Watson Zeng <zhiwei@synopsys.com>

I did not have a arcmwdt toolchain to verify it. Hi @IRISZZW , could you please help to check if this still causes problem now? (if we remove the __no_optimization)

@stephanosio
Copy link
Member

could set the vars volatile.

@yerabolu
Copy link
Contributor Author

Hi, @yerabolu , all looks good for me and I verified them in some boards such as k64f, nrf52840, nsim, etc. It works well.

I just have some concern on removing the __no_optimization, I checked why it had added, then saw in this previous commit: seems like it will affect boards which using the ARC MWDT toolchain.

commit fa41ec052d26e2497fc29e28dc0202317d715f30
Author: Watson Zeng <zhiwei@synopsys.com>
Date:   Mon May 24 16:35:36 2021 +0800

    tests: mem_alloc: workaround aggressive optimization

    As we don't use memory allocated in test_calloc, test_no_mem_malloc,
    and test_no_mem_realloc. malloc call can be optimized away (that really
    happens with ARC MWDT toolchain). That breaks the test. So disable
    optimization for these functions.

    Signed-off-by: Watson Zeng <zhiwei@synopsys.com>

I did not have a arcmwdt toolchain to verify it. Hi @IRISZZW , could you please help to check if this still causes problem now? (if we remove the __no_optimization)

Thank You Enjia for verifying it and looking into it. Yes, i am still looking into the __no_optimization attribute.

@enjiamai
Copy link
Collaborator

Hi @yerabolu , any update on this PR? Thanks.

Comment on lines +48 to +69
#if defined(CONFIG_MINIMAL_LIBC) && CONFIG_MINIMAL_LIBC_MALLOC_ARENA_SIZE == 0
ZTEST_USER(lib_dynamic_memalloc, test_no_mem_malloc)
{
int *iptr = NULL;

iptr = malloc(BUF_LEN);
zassert_is_null((iptr), "malloc failed, errno: %d", errno);
free(iptr);
iptr = NULL;
}

ZTEST_USER(lib_dynamic_memalloc, test_no_mem_realloc)
{
char *ptr = NULL;
char *reloc_ptr = NULL;

reloc_ptr = realloc(ptr, BUF_LEN);
zassert_is_null(reloc_ptr, "realloc failed, errno: %d", errno);
free(reloc_ptr);
reloc_ptr = NULL;
}
#else
Copy link
Collaborator

Choose a reason for hiding this comment

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

NAK.

Please keep __no_optimization function attribute. The potential issue is here and it won't go away as it's compiler valid behavior here to optimize out malloc / realloc in these cases.

Moreover it's not only related to MWDT - potentially it can be done by any toolcahin.

@github-actions
Copy link

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions bot added the Stale label Dec 19, 2022
@github-actions github-actions bot closed this Jan 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants