* [PATCH] x86-64: Fix gcc-7 warning in relocs.c
@ 2016-12-15 12:45 Markus Trippelsdorf
2016-12-19 10:56 ` [tip:x86/urgent] x86/tools: " tip-bot for Markus Trippelsdorf
0 siblings, 1 reply; 9+ messages in thread
From: Markus Trippelsdorf @ 2016-12-15 12:45 UTC (permalink / raw)
To: linux-kernel; +Cc: Thomas Gleixner
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 <markus@trippelsdorf.de>
diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
index 0c2fae8d929d..73eb7fd4aec4 100644
--- a/arch/x86/tools/relocs.c
+++ b/arch/x86/tools/relocs.c
@@ -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 */
--
Markus
^ permalink raw reply related [flat|nested] 9+ messages in thread* [tip:x86/urgent] x86/tools: Fix gcc-7 warning in relocs.c 2016-12-15 12:45 [PATCH] x86-64: Fix gcc-7 warning in relocs.c Markus Trippelsdorf @ 2016-12-19 10:56 ` tip-bot for Markus Trippelsdorf 2016-12-20 9:30 ` H. Peter Anvin 0 siblings, 1 reply; 9+ messages in thread From: tip-bot for Markus Trippelsdorf @ 2016-12-19 10:56 UTC (permalink / raw) To: linux-tip-commits; +Cc: mingo, tglx, markus, hpa, linux-kernel Commit-ID: 7ebb916782949621ff6819acf373a06902df7679 Gitweb: http://git.kernel.org/tip/7ebb916782949621ff6819acf373a06902df7679 Author: Markus Trippelsdorf <markus@trippelsdorf.de> AuthorDate: Thu, 15 Dec 2016 13:45:13 +0100 Committer: Thomas Gleixner <tglx@linutronix.de> 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 <markus@trippelsdorf.de> Link: http://lkml.kernel.org/r/20161215124513.GA289@x4 Signed-off-by: Thomas Gleixner <tglx@linutronix.de> --- arch/x86/tools/relocs.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c index 0c2fae8..73eb7fd 100644 --- a/arch/x86/tools/relocs.c +++ b/arch/x86/tools/relocs.c @@ -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 */ ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [tip:x86/urgent] x86/tools: Fix gcc-7 warning in relocs.c 2016-12-19 10:56 ` [tip:x86/urgent] x86/tools: " tip-bot for Markus Trippelsdorf @ 2016-12-20 9:30 ` H. Peter Anvin 2016-12-20 10:00 ` Markus Trippelsdorf 0 siblings, 1 reply; 9+ messages in thread From: H. Peter Anvin @ 2016-12-20 9:30 UTC (permalink / raw) To: markus, tglx, linux-kernel, mingo, linux-tip-commits 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 On 12/19/16 02:56, tip-bot for Markus Trippelsdorf wrote: > Commit-ID: 7ebb916782949621ff6819acf373a06902df7679 > Gitweb: http://git.kernel.org/tip/7ebb916782949621ff6819acf373a06902df7679 > Author: Markus Trippelsdorf <markus@trippelsdorf.de> > AuthorDate: Thu, 15 Dec 2016 13:45:13 +0100 > Committer: Thomas Gleixner <tglx@linutronix.de> > 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 <markus@trippelsdorf.de> > Link: http://lkml.kernel.org/r/20161215124513.GA289@x4 > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > > --- > arch/x86/tools/relocs.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c > index 0c2fae8..73eb7fd 100644 > --- a/arch/x86/tools/relocs.c > +++ b/arch/x86/tools/relocs.c > @@ -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 */ > s ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [tip:x86/urgent] x86/tools: Fix gcc-7 warning in relocs.c 2016-12-20 9:30 ` H. Peter Anvin @ 2016-12-20 10:00 ` Markus Trippelsdorf 2016-12-20 11:10 ` H. Peter Anvin 0 siblings, 1 reply; 9+ messages in thread From: Markus Trippelsdorf @ 2016-12-20 10:00 UTC (permalink / raw) To: H. Peter Anvin; +Cc: tglx, linux-kernel, mingo, linux-tip-commits On 2016.12.20 at 01:30 -0800, H. Peter Anvin wrote: > 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [tip:x86/urgent] x86/tools: Fix gcc-7 warning in relocs.c 2016-12-20 10:00 ` Markus Trippelsdorf @ 2016-12-20 11:10 ` H. Peter Anvin 2016-12-20 11:51 ` Markus Trippelsdorf 0 siblings, 1 reply; 9+ messages in thread From: H. Peter Anvin @ 2016-12-20 11:10 UTC (permalink / raw) To: Markus Trippelsdorf; +Cc: tglx, linux-kernel, mingo, linux-tip-commits On 12/20/16 02:00, Markus Trippelsdorf wrote: > On 2016.12.20 at 01:30 -0800, H. Peter Anvin wrote: >> 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. H > 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. > 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [tip:x86/urgent] x86/tools: Fix gcc-7 warning in relocs.c 2016-12-20 11:10 ` H. Peter Anvin @ 2016-12-20 11:51 ` Markus Trippelsdorf 2016-12-20 18:32 ` hpa 0 siblings, 1 reply; 9+ messages in thread From: Markus Trippelsdorf @ 2016-12-20 11:51 UTC (permalink / raw) To: H. Peter Anvin; +Cc: tglx, linux-kernel, mingo, linux-tip-commits On 2016.12.20 at 03:10 -0800, H. Peter Anvin wrote: > On 12/20/16 02:00, Markus Trippelsdorf wrote: > > On 2016.12.20 at 01:30 -0800, H. Peter Anvin wrote: > >> 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. > > Well, s/gcc/glibc/ then. > > > 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. -- Markus ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [tip:x86/urgent] x86/tools: Fix gcc-7 warning in relocs.c 2016-12-20 11:51 ` Markus Trippelsdorf @ 2016-12-20 18:32 ` hpa 2016-12-20 19:31 ` Markus Trippelsdorf 0 siblings, 1 reply; 9+ messages in thread From: hpa @ 2016-12-20 18:32 UTC (permalink / raw) To: Markus Trippelsdorf; +Cc: tglx, linux-kernel, mingo, linux-tip-commits On December 20, 2016 3:51:09 AM PST, Markus Trippelsdorf <markus@trippelsdorf.de> wrote: >On 2016.12.20 at 03:10 -0800, H. Peter Anvin wrote: >> On 12/20/16 02:00, Markus Trippelsdorf wrote: >> > On 2016.12.20 at 01:30 -0800, H. Peter Anvin wrote: >> >> 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. >> >> Well, s/gcc/glibc/ then. >> >> > 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. 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? -- Sent from my Android device with K-9 Mail. Please excuse my brevity. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [tip:x86/urgent] x86/tools: Fix gcc-7 warning in relocs.c 2016-12-20 18:32 ` hpa @ 2016-12-20 19:31 ` Markus Trippelsdorf 2016-12-20 20:44 ` H. Peter Anvin 0 siblings, 1 reply; 9+ messages in thread From: Markus Trippelsdorf @ 2016-12-20 19:31 UTC (permalink / raw) To: hpa; +Cc: tglx, linux-kernel, mingo, linux-tip-commits On 2016.12.20 at 10:32 -0800, hpa@zytor.com wrote: > On December 20, 2016 3:51:09 AM PST, Markus Trippelsdorf <markus@trippelsdorf.de> wrote: > >On 2016.12.20 at 03:10 -0800, H. Peter Anvin wrote: > >> On 12/20/16 02:00, Markus Trippelsdorf wrote: > >> > On 2016.12.20 at 01:30 -0800, H. Peter Anvin wrote: > >> >> 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. > >> > >> Well, s/gcc/glibc/ then. > >> > >> > 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. > > 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. -- Markus ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [tip:x86/urgent] x86/tools: Fix gcc-7 warning in relocs.c 2016-12-20 19:31 ` Markus Trippelsdorf @ 2016-12-20 20:44 ` H. Peter Anvin 0 siblings, 0 replies; 9+ messages in thread From: H. Peter Anvin @ 2016-12-20 20:44 UTC (permalink / raw) To: Markus Trippelsdorf; +Cc: tglx, linux-kernel, mingo, linux-tip-commits On 12/20/16 11:31, Markus Trippelsdorf wrote: > > 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-12-20 21:14 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-12-15 12:45 [PATCH] x86-64: Fix gcc-7 warning in relocs.c Markus Trippelsdorf 2016-12-19 10:56 ` [tip:x86/urgent] x86/tools: " tip-bot for Markus Trippelsdorf 2016-12-20 9:30 ` H. Peter Anvin 2016-12-20 10:00 ` Markus Trippelsdorf 2016-12-20 11:10 ` H. Peter Anvin 2016-12-20 11:51 ` Markus Trippelsdorf 2016-12-20 18:32 ` hpa 2016-12-20 19:31 ` Markus Trippelsdorf 2016-12-20 20:44 ` H. Peter Anvin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).