[PATCH 00/12] Fixes to get OSv to build on Fedora 34

[Nadav Har'El]

Here is an eclectic collection of patches needed to get OSv to compile
on Fedora 34. Most of the patches are about changes in gcc 11.

After this series "make" and "scripts/build image=rogue; scripts/run.py"
work, but the default Lua shell (scripts/build) and the tests (make
check) still don't work and will need more patches.

Nadav Har'El (12):
fix compatibility features.h for Fedora 34
Fix __cxa_finalize() signature
bsd/sha2.c: fix mismatched signatures
Fix warnings about misleading indentation
xen: fix overread and overwrite bug
Makefile: avoid warning in one Musl source file
fs/vfs/main.cc: fix gcc 11 build error
core/trace.cc: free aligned_alloc() with free()
sched: free aligned_alloc() with free()
core/mmu.cc: silence warning
add __libc_single_threaded symbol
scripts/setup.py: add Fedora 34

scripts/setup.py | 9 +++-
include/osv/sched.hh | 13 ++++++
bsd/sys/net/if.cc | 4 +-
bsd/sys/netinet/in_mcast.cc | 3 +-
bsd/sys/netinet/tcp_syncache.cc | 3 +-
bsd/sys/xen/evtchn.cc | 77 +++++++++++++++++----------------
bsd/sys/xen/xenbus/xenbus.cc | 16 +++----
core/mmu.cc | 4 ++
core/sched.cc | 12 ++---
core/trace.cc | 6 +--
fs/vfs/main.cc | 6 +--
runtime.cc | 18 ++++++--
Makefile | 1 +
bsd/sys/crypto/sha2/sha2.c | 12 ++---
bsd/sys/xen/evtchn.h | 5 ++-
include/glibc-compat/features.h | 5 +++
16 files changed, 119 insertions(+), 75 deletions(-)

--
2.31.1

[Nadav Har'El]

Recently (for example in Fedora 34), the C++ header files started to
include the system's <single_threaded.h>, and that include <features.h>
and assumes it is enough to define __BEGIN_DECLS. However, this macro
is actually defined in <sys/cdefs.h>, so we get many build errors like:

/usr/include/sys/single_threaded.h:24:1: error: ‘__BEGIN_DECLS’ does not name a type
24 | __BEGIN_DECLS
| ^~~~~~~~~~~~~

The fix is to include <sys/cdefs.h> from our compatibility <features.h>.
This is what glibc does too.

Signed-off-by: Nadav Har'El <n...@scylladb.com>
---
include/glibc-compat/features.h | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/include/glibc-compat/features.h b/include/glibc-compat/features.h
index 7f5c4ee7..cccae9c5 100644
--- a/include/glibc-compat/features.h
+++ b/include/glibc-compat/features.h
@@ -10,6 +10,11 @@

#include_next <features.h>

+/* Recently, some system header files started to rely on <features.h>
+ * already including <sys/cdefs.h>. So we need to do it.
+ */
+#include <sys/cdefs.h>
+
#define hidden __attribute__((__visibility__("hidden")))

#define __GNU_LIBRARY__ 6
--
2.31.1

[Nadav Har'El]

Starting in gcc 11, <cxxabi.h> declares the __cxa_finalize() function
to return void, not an int as it was previously. The change has no
practical importance (our implementation always returned 0 anyway, there
was never any interesting integer to returned). But to get our
implementation to compile against both gcc-11 and pre-gcc-11's version
of cxxabi.h, we need to hide cxxabi.h's declaration.

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

runtime.cc | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/runtime.cc b/runtime.cc
index 25ef7ea2..d5b471b4 100644
--- a/runtime.cc
+++ b/runtime.cc
@@ -11,7 +11,6 @@
#include <cstring>
#include <string.h>
#include <exception>
-#include <cxxabi.h>
#include <sys/mman.h>
#include <unistd.h>
#include <link.h>
@@ -62,6 +61,14 @@
#include <pty.h>
#include <osv/pid.h>

+// cxxabi.h from gcc 10 and earlier used to say that __cxa_finalize returns
+// an int, while it should return void (and does so on gcc 11). To allow us
+// to define __cxa_finalize with neither gcc 10 or 11 complaining, we need
+// to hide the declaration in the header file
+#define __cxa_finalize __cxa_finalize_ignore
+#include <cxxabi.h>
+#undef __cxa_finalize
+
#define __LC_LAST 13

#define __ALIAS(old, new) \
@@ -169,11 +176,11 @@ int __cxa_atexit(destructor_t destructor, void *arg, void *dso)
return 0;
}

-int __cxa_finalize(void *dso)
+void __cxa_finalize(void *dso)
{
if (!dso || dso == &__dso_handle) {
debug("__cxa_finalize() running kernel's destructors not supported\n");
- return 0;
+ return;
}
std::vector<std::pair<destructor_t,void*>> my_destructors;
WITH_LOCK(destructors_mutex) {
@@ -183,7 +190,7 @@ int __cxa_finalize(void *dso)
for (auto d : boost::adaptors::reverse(my_destructors)) {
d.first(d.second);
}
- return 0;
+ return;
}
}

--
2.31.1

[Nadav Har'El]

Gcc 11 found several places where the code has indentation which may
hint of a bug. For example:

if (something)
onething;
anotherthing;

In such code, the indentation suggests that the programmer indented
for both things to be done only if something is true - and yet the
lack of braces mean just onething is under the condition, and
anotherhing is run unconditionally!

I fixed the indentation or the code, as it seems to me the intention
was. In some cases it seems gcc found real bugs in the code (however,
these bugs seem in very obscure cases).

This issue reminds us why OSv's coding conventions require that "if"
must always use braces - even for a single statement (most of the
involved code predates this coding convention).

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

x

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

bsd/sys/net/if.cc | 4 ++--
bsd/sys/netinet/in_mcast.cc | 3 ++-
bsd/sys/netinet/tcp_syncache.cc | 3 ++-
3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/bsd/sys/net/if.cc b/bsd/sys/net/if.cc
index 2f2e4cbb..f717da37 100644
--- a/bsd/sys/net/if.cc
+++ b/bsd/sys/net/if.cc
@@ -2624,8 +2624,8 @@ if_allocmulti(struct ifnet *ifp, struct bsd_sockaddr *sa, struct bsd_sockaddr *l
free(ifma);
return (NULL);
}
- if (mflags & M_ZERO)
- bzero(dupsa, sa->sa_len);
+ if (mflags & M_ZERO)
+ bzero(dupsa, sa->sa_len);
bcopy(llsa, dupsa, llsa->sa_len);
ifma->ifma_lladdr = dupsa;

diff --git a/bsd/sys/netinet/in_mcast.cc b/bsd/sys/netinet/in_mcast.cc
index d34bf43f..aadd04b0 100644
--- a/bsd/sys/netinet/in_mcast.cc
+++ b/bsd/sys/netinet/in_mcast.cc
@@ -1001,9 +1001,10 @@ inm_merge(struct in_multi *inm, /*const*/ struct in_mfilter *imf)
/* Decrement ASM listener count on transition out of ASM mode. */
if (imf->imf_st[0] == MCAST_EXCLUDE && nsrc0 == 0) {
if ((imf->imf_st[1] != MCAST_EXCLUDE) ||
- (imf->imf_st[1] == MCAST_EXCLUDE && nsrc1 > 0))
+ (imf->imf_st[1] == MCAST_EXCLUDE && nsrc1 > 0)) {
CTR1(KTR_IGMPV3, "%s: --asm on inm at t1", __func__);
--inm->inm_st[1].iss_asm;
+ }
}

/* Increment ASM listener count on transition to ASM mode. */
diff --git a/bsd/sys/netinet/tcp_syncache.cc b/bsd/sys/netinet/tcp_syncache.cc
index b1fb17c2..16c4fc19 100644
--- a/bsd/sys/netinet/tcp_syncache.cc
+++ b/bsd/sys/netinet/tcp_syncache.cc
@@ -920,10 +920,11 @@ int syncache_expand(struct in_conninfo *inc, struct tcpopt *to,
*/
if ((to->to_flags & TOF_TS) && to->to_tsecr != sc->sc_ts&&
!TOEPCB_ISSET(sc)) {if
-( (s = tcp_log_addrs(inc, th, NULL, NULL)))
+( (s = tcp_log_addrs(inc, th, NULL, NULL))) {
bsd_log(LOG_DEBUG, "%s; %s: TSECR %u != TS %u, "
"segment rejected\n",
s, __func__, to->to_tsecr, sc->sc_ts);
+ }
goto failed;
}

--
2.31.1

[Nadav Har'El]

Gcc 11 gives a warning (which we make into an error) when the signature
of a function declared in the header file uses a type[N] parameter but
then the function is defined with just type[]. It needs to repeat type[N].

We fix several mistakes of this sort in one file, sha2.c.

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

bsd/sys/crypto/sha2/sha2.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/bsd/sys/crypto/sha2/sha2.c b/bsd/sys/crypto/sha2/sha2.c
index b4bee5b2..46468e5b 100644
--- a/bsd/sys/crypto/sha2/sha2.c
+++ b/bsd/sys/crypto/sha2/sha2.c
@@ -553,7 +553,7 @@ void SHA256_Update(SHA256_CTX* context, const sha2_byte *data, size_t len) {
usedspace = freespace = 0;
}

-void SHA256_Final(sha2_byte digest[], SHA256_CTX* context) {
+void SHA256_Final(sha2_byte digest[SHA256_DIGEST_LENGTH], SHA256_CTX* context) {
sha2_word32 *d = (sha2_word32*)digest;
unsigned int usedspace;

@@ -616,7 +616,7 @@ void SHA256_Final(sha2_byte digest[], SHA256_CTX* context) {
usedspace = 0;
}

-char *SHA256_End(SHA256_CTX* context, char buffer[]) {
+char *SHA256_End(SHA256_CTX* context, char buffer[SHA256_DIGEST_STRING_LENGTH]) {
sha2_byte digest[SHA256_DIGEST_LENGTH], *d = digest;
int i;

@@ -916,7 +916,7 @@ static void SHA512_Last(SHA512_CTX* context) {
SHA512_Transform(context, (sha2_word64*)context->buffer);
}

-void SHA512_Final(sha2_byte digest[], SHA512_CTX* context) {
+void SHA512_Final(sha2_byte digest[SHA512_DIGEST_LENGTH], SHA512_CTX* context) {
sha2_word64 *d = (sha2_word64*)digest;

/* Sanity check: */
@@ -945,7 +945,7 @@ void SHA512_Final(sha2_byte digest[], SHA512_CTX* context) {
bzero(context, sizeof(*context));
}

-char *SHA512_End(SHA512_CTX* context, char buffer[]) {
+char *SHA512_End(SHA512_CTX* context, char buffer[SHA512_DIGEST_STRING_LENGTH]) {
sha2_byte digest[SHA512_DIGEST_LENGTH], *d = digest;
int i;

@@ -991,7 +991,7 @@ void SHA384_Update(SHA384_CTX* context, const sha2_byte* data, size_t len) {
SHA512_Update((SHA512_CTX*)context, data, len);
}

-void SHA384_Final(sha2_byte digest[], SHA384_CTX* context) {
+void SHA384_Final(sha2_byte digest[SHA384_DIGEST_LENGTH], SHA384_CTX* context) {
sha2_word64 *d = (sha2_word64*)digest;

/* Sanity check: */
@@ -1020,7 +1020,7 @@ void SHA384_Final(sha2_byte digest[], SHA384_CTX* context) {
bzero(context, sizeof(*context));
}

-char *SHA384_End(SHA384_CTX* context, char buffer[]) {
+char *SHA384_End(SHA384_CTX* context, char buffer[SHA384_DIGEST_STRING_LENGTH]) {
sha2_byte digest[SHA384_DIGEST_LENGTH], *d = digest;
int i;

--
2.31.1

[Nadav Har'El]

Gcc 11 detected a bug in many places using the HYPERVISOR_event_channel_op
function was used. This function works on many types of operations, so
assumes it can read and write the maximal size of the operation's
parameters - 24 bytes. And yet, it was called for smaller, specific,
operation object.

This patch fixes all the calls to build a maximal-size operation object.

I'm not very happy about this patch - it is only needed for some
situation marked unlikely - where all this problematic copying happens -
but I want to restore the ability to build on Fedora 34 which has gcc
11.

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

bsd/sys/xen/evtchn.cc | 77 ++++++++++++++++++------------------
bsd/sys/xen/xenbus/xenbus.cc | 16 ++++----
bsd/sys/xen/evtchn.h | 5 ++-
3 files changed, 50 insertions(+), 48 deletions(-)

diff --git a/bsd/sys/xen/evtchn.cc b/bsd/sys/xen/evtchn.cc
index 733f0993..52f769fb 100644
--- a/bsd/sys/xen/evtchn.cc
+++ b/bsd/sys/xen/evtchn.cc
@@ -275,38 +275,38 @@ bind_local_port_to_irq(unsigned int local_port, int * port)
int
bind_listening_port_to_irq(unsigned int remote_domain, int * port)
{
- struct evtchn_alloc_unbound alloc_unbound;
+ struct evtchn_op alloc_unbound;
int err;

- alloc_unbound.dom = DOMID_SELF;
- alloc_unbound.remote_dom = remote_domain;
+ alloc_unbound.u.alloc_unbound.dom = DOMID_SELF;
+ alloc_unbound.u.alloc_unbound.remote_dom = remote_domain;

err = HYPERVISOR_event_channel_op(EVTCHNOP_alloc_unbound,
- &alloc_unbound);
+ &alloc_unbound.u);

- return err ? : bind_local_port_to_irq(alloc_unbound.port, port);
+ return err ? : bind_local_port_to_irq(alloc_unbound.u.alloc_unbound.port, port);
}

static int
bind_interdomain_evtchn_to_irq(unsigned int remote_domain,
unsigned int remote_port, int * port)
{
- struct evtchn_bind_interdomain bind_interdomain;
+ struct evtchn_op op;
int err;

- bind_interdomain.remote_dom = remote_domain;
- bind_interdomain.remote_port = remote_port;
+ op.u.bind_interdomain.remote_dom = remote_domain;
+ op.u.bind_interdomain.remote_port = remote_port;

err = HYPERVISOR_event_channel_op(EVTCHNOP_bind_interdomain,
- &bind_interdomain);
+ &op.u);

- return err ? : bind_local_port_to_irq(bind_interdomain.local_port, port);
+ return err ? : bind_local_port_to_irq(op.u.bind_interdomain.local_port, port);
}

static int
bind_virq_to_irq(unsigned int virq, unsigned int cpu, int * port)
{
- struct evtchn_bind_virq bind_virq;
+ struct evtchn_op op;
int evtchn = 0, irq;

mtx_lock_spin(&irq_mapping_update_lock);
@@ -315,11 +315,11 @@ bind_virq_to_irq(unsigned int virq, unsigned int cpu, int * port)
if ((irq = find_unbound_irq()) < 0)
goto out;

- bind_virq.virq = virq;
- bind_virq.vcpu = cpu;
- HYPERVISOR_event_channel_op(EVTCHNOP_bind_virq, &bind_virq);
+ op.u.bind_virq.virq = virq;
+ op.u.bind_virq.vcpu = cpu;
+ HYPERVISOR_event_channel_op(EVTCHNOP_bind_virq, &op.u);

- evtchn = bind_virq.port;
+ evtchn = op.u.bind_virq.port;

evtchn_to_irq[evtchn] = irq;
irq_info[irq] = mk_irq_info(IRQT_VIRQ, virq, evtchn);
@@ -341,7 +341,7 @@ out:
static int
bind_ipi_to_irq(unsigned int ipi, unsigned int cpu, int * port)
{
- struct evtchn_bind_ipi bind_ipi;
+ struct evtchn_op op;
int irq;
int evtchn = 0;

@@ -351,9 +351,9 @@ bind_ipi_to_irq(unsigned int ipi, unsigned int cpu, int * port)
if ((irq = find_unbound_irq()) < 0)
goto out;

- bind_ipi.vcpu = cpu;
- HYPERVISOR_event_channel_op(EVTCHNOP_bind_ipi, &bind_ipi);
- evtchn = bind_ipi.port;
+ op.u.bind_ipi.vcpu = cpu;
+ HYPERVISOR_event_channel_op(EVTCHNOP_bind_ipi, &op.u);
+ evtchn = op.u.bind_ipi.port;

evtchn_to_irq[evtchn] = irq;
irq_info[irq] = mk_irq_info(IRQT_IPI, ipi, evtchn);
@@ -375,15 +375,15 @@ out:
static void
unbind_from_irq(int irq)
{
- struct evtchn_close close;
int evtchn = evtchn_from_irq(irq);
int cpu;

mtx_lock_spin(&irq_mapping_update_lock);

if ((--irq_bindcount[irq] == 0) && VALID_EVTCHN(evtchn)) {
- close.port = evtchn;
- HYPERVISOR_event_channel_op(EVTCHNOP_close, &close);
+ struct evtchn_op op;
+ op.u.close.port = evtchn;
+ HYPERVISOR_event_channel_op(EVTCHNOP_close, &op.u);

switch (type_from_irq(irq)) {
case IRQT_VIRQ:
@@ -799,7 +799,7 @@ pirq_query_unmask(int pirq)
static void
xenpic_pirq_enable_intr(struct intsrc *isrc)
{
- struct evtchn_bind_pirq bind_pirq;
+ struct evtchn_op op;
int evtchn;
unsigned int irq;

@@ -810,11 +810,11 @@ xenpic_pirq_enable_intr(struct intsrc *isrc)
if (VALID_EVTCHN(evtchn))
goto out;

- bind_pirq.pirq = irq;
+ op.u.bind_pirq.pirq = irq;
/* NB. We are happy to share unless we are probing. */
- bind_pirq.flags = probing_irq(irq) ? 0 : BIND_PIRQ__WILL_SHARE;
+ op.u.bind_pirq.flags = probing_irq(irq) ? 0 : BIND_PIRQ__WILL_SHARE;

- if (HYPERVISOR_event_channel_op(EVTCHNOP_bind_pirq, &bind_pirq) != 0) {
+ if (HYPERVISOR_event_channel_op(EVTCHNOP_bind_pirq, &op.u) != 0) {
#ifndef XEN_PRIVILEGED_GUEST
panic("unexpected pirq call");
#endif
@@ -823,7 +823,7 @@ xenpic_pirq_enable_intr(struct intsrc *isrc)
mtx_unlock_spin(&irq_mapping_update_lock);
return;
}
- evtchn = bind_pirq.port;
+ evtchn = op.u.bind_pirq.port;

pirq_query_unmask(irq_to_pirq(irq));

@@ -916,8 +916,9 @@ unmask_evtchn(int port)

/* Slow path (hypercall) if this is a non-local port. */
if (unlikely(irq_is_legacy || (cpu != cpu_from_evtchn(port)))) {
- struct evtchn_unmask unmask = { .port = port };
- (void)HYPERVISOR_event_channel_op(EVTCHNOP_unmask, &unmask);
+ struct evtchn_op op;
+ op.u.unmask.port = port;
+ (void)HYPERVISOR_event_channel_op(EVTCHNOP_unmask, &op.u);
return;
}

@@ -942,8 +943,8 @@ void irq_resume(void)
evtchn_op_t op;
int cpu, pirq, virq, ipi, irq, evtchn;

- struct evtchn_bind_virq bind_virq;
- struct evtchn_bind_ipi bind_ipi;
+ struct evtchn_op op_bind_virq;
+ struct evtchn_op op_bind_ipi;

init_evtchn_cpu_bindings();

@@ -984,10 +985,10 @@ void irq_resume(void)
("irq_info inconsistent"));

/* Get a new binding from Xen. */
- bind_virq.virq = virq;
- bind_virq.vcpu = 0;
- HYPERVISOR_event_channel_op(EVTCHNOP_bind_virq, &bind_virq);
- evtchn = bind_virq.port;
+ op_bind_virq.u.bind_virq.virq = virq;
+ op_bind_virq.u.bind_virq.vcpu = 0;
+ HYPERVISOR_event_channel_op(EVTCHNOP_bind_virq, &op_bind_virq.u);
+ evtchn = op_bind_virq.u.bind_virq.port;

/* Record the new mapping. */
evtchn_to_irq[evtchn] = irq;
@@ -1007,9 +1008,9 @@ void irq_resume(void)

/* Get a new binding from Xen. */
memset(&op, 0, sizeof(op));
- bind_ipi.vcpu = 0;
- HYPERVISOR_event_channel_op(EVTCHNOP_bind_ipi, &bind_ipi);
- evtchn = bind_ipi.port;
+ op_bind_ipi.u.bind_ipi.vcpu = 0;
+ HYPERVISOR_event_channel_op(EVTCHNOP_bind_ipi, &op_bind_ipi.u);
+ evtchn = op_bind_ipi.u.bind_ipi.port;

/* Record the new mapping. */
evtchn_to_irq[evtchn] = irq;
diff --git a/bsd/sys/xen/xenbus/xenbus.cc b/bsd/sys/xen/xenbus/xenbus.cc
index c92df112..9a166f00 100644
--- a/bsd/sys/xen/xenbus/xenbus.cc
+++ b/bsd/sys/xen/xenbus/xenbus.cc
@@ -225,32 +225,32 @@ xenbus_grant_ring(device_t dev, unsigned long ring_mfn, grant_ref_t *refp)
int
xenbus_alloc_evtchn(device_t dev, evtchn_port_t *port)
{
- struct evtchn_alloc_unbound alloc_unbound;
+ struct evtchn_op op;
int err;

- alloc_unbound.dom = DOMID_SELF;
- alloc_unbound.remote_dom = xenbus_get_otherend_id(dev);
+ op.u.alloc_unbound.dom = DOMID_SELF;
+ op.u.alloc_unbound.remote_dom = xenbus_get_otherend_id(dev);

err = HYPERVISOR_event_channel_op(EVTCHNOP_alloc_unbound,
- &alloc_unbound);
+ &op.u);

if (err) {
xenbus_dev_fatal(dev, -err, "allocating event channel");
return (-err);
}
- *port = alloc_unbound.port;
+ *port = op.u.alloc_unbound.port;
return (0);
}

int
xenbus_free_evtchn(device_t dev, evtchn_port_t port)
{
- struct evtchn_close close;
+ struct evtchn_op op;
int err;

- close.port = port;
+ op.u.close.port = port;

- err = HYPERVISOR_event_channel_op(EVTCHNOP_close, &close);
+ err = HYPERVISOR_event_channel_op(EVTCHNOP_close, &op.u);
if (err) {
xenbus_dev_error(dev, -err, "freeing event channel %d", port);
return (-err);
diff --git a/bsd/sys/xen/evtchn.h b/bsd/sys/xen/evtchn.h
index 6d754b3e..e9bddd72 100644
--- a/bsd/sys/xen/evtchn.h
+++ b/bsd/sys/xen/evtchn.h
@@ -57,8 +57,9 @@ clear_evtchn(int port)
inline void
notify_remote_via_evtchn(int port)
{
- struct evtchn_send send = { .port = port };
- (void)HYPERVISOR_event_channel_op(EVTCHNOP_send, &send);
+ struct evtchn_op send;
+ send.u.send.port = port;
+ (void)HYPERVISOR_event_channel_op(EVTCHNOP_send, &send.u);
}

/*
--
2.31.1

[Nadav Har'El]

gcc 11 warns for musl/src/prng/seed48.c that seed48()'s signature in
the header file has a 3-char array, but the implementation has a char
pointer. This is correct, but gcc now warns about such inconsistencies,
and we turn those warnings into errors. Because this is an original
Musl source file that we don't want to needlessly change, let's just
avoid the warning with CFLAGS += -Wno-array-parameter for this specific
file.

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

Makefile | 1 +
1 file changed, 1 insertion(+)

diff --git a/Makefile b/Makefile
index f33b1cdb..7ba76820 100644
--- a/Makefile
+++ b/Makefile
@@ -1403,6 +1403,7 @@ musl += prng/lcong48.o
musl += prng/lrand48.o
musl += prng/mrand48.o
musl += prng/seed48.o
+$(out)/musl/src/prng/seed48.o: CFLAGS += -Wno-array-parameter
musl += prng/srand48.o
libc += random.o

--
2.31.1

[Nadav Har'El]

In fs/vfs/main.cc we have bootfs_start, which was defined as a single
character (extern char bootfs_start) but was really treated as the
beginning of a longer memory area. Gcc 11 can recognize that we access
beyond this single character, and fail the compilation.

To work around this problem, we declare bootfs_start as
extern char bootfs_start[]

Little needs to be changed besides this - except changing
&bootfs_start + offset - which gcc doesn't like, to
&bootfs_start[offset].

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

fs/vfs/main.cc | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/vfs/main.cc b/fs/vfs/main.cc
index 71110796..b411da60 100644
--- a/fs/vfs/main.cc
+++ b/fs/vfs/main.cc
@@ -2197,7 +2197,7 @@ struct bootfs_metadata {
char name[BOOTFS_PATH_MAX];
};

-extern char bootfs_start;
+extern char bootfs_start[];

int ramfs_set_file_data(struct vnode *vp, const void *data, size_t size);
void unpack_bootfs(void)
@@ -2223,7 +2223,7 @@ void unpack_bootfs(void)
if (md[i].type == bootfs_file_type::symlink) {
// This is a symbolic link record. The file's content is the
// target path, and we assume ends with a null.
- if (symlink(&bootfs_start + md[i].offset, md[i].name) != 0) {
+ if (symlink(&bootfs_start[md[i].offset], md[i].name) != 0) {
kprintf("couldn't symlink %s: %d\n", md[i].name, errno);
sys_panic("unpack_bootfs failed");
}
@@ -2250,7 +2250,7 @@ void unpack_bootfs(void)
}

struct vnode *vp = fp->f_dentry->d_vnode;
- ret = ramfs_set_file_data(vp, &bootfs_start + md[i].offset, md[i].size);
+ ret = ramfs_set_file_data(vp, &bootfs_start[md[i].offset], md[i].size);
if (ret) {
kprintf("ramfs_set_file_data failed, ret = %d\n", ret);
sys_panic("unpack_bootfs failed");
--
2.31.1

[Nadav Har'El]

Memory allocated with aligned_alloc() should be freed with free(), not
with delete - although in our actual implemention those are really the
same thing. gcc 11 warns about using sched::thread::make() - which uses
align_alloc() - and the deleting it with "delete", using
std::unique_ptr.

Gcc only warns about this issue in sched.cc so we only fix that file,
but note that other places of the code also have unique_ptr<thread>
and should eventually use a similar fix.

The fix is a new method sched::thread::make_unique<> which returns
a std::unique_ptr that holds a thread a knows how to properly delete it.

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

include/osv/sched.hh | 13 +++++++++++++
core/sched.cc | 12 ++++++------
2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/include/osv/sched.hh b/include/osv/sched.hh
index 1b2847be..010c4766 100644
--- a/include/osv/sched.hh
+++ b/include/osv/sched.hh
@@ -428,6 +428,19 @@ public:
}
return new(p) thread(std::forward<Args>(args)...);
}
+ // Since make() doesn't necessarily allocate with "new", dispose() should
+ // be used to free it, not "delete". However, in practice our new and
+ // delete are the same, so delete is fine.
+ static void dispose(thread* p) {
+ p->~thread();
+ free(p);
+ }
+ using thread_unique_ptr = std::unique_ptr<thread, decltype(&thread::dispose)>;
+ template <typename... Args>
+ static thread_unique_ptr make_unique(Args&&... args) {
+ return thread_unique_ptr(make(std::forward<Args>(args)...),
+ thread::dispose);
+ }
private:
explicit thread(std::function<void ()> func, attr attributes = attr(),
bool main = false, bool app = false);
diff --git a/core/sched.cc b/core/sched.cc
index 74b4ab95..52cc5472 100644
--- a/core/sched.cc
+++ b/core/sched.cc
@@ -135,7 +135,7 @@ public:
private:
mutex _mtx;
std::list<thread*> _zombies;
- std::unique_ptr<thread> _thread;
+ thread_unique_ptr _thread;
};

cpu::cpu(unsigned _id)
@@ -552,7 +552,7 @@ void thread::pin(cpu *target_cpu)
// load balancer thread) but a "good-enough" dirty solution is to
// temporarily create a new ad-hoc thread, "wakeme".
bool do_wakeme = false;
- std::unique_ptr<thread> wakeme(thread::make([&] () {
+ thread_unique_ptr wakeme(thread::make_unique([&] () {
wait_until([&] { return do_wakeme; });
t.wake();
}, sched::thread::attr().pin(source_cpu)));
@@ -590,7 +590,7 @@ void thread::pin(thread *t, cpu *target_cpu)
// where the target thread is currently running. We start here a new
// helper thread to follow the target thread's CPU. We could have also
// re-used an existing thread (e.g., the load balancer thread).
- std::unique_ptr<thread> helper(thread::make([&] {
+ thread_unique_ptr helper(thread::make_unique([&] {
WITH_LOCK(irq_lock) {
// This thread started on the same CPU as t, but by now t might
// have moved. If that happened, we need to move too.
@@ -701,7 +701,7 @@ void thread::unpin()
}
return;
}
- std::unique_ptr<thread> helper(thread::make([this] {
+ thread_unique_ptr helper(thread::make_unique([this] {
WITH_LOCK(preempt_lock) {
// helper thread started on the same CPU as "this", but by now
// "this" might migrated. If that happened helper need to migrate.
@@ -973,7 +973,7 @@ thread::thread(std::function<void ()> func, attr attr, bool main, bool app)
, _migration_lock_counter(0)
, _pinned(false)
, _id(0)
- , _cleanup([this] { delete this; })
+ , _cleanup([this] { dispose(this); })
, _app(app)
, _joiner(nullptr)
{
@@ -1600,7 +1600,7 @@ bool operator<(const timer_base& t1, const timer_base& t2)
}

thread::reaper::reaper()
- : _mtx{}, _zombies{}, _thread(thread::make([=] { reap(); }))
+ : _mtx{}, _zombies{}, _thread(thread::make_unique([=] { reap(); }))
{
_thread->start();
}
--
2.31.1

[Nadav Har'El]

Memory allocated with aligned_alloc() should be freed with free(), not
with delete - although in our actual implemention those are really the

same thing. gcc 11 warns about one such case, so instead of silencing
the warning, let's make it use free().

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

core/trace.cc | 6 +++---

1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/core/trace.cc b/core/trace.cc
index 4dd922ad..60fa18b9 100644
--- a/core/trace.cc
+++ b/core/trace.cc
@@ -61,16 +61,16 @@ constexpr size_t trace_page_size = 4096; // need not match arch page size
// Having a struct is more complex than it need be for just per-vcpu buffers,
// _but_ it is in line with later on having rotating buffers, thus wwhy not do it already
struct trace_buf {
- std::unique_ptr<char[]>
+ std::unique_ptr<char[], decltype(&free)>
_base;
size_t _last;
size_t _size;

trace_buf() :
- _base(nullptr), _last(0), _size(0) {
+ _base(nullptr, free), _last(0), _size(0) {
}
trace_buf(size_t size) :
- _base(static_cast<char*>(aligned_alloc(sizeof(long), size))), _last(
+ _base(static_cast<char*>(aligned_alloc(sizeof(long), size)), free), _last(
0), _size(size) {
static_assert(is_power_of_two(trace_page_size), "just checking");
assert(is_power_of_two(size) && "size must be power of two");
--
2.31.1

[Nadav Har'El]

gcc 11 warns about the MMU code writing 4096 byte to an area with 0
bytes. I am guessing this is a false alarm, but the code is too
convoluted for me to prove it :-( So for now, let's just slience this
build error.

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

core/mmu.cc | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/core/mmu.cc b/core/mmu.cc
index cd2cd550..3125a8aa 100644
--- a/core/mmu.cc
+++ b/core/mmu.cc
@@ -29,6 +29,10 @@
#include <osv/rwlock.h>
#include <numeric>

+// FIXME: Without this pragma, we get a lot of warnings that I don't know
+// how to explain or fix. For now, let's just ignore them :-(
+#pragma GCC diagnostic ignored "-Wstringop-overflow"
+
extern void* elf_start;
extern size_t elf_size;

--
2.31.1

[Nadav Har'El]

glibc has this variable (see
https://www.gnu.org/software/libc/manual/html_node/Single_002dThreaded.html)
which some C++ library functions recently started to use, so we need to
define it.

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

runtime.cc | 3 +++
1 file changed, 3 insertions(+)

diff --git a/runtime.cc b/runtime.cc
index d5b471b4..3335552d 100644
--- a/runtime.cc
+++ b/runtime.cc
@@ -641,3 +641,6 @@ char *ctermid(char *s)
*s = 0;
return s;
}
+
+// OSv is always multi-threaded.
+char __libc_single_threaded = 0;
--
2.31.1

[Nadav Har'El]

I'm not yet aware of anything special that needs to be installed for
this version.

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 8b03bfc6..f007c7fb 100755
--- a/scripts/setup.py
+++ b/scripts/setup.py
@@ -125,7 +125,14 @@ class Fedora(object):
ec2_post_install = None
version = '33'

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

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

[Nadav Har'El]

I forgot to mention that I verified that after these changes, OSv still builds on the older Fedora 33 (with gcc 10). Only the __cxa_finalize() patch in took some trickery to work on both.

--
Nadav Har'El

n...@scylladb.com

[Commit Bot]

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

fix compatibility features.h for Fedora 34

Recently (for example in Fedora 34), the C++ header files started to
include the system's <single_threaded.h>, and that include <features.h>
and assumes it is enough to define __BEGIN_DECLS. However, this macro
is actually defined in <sys/cdefs.h>, so we get many build errors like:

/usr/include/sys/single_threaded.h:24:1: error: ‘__BEGIN_DECLS’ does not name a type
24 | __BEGIN_DECLS
| ^~~~~~~~~~~~~

The fix is to include <sys/cdefs.h> from our compatibility <features.h>.
This is what glibc does too.

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

Message-Id: <20210614062057...@scylladb.com>

---
diff --git a/include/glibc-compat/features.h b/include/glibc-compat/features.h

[Commit Bot]

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

Fix __cxa_finalize() signature

Starting in gcc 11, <cxxabi.h> declares the __cxa_finalize() function
to return void, not an int as it was previously. The change has no
practical importance (our implementation always returned 0 anyway, there
was never any interesting integer to returned). But to get our
implementation to compile against both gcc-11 and pre-gcc-11's version
of cxxabi.h, we need to hide cxxabi.h's declaration.

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

Message-Id: <20210614062057...@scylladb.com>

---
diff --git a/runtime.cc b/runtime.cc

[Commit Bot]

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

bsd/sha2.c: fix mismatched signatures

Gcc 11 gives a warning (which we make into an error) when the signature
of a function declared in the header file uses a type[N] parameter but
then the function is defined with just type[]. It needs to repeat type[N].

We fix several mistakes of this sort in one file, sha2.c.

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

Message-Id: <20210614062057...@scylladb.com>

---
diff --git a/bsd/sys/crypto/sha2/sha2.c b/bsd/sys/crypto/sha2/sha2.c

[Commit Bot]

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

Fix warnings about misleading indentation

Gcc 11 found several places where the code has indentation which may
hint of a bug. For example:

if (something)
onething;
anotherthing;

In such code, the indentation suggests that the programmer indented
for both things to be done only if something is true - and yet the
lack of braces mean just onething is under the condition, and
anotherhing is run unconditionally!

I fixed the indentation or the code, as it seems to me the intention
was. In some cases it seems gcc found real bugs in the code (however,
these bugs seem in very obscure cases).

This issue reminds us why OSv's coding conventions require that "if"
must always use braces - even for a single statement (most of the
involved code predates this coding convention).

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

x

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

Message-Id: <20210614062057...@scylladb.com>

---
diff --git a/bsd/sys/net/if.cc b/bsd/sys/net/if.cc

--- a/bsd/sys/net/if.cc
+++ b/bsd/sys/net/if.cc
@@ -2624,8 +2624,8 @@ if_allocmulti(struct ifnet *ifp, struct bsd_sockaddr *sa, struct bsd_sockaddr *l
free(ifma);
return (NULL);
}
- if (mflags & M_ZERO)
- bzero(dupsa, sa->sa_len);
+ if (mflags & M_ZERO)
+ bzero(dupsa, sa->sa_len);
bcopy(llsa, dupsa, llsa->sa_len);
ifma->ifma_lladdr = dupsa;

diff --git a/bsd/sys/netinet/in_mcast.cc b/bsd/sys/netinet/in_mcast.cc

--- a/bsd/sys/netinet/in_mcast.cc
+++ b/bsd/sys/netinet/in_mcast.cc
@@ -1001,9 +1001,10 @@ inm_merge(struct in_multi *inm, /*const*/ struct in_mfilter *imf)
/* Decrement ASM listener count on transition out of ASM mode. */
if (imf->imf_st[0] == MCAST_EXCLUDE && nsrc0 == 0) {
if ((imf->imf_st[1] != MCAST_EXCLUDE) ||
- (imf->imf_st[1] == MCAST_EXCLUDE && nsrc1 > 0))
+ (imf->imf_st[1] == MCAST_EXCLUDE && nsrc1 > 0)) {
CTR1(KTR_IGMPV3, "%s: --asm on inm at t1", __func__);
--inm->inm_st[1].iss_asm;
+ }
}

/* Increment ASM listener count on transition to ASM mode. */
diff --git a/bsd/sys/netinet/tcp_syncache.cc b/bsd/sys/netinet/tcp_syncache.cc

[Commit Bot]

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

xen: fix overread and overwrite bug

Gcc 11 detected a bug in many places using the HYPERVISOR_event_channel_op
function was used. This function works on many types of operations, so
assumes it can read and write the maximal size of the operation's
parameters - 24 bytes. And yet, it was called for smaller, specific,
operation object.

This patch fixes all the calls to build a maximal-size operation object.

I'm not very happy about this patch - it is only needed for some
situation marked unlikely - where all this problematic copying happens -
but I want to restore the ability to build on Fedora 34 which has gcc
11.

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

Message-Id: <20210614062057...@scylladb.com>

---
diff --git a/bsd/sys/xen/evtchn.cc b/bsd/sys/xen/evtchn.cc

@@ -341,7 +341,7 @@ bind_virq_to_irq(unsigned int virq, unsigned int cpu, int * port)

static int
bind_ipi_to_irq(unsigned int ipi, unsigned int cpu, int * port)
{
- struct evtchn_bind_ipi bind_ipi;
+ struct evtchn_op op;
int irq;
int evtchn = 0;

@@ -351,9 +351,9 @@ bind_ipi_to_irq(unsigned int ipi, unsigned int cpu, int * port)
if ((irq = find_unbound_irq()) < 0)
goto out;

- bind_ipi.vcpu = cpu;
- HYPERVISOR_event_channel_op(EVTCHNOP_bind_ipi, &bind_ipi);
- evtchn = bind_ipi.port;
+ op.u.bind_ipi.vcpu = cpu;
+ HYPERVISOR_event_channel_op(EVTCHNOP_bind_ipi, &op.u);
+ evtchn = op.u.bind_ipi.port;

evtchn_to_irq[evtchn] = irq;
irq_info[irq] = mk_irq_info(IRQT_IPI, ipi, evtchn);

@@ -375,15 +375,15 @@ bind_ipi_to_irq(unsigned int ipi, unsigned int cpu, int * port)

diff --git a/bsd/sys/xen/evtchn.h b/bsd/sys/xen/evtchn.h

--- a/bsd/sys/xen/evtchn.h
+++ b/bsd/sys/xen/evtchn.h
@@ -57,8 +57,9 @@ clear_evtchn(int port)
inline void
notify_remote_via_evtchn(int port)
{
- struct evtchn_send send = { .port = port };
- (void)HYPERVISOR_event_channel_op(EVTCHNOP_send, &send);
+ struct evtchn_op send;
+ send.u.send.port = port;
+ (void)HYPERVISOR_event_channel_op(EVTCHNOP_send, &send.u);
}

/*

diff --git a/bsd/sys/xen/xenbus/xenbus.cc b/bsd/sys/xen/xenbus/xenbus.cc

[Commit Bot]

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

Makefile: avoid warning in one Musl source file

gcc 11 warns for musl/src/prng/seed48.c that seed48()'s signature in
the header file has a 3-char array, but the implementation has a char
pointer. This is correct, but gcc now warns about such inconsistencies,
and we turn those warnings into errors. Because this is an original
Musl source file that we don't want to needlessly change, let's just
avoid the warning with CFLAGS += -Wno-array-parameter for this specific
file.

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

Message-Id: <20210614062057...@scylladb.com>

---
diff --git a/Makefile b/Makefile

[Commit Bot]

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

fs/vfs/main.cc: fix gcc 11 build error

In fs/vfs/main.cc we have bootfs_start, which was defined as a single
character (extern char bootfs_start) but was really treated as the
beginning of a longer memory area. Gcc 11 can recognize that we access
beyond this single character, and fail the compilation.

To work around this problem, we declare bootfs_start as
extern char bootfs_start[]

Little needs to be changed besides this - except changing
&bootfs_start + offset - which gcc doesn't like, to
&bootfs_start[offset].

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

Message-Id: <20210614062057...@scylladb.com>

---
diff --git a/fs/vfs/main.cc b/fs/vfs/main.cc

[Commit Bot]

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

core/trace.cc: free aligned_alloc() with free()

Memory allocated with aligned_alloc() should be freed with free(), not
with delete - although in our actual implemention those are really the
same thing. gcc 11 warns about one such case, so instead of silencing
the warning, let's make it use free().

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

Message-Id: <20210614062057...@scylladb.com>

---
diff --git a/core/trace.cc b/core/trace.cc

[Commit Bot]

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

sched: free aligned_alloc() with free()

Memory allocated with aligned_alloc() should be freed with free(), not
with delete - although in our actual implemention those are really the
same thing. gcc 11 warns about using sched::thread::make() - which uses
align_alloc() - and the deleting it with "delete", using
std::unique_ptr.

Gcc only warns about this issue in sched.cc so we only fix that file,
but note that other places of the code also have unique_ptr<thread>
and should eventually use a similar fix.

The fix is a new method sched::thread::make_unique<> which returns
a std::unique_ptr that holds a thread a knows how to properly delete it.

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

Message-Id: <20210614062057...@scylladb.com>

---
diff --git a/core/sched.cc b/core/sched.cc
--- a/core/sched.cc
+++ b/core/sched.cc
@@ -135,7 +135,7 @@ class thread::reaper {

diff --git a/include/osv/sched.hh b/include/osv/sched.hh

[Commit Bot]

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

add __libc_single_threaded symbol

glibc has this variable (see
https://www.gnu.org/software/libc/manual/html_node/Single_002dThreaded.html)
which some C++ library functions recently started to use, so we need to
define it.

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

Message-Id: <20210614062057...@scylladb.com>

---
diff --git a/runtime.cc b/runtime.cc

[Commit Bot]

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

core/mmu.cc: silence warning

gcc 11 warns about the MMU code writing 4096 byte to an area with 0
bytes. I am guessing this is a false alarm, but the code is too
convoluted for me to prove it :-( So for now, let's just slience this
build error.

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

Message-Id: <20210614062057...@scylladb.com>

---
diff --git a/core/mmu.cc b/core/mmu.cc

[Commit Bot]

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

scripts/setup.py: add Fedora 34

I'm not yet aware of anything special that needs to be installed for
this version.

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

Message-Id: <20210614062057...@scylladb.com>

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