apache / nuttx

Apache NuttX is a mature, real-time embedded operating system (RTOS)
https://nuttx.apache.org/
Apache License 2.0
2.6k stars 1.11k forks source link

Implicit delaration of memset when building against Nuttx export. #9616

Closed g2gps closed 1 year ago

g2gps commented 1 year ago

I'm building a third party library against Nuttx export, with CONFIG_BUILD_KERNEL.

When building:

int test(int fd) {
    fd_set fds;
    int r;

    FD_ZERO(&fds);
    FD_SET(fd, &fds);

    if ((r = select(fd+1, NULL, &fds, NULL, NULL)) < 0)
        return -1;

    assert(r > 0);

    return 0;
}

I receive the warning:

test.c: In function 'test':
nuttx/include/sys/select.h:95:4: warning: implicit declaration of function 'memset' [-Wimplicit-function-declaration]
   95 |    memset((set), 0, sizeof(fd_set))
      |    ^~~~~~
test.c:63:5: note: in expansion of macro 'FD_ZERO'
   63 |     FD_ZERO(&fds);
      |     ^~~~~~~
test.c:30:1: note: include '<string.h>' or provide a declaration of 'memset'
   29 | #include "test.h"
  +++ |+#include <string.h>
   30 | 

But interestingly, when I modify an existing Nuttx example, in this case "hello", like this:

diff --git a/examples/hello/hello_main.c b/examples/hello/hello_main.c
index 2cd6cb400..a3635f218 100644
--- a/examples/hello/hello_main.c
+++ b/examples/hello/hello_main.c
@@ -22,8 +22,7 @@
  * Included Files
  ****************************************************************************/

-#include <nuttx/config.h>
-#include <stdio.h>
+#include <sys/select.h>

 /****************************************************************************
  * Public Functions
@@ -35,6 +34,9 @@

 int main(int argc, FAR char *argv[])
 {
-  printf("Hello, World!!\n");
+  fd_set fds;
+  FD_ZERO(&fds);
+
+  /* Just return something for the example */
   return 0;
 }

I don't receive any warning.

I'm building hello_main.c with:

riscv64-unknown-elf-gcc -c -fno-common -Wall -Wstrict-prototypes -Wshadow -Wundef -fno-common -pipe  -Os -fno-strict-aliasing -fomit-frame-pointer -march=rv32imac -mabi=ilp32 -isystem apps/import/include -isystem apps/import/include -D__NuttX__ -DNDEBUG  -I "apps/include"   hello_main.c -o  apps.examples.hello.o

The resulting Make.dep for hello_main is:

hello_main.c.apps.examples.hello.o: \
hello_main.c \
apps/import/include/sys/select.h \
apps/import/include/nuttx/config.h \
apps/import/include/limits.h \
apps/import/include/arch/limits.h \
apps/import/include/stdint.h \
apps/import/include/nuttx/compiler.h \
apps/import/include/arch/types.h \
apps/import/include/arch/inttypes.h \
apps/import/include/signal.h \
apps/import/include/time.h \
apps/import/include/sys/types.h \
apps/import/include/sys/time.h
# No files specified for dependency generation
# No files specified for dependency generation

I may be missing something obvious, but I can't see where the definition for memset is coming from?

Naturally, the issue could be fixed by adding #include <string.h> to sys/select.h, but it would be nice to understand why the same warning doesn't occur when building using Nuttx's make import from the apps directory.

mu578 commented 1 year ago

FD_ZERO should be defined as explicit_bzero and having select.h including string

#include <nuttx/config.h>

#include <limits.h>
#include <stdint.h>
#include <signal.h>
+#include <string.h>
#include <sys/time.h> // -> is including #include <sys/select.h> bad juju

#ifdef CONFIG_FDCHECK
#  include <nuttx/fdcheck.h>
#endif

...

or something like

#ifndef CONFIG_SOME_DONT_WANT_STRING_AVAILABILITY_HERE
static inline void explicit_bzero(FAR void * s, size_t n)
{ unsigned char * p = s; while (n-- > 0) { *p++ = 0; } }
#else
EXTERN void explicit_bzero(FAR void * s, size_t n);
#endif

because select.h references that symbol: it is then the owner unit, change should be made in that header.

#define FD_ZERO(set) \
   explicit_bzero((set), sizeof(fd_set))

@g2gps make import because there is certainly a conditional include somewhere prior or that:

CFLAGS += -Wno-implicit-function-declaration

FD_ZERO is often used with its companion FD_SET and such within select loops in multi-threaded environments, if there is a compiler optimization regarding memset it can cause all sort of overflow conditions ; from a security point of view: you really want FD_ZERO to guarantee that zeroing happens ; so explicit_bzero should be used in place.

xiaoxiang781216 commented 1 year ago

@g2gps the simplest method is:

  1. make V=1 to get the command line
  2. Append -E to the command line and run again
  3. Compare the generated file to see the difference