2013-06-09 02:34:29

by Daniel Tang

[permalink] [raw]
Subject: [PATCH 1/2] Fix a build warning in scripts/sortextable.h

The pointer passed to the _r() macro does not always match the type
of the function that it is aliasing and raises several of the following
warnings at compile time:

warning: passing argument 1 of ‘r8’ from incompatible pointer type

Fixed by casting the pointers to (void *) so they work with both the
32bit and 64bit code.

Signed-off-by: Daniel Tang <[email protected]>
---
scripts/sortextable.h | 29 +++++++++++++++--------------
1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/scripts/sortextable.h b/scripts/sortextable.h
index f5eb43d..0a38fbd 100644
--- a/scripts/sortextable.h
+++ b/scripts/sortextable.h
@@ -110,9 +110,9 @@ do_func(Elf_Ehdr *ehdr, char const *const fname, table_sort_t custom_sort)
int i;
int idx;

- shdr = (Elf_Shdr *)((char *)ehdr + _r(&ehdr->e_shoff));
+ shdr = (Elf_Shdr *)((char *)ehdr + _r((void *)&ehdr->e_shoff));
shstrtab_sec = shdr + r2(&ehdr->e_shstrndx);
- secstrtab = (const char *)ehdr + _r(&shstrtab_sec->sh_offset);
+ secstrtab = (const char *)ehdr + _r((void *)&shstrtab_sec->sh_offset);
for (i = 0; i < r2(&ehdr->e_shnum); i++) {
idx = r(&shdr[i].sh_name);
if (strcmp(secstrtab + idx, "__ex_table") == 0) {
@@ -122,8 +122,8 @@ do_func(Elf_Ehdr *ehdr, char const *const fname, table_sort_t custom_sort)
if ((r(&shdr[i].sh_type) == SHT_REL ||
r(&shdr[i].sh_type) == SHT_RELA) &&
r(&shdr[i].sh_info) == extab_index) {
- relocs = (void *)ehdr + _r(&shdr[i].sh_offset);
- relocs_size = _r(&shdr[i].sh_size);
+ relocs = (void *)ehdr + _r((void *)&shdr[i].sh_offset);
+ relocs_size = _r((void *)&shdr[i].sh_size);
}
if (strcmp(secstrtab + idx, ".symtab") == 0)
symtab_sec = shdr + i;
@@ -142,14 +142,14 @@ do_func(Elf_Ehdr *ehdr, char const *const fname, table_sort_t custom_sort)
fprintf(stderr, "no __ex_table in file: %s\n", fname);
fail_file();
}
- strtab = (const char *)ehdr + _r(&strtab_sec->sh_offset);
+ strtab = (const char *)ehdr + _r((void *)&strtab_sec->sh_offset);

- extab_image = (void *)ehdr + _r(&extab_sec->sh_offset);
+ extab_image = (void *)ehdr + _r((void *)&extab_sec->sh_offset);

if (custom_sort) {
- custom_sort(extab_image, _r(&extab_sec->sh_size));
+ custom_sort(extab_image, _r((void *)&extab_sec->sh_size));
} else {
- int num_entries = _r(&extab_sec->sh_size) / extable_ent_size;
+ int num_entries = _r((void *)&extab_sec->sh_size) / extable_ent_size;
qsort(extab_image, num_entries,
extable_ent_size, compare_extable);
}
@@ -159,12 +159,13 @@ do_func(Elf_Ehdr *ehdr, char const *const fname, table_sort_t custom_sort)

/* find main_extable_sort_needed */
sort_needed_sym = NULL;
- for (i = 0; i < _r(&symtab_sec->sh_size) / sizeof(Elf_Sym); i++) {
- sym = (void *)ehdr + _r(&symtab_sec->sh_offset);
+ for (i = 0; i < _r((void *)&symtab_sec->sh_size) / sizeof(Elf_Sym); i++)
+ {
+ sym = (void *)ehdr + _r((void *)&symtab_sec->sh_offset);
sym += i;
if (ELF_ST_TYPE(sym->st_info) != STT_OBJECT)
continue;
- idx = r(&sym->st_name);
+ idx = r((void *)&sym->st_name);
if (strcmp(strtab + idx, "main_extable_sort_needed") == 0) {
sort_needed_sym = sym;
break;
@@ -178,9 +179,9 @@ do_func(Elf_Ehdr *ehdr, char const *const fname, table_sort_t custom_sort)
}
sort_needed_sec = &shdr[r2(&sort_needed_sym->st_shndx)];
sort_done_location = (void *)ehdr +
- _r(&sort_needed_sec->sh_offset) +
- _r(&sort_needed_sym->st_value) -
- _r(&sort_needed_sec->sh_addr);
+ _r((void *)&sort_needed_sec->sh_offset) +
+ _r((void *)&sort_needed_sym->st_value) -
+ _r((void *)&sort_needed_sec->sh_addr);

#if 0
printf("sort done marker at %lx\n",
--
1.8.1.3


2013-06-09 02:34:51

by Daniel Tang

[permalink] [raw]
Subject: [PATCH 2/2] Fix a build warning in scripts/mod/file2alias.c

On some systems, __used is already defined in sys/cdefs.h and causes
a build warning:

scripts/mod/file2alias.c:85:1: warning: "__used" redefined
In file included from /usr/include/stdio.h:64,
from scripts/mod/modpost.h:1,
from scripts/mod/file2alias.c:13:
/usr/include/sys/cdefs.h:146:1: warning: this is the location of the previous definition

This adds an extra check before defining the __used macro to see if
the macro was already defined elsewhere.

Signed-off-by: Daniel Tang <[email protected]>
---
scripts/mod/file2alias.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index 45f9a33..ab55456 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -79,10 +79,12 @@ struct devtable **__start___devtable, **__stop___devtable;
extern struct devtable *__start___devtable[], *__stop___devtable[];
#endif /* __MACH__ */

-#if __GNUC__ == 3 && __GNUC_MINOR__ < 3
-# define __used __attribute__((__unused__))
-#else
-# define __used __attribute__((__used__))
+#if !defined(__used)
+# if __GNUC__ == 3 && __GNUC_MINOR__ < 3
+# define __used __attribute__((__unused__))
+# else
+# define __used __attribute__((__used__))
+# endif
#endif

/* Define a variable f that holds the value of field f of struct devid
--
1.8.1.3

2013-07-03 12:06:55

by Michal Marek

[permalink] [raw]
Subject: Re: [PATCH 1/2] Fix a build warning in scripts/sortextable.h

Added David Daney to CC.

On 9.6.2013 04:33, Daniel Tang wrote:
> The pointer passed to the _r() macro does not always match the type
> of the function that it is aliasing and raises several of the following
> warnings at compile time:
>
> warning: passing argument 1 of ‘r8’ from incompatible pointer type

In what environment (arch, compiler, glibc) are you seeing this?

Michal

> Fixed by casting the pointers to (void *) so they work with both the
> 32bit and 64bit code.
>
> Signed-off-by: Daniel Tang <[email protected]>
> ---
> scripts/sortextable.h | 29 +++++++++++++++--------------
> 1 file changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/scripts/sortextable.h b/scripts/sortextable.h
> index f5eb43d..0a38fbd 100644
> --- a/scripts/sortextable.h
> +++ b/scripts/sortextable.h
> @@ -110,9 +110,9 @@ do_func(Elf_Ehdr *ehdr, char const *const fname, table_sort_t custom_sort)
> int i;
> int idx;
>
> - shdr = (Elf_Shdr *)((char *)ehdr + _r(&ehdr->e_shoff));
> + shdr = (Elf_Shdr *)((char *)ehdr + _r((void *)&ehdr->e_shoff));
> shstrtab_sec = shdr + r2(&ehdr->e_shstrndx);
> - secstrtab = (const char *)ehdr + _r(&shstrtab_sec->sh_offset);
> + secstrtab = (const char *)ehdr + _r((void *)&shstrtab_sec->sh_offset);
> for (i = 0; i < r2(&ehdr->e_shnum); i++) {
> idx = r(&shdr[i].sh_name);
> if (strcmp(secstrtab + idx, "__ex_table") == 0) {
> @@ -122,8 +122,8 @@ do_func(Elf_Ehdr *ehdr, char const *const fname, table_sort_t custom_sort)
> if ((r(&shdr[i].sh_type) == SHT_REL ||
> r(&shdr[i].sh_type) == SHT_RELA) &&
> r(&shdr[i].sh_info) == extab_index) {
> - relocs = (void *)ehdr + _r(&shdr[i].sh_offset);
> - relocs_size = _r(&shdr[i].sh_size);
> + relocs = (void *)ehdr + _r((void *)&shdr[i].sh_offset);
> + relocs_size = _r((void *)&shdr[i].sh_size);
> }
> if (strcmp(secstrtab + idx, ".symtab") == 0)
> symtab_sec = shdr + i;
> @@ -142,14 +142,14 @@ do_func(Elf_Ehdr *ehdr, char const *const fname, table_sort_t custom_sort)
> fprintf(stderr, "no __ex_table in file: %s\n", fname);
> fail_file();
> }
> - strtab = (const char *)ehdr + _r(&strtab_sec->sh_offset);
> + strtab = (const char *)ehdr + _r((void *)&strtab_sec->sh_offset);
>
> - extab_image = (void *)ehdr + _r(&extab_sec->sh_offset);
> + extab_image = (void *)ehdr + _r((void *)&extab_sec->sh_offset);
>
> if (custom_sort) {
> - custom_sort(extab_image, _r(&extab_sec->sh_size));
> + custom_sort(extab_image, _r((void *)&extab_sec->sh_size));
> } else {
> - int num_entries = _r(&extab_sec->sh_size) / extable_ent_size;
> + int num_entries = _r((void *)&extab_sec->sh_size) / extable_ent_size;
> qsort(extab_image, num_entries,
> extable_ent_size, compare_extable);
> }
> @@ -159,12 +159,13 @@ do_func(Elf_Ehdr *ehdr, char const *const fname, table_sort_t custom_sort)
>
> /* find main_extable_sort_needed */
> sort_needed_sym = NULL;
> - for (i = 0; i < _r(&symtab_sec->sh_size) / sizeof(Elf_Sym); i++) {
> - sym = (void *)ehdr + _r(&symtab_sec->sh_offset);
> + for (i = 0; i < _r((void *)&symtab_sec->sh_size) / sizeof(Elf_Sym); i++)
> + {
> + sym = (void *)ehdr + _r((void *)&symtab_sec->sh_offset);
> sym += i;
> if (ELF_ST_TYPE(sym->st_info) != STT_OBJECT)
> continue;
> - idx = r(&sym->st_name);
> + idx = r((void *)&sym->st_name);
> if (strcmp(strtab + idx, "main_extable_sort_needed") == 0) {
> sort_needed_sym = sym;
> break;
> @@ -178,9 +179,9 @@ do_func(Elf_Ehdr *ehdr, char const *const fname, table_sort_t custom_sort)
> }
> sort_needed_sec = &shdr[r2(&sort_needed_sym->st_shndx)];
> sort_done_location = (void *)ehdr +
> - _r(&sort_needed_sec->sh_offset) +
> - _r(&sort_needed_sym->st_value) -
> - _r(&sort_needed_sec->sh_addr);
> + _r((void *)&sort_needed_sec->sh_offset) +
> + _r((void *)&sort_needed_sym->st_value) -
> + _r((void *)&sort_needed_sec->sh_addr);
>
> #if 0
> printf("sort done marker at %lx\n",
>

2013-07-03 12:07:52

by Michal Marek

[permalink] [raw]
Subject: Re: [PATCH 1/2] Fix a build warning in scripts/sortextable.h

On 3.7.2013 14:06, Michal Marek wrote:
> Added David Daney to CC.

Second attempt, sorry.


> On 9.6.2013 04:33, Daniel Tang wrote:
>> The pointer passed to the _r() macro does not always match the type
>> of the function that it is aliasing and raises several of the following
>> warnings at compile time:
>>
>> warning: passing argument 1 of ‘r8’ from incompatible pointer type
>
> In what environment (arch, compiler, glibc) are you seeing this?
>
> Michal
>
>> Fixed by casting the pointers to (void *) so they work with both the
>> 32bit and 64bit code.
>>
>> Signed-off-by: Daniel Tang <[email protected]>
>> ---
>> scripts/sortextable.h | 29 +++++++++++++++--------------
>> 1 file changed, 15 insertions(+), 14 deletions(-)
>>
>> diff --git a/scripts/sortextable.h b/scripts/sortextable.h
>> index f5eb43d..0a38fbd 100644
>> --- a/scripts/sortextable.h
>> +++ b/scripts/sortextable.h
>> @@ -110,9 +110,9 @@ do_func(Elf_Ehdr *ehdr, char const *const fname, table_sort_t custom_sort)
>> int i;
>> int idx;
>>
>> - shdr = (Elf_Shdr *)((char *)ehdr + _r(&ehdr->e_shoff));
>> + shdr = (Elf_Shdr *)((char *)ehdr + _r((void *)&ehdr->e_shoff));
>> shstrtab_sec = shdr + r2(&ehdr->e_shstrndx);
>> - secstrtab = (const char *)ehdr + _r(&shstrtab_sec->sh_offset);
>> + secstrtab = (const char *)ehdr + _r((void *)&shstrtab_sec->sh_offset);
>> for (i = 0; i < r2(&ehdr->e_shnum); i++) {
>> idx = r(&shdr[i].sh_name);
>> if (strcmp(secstrtab + idx, "__ex_table") == 0) {
>> @@ -122,8 +122,8 @@ do_func(Elf_Ehdr *ehdr, char const *const fname, table_sort_t custom_sort)
>> if ((r(&shdr[i].sh_type) == SHT_REL ||
>> r(&shdr[i].sh_type) == SHT_RELA) &&
>> r(&shdr[i].sh_info) == extab_index) {
>> - relocs = (void *)ehdr + _r(&shdr[i].sh_offset);
>> - relocs_size = _r(&shdr[i].sh_size);
>> + relocs = (void *)ehdr + _r((void *)&shdr[i].sh_offset);
>> + relocs_size = _r((void *)&shdr[i].sh_size);
>> }
>> if (strcmp(secstrtab + idx, ".symtab") == 0)
>> symtab_sec = shdr + i;
>> @@ -142,14 +142,14 @@ do_func(Elf_Ehdr *ehdr, char const *const fname, table_sort_t custom_sort)
>> fprintf(stderr, "no __ex_table in file: %s\n", fname);
>> fail_file();
>> }
>> - strtab = (const char *)ehdr + _r(&strtab_sec->sh_offset);
>> + strtab = (const char *)ehdr + _r((void *)&strtab_sec->sh_offset);
>>
>> - extab_image = (void *)ehdr + _r(&extab_sec->sh_offset);
>> + extab_image = (void *)ehdr + _r((void *)&extab_sec->sh_offset);
>>
>> if (custom_sort) {
>> - custom_sort(extab_image, _r(&extab_sec->sh_size));
>> + custom_sort(extab_image, _r((void *)&extab_sec->sh_size));
>> } else {
>> - int num_entries = _r(&extab_sec->sh_size) / extable_ent_size;
>> + int num_entries = _r((void *)&extab_sec->sh_size) / extable_ent_size;
>> qsort(extab_image, num_entries,
>> extable_ent_size, compare_extable);
>> }
>> @@ -159,12 +159,13 @@ do_func(Elf_Ehdr *ehdr, char const *const fname, table_sort_t custom_sort)
>>
>> /* find main_extable_sort_needed */
>> sort_needed_sym = NULL;
>> - for (i = 0; i < _r(&symtab_sec->sh_size) / sizeof(Elf_Sym); i++) {
>> - sym = (void *)ehdr + _r(&symtab_sec->sh_offset);
>> + for (i = 0; i < _r((void *)&symtab_sec->sh_size) / sizeof(Elf_Sym); i++)
>> + {
>> + sym = (void *)ehdr + _r((void *)&symtab_sec->sh_offset);
>> sym += i;
>> if (ELF_ST_TYPE(sym->st_info) != STT_OBJECT)
>> continue;
>> - idx = r(&sym->st_name);
>> + idx = r((void *)&sym->st_name);
>> if (strcmp(strtab + idx, "main_extable_sort_needed") == 0) {
>> sort_needed_sym = sym;
>> break;
>> @@ -178,9 +179,9 @@ do_func(Elf_Ehdr *ehdr, char const *const fname, table_sort_t custom_sort)
>> }
>> sort_needed_sec = &shdr[r2(&sort_needed_sym->st_shndx)];
>> sort_done_location = (void *)ehdr +
>> - _r(&sort_needed_sec->sh_offset) +
>> - _r(&sort_needed_sym->st_value) -
>> - _r(&sort_needed_sec->sh_addr);
>> + _r((void *)&sort_needed_sec->sh_offset) +
>> + _r((void *)&sort_needed_sym->st_value) -
>> + _r((void *)&sort_needed_sec->sh_addr);
>>
>> #if 0
>> printf("sort done marker at %lx\n",
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2013-07-03 12:12:21

by Daniel Tang

[permalink] [raw]
Subject: Re: [PATCH 1/2] Fix a build warning in scripts/sortextable.h

Hi,

On 03/07/2013, at 10:07 PM, Michal Marek <[email protected]> wrote:

> On 3.7.2013 14:06, Michal Marek wrote:
>> Added David Daney to CC.
>
> Second attempt, sorry.
>
>
>> On 9.6.2013 04:33, Daniel Tang wrote:
>>> The pointer passed to the _r() macro does not always match the type
>>> of the function that it is aliasing and raises several of the following
>>> warnings at compile time:
>>>
>>> warning: passing argument 1 of ?r8? from incompatible pointer type
>>
>> In what environment (arch, compiler, glibc) are you seeing this?
>>

I'm cross compiling the ARM arch with gcc version 4.7.2 on 64 bit OS X. It should throw that warning on any 64 bit environment (not just OS X) but I haven't tested it.

If you want, I can look a little deeper into this.

Cheers,
tangrs-

2013-07-03 12:28:18

by Michal Marek

[permalink] [raw]
Subject: Re: [PATCH 1/2] Fix a build warning in scripts/sortextable.h

On 3.7.2013 14:12, Daniel Tang wrote:
>> On 3.7.2013 14:06, Michal Marek wrote:
>>> On 9.6.2013 04:33, Daniel Tang wrote:
>>>> The pointer passed to the _r() macro does not always match the
>>>> type of the function that it is aliasing and raises several of
>>>> the following warnings at compile time:
>>>>
>>>> warning: passing argument 1 of ?r8? from incompatible pointer
>>>> type
>>>
>>> In what environment (arch, compiler, glibc) are you seeing this?
>>>
>
> I'm cross compiling the ARM arch with gcc version 4.7.2 on 64 bit OS
> X. It should throw that warning on any 64 bit environment (not just
> OS X) but I haven't tested it.

The cross compilation target should not matter, this is a host program.
It does work for me on Linux (openSUSE 12.3 FWIW), with gcc 4.7.2 and
glibc 2.17:

$ gcc -m64 -Wall -Wmissing-prototypes -Wstrict-prototypes -O2
-fomit-frame-pointer -Itools/include -o scripts/sortextable
scripts/sortextable.c; echo $?
0

What is the type of Elf64_Shdr::sh_offset in OS X's <elf.h>? In glibc,
this is Elf64_Off, which is a typedef name for uint64_t.

Michal

2013-07-03 12:31:53

by Michal Marek

[permalink] [raw]
Subject: Re: [PATCH 2/2] Fix a build warning in scripts/mod/file2alias.c

On 9.6.2013 04:33, Daniel Tang wrote:
> On some systems, __used is already defined in sys/cdefs.h and causes
> a build warning:
>
> scripts/mod/file2alias.c:85:1: warning: "__used" redefined
> In file included from /usr/include/stdio.h:64,
> from scripts/mod/modpost.h:1,
> from scripts/mod/file2alias.c:13:
> /usr/include/sys/cdefs.h:146:1: warning: this is the location of the previous definition
>
> This adds an extra check before defining the __used macro to see if
> the macro was already defined elsewhere.
>
> Signed-off-by: Daniel Tang <[email protected]>

Applied to kbuild.git#kbuild.

Michal

2013-07-03 12:38:00

by Daniel Tang

[permalink] [raw]
Subject: Re: [PATCH 1/2] Fix a build warning in scripts/sortextable.h

Hi,

On 03/07/2013, at 10:28 PM, Michal Marek <[email protected]> wrote:
>
>
> What is the type of Elf64_Shdr::sh_offset in OS X's <elf.h>? In glibc,
> this is Elf64_Off, which is a typedef name for uint64_t.

Huh, that's weird. Elf64_Off is defined as unsigned long in my elf.h which isn't 64 bit on x86_64 pretty sure.

It's a problem on my side then, apologies.

>
> Michal

Cheers,
tangrs-

2013-07-03 12:57:16

by Michal Marek

[permalink] [raw]
Subject: Re: [PATCH 1/2] Fix a build warning in scripts/sortextable.h

On 3.7.2013 14:37, Daniel Tang wrote:
> Hi,
>
> On 03/07/2013, at 10:28 PM, Michal Marek <[email protected]> wrote:
>>
>>
>> What is the type of Elf64_Shdr::sh_offset in OS X's <elf.h>? In
>> glibc, this is Elf64_Off, which is a typedef name for uint64_t.
>
> Huh, that's weird. Elf64_Off is defined as unsigned long in my elf.h
> which isn't 64 bit on x86_64 pretty sure.

unsigned long _is_ 64bit on x86_64 Linux (or any 64bit Linux), but it
might not be the case on OS X. But then, the <elf.h> on that platform
should use something that is actually 64bit for Elf64_Off.


> It's a problem on my side then, apologies.

If Elf64_Off is really defined as an 32bit integer on OS X, then yes.

Michal

2013-07-03 13:16:06

by Michal Marek

[permalink] [raw]
Subject: Re: [PATCH 1/2] Fix a build warning in scripts/sortextable.h

On 3.7.2013 14:57, Michal Marek wrote:
> On 3.7.2013 14:37, Daniel Tang wrote:
>> Hi,
>>
>> On 03/07/2013, at 10:28 PM, Michal Marek <[email protected]> wrote:
>>>
>>>
>>> What is the type of Elf64_Shdr::sh_offset in OS X's <elf.h>? In
>>> glibc, this is Elf64_Off, which is a typedef name for uint64_t.
>>
>> Huh, that's weird. Elf64_Off is defined as unsigned long in my elf.h
>> which isn't 64 bit on x86_64 pretty sure.
>
> unsigned long _is_ 64bit on x86_64 Linux (or any 64bit Linux), but it
> might not be the case on OS X. But then, the <elf.h> on that platform
> should use something that is actually 64bit for Elf64_Off.

Hm, according to
https://developer.apple.com/library/mac/documentation/Darwin/Conceptual/64bitPorting/transition/transition.html,
64bit OS X uses that same model as Linux, i.e. sizeof(long) ==
sizeof(void*). So the warning is probably caused by the fact that the
typedefs Elf64_Off and uint64_t are not identical (even though the types
have the same size). How are these defined on OS X?

Thanks,
Michal