On Fri, May 16, 2014 at 12:53:29PM +0100, James Hogan wrote: > On 14/05/14 15:17, Dmitry V. Levin wrote: [...] > > I have two questions wrt this patch: > > > > 1. Removing the conditional would result with print_rlimit32 defined for > > all architectures, including pure 64bit ones. I suppose we need a check > > like this anyway: > > > > #if PERSONALITY0_WORDSIZE == 4 || \ > > (SUPPORTED_PERSONALITIES > 1 && PERSONALITY1_WORDSIZE == 4) || \ > > (SUPPORTED_PERSONALITIES > 2 && PERSONALITY2_WORDSIZE == 4) > > There don't appear to be any arches supported (according to defs.h) with > >2 personalities with all wordsizes the same. > > Where there are 2 personalities with the same wordsize the definition of > current_wordsize takes this into account and defines it as > PERSONALITY0_WORDSIZE.
Yes, so the check could be simplified to
#if !defined(current_wordsize) || current_wordsize == 4
> So this would seem to allow the compiler to optimise out whichever
> print_rlimit{32,64} is unused, and a quick look at the x86_64 assembly
> output (gcc 4.7.3 -O2) with a hacked current_wordsize in resource.c
> confirms that.
Yes, and some day the compiler will also optimize out decode_rlimit
because it ends up the same as decode_rlimit64.
I'd rather not wait when it happens and add the check.
> There is of course the "!addr" and "!verbose(tcp || (exiting(tcp) &&
> syserror(tcp))" conditions which seem to be skipped for the pure 64-bit
> case at the moment which I'm not sure was intentional anyway?
Hopefully, they are not skipped. I admit these function names
(sprint_rlim64, print_rlimit64, and decode_rlimit64) are somewhat
confusing. The last of them does all these top level checks.
> > 2. I'm not sure about architectures where kernel wordsize is 8 but user
>
> > wordsize is 4 (like x32 and mips n32); what's the correct rlim_t for these
> > architectures?
>
> Hmm, that's a good point.
>
> MIPS n32 appears to use the default __kernel_ulong_t define of unsigned
> long (32bit) so it would work fine I think (although sadly it appears
> that strace doesn't support personalities on MIPS yet).
>
> x32 appears to define __kernel_ulong_t as unsigned long long (64bit), so
> strace handling of {get,set}rlimit would already appear to be broken for
> that (this patch doesn't make that any worse).
Thanks. I suppose this patch fixes all these rlim_t issues:
--- a/configure.ac
+++ b/configure.ac
@@ -309,7 +309,6 @@ AC_CACHE_CHECK([for BLKGETSIZE64],
[ac_cv_have_blkgetsize64],
AC_CHECK_SIZEOF([long])
AC_CHECK_SIZEOF([long long])
AC_CHECK_SIZEOF([off_t],,[#include <sys/types.h>])
-AC_CHECK_SIZEOF([rlim_t],,[#include <sys/resource.h>])
AC_CACHE_CHECK([for SA_RESTORER], [st_cv_sa_restorer],
[st_cv_sa_restorer="$(echo SA_RESTORER |
--- a/resource.c
+++ b/resource.c
@@ -88,10 +88,6 @@ static const struct xlat resources[] = {
XLAT_END
};
-#if !(SIZEOF_RLIM_T == 4 || SIZEOF_RLIM_T == 8)
-# error "Unsupported SIZEOF_RLIM_T value"
-#endif
-
static const char *
sprint_rlim64(uint64_t lim)
{
@@ -135,7 +131,7 @@ decode_rlimit64(struct tcb *tcp, unsigned long addr)
print_rlimit64(tcp, addr);
}
-#if SIZEOF_RLIM_T == 4 || SUPPORTED_PERSONALITIES > 1
+#if !defined(current_wordsize) || current_wordsize == 4
static const char *
sprint_rlim32(uint32_t lim)
@@ -176,22 +172,22 @@ decode_rlimit(struct tcb *tcp, unsigned long addr)
else if (!verbose(tcp) || (exiting(tcp) && syserror(tcp)))
tprintf("%#lx", addr);
else {
-# if SIZEOF_RLIM_T == 4
- print_rlimit32(tcp, addr);
+# ifdef X32
+ if (current_personality == 1)
# else
if (current_wordsize == 4)
+# endif
print_rlimit32(tcp, addr);
else
print_rlimit64(tcp, addr);
-# endif
}
}
-#else /* SIZEOF_RLIM_T == 8 && SUPPORTED_PERSONALITIES == 1 */
+#else /* defined(current_wordsize) && current_wordsize != 4 */
# define decode_rlimit decode_rlimit64
-#endif /* SIZEOF_RLIM_T == 4 || SUPPORTED_PERSONALITIES > 1 */
+#endif
int
sys_getrlimit(struct tcb *tcp)
--
ldv
pgp2QcQ9IjJcA.pgp
Description: PGP signature
------------------------------------------------------------------------------ "Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE Instantly run your Selenium tests across 300+ browser/OS combos. Get unparalleled scalability from the best Selenium testing platform available Simple to use. Nothing to install. Get started now for free." http://p.sf.net/sfu/SauceLabs
_______________________________________________ Strace-devel mailing list Strace-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/strace-devel
