Hi, I recently ran in to a problem with stack corruption [0]. I basically used up more time than I'd like to admit trying to figure out heisenbugs - segfaults, out of memory exceptions, thread crashes, and once I saw an overlapping memory access error (which I'd never seen before, and never saw again).

Anyway, this is what I narrowed it down to eventually in client code.

@safe:
auto foundValue = makeRange("one")
  .algorithm;
useValue(foundValue); // BOOM

With some help from the forums, the essence of the problem was the following:

1. algorithm returned by auto ref.
2. the range's front was "ref front() {...}"

What was happening was that madeRange made a temporary and passed it on to algorithm, which also saw a temporary. The return type on algorithm was auto ref, and since front on the range also returned a ref, I was returning a ref to a value in a temporary. Algorithm was in a dependency, and the range type was inside a different dependency (confession - I wrote both those dependencies but, heh, anyway)

The solutions were to either:

1. slap a return on the front function: ref T front() return {...} (I think - not even sure)
2. remove auto ref from algorithm and return by value

There are a number of issues, and also a number of questions.

- The problem with solution 1 is that you usually do not have control over libraries and their ranges. They may have been declared with return, they may have not. I went through Phobos and saw a lot of ref front() {...}
- The problem with solution 2 is that you want to return by ref so you can to avoid copying.

(Side note: the specification for return ref - https://dlang.org/spec/function.html#return-ref-parameters - says that "inout ref parameters imply the return attribute.", but I got corruption with "ref inout(T) front() inout {...}". Is that a bug with inout?)

The access pattern is safe when:
- The object that is passed in to algorithm is not a temporary.
- The algorithm returns by copy

Best practices seem to be:
- If you are returning by ref from any object accessor, always use the return qualifier. Is there any reason not to?

The only reliable way to solve this from an algorithm author point of view seems to be to have the algorithm return by value. But then you sacrifice efficiency. Are there any other ways? Can it ever be safe to return by auto ref from a generic algorithm?

And finally, I've been keeping note of various gotchas in D, and traps, etc [1] and I'm wondering if anyone else takes note of these things? Can you post links to them here if you do?

I know about the D-idioms thing [2] (great info!!). But it feels that when you write libraries for D, there're a lot of nuanced and non-obvious gotchas like these and they're not documented anywhere that I know of. Should we start doing this somewhere - the name of which is in the title of the post :)

Cheers,
- Ali

[0]: https://forum.dlang.org/thread/hhxabgmrwvqvmezezyym@forum.dlang.org
[1]: https://github.com/aliak00/d-isms/tree/master/da-faq
[2]: https://p0nce.github.io/d-idioms/

On 23.07.19 15:51, aliak wrote:
> Hi, I recently ran in to a problem with stack corruption [0].
[...]
> Anyway, this is what I narrowed it down to eventually in client code.
> 
> @safe:
> auto foundValue = makeRange("one")
>    .algorithm;
> useValue(foundValue); // BOOM
[...]
> And finally, I've been keeping note of various gotchas in D, and traps, etc [1] and I'm wondering if anyone else takes note of these things? Can you post links to them here if you do?
[...]
> [0]: https://forum.dlang.org/thread/hhxabgmrwvqvmezezyym@forum.dlang.org
> [1]: https://github.com/aliak00/d-isms/tree/master/da-faq

If that code leads to memory corruption, it shouldn't compile as @safe. You're looking at a compiler bug, not a gotcha.

Might already be filed here:
https://issues.dlang.org/show_bug.cgi?id=17927

