After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 787152 - Fixed size arrays no longer zeroed when partially initialized
Fixed size arrays no longer zeroed when partially initialized
Status: RESOLVED FIXED
Product: vala
Classification: Core
Component: Arrays
0.37.x
Other All
: Normal normal
: ---
Assigned To: Vala maintainers
Vala maintainers
Depends on:
Blocks:
 
 
Reported: 2017-09-01 18:39 UTC by Al Thomas
Modified: 2017-09-02 15:42 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
codegen: Initialize temp-variable for fixed-size arrays to zero first (1.67 KB, patch)
2017-09-02 07:50 UTC, Rico Tzschichholz
committed Details | Review

Description Al Thomas 2017-09-01 18:39:25 UTC
The following code:

void main () {
	uint8 test[10] = {1, 2, 3};
	uint8 test2[10] = test;
	for (int count = 0; count < test.length; count++) {
		assert (test[count] == test2[count]);
		print (@"count: $count, value: $(test[count])\n");
	}
}

when compiled with valac 0.30.2 and run, it produces:

count: 0, value: 1
count: 1, value: 2
count: 2, value: 3
count: 3, value: 0
count: 4, value: 0
count: 5, value: 0
count: 6, value: 0
count: 7, value: 0
count: 8, value: 0
count: 9, value: 0

When compiled with valac 0.37.91 and run, it produces:

count: 0, value: 1
count: 1, value: 2
count: 2, value: 3
count: 3, value: 52
count: 4, value: 76
count: 5, value: 127
count: 6, value: 0
count: 7, value: 0
count: 8, value: 184
count: 9, value: 153

So the array is not initialized to zero first, before being initialized to {1, 2, 3}.

With valac 0.30.2 the relevant C code produced is:

void _vala_main (void) {
        guint8 test[10] = {0};
        guint8 _tmp0_[10] = {0};
        guint8 test2[10] = {0};
        _tmp0_[0] = (guint8) 1;
        _tmp0_[1] = (guint8) 2;
        _tmp0_[2] = (guint8) 3;
        memcpy (test, _tmp0_, 10 * sizeof (guint8));
        memcpy (test2, test, 10 * sizeof (guint8));

With valac 0.37.91 the relevant C code is:

void _vala_main (void) {
        guint8 test[10] = {0};
        guint8 _tmp0_[10];
        guint8 test2[10] = {0};
        _tmp0_[0] = (guint8) 1;
        _tmp0_[1] = (guint8) 2;
        _tmp0_[2] = (guint8) 3;
        memcpy (test, _tmp0_, 10 * sizeof (guint8));
        memcpy (test2, test, 10 * sizeof (guint8));

_tmp0_[10] is no longer initialized to zero first with the newer version of Vala. Ironically test[10] is still initialized to zero, so this can't be a performance enhancement?
Comment 1 Rico Tzschichholz 2017-09-02 07:50:43 UTC
Created attachment 358970 [details] [review]
codegen: Initialize temp-variable for fixed-size arrays to zero first
Comment 2 Al Thomas 2017-09-02 14:25:38 UTC
Review of attachment 358970 [details] [review]:

Thanks for patching this so quickly. The patch looks good and fixes the problem.

I cannot understand, though, why the array is initialized by creating a temporary array then copying it to the declared array. Fixed size arrays are stack allocated I cannot understand the advantage the copying brings.

For example this Vala program:

void main () {
    uint8 test[5] = {1, 2, 3};
}

produces the C code:

void _vala_main (void) {
        guint8 test[5] = {0};
        guint8 _tmp0_[5] = {0};
        _tmp0_[0] = (guint8) 1;
        _tmp0_[1] = (guint8) 2;
        _tmp0_[2] = (guint8) 3;
        memcpy (test, _tmp0_, 5 * sizeof (guint8));
}

What would be wrong with this?:

void _vala_main (void) {
        guint8 test[5] = {0};
        test[0] = (guint8) 1;
        test[1] = (guint8) 2;
        test[2] = (guint8) 3;
}
Comment 3 Rico Tzschichholz 2017-09-02 15:22:46 UTC
The wide usage of temp-vars helps to deal with scoping and ordering issues. Avoiding temp-vars in such special cases requires some efforts to not break the existing behavior.
Comment 4 Rico Tzschichholz 2017-09-02 15:42:18 UTC
Attachment 358970 [details] pushed as e8d47c0 - codegen: Initialize temp-variable for fixed-size arrays to zero first