x86-64: Fix gcc-7 warning in relocs.c

PATCH In review Markus Trippelsdorf 3 participants 9 messages Dec 15, 2016 – Dec 20, 2016 x86

lore

Share & tools

lore

Markus Trippelsdorf

author

2016-12-15 12:52 UTC #0 lore raw

PATCH 1/1 x86-64: Fix gcc-7 warning in relocs.c

gcc-7 warns:

In file included from arch/x86/tools/relocs_64.c:17:0:
arch/x86/tools/relocs.c: In function ‘process_64’:
arch/x86/tools/relocs.c:953:2: warning: argument 1 null where non-null expected [-Wnonnull]
qsort(r->offset, r->count, sizeof(r->offset[0]), cmp_relocs); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from arch/x86/tools/relocs.h:6:0,

                 from arch/x86/tools/relocs_64.c:1:

/usr/include/stdlib.h:741:13: note: in a call to function ‘qsort’ declared here
extern void qsort

This happens because relocs16 is not used for ELF_BITS == 64, so there is no point in trying to sort it.
Fixed by guarding the sort_relocs(&relocs16) call.

Signed-off-by Markus Trippelsdorf <[email protected]>

arch/x86/tools/relocs.c +2 -1

index 0c2fae8d929d..73eb7fd4aec4 100644
@@ -992,11 +992,12 @@ static void emit_relocs(int as_text, int use_real_mode)
 		die("Segment relocations found but --realmode not specified\n");
 
 	/* Order the relocations for more efficient processing */
-	sort_relocs(&relocs16);
 	sort_relocs(&relocs32);
 #if ELF_BITS == 64
 	sort_relocs(&relocs32neg);
 	sort_relocs(&relocs64);
+#else
+	sort_relocs(&relocs16);
 #endif
 
 	/* Print the relocations */

Signature · 1 line

Markus

tip-bot for Markus Trippelsdorf

merge bot

2016-12-19 10:56 UTC #1 lore raw

[tip:x86/urgent] x86/tools: Fix gcc-7 warning in relocs.c

x86/tools: Fix gcc-7 warning in relocs.c

Commit-ID:  7ebb916782949621ff6819acf373a06902df7679
Gitweb:     http://git.kernel.org/tip/7ebb916782949621ff6819acf373a06902df7679
Author:     Markus Trippelsdorf <[email protected]>
AuthorDate: Thu, 15 Dec 2016 13:45:13 +0100
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Mon, 19 Dec 2016 11:50:24 +0100

x86/tools: Fix gcc-7 warning in relocs.c

gcc-7 warns:

                 from arch/x86/tools/relocs_64.c:1:

/usr/include/stdlib.h:741:13: note: in a call to function ‘qsort’ declared here
extern void qsort

This happens because relocs16 is not used for ELF_BITS == 64, so there is no point in trying to sort it.

Make the sort_relocs(&relocs16) call 32bit only.

Signed-off-by Markus Trippelsdorf <[email protected]>

Link http://lkml.kernel.org/r/20161215124513.GA289@x4 open here

Signed-off-by Thomas Gleixner <[email protected]>

---
 arch/x86/tools/relocs.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

arch/x86/tools/relocs.c +2 -1

index 0c2fae8..73eb7fd 100644
@@ -992,11 +992,12 @@ static void emit_relocs(int as_text, int use_real_mode)
 		die("Segment relocations found but --realmode not specified\n");
 
 	/* Order the relocations for more efficient processing */
-	sort_relocs(&relocs16);
 	sort_relocs(&relocs32);
 #if ELF_BITS == 64
 	sort_relocs(&relocs32neg);
 	sort_relocs(&relocs64);
+#else
+	sort_relocs(&relocs16);
 #endif
 
 	/* Print the relocations */

H. Peter Anvin

reviewer

2016-12-20 09:31 UTC #2 lore raw

Re: [tip:x86/urgent] x86/tools: Fix gcc-7 warning in relocs.c

I'd strongly prefer a non-data-dependent solution, specifically adding at the top of sort_relocs():

if (!r->count)

	return;

However, by my reading of the C and POSIX standards, this is a gcc
error: qsort() should do nothing if the count is zero.

-hpa

Quoted 43 lines · tip-bot for Markus Trippelsdorf

Commit-ID:  7ebb916782949621ff6819acf373a06902df7679
Gitweb:     http://git.kernel.org/tip/7ebb916782949621ff6819acf373a06902df7679
Author:     Markus Trippelsdorf <[email protected]>
AuthorDate: Thu, 15 Dec 2016 13:45:13 +0100
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Mon, 19 Dec 2016 11:50:24 +0100
x86/tools: Fix gcc-7 warning in relocs.c
gcc-7 warns:
In file included from arch/x86/tools/relocs_64.c:17:0:
arch/x86/tools/relocs.c: In function ‘process_64’:
arch/x86/tools/relocs.c:953:2: warning: argument 1 null where non-null expected [-Wnonnull]
qsort(r->offset, r->count, sizeof(r->offset[0]), cmp_relocs); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from arch/x86/tools/relocs.h:6:0,
                 from arch/x86/tools/relocs_64.c:1:
