I am working on a port of the open-vm-tools for UnixWare 7.1.4 compiled with the native C & C++ compiler. As I was coding AT&T/USLC enhanced ASM functions equivalent to the __asm__ definitions in vm_atomic.h, I noticed that the GCC x86 __asm__ statement for Atomic_FetchAnd AddUnfenced() is clearly incorrect.

The comments clearly detail the function as "Atomic read (returned), add a value, write". It also states that the results are the "value of the variable before the operation". All functions calling Atomic_FetchAndAddUnfenced() directly or through layered calls:

 * Atomic_FetchAndAdd()
 * Atomic_FetchAndInc()
 * Atomic_FetchAndDec()

all clearly expect the return value to be the value of the variable before the operation.

The ASM code generated for the GCC x86 compiler (as well as the MSC compiler before version 1310) is returning the the uint32 "value" that is to be added to the memory variable, not the value of the memory variable before the operation.

Lifting just the GCC x86 ASM definition from vm_atomic.h

typedef long uint32;

/* Basic atomic type: 32 bits */
typedef struct Atomic_uint32 {
   volatile uint32 value;
} Atomic_uint32;

uint32
Atomic_FetchAndAddUnfenced(Atomic_uint32 *var, // IN
                           uint32 val)         // IN
{
   /* Checked against the Intel manual and GCC --walken */
   __asm__ __volatile__(
      "lock; xaddl %0, %1"
      : "=r" (val),
        "+m" (var->value)
      : "0" (val)
      : "cc"
   );
   return val;
}

and compiling with "gcc -S -O2" yields the following assembly code (comments added)

        .file   "gccasmadd.c"
        .version        "01.01"
        .text
.globl Atomic_FetchAndAddUnfenced
        .type   Atomic_FetchAndAddUnfenced, @function
Atomic_FetchAndAddUnfenced:
        pushl   %ebp
        movl    %esp, %ebp
        movl    8(%ebp), %edx        # var into %edx
        movl    12(%ebp), %eax        # value into %eax
/APP
lock; xaddl %eax, (%edx) # add value to memory addressed by %edx
/NO_APP
        popl    %ebp
ret # return (%eax) value is the incoming parameter "value" !!!!
        .size   Atomic_FetchAndAddUnfenced, .-Atomic_FetchAndAddUnfenced
        .ident  "GCC: (GNU) 4.0.2"

From this, a number of pieces of code cannot function as expected if the GNU C/C++ compiler
is used.  Atomic_FetchAndInc() will always return the value 1.

 * HgfsChannel server and data reference counts will always appear to
   be 1; never zero which causes channel initialization.
 * Linux VMCI delayed datagram queues would never MAX out - always 1.
 * VThreadBaseSimpleNoID() will always come up with a newID of 1 with
   the following statement
     o newID = Atomic_FetchAndInc(&vthreadBaseGlobals.dynamicID);

And there are defines and conditionals in the header which suggests that this may be a generic header that is used across other VMware products. That makes me wonder what other problems may exist.

While I am working with the open-vm-tools-2011.04.25-402641 source drop as a starting point, I have confirmed that the __asm__ code in question in /lib/include/vm_atomic.h is still present in recent the releases.

 * open-vm-tools-2012.05.21-724730
 * open-vm-tools-8.8.2-590212
 * open-vm-tools-8.6.5-621624

Is it possible that vm_atomic.h in the open-vm-tools source tree simply needs a more recent and presumably correct version from the VMware source tree?

-- John Wolfe      UnXis, Inc



------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
open-vm-tools-devel mailing list
open-vm-tools-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/open-vm-tools-devel

Reply via email to