| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20230807141252.24482-1-falcon@tinylab.org>
Date: Mon, 7 Aug 2023 22:12:52 +0800
From: Zhangjin Wu <falcon@...ylab.org>
To: david.laight@...lab.com
Cc: arnd@...db.de, falcon@...ylab.org, linux-kernel@...r.kernel.org,
linux-kselftest@...r.kernel.org, thomas@...ch.de, w@....eu
Subject: RE: [PATCH v3] tools/nolibc: fix up size inflate regression
Hi, David
> From: Zhangjin Wu
> > Sent: 07 August 2023 06:58
> ...
> > +/* __auto_type is used instead of __typeof__ to workaround the build error
> > + * 'error: assignment of read-only variable' when the argument has 'const' in
> > + * the type, but __auto_type is a new feature from newer gcc version and it
> > + * only works with 'const' from gcc 11.0 (__GXX_ABI_VERSION = 1016)
> > + * https://gcc.gnu.org/legacy-ml/gcc-patches/2013-11/msg01378.html
> > + */
>
> You can use typeof((x) + 0) to lose the 'const' flag.
> The only downside is that char/short become int.
>
Great, thanks!
let's use it, and at least kill the branch using fixed 'long' type.
#if __GXX_ABI_VERSION >= 1016
#define __typeofdecl(arg) __auto_type
#else
#define __typeofdecl(arg) __typeof__(arg)
#endif
#define __sysret(arg) \
({ \
__typeofdecl((arg) + 0) __ret = (arg); \
if (__is_signed_type(__typeof__(arg))) { \
if ((long)__ret < 0) { \
SET_ERRNO(-(long)__ret); \
__ret = (__typeof__(arg))-1L; \
} \
} else { \
if ((unsigned long)__ret >= (unsigned long)-MAX_ERRNO) { \
SET_ERRNO(-(long)__ret); \
__ret = (__typeof__(arg))-1L; \
} \
} \
__ret; \
})
My simple test on nolibc-test shows David's typeof solution does give
the same size result like __auto_type.
what's your suggestion? simply give up the '__auto_type' stuff and use
the generic __typeof__ version?
Willy, could you please test David's typeof solution on the one which
have 3-4% size inflating? or any other big programs using nolibc.
> > +
> > +#if __GXX_ABI_VERSION >= 1016
> > +#define __GXX_HAS_AUTO_TYPE_WITH_CONST_SUPPORT
> > +#endif
> > +
> > +#ifdef __GXX_HAS_AUTO_TYPE_WITH_CONST_SUPPORT
> > +#define __sysret(arg) \
> > +({ \
> > + __auto_type __ret = (arg); \
> > + if (__is_signed_type(__typeof__(arg))) { \
> > + if (__ret < 0) { \
> > + SET_ERRNO(-(long)__ret); \
> > + __ret = (__typeof__(arg))(-1L); \
> > + } \
> > + } else { \
> > + if ((unsigned long)__ret >= (unsigned long)-MAX_ERRNO) { \
> > + SET_ERRNO(-(long)__ret); \
> > + __ret = (__typeof__(arg))(-1L); \
> > + } \
> > + } \
> > + __ret; \
> > +})
> > +
> > +#else /* ! __GXX_HAS_AUTO_TYPE_WITH_CONST_SUPPORT */
> > +#define __sysret(arg) \
> > +({ \
> > + long __ret = (long)(arg); \
> > + if (__is_signed_type(__typeof__(arg))) { \
> > + if (__ret < 0) { \
> > + SET_ERRNO(-__ret); \
> > + __ret = -1L; \
> > + } \
> > + } else { \
> > + if ((unsigned long)__ret >= (unsigned long)-MAX_ERRNO) { \
> > + SET_ERRNO(-__ret); \
> > + __ret = -1L; \
> > + } \
> > + } \
> > + (__typeof__(arg))__ret; \
> > +})
> > +#endif /* ! __GXX_HAS_AUTO_TYPE_WITH_CONST_SUPPORT */
>
> with (retyped so it may be wrong):
> #define is_constexpr(x) sizeof(*(0 ? (void *)((long)(x) * 0) : (int *)0)) == 1)
> and (because even (void *)0 isn't completely constant):
> #define is_pointer(x) (!is_constexpr((typeof(x))0))
>
> You can probably do:
> #define __sysret(arg) \
> ({ \
> typeof((arg) + 0) __ret = arg; \
> if (__built_choose_expr(is_pointer(arg), (unsigned long)-(MAX_ERRNO+1), __ret) \
> < (__built_choose_expr(is_pointer(arg), (unsigned long)__ret, 0)) { \
> SET_ERRNO(-__ret); \
> __reg = typeof(ret)-1L; \
> } \
> __ret; \
> })
>
> Apart from the annoyance of having to reverse the conditional
> that only has one copy of the check.
>
> Using two __builtin_choose_expr() saves you having to write two
> comparisons that are valid for both pointer and integer.
>
It works perfectly.
/*
* This returns a constant expression while determining if an argument is
* a constant expression, most importantly without evaluating the argument.
* Glory to Martin Uecker <Martin.Uecker@....uni-goettingen.de>
* (from include/linux/const.h)
*/
#define __is_constexpr(x) \
(sizeof(int) == sizeof(*(8 ? ((void *)((long)(x) * 0l)) : (int *)8)))
#define __is_pointer(x) (!__is_constexpr((__typeof__(x))0))
#define __sysret(arg) \
({ \
__typeofdecl((arg) + 0) __ret = (arg); \
if (__builtin_choose_expr(__is_pointer(arg), (unsigned long)-(MAX_ERRNO + 1), ((long)__ret)) \
< __builtin_choose_expr(__is_pointer(arg), (unsigned long)__ret, 0)) { \
SET_ERRNO(-(long)__ret); \
__ret = (__typeof__(arg))-1L; \
} \
__ret; \
})
I have tried the 'is_constexpr()' macro but failed and didn't look into
it, your explanation here [1] is very clear:
You'll find that (void *)0 isn't 'constant enough' for
is_constexpr() - so is_constexpr((type)0) can be used
to detect pointer types.
Best regards,
Zhangjin
---
[1]: https://lore.kernel.org/lkml/a1732bbffd1542d3b9dd34c92f45076c@AcuMS.aculab.com/
> David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
Powered by blists - more mailing lists