/usr/include/stdlib.h:741:13: note: in a call to function ‘qsort’ declared here
extern void qsort
This happens because relocs16 is not used for ELF_BITS == 64, so there is no point in trying to sort it.
Make the sort_relocs(&relocs16) call 32bit only.
Signed-off-by Markus Trippelsdorf <[email protected]>
Link http://lkml.kernel.org/r/20161215124513.GA289@x4 open here
Signed-off-by Thomas Gleixner <[email protected]>
---
 arch/x86/tools/relocs.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
arch/x86/tools/relocs.c +0 -0
index 0c2fae8..73eb7fd 100644
@@ -992,11 +992,12 @@ static void emit_relocs(int as_text, int use_real_mode)
 		die("Segment relocations found but --realmode not specified\n");
 
 	/* Order the relocations for more efficient processing */

arch/x86/tools/relocs.c static void emit_relocs(int as_text, int use_real_mode) · line 1003

Quoted 10 lines

arch/x86/tools/relocs.c +2 -1

-	sort_relocs(&relocs16);
 	sort_relocs(&relocs32);
 #if ELF_BITS == 64
 	sort_relocs(&relocs32neg);
 	sort_relocs(&relocs64);
+#else
+	sort_relocs(&relocs16);
 #endif
 
 	/* Print the relocations */

Markus Trippelsdorf

author

2016-12-20 10:01 UTC #3 lore raw

Re: [tip:x86/urgent] x86/tools: Fix gcc-7 warning in relocs.c

Quoted 8 lines · H. Peter Anvin

I'd strongly prefer a non-data-dependent solution, specifically adding at the top of sort_relocs():
if (!r->count)
	return;
However, by my reading of the C and POSIX standards, this is a gcc
error: qsort() should do nothing if the count is zero.

No, it is invoking undefined behavior.

Notice the nonnull attribute in /usr/include/stdlib.h:

739 /* Sort NMEMB elements of BASE, of SIZE bytes each,
740 using COMPAR to perform the comparisons. */
741 extern void qsort (void *__base, size_t __nmemb, size_t __size,
742 __compar_fn_t __compar) __nonnull ((1, 4));

But feel free to revert my patch and add your solution.

--
Markus

H. Peter Anvin

reviewer

2016-12-20 11:10 UTC #4 lore raw

Re: [tip:x86/urgent] x86/tools: Fix gcc-7 warning in relocs.c

Quoted 11 lines · Markus Trippelsdorf

Quoted 8 lines · H. Peter Anvin
I'd strongly prefer a non-data-dependent solution, specifically adding at the top of sort_relocs():
if (!r->count)
	return;
However, by my reading of the C and POSIX standards, this is a gcc
error: qsort() should do nothing if the count is zero.
No, it is invoking undefined behavior.

Quoted 8 lines

Notice the nonnull attribute in /usr/include/stdlib.h:
739 /* Sort NMEMB elements of BASE, of SIZE bytes each,
740 using COMPAR to perform the comparisons. */
741 extern void qsort (void *__base, size_t __nmemb, size_t __size,
742 __compar_fn_t __compar) __nonnull ((1, 4));
But feel free to revert my patch and add your solution.

Well, s/gcc/glibc/ then.

Quoted 6 lines

The qsort() function shall sort an array of nel objects, the
initial element of which is pointed to by base. The size of
each object, in bytes, is specified by the width argument. If the nel argument has the value zero, the comparison function pointed to by compar shall not be called and no rearrangement shall take place.

-hpa

Markus Trippelsdorf

author

2016-12-20 11:51 UTC #5 lore raw

Re: [tip:x86/urgent] x86/tools: Fix gcc-7 warning in relocs.c

Quoted 15 lines · H. Peter Anvin

Quoted 14 lines · Markus Trippelsdorf
Quoted 8 lines · H. Peter Anvin
I'd strongly prefer a non-data-dependent solution, specifically adding at the top of sort_relocs():
if (!r->count)
	return;
However, by my reading of the C and POSIX standards, this is a gcc
error: qsort() should do nothing if the count is zero.
No, it is invoking undefined behavior.
Notice the nonnull attribute in /usr/include/stdlib.h:

Quoted 11 lines

