On Thu, 11 Nov 2010 23:45:39 +0100
Pelle Månsson <pelle.mansson@gmail.com> wrote:

> On 11/11/2010 10:15 PM, spir wrote:
> 
> > Well, since the pattern is OK _after_ call to Tuple's constructor (which does nothing more than recording its sub-patterns, see below) and only gets wrong when qutting this(), I fail to see how Tuple could be cause of anything. Also, the corruption is visible _before_ calling match() (which here delegates to Tuple's check()).
> >
> > I understand your note about returning an object actually allocated on the stack -- but here there are only implicitely referenced objects (class instances). This would mean that D creates the 2 sub-patterns on the stack? But why those objects, precisely? (Also note that they are of different classes: one is here a "String", the other a "ZeroOrMore"). They are stored in an array.
> >
> > What's troubling is that the array elements, meaning the supposed subpattern addresses, have changed. Maybe the patterns themselves are still ok, but the array data only are corrupted?
> > Oh, I may try to cast to String the memory area pointed inside this()....... Does not seem to work: I recorded the pointer read in this() (as String*) into a global; then in the test func:
> >      writeln("p: ",p);       	// ok, same address as in this()
> >      writeln(cast(String)(*p));	// segfault!
> >
> > Anyway, just in case my reasoning is wrong, here is the whole Tuple class:
> >
> > ====================================================
> > class Tuple : Pattern {
> >      /**  pattern type for tuple of given patterns */
> >      static string typename = "Tuple";
> >      Pattern[] patterns;
> >      this (Pattern[] patterns...) {
> >          this.patterns = patterns;
> 
> You need to dup that. Arguments are passed on the stack.

Sorry, but I don't understand your hint. Where is the dup-ed array supposed to be allocated? Isn't the assignment supposed to copy it to the field, anyway? How else could one store an array to a field? And how are contained referenced objects supposed to stop vanishing thank to dup?
(I tried, anyway, but the did not stop segfault.)

I have one more example of segfault, in a brand new demo func matching 4 operations. Very strange. With additions, all works fine. When I replace '+' by '-', I get a segfault _sometimes_. When I use '*' or '/', seems I get a segfault _everytime_. The patterns for the 4 operations are all the same, indeed.
Also, since the demo matches calculations of several operations, there can be whitespace. If I add some space after the operation, I get a segfault even for '+'.

What I intend to do is uninstall dmd2 to replace it by dmd1 and see what happens. What do you think?


Denis
-- -- -- -- -- -- --
vit esse estrany ☣

spir.wikidot.com

On Fri, 12 Nov 2010 06:34:02 -0500, spir <denis.spir@gmail.com> wrote:

> On Thu, 11 Nov 2010 23:45:39 +0100
> Pelle Månsson <pelle.mansson@gmail.com> wrote:
>
>> On 11/11/2010 10:15 PM, spir wrote:
>>
>> > Well, since the pattern is OK _after_ call to Tuple's constructor  
>> (which does nothing more than recording its sub-patterns, see below) and only gets wrong when qutting this(), I fail to see how Tuple could be cause of anything. Also, the corruption is visible _before_ calling match() (which here delegates to Tuple's check()).
>> >
>> > I understand your note about returning an object actually allocated  
>> on the stack -- but here there are only implicitely referenced objects (class instances). This would mean that D creates the 2 sub-patterns on the stack? But why those objects, precisely? (Also note that they are of different classes: one is here a "String", the other a "ZeroOrMore"). They are stored in an array.
>> >
>> > What's troubling is that the array elements, meaning the supposed  
>> subpattern addresses, have changed. Maybe the patterns themselves are still ok, but the array data only are corrupted?
>> > Oh, I may try to cast to String the memory area pointed inside  
>> this()....... Does not seem to work: I recorded the pointer read in this() (as String*) into a global; then in the test func:
>> >      writeln("p: ",p);       	// ok, same address as in this()
>> >      writeln(cast(String)(*p));	// segfault!
>> >
>> > Anyway, just in case my reasoning is wrong, here is the whole Tuple  
>> class:
>> >
>> > ====================================================
>> > class Tuple : Pattern {
>> >      /**  pattern type for tuple of given patterns */
>> >      static string typename = "Tuple";
>> >      Pattern[] patterns;
>> >      this (Pattern[] patterns...) {
>> >          this.patterns = patterns;
>>
>> You need to dup that. Arguments are passed on the stack.
>
> Sorry, but I don't understand your hint. Where is the dup-ed array supposed to be allocated? Isn't the assignment supposed to copy it to the field, anyway? How else could one store an array to a field? And how are contained referenced objects supposed to stop vanishing thank to dup?
> (I tried, anyway, but the did not stop segfault.)

Pelle, I spent all this time helping him, and you swoop in with the answer :)

Yes, he is right, you need to dup the patterns argument.  It is something I very recently discovered.

Here is what happens:

void foo(int[] arg...) {}

foo(1,2,3);

What the compiler does is push 1, 2, and 3 onto the stack, then passes in a dynamic array reference to that stack data.  It does this because heap allocations are much more expensive than stack allocations.  When you just "save" the data, it is no longer valid.  The reason the code all works within the List constructor is because that is the stack frame where the array is pushed onto the stack.

Note, you can do foo([1,2,3]), and you will be wasting time duping, but I have found a solution to that, overloading:

void foo(int[] arg) {}

If you pass in an array, then the second overload is used, if you pass in individual arguments the first overload is used.

Essentially, if you change your line above to:

this.patterns = patterns.dup;

All is well, you should be good.

-Steve

On Fri, 12 Nov 2010 08:03:26 -0500
"Steven Schveighoffer" <schveiguy@yahoo.com> wrote:


> Pelle, I spent all this time helping him, and you swoop in with the answer :)
> 
> Yes, he is right, you need to dup the patterns argument.  It is something I very recently discovered.

Wow, first thought dup did not solve the issue, but it fact I had introduced another bug in the meanyrime ;-) All works fine, now. I'll be able to restart working on this. D soon has a prototype for a PEG lib.

> Here is what happens:
> 
> void foo(int[] arg...) {}
> 
> foo(1,2,3);
> 
> What the compiler does is push 1, 2, and 3 onto the stack, then passes in a dynamic array reference to that stack data.  It does this because heap allocations are much more expensive than stack allocations.  When you just "save" the data, it is no longer valid.  The reason the code all works within the List constructor is because that is the stack frame where the array is pushed onto the stack.

Right.

> Note, you can do foo([1,2,3]), and you will be wasting time duping, but I have found a solution to that, overloading:
> 
> void foo(int[] arg) {}
> 
> If you pass in an array, then the second overload is used, if you pass in individual arguments the first overload is used.

All right. I find a bit strange that the compiler accepts f([1,2,3]) when the declaration reads eg void f(int[] ints...). Anyway, it cannot harm, I guess.

> Essentially, if you change your line above to:
> 
> this.patterns = patterns.dup;

Works, but I don't understand why: isn't the duplicate also allocated on the stack? I mean, dup is supposed to just duplicate, isn't it? what does it give to the new structure that the original one hasn't? I thought I would have to do for instance:
	ints[] x;
	void f(int[] ints...) {
	  x.length = ints.length;   // allocate new area on the heap
	  x[] = ints[];             // copy elements in there
	}

> All is well, you should be good.

yo!

> -Steve

Thank you and Pelle for your help,
Denis
-- -- -- -- -- -- --
vit esse estrany ☣

spir.wikidot.com

On 11/12/2010 02:03 PM, Steven Schveighoffer wrote:
>
> Pelle, I spent all this time helping him, and you swoop in with the
> answer :)

I was in a rush when answering, causing the swoopiness of my post. :-)

I only knew the answer because I had almost exactly the same bug a week ago, or so. Coincidentally, for a parser generator.

On Sat, 13 Nov 2010 15:57:50 -0500, spir <denis.spir@gmail.com> wrote:

> On Fri, 12 Nov 2010 08:03:26 -0500
> "Steven Schveighoffer" <schveiguy@yahoo.com> wrote:

>> Essentially, if you change your line above to:
>>
>> this.patterns = patterns.dup;
>
> Works, but I don't understand why: isn't the duplicate also allocated on the stack? I mean, dup is supposed to just duplicate, isn't it? what does it give to the new structure that the original one hasn't? I thought I would have to do for instance:
> 	ints[] x;
> 	void f(int[] ints...) {
> 	  x.length = ints.length;   // allocate new area on the heap
> 	  x[] = ints[];             // copy elements in there
> 	}
>

When doing this:

arr1 = arr2;

It copies the reference to the data, not the data itself.  Since the data is on the stack, you are still pointing at the stack

arr1 = arr2.dup;

copies the data itself, then returns a reference to the new data.  The reference is stored wherever the reference is stored.  In your case, the reference this.patterns is stored in the class, so it's also stored on the heap.

I hope this is clearer.

-Steve