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]>
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
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
Signed-off-by: Thomas Gleixner <[email protected]>
---
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 */
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 <[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
> Signed-off-by: Thomas Gleixner <[email protected]>
>
> ---
> 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
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
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
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
On December 20, 2016 3:51:09 AM PST, Markus Trippelsdorf <[email protected]> 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.
On 2016.12.20 at 10:32 -0800, [email protected] wrote:
> On December 20, 2016 3:51:09 AM PST, Markus Trippelsdorf <[email protected]> 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
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