<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<meta name="Generator" content="Microsoft Word 14 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:"MS Mincho";
panose-1:2 2 6 9 4 2 5 8 3 4;}
@font-face
{font-family:"MS Mincho";
panose-1:2 2 6 9 4 2 5 8 3 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:"\@MS Mincho";
panose-1:2 2 6 9 4 2 5 8 3 4;}
@font-face
{font-family:Consolas;
panose-1:2 11 6 9 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0cm;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri","sans-serif";}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
span.StylE-mailovZprvy17
{mso-style-type:personal-compose;
font-family:"Calibri","sans-serif";
color:windowtext;}
.MsoChpDefault
{mso-style-type:export-only;}
@page WordSection1
{size:612.0pt 792.0pt;
margin:70.85pt 70.85pt 70.85pt 70.85pt;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="CS" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal"><span lang="EN-US">Hi,<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">While cleaning some warnings I stumbled upon several very suspicious lines of code in Symphony 5.5.7.<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">Note that item 1 is a change in the headers, and unless this is fixed this causes warnings in programs that link libsymphony.<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">Items 2 - 4 are possible bugs in Symphony.<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">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.<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal" style="text-autospace:none"><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white">
</span><span style="font-size:9.5pt;font-family:Consolas;color:blue;background:white;mso-highlight:white">virtual</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white"> std::string getRowName(</span><span style="font-size:9.5pt;font-family:Consolas;color:blue;background:white;mso-highlight:white">int</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white">
rowIndex,<o:p></o:p></span></p>
<p class="MsoNormal" style="text-autospace:none"><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white">
</span><span style="font-size:9.5pt;font-family:Consolas;color:blue;background:white;mso-highlight:white">unsigned</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white"> maxLen =
</span><span style="font-size:9.5pt;font-family:Consolas;color:blue;background:white;mso-highlight:white">static_cast</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white"><</span><span style="font-size:9.5pt;font-family:Consolas;color:blue;background:white;mso-highlight:white">unsigned</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white">>(std::string::npos))
</span><span style="font-size:9.5pt;font-family:Consolas;color:blue;background:white;mso-highlight:white">const</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white">;<o:p></o:p></span></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">I think that it is better to use<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal" style="text-autospace:none"><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white">
</span><span style="font-size:9.5pt;font-family:Consolas;color:blue;background:white;mso-highlight:white">virtual</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white"> std::string getRowName(</span><span style="font-size:9.5pt;font-family:Consolas;color:blue;background:white;mso-highlight:white">int</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white">
rowIndex,<o:p></o:p></span></p>
<p class="MsoNormal" style="text-autospace:none"><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white">
</span><span style="font-size:9.5pt;font-family:Consolas;color:blue;background:white;mso-highlight:white">unsigned</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white"> maxLen = std::numeric_limits<</span><span style="font-size:9.5pt;font-family:Consolas;color:blue;background:white;mso-highlight:white">unsigned</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white">>::max())
</span><span style="font-size:9.5pt;font-family:Consolas;color:blue;background:white;mso-highlight:white">const</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white">;<o:p></o:p></span></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">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 lang="EN-US">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><o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><span lang="EN-US">2. In lp_heuristics.c, line 1410:<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal" style="text-autospace:none"><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white">
</span><span style="font-size:9.5pt;font-family:Consolas;color:blue;background:white;mso-highlight:white">if</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white">(dive_depth > dive_depth_limit/2){
<o:p></o:p></span></p>
<p class="MsoNormal" style="text-autospace:none"><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white"> fix_incr_cnt =(</span><span style="font-size:9.5pt;font-family:Consolas;color:blue;background:white;mso-highlight:white">int</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white">)(1.0*(frac_ip_cnt
- d_fixed_cnt)/<o:p></o:p></span></p>
<p class="MsoNormal" style="text-autospace:none"><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white"> (dive_depth_limit > (dive_depth ? dive_depth_limit - dive_depth : 1)));<o:p></o:p></span></p>
<p class="MsoNormal" style="text-autospace:none"><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white">
</span><span style="font-size:9.5pt;font-family:Consolas;color:blue;background:white;mso-highlight:white">if</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white">(fix_incr_cnt < 1) fix_incr_cnt = 1;<o:p></o:p></span></p>
<p class="MsoNormal" style="text-autospace:none"><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white"> }<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">Look at the division. The divider is a bool value!
</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white">(dive_depth_limit > (dive_depth ? dive_depth_limit - dive_depth : 1))</span><span style="font-size:9.5pt;font-family:Consolas;color:black"><o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">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><span style="font-size:9.5pt;font-family:Consolas;color:black"><o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal" style="text-autospace:none"><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white">
</span><span style="font-size:9.5pt;font-family:Consolas;color:blue;background:white;mso-highlight:white">if</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white">(dive_depth > dive_depth_limit/2){<o:p></o:p></span></p>
<p class="MsoNormal" style="text-autospace:none"><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white"> fix_incr_cnt =(</span><span style="font-size:9.5pt;font-family:Consolas;color:blue;background:white;mso-highlight:white">int</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white">)(1.0*(frac_ip_cnt
- d_fixed_cnt)/<o:p></o:p></span></p>
<p class="MsoNormal" style="text-autospace:none"><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white"> ((dive_depth_limit > dive_depth) ? dive_depth_limit - dive_depth : 1));<o:p></o:p></span></p>
<p class="MsoNormal" style="text-autospace:none"><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white">
</span><span style="font-size:9.5pt;font-family:Consolas;color:blue;background:white;mso-highlight:white">if</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white">(fix_incr_cnt < 1) fix_incr_cnt = 1;<o:p></o:p></span></p>
<p class="MsoNormal" style="text-autospace:none"><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white"> }<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">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<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">3. In master_prep_base.c:504, I can also see something very suspicious:<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal" style="text-autospace:none"><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white">
</span><span style="font-size:9.5pt;font-family:Consolas;color:blue;background:white;mso-highlight:white">if</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white">(</span><span style="font-size:9.5pt;font-family:Consolas;color:gray;background:white;mso-highlight:white">P</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white">->ub
< (</span><span style="font-size:9.5pt;font-family:Consolas;color:gray;background:white;mso-highlight:white">P</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white">->mip->obj_sense ==
</span><span style="font-size:9.5pt;font-family:Consolas;color:#6F008A;background:white;mso-highlight:white">SYM_MAXIMIZE</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white">) ? -(</span><span style="font-size:9.5pt;font-family:Consolas;color:gray;background:white;mso-highlight:white">P</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white">->mip->obj_offset)
: <o:p></o:p></span></p>
<p class="MsoNormal" style="text-autospace:none"><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white"> </span><span style="font-size:9.5pt;font-family:Consolas;color:gray;background:white;mso-highlight:white">P</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white">->mip->obj_offset){<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">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<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal" style="text-autospace:none"><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white">
</span><span style="font-size:9.5pt;font-family:Consolas;color:blue;background:white;mso-highlight:white">if</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white">(</span><span style="font-size:9.5pt;font-family:Consolas;color:gray;background:white;mso-highlight:white">P</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white">->ub
< ((</span><span style="font-size:9.5pt;font-family:Consolas;color:gray;background:white;mso-highlight:white">P</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white">->mip->obj_sense ==
</span><span style="font-size:9.5pt;font-family:Consolas;color:#6F008A;background:white;mso-highlight:white">SYM_MAXIMIZE</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white">) ? -(</span><span style="font-size:9.5pt;font-family:Consolas;color:gray;background:white;mso-highlight:white">P</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white">->mip->obj_offset)
: <o:p></o:p></span></p>
<p class="MsoNormal" style="text-autospace:none"><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white"> </span><span style="font-size:9.5pt;font-family:Consolas;color:gray;background:white;mso-highlight:white">P</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white">->mip->obj_offset)){<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">Again the warning was master_prep_base.c(504): warning C4804: '<' : unsafe use of type 'bool' in operation<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US">4. In master.c:231 and 233, there are assignments like this:<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal" style="text-autospace:none"><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white"> tm_par->warm_start_node_limit = (</span><span style="font-size:9.5pt;font-family:Consolas;color:blue;background:white;mso-highlight:white">int</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white">)</span><span style="font-size:9.5pt;font-family:Consolas;color:#6F008A;background:white;mso-highlight:white">SYM_INFINITY</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white">;
<o:p></o:p></span></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><span lang="EN-US">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<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal" style="text-autospace:none"><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white"> tm_par->warm_start_node_limit =
</span><span style="font-size:9.5pt;font-family:Consolas;color:#6F008A;background:white;mso-highlight:white">INT_MAX</span><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white">;
<o:p></o:p></span></p>
<p class="MsoNormal" style="text-autospace:none"><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white"><o:p> </o:p></span></p>
<p class="MsoNormal" style="text-autospace:none"><span lang="EN-US">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 :-)<o:p></o:p></span></p>
<p class="MsoNormal" style="text-autospace:none"><span lang="EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal" style="text-autospace:none"><span lang="EN-US">Cheers,<o:p></o:p></span></p>
<p class="MsoNormal" style="text-autospace:none"><span lang="EN-US">Filip<o:p></o:p></span></p>
<p class="MsoNormal" style="text-autospace:none"><span style="font-size:9.5pt;font-family:Consolas;color:black;background:white;mso-highlight:white"><o:p> </o:p></span></p>
</div>
</body>
</html>