[perl #58866] calling a PIR sub with 206 params segfaults parrot
On Thu Sep 18 08:52:10 2008, julianalbo wrote: I changed the fix in r31230 to allocate char instead of char *, adjusted the formula for buffer size and added a comment explaining it to lower the level of black magic, and added a check for each item, dropping the XXX comment that asked for it. I hope this is enough understanding of the error ;) Thanks. I think neither of us read the code quite correctly, but your patch was a significant improvement. It also fixed a hard-to-reproduce bug that GeJ was running into on FreeBSD 7.1. In looking at the code, I can see some ways to make the test more comprehensive. I'm going to reopen it as a way to remind me to write some more exhaustive tests which exercise sub signatures as well as sub calls.
[perl #58866] calling a PIR sub with 206 params segfaults parrot
On Wed Sep 17 08:31:26 2008, particle wrote: On Tue, Sep 16, 2008 at 11:45 PM, Christoph Otto via RT [EMAIL PROTECTED] wrote: On Tue Sep 16 15:00:24 2008, [EMAIL PROTECTED] wrote: On Tuesday 16 September 2008 14:47:58 NotFound wrote: It certainly shouldn't segfault. But, the question is: why does it segfault at 206 parameters? Throwing an exception to avoid an error we don't understand isn't good for the long-term health of the VM. The problem is located inside compilers/imcc/pcc.c:pcc_get_args function. It has the comment /* XXX check avail len */ just at the point where the segfault happens. char buf[1024] is the variable overrunned. That sounds like a bog-standard static variable overflow, where each parameter requires five bytes of storage. If that's a good rule of thumb, we could malloc/free that buffer instead, and then beat anyone who uses more than a dozen parameters. -- c Looking at the code, it's 5n+1. r31200 changes the static buffer to a dynamic one of the correct size. The original PIR code now runs without segfaulting, as does a version with 20,000 int params. make test also passes, so nothing new appears to be broken. With the assumption that the said beatings will be a manual process, I'm marking this resolved. you didn't mention it, but i sincerely hope you committed a test with 20,000 params that proves this and makes sure we don't regress in a future revision. parrot needs much more stress testing like this, and it would be a shame to miss this opportunity. ~jerry I didn't think of it until after I went to bed, but I added one with 1000 params in r31208 the next morning. The 20,000 param version took more than a second to run and I didn't see any reason to slow the tests down more than necessary.
Re: [perl #58866] calling a PIR sub with 206 params segfaults parrot
I changed the fix in r31230 to allocate char instead of char *, adjusted the formula for buffer size and added a comment explaining it to lower the level of black magic, and added a check for each item, dropping the XXX comment that asked for it. I hope this is enough understanding of the error ;) -- Salu2
[perl #58866] calling a PIR sub with 206 params segfaults parrot
On Tue Sep 16 15:00:24 2008, [EMAIL PROTECTED] wrote: On Tuesday 16 September 2008 14:47:58 NotFound wrote: It certainly shouldn't segfault. But, the question is: why does it segfault at 206 parameters? Throwing an exception to avoid an error we don't understand isn't good for the long-term health of the VM. The problem is located inside compilers/imcc/pcc.c:pcc_get_args function. It has the comment /* XXX check avail len */ just at the point where the segfault happens. char buf[1024] is the variable overrunned. That sounds like a bog-standard static variable overflow, where each parameter requires five bytes of storage. If that's a good rule of thumb, we could malloc/free that buffer instead, and then beat anyone who uses more than a dozen parameters. -- c Looking at the code, it's 5n+1. r31200 changes the static buffer to a dynamic one of the correct size. The original PIR code now runs without segfaulting, as does a version with 20,000 int params. make test also passes, so nothing new appears to be broken. With the assumption that the said beatings will be a manual process, I'm marking this resolved.
Re: [perl #58866] calling a PIR sub with 206 params segfaults parrot
On Tue, Sep 16, 2008 at 11:45 PM, Christoph Otto via RT [EMAIL PROTECTED] wrote: On Tue Sep 16 15:00:24 2008, [EMAIL PROTECTED] wrote: On Tuesday 16 September 2008 14:47:58 NotFound wrote: It certainly shouldn't segfault. But, the question is: why does it segfault at 206 parameters? Throwing an exception to avoid an error we don't understand isn't good for the long-term health of the VM. The problem is located inside compilers/imcc/pcc.c:pcc_get_args function. It has the comment /* XXX check avail len */ just at the point where the segfault happens. char buf[1024] is the variable overrunned. That sounds like a bog-standard static variable overflow, where each parameter requires five bytes of storage. If that's a good rule of thumb, we could malloc/free that buffer instead, and then beat anyone who uses more than a dozen parameters. -- c Looking at the code, it's 5n+1. r31200 changes the static buffer to a dynamic one of the correct size. The original PIR code now runs without segfaulting, as does a version with 20,000 int params. make test also passes, so nothing new appears to be broken. With the assumption that the said beatings will be a manual process, I'm marking this resolved. you didn't mention it, but i sincerely hope you committed a test with 20,000 params that proves this and makes sure we don't regress in a future revision. parrot needs much more stress testing like this, and it would be a shame to miss this opportunity. ~jerry
Re: [perl #58866] calling a PIR sub with 206 params segfaults parrot
Christoph Otto (via RT) wrote: I was looking at #45357 ([TODO] Which exception should be thrown with register overflow?) and found that Parrot doesn't like subs with more than 205 params. This seems like a perfectly reasonable limit, but perhaps the behavior could be more user-friendly.* Maybe an EXCEPTION_ERR_OVERFLOW should be thrown here instead? Using the attached patch to reproduce: $ perl mktoomanyargs.pltoomanyargs.pir ./parrot toomanyargs.pir Segmentation fault (core dumped) *On the other hand, we may choose to be overtly hostile to users who do things like call subs with 206 parameters. It's a design decision. It certainly shouldn't segfault. But, the question is: why does it segfault at 206 parameters? Throwing an exception to avoid an error we don't understand isn't good for the long-term health of the VM. Allison
Re: [perl #58866] calling a PIR sub with 206 params segfaults parrot
It certainly shouldn't segfault. But, the question is: why does it segfault at 206 parameters? Throwing an exception to avoid an error we don't understand isn't good for the long-term health of the VM. The problem is located inside compilers/imcc/pcc.c:pcc_get_args function. It has the comment /* XXX check avail len */ just at the point where the segfault happens. char buf[1024] is the variable overrunned. -- Salu2
Re: [perl #58866] calling a PIR sub with 206 params segfaults parrot
On Tuesday 16 September 2008 14:47:58 NotFound wrote: It certainly shouldn't segfault. But, the question is: why does it segfault at 206 parameters? Throwing an exception to avoid an error we don't understand isn't good for the long-term health of the VM. The problem is located inside compilers/imcc/pcc.c:pcc_get_args function. It has the comment /* XXX check avail len */ just at the point where the segfault happens. char buf[1024] is the variable overrunned. That sounds like a bog-standard static variable overflow, where each parameter requires five bytes of storage. If that's a good rule of thumb, we could malloc/free that buffer instead, and then beat anyone who uses more than a dozen parameters. -- c
