Skip to content

x86 stack check fail w/posix-lib & newlib #13701

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
galak opened this issue Feb 23, 2019 · 4 comments
Closed

x86 stack check fail w/posix-lib & newlib #13701

galak opened this issue Feb 23, 2019 · 4 comments
Assignees
@nashif @andrewboie
Labels
area: POSIX POSIX API Library area: X86 x86 Architecture (32-bit) bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug

Comments

@galak
Copy link
Collaborator

galak commented Feb 23, 2019 •
edited
Loading

When trying to run the posix tests w/newlib as enabled by PR #13685

./scripts/sanitycheck -T tests/posix/ -p qemu_x86

qemu_x86 tests/posix/common/portability.posix.newlib FAILED: handler_crash

We get:

SDK 0.9.5:

SeaBIOS (version rel-1.11.2-0-gf9626ccb91-prebuilt.qemu-project.org)
Booting from ROM..***** Booting Zephyr OS v1.14.0-rc1-501-ge03b7fad46 *****
***** Double Fault *****
***** Stack Check Fail! *****
Current thread ID = 0x00403160
eax: 0x00000001, ebx: 0x004011e0, ecx: 0x00000000, edx: 0x0043e00c
esi: 0x00401020, edi: 0x00400000, ebp: 0x004011e0, esp: 0x0043e000
eflags: 0x00000202 cs: 0x0008
call trace:
eip: 0x00002cc7
     corrupted? (bp=0x004011e0)
Fatal fault in essential thread! Spinning...
Terminate emulator due to fatal kernel error

SDK 0.10.0:

SeaBIOS (version rel-1.12.0-0-ga698c8995f-prebuilt.qemu.org)
Booting from ROM..***** Booting Zephyr OS v1.14.0-rc1-501-ge03b7fad46 *****
***** Double Fault *****
***** Stack Check Fail! *****
Current thread ID = 0x00403160
eax: 0x00000001, ebx: 0x0043e064, ecx: 0x00000000, edx: 0x00000012
esi: 0x004011dc, edi: 0x0043e060, ebp: 0x0043e040, esp: 0x0043dfd8
eflags: 0x00000202 cs: 0x0008
call trace:
eip: 0x000026f3
     0x0000275e (0x401020)
     0x00001e26 (0x401020)
     0x000016ed (0x401020)
     0x00001568 (0x401020)
     0x0000a616 (0x14c4f)
     0x00003255 (0x124bd)
     0x0000a7f0 (0x4430ec)
     0x0000ac27 (0x43e1e4)
Fatal fault in essential thread! Spinning...
Terminate emulator due to fatal kernel error
@galak galak added bug The issue is a bug, or the PR is fixing a bug area: X86 x86 Architecture (32-bit) area: POSIX POSIX API Library priority: medium Medium impact/importance bug labels Feb 23, 2019
@nashif
Copy link
Member

nashif commented Feb 23, 2019

@galak why was this merged although it is known to be failing? Now all CI jobs are failing because of this.

@nashif nashif mentioned this issue Feb 23, 2019
Merged
@galak
Copy link
Collaborator Author

galak commented Feb 23, 2019

@galak why was this merged although it is known to be failing? Now all CI jobs are failing because of this.

My bad, wasn't thinking about the CI impact to everything else.

Is there someone who can look at the x86 issue.

@andrewboie
Copy link
Contributor

The error message is correct; this is a stack overflow in _main_thread.
It appears if CONFIG_ZTEST is enabled, CONFIG_MAIN_STACK_SIZE defaults to 512, which is not enough.

@andrewboie
Copy link
Contributor

andrewboie commented Feb 24, 2019 •
edited
Loading

@galak I've unpeeled this enough to see what is going on.

For this test, CONFIG_STDOUT_CONSOLE=y, which causes TC_PRINT() and related macros to map to libc printf() instead of the kernel's printk(). Newlib's printf() uses tons of stack space, even more than minimal libc, and LOTS more than printk(), and we are blowing stack in _ztest_run_test_suite() before the test can even run.

For some reason, CONFIG_STDOUT_CONSOLE is defaulting to Y when newlib is enabled:

config STDOUT_CONSOLE
	bool "Send stdout to console"
	depends on CONSOLE_HAS_DRIVER
	default NEWLIB_LIBC
	help
	  This option directs standard output (e.g. printf) to the console
	  device, rather than suppressing it entirely. See also EARLY_CONSOLE
	  option.

I think the right thing to do is always enable CONFIG_STDOUT_CONSOLE, but get rid of this policy which is redirecting testcase output to printf just because it is turned on.

@andrewboie andrewboie mentioned this issue Feb 24, 2019
Merged
andrewboie pushed a commit to andrewboie/zephyr that referenced this issue Feb 25, 2019
libc: fix CONFIG_STDOUT_CONSOLE semantics
f13b847 
The intent of this Kconfig is to allow libc stdout
functions like printf() to send their output to the
active console driver instead of discarding it.

This somehow evolved into preferring to use
printf() instead of printk() for all test case output
if enabled. Libc printf() implementation for both
minimal libc and newlib use considerably more stack
space than printk(), with nothing gained by using
them.

Remove all instances where we are conditionally
sending test case output based on this config, enable
it by default, and adjust a few tests that disabled
this because they were blowing stack.

printk() and vprintk() now work as expected for
unit_testing targets, they are just wrappers for
host printf().

Fixes: zephyrproject-rtos#13701

Signed-off-by: Andrew Boie <[email protected]>
galak pushed a commit that referenced this issue Feb 26, 2019
@galak
libc: fix CONFIG_STDOUT_CONSOLE semantics
feab370 
The intent of this Kconfig is to allow libc stdout
functions like printf() to send their output to the
active console driver instead of discarding it.

This somehow evolved into preferring to use
printf() instead of printk() for all test case output
if enabled. Libc printf() implementation for both
minimal libc and newlib use considerably more stack
space than printk(), with nothing gained by using
them.

Remove all instances where we are conditionally
sending test case output based on this config, enable
it by default, and adjust a few tests that disabled
this because they were blowing stack.

printk() and vprintk() now work as expected for
unit_testing targets, they are just wrappers for
host printf().

Fixes: #13701

Signed-off-by: Andrew Boie <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nashif nashif

@andrewboie andrewboie

Labels
area: POSIX POSIX API Library area: X86 x86 Architecture (32-bit) bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@nashif @andrewboie @galak