<div dir="ltr"><div>Yes, I just noticed that if the pre-processor level is set to -1, then SYMPHONY crashes. Level 0 doesn't do anything that would normally be considered pre-processing, just some bookkeeping, and is probably the lowest level that should probably be allowed. At level 0, I haven't seen any problems. I will fix the issue in the next release. Feel free to follow up with more information on the problem.<br><br></div>Cheers,<br><br>Ted<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Nov 5, 2014 at 5:50 PM, Konvicka Filip <span dir="ltr"><<a href="mailto:Filip.Konvicka@logis.cz" target="_blank">Filip.Konvicka@logis.cz</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div link="blue" vlink="purple" lang="EN-US">
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Ted,<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Thanks very much! And no need to apologize, you’re still very fast. If there’s anything else I’ll try to post another message. One thing I noticed was that
if preprocessing is turned off weird stuff happens...I was trying to keep my row/column numbering but the preprocess step prevented that – so I tried to turn it off, but I think the program crashed. I’ll try to reproduce this.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Thanks again,<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Filip<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"" lang="CS">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"" lang="CS"> Ted Ralphs [mailto:<a href="mailto:ted@lehigh.edu" target="_blank">ted@lehigh.edu</a>]
<br>
<b>Sent:</b> Tuesday, October 28, 2014 2:12 AM<br>
<b>To:</b> Konvicka Filip<br>
<b>Cc:</b> <a href="mailto:symphony@list.coin-or.org" target="_blank">symphony@list.coin-or.org</a><br>
<b>Subject:</b> Re: [Symphony] Suspicious Code<u></u><u></u></span></p><div><div class="h5">
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<div>
<div>
<p class="MsoNormal" style="margin-bottom:12.0pt">Hi Filip,<u></u><u></u></p>
</div>
<p class="MsoNormal" style="margin-bottom:12.0pt">Sorry for the delay. I made the fixes you pointed out and pushed them to trunk and stable. The fixes for CoinUtils have already been released along with some other fixes I made. I will have a new release of
SYMPHONY incorporating everything in a day or two. Thanks for the feedback!<u></u><u></u></p>
</div>
<p class="MsoNormal">Cheers,<br>
<br>
Ted<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<p class="MsoNormal">On Thu, Sep 25, 2014 at 12:52 PM, Ted Ralphs <<a href="mailto:ted@lehigh.edu" target="_blank">ted@lehigh.edu</a>> wrote:<u></u><u></u></p>
<div>
<div>
<p class="MsoNormal" style="margin-bottom:12.0pt">Thanks for the report! I haven't had a chance to fix these issues yet, but I will do so soon.<u></u><u></u></p>
</div>
<p class="MsoNormal">Cheers,<br>
<br>
Ted<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<div>
<div>
<p class="MsoNormal">On Tue, Sep 16, 2014 at 7:03 AM, Konvicka Filip <<a href="mailto:Filip.Konvicka@logis.cz" target="_blank">Filip.Konvicka@logis.cz</a>> wrote:<u></u><u></u></p>
</div>
</div>
<blockquote style="border:none;border-left:solid #cccccc 1.0pt;padding:0cm 0cm 0cm 6.0pt;margin-left:4.8pt;margin-right:0cm">
<div>
<div>
<div>
<div>
<p class="MsoNormal">Hi,<span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal"> <span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal">While cleaning some warnings I stumbled upon several very suspicious lines of code in Symphony 5.5.7.<span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal"> <span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal">Note that item 1 is a change in the headers, and unless this is fixed this causes warnings in programs that link libsymphony.<span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal">Items 2 - 4 are possible bugs in Symphony.<span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal"> <span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal">1. In OsiSolverInterface.hpp and OsiClpSolverInterface.hpp, there are functions defined with the default parameter values of std::string::npos. The type of this is size_t, but
the argument for which the default is defined is unsigned. In 64bit builds, this leads to a truncation of the value.<span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal"> <span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal" style="text-autospace:none">
<span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS">
</span><span style="font-size:9.5pt;font-family:Consolas;color:blue;background:white" lang="CS">virtual</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS"> std::string getRowName(</span><span style="font-size:9.5pt;font-family:Consolas;color:blue;background:white" lang="CS">int</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS">
rowIndex,</span><span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal" style="text-autospace:none">
<span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS">
</span><span style="font-size:9.5pt;font-family:Consolas;color:blue;background:white" lang="CS">unsigned</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS"> maxLen =
</span><span style="font-size:9.5pt;font-family:Consolas;color:blue;background:white" lang="CS">static_cast</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS"><</span><span style="font-size:9.5pt;font-family:Consolas;color:blue;background:white" lang="CS">unsigned</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS">>(std::string::npos))
</span><span style="font-size:9.5pt;font-family:Consolas;color:blue;background:white" lang="CS">const</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS">;</span><span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal"><span lang="CS"> <u></u><u></u></span></p>
<p class="MsoNormal"><span lang="CS">I think that it is better to use<u></u><u></u></span></p>
<p class="MsoNormal"><span lang="CS"> <u></u><u></u></span></p>
<p class="MsoNormal" style="text-autospace:none">
<span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS">
</span><span style="font-size:9.5pt;font-family:Consolas;color:blue;background:white" lang="CS">virtual</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS"> std::string getRowName(</span><span style="font-size:9.5pt;font-family:Consolas;color:blue;background:white" lang="CS">int</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS">
rowIndex,</span><span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal" style="text-autospace:none">
<span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS">
</span><span style="font-size:9.5pt;font-family:Consolas;color:blue;background:white" lang="CS">unsigned</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS"> maxLen = std::numeric_limits<</span><span style="font-size:9.5pt;font-family:Consolas;color:blue;background:white" lang="CS">unsigned</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS">>::max())
</span><span style="font-size:9.5pt;font-family:Consolas;color:blue;background:white" lang="CS">const</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS">;</span><span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal"><span lang="CS"> <u></u><u></u></span></p>
<p class="MsoNormal"><span lang="CS">If you look at how COIN_INT_MAX is defined in CoinFinite.hpp, this is seems to be an acceptable COIN coding practice. There are 5 occurrences in total of this (in
</span>OsiSolverInterface.hpp and OsiClpSolverInterface.hpp). The only use of this that I could find is OsiNames.cpp which uses this as a limit to std::string::substr. Also this change should not break the ABI since this does not affect the symbols.<span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal"><span lang="CS"> <u></u><u></u></span></p>
<p class="MsoNormal">2. In lp_heuristics.c, line 1410:<span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal"> <span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal" style="text-autospace:none">
<span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS">
</span><span style="font-size:9.5pt;font-family:Consolas;color:blue;background:white" lang="CS">if</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS">(dive_depth > dive_depth_limit/2){
</span><span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal" style="text-autospace:none">
<span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS"> fix_incr_cnt =(</span><span style="font-size:9.5pt;font-family:Consolas;color:blue;background:white" lang="CS">int</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS">)(1.0*(frac_ip_cnt
- d_fixed_cnt)/</span><span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal" style="text-autospace:none">
<span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS"> (dive_depth_limit > (dive_depth ? dive_depth_limit - dive_depth : 1)));</span><span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal" style="text-autospace:none">
<span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS">
</span><span style="font-size:9.5pt;font-family:Consolas;color:blue;background:white" lang="CS">if</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS">(fix_incr_cnt < 1) fix_incr_cnt = 1;</span><span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal" style="text-autospace:none">
<span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS"> }</span><span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal"> <span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal">Look at the division. The divider is a bool value!
<span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS">
(dive_depth_limit > (dive_depth ? dive_depth_limit - dive_depth : 1))</span><span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal">I think that most likely this should have been (dive_depth_limit > dive_depth) ? dive_depth_limit-dive_depth : 1, which by the way is equivalent to std::max(dive_depth_limit - dive_depth,
1).<span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal"> <span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal" style="text-autospace:none">
<span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS">
</span><span style="font-size:9.5pt;font-family:Consolas;color:blue;background:white" lang="CS">if</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS">(dive_depth > dive_depth_limit/2){</span><span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal" style="text-autospace:none">
<span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS"> fix_incr_cnt =(</span><span style="font-size:9.5pt;font-family:Consolas;color:blue;background:white" lang="CS">int</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS">)(1.0*(frac_ip_cnt
- d_fixed_cnt)/</span><span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal" style="text-autospace:none">
<span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS"> ((dive_depth_limit > dive_depth) ? dive_depth_limit - dive_depth : 1));</span><span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal" style="text-autospace:none">
<span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS">
</span><span style="font-size:9.5pt;font-family:Consolas;color:blue;background:white" lang="CS">if</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS">(fix_incr_cnt < 1) fix_incr_cnt = 1;</span><span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal" style="text-autospace:none">
<span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS"> }</span><span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal"> <span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal">By the way, the warning that caught the compiler's attention was - lp_heuristics.c(1412): warning C4804: '/' : unsafe use of type 'bool' in operation<span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal"> <span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal">3. In master_prep_base.c:504, I can also see something very suspicious:<span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal"> <span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal" style="text-autospace:none">
<span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS">
</span><span style="font-size:9.5pt;font-family:Consolas;color:blue;background:white" lang="CS">if</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS">(</span><span style="font-size:9.5pt;font-family:Consolas;color:gray;background:white" lang="CS">P</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS">->ub
< (</span><span style="font-size:9.5pt;font-family:Consolas;color:gray;background:white" lang="CS">P</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS">->mip->obj_sense ==
</span><span style="font-size:9.5pt;font-family:Consolas;color:#6f008a;background:white" lang="CS">SYM_MAXIMIZE</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS">) ? -(</span><span style="font-size:9.5pt;font-family:Consolas;color:gray;background:white" lang="CS">P</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS">->mip->obj_offset)
: </span><span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal" style="text-autospace:none">
<span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS"> </span><span style="font-size:9.5pt;font-family:Consolas;color:gray;background:white" lang="CS">P</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS">->mip->obj_offset){</span><span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal"> <span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal">This is actually evaluated as (P->ub < (P->mip->obj_sense == SYM_MAXIMIZE)) ? -(P->mip->obj_offset) : P->mip->obj_offset, but obviously the intention was<span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal"> <span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal" style="text-autospace:none">
<span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS">
</span><span style="font-size:9.5pt;font-family:Consolas;color:blue;background:white" lang="CS">if</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS">(</span><span style="font-size:9.5pt;font-family:Consolas;color:gray;background:white" lang="CS">P</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS">->ub
< ((</span><span style="font-size:9.5pt;font-family:Consolas;color:gray;background:white" lang="CS">P</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS">->mip->obj_sense ==
</span><span style="font-size:9.5pt;font-family:Consolas;color:#6f008a;background:white" lang="CS">SYM_MAXIMIZE</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS">) ? -(</span><span style="font-size:9.5pt;font-family:Consolas;color:gray;background:white" lang="CS">P</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS">->mip->obj_offset)
: </span><span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal" style="text-autospace:none">
<span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS"> </span><span style="font-size:9.5pt;font-family:Consolas;color:gray;background:white" lang="CS">P</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS">->mip->obj_offset)){</span><span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal"> <span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal">Again the warning was master_prep_base.c(504): warning C4804: '<' : unsafe use of type 'bool' in operation<span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal"> <span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal">4. In master.c:231 and 233, there are assignments like this:<span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal"> <span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal" style="text-autospace:none">
<span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS"> tm_par->warm_start_node_limit = (</span><span style="font-size:9.5pt;font-family:Consolas;color:blue;background:white" lang="CS">int</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS">)</span><span style="font-size:9.5pt;font-family:Consolas;color:#6f008a;background:white" lang="CS">SYM_INFINITY</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS">;
</span><span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal"><span lang="CS"> <u></u><u></u></span></p>
<p class="MsoNormal">warm_start_node_limit is an int, while SYM_INFINITY is defined as 1e20 (i.e. a double). This does not make much sense (the conversion results in some strange value), so I think
that it's much safer to write<span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal"> <span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal" style="text-autospace:none">
<span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS"> tm_par->warm_start_node_limit =
</span><span style="font-size:9.5pt;font-family:Consolas;color:#6f008a;background:white" lang="CS">INT_MAX</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS">;
</span><span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal" style="text-autospace:none">
<span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS"> </span><span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal" style="text-autospace:none">
Note that 1e20 mod (UINT_MAX+1) is the number that gets assigned, which is 1661992960 for 4byte ints but zero for 2byte ints. Lucky we don't have small ints anymore :-)<span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal" style="text-autospace:none">
<span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal" style="text-autospace:none">
Cheers,<span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal" style="text-autospace:none">
Filip<span lang="CS"><u></u><u></u></span></p>
<p class="MsoNormal" style="text-autospace:none">
<span style="font-size:9.5pt;font-family:Consolas;color:black;background:white" lang="CS"> </span><span lang="CS"><u></u><u></u></span></p>
</div>
</div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
</div>
<p class="MsoNormal" style="margin-bottom:12.0pt">_______________________________________________<br>
Symphony mailing list<br>
<a href="mailto:Symphony@list.coin-or.org" target="_blank">Symphony@list.coin-or.org</a><br>
<a href="http://list.coin-or.org/mailman/listinfo/symphony" target="_blank">http://list.coin-or.org/mailman/listinfo/symphony</a><u></u><u></u></p>
</blockquote>
</div>
<p class="MsoNormal"><span style="color:#888888"><br>
<br clear="all">
<br>
<span>-- </span><br>
<span>Dr. Ted Ralphs</span><br>
<span>Associate Professor, Lehigh University</span><br>
<span><a href="tel:%28610%29%20628-1280" target="_blank">(610) 628-1280</a></span><br>
<span>ted 'at' lehigh 'dot' edu</span><br>
<span><a href="http://coral.ie.lehigh.edu/~ted" target="_blank">coral.ie.lehigh.edu/~ted</a></span></span><u></u><u></u></p>
</div>
</div>
<p class="MsoNormal"><br>
<br clear="all">
<br>
-- <u></u><u></u></p>
<div>
<p class="MsoNormal">Dr. Ted Ralphs<br>
Professor, Lehigh University<br>
<a href="tel:%28610%29%20628-1280" value="+16106281280" target="_blank">(610) 628-1280</a><br>
ted 'at' lehigh 'dot' edu<br>
<a href="http://coral.ie.lehigh.edu/~ted" target="_blank">coral.ie.lehigh.edu/~ted</a><u></u><u></u></p>
</div>
</div>
</div></div></div>
</div>
</blockquote></div><br><br clear="all"><br>-- <br><div class="gmail_signature"><div dir="ltr">Dr. Ted Ralphs<br>Professor, Lehigh University<br>(610) 628-1280<br>ted 'at' lehigh 'dot' edu<br><a href="http://coral.ie.lehigh.edu/~ted" target="_blank">coral.ie.lehigh.edu/~ted</a><br></div></div>
</div>