Bug in glibc `div()` code?

 

As quoted in "Integer division rounding with negatives in C++", in C before C99 (ie in C89) and in C++ before C++11 (ie in C++98 and C++03), for an integer division computation where either operand is negative the sign of the remainder (or equivalently, the rounding direction of the quotient) is implementation-defined.

Then comes the standard function std::div which is specified to truncate the quotient towards zero (ie the remainder has the same sign as the dividend (numerator)) (see for example this answer to "what is purpose of div() library function?").

Here is glibc's code for div() (source) (also quoted in "Is div function useful (stdlib.h)?"):

(Note: div_t is defined as: 

typedef struct
  {
    int quot;
    int rem;
  } div_t;

-- end note.) 

/* Return the `div_t' representation of NUMER over DENOM.  */
div_t
div (numer, denom)
     int numer, denom;
{
  div_t result;

  result.quot = numer / denom;
  result.rem = numer % denom;

  /* The ANSI standard says that |QUOT| <= |NUMER / DENOM|, where
     NUMER / DENOM is to be computed in infinite precision.  In
     other words, we should always truncate the quotient towards
     zero, never -infinity.  Machine division and remainer may
     work either way when one or both of NUMER or DENOM is
     negative.  If only one is negative and QUOT has been
     truncated towards -infinity, REM will have the same sign as
     DENOM and the opposite sign of NUMER; if both are negative
     and QUOT has been truncated towards -infinity, REM will be
     positive (will have the opposite sign of NUMER).  These are
     considered `wrong'.  If both are NUM and DENOM are positive,
     RESULT will always be positive.  This all boils down to: if
     NUMER >= 0, but REM < 0, we got the wrong answer.  In that
     case, to get the right answer, add 1 to QUOT and subtract
     DENOM from REM.  */

  if (numer >= 0 && result.rem < 0)
    {
      ++result.quot;
      result.rem -= denom;
    }

  return result;
}

As you can see there is a test after the big comment block, whose purpose is to "correct" the result if the built-in division truncates towards -infinity instead of towards zero.

Now the question:

Isn't there a bug in that code?

Let's first consider the example call div(42, -5) . "In math" 42/-5 is exactly -8.4 , so theoretically in C89 and C++03 42 / -5 could yield either -8 (truncated) or -9 (floored) depending on the implementation. Reading the code:

  • If 42 / -5 yields -8 then 42 % -5 yields 2 (as 42 == -8 * -5 + 2 ), so the test is (42 >= 0 && 2 < 0) which is not true and the above function returns -8 and 2 , as wanted;
  • If 42 / -5 yields -9 then 42 % -5 yields -3 (as 42 == -9 * -5 + -3 ), so the test is (42 >= 0 && -3 < 0) which is true, so the above function returns the "corrected" -9 + 1 and -3 - -5 , ie -8 and 2 , as wanted.

    • But now let's consider the call div(-42, 5) (signs inverted):

  • If -42 / 5 yields -8 then -42 % 5 yields -2 (as -42 == -8 * 5 + -2 ), so the test is (-42 >= 0 && -2 < 0) which is not true and the above function returns -8 and -2 , as wanted;
  • If -42 / 5 yields -9 then -42 % 5 yields 3 (as -42 == -9 * 5 + 3 ), so the test is (-42 >= 0 && 3 < 0) which... is not true! and the above function returns -9 and 3 instead of -8 and -2 !

    • The comment in the code above first seems right when it says that the situation that needs a correction is when "REM has the opposite sign of NUMER", but then it makes the huge simplification "This all boils down to: if NUMER >= 0, but REM < 0, we got the wrong answer", which seems wrong (incomplete) to me because it omits the case "if NUMER < 0, but REM > 0" ( -42 and 3 in the previous example).

    I can hardly believe that such a bug would have remained unnoticed since 1992 or 1990 (apparently someone tried to "fix" it but it still seems incorrect because div(-42, 5) could return -10 and 8 )... Arguably, most implementations have been truncating towards zero by default (and all are required to do so starting from C99 and C++11, so the issue is "moot" in the latest Standards 1) so the bug wouldn't manifest on them, but still... Maybe I'm missing something here?

    Thank you for any insights.

    1 (Edit) As for "the issue is moot in C++11 and C99 (and newer)": accordingly, in these Standards the built-in division is required to truncate towards zero, so we never need to adjust the result, but doesn't that then mean that the present implementation is more complex than needed and unnecessarily inefficient? The "big comment" is obsolete and the if test useless, so shouldn't that part just be removed entirely?

    As the original author of the code, I have to say: You're right. It's broken. We had no systems that behaved "the wrong way" for testing, and I probably wrote the above too late (or early...) in the day or something.

    We're saved by the newer standards, and the entire thing should be cleaned out, maybe with a small #ifdef (and corrected adjustment code) for pre-C99 if necessary.

    (Also I'll note that the original was not indented with GNU-style indentation :-) )

    But in this case numer is equal to -42, (so it is negative) this made the condition false automatically. In fact if you try to execute a simple code which contains the function you would see that (debugging it) the if statement: 

    // false in this case, numer = -42 and result.rem = -2
if(numer >= 0 && result.rem < 0)

    Here you can see the compiled result:enter link description here

    I hope that the answer is correct and will help you to understand the error.

    链接地址: http://www.djcxy.com/p/72966.html

    上一篇: 什么是const指针的要点？

    下一篇: glibc`div（）`代码中的错误？