[PATCH 1/3] memset_base(): avoid warnings in stricter C++ compiler

[Nadav Har'El]

If we try to compile memset.c with a C++ compiler, we get warnings on
various implied pointer conversions. Let's make them explict, and this
way the code can compile as either C or C++ code.

I want to be able to compile it as C++ code because in the next patch
I want to use it in fastlz/lzloader.cc.

Signed-off-by: Nadav Har'El <n...@scylladb.com>
---
libc/string/memset.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libc/string/memset.c b/libc/string/memset.c
index 0d7f1c9d..c42d00cf 100644
--- a/libc/string/memset.c
+++ b/libc/string/memset.c
@@ -9,13 +9,13 @@

void *memset_base(void *dest, int c, size_t n)
{
- unsigned char *s = dest;
+ unsigned char *s = (unsigned char *)dest;
c = (unsigned char)c;
for (; ((uintptr_t)s & ALIGN) && n; n--) *s++ = c;
if (n) {
size_t *w, k = ONES * c;
- for (w = (void *)s; n>=SS; n-=SS, w++) *w = k;
- for (s = (void *)w; n; n--, s++) *s = c;
+ for (w = (size_t *)s; n>=SS; n-=SS, w++) *w = k;
+ for (s = (unsigned char *)w; n; n--, s++) *s = c;
}
return dest;
}
--
2.26.2

[Nadav Har'El]

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 <n...@scylladb.com>
---

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

[Nadav Har'El]

After the previous patches, OSv now correctly builds on Fedora 32.
So support Fedora 32 in setup.py. The same packages needed in previous Fedora
releases are also needed in Fedora 32.

Signed-off-by: Nadav Har'El <n...@scylladb.com>
---

scripts/setup.py | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/scripts/setup.py b/scripts/setup.py
index 14f276d3..13062c58 100755
--- a/scripts/setup.py
+++ b/scripts/setup.py
@@ -119,7 +119,14 @@ class Fedora(object):
ec2_post_install = None
version = '31'

- versions = [Fedora_25, Fedora_26, Fedora_27, Fedora_28, Fedora_29, Fedora_30, Fedora_31]
+ class Fedora_32(object):
+ packages = []
+ ec2_packages = []
+ test_packages = []
+ ec2_post_install = None
+ version = '32'
+
+ versions = [Fedora_25, Fedora_26, Fedora_27, Fedora_28, Fedora_29, Fedora_30, Fedora_31, Fedora_32]

class RHELbased(Fedora):
name = ['Scientific Linux', 'NauLinux', 'CentOS Linux', 'Red Hat Enterprise Linux', 'Oracle Linux']
--
2.26.2

[Commit Bot]

From: Nadav Har'El <n...@scylladb.com>
Committer: Nadav Har'El <n...@scylladb.com>
Branch: master

memset_base(): avoid warnings in stricter C++ compiler

If we try to compile memset.c with a C++ compiler, we get warnings on
various implied pointer conversions. Let's make them explict, and this
way the code can compile as either C or C++ code.

I want to be able to compile it as C++ code because in the next patch
I want to use it in fastlz/lzloader.cc.

Signed-off-by: Nadav Har'El <n...@scylladb.com>

---

diff --git a/libc/string/memset.c b/libc/string/memset.c

[Commit Bot]

From: Nadav Har'El <n...@scylladb.com>
Committer: Nadav Har'El <n...@scylladb.com>
Branch: master

lzloader: fix memset() implementation

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 <n...@scylladb.com>

---

diff --git a/fastlz/lzloader.cc b/fastlz/lzloader.cc

[Commit Bot]

From: Nadav Har'El <n...@scylladb.com>
Committer: Nadav Har'El <n...@scylladb.com>
Branch: master

setup.py: support Fedora 32

After the previous patches, OSv now correctly builds on Fedora 32.
So support Fedora 32 in setup.py. The same packages needed in previous Fedora
releases are also needed in Fedora 32.

Signed-off-by: Nadav Har'El <n...@scylladb.com>

---

diff --git a/scripts/setup.py b/scripts/setup.py
--- a/scripts/setup.py
+++ b/scripts/setup.py
@@ -119,7 +119,14 @@ class Fedora_31(object):

[Waldek Kozaczuk]

Great investigative work! Now we support latest Ubuntu and Fedora.

Sent from my iPhone

> On May 23, 2020, at 15:47, Nadav Har'El <n...@scylladb.com> wrote:
>
> After the previous patches, OSv now correctly builds on Fedora 32.

> --
> 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 osv-dev+u...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/osv-dev/20200523204734.71598-3-nyh%40scylladb.com.

[Rick Payne]

I think this is also related to 913. I'll try without my patch (which
we've been having to use since then).

Rick

[Nadav Har'El]

On Mon, May 25, 2020 at 7:31 AM Rick Payne <ri...@rossfell.co.uk> 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.

 

--
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 osv-dev+u...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/osv-dev/d18a6dd80dcdad104b8ed7cc0348e0ebc36dfd19.camel%40rossfell.co.uk.

[Rick Payne]

On Mon, 2020-05-25 at 09:15 +0300, Nadav Har'El wrote:
>
> Do you mean
> https://github.com/cloudius-systems/osv/commit/d52cb12546ff2acd5255a2ac8897891e421f07dc
> which just turned off optimization?

Yes.

> 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).

Right, I recall at the time that the memset fix looked like it should
do it, but didn't.

> 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.

I suspect it was a compiler issue, as I can't reproduce it on Ubuntu
19.10, using gcc 9.2.1. Compiling that file with -O2 works for me now.

Rick