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

[Neel Natu]

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))

 /*
  * Host to big endian, host to little endian, big endian to host, and little

best
Neel

>> Modified:
>>   head/libexec/rtld-elf/mips/reloc.c
>>
>> Modified: head/libexec/rtld-elf/mips/reloc.c
>> ==============================================================================
>> --- head/libexec/rtld-elf/mips/reloc.c        Mon Aug  9 23:32:37 2010        (r211129)
>> +++ head/libexec/rtld-elf/mips/reloc.c        Tue Aug 10 05:15:35 2010        (r211130)
>> @@ -75,7 +75,7 @@ void _rtld_relocate_nonplt_self(Elf_Dyn
>>   * ELF64 MIPS encodes the relocs uniquely.  The first 32-bits of info contain
>>   * the symbol index.  The top 32-bits contain three relocation types encoded
>>   * in big-endian integer with first relocation in LSB.  This means for little
>> - * endian we have to byte swap that interger (r_type).
>> + * endian we have to byte swap that integer (r_type).
>>   */
>>  #define      Elf_Sxword                      Elf64_Sxword
>>  #define      ELF_R_NXTTYPE_64_P(r_type)      ((((r_type) >> 8) & 0xff) == R_TYPE(64))
>> @@ -83,7 +83,7 @@ void _rtld_relocate_nonplt_self(Elf_Dyn
>>  #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)
>>
>