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

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

[Markus Trippelsdorf]

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

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

[tip-bot for Markus Trippelsdorf]

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 */

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

[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.

	-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

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

[Markus Trippelsdorf]

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

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

[H. Peter Anvin]

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

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

[Markus Trippelsdorf]

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

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

[hpa]

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.

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

[Markus Trippelsdorf]

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

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

[H. Peter Anvin]

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