Bug 16708 – opAssign and struct-member padding break comparison and invariants

Status
NEW
Severity
major
Priority
P2
Component
dmd
Product
D
Version
D2
Platform
x86_64
OS
Linux
Creation time
2016-11-21T13:51:13Z
Last change time
2024-12-13T18:50:45Z
Keywords
industry
Assigned to
No Owner
Creator
Tomer Filiba (weka)
Moved to GitHub: dmd#19208 →

Comments

Comment #0 by tomer — 2016-11-21T13:51:13Z
When a struct defines an opAssign it is obviously "on its own", but it can't possibly assign to the padding (which is something built-in assignment does). Suppose you get the memory for the struct from a memory-pool and you assign a value to it directly. The memory was uninitialized at first, but there are no destructors involved, so there's nothing inherently wrong. Here's an example: ======================================================== import std.stdio; struct MyShort { ushort value; void opAssign(MyShort rhs) nothrow @safe @nogc { value = rhs.value; } } struct YourStruct { MyShort a; int b; } void main() { YourStruct ys1; YourStruct ys2; (cast(ubyte*)&ys2)[0 .. YourStruct.sizeof] = 0xff; writeln("ys1=", (cast(ubyte*)&ys1)[0 .. YourStruct.sizeof]); writeln("ys2=", (cast(ubyte*)&ys2)[0 .. YourStruct.sizeof]); // ys1=[0, 0, 0, 0, 0, 0, 0, 0] // ys2=[255, 255, 255, 255, 255, 255, 255, 255] ys1 = YourStruct(MyShort(17), 18); ys2 = YourStruct(MyShort(17), 18); writeln("ys1=", (cast(ubyte*)&ys1)[0 .. YourStruct.sizeof]); writeln("ys2=", (cast(ubyte*)&ys2)[0 .. YourStruct.sizeof]); // ys1=[17, 0, 0, 0, 18, 0, 0, 0] // ys2=[17, 0, 255, 255, 18, 0, 0, 0] <<< does not assign the padding writeln(ys1.a == ys2.a); // true writeln(ys1.b == ys2.b); // true writeln(ys1 == ys2); // false } ======================================================== So here we get two variables for which memcmp returns != 0, but all their fields are equal.
Comment #1 by eyal.lotem — 2016-11-21T14:02:02Z
Another way to put this is that: x = y; Uses a "fast path" (memcpy) when the type of 'x' doesn't define opAssign, or a "slow path" when the type of 'x' defines opAssign. The fast path does the correct thing and copies fields and padding bytes. The slow path does the wrong thing and calls opAssign to copy fields, but leaves padding bytes potentially uninitialized (which makes some operations *incorrect*, e.g: hashing the object).
Comment #2 by ag0aep6g — 2016-11-21T14:49:35Z
I'd argue that equality should not compare the padding. See issue 16216.
Comment #3 by robert.schadek — 2024-12-13T18:50:45Z
THIS ISSUE HAS BEEN MOVED TO GITHUB https://github.com/dlang/dmd/issues/19208 DO NOT COMMENT HERE ANYMORE, NOBODY WILL SEE IT, THIS ISSUE HAS BEEN MOVED TO GITHUB