unikraft / lib-newlib

Unikraft port of newlib, a C standard library
Other
5 stars 23 forks source link

Update newlib to the latest Unikraft version #28

Closed eduardvintila closed 1 year ago

eduardvintila commented 2 years ago

This PR adapts newlib to the latest changes introduced in Unikraft core.

If app-python3 is to be tested, one needs to apply the small modification from this PR: https://github.com/unikraft/lib-python3/pull/12

Newlib still doesn't provide a Pthreads implementation, so the pthread-embedded PR can be used to test complex applications such as redis, sqlite or nginx.

Signed-off-by: Eduard Vintilă eduard.vintila47@gmail.com

John-Ted commented 1 year ago

When trying to build app-sqlite, I get the following error:

/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/unikraft/lib/vfscore/main.c:785:5: error: conflicting types for ‘ioctl’; have ‘int(int,  long unsigned int, ...)’
  785 | int ioctl(int fd, unsigned long int request, ...)
      |     ^~~~~
In file included from /home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/libs/lib-newlib/include/sys/ioctl.h:35,
                 from /home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/unikraft/lib/vfscore/main.c:37:
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/libs/lib-newlib/musl-imported/include/sys/ioctl.h:143:5: note: previous declaration of ‘ioctl’ with type ‘int(int,  int, ...)’
  143 | int ioctl (int, int, ...);
      |     ^~~~~

Possibly related: https://github.com/unikraft/unikraft/commit/f22c4fbf2805890ac74f5401eff78492ddaddec0

eduardvintila commented 1 year ago

Thanks a lot, @John-Ted - great catch! I have now changed the ioctl signature to match the latest changes in Unikraft core. app-sqlite and the other complex apps should compile now 👍🏻

John-Ted commented 1 year ago

Getting the following warnings when compiling sqlite. Are they relevant? Functionality does not appear to be affected. I did not find any other issues.

In file included from /home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/libs/lib-newlib/musl-imported/src/network/htonl.c:2:
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/libs/lib-newlib/musl-imported/include/byteswap.h: In function ‘__bswap_32’:
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/libs/lib-newlib/musl-imported/include/byteswap.h:14:32: warning: suggest parentheses around arithmetic in operand of ‘|’ [-Wparentheses]
   14 |         return __x>>24 | __x>>8&0xff00 | __x<<8&0xff0000 | __x<<24;
      |                          ~~~~~~^~~~~~~
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/libs/lib-newlib/musl-imported/include/byteswap.h:14:48: warning: suggest parentheses around arithmetic in operand of ‘|’ [-Wparentheses]
   14 |         return __x>>24 | __x>>8&0xff00 | __x<<8&0xff0000 | __x<<24;
      |                                          ~~~~~~^~~~~~~~~
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/libs/lib-newlib/musl-imported/include/byteswap.h: In function ‘__bswap_64’:
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/libs/lib-newlib/musl-imported/include/byteswap.h:19:31: warning: suggest parentheses around ‘+’ inside ‘<<’ [-Wparentheses]
   19 |         return __bswap_32(__x)+0ULL<<32 | __bswap_32(__x>>32);
      |                ~~~~~~~~~~~~~~~^~~~~
  CC      libsqlite: sqlite3.o
In file included from /home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/libs/lib-newlib/musl-imported/src/network/htons.c:2:
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/libs/lib-newlib/musl-imported/include/byteswap.h: In function ‘__bswap_32’:
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/libs/lib-newlib/musl-imported/include/byteswap.h:14:32: warning: suggest parentheses around arithmetic in operand of ‘|’ [-Wparentheses]
   14 |         return __x>>24 | __x>>8&0xff00 | __x<<8&0xff0000 | __x<<24;
      |                          ~~~~~~^~~~~~~
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/libs/lib-newlib/musl-imported/include/byteswap.h:14:48: warning: suggest parentheses around arithmetic in operand of ‘|’ [-Wparentheses]
   14 |         return __x>>24 | __x>>8&0xff00 | __x<<8&0xff0000 | __x<<24;
      |                                          ~~~~~~^~~~~~~~~
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/libs/lib-newlib/musl-imported/include/byteswap.h: In function ‘__bswap_64’:
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/libs/lib-newlib/musl-imported/include/byteswap.h:19:31: warning: suggest parentheses around ‘+’ inside ‘<<’ [-Wparentheses]
   19 |         return __bswap_32(__x)+0ULL<<32 | __bswap_32(__x>>32);
      |                ~~~~~~~~~~~~~~~^~~~~
  CC      libcontext: ctx.isr.o
