Bug 17821 – atomicStore compile error when target is larger than source

Status
NEW
Severity
enhancement
Priority
P4
Component
druntime
Product
D
Version
D2
Platform
All
OS
All
Creation time
2017-09-10T11:17:07Z
Last change time
2024-12-07T13:37:36Z
Keywords
industry
Assigned to
No Owner
Creator
Eyal
Moved to GitHub: dmd#17350 →

Comments

Comment #0 by eyal — 2017-09-10T11:17:07Z
LDC version of core.atomic.atomicStore: void atomicStore(MemoryOrder ms = MemoryOrder.seq, T, V1)( ref shared T val, V1 newval ) pure nothrow @nogc @trusted if( __traits( compiles, { val = newval; } ) ) { alias Int = _AtomicType!T; auto target = cast(shared(Int)*)cast(void*)&val; auto newPtr = cast(Int*)&newval; // this cast is wrong! llvm_atomic_store!Int(*newPtr, target, _ordering!(ms)); } If V1 is a smaller type than T it will cast ptr-to-V1 to ptr-to-T and dereference that to read garbage. Example: shared ulong x; atomicStore(x, 0); // this assigns the low 32 bits correctly, but the top 32 bits of x are set to garbage from the stack
Comment #1 by johanengelen — 2017-09-10T19:36:18Z
The bad codegen bug is LDC specific. DMD errors upon the atomicStore!(ulong,int) call ("core/atomic.d(1185): Error: bad type/size of operands 'mov'"). Test cases: ``` { shared ulong x = 0x1234_5678_8765_4321; atomicStore(x, 0); assert(x == 0); } { struct S { ulong x; alias x this; } shared S s; s = 0x1234_5678_8765_4321; atomicStore(s, 0); assert(s.x == 0); } ``` LDC fix: https://github.com/ldc-developers/druntime/pull/102 I am assuming that we want to preserve the current atomicStore interface, that says it can be called with all types for which `val = newval` is valid.
Comment #2 by johanengelen — 2017-09-11T21:05:16Z
Fixed in LDC 1.4.0. The remaining issue is compilation error with DMD for: ``` import core.atomic; shared ulong x; atomicStore(x, 0); ```
Comment #3 by stanislav.blinov — 2021-12-08T19:35:34Z
This compiles since 2.089.1: void main() { import core.atomic; { shared ulong x = 0x1234_5678_8765_4321; atomicStore(x, 0); assert(x == 0); } { struct S { ulong x; alias x this; } shared S s; s = 0x1234_5678_8765_4321; atomicStore(s, S(0)); // note this is not atomicStore(s, 0); assert(s.x == 0); } } I'm not sure that atomicStore should even try alias this. This can probably be closed as resolved now.
Comment #4 by robert.schadek — 2024-12-07T13:37:36Z
THIS ISSUE HAS BEEN MOVED TO GITHUB https://github.com/dlang/dmd/issues/17350 DO NOT COMMENT HERE ANYMORE, NOBODY WILL SEE IT, THIS ISSUE HAS BEEN MOVED TO GITHUB