Dmitry,

Thanks for removing the blinders on my eyes.
Obviously I read through that many times and only saw the add.

-- John :-[

Dmitry Torokhov wrote:
John,

Unlike regular "addl", "xaddl %eax, (%edx)" is "Exchange and Add" instruction, 
it exchanges old value in %(ebx) with data in %eax and adds and stores the 
result in %(ebx), leaving %eax with old data that is later returned.

Thanks,
Dmitry


On Tuesday, July 24, 2012 12:58:08 PM John Wolfe wrote:
  
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