Re: [Xen-devel] [PATCH v6 1/4] xen: introduce SYMBOL

[Julien Grall <julien.grall@xxxxxxxxx>]

Hi,

Sorry for the formatting.

On Wed, 9 Jan 2019, 18:43 Stefano Stabellini, <sstabellini@xxxxxxxxxx> wrote:

Introduce a macro, SYMBOL, which is similar to RELOC_HIDE, but it is
meant to be used everywhere symbols such as _stext and _etext are used
in the code. It can take an array type as a parameter, and it returns
the same type.

SYMBOL is needed when accessing symbols such as _stext and _etext
because the C standard forbids for both comparisons and substraction
(see C Standard, 6.5.6 [ISO/IEC 9899:2011] and [1]) between pointers
pointing to different objects. _stext, _etext, etc. are all pointers to
different objects from ANCI C point of view.

This does not make sense because you still return a pointer and therefore the undefined behavior is still present.

I really don't believe this patch is going to make the MISRA tool happy. Furthermore, IIRC, Linux to returns unsigned long. So I would like to understand why the trick is no needed for us...

At that stage, we should probably involve MlSRA folks (PRQA) to have a better understanding on what is expected.

Cheers,

To work around potential C compiler issues (which have actually
been found, see the comment on top of RELOC_HIDE in Linux), and to help
with certifications, let's introduce some syntactic sugar to be used in
following patches.

[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

Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>
CC: JBeulich@xxxxxxxx
CC: andrew.cooper3@xxxxxxxxxx
CC: wei.liu2@xxxxxxxxxx
---
Changes in v6:
- drop acks
- don't use RELOC_HIDE for the implementation
- return native type from SYMBOL

Changes in v4:
- add acked-bys
- remove unneeded parenthesis

Changes in v3:
- improve commit message
- rename __symbol to SYMBOL to avoid name space violations

Changes in v2:
- do not cast return to char*
- move to common header
---
 xen/include/xen/compiler.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/xen/include/xen/compiler.h b/xen/include/xen/compiler.h
index ff6c0f5..d4c856c 100644
--- a/xen/include/xen/compiler.h
+++ b/xen/include/xen/compiler.h
@@ -99,6 +99,16 @@
     __asm__ ("" : "=r"(__ptr) : "0"(ptr));      \
     (typeof(ptr)) (__ptr + (off)); })

+/*
+ * Similar to RELOC_HIDE, but written to be used with symbols such as
+ * _stext and _etext to avoid undefined behavior comparing pointers to
+ * different objects. It can handle array types.
+ */
+#define SYMBOL(ptr)                               \
+  ({ unsigned long __ptr;                       \
+    __asm__ ("" : "=r"(__ptr) : "0"(ptr));      \
+    (typeof(*(ptr)) *) (__ptr); })
+
 #ifdef __GCC_ASM_FLAG_OUTPUTS__
 # define ASM_FLAG_OUT(yes, no) yes
 #else
--
1.9.1


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

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