Quoted 6 lines
739 /* Sort NMEMB elements of BASE, of SIZE bytes each,
740 using COMPAR to perform the comparisons. */
741 extern void qsort (void *__base, size_t __nmemb, size_t __size,
742 __compar_fn_t __compar) __nonnull ((1, 4));
But feel free to revert my patch and add your solution.
Well, s/gcc/glibc/ then.
Quoted 2 lines
The qsort() function shall sort an array of nel objects, the initial element of which is pointed to by base

NULL does not point to any object, therefore it is UB.

Signature · 1 line

Markus

[email protected]

reviewer

2016-12-20 18:33 UTC #6 lore raw

Re: [tip:x86/urgent] x86/tools: Fix gcc-7 warning in relocs.c

Quoted 24 lines · Markus Trippelsdorf

Quoted 3 lines · H. Peter Anvin
Quoted 2 lines · Markus Trippelsdorf
Quoted 1 line · H. Peter Anvin
I'd strongly prefer a non-data-dependent solution, specifically
adding
Quoted 19 lines
Quoted 18 lines
Quoted 7 lines
at the top of sort_relocs():
if (!r->count)
	return;
However, by my reading of the C and POSIX standards, this is a gcc
error: qsort() should do nothing if the count is zero.
No, it is invoking undefined behavior.
Notice the nonnull attribute in /usr/include/stdlib.h:
739 /* Sort NMEMB elements of BASE, of SIZE bytes each,
740 using COMPAR to perform the comparisons. */
741 extern void qsort (void *__base, size_t __nmemb, size_t __size,
742 __compar_fn_t __compar) __nonnull ((1, 4));
But feel free to revert my patch and add your solution.

Quoted 7 lines

Quoted 3 lines
Well, s/gcc/glibc/ then.
Quoted 1 line
The qsort() function shall sort an array of nel objects,
the
Quoted 1 line
Quoted 1 line
initial element of which is pointed to by base
NULL does not point to any object, therefore it is UB.

That seems, quite frankly, like a pretty idiotic lawyerism. Why would a pointer that by spec is never referenced not be able to be null?

Signature · 1 line

Sent from my Android device with K-9 Mail. Please excuse my brevity.

Markus Trippelsdorf

author

2016-12-20 19:32 UTC #7 lore raw

Re: [tip:x86/urgent] x86/tools: Fix gcc-7 warning in relocs.c

Quoted 27 lines · On 2016.12.20 at 10:32 -0800, [email protected] wrote:

Quoted 26 lines · Markus Trippelsdorf
Quoted 3 lines · H. Peter Anvin
Quoted 2 lines · Markus Trippelsdorf
Quoted 1 line · H. Peter Anvin
I'd strongly prefer a non-data-dependent solution, specifically
adding
Quoted 21 lines
Quoted 10 lines
Quoted 7 lines
at the top of sort_relocs():
if (!r->count)
	return;
However, by my reading of the C and POSIX standards, this is a gcc
error: qsort() should do nothing if the count is zero.
No, it is invoking undefined behavior.
Quoted 8 lines
Notice the nonnull attribute in /usr/include/stdlib.h:
739 /* Sort NMEMB elements of BASE, of SIZE bytes each,
740 using COMPAR to perform the comparisons. */
741 extern void qsort (void *__base, size_t __nmemb, size_t __size,
742 __compar_fn_t __compar) __nonnull ((1, 4));
But feel free to revert my patch and add your solution.
Well, s/gcc/glibc/ then.

Quoted 8 lines

Quoted 5 lines
Quoted 1 line
Quoted 1 line
The qsort() function shall sort an array of nel objects,
the
Quoted 1 line
Quoted 1 line
initial element of which is pointed to by base
NULL does not point to any object, therefore it is UB.
That seems, quite frankly, like a pretty idiotic lawyerism.
Why would a pointer that by spec is never referenced not be able to be null?

Thank you. Let me quote the standard for you:

7.1.4
»If an argument to a function has an invalid value (such as a value outside the domain of the function, or a pointer outside the address space of the program, or a null pointer, or a pointer to non-modifiable storage when the corresponding parameter is not const-qualified) or a type (after promotion) not expected by a function with variable number of arguments, the behavior is undefined.«

7.24.1(2)
»Where an argument declared as size_t n specifies the length of the array for a function, n can have the value zero […] pointer arguments on such a call shall still have valid values, as described in 7.1.4.«

The same applies to memcpy, etc.

The compiler can assume that these pointers are not NULL and optimizes accordingly.

Signature · 1 line

Markus

H. Peter Anvin

reviewer

2016-12-20 21:14 UTC #8 lore raw

Re: [tip:x86/urgent] x86/tools: Fix gcc-7 warning in relocs.c

Quoted 6 lines · Markus Trippelsdorf

7.24.1(2)
»Where an argument declared as size_t n specifies the length of the array for a function, n can have the value zero […] pointer arguments on such a call shall still have valid values, as described in 7.1.4.«

OK, fair enough.

-hpa