[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v7 4/4] xen/common: use SYMBOL_HIDE when required



On Thu, 10 Jan 2019, Stefano Stabellini wrote:
> diff --git a/xen/include/xen/kernel.h b/xen/include/xen/kernel.h
> index 548b64d..3fa56ff 100644
> --- a/xen/include/xen/kernel.h
> +++ b/xen/include/xen/kernel.h
> @@ -66,27 +66,27 @@
>  })
>  
>  extern char _start[], _end[], start[];
> -#define is_kernel(p) ({                         \
> -    char *__p = (char *)(unsigned long)(p);     \
> -    (__p >= _start) && (__p < _end);            \
> +#define is_kernel(p) ({                                             \
> +    const char *p__ = (char *)(unsigned long)(p);                   \
> +    (p__ >= SYMBOL_HIDE(_start)) && (p__ < SYMBOL_HIDE(_end));      \
>  })
>  
>  extern char _stext[], _etext[];
> -#define is_kernel_text(p) ({                    \
> -    char *__p = (char *)(unsigned long)(p);     \
> -    (__p >= _stext) && (__p < _etext);          \
> +#define is_kernel_text(p) ({                                        \
> +    const char *p__ = (char *)(unsigned long)(p);                   \
> +    (p__ >= SYMBOL_HIDE(_stext)) && (p__ < SYMBOL_HIDE(_etext));    \
>  })
>  
>  extern const char _srodata[], _erodata[];
> -#define is_kernel_rodata(p) ({                  \
> -    const char *__p = (const char *)(unsigned long)(p);     \
> -    (__p >= _srodata) && (__p < _erodata);      \
> +#define is_kernel_rodata(p) ({                                      \
> +    const char *p__ = (const char *)(unsigned long)(p);             \
> +    (p__ >= SYMBOL_HIDE(_srodata)) && (p__ < SYMBOL_HIDE(_erodata));\
>  })
>  
>  extern char _sinittext[], _einittext[];
> -#define is_kernel_inittext(p) ({                \
> -    char *__p = (char *)(unsigned long)(p);     \
> -    (__p >= _sinittext) && (__p < _einittext);  \
> +#define is_kernel_inittext(p) ({                                         \
> +    const char *p__ = (char *)(unsigned long)(p);                        \
> +    (p__ >= SYMBOL_HIDE(_sinittext)) && (p__ < SYMBOL_HIDE(_einittext)); \
>  })

I have just realized that it would be nicer to also switch the casts to
const char*. See patch below with this small change:


From 7fa76d73b275285ccf942eb0397ae52fb18fa683 Mon Sep 17 00:00:00 2001
From: Stefano Stabellini <sstabellini@xxxxxxxxxx>
Date: Thu, 10 Jan 2019 10:50:41 -0800
Subject: [PATCH] xen/common: use SYMBOL_HIDE when required

Use SYMBOL_HIDE in cases of comparisons and subtractions of:

_start, _end, _stext, _etext, _srodata, _erodata, _sinittext,
_einittext, __note_gnu_build_id_start, __note_gnu_build_id_end,
__lock_profile_start, __lock_profile_end, __initcall_start,
__initcall_end, __presmp_initcall_end, __ctors_start, __ctors_end,
__end_schedulers_array, __start_schedulers_array, __start_bug_frames,
__stop_bug_frames_0, __stop_bug_frames_1, __stop_bug_frames_2,
__stop_bug_frames_3,

as by the C standard [1].

M3CM: Rule-18.2: Subtraction between pointers shall only be applied to
pointers that address elements of the same array

Since we are changing the body of is_kernel_text and friends, take the
opportunity to remove the leading underscores in the local variables
names, which are violationg namespace rules. Also make the local p__
variable const.

[1] 
https://wiki.sei.cmu.edu/confluence/display/c/ARR36-C.+Do+not+subtract+or+compare+two+pointers+that+do+not+refer+to+the+same+array

QAVerify: 2761
Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
CC: JBeulich@xxxxxxxx
CC: andrew.cooper3@xxxxxxxxxx
---
Changes in v7:
- add back second const in bug_frames
- switch from void* casts to unsigned long casts in xen_build_init
- switch p__ to const char*
- rename SYMBOL to SYMBOL_HIDE

Changes in v6:
- more accurate commit message
- remove hard tabs
- remove leading underscores
- code style
- use SYMBOL_HIDE only on the problematic bug_frames symbols
- use new SYMBOL_HIDE macro that returns the native type

Changes in v5:
- remove two spurious changes
- split into three patches
- remove SYMBOL_HIDE() from derived variables
---
 xen/common/kernel.c         |  8 ++++++--
 xen/common/lib.c            |  3 ++-
 xen/common/schedule.c       |  6 ++++--
 xen/common/spinlock.c       |  4 +++-
 xen/common/version.c        |  6 +++---
 xen/common/virtual_region.c | 12 ++++++------
 xen/include/xen/kernel.h    | 24 ++++++++++++------------
 7 files changed, 36 insertions(+), 27 deletions(-)

diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index 5766a0f..e546ba1 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -312,14 +312,18 @@ extern const initcall_t __initcall_start[], 
__presmp_initcall_end[],
 void __init do_presmp_initcalls(void)
 {
     const initcall_t *call;
-    for ( call = __initcall_start; call < __presmp_initcall_end; call++ )
+    for ( call = SYMBOL_HIDE(__initcall_start);
+          call < SYMBOL_HIDE(__presmp_initcall_end);
+          call++ )
         (*call)();
 }
 
 void __init do_initcalls(void)
 {
     const initcall_t *call;
-    for ( call = __presmp_initcall_end; call < __initcall_end; call++ )
+    for ( call = SYMBOL_HIDE(__presmp_initcall_end);
+          call < SYMBOL_HIDE(__initcall_end);
+          call++ )
         (*call)();
 }
 
diff --git a/xen/common/lib.c b/xen/common/lib.c
index 8ebec81..a788a1c 100644
--- a/xen/common/lib.c
+++ b/xen/common/lib.c
@@ -497,7 +497,8 @@ extern const ctor_func_t __ctors_start[], __ctors_end[];
 void __init init_constructors(void)
 {
     const ctor_func_t *f;
-    for ( f = __ctors_start; f < __ctors_end; ++f )
+
+    for ( f = SYMBOL_HIDE(__ctors_start); f < SYMBOL_HIDE(__ctors_end); ++f )
         (*f)();
 
     /* Putting this here seems as good (or bad) as any other place. */
diff --git a/xen/common/schedule.c b/xen/common/schedule.c
index a957c5e..3e00fcf 100644
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -67,8 +67,10 @@ DEFINE_PER_CPU(struct scheduler *, scheduler);
 /* Scratch space for cpumasks. */
 DEFINE_PER_CPU(cpumask_t, cpumask_scratch);
 
-extern const struct scheduler *__start_schedulers_array[], 
*__end_schedulers_array[];
-#define NUM_SCHEDULERS (__end_schedulers_array - __start_schedulers_array)
+extern const struct scheduler *__start_schedulers_array[],
+                              *__end_schedulers_array[];
+#define NUM_SCHEDULERS (SYMBOL_HIDE(__end_schedulers_array) - \
+                        SYMBOL_HIDE(__start_schedulers_array))
 #define schedulers __start_schedulers_array
 
 static struct scheduler __read_mostly ops;
