Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.
/ druntime Public archive

Comments

Fix __cmp(void[], void[])#1815

Merged
dlang-bot merged 1 commit intodlang:stablefrom
kinke:fixCmp
May 28, 2017
Merged

Fix __cmp(void[], void[])#1815
dlang-bot merged 1 commit intodlang:stablefrom
kinke:fixCmp

Conversation

@kinke
Copy link
Contributor

@kinke kinke commented Apr 22, 2017

The testcase dies with an access violation with DMD 2.074 on Windows:

object.Error@(0): Access Violation
----------------
0x0041C60D in memcmp
0x00402263 in pure nothrow @nogc @safe int object.__cmp!(void, const(void)).__cmp(void[], const(void)[]) at C:\LDC\dmd2\windows\bin\..\..\src\druntime\import\object.d(3407)
0x004021F7 in _Dmain at C:\LDC\ninja-ldc\..\current.d(27)

@PetarKirov
Copy link
Member

I think that (T[] a, U[] b) => a == b, (T[] a, U[] b) => a < b and friends should be disallowed if either is(Unqual!T == void) or is(Unqual!U == void) and allow only a is b.

@PetarKirov
Copy link
Member

And if bitwise comparison is desired, the arrays should be casted to const(ubyte)[].

@kinke
Copy link
Contributor Author

kinke commented Apr 25, 2017

I don't have a strong opinion on a < b for void[] arrays, but I'm very much against disabling the (in)equality check. I'm under the impression that void[] is used quite often, to describe an untyped range of memory, so having to cast all of these manually when checking for (in)equality seems damn ugly and inconvenient to me. (edit: yep, that should be fine via is).

Much more importantly, the front-end should make sure bugs like these can't occur (and the DMD backend hardened in this regard). The nested function indexes the void[] array, returns a void by ref (!), the test takes the (apparently invalid) address of the returned void reference and forwards it to memcmp...
Indexing a void[] array could be useful to obtain addresses (&slice[2], but that can be done more cleanly via slice.ptr + 2), but a function returning ref void and taking the address of a void value is... definitely strange. :]
If returning a ubyte when indexing a void[] array is deemed too strange, I'd be in favor of just prohibiting it (slicing obviously still allowed).

@PetarKirov
Copy link
Member

CC @andralex @WalterBright

@MartinNowak
Copy link
Member

hx for the fix
Filed the ref void-return as Issue 17447 – ref void return should be an error

@dlang-bot dlang-bot merged commit 9093cfe into dlang:stable May 28, 2017
@kinke
Copy link
Contributor Author

kinke commented May 28, 2017

Thanks Martin.

@kinke kinke deleted the fixCmp branch May 28, 2017 11:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants