[erlang-bugs] inet:getstat/2 broken on some 64bit platforms

[Patrik Nyblom]

On 12/17/2012 02:52 PM, Tim Watson wrote:
> In the RabbitMQ Management Plugin (http://www.rabbitmq.com/management.html) we've seen _oct variants wrap after 2^32 bytes are sent, even on 64bit platforms. Looking at inet_drv.c, the code in inet_fill_stat assumes that unsigned longs are always 32 bit, which is a false assumption.
Well, actually inet_fill_stat doesn't, it uses only the lowest 32 bits, 
regardless of the size of unsigned long. The bug is in inet_input_count 
and inet_output_count, they assume that the counter will wrap.

The simple fix would be something like:
---------------------
--- a/erts/emulator/drivers/common/inet_drv.c
+++ b/erts/emulator/drivers/common/inet_drv.c
@@ -7836,7 +7836,7 @@ static ErlDrvSSizeT inet_ctl(inet_descriptor* 
desc, int cmd, char* buf,
  static void inet_output_count(inet_descriptor* desc, ErlDrvSizeT len)
  {
      unsigned long n = desc->send_cnt + 1;
-    unsigned long t = desc->send_oct[0] + len;
+    unsigned long t = (desc->send_oct[0] + len) & 0xFFFFFFFFUL;
      int c = (t < desc->send_oct[0]);
      double avg = desc->send_avg;

@@ -7856,7 +7856,7 @@ static void inet_output_count(inet_descriptor* 
desc, ErlDrvSizeT len)
  static void inet_input_count(inet_descriptor* desc, ErlDrvSizeT len)
  {--- a/erts/emulator/drivers/common/inet_drv.c
+++ b/erts/emulator/drivers/common/inet_drv.c
@@ -7836,7 +7836,7 @@ static ErlDrvSSizeT inet_ctl(inet_descriptor* 
desc, int cmd, char* buf,
  static void inet_output_count(inet_descriptor* desc, ErlDrvSizeT len)
  {
      unsigned long n = desc->send_cnt + 1;
-    unsigned long t = desc->send_oct[0] + len;
+    unsigned long t = (desc->send_oct[0] + len) & 0xFFFFFFFFUL;
      int c = (t < desc->send_oct[0]);
      double avg = desc->send_avg;

@@ -7856,7 +7856,7 @@ static void inet_output_count(inet_descriptor* 
desc, ErlDrvSizeT len)
  static void inet_input_count(inet_descriptor* desc, ErlDrvSizeT len)
  {
      unsigned long n = desc->recv_cnt + 1;
-    unsigned long t = desc->recv_oct[0] + len;
+    unsigned long t = (desc->recv_oct[0] + len) & 0xFFFFFFFFUL;
      int c = (t < desc->recv_oct[0]);
      double avg = desc->recv_avg;
      double dvi;
(E
      unsigned long n = desc->recv_cnt + 1;
-    unsigned long t = desc->recv_oct[0] + len;
+    unsigned long t = (desc->recv_oct[0] + len) & 0xFFFFFFFFUL;
      int c = (t < desc->recv_oct[0]);
      double avg = desc->recv_avg;
      double dvi;
---------------------
Of course it would be better to use 64bit integers directly in the 64bit 
port, but this would fix the problem for now...

> The standard simply states that `unsigend long' is not smaller than 32 bits (or despite theoretically being untrue, that in practical use `sizeof(a) <= sizeof(long)' should probably hold) - anyway there are many platforms standardised on ILP64 and LP64 schemes, and on those platforms we end up with 128 bit counters and subsequently lose bits 32-63 and 96-127. Is there a compelling reason we don't want to use the definitions in stdint.h here? AFAIK this is only missing on OSF/1 4.0 (and under MSVC 9) and whilst it may be incomplete on some platforms the type defs are at least there. Either way this code appears to be wrong for many 64 bit unices.
We have a lot of types in our sys.h that could have been (and can be) 
used. I suppose the code is just old and did not account for 64bit at all.
>
> Cheers,
>
> Tim Watson
> Staff Engineer
> RabbitMQ / VMWare
> _______________________________________________
> erlang-bugs mailing list
> erlang-bugs@REDACTED
> http://erlang.org/mailman/listinfo/erlang-bugs
Thanks for reporting!
Cheers,
/Patrik