Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix for crash for functions with > 127 local vars #124

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

virt00l
Copy link

@virt00l virt00l commented Dec 7, 2017

Actual crash happened due to _OP_CALL target argument is overflowed
signed char (7 bit). Treat argument as as unsigned (with exception of
value -1/255 which is special) in order to fix it

Actual crash happened due to _OP_CALL target argument is overflowed
signed char (7 bit). Treat argument as as unsigned (with exception of
value -1/255 which is special) in order to fix it
@mingodad
Copy link
Contributor

mingodad commented Dec 13, 2017

Could you provide a code sample showing the crash ?
Cheers !

Maybe what you want to say is "fix for crash for functions with > 127 parameters".
I'm testing with this sample and I do not get any crash:

/*SquiLu
local function genLocalVars(n)
{
	for(local i=0; i < n; ++i) putsnl("local v" + i + " = " + i + ";");
}
SquiLu*/
local function tmv()
{
//@genLocalVars(250);
// generated-code:begin
local v0 = 0;
local v1 = 1;
local v2 = 2;
local v3 = 3;
local v4 = 4;
local v5 = 5;
local v6 = 6;
local v7 = 7;
local v8 = 8;
local v9 = 9;
local v10 = 10;
local v11 = 11;
local v12 = 12;
local v13 = 13;
local v14 = 14;
local v15 = 15;
local v16 = 16;
local v17 = 17;
local v18 = 18;
local v19 = 19;
local v20 = 20;
local v21 = 21;
local v22 = 22;
local v23 = 23;
local v24 = 24;
local v25 = 25;
local v26 = 26;
local v27 = 27;
local v28 = 28;
local v29 = 29;
local v30 = 30;
local v31 = 31;
local v32 = 32;
local v33 = 33;
local v34 = 34;
local v35 = 35;
local v36 = 36;
local v37 = 37;
local v38 = 38;
local v39 = 39;
local v40 = 40;
local v41 = 41;
local v42 = 42;
local v43 = 43;
local v44 = 44;
local v45 = 45;
local v46 = 46;
local v47 = 47;
local v48 = 48;
local v49 = 49;
local v50 = 50;
local v51 = 51;
local v52 = 52;
local v53 = 53;
local v54 = 54;
local v55 = 55;
local v56 = 56;
local v57 = 57;
local v58 = 58;
local v59 = 59;
local v60 = 60;
local v61 = 61;
local v62 = 62;
local v63 = 63;
local v64 = 64;
local v65 = 65;
local v66 = 66;
local v67 = 67;
local v68 = 68;
local v69 = 69;
local v70 = 70;
local v71 = 71;
local v72 = 72;
local v73 = 73;
local v74 = 74;
local v75 = 75;
local v76 = 76;
local v77 = 77;
local v78 = 78;
local v79 = 79;
local v80 = 80;
local v81 = 81;
local v82 = 82;
local v83 = 83;
local v84 = 84;
local v85 = 85;
local v86 = 86;
local v87 = 87;
local v88 = 88;
local v89 = 89;
local v90 = 90;
local v91 = 91;
local v92 = 92;
local v93 = 93;
local v94 = 94;
local v95 = 95;
local v96 = 96;
local v97 = 97;
local v98 = 98;
local v99 = 99;
local v100 = 100;
local v101 = 101;
local v102 = 102;
local v103 = 103;
local v104 = 104;
local v105 = 105;
local v106 = 106;
local v107 = 107;
local v108 = 108;
local v109 = 109;
local v110 = 110;
local v111 = 111;
local v112 = 112;
local v113 = 113;
local v114 = 114;
local v115 = 115;
local v116 = 116;
local v117 = 117;
local v118 = 118;
local v119 = 119;
local v120 = 120;
local v121 = 121;
local v122 = 122;
local v123 = 123;
local v124 = 124;
local v125 = 125;
local v126 = 126;
local v127 = 127;
local v128 = 128;
local v129 = 129;
local v130 = 130;
local v131 = 131;
local v132 = 132;
local v133 = 133;
local v134 = 134;
local v135 = 135;
local v136 = 136;
local v137 = 137;
local v138 = 138;
local v139 = 139;
local v140 = 140;
local v141 = 141;
local v142 = 142;
local v143 = 143;
local v144 = 144;
local v145 = 145;
local v146 = 146;
local v147 = 147;
local v148 = 148;
local v149 = 149;
local v150 = 150;
local v151 = 151;
local v152 = 152;
local v153 = 153;
local v154 = 154;
local v155 = 155;
local v156 = 156;
local v157 = 157;
local v158 = 158;
local v159 = 159;
local v160 = 160;
local v161 = 161;
local v162 = 162;
local v163 = 163;
local v164 = 164;
local v165 = 165;
local v166 = 166;
local v167 = 167;
local v168 = 168;
local v169 = 169;
local v170 = 170;
local v171 = 171;
local v172 = 172;
local v173 = 173;
local v174 = 174;
local v175 = 175;
local v176 = 176;
local v177 = 177;
local v178 = 178;
local v179 = 179;
local v180 = 180;
local v181 = 181;
local v182 = 182;
local v183 = 183;
local v184 = 184;
local v185 = 185;
local v186 = 186;
local v187 = 187;
local v188 = 188;
local v189 = 189;
local v190 = 190;
local v191 = 191;
local v192 = 192;
local v193 = 193;
local v194 = 194;
local v195 = 195;
local v196 = 196;
local v197 = 197;
local v198 = 198;
local v199 = 199;
local v200 = 200;
local v201 = 201;
local v202 = 202;
local v203 = 203;
local v204 = 204;
local v205 = 205;
local v206 = 206;
local v207 = 207;
local v208 = 208;
local v209 = 209;
local v210 = 210;
local v211 = 211;
local v212 = 212;
local v213 = 213;
local v214 = 214;
local v215 = 215;
local v216 = 216;
local v217 = 217;
local v218 = 218;
local v219 = 219;
local v220 = 220;
local v221 = 221;
local v222 = 222;
local v223 = 223;
local v224 = 224;
local v225 = 225;
local v226 = 226;
local v227 = 227;
local v228 = 228;
local v229 = 229;
local v230 = 230;
local v231 = 231;
local v232 = 232;
local v233 = 233;
local v234 = 234;
local v235 = 235;
local v236 = 236;
local v237 = 237;
local v238 = 238;
local v239 = 239;
local v240 = 240;
local v241 = 241;
local v242 = 242;
local v243 = 243;
local v244 = 244;
local v245 = 245;
local v246 = 246;
local v247 = 247;
local v248 = 248;
local v249 = 249;
// generated-code:end
	return 5;
}

print(tmv());

@virt00l
Copy link
Author

virt00l commented Dec 15, 2017

I don't have time right now to triangulate this issue further.
But essentially key here is return value of function.
So it's should be either:

// assume hereo that tmpv() is big function with lot of local vars
function whatever() { local test = tmv(); }

or

function whatever() { return 4; }
function tmpv()
{
//@genLocalVars(250);
// generated-code:begin
local v0 = 0;
...
 tmp = whatever()
}

Something along this lines.
May be on this weekend I will have more time to investigate this further.

@mingodad
Copy link
Contributor

Thank you so much !
Adding the:

function whatever() { return 4; }
function tmpv()
{
//@genLocalVars(250);
// generated-code:begin
local v0 = 0;
...
 tmp = whatever();
 return tmp + 5;
}

Gives an error and after applying your patch it works.
Here is the commit in SquiLu mingodad/squilu@1cfc0f8

@todace
Copy link

todace commented Dec 19, 2017

@albertodemichelis can you review this PR please?

@albertodemichelis
Copy link
Owner

I think we should simply add a check in the compiler that throws an error if the stack frame is larger than 127. It would be possible to increase the max stack frame to 254 in the future as suggested by this commit, but I have to verify that there aren't other implications.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants