Re: [PATCH 1/5] Fix {INLINE,INTERNAL}_SYSCALL macros for x32

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

On 29/06/2017 15:50, Florian Weimer wrote:
> On 06/29/2017 04:15 PM, Adhemerval Zanella wrote:
>> +/* Create a variable 'name' based on type 'X' to avoid explicit types.
>> +   This is mainly used set use 64-bits arguments in x32.   */
>> +#define TYPEFY(X, name) __typeof__ ((X) - (X)) name
>> +/* Explicit cast the argument to avoid integer from pointer warning on
>> +   x32.  */
>> +#define ARGIFY(X) ((__typeof__ ((X) - (X))) (X))
> 
> I think cast_to_integer was added for this purpose.

The 'cast_to_integer' macro was added after my initial (I based the ARGIFY
macro on the mips64n32 one) and checking it by replacing ARGIGY with 
'cast_to_integer' I see:

---
gconv_cache.c: In function ‘__gconv_load_cache’:
../include/libc-pointer-arith.h:34:45: error: cast specifies array type
   __integer_if_pointer_type_sub(__typeof__ ((__typeof__ (expr)) 0), \
                                             ^
../include/libc-pointer-arith.h:29:39: note: in definition of macro ‘__integer_if_pointer_type_sub’
   __typeof__ (*(0 ? (__typeof__ (0 ? (T *) 0 : (void *) (P))) 0 \
                                       ^
../include/libc-pointer-arith.h:38:33: note: in expansion of macro ‘__integer_if_pointer_type’
 # define cast_to_integer(val) ((__integer_if_pointer_type (val)) (val))
                                 ^~~~~~~~~~~~~~~~~~~~~~~~~
../sysdeps/unix/sysv/linux/x86_64/sysdep.h:284:29: note: in expansion of macro ‘cast_to_integer’
     TYPEFY (arg1, __arg1) = cast_to_integer (arg1);    \
                             ^~~~~~~~~~~~~~~
../sysdeps/unix/sysv/linux/x86_64/sysdep.h:230:2: note: in expansion of macro ‘internal_syscall3’
  internal_syscall##nr (SYS_ify (name), err, args)
  ^~~~~~~~~~~~~~~~
../sysdeps/unix/sysv/linux/x86_64/sysdep.h:196:35: note: in expansion of macro ‘INTERNAL_SYSCALL’
     unsigned long int resultvar = INTERNAL_SYSCALL (name, , nr, args);       \
                                   ^~~~~~~~~~~~~~~~
../sysdeps/unix/sysv/linux/not-cancel.h:31:4: note: in expansion of macro ‘INLINE_SYSCALL’
    INLINE_SYSCALL (open, 3, name, flags, mode)
---

And I recall this was one the issue I had when creating this patch. Using compound
literal on __typeof__ cast did not help due we build with -Wwrite-strings it and
in turn it enables -Wdiscarded-array-qualifiers.  This triggers a 'pointer to
array loses qualifier', because constness type is propagate.

So to actually use cast_to_integer workable on char array (and on other arrays
as well) one solution is to incorporate the arithmetic promotion used on 
mips64n32 sysdep and also on this patch:

---
/* Type cast using arithmetic promotion.  It also remove l-value-constness
   when using char arrays on cast_to_interger (for instance
   cast_to_interger ("...") will not trigger a 'pointer to array loses
   qualifier' warning due -Wdiscarded-array-qualifiers from -Wwrite-strings.
 */
# define __typeof_arith_promote(T) __typeof__((T) - (T))

/* 1 if 'type' is a pointer type, 0 otherwise.  */
# define __pointer_type(type) (__builtin_classify_type ((type) 0) == 5)

/* intptr_t if P is true, or T if P is false.  */
# define __integer_if_pointer_type_sub(T, P) \
  __typeof__ (*(0 ? (__typeof__ (0 ? (T *) 0: (void *) (P))) 0 \
                  : (__typeof__ (0 ? (intptr_t *) 0: (void *) (!(P)))) 0))

/* intptr_t if EXPR has a pointer type, or the type of EXPR otherwise.  */
# define __integer_if_pointer_type(expr) \
  __integer_if_pointer_type_sub(__typeof__ ((__typeof_arith_promote (expr)) 0), \
                                __pointer_type (__typeof_arith_promote (expr)))

/* Cast an integer or a pointer VAL to integer with proper type.  */
# define cast_to_integer(val) ((__integer_if_pointer_type (val)) (val))
---

I am not sure how safe is the arithmetic promotion in the all the possible
cases where one could use cast_to_integer, however current cases on atomic
and syscall usage should be safe (make check shows no regression on
x86_64{-x32} with the pread auto-generate removal applied, so they use the 
default Linux implementation. If you think it is worth I can resend with the
libc-pointer-arith.h modification.