svn commit: r211130 - head/libexec/rtld-elf/mips

[Jayachandran C.]

On Tue, Aug 10, 2010 at 12:12 PM, Neel Natu <neelnatu at gmail.com> wrote:
> Hi Stefan,
>
> On Mon, Aug 9, 2010 at 11:28 PM, Stefan Farfeleder <stefanf at freebsd.org> wrote:
>> On Tue, Aug 10, 2010 at 05:15:35AM +0000, Neel Natu wrote:
>>> Author: neel
>>> Date: Tue Aug 10 05:15:35 2010
>>> New Revision: 211130
>>> URL: http://svn.freebsd.org/changeset/base/211130
>>>
>>> Log:
>>>   Fix compilation error for 64-bit little endian build:
>>>   libexec/rtld-elf/mips/reloc.c:196: warning: right shift count >= width of type
>>>
>>>   When the expression '(r_info) >> 32' was passed to bswap32() it was promptly
>>>   changed to '(uint32_t)(r_info) >> 32' which is not what we intended.
>>
>> Wouldn't it be better to fix the bswap32 macro instead?
>>
>
> I think you are right. Can you take a look at the following patch instead?
>
> If I hear no objections, I will commit it tomorrow.
>
> Index: libexec/rtld-elf/mips/reloc.c
> ===================================================================
> --- libexec/rtld-elf/mips/reloc.c       (revision 211131)
> +++ libexec/rtld-elf/mips/reloc.c       (working copy)
> @@ -83,7 +83,7 @@
>  #undef ELF_R_SYM
>  #undef ELF_R_TYPE
>  #define ELF_R_SYM(r_info)              ((r_info) & 0xffffffff)
> -#define ELF_R_TYPE(r_info)             bswap32(((r_info) >> 32))
> +#define ELF_R_TYPE(r_info)             bswap32((r_info) >> 32)
>  #endif
>  #else
>  #define        ELF_R_NXTTYPE_64_P(r_type)      (0)
> Index: sys/sys/endian.h
> ===================================================================
> --- sys/sys/endian.h    (revision 211131)
> +++ sys/sys/endian.h    (working copy)
> @@ -56,9 +56,9 @@
>  /*
>  * General byte order swapping functions.
>  */
> -#define        bswap16(x)      __bswap16(x)
> -#define        bswap32(x)      __bswap32(x)
> -#define        bswap64(x)      __bswap64(x)
> +#define        bswap16(x)      __bswap16((x))
> +#define        bswap32(x)      __bswap32((x))
> +#define        bswap64(x)      __bswap64((x))
>

I think there is a problem in  sys/mips/include/_endian.h
--
#define __bswap16(x)    (__uint16_t)(__is_constant(x) ?         \
        __bswap16_const((__uint16_t)x) :  __bswap16_var((__uint16_t)x))
#define __bswap32(x)    (__uint32_t)(__is_constant(x) ?         \
        __bswap32_const((__uint32_t)x) :  __bswap32_var((__uint32_t)x))
#define __bswap64(x)    (__uint64_t)(__is_constant(x) ?         \
        __bswap64_const((__uint64_t)x) :  __bswap64_var((__uint64_t)x))
--

I'm not sure why the cast is needed, but we should have a braces
around x, unless I'm completely mistaken.

JC.