Discussion:
Weird parsing error in Pike 8.1
Stephen R. van den Berg
2014-11-20 08:18:39 UTC
Permalink
Could someone take a look at the difference between these two
commits, and then fix the parsing problem in Pike?
My first attempt at reducing the problem to something which
could be stuck into the testsuite failed, so I'm presenting it as is:

commit 907a7ab712f8eae22af910b2eb5c5d4e869502fa
Author: Stephen R. van den Berg <***@cuci.nl>
Date: Thu Nov 20 02:39:04 2014 +0100

This kludges around a parser bug in Pike 8.1.

It's a bit difficult to create an isolated testcase out of this.
Somebody please look into this.
object b=Sql.pgsql("");
/usr/local/pike/8.1.0/lib/modules/Sql.pmod/pgsql.pike:696:switch(): Conditional expression is void.

commit c7de868af7d978b1ca9dddecbbab7c6357e9e7dc
--
Stephen.
Chris Angelico
2014-11-20 08:53:10 UTC
Permalink
Post by Stephen R. van den Berg
Could someone take a look at the difference between these two
commits, and then fix the parsing problem in Pike?
My first attempt at reducing the problem to something which
Well, I don't know what the actual cause of the problem is, but I
found a stick that I can poke the bug with. Have a look at commit
22dbe6 - there was a typo in an apparently-unrelated piece of code,
the final _processloop function. I've no idea why that didn't result
in its own compilation error. But with that fixed, reverting c7de86
doesn't reinstate the behaviour you referred to. Is that any sort of
clue? It doesn't say anything to my mind.

ChrisA
Stephen R. van den Berg
2014-11-20 09:02:16 UTC
Permalink
Post by Chris Angelico
Well, I don't know what the actual cause of the problem is, but I
found a stick that I can poke the bug with. Have a look at commit
22dbe6 - there was a typo in an apparently-unrelated piece of code,
the final _processloop function. I've no idea why that didn't result
in its own compilation error. But with that fixed, reverting c7de86
doesn't reinstate the behaviour you referred to. Is that any sort of
clue? It doesn't say anything to my mind.
That's not a typo actually.
As of yesterday, there is a conxion and a conxiin class.
The conxiin is the one that does the read buffering, the conxion
is the one that does the write buffering (and has a conxiin as a member).

So, please revert this patch.
--
Stephen.
Chris Angelico
2014-11-20 09:04:42 UTC
Permalink
Post by Stephen R. van den Berg
Post by Chris Angelico
Well, I don't know what the actual cause of the problem is, but I
found a stick that I can poke the bug with. Have a look at commit
22dbe6 - there was a typo in an apparently-unrelated piece of code,
the final _processloop function. I've no idea why that didn't result
in its own compilation error. But with that fixed, reverting c7de86
doesn't reinstate the behaviour you referred to. Is that any sort of
clue? It doesn't say anything to my mind.
That's not a typo actually.
As of yesterday, there is a conxion and a conxiin class.
The conxiin is the one that does the read buffering, the conxion
is the one that does the write buffering (and has a conxiin as a member).
So, please revert this patch.
Ooooops! I thought I did a quick "search across files" to check for
any other references to conxiin, and didn't come across any... but I
must have done that in the wrong directory, or something. Sorry about
that. Reverted.

It did still poke the bug, though, so maybe there's something to be
learned. What, I don't know, though.

