Warning/Potential bug about using newlib 4.3 or newer on ARM

[nslowell]

The following issue doesn't seem to exist in any currently supported workflows, but external workflows (i.e. meta-zephyr for building zephyr with yocto) have led me to the following issue that I thought was worth bringing up, especially b/c I think it will be a problem at somepoint in the future assuming continued newlib & picolib upgrades in the SDK and it might dictate some adjustments that are needed in zephyr's posix code domain.

TL;DR

Newlib added sysconf for arm which can cause precompiled newlib to call into zephyr's sysconf function, but the symbolic constants for sysconf (e.g. _SC_PAGE_SIZE) are currently defined with different values b/w newlib's unistd.h and zephyr, which cause incorrect sysconf processing.

Reproduction:

1. Follow Zephyr Getting Started to get Zephyr and Zephyr-SDK
2. Clone Zephyr-SDK source

git clone git@github.com:zephyrproject-rtos/sdk-ng.git ~/sdk-ng
cd ~/sdk-ng
git submodule update --init

3. Update/merge newlib 4.1 branch to 4.3 (if you wish to proof 4.1 first, that's fine. Follow steps 4 through 11 before merging, then repeat after merging to 4.3)

cd ~/sdk-ng/newlib
git checkout zephyr-newlib-4.1.0
git remote add upstream https://sourceware.org/git/newlib-cygwin.git
git fetch upstream
git merge tags/newlib-4.3.0

4. Build arm toolchain (and go for a walk)

cd ~/sdk-ng
mkdir build
./contrib/linux_build_toolchain.sh -c arm-zephyr-eabi -s ~/sdk-ng -o ~/sdk-ng/build

5. Copy new arm toolchain to zephyr-sdk (probably better ways to do this, but this works)

mv ~/zephyr-sdk-0.17.0/arm-zephyr-eabi ~/zephyr-sdk-0.17.0/backup-arm-zephyr-eabi
cp -r build/output/arm-zephyr-eabi ~/zephyr-sdk-0.17.0/

6. Get Zephyr Build ready
   cd ~/zephyrproject/zephyr
7. Modify dts/arm/ti/lm3s6965.dtsi and change flash0 reg to 256*1024*1024 (just so the image will "fit" in FLASH)
8. Modify lib/posix/options/sysconf.c to see behavior
   Add
   printk("%s(%d)\n", __func__, x);
   to the top of the sysconf function
9. Prepare a C++ application that looks like the following (e.g. modified samples/cpp/hello_world)
   prj.conf:

	CONFIG_CPP=y
	CONFIG_REQUIRES_FULL_LIBCPP=y
	CONFIG_NEWLIB_LIBC=y
	CONFIG_POSIX_API=y
	#This is just so the compiler doesn't remove our malloc call due to the simple code example
	CONFIG_NO_OPTIMIZATIONS=y

CMakeLists.txt:

	# SPDX-License-Identifier: Apache-2.0
	cmake_minimum_required(VERSION 3.20.0)

	find_package(Zephyr REQUIRED HINTS $ENV{ZEPHYR_BASE})
	project(hello_cpp_world)

	target_sources(app PRIVATE src/main.cpp)

src/main.cpp:

#include <stdlib.h>

int main(void)
{
	int *a;
	a = (int*)malloc(4096); //simulated new() call
	return 0;
}

10. Build and run the application for arm

cd ~/zephyrproject/zephyr
west build -p always -b qemu_cortex_m3 samples/cpp/hello_world
west build -t run

11. Observe output is the following:

qemu-system-arm: warning: nic stellaris_enet.0 has no peer
Timer with period zero, disabling
*** Booting Zephyr OS build v4.1.0-837-g62f5386518a0 ***
sysconf(8)

If you look in ~/sdk-ng/newlib/newlib/libc/stdlib/_mallocr.c, line 666, you will see the sysconf(_SC_PAGE_SIZE) call.
The problem is that newlib was built using ~/sdk-ng/newlib/newlib/libc/include/sys/unistd.h which defines _SC_PAGE_SIZE as 8.
But zephyr is using include/zephyr/posix/sys/sysconf.h and sees _SC_PAGE_SIZE as 110 and 8 is _SC_MAPPED_FILES.
So zephyr's sysconf isn't returning the correct switch case value when called from newlib.

Resolution

I'm not really sure what the "correct" fix is, since at first glance it seems like there are a few changes that could be made, so the following is very much me thinking out loud.

1. Mostly, looking at unistd.h from newlib, although the POSIX standard doesn't seem to dictate specific values, it seems like zephyr should define its symbolic constants for sysconf to have the same values as newlib instead of an enum list in include/zephyr/posix/sysconf.h. But I know that's easier said than done b/c it messes with using z_sysconf()
2. Perhaps newlib's new sysconf means zephyr's sysconf shouldn't be included for arm? Not sure.
3. Perhaps something with CONFIG_POSIX_SYSCONF_IMPL_FULL vs CONFIG_POSIX_SYSCONF_IMPL_MACRO needs to be adjusted. I know switching from Full to Macro gets around this problem. Maybe FULL can't/shouldn't be the default (?)

I'm interested to have @cfriedt and @ycsin give their take on this, since its POSIX.
But this has a libc component, so @stephanosio @aescolar @nashif and @keith-packard might also have good input.

Thanks.

[cfriedt]

@nslowell - it might be possible to adjust the symbolic values for _SC_PAGE_SIZE and others within Zephyr to align with those from Newlib without introducing any regressions. If there is some base offset that needs to be subtracted to fit compactly in an array, it should be relatively easy to do.

Are you comfortable making a PR for that?

[nslowell]

@cfriedt - The simple change is switching the current enum list to this:

enum {
	_SC_ARG_MAX = 0,
	_SC_CHILD_MAX = 1,
	_SC_CLK_TCK = 2,
	_SC_NGROUPS_MAX = 3,
	_SC_OPEN_MAX = 4,
	_SC_JOB_CONTROL = 5,
	_SC_SAVED_IDS = 6,
	_SC_VERSION = 7,
	_SC_PAGESIZE = 8,
	_SC_PAGE_SIZE = _SC_PAGESIZE,
	_SC_NPROCESSORS_CONF = 9,
	_SC_NPROCESSORS_ONLN = 10,
	_SC_PHYS_PAGES = 11,
	_SC_AVPHYS_PAGES = 12,
	_SC_MQ_OPEN_MAX = 13,
	_SC_MQ_PRIO_MAX = 14,
	_SC_RTSIG_MAX = 15,
	_SC_SEM_NSEMS_MAX = 16,
	_SC_SEM_VALUE_MAX = 17,
	_SC_SIGQUEUE_MAX = 18,
	_SC_TIMER_MAX = 19,
	_SC_TZNAME_MAX = 20,
	_SC_ASYNCHRONOUS_IO = 21,
	_SC_FSYNC = 22,
	_SC_MAPPED_FILES = 23,
	_SC_MEMLOCK = 24,
	_SC_MEMLOCK_RANGE = 25,
	_SC_MEMORY_PROTECTION = 26,
	_SC_MESSAGE_PASSING = 27,
	_SC_PRIORITIZED_IO = 28,
	_SC_REALTIME_SIGNALS = 29,
	_SC_SEMAPHORES = 30,
	_SC_SHARED_MEMORY_OBJECTS = 31,
	_SC_SYNCHRONIZED_IO = 32,
	_SC_TIMERS = 33,
	_SC_AIO_LISTIO_MAX = 34,
	_SC_AIO_MAX = 35,
	_SC_AIO_PRIO_DELTA_MAX = 36,
	_SC_DELAYTIMER_MAX = 37,
	_SC_THREAD_KEYS_MAX = 38,
	_SC_THREAD_STACK_MIN = 39,
	_SC_THREAD_THREADS_MAX = 40,
	_SC_TTY_NAME_MAX = 41,
	_SC_THREADS = 42,
	_SC_THREAD_ATTR_STACKADDR = 43,
	_SC_THREAD_ATTR_STACKSIZE = 44,
	_SC_THREAD_PRIORITY_SCHEDULING = 45,
	_SC_THREAD_PRIO_INHERIT = 46,
	_SC_THREAD_PRIO_PROTECT = 47,
	_SC_THREAD_PRIO_CEILING = _SC_THREAD_PRIO_PROTECT,
	_SC_THREAD_PROCESS_SHARED = 48,
	_SC_THREAD_SAFE_FUNCTIONS = 49,
	_SC_GETGR_R_SIZE_MAX = 50,
	_SC_GETPW_R_SIZE_MAX = 51,
	_SC_LOGIN_NAME_MAX = 52,
	_SC_THREAD_DESTRUCTOR_ITERATIONS = 53,
	_SC_ADVISORY_INFO = 54,
	_SC_ATEXIT_MAX = 55,
	_SC_BARRIERS = 56,
	_SC_BC_BASE_MAX = 57,
	_SC_BC_DIM_MAX = 58,
	_SC_BC_SCALE_MAX = 59,
	_SC_BC_STRING_MAX = 60,
	_SC_CLOCK_SELECTION = 61,
	_SC_COLL_WEIGHTS_MAX = 62,
	_SC_CPUTIME = 63,
	_SC_EXPR_NEST_MAX = 64,
	_SC_HOST_NAME_MAX = 65,
	_SC_IOV_MAX = 66,
	_SC_IPV6 = 67,
	_SC_LINE_MAX = 68,
	_SC_MONOTONIC_CLOCK = 69,
	_SC_RAW_SOCKETS = 70,
	_SC_READER_WRITER_LOCKS = 71,
	_SC_REGEXP = 72,
	_SC_RE_DUP_MAX = 73,
	_SC_SHELL = 74,
	_SC_SPAWN = 75,
	_SC_SPIN_LOCKS = 76,
	_SC_SPORADIC_SERVER = 77,
	_SC_SS_REPL_MAX = 78,
	_SC_SYMLOOP_MAX = 79,
	_SC_THREAD_CPUTIME = 80,
	_SC_THREAD_SPORADIC_SERVER = 81,
	_SC_TIMEOUTS = 82,
	_SC_TRACE = 83,
	_SC_TRACE_EVENT_FILTER = 84,
	_SC_TRACE_EVENT_NAME_MAX = 85,
	_SC_TRACE_INHERIT = 86,
	_SC_TRACE_LOG = 87,
	_SC_TRACE_NAME_MAX = 88,
	_SC_TRACE_SYS_MAX = 89,
	_SC_TRACE_USER_EVENT_MAX = 90,
	_SC_TYPED_MEMORY_OBJECTS = 91,
	_SC_V7_ILP32_OFF32 = 92,
	_SC_V6_ILP32_OFF32 = _SC_V7_ILP32_OFF32,
	_SC_XBS5_ILP32_OFF32 = _SC_V7_ILP32_OFF32,
	_SC_V7_ILP32_OFFBIG = 93,
	_SC_V6_ILP32_OFFBIG = _SC_V7_ILP32_OFFBIG,
	_SC_XBS5_ILP32_OFFBIG = _SC_V7_ILP32_OFFBIG,
	_SC_V7_LP64_OFF64 = 94,
	_SC_V6_LP64_OFF64 = _SC_V7_LP64_OFF64,
	_SC_XBS5_LP64_OFF64 = _SC_V7_LP64_OFF64,
	_SC_V7_LPBIG_OFFBIG = 95,
	_SC_V6_LPBIG_OFFBIG = _SC_V7_LPBIG_OFFBIG,
	_SC_XBS5_LPBIG_OFFBIG = _SC_V7_LPBIG_OFFBIG,
	_SC_XOPEN_CRYPT = 96,
	_SC_XOPEN_ENH_I18N = 97,
	_SC_XOPEN_LEGACY = 98,
	_SC_XOPEN_REALTIME = 99,
	_SC_STREAM_MAX = 100,
	_SC_PRIORITY_SCHEDULING = 101,
	_SC_XOPEN_REALTIME_THREADS = 102,
	_SC_XOPEN_SHM = 103,
	_SC_XOPEN_STREAMS = 104,
	_SC_XOPEN_UNIX = 105,
	_SC_XOPEN_VERSION = 106,
	_SC_2_CHAR_TERM = 107,
	_SC_2_C_BIND = 108,
	_SC_2_C_DEV = 109,
	_SC_2_FORT_DEV = 110,
	_SC_2_FORT_RUN = 111,
	_SC_2_LOCALEDEF = 112,
	_SC_2_PBS = 113,
	_SC_2_PBS_ACCOUNTING = 114,
	_SC_2_PBS_CHECKPOINT = 115,
	_SC_2_PBS_LOCATE = 116,
	_SC_2_PBS_MESSAGE = 117,
	_SC_2_PBS_TRACK = 118,
	_SC_2_SW_DEV = 119,
	_SC_2_UPE = 120,
	_SC_2_VERSION = 121,
	_SC_THREAD_ROBUST_PRIO_INHERIT = 122,
	_SC_THREAD_ROBUST_PRIO_PROTECT = 123,
	_SC_XOPEN_UUCP = 124,
	_SC_LEVEL1_ICACHE_SIZE = 125,
	_SC_LEVEL1_ICACHE_ASSOC = 126,
	_SC_LEVEL1_ICACHE_LINESIZE = 127,
	_SC_LEVEL1_DCACHE_SIZE = 128,
	_SC_LEVEL1_DCACHE_ASSOC = 129,
	_SC_LEVEL1_DCACHE_LINESIZE = 130,
	_SC_LEVEL2_CACHE_SIZE = 131,
	_SC_LEVEL2_CACHE_ASSOC = 132,
	_SC_LEVEL2_CACHE_LINESIZE = 133,
	_SC_LEVEL3_CACHE_SIZE = 134,
	_SC_LEVEL3_CACHE_ASSOC = 135,
	_SC_LEVEL3_CACHE_LINESIZE = 136,
	_SC_LEVEL4_CACHE_SIZE = 137,
	_SC_LEVEL4_CACHE_ASSOC = 138,
	_SC_LEVEL4_CACHE_LINESIZE = 139,
	_SC_POSIX_26_VERSION = 140,
};

which allows
#define z_sysconf(x) (long)CONCAT(__z_posix_sysconf, x)
to keep working in sysconf.c

If we switch to a list of #defines like newlib, then more changes will be needed (unknown at the moment) b/c z_sysconf will break.

Thoughts?

[cfriedt]

@nslowell - that seems quite reasonable to me. The enums are intentionally unordered.

If you make a PR, I'll review. You might want to keep it in the draft state until it's all green.

The only issue I could foresee is that maybe picolibc and Newlib might have differing values, but we should be able to work around that as well if it's an issue.

My preference would be to not use explicit integer values, but it isn't pressing.

If we do use integer values, then we might as well use #defines

[nslowell]

@cfriedt Here is my draft PR: #88177

but I think this (85aebac) broke twister-build-prep.