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.
(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...
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.
Raising the severity as other assemblers apparently have different defaults, causing hard to find bugs.
*** Issue 7388 has been marked as a duplicate of this issue. ***
The following code: uint func() { asm { naked; inc [EAX]; inc byte ptr [EAX]; inc short ptr [EAX]; inc int ptr [EAX]; inc long ptr [EAX]; } } generates: __D5test24funcFZk: inc byte ptr [EAX] inc byte ptr [EAX] inc word ptr [EAX] inc dword ptr [EAX] inc byte ptr [EAX] <== !!!!! That last is certainly a problem; it should be rejected by the compiler. As for the first instruction, I'm concerned about changing the behavior. Messing with people's existing, working asm code is risky.
(In reply to Walter Bright from comment #5) > As for the first instruction, I'm concerned about changing the behavior. > Messing with people's existing, working asm code is risky. See issue 7388. The problem is that if you copy/paste code from another program, you'll get a different result. It probably should be deprecated, forcing users to explicitly specify the operand size.
(In reply to Walter Bright from comment #5) > The following code: > > uint func() > { > asm > { > naked; > inc long ptr [EAX]; > } > } > > generates: > > __D5test24funcFZk: > inc byte ptr [EAX] <== !!!!! > Filed as https://issues.dlang.org/show_bug.cgi?id=21181 so as not to confuse this issue.
Looking at my ancient Intel 8088 manual, it says: INC [BX] is ambiguous and so must use BYTE PTR or WORD PTR. Since the behavior of iasm is modeled after Intel's syntax, that pretty much settles it.
@WalterBright created dlang/dmd pull request #11603 "fix Issue 2617 - asm silently accepts ambiguous-sized operations" fixing this issue: - fix Issue 2617 - asm silently accepts ambiguous-sized operations https://github.com/dlang/dmd/pull/11603
dlang/dmd pull request #11603 "fix Issue 2617 - asm silently accepts ambiguous-sized operations" was merged into master: - c4805400cdb7a31a07941e5fb741bbbb02b96def by Walter Bright: fix Issue 2617 - asm silently accepts ambiguous-sized operations https://github.com/dlang/dmd/pull/11603