Re: qsort comparision function bug

[Damien Miller]

On Mon, 21 Jan 2019, Dariusz Sendkowski wrote:

> Wouldn't it lead to undefined behavior?
> According to the standard: "... The value of the result of an integer
> arithmetic or conversion function cannot be represented (7.8.2.1, 7.8.2.2,
> 7.8.2.3, 7.8.2.4, 7.22.6.1, 7.22.6.2, 7.22.1) ..."
> This dump case is not compiled with -fwrapv flag, so I guess it might lead
> to undefined behavior.

OpenBSD sets -fwrapv by default. From clang-local(1):

 -   The -fwrapv option to treat signed integer overflows as defined is
 enabled by default to prevent dangerous optimizations which could
 remove security critical overflow checks.

Re: qsort comparision function bug

[Dariusz Sendkowski]

Wouldn't it lead to undefined behavior?
According to the standard: "... The value of the result of an integer
arithmetic or conversion function cannot be represented (7.8.2.1, 7.8.2.2,
7.8.2.3, 7.8.2.4, 7.22.6.1, 7.22.6.2, 7.22.1) ..."
This dump case is not compiled with -fwrapv flag, so I guess it might lead
to undefined behavior.



pon., 21 sty 2019 o 20:55 Otto Moerbeek  napisał(a):

> Hi,
>
> This code is buggy, since subtraction and then casting to int will not
> produce the right result if the values are too far apart.
>
> There must be more like this in the tree any volunteers?
>
> In general, you don't want to do subtraction, even for ints. For
> example: compare INT_MIN and 1.  INT_MIN - 1 is a large positive
> number. Sadly the internet is full of bad examples using subtraction.
>
> -Otto
>
> Index: optr.c
> ===
> RCS file: /cvs/src/sbin/dump/optr.c,v
> retrieving revision 1.39
> diff -u -p -r1.39 optr.c
> --- optr.c  12 Oct 2015 15:12:44 -  1.39
> +++ optr.c  21 Jan 2019 19:45:24 -
> @@ -423,6 +423,7 @@ datesort(const void *a1, const void *a2)
>
> diff = strncmp(d1->dd_name, d2->dd_name, sizeof(d1->dd_name));
> if (diff == 0)
> -   return (d2->dd_ddate - d1->dd_ddate);
> +   return (d2->dd_ddate < d1->dd_ddate ? -1 :
> +   (d2->dd_ddate > d1->dd_ddate ? 1 : 0));
> return (diff);
>  }
>
>

qsort comparision function bug

[Otto Moerbeek]

Hi,

This code is buggy, since subtraction and then casting to int will not
produce the right result if the values are too far apart.

There must be more like this in the tree any volunteers?

In general, you don't want to do subtraction, even for ints. For
example: compare INT_MIN and 1.  INT_MIN - 1 is a large positive
number. Sadly the internet is full of bad examples using subtraction.

-Otto

Index: optr.c
===
RCS file: /cvs/src/sbin/dump/optr.c,v
retrieving revision 1.39
diff -u -p -r1.39 optr.c
--- optr.c  12 Oct 2015 15:12:44 -  1.39
+++ optr.c  21 Jan 2019 19:45:24 -
@@ -423,6 +423,7 @@ datesort(const void *a1, const void *a2)
 
diff = strncmp(d1->dd_name, d2->dd_name, sizeof(d1->dd_name));
if (diff == 0)
-   return (d2->dd_ddate - d1->dd_ddate);
+   return (d2->dd_ddate < d1->dd_ddate ? -1 :
+   (d2->dd_ddate > d1->dd_ddate ? 1 : 0));
return (diff);
 }