diff --git a/xen/common/spinlock.c b/xen/common/spinlock.c
index 6bc52d7..a870f03 100644
--- a/xen/common/spinlock.c
+++ b/xen/common/spinlock.c
@@ -474,7 +474,9 @@ static int __init lock_prof_init(void)
 {
     struct lock_profile **q;
 
-    for ( q = &__lock_profile_start; q < &__lock_profile_end; q++ )
+    for ( q = SYMBOL_HIDE(&__lock_profile_start);
+          q < SYMBOL_HIDE(&__lock_profile_end);
+          q++ )
     {
         (*q)->next = lock_profile_glb_q.elem_q;
         lock_profile_glb_q.elem_q = *q;
diff --git a/xen/common/version.c b/xen/common/version.c
index 223cb52..e882793 100644
--- a/xen/common/version.c
+++ b/xen/common/version.c
@@ -147,14 +147,14 @@ static int __init xen_build_init(void)
     int rc;
 
     /* --build-id invoked with wrong parameters. */
-    if ( __note_gnu_build_id_end <= &n[0] )
+    if ( SYMBOL_HIDE(__note_gnu_build_id_end) <= &n[0] )
         return -ENODATA;
 
     /* Check for full Note header. */
-    if ( &n[1] >= __note_gnu_build_id_end )
+    if ( &n[1] >= SYMBOL_HIDE(__note_gnu_build_id_end) )
         return -ENODATA;
 
-    sz = (void *)__note_gnu_build_id_end - (void *)n;
+    sz = (unsigned long)SYMBOL_HIDE(__note_gnu_build_id_end) - (unsigned 
long)n;
 
     rc = xen_build_id_check(n, sz, &build_id_p, &build_id_len);
 
diff --git a/xen/common/virtual_region.c b/xen/common/virtual_region.c
index aa23918..cd7f373 100644
--- a/xen/common/virtual_region.c
+++ b/xen/common/virtual_region.c
@@ -103,13 +103,13 @@ void __init setup_virtual_regions(const struct 
exception_table_entry *start,
 {
     size_t sz;
     unsigned int i;
-    static const struct bug_frame *const __initconstrel bug_frames[] = {
-        __start_bug_frames,
-        __stop_bug_frames_0,
-        __stop_bug_frames_1,
-        __stop_bug_frames_2,
+    const struct bug_frame *const bug_frames[] = {
+        SYMBOL_HIDE(__start_bug_frames),
+        SYMBOL_HIDE(__stop_bug_frames_0),
+        SYMBOL_HIDE(__stop_bug_frames_1),
+        SYMBOL_HIDE(__stop_bug_frames_2),
 #ifdef CONFIG_X86
-        __stop_bug_frames_3,
+        SYMBOL_HIDE(__stop_bug_frames_3),
 #endif
         NULL
     };
diff --git a/xen/include/xen/kernel.h b/xen/include/xen/kernel.h
index 548b64d..0733dfe 100644
--- a/xen/include/xen/kernel.h
+++ b/xen/include/xen/kernel.h
@@ -66,27 +66,27 @@
 })
 
 extern char _start[], _end[], start[];
-#define is_kernel(p) ({                         \
-    char *__p = (char *)(unsigned long)(p);     \
-    (__p >= _start) && (__p < _end);            \
+#define is_kernel(p) ({                                             \
+    const char *p__ = (const char *)(unsigned long)(p);             \
+    (p__ >= SYMBOL_HIDE(_start)) && (p__ < SYMBOL_HIDE(_end));      \
 })
 
 extern char _stext[], _etext[];
-#define is_kernel_text(p) ({                    \
-    char *__p = (char *)(unsigned long)(p);     \
-    (__p >= _stext) && (__p < _etext);          \
+#define is_kernel_text(p) ({                                        \
+    const char *p__ = (const char *)(unsigned long)(p);             \
+    (p__ >= SYMBOL_HIDE(_stext)) && (p__ < SYMBOL_HIDE(_etext));    \
 })
 
 extern const char _srodata[], _erodata[];
-#define is_kernel_rodata(p) ({                  \
-    const char *__p = (const char *)(unsigned long)(p);     \
-    (__p >= _srodata) && (__p < _erodata);      \
+#define is_kernel_rodata(p) ({                                      \
+    const char *p__ = (const char *)(unsigned long)(p);             \
+    (p__ >= SYMBOL_HIDE(_srodata)) && (p__ < SYMBOL_HIDE(_erodata));\
 })
 
 extern char _sinittext[], _einittext[];
-#define is_kernel_inittext(p) ({                \
-    char *__p = (char *)(unsigned long)(p);     \
-    (__p >= _sinittext) && (__p < _einittext);  \
+#define is_kernel_inittext(p) ({                                         \
+    const char *p__ = (const char *)(unsigned long)(p);                  \
+    (p__ >= SYMBOL_HIDE(_sinittext)) && (p__ < SYMBOL_HIDE(_einittext)); \
 })
 
 extern enum system_state {
-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.