<html><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Dec 18, 2008, at 6:30 PM, Chris Goller wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">This patch fixes a portion of case 1889 in Bugzilla. This patch causes the compiler to report an error when a constant array is declared with an integer literal size that is too big.</blockquote><div><br></div>Nice, it would be great to fix that issue.</div><div><br><blockquote type="cite">I'm new to the code base, so, I'm not sure if I did everything correctly. For example, what is the best way to get the number of bytes of each allocated per element of an array?<br> Here is the way I'm doing it... <br> unsigned SizeTypeBits = static_cast<unsigned>(Context.getTypeSize(Context.getSizeType()));<br> llvm::APSInt EltBytes(SizeTypeBits); EltBytes = Context.getTypeSize(T) / Context.Target.getCharWidth();</blockquote><div><br></div><div>Yep, that seems right, but use uint64_t instead of 'unsigned' to avoid clipping in 64-bit cases.</div><br><blockquote type="cite">If this is right, maybe it could be added to ArrayType?</blockquote><div><br></div><div>Is this specific to arrays?</div><br><blockquote type="cite">Also, what is the max address of an array? I can't find anything in the c or c++ standard about that. I ended up checking a few compilers, google, etc. on this and it seems to be LONG_MAX. <br> Is this always the case meaning is it different on different targets?</blockquote><div><br></div><div>Do you mean the maximum index? I don't think there is a specification. I think it would make sense to do "sizeof pointer - 2 bits" for example. Clang can establish its own arbitrary limit.</div><br><blockquote type="cite">Additionally, are there different rules about how large an array can be allocated if it is static?</blockquote><div><br></div>Nope, I think it should just be a limit on *object* size, independent of whether that is a big struct, array or whatever.</div><div><br><blockquote type="cite">Sadly, this patch doesn't help with the specific problem for the case in Bugzilla. That case involved an expression that evaluated to larger than maxlong, for example char BigArray[0x7fffffff + 1].<br> The expression is evaluated in bool Expr::isIntegerConstantExpr and that function has a few FIXME comments:<br><br>/// FIXME: Pass up a reason why! Invalid operation in i-c-e, division by zero,<br>/// comma, etc<br>///<br> /// FIXME: This should ext-warn on overflow during evaluation! ISO C does not<br>/// permit this. This includes things like (int)1e1000<br></blockquote><br><div>Yeah, I think catching overflow is really what is needed here.</div><br><blockquote type="cite">So, to patch this problem correctly, I'll need to know if overflow occurred as opposed to simply an expression that ended up less than zero. I'm going to work on returning various<br> error codes from the isIntegerConstantExpr, but a change to that method will affect a bunch of different areas, so, I'm sure I'll need some help! Does anyone have any ideas about how<br>to handle overflow during the evaluation of expressions?<br></blockquote></div><br><div>There is code in PPExpressions.cpp that both does arithmetic and captures whether overflow happened or not, stuff like:</div><div><br></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; "> <span style="color: #aa0d91">case</span> <span style="color: #3f6e74">tok</span>::<span style="color: #26474b">star</span>:</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; "> <span style="color: #3f6e74">Res</span> = <span style="color: #26474b">LHS</span>.<span style="color: #3f6e74">Val</span> * <span style="color: #3f6e74">RHS</span>.<span style="color: #3f6e74">Val</span>;</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; "> <span style="color: #aa0d91">if</span> (<span style="color: #26474b">LHS</span>.<span style="color: #3f6e74">Val</span> != <span style="color: #1c00cf">0</span> && <span style="color: #3f6e74">RHS</span>.<span style="color: #3f6e74">Val</span> != <span style="color: #1c00cf">0</span>)</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 10px/normal Monaco; color: rgb(63, 110, 116); "><span style="color: #000000"> </span>Overflow<span style="color: #000000"> = </span>Res<span style="color: #000000">/</span>RHS<span style="color: #000000">.</span>Val<span style="color: #000000"> != </span><span style="color: #26474b">LHS</span><span style="color: #000000">.</span>Val<span style="color: #000000"> || </span>Res<span style="color: #000000">/</span><span style="color: #26474b">LHS</span><span style="color: #000000">.</span>Val<span style="color: #000000"> != </span>RHS<span style="color: #000000">.</span>Val<span style="color: #000000">;</span></div><div><br></div><div>I really don't think this logic should be in the preprocessor :). It would be better to add methods to APSInt that do this, something like:</div><div><br></div><div>APSPInt smul(const APSInt&, const APSInt&, bool &Overflow)</div><div><br></div><div>etc.</div><div><br></div><div>-Chris</div><div><br></div></body></html>