Thread overview
[Issue 2617] New: incorrect code generation for asm{ inc eax }
Jan 25, 2009
d-bugmail
Jan 26, 2009
d-bugmail
[Issue 2617] asm silently accepts ambiguous-sized operations
Jan 26, 2009
d-bugmail
Jan 30, 2012
yebblies
Jan 30, 2012
yebblies
January 25, 2009
http://d.puremagic.com/issues/show_bug.cgi?id=2617

           Summary: incorrect code generation for asm{ inc eax }
           Product: D
           Version: 2.022
          Platform: PC
        OS/Version: Linux
            Status: NEW
          Keywords: wrong-code
          Severity: normal
          Priority: P2
         Component: DMD
        AssignedTo: bugzilla@digitalmars.com
        ReportedBy: jason.james.house@gmail.com


The following code fails to assert.

import tango.core.Atomic;
void main(){
  int j;
  foreach (i; 1..500_000){
    atomicIncrement!(msync.raw, int)(j);
    assert(j == (i%256));
  }
}

Digging deeper, this is a problem because the atomicIncrement is translated to the following assembly.  Note the critical line of "lock incb" which should be "lock inc"

 80496b4:       55                      push   %ebp
 80496b5:       8b ec                   mov    %esp,%ebp
 80496b7:       50                      push   %eax
 80496b8:       8b 45 fc                mov    -0x4(%ebp),%eax
 80496bb:       f0 fe 00                lock incb (%eax)
 80496be:       8b 00                   mov    (%eax),%eax
 80496c0:       8b e5                   mov    %ebp,%esp
 80496c2:       5d                      pop    %ebp
 80496c3:       c3                      ret

Here's the original assembly in the Tango module:
asm
{
  mov EAX, val;
  lock; // lock always needed to make this op atomic
  inc [EAX];
  mov EAX, [EAX];
}

This problem appears in both D1 and D2 and prevents lockless algorithms.


-- 

January 26, 2009
http://d.puremagic.com/issues/show_bug.cgi?id=2617





------- Comment #1 from 2korden@gmail.com  2009-01-26 00:25 -------
(In reply to comment #0)
> The following code fails to assert.
> 
> import tango.core.Atomic;
> void main(){
>   int j;
>   foreach (i; 1..500_000){
>     atomicIncrement!(msync.raw, int)(j);
>     assert(j == (i%256));
>   }
> }
> 

Shouldn't that be:
assert(j == i); // ?

I have noted that Tango atomicIncrement increments least significant byte only, too, and was wondering whether it is intended...


-- 

January 26, 2009
http://d.puremagic.com/issues/show_bug.cgi?id=2617


clugdbug@yahoo.com.au changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|wrong-code                  |accepts-invalid
            Summary|incorrect code generation   |asm silently accepts
                   |for asm{ inc eax }          |ambiguous-sized operations
            Version|2.022                       |1.00




------- Comment #2 from clugdbug@yahoo.com.au  2009-01-26 04:33 -------
The original bug is not valid. It's certainly not wrong-code. And there is no 'inc eax' involved! (inc [EAX]; is completely different). The Tango code has a bug; it should be

inc int ptr [EAX];

You can argue that there's an accepts-invalid bug in the assembler. I hate that it silently accepts ambiguous-sized operations, always assuming 'byte' size. Applies also to

mul [EBX]; // multiplies AL by byte ptr [EAX].

I've been bitten by this quite a few times, on really ancient versions of DMD. It's nothing to do with D2.


-- 

January 30, 2012
http://d.puremagic.com/issues/show_bug.cgi?id=2617


yebblies <yebblies@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |yebblies@gmail.com
           Platform|x86                         |All
            Version|1.00                        |D1 & D2
         OS/Version|Linux                       |All
           Severity|normal                      |major


--- Comment #3 from yebblies <yebblies@gmail.com> 2012-01-30 17:04:24 EST ---
Raising the severity as other assemblers apparently have different defaults, causing hard to find bugs.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
January 30, 2012
http://d.puremagic.com/issues/show_bug.cgi?id=2617


yebblies <yebblies@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |thecybershadow@gmail.com


--- Comment #4 from yebblies <yebblies@gmail.com> 2012-01-30 17:04:33 EST ---
*** Issue 7388 has been marked as a duplicate of this issue. ***

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------