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

[Julien Grall <julien.grall@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3063 bytes --]

Hi,

Sorry for the formatting.

On Wed, 9 Jan 2019, 18:43 Stefano Stabellini, <sstabellini@kernel.org>
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@xilinx.com>
> CC: JBeulich@suse.com
> CC: andrew.cooper3@citrix.com
> CC: wei.liu2@citrix.com
> ---
> 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@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel

[-- Attachment #1.2: Type: text/html, Size: 4653 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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