On Mon, May 25, 2020 at 7:31 AM Rick Payne <[email protected]> wrote:
> > I think this is also related to 913. Indeed! :-) I had a sense of deja-vu with this memset() loop, and dug up the patch I wrote back then and just updated it. > I'll try without my patch (which > we've been having to use since then). > Do you mean https://github.com/cloudius-systems/osv/commit/d52cb12546ff2acd5255a2ac8897891e421f07dc which just turned off optimization? At the time, I suggested the memset() fix for #913, because seemed to me like an "obvious" caused for infinite recursion (which happened now in Fedora 32). But you said you tested and it didn't help - in your case you saw that memset() wasn't even used (you commented it out and everything still compiled fine). So I guess we can't revert the turn-off-optimization patch. Maybe it's not actually needed for modern compilers (you can check...) but it will still be needed for the specific compiler which caused you problem #913 in the first place. > > Rick > > On Sat, 2020-05-23 at 23:47 +0300, Nadav Har'El wrote: > > Some compilers apparently optimize code in fastlz/ to call memset(), > > so > > the uncompressing boot loader, fastlz/lzloader.cc, needs to implement > > this function. The current implementation called the "builtin" > > memset, > > which, if you look at the compilation result, actually calls memset() > > and results in endless recursion and a hanging boot... This started > > happening on Fedora 32 with Gcc 10, for example. > > > > So let's implement memset() using the base_memset() we already have > > in > > libc/string/memset.c. > > > > Fixes #1084. > > > > Signed-off-by: Nadav Har'El <[email protected]> > > --- > > fastlz/lzloader.cc | 15 ++++++++++----- > > 1 file changed, 10 insertions(+), 5 deletions(-) > > > > diff --git a/fastlz/lzloader.cc b/fastlz/lzloader.cc > > index f65fb2be..7eae2191 100644 > > --- a/fastlz/lzloader.cc > > +++ b/fastlz/lzloader.cc > > @@ -21,11 +21,16 @@ extern char _binary_loader_stripped_elf_lz_start; > > extern char _binary_loader_stripped_elf_lz_end; > > extern char _binary_loader_stripped_elf_lz_size; > > > > -// std libraries used by fastlz. > > -extern "C" void *memset(void *s, int c, size_t n) > > -{ > > - return __builtin_memset(s, c, n); > > -} > > +// The code in fastlz.cc does not call memset(), but some version of > > gcc > > +// implement some assignments by calling memset(), so we need to > > implement > > +// a memset() function. This is not performance-critical so let's > > stick to > > +// the basic implementation we have in libc/string/memset.c. To > > avoid > > +// compiling this source file a second time (the loader needs > > different > > +// compile parameters), we #include it here instead. > > +extern "C" void *memset(void *s, int c, size_t n); > > +#define memset_base memset > > +#include "libc/string/memset.c" > > +#undef memset_base > > > > extern "C" void uncompress_loader() > > { > > -- > > 2.26.2 > > > > -- > 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/d18a6dd80dcdad104b8ed7cc0348e0ebc36dfd19.camel%40rossfell.co.uk > . > -- 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/CANEVyjsXrWL_BcEP7gGKOWGtMaPXFY43AvZKO%2BtjrTBj3A295w%40mail.gmail.com.
