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

test_nvs_host: fix asan reported bugs (IDFGH-9860) #11117

Merged
merged 1 commit into from
Apr 25, 2023

Conversation

codysch
Copy link
Contributor

@codysch codysch commented Apr 3, 2023

  • Use of sizeof(uint32_t *) instead of sizeof(uint32_t) in arguments
  • Use of memcmp without checking that length agreed (add memeq)

These were observed when running these tests built with clang 14 on a MacOS 12.6.3 system (Xcode 13.1). I suspect some of them will appear on any system where uint32_t * and uint32_t are different sizes, and where reading some data should cause the test to fail (due to lack of certain python tooling)

 - Use of `sizeof(uint32_t *)` instead of `sizeof(uint32_t)` in
   arguments
 - Use of `memcmp` without checking that length agreed (add `memeq`)
@CLAassistant
Copy link

CLAassistant commented Apr 3, 2023

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@igrr igrr added the PR-Sync-Merge Pull request sync as merge commit label Apr 12, 2023
@igrr
Copy link
Member

igrr commented Apr 12, 2023

sha=630343e4b9a706b25cf06c8c407463c774ad6db7

@espressif-bot espressif-bot added the Status: Opened Issue is new label Apr 12, 2023
@github-actions github-actions bot changed the title test_nvs_host: fix asan reported bugs test_nvs_host: fix asan reported bugs (IDFGH-9860) Apr 12, 2023
@espressif-bot espressif-bot added Status: In Progress Work is in progress and removed Status: Opened Issue is new labels Apr 12, 2023
@igrr
Copy link
Member

igrr commented Apr 13, 2023

@codysch I've tried to run our test pipeline but it failed on memeq comparisons, here is the log: https://gist.github.com/igrr/abff87b6b6f920203badc1f3f2543bde
I haven't had time to dig deeper into this yet; if you have an idea why this happens, please let me know!

Edit: I have now fixed the failing tests, will merge the PR soon.

@espressif-bot espressif-bot added Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally Resolution: Done Issue is done internally and removed Status: In Progress Work is in progress Resolution: NA Issue resolution is unavailable labels Apr 20, 2023
@espressif-bot espressif-bot merged commit 630343e into espressif:master Apr 25, 2023
@codysch codysch deleted the test-nvs-host-asan branch October 9, 2023 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR-Sync-Merge Pull request sync as merge commit Resolution: Done Issue is done internally Status: Done Issue is done internally
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants