GNOME Bugzilla – Bug 661041
Pointer corruption after modifying a string from a passed-in struct, then saving the string elsewhere
Last modified: 2018-05-22 14:10:53 UTC
Steps to reproduce: 1) Compile and run the following program: /******************************************/ struct SSS { public string ss; } class CCC { public string cs; public CCC(SSS incoming) { incoming.ss = "test"; this.cs = incoming.ss; } } void main() { SSS thing1 = SSS(); thing1.ss = "uncorrupted string"; CCC thing2 = new CCC(thing1); } /******************************************/ ...and observe the results. Notice that a crash occurs, usually with the following error in the console: *** glibc detected *** ./testcase3: double free or corruption (fasttop): 0x(address) *** Note: the code compiles and runs without crashing when using valac-0.12.
Thanks for the bug report. Notice this is a programming error. You're assigning "test" to a field of an unowned struct, which will first free the old contents. Structs are always copied when passed, so maybe you want CCC(ref SSS incoming) to change the original passed struct.
Well, of course the example is somewhat contrived, but the intention wasn't actually to change the original passed struct. Here's a simpler test case that gets to the heart of the matter: struct SSS { public string str; } void foo(SSS s) { s.str = null; } void main() { SSS s = SSS(); s.str = "xyzzy"; foo(s); } This will also crash. When main() calls foo(), the generated code performs only a shallow copy of the struct. This means that the caller's struct and the callee's struct point to the same string. When the callee sets str to null, the string is freed. Then the same string is freed again when the struct goes out of scope at the end of main(), yielding a crash. So, is the crash a bug in this program or in Valac? I had assumed (perhaps naively) that when any struct is passed by value, Vala will copy all strings in the struct, so that the callee can safely modify string pointers in its copy of the struct. Is this not the case?
(In reply to comment #2) > Well, of course the example is somewhat contrived, but the intention wasn't > actually to change the original passed struct. Here's a simpler test case that > gets to the heart of the matter: > > struct SSS { > public string str; > } > > void foo(SSS s) { > s.str = null; > } > > void main() > { > SSS s = SSS(); > s.str = "xyzzy"; > foo(s); > } > > This will also crash. When main() calls foo(), the generated code performs > only a shallow copy of the struct. It's exactly the same example as the previous one. It is supposed to do a shallow copy. Vala should somewhat error out, though it's something really difficult in this context. > So, is the crash a bug in this program or in Valac? I had assumed (perhaps > naively) that when any struct is passed by value, Vala will copy all strings in > the struct, so that the callee can safely modify string pointers in its copy of > the struct. Is this not the case? You must use an owned parameter.
Actually, in Vala 0.12, structs were *not* copied when passed: SSS s = {0}; gchar* _tmp0_; memset (&s, 0, sizeof (SSS)); _tmp0_ = g_strdup ("xyzzy"); _g_free0 (s.str); s.str = _tmp0_; foo (&s); sss_destroy (&s); Note the foo(&s) line. No copy is made prior to passing, and no copy is performed in foo(). In Vala 0.14, a shallow copy is made prior to passing, meaning all pointers inside the struct are now doubly-referenced: SSS s = {0}; gchar* _tmp0_; * SSS _tmp1_; memset (&s, 0, sizeof (SSS)); _tmp0_ = g_strdup ("xyzzy"); _g_free0 (s.str); s.str = _tmp0_; * _tmp1_ = s; * foo (&_tmp1_); sss_destroy (&s); (Interesting lines asterisked.) Here's foo, which is practically the same in both 0.12 and 0.14: gchar* _tmp0_; _tmp0_ = g_strdup ("abc"); _g_free0 ((*s).str); (*s).str = _tmp0_; (0.14 includes a null argument guard, but that's irrelevant here.) The code in Adam's comment sets str to NULL, but here I had Vala set it to another string. Note that because in Vala 0.14 the shallow-copied struct is never destroyed, the duplicated "abc" is never freed. Hence, this new behavior introduces a memory leak. I find this new behavior troubling for a lot of reasons. First, it's not like the above Vala code is using pointers, which are marked as "use at own risk" in the Vala documentation. What this new behavior says is, using a standard data type with the default ownership conferral is dangerous. It seems to me that a compiler warning is required in the least. Second, it's not symmetric with other data type's behaviors, specifically strings and arrays. Neither a string nor an array are copied before passed as unowned variables. This means that an unowned array and string may be modified by the called method (the string via its data member) but not a struct: struct SSS { int a; } void foo(SSS s) { s.a = 1; } void main() { SSS s = SSS(); s.a = 0; foo(s); stdout.printf("%d\n", s.a); } In Vala 0.12, the program prints one. In Vala 0.14, the program prints zero. In short, this is not how it's always been and we're simply tripping on our own shoelaces. I propose this change get some serious reconsideration, because of its asymmetric behavioral changes, the way it casually introduces double-frees without warning, and because of the memory leak it introduces.
Is there any movement on this ticket? The code Adam posted in Comment 2 still crashes in trunk (0.15.1.37-d2de1) due to a double-free. My position on this bug remains unchanged. Structs in Vala are seriously broken without some kind of change.
(In reply to comment #5) > Is there any movement on this ticket? The code Adam posted in Comment 2 still > crashes in trunk (0.15.1.37-d2de1) due to a double-free. > > My position on this bug remains unchanged. Structs in Vala are seriously > broken without some kind of change. As said, it's a programming error. The fact that structs are now shallow copied is intentional. Use an owned parameter instead if you want a full copy.
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/vala/issues/237.