Currently, the free block bitmap is roughly 4 times larger than it
needs to, wasting memory.
Let's assume maxsz = 128, minsz = 8 and n_max = 40.
Z_MPOOL_LVLS(128, 8) returns 3. The block size for level #0 is 128,
the block size for level #1 is 128/4 = 32, and the block size for
level #2 is 32/4 = 8. Hence levels 0, 1, and 2 for a total of 3 levels.
So far so good.
Now let's look at Z_MPOOL_LBIT_WORDS(). We get:
Z_MPOOL_LBIT_WORDS_UNCLAMPED(40, 0) = ((40 << 0) + 31) / 32 = 2
Z_MPOOL_LBIT_WORDS_UNCLAMPED(40, 1) = ((40 << 2) + 31) / 32 = 5
Z_MPOOL_LBIT_WORDS_UNCLAMPED(40, 2) = ((40 << 4) + 31) / 32 = 20
None of those are < 2 so Z_MPOOL_LBIT_WORDS() takes the results from
Z_MPOOL_LBIT_WORDS_UNCLAMPED().
Finally, let's look at _MPOOL_BITS_SIZE(. It sums all possible levels
with Z_MPOOL_LBIT_BYTES() which is:
#define Z_MPOOL_LBIT_BYTES(maxsz, minsz, l, n_max) \
(Z_MPOOL_LVLS((maxsz), (minsz)) >= (l) ? \
4 * Z_MPOOL_LBIT_WORDS((n_max), l) : 0)
Or given what we already have:
Z_MPOOL_LBIT_BYTES(128, 8, 0, 40) = (3 >= 0) ? 4 * 2 : 0 = 8
Z_MPOOL_LBIT_BYTES(128, 8, 1, 40) = (3 >= 1) ? 4 * 5 : 0 = 20
Z_MPOOL_LBIT_BYTES(128, 8, 2, 40) = (3 >= 2) ? 4 * 20 : 0 = 80
Z_MPOOL_LBIT_BYTES(128, 8, 3, 40) = (3 >= 3) ? 4 * ??
Wait... we're missing this one:
Z_MPOOL_LBIT_WORDS_UNCLAMPED(40, 3) = ((40 << 6) + 31) / 32 = 80
then:
Z_MPOOL_LBIT_BYTES(128, 8, 3, 40) = (3 >= 3) ? 4 * 80 : 0 = 320
Further levels yeld (3 >= 4), (3 >= 5), etc. so they're all false and
produce 0.
So this means that we're statically allocating 428 bytes to the bitmap
when clearly only the first 3 Z_MPOOL_LBIT_BYTES() results for the
corresponding 3 levels that we have should be summed e.g. only
108 bytes.
Here the code logic gets confused between level numbers and the number
levels, hence the extra allocation which happens to be exponential.
Signed-off-by: Nicolas Pitre <npitre@baylibre.com>
When splitting the pointer from the flag, ~SYS_SFLIST_FLAGS_MASK remains
a 32-bit value because of the lack of an L qualifier. Let's qualify it
with UL so the top half of 64-bit pointers is not truncated.
Signed-off-by: Nicolas Pitre <npitre@baylibre.com>
In z_sys_mem_pool_block_alloc() the size of the first level block
allocation is rounded up to the next 4-bite boundary. This means one
or more of the trailing blocks could overlap the free block bitmap.
Let's consider this code from kernel.h:
#define K_MEM_POOL_DEFINE(name, minsz, maxsz, nmax, align) \
char __aligned(align) _mpool_buf_##name[_ALIGN4(maxsz * nmax) \
+ _MPOOL_BITS_SIZE(maxsz, minsz, nmax)]; \
The static pool allocation rounds up the product of maxsz and nmax not
size of individual blocks. If we have, say maxsz = 10 and nmax = 20,
the result of _ALIGN4(10 * 20) is 200. That's the offset at which the
free block bitmap will be located.
However, because z_sys_mem_pool_block_alloc() does this:
lsizes[0] = _ALIGN4(p->max_sz);
Individual level 0 blocks will have a size of 12 not 10. That means
the 17th block will extend up to offset 204, 18th block up to 216, 19th
block to 228, and 20th block to 240. So 4 out of the 20 blocks are
overflowing the static pool area and 3 of them are even located
completely outside of it.
In this example, we have only 20 blocks that can't be split so there is
no extra free block bitmap allocation beyond the bitmap embedded in the
sys_mem_pool_lvl structure. This means that memory corruption will
happen in whatever data is located alongside the _mpool_buf_##name
array. But even with, say, 40 blocks, or larger blocks, the extra bitmap
size would be small compared to the extent of the overflow, and it would
get corrupted too of course.
And the data corruption will happen even without allocating any memory
since z_sys_mem_pool_base_init() stores free_list pointer nodes into
those blocks, which in turn may get corrupted if that other data is
later modified instead.
Fixing this issue is simple: rounding on the static pool allocation is
"misparenthesized". Let's turn
_ALIGN4(maxsz * nmax)
into
_ALIGN4(maxsz) * nmax
But that's not sufficient.
In z_sys_mem_pool_base_init() we have:
size_t buflen = p->n_max * p->max_sz, sz = p->max_sz;
u32_t *bits = (u32_t *)((u8_t *)p->buf + buflen);
Considering the same parameters as above, here we're locating the extra
free block bitmap at offset `buflen` which is 20 * 10 = 200, again below
the reach of the last 4 memory blocks. If the number of blocks gets past
the size of the embedded bitmap, it will overlap memory blocks.
Also, the block_ptr() call used here to initialize the free block linked
list uses unrounded p->max_sz, meaning that it is initially not locating
dlist nodes within the same block boundaries as what is expected from
z_sys_mem_pool_block_alloc(). This opens the possibility for allocated
adjacent blocks to overwrite dlist nodes, leading to random crashes in
the future.
So a complete fix must round up p->max_sz here too.
Given that runtime usage of max_sz should always be rounded up, it is
then preferable to round it up once at compile time instead and avoid
further mistakes of that sort. The existing _ALIGN4() usage on p->max_sz
at run time are then redundant.
Signed-off-by: Nicolas Pitre <npitre@baylibre.com>
Many things are currently aligned on 32-bit boundaries. On 64-bit
targets this should be 64-bit boundaries instead. Provide a pair of
macros, namely WB_UP() and WB_DN(), that do the right thing in both
cases.
The naming is short on purpose as this gets will be used within other
macro definitions, sometimes multiple times per line, and having a
longer name would make for excessively long lines.
Signed-off-by: Nicolas Pitre <npitre@baylibre.com>
On 64-bit systems the most notable difference is due to longs and
pointers being 64-bit wide. Therefore there must be a distinction
between ints and longs. Similar to the prf.c case, this patch properly
implements the h, hh, l, ll and z length modifiers as well as some small
cleanups.
Signed-off-by: Nicolas Pitre <npitre@baylibre.com>
The INT_TO_POINTER and POINTER_TO_INT macros must accommodate larger
pointers on 64-bit systems that don't fit into an int.
In the INT_TO_POINTER case, we have to use an extra cast to intptr_t
as an intermediate widening type to avoid complaints from the compiler
when converting from an int.
This change makes no difference on 32-bit systems.
Signed-off-by: Nicolas Pitre <npitre@baylibre.com>
We had both kernel and os as domains covering low level layers, just use
one and fix the issue of the os domain not being registered.
Signed-off-by: Anas Nashif <anas.nashif@intel.com>
With code that looks like this:
for (int i = ARRAY_SIZfoo) - 1; i >= 0; i--) ...
If foo is empty, ARRAY_SIZfoo) will return 0. But since it is
implemented using an unsigned long, the answer to 0UL - 1 is
18446744073709551615 on a 64-bit system, and that doesn't fit into
an int. The compiler complains with:
warning: overflow in conversion from ‘long unsigned int’ to ‘int’ changes value from ‘18446744073709551615’ to ‘-1’ [-Woverflow]
Let's fix that and get the expected behavior simply by turning the
unsigned long into a signed long.
Signed-off-by: Nicolas Pitre <npitre@baylibre.com>
Enclose the align argument in parents to make sure the cast applies to
the whole expression when expanded. This is especially important if the
argument contains a ternary operator.
Signed-off-by: Nicolas Pitre <npitre@baylibre.com>
From POSIX.1-2017:
The getsockname() function shall retrieve the locally-bound name of the
specified socket, store this address in the sockaddr structure pointed
to by the address argument, and store the length of this address in the
object pointed to by the address_len argument.
The address_len argument points to a socklen_t object which on input
specifies the length of the supplied sockaddr structure, and on output
specifies the length of the stored address. If the actual length of the
address is greater than the length of the supplied sockaddr structure,
the stored address shall be truncated.
If the socket has not been bound to a local name, the value stored in
the object pointed to by address is unspecified.
Signed-off-by: Tomasz Gorochowik <tgorochowik@antmicro.com>
When available, use the GCC / Clang builtins to implement the
math_extras functions. Otherwise, use the portable versions.
Signed-off-by: Jakob Olesen <jolesen@fb.com>
Add a <misc/math_extras.h> header file with portable implementations of
a number of commonly used math and bit counting operations that are not
available in the C programming language.
The math_extras functions fall into two groups:
1. Unsigned integer arithmetic with overflow detection. Addition and
multiplication are provides for the u32_t, u64_t, and size_t types.
This commit doesn't include subtraction and division, and there are
no signed operations. These could be added later if there is a need.
2. Bit-counting operations like clz, ctz, and ffs. These are provided
for u32_t and u64_t only. I don't see a need for size_t operations,
but they could be added if needed.
A follow-on commit will add more efficient implementations using
compiler builtins for those compilers that support it.
Another commit will replace other uses of "naked" compiler builtins with
calls to these functions.
Signed-off-by: Jakob Olesen <jolesen@fb.com>
When some header are included into C++ source file, this kind of
compilations errors are generated:
error: invalid conversion from 'void*'
to 'u32_t*' {aka 'unsigned int*'} [-fpermissive]
Signed-off-by: Benoit Leforestier <benoit.leforestier@gmail.com>
Extended the UTIL_DEC_N definitions to handle the UTIL_REPEAT macro with
up to 256 repetitions.
Signed-off-by: Kamil Piszczek <Kamil.Piszczek@nordicsemi.no>
This macro is slated for complete removal, as it's not possible
on arches with an MPU stack guard to know the true buffer bounds
without also knowing the runtime state of its associated thread.
As removing this completely would be invasive to where we are
in the 1.14 release, demote to a private kernel Z_ API instead.
The current way that the macro is being used internally will
not cause any undue harm, we just don't want any external code
depending on it.
The final work to remove this (and overhaul stack specification in
general) will take place in 1.15 in the context of #14269Fixes: #14766
Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
Rename reserved function names in the subsys/ subdirectory except
for static _mod_pub_set and _mod_unbind functions in bluetooth mesh
cfg_srv.c which clash with the similarly named global functions.
Signed-off-by: Patrik Flykt <patrik.flykt@intel.com>
Permission management no longer necessary, the former
parameter for the mutex is now simply ignored.
Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
For systems without userspace enabled, these work the same
as a k_mutex.
For systems with userspace, the sys_mutex may exist in user
memory. It is still tracked as a kernel object, but has an
underlying k_mutex that is looked up in the kernel object
table.
Future enhancements will optimize sys_mutex to not require
syscalls for uncontended sys_mutexes, using atomic ops
instead.
Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
For some reason we missed _zephyr_fputc in commit
4344e27c26. Rename _zephyr_fputc to just
zephyr_fputc and fixup associated code to build.
Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
Commit 4344e27c26 changed the reserved
function names, but got the naming wrong for fwrite. Just use the
name zephyr_fwrite everywhere.
Fixes#14275
Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
Update reserved function names starting with one underscore, replacing
them as follows:
'_k_' with 'z_'
'_K_' with 'Z_'
'_handler_' with 'z_handl_'
'_Cstart' with 'z_cstart'
'_Swap' with 'z_swap'
This renaming is done on both global and those static function names
in kernel/include and include/. Other static function names in kernel/
are renamed by removing the leading underscore. Other function names
not starting with any prefix listed above are renamed starting with
a 'z_' or 'Z_' prefix.
Function names starting with two or three leading underscores are not
automatcally renamed since these names will collide with the variants
with two or three leading underscores.
Various generator scripts have also been updated as well as perf,
linker and usb files. These are
drivers/serial/uart_handlers.c
include/linker/kobject-text.ld
kernel/include/syscall_handler.h
scripts/gen_kobject_list.py
scripts/gen_syscall_header.py
Signed-off-by: Patrik Flykt <patrik.flykt@intel.com>
Used to sanitize array indexes without using any branch
instructions. Should be portable to any architecture.
Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
We used to leave byte-long placeholder symbols to ensure
that empty application memory sections did not cause
build errors that were very difficult to understand.
Now we use some relatively portable inline assembly to
generate a symbol, but don't take up any extra space.
The malloc and libc partitions are now only instantiated
if there is some data to put in them.
Fixes: #13923
Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
Added __weak keyword to to support overriding assert_post_action().
This allows system designers to change/augment the assert behaviour,
i.e. add logging to persistant storage of program counter, line
number etc, and/or change reboot behaviour.
Signed-off-by: Tim Sørensen (TIMS) <tims@oticon.com>
MISRA rules (see #11425) forbid recursive algorithms. In the case of
rb_walk(), it's not actually used anywhere but a test right now, so we
can simply disable the API when CONFIG_MISRA_SANE is defined. Mempool
had a (IMHO, fairly clever) tail recursive loop in bfree_recombine()
which can be trivially transformed into an only slightly uglier
iterative version.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
MISRA rules (see #9892) forbid alloca() and family, even though those
features can be valuable performance and memory size optimizations
useful to Zephyr.
Introduce a MISRA_SANE kconfig, which when true enables a gcc error
condition whenever a variable length array is used.
When enabled, the mempool code will use a theoretical-maximum array
size on the stack instead of one tailored to the current pool
configuration.
The rbtree code will do similarly, but because the theoretical maximum
is quite a bit larger (236 bytes on 32 bit platforms) the array is
placed into struct rbtree instead so it can live in static data (and
also so I don't have to go and retune all the test stack sizes!).
Current code only uses at most two of these (one in the scheduler when
SCHED_SCALABLE is selected, and one for dynamic kernel objects when
USERSPACE and DYNAMIC_OBJECTS are set).
This tunable is false by default, but is selected in a single test (a
subcase of tests/kernel/common) for coverage. Note that the I2C and
SPI subsystems contain uncorrected VLAs, so a few platforms need to be
blacklisted with a filter.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
In case newlib is enabled, but POSIX subsys isn't, there're adhoc
implementations of read() and write() which work only with adhoc
stdin/stdout emulation layer. These are backed by system calls named
like "read" and "write". Rename all these functions and syscalls to
explicitly mention stdin/stdout in the names, to free namespace
for the implementation of generic read/write syscalls which will
integrate with POSIX fdtable.
Signed-off-by: Paul Sokolovsky <paul.sokolovsky@linaro.org>
When printk is used stack size expanding may be needed. This is
especially a problem for threads with small stacks.
Signed-off-by: Pawel Dunaj <pawel.dunaj@nordicsemi.no>
This is an integral part of userspace and cannot be used
on its own. Fold into the main userspace configuration.
Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
The macro RB_FOR_EACH_CONTAINER could run infinitely when the function
_rb_foreach_next returns NULL.
Signed-off-by: Flavio Ceolin <flavio.ceolin@intel.com>
We need a generic name for the partition containing
essential C library globals. We're going to need to
add the stack canary guard to this area so user mode
can read it.
Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
There are issues using lowercase min and max macros when compiling a C++
application with a third-party toolchain such as GNU ARM Embedded when
using some STL headers i.e. <chrono>.
This is because there are actual C++ functions called min and max
defined in some of the STL headers and these macros interfere with them.
By changing the macros to UPPERCASE, which is consistent with almost all
other pre-processor macros this naming conflict is avoided.
All files that use these macros have been updated.
Signed-off-by: Carlos Stuart <carlosstuart1970@gmail.com>
* Newlib now defines a special z_newlib_partition containing
all globals relevant to newlib. Most of these are in libc.a
with a heap tracking variable in newlib's hooks.
* Both C libraries now expose a k_mem_partition containing the
bounds of the malloc heap arena. Threads that want to use
libc malloc() will need to add this to their memory domain.
* z_newlib_get_heap_bounds has been removed, in favor of the
memory partition for the heap arena
* ztest now includes the C library partitions in its memory
domain.
* The mem_alloc test now runs in user mode to prove that this
all works for both C libraries.
Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
Replaced forever loop in assert with call to a function.
In post_assert_action() function, k_panic is called.
Forever loop was preventing logs to be printed and had behavior
ependent on the context (low prioriy thread - system continue to
ork, irq - system is blocked).
Signed-off-by: Krzysztof Chruscinski <krzysztof.chruscinski@nordicsemi.no>
The sys_dlist_insert_*() functions had a behavior where a NULL
argument for the insertion position to sys_dlist_insert_after/before()
was interpreted as "the end of the list". We never used that
convention (except in one spot internal to dlist.h which was not
itself used anywhere), and of course already have an API for appending
and prepending to a list.
In practice this was a performance disaster. The NULL check is
virtually never provable statically by the compiler, so that test and
branch is present always. And worse, the check and call to another
function was pushing this beyond the complexity limit for gcc to
inline a function (at -Os optimization anyway), forcing us to use
function calls for what should be a ~8 instruction sequence. The
upshot is that dlist insertions were 2-3x slower than they needed to
be.
Deprecate these older APIs and introduce a new sys_dlist_insert() call
which can be much better optimized.
Signed-off-by: Andy Ross <andrew.j.ross@intel.com>
Some of macros initially created in the logger has been moved
to util.h. This commit replaces custom macros with the one
from util.h
Signed-off-by: Krzysztof Chruscinski <krzysztof.chruscinski@nordicsemi.no>
Added macros for conditional code generation based on a flag.
Additionally, added macros for getting first and second argument
from variable list of arguments and getting all arguments except
the first one.
Signed-off-by: Krzysztof Chruscinski <krzysztof.chruscinski@nordicsemi.no>
The original implementation allows a list to be corrupted by list
operations on the removed node. Existing code attempts to avoid this by
using external state to determine whether a node is in a list, but this
is fragile and fails when the state that holds the flag value is changed
after the node is removed, e.g. in preparation for re-using the node.
Follow Linux in invalidating the link pointers in a removed node. Add
API so that detection of particpation in a list is available at the node
abstraction.
This solution relies on the following steady-state invariants:
* A node (as opposed to a list) will never be adjacent to itself in a
list;
* The next and prev pointers of a node are always either both null or
both non-null.
Signed-off-by: Peter A. Bigot <pab@pabigot.com>
printk is supposed to be very lean, but should at least not
print garbage values. Now when a 64-bit integral value is
passed in to be printed, 'ERR' will be reported if it doesn't
fit in 32-bits instead of truncating it.
The printk documentation was slightly out of date, this has been
updated.
Fixes: #7179
Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
This is a custom Gcov implementation. Taking excerpts from gcc
gcc libgcc/libgcov.h and gcc/gcov-io.h.
Ported to zephyr by Ramakrishna Pallala <ramakrishna.pallala@intel.com>
and Adithya Baglody <adithya.nagaraj.baglody@intel.com>
Signed-off-by: Adithya Baglody <adithya.nagaraj.baglody@intel.com>
This new macro will be able to do FOR_EACH with a fixed argument.
This fixed argument will always be called as the 2nd argument
to the function call(_fn).
Signed-off-by: Adithya Baglody <adithya.nagaraj.baglody@intel.com>
As extend fdtable usage to more cases, there regularly arises a need
to forward ioctl/fcntl arguments to another ioctl vmethod, which is
complicated because it defined as taking variadic arguments. The only
portable solution is to convert variadic arguments to va_list at the
first point of entry from client code, and then pass va_list around.
To facilitate calling ioctl with variadic arguments from system code,
z_fdtable_call_ioctl() helper function is added.
Signed-off-by: Paul Sokolovsky <paul.sokolovsky@linaro.org>
This commit reworks socket poll implementation to support multiple
socket implementations.
To achieve that, two ioctl poll helper requests were added:
ZFD_IOCTL_POLL_PREPARE and ZFD_IOCTL_POLL_UPDATE. The poll
implementation calls ioctl with these requests for each socket
requested in the fds table.
The first request is responsible for preparing k_poll_event objects
for specific socket. It can request to skip waiting in k_poll by
returning EALREADY through errno.
The latter request is responsible for processing outcome of k_poll for
each socket. It can request to retry the k_poll by returning EAGAIN
through errno.
Signed-off-by: Robert Lubos <robert.lubos@nordicsemi.no>
