As the e-mail said, it was "clang" that was used as the compiler. There
are two distinct considerations. The first was in etBUFFSIZE
signed/unsigned promotion, which arose from the looseness of c-typing
rules and signed/unsigned promotion versus c++. It otherwise is not
critical, while debugging builds with various compilation settings I
caught it doing a "unsigned" promotion of "precision" when precision was
set to (-1) and that result in precision being assigned "etBUFFSIZE-40".
Probably somewhat confusing since I saw the original mail had I lost the
(int) cast. I immediately sent a update with the errata (since I had no
ticket mechanism), but that we probably overlooked as it was subtle in
the e-mail. Here it is again, it was supposed to read as:
/* Limit the precision to prevent overflowing buf[] during
conversion */
if( precision > (int)(etBUFSIZE-40) && (infop->flags &
FLAG_STRING)==0 ){
precision = etBUFSIZE-40;
}
The second is a bug I caught last year after a number of intermittent
crashes until I had a 100% repro occurring that I could track and debug.
The details of the second issue are/were explained in the
fixed-source-code's revision comments that was in the original e-mail
posting.
// DSIM: This code has a bug where it assumes NULL '\0' terminated
bufpt
// However, if *precision* is provided and> 0 and limit is -1 then
// precision defines maximum number of characters in bufpt. Without
// that rule, and bufpt not being '\0' null terminated, this code
// BLOWS UP randomly since bufpt memory can be random beyond
bufpt[precision].
length = StrNLen32(bufpt, limit); //<== SOURCE OF BUG (GOES BOOM
RARELY, BUT RANDOMLY!)-- David
p.s., I have other fixes but we'll see how this one goes.
Richard Hipp <mailto:d...@sqlite.org>
Sunday, May 21, 2017 3:40 PM
On 5/21/17, David Simmons<d...@smallscript.com> wrote:
Location: "fossil\src\printf.c"
Section(s): (line 385)
/* Limit the precision to prevent overflowing buf[] during
conversion */
if( precision> (etBUFSIZE-40)&& (infop->flags& FLAG_STRING)==0 ){
precision = etBUFSIZE-40;
}
Issue: missing parens to ensure correct ordering of expressing. Lacking
parens, it sets precision to "etBUFSIZE-40" ~ (500-40) => 460
Impact: inefficient behavior, possible output errors.
What compiler are you using that thinks that the ">" operator has
higher precedence than the "-" operator?
And I don't quite understand what the second complaint (below) is
about. Could you explain the problem again - perhaps with an example
that shows the failure?
Location: "fossil\src\printf.c"
Section(s): (line 200 ... line 645)
static int StrNLen32(const char *z, int N){
...elided...
case etSTRINGID:
case etSTRING:
case etDYNSTRING: {
int limit = flag_alternateform ? va_arg(ap,int) : -1;
bufpt = va_arg(ap,char*);
if(bufpt == 0){
bufpt = "";
}else if( xtype==etDYNSTRING ){
zExtra = bufpt;
}else if( xtype==etSTRINGID ){
precision = hashDigits(flag_altform2);
}
// printf rules say: (fixes from DSIM)
// "width" is a minimum for the output of this param
// "precision" is a maximum or exact size of this param
if(precision>= 0&& xtype != etSTRINGID) {
limit = length = precision;
int uLen32 = StrNLen32(bufpt, limit);
// DSim: assert(uLen32 == length);
}
else {
if(limit< 0) {
if(precision> 0)
limit = precision;
else if(width)
limit = width;
}
else if(limit> precision) {
limit = precision;
}
else if(limit> width) {
limit = width;
}
// DSIM: This code has a bug where it assumes NULL '\0'
terminated bufpt
// However, if *precision* is provided and> 0 and limit is
-1 then
// precision defines maximum number of characters in bufpt.
Without
// that rule, and bufpt not being '\0' null terminated, this code
// BLOWS UP randomly since bufpt memory can be random beyond
bufpt[precision].
length = StrNLen32(bufpt, limit); //<== SOURCE OF BUG (GOES
BOOM RARELY, BUT RANDOMLY!)
}
break;
}
Issue: code does not insure reading past end of z, due to calling code
in '%...s' miscalculation of default value for N.
Impact: intermittent app-crashes
_______________________________________________
fossil-dev mailing list
fossil-dev@mailinglists.sqlite.org
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/fossil-dev
David Simmons <mailto:d...@smallscript.com>
Sunday, May 21, 2017 2:00 PM
Location: "fossil\src\printf.c"
Section(s): (line 385)
/* Limit the precision to prevent overflowing buf[] during
conversion */
if( precision > (etBUFSIZE-40) && (infop->flags & FLAG_STRING)==0 ){
precision = etBUFSIZE-40;
}
Issue: missing parens to ensure correct ordering of expressing.
Lacking parens, it sets precision to "etBUFSIZE-40" ~ (500-40) => 460
Impact: inefficient behavior, possible output errors.
Location: "fossil\src\printf.c"
Section(s): (line 200 ... line 645)
static int StrNLen32(const char *z, int N){
...elided...
case etSTRINGID:
case etSTRING:
case etDYNSTRING: {
int limit = flag_alternateform ? va_arg(ap,int) : -1;
bufpt = va_arg(ap,char*);
if(bufpt == 0){
bufpt = "";
}else if( xtype==etDYNSTRING ){
zExtra = bufpt;
}else if( xtype==etSTRINGID ){
precision = hashDigits(flag_altform2);
}
// printf rules say: (fixes from DSIM)
// "width" is a minimum for the output of this param
// "precision" is a maximum or exact size of this param
if(precision >= 0 && xtype != etSTRINGID) {
limit = length = precision;
int uLen32 = StrNLen32(bufpt, limit);
// DSim: assert(uLen32 == length);
}
else {
if(limit < 0) {
if(precision > 0)
limit = precision;
else if(width)
limit = width;
}
else if(limit > precision) {
limit = precision;
}
else if(limit > width) {
limit = width;
}
// DSIM: This code has a bug where it assumes NULL '\0'
terminated bufpt
// However, if *precision* is provided and > 0 and limit is
-1 then
// precision defines maximum number of characters in bufpt.
Without
// that rule, and bufpt not being '\0' null terminated, this
code
// BLOWS UP randomly since bufpt memory can be random beyond
bufpt[precision].
length = StrNLen32(bufpt, limit); // <== SOURCE OF BUG (GOES
BOOM RARELY, BUT RANDOMLY!)
}
break;
}
Issue: code does not insure reading past end of z, due to calling code
in '%...s' miscalculation of default value for N.
Impact: intermittent app-crashes
_______________________________________________
fossil-dev mailing list
fossil-dev@mailinglists.sqlite.org
http://mailinglists.sqlite.org/cgi-bin/mailman/listinfo/fossil-dev
