Beyond the 3 patches, I sent earlier, I have since identified 53 files that are still under libc/ that were committed as part of this commit - https://github.com/cloudius-systems/osv/commit/79d5ad7d6f7bbcc20edf2ce2bbf3cc08f04b08dd and have not changed since then - seem like next set of candidates to point back to musl/:
libc/locale/duplocale.c libc/locale/isalnum_l.c libc/locale/isalpha_l.c libc/locale/isblank_l.c libc/locale/iscntrl_l.c libc/locale/isdigit_l.c libc/locale/isgraph_l.c libc/locale/islower_l.c libc/locale/isprint_l.c libc/locale/ispunct_l.c libc/locale/isspace_l.c libc/locale/isupper_l.c libc/locale/iswalnum_l.c libc/locale/iswalpha_l.c libc/locale/iswblank_l.c libc/locale/iswcntrl_l.c libc/locale/iswctype_l.c libc/locale/iswdigit_l.c libc/locale/iswgraph_l.c libc/locale/iswlower_l.c libc/locale/iswprint_l.c libc/locale/iswpunct_l.c libc/locale/iswspace_l.c libc/locale/iswupper_l.c libc/locale/iswxdigit_l.c libc/locale/isxdigit_l.c libc/locale/langinfo.c libc/locale/nl_langinfo_l.c libc/locale/setlocale.c libc/locale/strcoll.c libc/locale/strcoll_l.c libc/locale/strfmon.c libc/locale/strftime_l.c libc/locale/strxfrm.c libc/locale/strxfrm_l.c libc/locale/tolower_l.c libc/locale/towlower_l.c libc/locale/towupper_l.c libc/locale/uselocale.c libc/locale/wcscoll.c libc/locale/wcscoll_l.c libc/locale/wcsxfrm.c libc/locale/wcsxfrm_l.c libc/locale/wctype_l.c libc/time/__asctime.c libc/time/gmtime.c libc/time/gmtime_r.c libc/time/localtime.c libc/time/localtime_r.c libc/time/mktime.c libc/time/strftime.c libc/time/timegm.c libc/time/wcsftime.c I believe these are copies from the older musl version. Waldek On Monday, July 27, 2020 at 12:42:27 AM UTC-4 Waldek Kozaczuk wrote: > Based on some conversations I have found on the list, I think we should > first clean libc/ directory from stuff that could come directly from musl > and ideally minimize/reorganize these changes maybe by moving changes to > separate files. > > Here is what I have found so far - these ones from libc/string/ that > differ only with #undef (seems like Avi made those changes - > https://github.com/cloudius-systems/osv/commit/0ac3dd155ac0a8f4606fcb9abf3c7785da395214 > ): > > grep undef libc/string/* > libc/string/strchr.c:#undef strchr > libc/string/strcmp.c:#undef strcmp > libc/string/strcspn.c:#undef strcspn > libc/string/strdup.c:#undef __strdup > libc/string/strncmp.c:#undef strncmp > libc/string/strpbrk.c:#undef strpbrk > libc/string/strsep.c:#undef strsep > libc/string/strspn.c:#undef strspn > libc/string/strtok_r.c:#undef strtok_r > > I have also noticed some files with differences like this: > diff musl/src/string/strlcpy.c ./libc/string/strlcpy.c > 10c10 > < #define HASZERO(x) ((x)-ONES & ~(x) & HIGHS) > --- > > #define HASZERO(x) (((x)-ONES) & ~(x) & HIGHS) > > or > diff ./musl/src/string/wmemcpy.c libc/string/wmemcpy.c > 4c4 > < wchar_t *wmemcpy(wchar_t *restrict d, const wchar_t *restrict s, size_t > n) > --- > > wchar_t *wmemcpy(wchar_t *__restrict d, const wchar_t *__restrict s, > size_t n) > > Some have both. I wonder if these changes are necessary. > > Finally, I was wondering which version of musl we should target - 1.1.24 > (the last one from 1.1* series and end of life) or 1.2.0 which is the > newest one - http://musl.libc.org/releases.html. > > On Sunday, July 26, 2020 at 10:48:11 AM UTC-4, Nadav Har'El wrote: >> >> >> On Fri, Jul 24, 2020 at 12:02 AM Waldek Kozaczuk <[email protected]> >> wrote: >> >>> Hi, >>> >>> As I have been researching what it would take to add floating-point >>> support to the AArch64 port >>> >> >> What is the problem with aarch64 floating point, and why is it related to >> musl? >> >> Is it just the esoteric functions like feenableexcept() and fegetenv() >> you mentioned - or are somehow all the math functions broken? >> Does the normal floating point - besides those functions - work correctly? >> >> Note that OSv uses floating point in the kernel (mostly for calculations >> in the scheduler...) and in x86 we had a lot of grief coming >> from this, needing to save the floating point state on context switches >> and things like that - I wonder if there isn't similar problems >> in aarch64 which have nothing to do with Musl. >> >> >> >>> , I started realizing that it might be better (not necessarily easier) >>> to upgrade musl. >>> >> >> This is definitely a good idea, but won't be trivial. I think I once >> wrote an outline of how this could be done, but I don't remember where I >> wrote it :-) >> > I think this might be your plan: > "1. Bring the latest Musl version into a "musl-1.1.5" subdirectory in OSv. > 2. Leave the old "musl/" and "libc/" directories as well. > 3. In libc/build.mk & build.mk, make a new list of objects (say, "nmusl") > which will take code from musl-1.1.5/ instead of musl/. > 4. Start to switch individual files and directories from "musl += ..." to > "nmusl += ...". We can start with the math functions which Claudio needed. > Eventually everything can be converted to nmusl and hopefully nothing or > little will break and need to be fixed. > 5. Replace our include/api with musl-1.1.5/include. Would be even better > to drop include/api, and just use musl-1.1.5/include directly if we can. I > think, though, this is low priority, and might take quite a bit of work > ("git log include/api" shows we modified this quite a bit since we took it > from musl). I'm hoping that the old header files will work correctly also > for the newer musl." > from this conversation - > https://groups.google.com/g/osv-dev/c/9gc1WQE5yqk/m/hzrjOD-qj88J. > > I think this one applies as well - > https://groups.google.com/g/osv-dev/c/WYtzG1ST0RE/m/MRlP0KNS1WcJ. Claudio > wrote this back then: > "Newer musl has support for ld128, which is needed for aarch64." > >> >> The short story is that we already have in libc/ various files copied >> from an old version of musl which we changed for various reasons, and may >> or may not need to make similar changes to the newer musl. See commits >> 5a1d4c2451a94809d1ad580272410d8c51e5171b >> 51722a9552122cbd834329f4c2f13017f353855a >> 0b651428b91663255d8da9a4913663d0cd4cc710 as recent examples of things that >> caused us to change things in Musl. >> >> If you have the energy to work on this Musl update, I agree that it's a >> good idea. >> Also this time around we should try to make even more of an effort not to >> modify or fork Musl source files as much as we can. >> >> >>> At first, I hoped I could just add fegetenv support ( >>> https://linux.die.net/man/3/fegetenv) by adding fenv.S from the latest >>> musl. But very soon I have discovered we need to support LDBL_MANT_DIG=113 >>> (please see comments in include/api/aarch64/bits/float.h) which triggers >>> many missing symbols for LDBL_MANT_DIG == 113 (see also >>> musl/src/math/__invtrigl.* for example) which I added manually from latest >>> musl (would really require moving it to libc) but then there are 10 or so >>> other math-related files like musl/src/math/lgammal.c and vfprintf.c-like >>> ones that also need updates. And this is just for floating-point support >>> >> >> I'm afraid I don't understand the details here - does floating point >> *without* these math functions already work? >> >> >>> (no signals, setjmp, and other goodies we also need to support AArch64 >>> and might get from musl as well). >>> >>> This led me to question this approach and realize that it might be >>> better to simply upgrade musl to the latest version which has the latest >>> AArch64 support plus all bugs fixed in the last 7 years. >>> >>> So I was wondering what you think about my pretty controversial idea? >>> >>> Obviously it is not a small effort and somewhat risky as things might >>> break (what is a chance of that? any concrete examples?). But I think it is >>> necessary and there is some concrete upside to that. >>> >> >> The risk is that *everything* might break ;-) Every place where we have a >> file in libc/ instead of musl/, it was because we had a problem with the >> musl code and had to work around it to fix some bug - and you'll need to >> watch out not to recreate the same original bug. Hopefully, running the >> tests will find many of these problems. >> >> >>> >>> Obviously we cannot simply update musl git subproject to the latest >>> version as a significant number of files have been changed since and moved >>> to libc subdirectory (or also somewhere else?) or were manually added from >>> whatever version of musl at some point. So here is a high-level recipe on >>> how we could do it: >>> >>> For each *.c|*.cc file (and probably headers) under libc/ find >>> corresponding .c file under musl/src (sometimes the file will be .cc like >>> libc/misc/getopt.cc where we had to add C++ code): >>> >>> - if file not found under musl nothing has to be done (correct?) >>> - otherwise, find a delta between the two and apply to the copy in >>> new musl (this would be in most cases manual step, as very often the old >>> file in musl would have changed since 2013) >>> >>> >> Yes. Note that to complicate things even further, some of these modified >> musl source files actually did *not come* from the version of Musl we >> have in musl/, but rather an *older *version. So some of these >> differences you will see are not deliberate differences we did, but rather >> actually a few months of Musl evolution that has nothing to do with OSv. >> You can "git blame" / "git log" those libc files to see what changes we >> *really* did to them, and *why*. >> >> >>> As I understand many files under libc/ never came from musl - some were >>> written from scratch (see libc/mman.cc) and some may have come from >>> different place (see libc/locale/*_l.c which I think delegate to musl). >>> >> >> Right. Some of it we wrote ourselves. In particular, any musl function >> whose main function is to call some system call (like the mmap() stuff in >> mman.cc, posix threads stuff, etc.), doesn't make sense in OSv and we wrote >> a completely different implementation. >> >> >>> >>> Anything I have missed? >>> >>> How do we test all this? Besides unit tests we also have this - >>> https://github.com/cloudius-systems/osv/wiki/Automated-Testing-Framework - >>> which allows us to test OSv against 30+ apps. >>> >>> Do you have any suggestions for the strategy? Maybe something that would >>> make future upgrades easier? What about reviewing the changes - how to make >>> easier to review? Maybe a separate branch? Any step by step approach ideas? >>> >> >> You can start by renaming musl/ to musl-old/ (note that it's a submodule, >> not a directory!) and create a new musl/ submodule pointing to a recent >> release of musl, and then start to handle the fallout from this change... >> You'll probably need to change things to even get these new source files to >> compile (probably need to change header files, or something) and then start >> to think about all the files we copied to libc/, whether we can get rid of >> some of them (e.g., when a big was also fixed in Musl, or maybe you can >> upstream our changes!), and if not, apply our changes to the new version >> instead of the ancient version. >> >> >>> >>> I think that Geraldo Netto started working on this at some point, but I >>> am not sure where he left it off. >>> >>> Waldek >>> >>> PS. I am enclosing a file that is an approximation of a diff between >>> libc and musl/src directories which I achieved more-less in this way: >>> >> >> Please note that as I explained above, some of the files in libc/ were >> actually taken from an older musl than you see in musl/src, so some of >> the differences aren't real changes we really need, but just some >> spurious changes between the two versions in musl. Sometimes it's the >> combination >> of the two - we kept a file in libc/ because it had some changes we >> needed, but it also has some spurious changes vs. the version in musl/src, >> which >> we didn't really need. >> >> > find libc/ -type f -name \*c > /tmp/libc >>> >>> #remove libc prefix >>> >>> find_diff (which is:) >>> >>> ------ >>> >>> #!/bin/bash >>> >>> execute_diff() >>> >>> { >>> >>> local NAME="$1" >>> >>> echo "-----------------------------------------" >>> >>> echo "DIFF: $NAME" >>> >>> diff musl/src$NAME libc$NAME 2>&1 >>> >>> } >>> >>> export -f execute_diff >>> >>> cat /tmp/libc2 | xargs -I{} bash -c "execute_diff {}" >>> >>> -- >>> You received this message because you are subscribed to the Google >>> Groups "OSv Development" group. >>> >> To unsubscribe from this group and stop receiving emails from it, send an >>> email to [email protected]. >> >> >>> To view this discussion on the web visit >>> https://groups.google.com/d/msgid/osv-dev/d61435ae-dc51-48a4-938b-e77eb7f2725an%40googlegroups.com >>> >>> <https://groups.google.com/d/msgid/osv-dev/d61435ae-dc51-48a4-938b-e77eb7f2725an%40googlegroups.com?utm_medium=email&utm_source=footer> >>> . >>> >> -- You received this message because you are subscribed to the Google Groups "OSv Development" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/osv-dev/fe21233f-c410-44eb-a18c-b474f6019db0n%40googlegroups.com.
