toHexString always returns stack allocated string

[dlangBugzillaToGithub]

jbc.engelen (@JohanEngelen) reported this on 2016-09-21T12:04:13Z

Transfered from https://issues.dlang.org/show_bug.cgi?id=16519

CC List

• bugzilla (@WalterBright)
• bugzilla (@WalterBright)
• greensunny12
• schveiguy (@schveiguy)

Description

What's the bug in the following code:

```d
import std.digest.md;
import std.stdio;

pragma(inline, false) // just in case
string getHash()
{
    ubyte[16] hash = [1,2,3,4,5,6,6,78,8,8,7,7,6,3,2,3];
    string a = toHexString(hash);
    return a;
}

pragma(inline, false) // just in case
void destroystack()
{
    writeln("asd","asd","asd","asd","asd","asd");
}

void main()
{
    string a = getHash();
    destroystack();
    writeln(a);
}
```
It prints garbage after the "asdasdasdasdasdasd" line.

Hint: when changing
```
    string a = toHexString(hash);
    return a;
```
to
```
    return toHexString(hash);
```
the compiler errors with:
`Error: escaping reference to stack allocated value returned by toHexString(hash)`.

So:
-  the documentation of toHexString says that the overloads returning a string return a GC allocated string
- the _implementation_ of `string toHexString(...)` does a `new char[16]`, so GC allocates.
- `string a = toHexString(hash);` calls `char[num*2] toHexString(...)` instead of `string toHexString(...)`.   OOPS.

I don't know whether this is a compiler bug (choosing the wrong overload) or a Phobos bug (overloads don't work like that).

[dlangBugzillaToGithub]

jbc.engelen (@JohanEngelen) commented on 2016-09-21T13:14:16Z

https://forum.dlang.org/thread/[email protected]


So not a bug maybe, but something very nasty. Please adjust the API or (greatly) expand the documentation.

[dlangBugzillaToGithub]

schveiguy (@schveiguy) commented on 2016-09-21T20:27:09Z

The underlying issue is the allowance to slice a static array RValue, which can never be valid. This is issue 12625.

Now, we may fix this, and then what happens is that your code that compiled before now doesn't compile with the same message as the "bad" version. But that's not good either. toHexString should work on a static array without creating another static array.

So I'll leave this bug open.

[dlangBugzillaToGithub]

greensunny12 commented on 2018-02-27T22:13:47Z

FWIW -dip1000 warns about this problem:

foo.d(8): Deprecation: slice of static array temporary returned by toHexString(hash) assigned to longer lived variable a

but I still think we should do something about the API.

[dlangBugzillaToGithub]

dfj1esp02 commented on 2018-03-20T15:40:45Z

(In reply to Seb from comment #3)
> foo.d(8): Deprecation: slice of static array temporary returned by
> toHexString(hash) assigned to longer lived variable a
> 
> but I still think we should do something about the API.

Slicing a temporary and storing it anywhere should be deprecated in unsafe code too. It should be valid to only pass it as a function argument.

[dlangBugzillaToGithub]

bugzilla (@WalterBright) commented on 2019-12-30T15:01:32Z

(In reply to Steven Schveighoffer from comment #2)
> toHexString should work on a static array without
> creating another static array.
> 
> So I'll leave this bug open.

What's wrong with this?

(In reply to Seb from comment #3)
> FWIW -dip1000 warns about this problem:
> 
> foo.d(8): Deprecation: slice of static array temporary returned by
> toHexString(hash) assigned to longer lived variable a

Can't find any information on when this deprecation cycle will end. Do you know?

[Inkrementator]

@LightBender resolved

https://dlang.org/library/std/digest/to_hex_string.html

The function overloads returning a string allocate their return values using the GC. The versions returning static arrays use pass-by-value for the return value, effectively avoiding dynamic allocation.