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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 9 Jan 2019, Julien Grall wrote:
> 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.

Julien, thanks for chiming in. Yes, I completely agree with you.

My thinking for the current changes is that they are better than nothing
as they clearly mark all problematic sites with "SYMBOL", even if the
implementation of SYMBOL might not be good enough. Then, we double-check
with PRQA and others, once we get their feedback we can still change the
return type of SYMBOL if we need to, and it will be easy to do at that
point (it only took me 1h yesterday to make the opposite change).



>       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+arr
>       ay
> 
>       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