In file included from /home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/libs/lib-newlib/musl-imported/src/network/ntohl.c:2:
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/libs/lib-newlib/musl-imported/include/byteswap.h: In function ‘__bswap_32’:
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/libs/lib-newlib/musl-imported/include/byteswap.h:14:32: warning: suggest parentheses around arithmetic in operand of ‘|’ [-Wparentheses]
   14 |         return __x>>24 | __x>>8&0xff00 | __x<<8&0xff0000 | __x<<24;
      |                          ~~~~~~^~~~~~~
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/libs/lib-newlib/musl-imported/include/byteswap.h:14:48: warning: suggest parentheses around arithmetic in operand of ‘|’ [-Wparentheses]
   14 |         return __x>>24 | __x>>8&0xff00 | __x<<8&0xff0000 | __x<<24;
      |                                          ~~~~~~^~~~~~~~~
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/libs/lib-newlib/musl-imported/include/byteswap.h: In function ‘__bswap_64’:
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/libs/lib-newlib/musl-imported/include/byteswap.h:19:31: warning: suggest parentheses around ‘+’ inside ‘<<’ [-Wparentheses]
   19 |         return __bswap_32(__x)+0ULL<<32 | __bswap_32(__x>>32);
      |                ~~~~~~~~~~~~~~~^~~~~
  CC      libcontext: ectx.isr.o
In file included from /home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/libs/lib-newlib/musl-imported/src/network/ntohs.c:2:
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/libs/lib-newlib/musl-imported/include/byteswap.h: In function ‘__bswap_32’:
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/libs/lib-newlib/musl-imported/include/byteswap.h:14:32: warning: suggest parentheses around arithmetic in operand of ‘|’ [-Wparentheses]
   14 |         return __x>>24 | __x>>8&0xff00 | __x<<8&0xff0000 | __x<<24;
      |                          ~~~~~~^~~~~~~
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/libs/lib-newlib/musl-imported/include/byteswap.h:14:48: warning: suggest parentheses around arithmetic in operand of ‘|’ [-Wparentheses]
   14 |         return __x>>24 | __x>>8&0xff00 | __x<<8&0xff0000 | __x<<24;
      |                                          ~~~~~~^~~~~~~~~
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/libs/lib-newlib/musl-imported/include/byteswap.h: In function ‘__bswap_64’:
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/libs/lib-newlib/musl-imported/include/byteswap.h:19:31: warning: suggest parentheses around ‘+’ inside ‘<<’ [-Wparentheses]
   19 |         return __bswap_32(__x)+0ULL<<32 | __bswap_32(__x>>32);
      |                ~~~~~~~~~~~~~~~^~~~~
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/apps/app-sqlite/build/libsqlite/origin/sqlite-amalgamation-3400100/sqlite3.c: In function ‘dotlockLock’:
/home/teodor/Documents/ac/unikraft/pr_reviews/pr_newlib/apps/app-sqlite/build/libsqlite/origin/sqlite-amalgamation-3400100/sqlite3.c:38961:5: warning: implicit declaration of function ‘utime’; did you mean ‘utimes’? [-Wimplicit-function-declaration]
38961 |     utime(zLockFile, NULL);
      |     ^~~~~
      |     utimes
eduardvintila commented 1 year ago

Getting the following warnings when compiling sqlite. Are they relevant? Functionality does not appear to be affected. I did not find any other issues.

@John-Ted, those warnings are from the upstream code of musl and sqlite. They were already present before. We can try to suppress them, but I don't I think I have time to cover all of them just before this release. Maybe this is a task for a future PR : )

@mariasfiraiala, thanks for testing. app-nginx works on my side, even with your config file. I think the problem has to do with the order of the libraries inside the Makefile. @John-Ted had a similar issue with the pthread-embedded PR.

The lib-pthread-embedded library should be the first in the list, followed by lib-newlib and the rest of the libraries, like so:

LIBS := $(UK_LIBS)/lib-pthread-embedded:$(UK_LIBS)/lib-newlib:$(UK_LIBS)/lib-lwip:$(UK_LIBS)/lib-nginx

This is also mentioned in the README.md file for both lib-nginx and lib-pthread-embedded. Please let me know if this was, indeed, the problem. Thanks!

mariasfiraiala commented 1 year ago

The lib-pthread-embedded library should be the first in the list, followed by lib-newlib and the rest of the libraries

That was it, thanks!

teotiron commented 1 year ago

Tested this with app-sqlite, app-nginx and app-redis and it properly works. Let's merge this.