Skip to content

tests/lib/ringbuffer: Assertion failure at test_ring_buffer_main() #14869

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.

Sign up for GitHub

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

Jump to bottom
Closed
cinlyooi-intel opened this issue Mar 25, 2019 · 5 comments · Fixed by #15311
Closed

tests/lib/ringbuffer: Assertion failure at test_ring_buffer_main() #14869

cinlyooi-intel opened this issue Mar 25, 2019 · 5 comments · Fixed by #15311
Assignees
@andrewboie
Labels
bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug

Comments

@cinlyooi-intel
Copy link
Contributor

Describe the bug
(memcmp((char *)getdata, rb_data, getsize * sizeof(u32_t)) == 0

Seen on Altera Max10

To Reproduce
Steps to reproduce the behavior:

  1. mkdir build; cd build
  2. cmake -DBOARD=altera_max10
  3. make
  4. make run

Expected behavior
A clear and concise description of what you expected to happen.

Screenshots or console output

***** Booting Zephyr OS v1.14.0-rc1-1386-g5d35b6632765 (delayed boot 3000ms) *****
Running test suite test_ringbuffer_api
===================================================================
starting test - test_ringbuffer_init
PASS - test_ringbuffer_init
===================================================================
starting test - test_ringbuffer_declare_pow2
PASS - test_ringbuffer_declare_pow2
===================================================================
starting test - test_ringbuffer_declare_size
PASS - test_ringbuffer_declare_size
===================================================================
starting test - test_ringbuffer_put_get_thread
PASS - test_ringbuffer_put_get_thread
===================================================================
starting test - test_ringbuffer_put_get_isr
PASS - test_ringbuffer_put_get_isr
===================================================================
starting test - test_ringbuffer_put_get_thread_isr
PASS - test_ringbuffer_put_get_thread_isr
===================================================================
starting test - test_ringbuffer_pow2_put_get_thread_isr
PASS - test_ringbuffer_pow2_put_get_thread_isr
===================================================================
starting test - test_ringbuffer_size_put_get_thread_isr
PASS - test_ringbuffer_size_put_get_thread_isr
===================================================================
starting test - test_ring_buffer_main
    Assertion failed at /home/jenkins/workspace/zephyr-master-ci-master/zephyr.git/tests/lib/ringbuffer/src/main.c:95: test_ring_buffer_main: ((memcmp((char *)getdata, rb_data, getsize * sizeof(u32_t)) == 0) is false)**
data corrupted
FAIL - test_ring_buffer_main
===================================================================
starting test - test_ringbuffer_raw
PASS - test_ringbuffer_raw
===================================================================
starting test - test_ringbuffer_alloc_put
PASS - test_ringbuffer_alloc_put
===================================================================
starting test - test_byte_put_free
PASS - test_byte_put_free
===================================================================
Test suite test_ringbuffer_api failed.
===================================================================

Environment (please complete the following information):

  • OS: fedora29
  • Toolchain: zephyr
  • Commit SHA or Version used: 5d35b66

Additional context
Add any other context about the problem here.
@cinlyooi-intel cinlyooi-intel added the bug The issue is a bug, or the PR is fixing a bug label Mar 25, 2019
@rljordan-zz rljordan-zz added the priority: low Low impact/importance bug label Mar 26, 2019
@andrewboie
Copy link
Contributor

does not fail on qemu_nios2

@andrewboie
Copy link
Contributor

andrewboie commented Apr 9, 2019 •
edited
Loading

I've reproduced on Max10 HW, but this is still mysterious. I think this has something to do with unaligned memory access to the ring buffer storage area, still digging

@andrewboie
Copy link
Contributor

andrewboie commented Apr 9, 2019 •
edited
Loading

Seems that on the real Max10 board, trying to array reference an unaligned pointer implictly changes stuff. The rb_data[] array passed to ring_buf_item_put() is not 4-byte aligned.

In emulation, and presumably on other hardware, we see:

RB PUT TO ADDR 0x00400e47
data[i] is 44434241

But on the Max10 board, the unaligned pointer is treated differently:

RB PUT TO ADDR 0x00400e33
data[i] is 41000000

We expect data[i] to be 0x44434241, which is what "ABCD" would look like if cast to a u32_t. It seems that the CPU is instead doing something which causes the array beginning to be treated as 0x00400e30 instead.

I don't know if this is an undefined behavior problem with the test, or some bug on Max10.

andrewboie pushed a commit to andrewboie/zephyr that referenced this issue Apr 9, 2019
tests: ringbuffer: avoid unaligned mem access
90738aa 
Casting the rb_data character array to a u32_t can result
in an unaligned u32_t * pointer being passed to
ring_buf_item_put(), since rb_data is only byte-aligned.

Our Altera Max10 CPU build is not configured to detect
unaligned memory access and throw an exception, it is
instead rounding down the memory address
of the data pointer to the nearest 4-byte value, causing
the wrong data to be copied into the ring buffer.

It appears that in the C standard this is considered
Undefined Behavior so the approach this patch takes is
to fix the test, by ensuring that rb_data is aligned to
u32_t.

Fixes: zephyrproject-rtos#14869

Signed-off-by: Andrew Boie <[email protected]>
@andrewboie andrewboie mentioned this issue Apr 9, 2019
Merged
@andrewboie
Copy link
Contributor

https://stackoverflow.com/questions/51126257/what-does-the-standard-say-about-unaligned-memory-access

I'm gonna go with this being undefined behavior.

nashif pushed a commit that referenced this issue Apr 10, 2019
@nashif
tests: ringbuffer: avoid unaligned mem access
9926f94 
Casting the rb_data character array to a u32_t can result
in an unaligned u32_t * pointer being passed to
ring_buf_item_put(), since rb_data is only byte-aligned.

Our Altera Max10 CPU build is not configured to detect
unaligned memory access and throw an exception, it is
instead rounding down the memory address
of the data pointer to the nearest 4-byte value, causing
the wrong data to be copied into the ring buffer.

It appears that in the C standard this is considered
Undefined Behavior so the approach this patch takes is
to fix the test, by ensuring that rb_data is aligned to
u32_t.

Fixes: #14869

Signed-off-by: Andrew Boie <[email protected]>
@arun1joshi
Copy link
Contributor

verified that the tests/lib/ringbuffer test cases are passed with the latest commit 9926f94

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@andrewboie andrewboie

Labels
bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

tests: ringbuffer: avoid unaligned mem access andrewboie/zephyr
4 participants
@andrewboie @rljordan-zz @cinlyooi-intel @arun1joshi