Thread overview | |||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
|
April 14, 2016 What is going on in this code? | ||||
---|---|---|---|---|
| ||||
I have encountered a weird error that only seems to occur when building in debug mode. In release everything works fine. I wanted to post here before raising a new issue in case I'm just horribly blind at what is happening here. --- import std.stdio; import std.bitmanip; ubyte[] serialize(uint t) { ubyte[] bytes = nativeToBigEndian(t); return bytes; } void main() { writeln(serialize(0)); } --- $ dub --build=debug Performing "debug" build using dmd for x86. test ~master: target for configuration "application" is up to date. To force a rebuild of up-to-date targets, run again with --force. Running .\test.exe [12, 254, 24, 0] $ dub --build=release Performing "release" build using dmd for x86. test ~master: target for configuration "application" is up to date. To force a rebuild of up-to-date targets, run again with --force. Running .\test.exe [0, 0, 0, 0] Why is it returning a different range in debug but not in release? |
April 14, 2016 Re: What is going on in this code? | ||||
---|---|---|---|---|
| ||||
Posted in reply to Carlin | On 14.04.2016 12:14, Carlin wrote:
> import std.stdio;
> import std.bitmanip;
>
> ubyte[] serialize(uint t)
> {
> ubyte[] bytes = nativeToBigEndian(t);
> return bytes;
> }
>
> void main()
> {
> writeln(serialize(0));
> }
Your code is wrong. It's really easy to get wrong, though. Too easy probably.
nativeToBigEndian returns a fixed-size array. That's a value type. By assigning it to a ubyte[], you're slicing the return value. That is, you make a reference to temporary data. The reference becomes invalid as soon the assignment is over, so you're returning a slice to garbage memory.
To make a ubyte[] from the result of nativeToBigEndian, you have to dup it:
----
ubyte[] serialize(uint t)
{
return nativeToBigEndian(t).dup;
}
----
|
April 14, 2016 Re: What is going on in this code? | ||||
---|---|---|---|---|
| ||||
Posted in reply to ag0aep6g | On Thursday, 14 April 2016 at 10:29:43 UTC, ag0aep6g wrote:
> On 14.04.2016 12:14, Carlin wrote:
>> import std.stdio;
>> import std.bitmanip;
>>
>> ubyte[] serialize(uint t)
>> {
>> ubyte[] bytes = nativeToBigEndian(t);
>> return bytes;
>> }
>>
>> void main()
>> {
>> writeln(serialize(0));
>> }
>
> Your code is wrong. It's really easy to get wrong, though. Too easy probably.
>
> nativeToBigEndian returns a fixed-size array. That's a value type. By assigning it to a ubyte[], you're slicing the return value. That is, you make a reference to temporary data. The reference becomes invalid as soon the assignment is over, so you're returning a slice to garbage memory.
>
> To make a ubyte[] from the result of nativeToBigEndian, you have to dup it:
> ----
> ubyte[] serialize(uint t)
> {
> return nativeToBigEndian(t).dup;
> }
> ----
Thanks, that works perfectly! I couldn't figure it out and I knew I had to be missing something really basic. I still don't understand though why it works in release but not in debug.
|
April 14, 2016 Re: What is going on in this code? | ||||
---|---|---|---|---|
| ||||
Posted in reply to Carlin | On Thursday, 14 April 2016 at 13:08:56 UTC, Carlin wrote:
> On Thursday, 14 April 2016 at 10:29:43 UTC, ag0aep6g wrote:
>> [...]
>
> Thanks, that works perfectly! I couldn't figure it out and I knew I had to be missing something really basic. I still don't understand though why it works in release but not in debug.
because with release build dub will enable inlining. So I guess there is a stack frame elimination
|
April 14, 2016 Re: What is going on in this code? | ||||
---|---|---|---|---|
| ||||
Posted in reply to ag0aep6g | On 4/14/16 6:29 AM, ag0aep6g wrote: > On 14.04.2016 12:14, Carlin wrote: >> import std.stdio; >> import std.bitmanip; >> >> ubyte[] serialize(uint t) >> { >> ubyte[] bytes = nativeToBigEndian(t); >> return bytes; >> } >> >> void main() >> { >> writeln(serialize(0)); >> } > > Your code is wrong. It's really easy to get wrong, though. Too easy > probably. > > nativeToBigEndian returns a fixed-size array. That's a value type. By > assigning it to a ubyte[], you're slicing the return value. That is, you > make a reference to temporary data. The reference becomes invalid as > soon the assignment is over, so you're returning a slice to garbage memory. > > To make a ubyte[] from the result of nativeToBigEndian, you have to dup it: > ---- > ubyte[] serialize(uint t) > { > return nativeToBigEndian(t).dup; > } > ---- That is awful. I propose we make slicing such a return value implicitly an error. I can't think of a valid reason for allowing it. I'm not the only one: https://issues.dlang.org/show_bug.cgi?id=12625 -Steve |
April 14, 2016 Re: What is going on in this code? | ||||
---|---|---|---|---|
| ||||
Posted in reply to Daniel Kozak | On Thursday, 14 April 2016 at 13:30:42 UTC, Daniel Kozak wrote:
> On Thursday, 14 April 2016 at 13:08:56 UTC, Carlin wrote:
>> On Thursday, 14 April 2016 at 10:29:43 UTC, ag0aep6g wrote:
>>> [...]
>>
>> Thanks, that works perfectly! I couldn't figure it out and I knew I had to be missing something really basic. I still don't understand though why it works in release but not in debug.
>
> because with release build dub will enable inlining. So I guess there is a stack frame elimination
So if you add this:
pragma(inline, false)
before ubyte[] serialize(uint t)
It should not work even with release build type
|
April 14, 2016 Re: What is going on in this code? | ||||
---|---|---|---|---|
| ||||
Posted in reply to Steven Schveighoffer | On Thursday, 14 April 2016 at 13:31:25 UTC, Steven Schveighoffer wrote:
> That is awful.
>
> I propose we make slicing such a return value implicitly an error. I can't think of a valid reason for allowing it.
>
> I'm not the only one: https://issues.dlang.org/show_bug.cgi?id=12625
>
> -Steve
Absolutely agreed. Why haven't we done it already?
|
April 15, 2016 Re: What is going on in this code? | ||||
---|---|---|---|---|
| ||||
Posted in reply to Steven Schveighoffer | On Thursday, April 14, 2016 09:31:25 Steven Schveighoffer via Digitalmars-d wrote: > That is awful. > > I propose we make slicing such a return value implicitly an error. I can't think of a valid reason for allowing it. > > I'm not the only one: https://issues.dlang.org/show_bug.cgi?id=12625 Totally agree, though I wish that we could take it one step further and get rid of the implicit slicing of static arrays. It's an unsafe feature, and once https://issues.dlang.org/show_bug.cgi?id=8838 has been fixed, and slicing a static array is flagged as @system, allowing the implicit slicing makes it really easy to miss that it's happening, and if multiple @system operations are happening in a function, the slicing of the static array could easily be missed by a programmer verifying the safety of the code in order to mark it with @trusted, whereas if the slice were explicit, it would be obvious. Unfortunately, I'm willing to bet that there's no way that Walter would give his okay on making the implicit slicing illegal because of how much code would break as a result. - Jonathan M Davis |
April 15, 2016 Re: What is going on in this code? | ||||
---|---|---|---|---|
| ||||
On Friday, April 15, 2016 21:16:44 Jonathan M Davis via Digitalmars-d wrote: > On Thursday, April 14, 2016 09:31:25 Steven Schveighoffer via Digitalmars-d > > wrote: > > That is awful. > > > > I propose we make slicing such a return value implicitly an error. I can't think of a valid reason for allowing it. > > > > I'm not the only one: https://issues.dlang.org/show_bug.cgi?id=12625 > > Totally agree, though I wish that we could take it one step further and get rid of the implicit slicing of static arrays. It's an unsafe feature, and once https://issues.dlang.org/show_bug.cgi?id=8838 has been fixed, and slicing a static array is flagged as @system, allowing the implicit slicing makes it really easy to miss that it's happening, and if multiple @system operations are happening in a function, the slicing of the static array could easily be missed by a programmer verifying the safety of the code in order to mark it with @trusted, whereas if the slice were explicit, it would be obvious. Unfortunately, I'm willing to bet that there's no way that Walter would give his okay on making the implicit slicing illegal because of how much code would break as a result. Well, for whatever it's worth, I created an enhancement request. Maybe we'll get lucky and be able to talk Walter into it: https://issues.dlang.org/show_bug.cgi?id=15932 8838 and 12625 still need to be fixed regardless, but if we got rid of the implicit slicing of static arrays, then you'd at least always be able to see what's going on and more easily catch slicing-related bugs. - Jonathan M Davis |
Copyright © 1999-2021 by the D Language Foundation