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

xtimer: valgrind complains about uninitialized variables for jumps #4372

Closed
miri64 opened this issue Dec 1, 2015 · 11 comments
Closed

xtimer: valgrind complains about uninitialized variables for jumps #4372

miri64 opened this issue Dec 1, 2015 · 11 comments
Assignees
Labels
Area: timers Area: timer subsystems Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Comments

@miri64
Copy link
Member

miri64 commented Dec 1, 2015

Running tests/xtimer_msg_receive_timeout in valgrind yields the following result:

main(): This is RIOT! (Version: 2015.12-devel-718-gc143-beutlin-pkg/port/lwip)
==7035== Conditional jump or move depends on uninitialised value(s)
==7035==    at 0x804BF92: _is_set (sys/xtimer/xtimer_core.c:53)
==7035==    by 0x804C398: xtimer_remove (sys/xtimer/xtimer_core.c:244)
==7035==    by 0x804C114: xtimer_set (sys/xtimer/xtimer_core.c:122)
==7035==    by 0x804BCB6: xtimer_set_msg (sys/xtimer/xtimer.c:124)
==7035==    by 0x804B99D: main (tests/xtimer_msg_receive_timeout/main.c:34)
==7035==  Uninitialised value was created by a stack allocation
==7035==    at 0x804B949: main (tests/xtimer_msg_receive_timeout/main.c:27)
==7035== 
==7035== Conditional jump or move depends on uninitialised value(s)
==7035==    at 0x804BF9C: _is_set (sys/xtimer/xtimer_core.c:53)
==7035==    by 0x804C398: xtimer_remove (sys/xtimer/xtimer_core.c:244)
==7035==    by 0x804C114: xtimer_set (sys/xtimer/xtimer_core.c:122)
==7035==    by 0x804BCB6: xtimer_set_msg (sys/xtimer/xtimer.c:124)
==7035==    by 0x804B99D: main (tests/xtimer_msg_receive_timeout/main.c:34)
==7035==  Uninitialised value was created by a stack allocation
==7035==    at 0x804B949: main (tests/xtimer_msg_receive_timeout/main.c:27)
==7035== 
Message received: 44
Timeout!
Message received: 44
Timeout!

This is due to target and/or long_target being checked early on an xtimer_remove() in xtimer_set() and the xtimer not being used yet. It works for now because the timer is set in the BSS section, which results in the timer always being 0 in the beginning, but it might lead to unwanted behavior.

@miri64 miri64 added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: timers Area: timer subsystems labels Dec 1, 2015
@miri64 miri64 added this to the Release 2015.12 milestone Dec 1, 2015
@OlegHahm
Copy link
Member

OlegHahm commented Dec 1, 2015

I thought I fixed that in #3902

@miri64
Copy link
Member Author

miri64 commented Dec 1, 2015

I'll check again, if I maybe have used an old version of master.

@OlegHahm
Copy link
Member

OlegHahm commented Dec 1, 2015

I also haven't looked if I missed something in #3902, so it might be that it still may be uninitialized.

@miri64
Copy link
Member Author

miri64 commented Dec 1, 2015

nope, still an issue in current master.

@miri64
Copy link
Member Author

miri64 commented Dec 1, 2015

(562218d)

@miri64
Copy link
Member Author

miri64 commented Dec 1, 2015

Mh, may it be that #4272 reverted that for xtimer_msg_receive_timeout()?

@miri64
Copy link
Member Author

miri64 commented Dec 1, 2015

Ah, the problem is not xtimer_msg_receive_timeout() but xtimer_msg_set[64](). #3902 doesn't offer a solution for that.

@kaspar030
Copy link
Contributor

That uninitialized check might cause an iteration through active timers, but an't lad to corruption or a crash. Therefore I'd like to postpone it for after the release.

@kaspar030 kaspar030 removed this from the Release 2015.12 milestone Dec 7, 2015
@miri64
Copy link
Member Author

miri64 commented Dec 7, 2015

Maybe the fix isn't so much a code, but a documentation problem ;-). If you use xtrimer_msg_set() you need to initialize those two values.

@OlegHahm OlegHahm modified the milestone: Release 2016.03 Dec 7, 2015
@haukepetersen
Copy link
Contributor

@kaspar030, @authmillenon: should we postpone this again?

@miri64
Copy link
Member Author

miri64 commented Mar 31, 2016

Provided fix in #5216

lebrush pushed a commit to lebrush/RIOT that referenced this issue Jun 27, 2016
jia200x pushed a commit to jia200x/RIOT that referenced this issue Nov 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: timers Area: timer subsystems Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

No branches or pull requests

4 participants