On 23.07.19 15:51, aliak wrote:
> 
> (Side note: the specification for return ref - https://dlang.org/spec/function.html#return-ref-parameters - says that "inout ref parameters imply the return attribute.", but I got corruption with "ref inout(T) front() inout {...}". Is that a bug with inout?)

I think in this case the specification is at fault. It's perfectly fine (and useful) to have an `inout` function that returns `void`.

On Tue, Jul 23, 2019 at 01:51:42PM +0000, aliak via Digitalmars-d wrote: [...]
> With some help from the forums, the essence of the problem was the following:
> 
> 1. algorithm returned by auto ref.
> 2. the range's front was "ref front() {...}"
[...]

Isn't this precisely the sort of problem dip25 / dip1000 are supposed to catch?  Are you compiling with -dip25 -dip1000?  Are DIP25/DIP1000 (or their current implementations) not catching this particular case for some reason?


T

-- 
I've been around long enough to have seen an endless parade of magic new techniques du jour, most of which purport to remove the necessity of thought about your programming problem.  In the end they wind up contributing one or two pieces to the collective wisdom, and fade away in the rearview mirror. -- Walter Bright

On Tuesday, 23 July 2019 at 17:17:25 UTC, H. S. Teoh wrote:
> On Tue, Jul 23, 2019 at 01:51:42PM +0000, aliak via Digitalmars-d wrote: [...]
>> With some help from the forums, the essence of the problem was the following:
>> 
>> 1. algorithm returned by auto ref.
>> 2. the range's front was "ref front() {...}"
> [...]
>
> Isn't this precisely the sort of problem dip25 / dip1000 are supposed to catch?  Are you compiling with -dip25 -dip1000?  Are DIP25/DIP1000 (or their current implementations) not catching this particular case for some reason?
>
>
> T

Not compiling with those no.

Is @safe supposed to solve all memory corruption or is it only if dip1000 and dip25 is on. So default @safe is not @safe?

On Tuesday, 23 July 2019 at 15:40:42 UTC, Timon Gehr wrote:
> On 23.07.19 15:51, aliak wrote:
>> 
>> (Side note: the specification for return ref - https://dlang.org/spec/function.html#return-ref-parameters - says that "inout ref parameters imply the return attribute.", but I got corruption with "ref inout(T) front() inout {...}". Is that a bug with inout?)
>
> I think in this case the specification is at fault. It's perfectly fine (and useful) to have an `inout` function that returns `void`.

Void would not be a ref so return would not be inferred (according to the spec)?

On Tuesday, 23 July 2019 at 15:22:37 UTC, ag0aep6g wrote:
> On 23.07.19 15:51, aliak wrote:
>> Hi, I recently ran in to a problem with stack corruption [0].
> [...]
>> Anyway, this is what I narrowed it down to eventually in client code.
>> 
>> @safe:
>> auto foundValue = makeRange("one")
>>    .algorithm;
>> useValue(foundValue); // BOOM
> [...]
>> And finally, I've been keeping note of various gotchas in D, and traps, etc [1] and I'm wondering if anyone else takes note of these things? Can you post links to them here if you do?
> [...]
>> [0]: https://forum.dlang.org/thread/hhxabgmrwvqvmezezyym@forum.dlang.org
>> [1]: https://github.com/aliak00/d-isms/tree/master/da-faq
>
> If that code leads to memory corruption, it shouldn't compile as @safe. You're looking at a compiler bug, not a gotcha.
>
> Might already be filed here:
> https://issues.dlang.org/show_bug.cgi?id=17927

Hmm, if the spec is not wrong, and return should be inferred, then could it be an ldc bug? If you run this with the llvm sanitizer (on my system at least) you'll get an ASAN stack corruption assert with details.

import std;

struct W(T) {
    T value;
    ref inout(T) front() inout { return value; }
}

auto ref get(T)(W!T value) {
    return value.front;
}

struct S {
    string a;
}

void main() @safe {
    S[] arr;
    arr ~= W!S(S("one")).get;
    writeln(arr[0]);
}

Compiler/run with: ldc2 -fsanitize=address -g -disable-fp-elim temp.d && ./temp

But then again, I have no idea if the dmd version is passing "by luck" or if for some reason dmd properly infers it as return ref and somehow ldc messes that up.

On Wednesday, 24 July 2019 at 07:06:07 UTC, aliak wrote:
> Not compiling with those no.

You probably should.

There is one issue with dub that I know of. When you use dip1000 and use dependencies that don't, things in the dependency might mangle differently between the lib build and your project build if they have an extra attribute inferred. See e.g. https://github.com/dlang-community/stdx-allocator/pull/27#issuecomment-505000664 for more details.

I should probably create a bug somewhere.

> Is @safe supposed to solve all memory corruption or is it only if dip1000 and dip25 is on. So default @safe is not @safe?

From what I understood, after the current transition period, dip1000/dip25 will be always on. Right now, @safe is like @semi_safe

On Wednesday, 24 July 2019 at 07:54:05 UTC, Sebastiaan Koppe wrote:
> On Wednesday, 24 July 2019 at 07:06:07 UTC, aliak wrote:
>> Not compiling with those no.
>
> You probably should.

You're probably right :)

>
> There is one issue with dub that I know of. When you use dip1000 and use dependencies that don't, things in the dependency might mangle differently between the lib build and your project build if they have an extra attribute inferred.

These are the kinds of things I'm afraid of actually. Is that a dub issue though?
Also if the depdendency is not dip1000/dip25 compatible (Which i assume - maybe incorreclty - that most are not) then what? Have you encountered much of that?

On 24.07.19 09:18, aliak wrote:
> On Tuesday, 23 July 2019 at 15:40:42 UTC, Timon Gehr wrote:
>> On 23.07.19 15:51, aliak wrote:
>>>
>>> (Side note: the specification for return ref - https://dlang.org/spec/function.html#return-ref-parameters - says that "inout ref parameters imply the return attribute.", but I got corruption with "ref inout(T) front() inout {...}". Is that a bug with inout?)
>>
>> I think in this case the specification is at fault. It's perfectly fine (and useful) to have an `inout` function that returns `void`.
> 
> Void would not be a ref so return would not be inferred (according to the spec)?

No? The spec says that inout ref parameters imply the return attribute.