Hi Dmitry, On 14/05/14 15:17, Dmitry V. Levin wrote: > On Fri, May 02, 2014 at 02:15:41PM +0100, James Hogan wrote: >> When strace is built with large file support definitions in CFLAGS (as >> may be provided by buildroot) the C library headers may expose a 64-bit >> rlim_t even though the struct rlimit fields used by the system call >> interface are only 32-bit. The SIZEOF_RLIM_T will then be 8 which >> results in bad decoding of the getrlimit and setrlimit syscalls. > > If SIZEOF_RLIM_T cannot be relied upon, it shouldn't be used, and its > definition has to be removed as well.
True.
>> This is fixed by removing the "#if SIZEOF_RLIM_T == 4 ||
>> SUPPORTED_PERSONALITIES > 1" conditional, since the remaining code
>> already handles multiple personalities based on the value of
>> current_wordsize, which is set correctly even for a single personality.
>
> 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.
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.
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?
> 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).
Cheers
James
signature.asc
Description: OpenPGP digital 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
