Re: [PATCH] Fix wordsize-32 mmap offset for negative value (BZ#18877)

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

On 21-01-2016 12:12, Joseph Myers wrote:
> On Thu, 21 Jan 2016, Adhemerval Zanella wrote:
> 
>> You are correct, I bluntly used the bz report testcase as the one for the
>> patch.  And checking both Linux and POSIX negative offset are not really
>> specified, so the issue at BZ#18877 can't really being considered a regression
>> since the offset usage is undefined behaviour.  The fix itself on the
>> wordsize-32 I still consider valid since it was using a shift arithmetic
>> signed is implementation-defined behaviour (and the divide operation is
>> assumed to not be implementation-defined).
> 
> The shift is fully defined in GNU C, which is what is used by glibc.  The 
> actual fix isn't the use of the divide (if the low bits are zero, signed 
> division and signed arithmetic right shift have the same effect) - it's 
> that you're dividing by an *unsigned* quantity whose type is such that C 
> type promotion rules cause the signed offset to be converted to unsigned 
> before the division (and converting to an unsigned type before shifting 
> right would have just the same effect).

Right I oversee the unsigned promotion rule, so seems the right signed
shift should be safe.

> 
>> About the test I think we should just remove it and state that invalid
>> offsets are undefined or architecture define.
> 
> I don't think saying they are architecture-defined makes sense - in 
> general the glibc API should be consistent between architectures (and we 
> test for the glibc API, not just for the POSIX API).  And if it's 
> undefined (as opposed to invalid and requiring an error), again, what is 
> the basis for that in POSIX?
> 

OK I am open to suggestion in such case to how to handle negative offsets.