From 8add1b91854143c0b365dfb49de937e132bf9596 Mon Sep 17 00:00:00 2001
From: Nadav Har'El <nyh@cloudius-systems.com>
Date: Mon, 25 Nov 2013 15:21:13 +0200
Subject: [PATCH] Warn about incorrect use of percpu<> / PERCPU(..).
This patch causes incorrect usage of percpu<>/PERCPU() to cause
compilation errors instead of silent runtime corruptions.
Thanks to Dmitry for first noticing this issue in xen_intr.cc (see his
separate patch), and to Avi for suggesting a compile-time fix.
With this patch:
1. Using percpu<...> to *define* a per-cpu variable fails compilation.
Instead, PERCPU(...) must be used for the definition, which is important
because it places the variable in the ".percpu" section.
2. If a *declaration* is needed additionally (e.g., for a static class
member), percpu<...> must be used, not PERCPU().
Trying to use PERCPU() for declaration will cause a compilation error.
3. PERCPU() only works on statically-constructed objects - global variables,
static function-variables and static class-members. Trying to use it
on a dynamically-constructed object - stack variable, class field,
or operator new - will cause a compilation error.
With this patch, the bug in xen_intr.cc would have been caught at
compile time.
Signed-off-by: Nadav Har'El <nyh@cloudius-systems.com>
Signed-off-by: Pekka Enberg <penberg@cloudius-systems.com>
---
drivers/kvmclock.cc | 2 +-
include/osv/percpu.hh | 23 +++++++++++++++++++----
2 files changed, 20 insertions(+), 5 deletions(-)
diff --git a/drivers/kvmclock.cc b/drivers/kvmclock.cc
index 408a022ff..b6489ce1c 100644
--- a/drivers/kvmclock.cc
+++ b/drivers/kvmclock.cc
@@ -30,7 +30,7 @@ private:
static bool _new_kvmclock_msrs;
pvclock_wall_clock* _wall;
u64 _wall_ns;
- static PERCPU(pvclock_vcpu_time_info, _sys);
+ static percpu<pvclock_vcpu_time_info> _sys;
sched::cpu::notifier cpu_notifier;
};
diff --git a/include/osv/percpu.hh b/include/osv/percpu.hh
index 1f9f85e49..82f15b655 100644
--- a/include/osv/percpu.hh
+++ b/include/osv/percpu.hh
@@ -12,14 +12,28 @@
#include <type_traits>
#include <memory>
-extern char _percpu_start[];
-
extern __thread char* percpu_base;
template <typename T>
class percpu {
public:
- constexpr percpu() {}
+ // We need percpu<T> variables to be defined with the PERCPU macro so
+ // they'll end up in the right section. To enforce this, we add a fake
+ // argument to the constructor. The type percpu<T> still needs to be
+ // used to declare the variable separately from its definition (which
+ // is needed for class static variables).
+ static constexpr struct
+ please_use_PERCPU_macro {} please_use_PERCPU_macro {};
+ // percpu<T>'s constructor needs to be constexpr so that the .percpu
+ // section can be constructed at compile time, and then at early run time
+ // be copied to per-cpu copies of this section, without risking that the
+ // constructor hasn't run yet.
+ explicit constexpr percpu(struct please_use_PERCPU_macro) { }
+ // You can't copy a per-cpu variable and get a new per-cpu variable.
+ // Neither can one be moved (its address is important).
+ percpu(const percpu&) = delete;
+ percpu(percpu&&) = delete;
+
T* operator->() {
return addr();
}
@@ -39,7 +53,8 @@ private:
friend size_t dynamic_percpu_base();
};
-#define PERCPU(type, var) percpu<type> var __attribute__((section(".percpu")))
+#define PERCPU(type, var) __attribute__((section(".percpu"))) \
+ percpu<type> var (percpu<type>::please_use_PERCPU_macro)
size_t dynamic_percpu_alloc(size_t size, size_t align);
void dynamic_percpu_free(size_t offset, size_t size);
--
GitLab