ChrisA
Stephen R. van den Berg
2014-11-20 09:05:49 UTC
Permalink
Post by Chris Angelico
22dbe6 - there was a typo in an apparently-unrelated piece of code,
in its own compilation error. But with that fixed, reverting c7de86
doesn't reinstate the behaviour you referred to. Is that any sort of
clue? It doesn't say anything to my mind.
With regards to apparently-unrelated and clue, yes, this is a clue, but
it points (AFAICS) towards some obscure stuff which is either
not right in the perceived declaration of the Stdio.Buffer.read_int32
member, or a parser in Pike running on steroids mixing up ints and
voids under the right stellar constellations.
--
Stephen.
Arne Goedeke
2014-11-20 12:18:10 UTC
Permalink
The type of read_int32 is broken, because the min max values end up
being min=0 and max=-1. The max value is MAX_UINT32, which does not fit
into INT32 and ends up being -1.
Even in debug mode this is not caught, because (for some reason) the
debug check in push_int_type does only check for min < max if they have
the same sign (anyone can explain why?).
I will commit a fix to precompile.pike which prevents that. Not sure if
that fixes your problem, though.

arne
Post by Stephen R. van den Berg
Post by Chris Angelico
22dbe6 - there was a typo in an apparently-unrelated piece of code,
in its own compilation error. But with that fixed, reverting c7de86
doesn't reinstate the behaviour you referred to. Is that any sort of
clue? It doesn't say anything to my mind.
With regards to apparently-unrelated and clue, yes, this is a clue, but
it points (AFAICS) towards some obscure stuff which is either
not right in the perceived declaration of the Stdio.Buffer.read_int32
member, or a parser in Pike running on steroids mixing up ints and
voids under the right stellar constellations.
Arne Goedeke
2014-11-20 12:27:17 UTC
Permalink
And grubba was faster than me. My solution was a bit different, it just
sets the type to 'tInt' if either of the min/max values does not fit
into INT32. I believe that is more correct.

arne

diff --git a/lib/modules/Tools.pmod/Standalone.pmod/precompile.pike
b/lib/modules/Tools.pmod/Standalone.pmod/precompile.pike
index 0a58823..05ca96f 100644
--- a/lib/modules/Tools.pmod/Standalone.pmod/precompile.pike
+++ b/lib/modules/Tools.pmod/Standalone.pmod/precompile.pike
@@ -821,12 +821,26 @@ class PikeType
case "program": return "tPrg(tObj)";
case "any": return "tAny";
case "mixed": return "tMix";
- case "int":
+ case "int": {
// NOTE! This piece of code KNOWS that PIKE_T_INT is 8!
- return stringify(sprintf("\010%4c%4c",
- (int)(string)(args[0]->t),
- (int)(string)(args[1]->t)));
+ int min = (int)(string)(args[0]->t);
+ int max = (int)(string)(args[1]->t);
+
+ if (min > max) error("Minimum > Maximum in int type.\n");
+
+ if (max < -0x80000000 || max > 0x7fffffff) {
+ warn("Maximum %d does not fit into INT32.\n", max);
+ return "tInt";
+ }

+ if (min < -0x80000000 || min > 0x7fffffff) {
+ warn("Maximum %d does not fit into INT32.\n", max);
+ return "tInt";
+ }
+
+ return stringify(sprintf("\010%4c%4c", min, max));
+
+ }
case "bignum":
case "longest":
return "tInt";
Post by Arne Goedeke
The type of read_int32 is broken, because the min max values end up
being min=0 and max=-1. The max value is MAX_UINT32, which does not fit
into INT32 and ends up being -1.
Even in debug mode this is not caught, because (for some reason) the
debug check in push_int_type does only check for min < max if they have
the same sign (anyone can explain why?).
I will commit a fix to precompile.pike which prevents that. Not sure if
that fixes your problem, though.
arne
Post by Stephen R. van den Berg
Post by Chris Angelico
22dbe6 - there was a typo in an apparently-unrelated piece of code,
in its own compilation error. But with that fixed, reverting c7de86
doesn't reinstate the behaviour you referred to. Is that any sort of
clue? It doesn't say anything to my mind.
With regards to apparently-unrelated and clue, yes, this is a clue, but
it points (AFAICS) towards some obscure stuff which is either
not right in the perceived declaration of the Stdio.Buffer.read_int32
member, or a parser in Pike running on steroids mixing up ints and
voids under the right stellar constellations.
Loading...