From 38ccc34aa839a22ac1ddfef1d0f546e3447de3c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Tue, 27 Jan 2015 21:08:45 +0100 Subject: [PATCH 01/35] First try --- src/expression.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/expression.c b/src/expression.c index b654d4a2dc0f..cf18a0448e4b 100644 --- a/src/expression.c +++ b/src/expression.c @@ -10046,6 +10046,29 @@ Expression *SliceExp::semantic(Scope *sc) lwr = lwr->optimize(WANTvalue); upr = upr->optimize(WANTvalue); + if (lwr->op == TOKdiv) + { + assert(reinterpret_cast(lwr)); // TODO remove + DivExp* lwrDiv = (DivExp*)lwr; // TODO always true? + } + else if (lwr->op == TOKvar) + { + assert(reinterpret_cast(lwr)); // TODO remove + VarExp* lwrVar = (VarExp*)lwr; // TODO always true? + } + + if (upr->op == TOKdiv) + { + assert(reinterpret_cast(upr)); // TODO remove + DivExp* uprDiv = (DivExp*)upr; // TODO always true? + } + else if (upr->op == TOKvar) + { + assert(reinterpret_cast(upr)); // TODO remove + VarExp* uprVar = (VarExp*)upr; // TODO always true? + printf("xxx"); + } + IntRange lwrRange = getIntRange(lwr); IntRange uprRange = getIntRange(upr); From bc850d56b6f36a1c0d624c35f2467d19963bb89a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Tue, 27 Jan 2015 23:19:35 +0100 Subject: [PATCH 02/35] First version that does anything useful --- src/expression.c | 49 ++++++++++++++++++++++++++++++++++-------------- 1 file changed, 35 insertions(+), 14 deletions(-) diff --git a/src/expression.c b/src/expression.c index cf18a0448e4b..d765d94e0a64 100644 --- a/src/expression.c +++ b/src/expression.c @@ -9812,6 +9812,20 @@ Expression *SliceExp::syntaxCopy() return se; } +static bool isOpDollar(VarExp* ve) +{ + VarDeclaration* decl = ve->var->isVarDeclaration(); + if (decl) + { + if (decl->ident->len == 8 && + memcmp(decl->ident->string, "__dollar", decl->ident->len) == 0) // TODO is there a function for this? + { + return true; + } + } + return false; +} + Expression *SliceExp::semantic(Scope *sc) { #if LOGSEMANTIC @@ -10046,27 +10060,34 @@ Expression *SliceExp::semantic(Scope *sc) lwr = lwr->optimize(WANTvalue); upr = upr->optimize(WANTvalue); - if (lwr->op == TOKdiv) + // avoid bounds-checking for slice bounds in form $/n + // How to use resolveOpDollar? + if (upr->op == TOKdiv) // start with upper because of lowerIsLessThanUpper { - assert(reinterpret_cast(lwr)); // TODO remove - DivExp* lwrDiv = (DivExp*)lwr; // TODO always true? + DivExp* div = (DivExp*)upr; // TODO is*Exp predicate? } - else if (lwr->op == TOKvar) + else if (upr->op == TOKvar) { - assert(reinterpret_cast(lwr)); // TODO remove - VarExp* lwrVar = (VarExp*)lwr; // TODO always true? + VarExp* var = (VarExp*)upr; // TODO is*Exp predicate? + const bool isInBounds = isOpDollar(var); + if (isInBounds) + { + e1->warning("Avoiding boundscheck for upper bound '%s'", var->toChars()); + } + this->upperIsInBounds = isInBounds; } - - if (upr->op == TOKdiv) + if (lwr->op == TOKdiv) // then lower { - assert(reinterpret_cast(upr)); // TODO remove - DivExp* uprDiv = (DivExp*)upr; // TODO always true? + DivExp* div = (DivExp*)lwr; // TODO is*Exp predicate? } - else if (upr->op == TOKvar) + else if (lwr->op == TOKvar) { - assert(reinterpret_cast(upr)); // TODO remove - VarExp* uprVar = (VarExp*)upr; // TODO always true? - printf("xxx"); + VarExp* var = (VarExp*)lwr; // TODO is*Exp predicate? + const bool isInBounds = isOpDollar(var); + if (isInBounds) + { + e1->warning("Avoiding boundscheck for lower bound '%s'", var->toChars()); + } } IntRange lwrRange = getIntRange(lwr); From cf7f4087aafe7c9e5ea740afa6e82c1347340c34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Tue, 27 Jan 2015 23:22:22 +0100 Subject: [PATCH 03/35] Add utility function lowerIsInBounds --- src/expression.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/expression.h b/src/expression.h index 616d56566aa5..9c241ee179a2 100644 --- a/src/expression.h +++ b/src/expression.h @@ -1009,6 +1009,8 @@ class SliceExp : public UnaExp VarDeclaration *lengthVar; bool upperIsInBounds; // true if upr <= e1.length bool lowerIsLessThanUpper; // true if lwr <= upr + bool lowerIsInBounds() const { return (lowerIsLessThanUpper && + upperIsInBounds); }; // true if lwr <= e1.length SliceExp(Loc loc, Expression *e1, Expression *lwr, Expression *upr); Expression *syntaxCopy(); From 991c6bd2de3435da83f205dcf6f164555842eb30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Tue, 27 Jan 2015 23:47:11 +0100 Subject: [PATCH 04/35] Set lowerIsLessThanUpper to true for [$ .. $] slicings --- src/expression.c | 52 +++++++++++++++++++++++++++++------------------- 1 file changed, 31 insertions(+), 21 deletions(-) diff --git a/src/expression.c b/src/expression.c index d765d94e0a64..37f37cb451ba 100644 --- a/src/expression.c +++ b/src/expression.c @@ -10062,31 +10062,41 @@ Expression *SliceExp::semantic(Scope *sc) // avoid bounds-checking for slice bounds in form $/n // How to use resolveOpDollar? - if (upr->op == TOKdiv) // start with upper because of lowerIsLessThanUpper { - DivExp* div = (DivExp*)upr; // TODO is*Exp predicate? - } - else if (upr->op == TOKvar) - { - VarExp* var = (VarExp*)upr; // TODO is*Exp predicate? - const bool isInBounds = isOpDollar(var); - if (isInBounds) + bool upperAtEnd = false; + if (upr->op == TOKdiv) // start with upper because of lowerIsLessThanUpper { - e1->warning("Avoiding boundscheck for upper bound '%s'", var->toChars()); + DivExp* div = (DivExp*)upr; // TODO is*Exp predicate? } - this->upperIsInBounds = isInBounds; - } - if (lwr->op == TOKdiv) // then lower - { - DivExp* div = (DivExp*)lwr; // TODO is*Exp predicate? - } - else if (lwr->op == TOKvar) - { - VarExp* var = (VarExp*)lwr; // TODO is*Exp predicate? - const bool isInBounds = isOpDollar(var); - if (isInBounds) + else if (upr->op == TOKvar) + { + VarExp* var = (VarExp*)upr; // TODO is*Exp predicate? + const bool isInBounds = isOpDollar(var); + if (isInBounds) + { + e1->warning("Avoiding boundscheck for upper part '%s'", var->toChars()); + } + this->upperIsInBounds = isInBounds; + upperAtEnd = isInBounds; + } + bool lowerAtEnd = false; + if (lwr->op == TOKdiv) // then lower + { + DivExp* div = (DivExp*)lwr; // TODO is*Exp predicate? + } + else if (lwr->op == TOKvar) + { + VarExp* var = (VarExp*)lwr; // TODO is*Exp predicate? + const bool isInBounds = isOpDollar(var); + if (isInBounds) + { + e1->warning("Avoiding boundscheck for lower part '%s'", var->toChars()); + } + lowerAtEnd = isInBounds; + } + if (lowerAtEnd && upperAtEnd) { - e1->warning("Avoiding boundscheck for lower bound '%s'", var->toChars()); + lowerIsLessThanUpper = true; } } From 80f1aaa9395d37abd1032fd5845a40ceef404e69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Wed, 28 Jan 2015 00:20:40 +0100 Subject: [PATCH 05/35] Simplify isOpDollar by making use of Id::dollar --- src/expression.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/expression.c b/src/expression.c index 37f37cb451ba..11f4b6b3d7db 100644 --- a/src/expression.c +++ b/src/expression.c @@ -9815,15 +9815,7 @@ Expression *SliceExp::syntaxCopy() static bool isOpDollar(VarExp* ve) { VarDeclaration* decl = ve->var->isVarDeclaration(); - if (decl) - { - if (decl->ident->len == 8 && - memcmp(decl->ident->string, "__dollar", decl->ident->len) == 0) // TODO is there a function for this? - { - return true; - } - } - return false; + return decl && decl->ident == Id::dollar; } Expression *SliceExp::semantic(Scope *sc) @@ -10060,7 +10052,7 @@ Expression *SliceExp::semantic(Scope *sc) lwr = lwr->optimize(WANTvalue); upr = upr->optimize(WANTvalue); - // avoid bounds-checking for slice bounds in form $/n + // avoid bounds-checking for slice bounds when possible // How to use resolveOpDollar? { bool upperAtEnd = false; From 717d482df4cef5621b0c63796cdac728c4f85e76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Wed, 28 Jan 2015 00:52:53 +0100 Subject: [PATCH 06/35] Handle case were lower bound is zero --- src/expression.c | 51 ++++++++++++++++++++++++++++++------------------ 1 file changed, 32 insertions(+), 19 deletions(-) diff --git a/src/expression.c b/src/expression.c index 11f4b6b3d7db..f2a816d9cdd6 100644 --- a/src/expression.c +++ b/src/expression.c @@ -10052,41 +10052,54 @@ Expression *SliceExp::semantic(Scope *sc) lwr = lwr->optimize(WANTvalue); upr = upr->optimize(WANTvalue); - // avoid bounds-checking for slice bounds when possible - // How to use resolveOpDollar? + // avoid slice bounds-checking. TODO use resolveOpDollar? { - bool upperAtEnd = false; - if (upr->op == TOKdiv) // start with upper because of lowerIsLessThanUpper + // upper + bool uprAtStart = false, uprAtEnd = false; + if (upr->op == TOKint64) { - DivExp* div = (DivExp*)upr; // TODO is*Exp predicate? + IntegerExp* int_ = (IntegerExp*)upr; // TODO is*Exp predicate? + const bool isInBounds = int_->value == 0; + if (isInBounds) { e1->warning("Avoiding boundscheck for upper part '%s'", int_->toChars()); } + uprAtStart = isInBounds; } else if (upr->op == TOKvar) { VarExp* var = (VarExp*)upr; // TODO is*Exp predicate? const bool isInBounds = isOpDollar(var); - if (isInBounds) - { - e1->warning("Avoiding boundscheck for upper part '%s'", var->toChars()); - } + if (isInBounds) { e1->warning("Avoiding boundscheck for upper part '%s'", var->toChars()); } this->upperIsInBounds = isInBounds; - upperAtEnd = isInBounds; + uprAtEnd = isInBounds; } - bool lowerAtEnd = false; - if (lwr->op == TOKdiv) // then lower + else if (upr->op == TOKdiv) { - DivExp* div = (DivExp*)lwr; // TODO is*Exp predicate? + DivExp* div = (DivExp*)upr; // TODO is*Exp predicate? + } + + // lower + bool lwrAtStart = false, lwrAtEnd = false; + if (lwr->op == TOKint64) + { + IntegerExp* int_ = (IntegerExp*)lwr; // TODO is*Exp predicate? + const bool isInBounds = int_->value == 0; + if (isInBounds) { e1->warning("Avoiding boundscheck for lower part '%s'", int_->toChars()); } + lwrAtStart = isInBounds; } else if (lwr->op == TOKvar) { VarExp* var = (VarExp*)lwr; // TODO is*Exp predicate? const bool isInBounds = isOpDollar(var); - if (isInBounds) - { - e1->warning("Avoiding boundscheck for lower part '%s'", var->toChars()); - } - lowerAtEnd = isInBounds; + if (isInBounds) { e1->warning("Avoiding boundscheck for lower part '%s'", var->toChars()); } + lwrAtEnd = isInBounds; } - if (lowerAtEnd && upperAtEnd) + else if (lwr->op == TOKdiv) // then lower + { + DivExp* div = (DivExp*)lwr; // TODO is*Exp predicate? + } + + // lower and upper + if (lwrAtStart || // [0 .. X] + lwrAtEnd && uprAtEnd) // [$ .. $] { lowerIsLessThanUpper = true; } From 2be613e50c106470e5cebbe75fd9b9c6a5de322e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Wed, 28 Jan 2015 00:57:31 +0100 Subject: [PATCH 07/35] Simplify --- src/expression.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/src/expression.c b/src/expression.c index f2a816d9cdd6..11597d4347dc 100644 --- a/src/expression.c +++ b/src/expression.c @@ -10059,17 +10059,15 @@ Expression *SliceExp::semantic(Scope *sc) if (upr->op == TOKint64) { IntegerExp* int_ = (IntegerExp*)upr; // TODO is*Exp predicate? - const bool isInBounds = int_->value == 0; - if (isInBounds) { e1->warning("Avoiding boundscheck for upper part '%s'", int_->toChars()); } - uprAtStart = isInBounds; + uprAtStart = int_->value == 0; + if (uprAtStart) { e1->warning("Avoiding boundscheck for upper part '%s'", int_->toChars()); } } else if (upr->op == TOKvar) { VarExp* var = (VarExp*)upr; // TODO is*Exp predicate? - const bool isInBounds = isOpDollar(var); - if (isInBounds) { e1->warning("Avoiding boundscheck for upper part '%s'", var->toChars()); } - this->upperIsInBounds = isInBounds; - uprAtEnd = isInBounds; + uprAtEnd = isOpDollar(var); + if (uprAtEnd) { e1->warning("Avoiding boundscheck for upper part '%s'", var->toChars()); } + this->upperIsInBounds = uprAtEnd; } else if (upr->op == TOKdiv) { @@ -10081,16 +10079,14 @@ Expression *SliceExp::semantic(Scope *sc) if (lwr->op == TOKint64) { IntegerExp* int_ = (IntegerExp*)lwr; // TODO is*Exp predicate? - const bool isInBounds = int_->value == 0; - if (isInBounds) { e1->warning("Avoiding boundscheck for lower part '%s'", int_->toChars()); } - lwrAtStart = isInBounds; + lwrAtStart = int_->value == 0; + if (lwrAtStart) { e1->warning("Avoiding boundscheck for lower part '%s'", int_->toChars()); } } else if (lwr->op == TOKvar) { VarExp* var = (VarExp*)lwr; // TODO is*Exp predicate? - const bool isInBounds = isOpDollar(var); - if (isInBounds) { e1->warning("Avoiding boundscheck for lower part '%s'", var->toChars()); } - lwrAtEnd = isInBounds; + lwrAtEnd = isOpDollar(var); + if (lwrAtEnd) { e1->warning("Avoiding boundscheck for lower part '%s'", var->toChars()); } } else if (lwr->op == TOKdiv) // then lower { From 4df5e9a0a5ccd8841b931eaa982fca5767983bb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Wed, 28 Jan 2015 01:26:55 +0100 Subject: [PATCH 08/35] Implement Step 3 --- src/expression.c | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/src/expression.c b/src/expression.c index 11597d4347dc..77c5f4395133 100644 --- a/src/expression.c +++ b/src/expression.c @@ -10055,7 +10055,8 @@ Expression *SliceExp::semantic(Scope *sc) // avoid slice bounds-checking. TODO use resolveOpDollar? { // upper - bool uprAtStart = false, uprAtEnd = false; + bool uprAtStart = false, uprIn = false, uprAtEnd = false; + long uprDiv = 0; // non-zero means [ _ .. $/uprDiv ] if (upr->op == TOKint64) { IntegerExp* int_ = (IntegerExp*)upr; // TODO is*Exp predicate? @@ -10072,10 +10073,21 @@ Expression *SliceExp::semantic(Scope *sc) else if (upr->op == TOKdiv) { DivExp* div = (DivExp*)upr; // TODO is*Exp predicate? + if (div->e1->op == TOKvar && + div->e2->op == TOKint64) + { + VarExp* p = (VarExp*)div->e1; // TODO is*Exp predicate? + IntegerExp* q = (IntegerExp*)div->e2; // TODO is*Exp predicate? + uprDiv = q->value; + uprIn = uprDiv >= 1; + if (uprIn) { e1->warning("Avoiding boundscheck for upper part '%s'", div->toChars()); } + this->upperIsInBounds = uprIn; + } } // lower - bool lwrAtStart = false, lwrAtEnd = false; + bool lwrAtStart = false, lwrIn = false, lwrAtEnd = false; + long lwrDiv = 0; // non-zero means [ $/lwrDiv .. _ ] if (lwr->op == TOKint64) { IntegerExp* int_ = (IntegerExp*)lwr; // TODO is*Exp predicate? @@ -10088,14 +10100,24 @@ Expression *SliceExp::semantic(Scope *sc) lwrAtEnd = isOpDollar(var); if (lwrAtEnd) { e1->warning("Avoiding boundscheck for lower part '%s'", var->toChars()); } } - else if (lwr->op == TOKdiv) // then lower + else if (lwr->op == TOKdiv) { DivExp* div = (DivExp*)lwr; // TODO is*Exp predicate? + if (div->e1->op == TOKvar && + div->e2->op == TOKint64) + { + VarExp* p = (VarExp*)div->e1; // TODO is*Exp predicate? + IntegerExp* q = (IntegerExp*)div->e2; // TODO is*Exp predicate? + lwrDiv = q->value; + lwrIn = lwrDiv >= 1; + if (lwrIn) { e1->warning("Avoiding boundscheck for lower part '%s'", div->toChars()); } + } } // lower and upper if (lwrAtStart || // [0 .. X] - lwrAtEnd && uprAtEnd) // [$ .. $] + lwrAtEnd && uprAtEnd || // [$ .. $] + (lwrDiv != 0 && uprDiv != 0 && lwrDiv >= uprDiv)) // [$/m .. $/n], m >= n { lowerIsLessThanUpper = true; } From a811f6e4b1b295f89625da5c42589968dcd86dde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Wed, 28 Jan 2015 01:36:13 +0100 Subject: [PATCH 09/35] Keep lowerIsLessThanUpper as true if existing logic cannot prove things --- src/expression.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/expression.c b/src/expression.c index 77c5f4395133..7605c03f0bd1 100644 --- a/src/expression.c +++ b/src/expression.c @@ -10140,12 +10140,12 @@ Expression *SliceExp::semantic(Scope *sc) } else if (t1b->ty == Tpointer) { - this->upperIsInBounds = true; + this->upperIsInBounds = true; // no boundscheck for x.ptr[_ .. _] } else assert(0); - this->lowerIsLessThanUpper = (lwrRange.imax <= uprRange.imin); + this->lowerIsLessThanUpper = this->lowerIsLessThanUpper || (lwrRange.imax <= uprRange.imin); //printf("upperIsInBounds = %d lowerIsLessThanUpper = %d\n", upperIsInBounds, lowerIsLessThanUpper); } From ad6bebb764fa71bfdb4f7d7f8041195e4462d357 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Wed, 28 Jan 2015 01:43:59 +0100 Subject: [PATCH 10/35] Put lower before upper and remove intermediate --- src/expression.c | 64 +++++++++++++++++++++++------------------------- 1 file changed, 30 insertions(+), 34 deletions(-) diff --git a/src/expression.c b/src/expression.c index 7605c03f0bd1..a7ac1d39a916 100644 --- a/src/expression.c +++ b/src/expression.c @@ -10054,6 +10054,33 @@ Expression *SliceExp::semantic(Scope *sc) // avoid slice bounds-checking. TODO use resolveOpDollar? { + // lower + bool lwrAtStart = false, lwrIn = false, lwrAtEnd = false; + long lwrDiv = 0; // non-zero means [ $/lwrDiv .. _ ] + if (lwr->op == TOKint64) + { + IntegerExp* int_ = (IntegerExp*)lwr; // TODO is*Exp predicate? + lwrAtStart = int_->value == 0; + if (lwrAtStart) { e1->warning("Avoiding boundscheck for lower part '%s'", int_->toChars()); } + } + else if (lwr->op == TOKvar) + { + VarExp* var = (VarExp*)lwr; // TODO is*Exp predicate? + lwrAtEnd = isOpDollar(var); + if (lwrAtEnd) { e1->warning("Avoiding boundscheck for lower part '%s'", var->toChars()); } + } + else if (lwr->op == TOKdiv) + { + DivExp* div = (DivExp*)lwr; // TODO is*Exp predicate? + if (div->e1->op == TOKvar && isOpDollar((VarExp*)div->e1) && + div->e2->op == TOKint64) + { + lwrDiv = ((IntegerExp*)div->e2)->value; + lwrIn = lwrDiv >= 1; + if (lwrIn) { e1->warning("Avoiding boundscheck for lower part '%s'", div->toChars()); } + } + } + // upper bool uprAtStart = false, uprIn = false, uprAtEnd = false; long uprDiv = 0; // non-zero means [ _ .. $/uprDiv ] @@ -10073,50 +10100,19 @@ Expression *SliceExp::semantic(Scope *sc) else if (upr->op == TOKdiv) { DivExp* div = (DivExp*)upr; // TODO is*Exp predicate? - if (div->e1->op == TOKvar && + if (div->e1->op == TOKvar && isOpDollar((VarExp*)div->e1) && div->e2->op == TOKint64) { - VarExp* p = (VarExp*)div->e1; // TODO is*Exp predicate? - IntegerExp* q = (IntegerExp*)div->e2; // TODO is*Exp predicate? - uprDiv = q->value; + uprDiv = ((IntegerExp*)div->e2)->value; uprIn = uprDiv >= 1; if (uprIn) { e1->warning("Avoiding boundscheck for upper part '%s'", div->toChars()); } this->upperIsInBounds = uprIn; } } - // lower - bool lwrAtStart = false, lwrIn = false, lwrAtEnd = false; - long lwrDiv = 0; // non-zero means [ $/lwrDiv .. _ ] - if (lwr->op == TOKint64) - { - IntegerExp* int_ = (IntegerExp*)lwr; // TODO is*Exp predicate? - lwrAtStart = int_->value == 0; - if (lwrAtStart) { e1->warning("Avoiding boundscheck for lower part '%s'", int_->toChars()); } - } - else if (lwr->op == TOKvar) - { - VarExp* var = (VarExp*)lwr; // TODO is*Exp predicate? - lwrAtEnd = isOpDollar(var); - if (lwrAtEnd) { e1->warning("Avoiding boundscheck for lower part '%s'", var->toChars()); } - } - else if (lwr->op == TOKdiv) - { - DivExp* div = (DivExp*)lwr; // TODO is*Exp predicate? - if (div->e1->op == TOKvar && - div->e2->op == TOKint64) - { - VarExp* p = (VarExp*)div->e1; // TODO is*Exp predicate? - IntegerExp* q = (IntegerExp*)div->e2; // TODO is*Exp predicate? - lwrDiv = q->value; - lwrIn = lwrDiv >= 1; - if (lwrIn) { e1->warning("Avoiding boundscheck for lower part '%s'", div->toChars()); } - } - } - // lower and upper if (lwrAtStart || // [0 .. X] - lwrAtEnd && uprAtEnd || // [$ .. $] + (lwrAtEnd && uprAtEnd) || // [$ .. $] (lwrDiv != 0 && uprDiv != 0 && lwrDiv >= uprDiv)) // [$/m .. $/n], m >= n { lowerIsLessThanUpper = true; From a56d4bf504c05afe8ae673c1d8ee4692c908c7f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Wed, 28 Jan 2015 02:05:26 +0100 Subject: [PATCH 11/35] Simplify and use toInteger --- src/expression.c | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/expression.c b/src/expression.c index a7ac1d39a916..977826ff2879 100644 --- a/src/expression.c +++ b/src/expression.c @@ -10056,18 +10056,17 @@ Expression *SliceExp::semantic(Scope *sc) { // lower bool lwrAtStart = false, lwrIn = false, lwrAtEnd = false; - long lwrDiv = 0; // non-zero means [ $/lwrDiv .. _ ] + dinteger_t lwrDiv = 0; // non-zero means [ $/lwrDiv .. _ ] if (lwr->op == TOKint64) { - IntegerExp* int_ = (IntegerExp*)lwr; // TODO is*Exp predicate? - lwrAtStart = int_->value == 0; - if (lwrAtStart) { e1->warning("Avoiding boundscheck for lower part '%s'", int_->toChars()); } + lwrAtStart = lwr->toInteger() == 0; + if (lwrAtStart) { lwr->warning("Avoiding boundscheck"); } } else if (lwr->op == TOKvar) { VarExp* var = (VarExp*)lwr; // TODO is*Exp predicate? lwrAtEnd = isOpDollar(var); - if (lwrAtEnd) { e1->warning("Avoiding boundscheck for lower part '%s'", var->toChars()); } + if (lwrAtEnd) { lwr->warning("Avoiding boundscheck"); } } else if (lwr->op == TOKdiv) { @@ -10075,26 +10074,25 @@ Expression *SliceExp::semantic(Scope *sc) if (div->e1->op == TOKvar && isOpDollar((VarExp*)div->e1) && div->e2->op == TOKint64) { - lwrDiv = ((IntegerExp*)div->e2)->value; + lwrDiv = div->e2->toInteger(); lwrIn = lwrDiv >= 1; - if (lwrIn) { e1->warning("Avoiding boundscheck for lower part '%s'", div->toChars()); } + if (lwrIn) { lwr->warning("Avoiding boundscheck"); } } } // upper bool uprAtStart = false, uprIn = false, uprAtEnd = false; - long uprDiv = 0; // non-zero means [ _ .. $/uprDiv ] + dinteger_t uprDiv = 0; // non-zero means [ _ .. $/uprDiv ] if (upr->op == TOKint64) { - IntegerExp* int_ = (IntegerExp*)upr; // TODO is*Exp predicate? - uprAtStart = int_->value == 0; - if (uprAtStart) { e1->warning("Avoiding boundscheck for upper part '%s'", int_->toChars()); } + uprAtStart = upr->toInteger() == 0; + if (uprAtStart) { upr->warning("Avoiding boundscheck"); } } else if (upr->op == TOKvar) { VarExp* var = (VarExp*)upr; // TODO is*Exp predicate? uprAtEnd = isOpDollar(var); - if (uprAtEnd) { e1->warning("Avoiding boundscheck for upper part '%s'", var->toChars()); } + if (uprAtEnd) { upr->warning("Avoiding boundscheck"); } this->upperIsInBounds = uprAtEnd; } else if (upr->op == TOKdiv) @@ -10103,9 +10101,9 @@ Expression *SliceExp::semantic(Scope *sc) if (div->e1->op == TOKvar && isOpDollar((VarExp*)div->e1) && div->e2->op == TOKint64) { - uprDiv = ((IntegerExp*)div->e2)->value; + uprDiv = div->e2->toInteger(); uprIn = uprDiv >= 1; - if (uprIn) { e1->warning("Avoiding boundscheck for upper part '%s'", div->toChars()); } + if (uprIn) { upr->warning("Avoiding boundscheck"); } this->upperIsInBounds = uprIn; } } From 6079a6d4610f716918cadf51c9c8cd112566cbe2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Wed, 28 Jan 2015 16:22:13 +0100 Subject: [PATCH 12/35] Implement Step 4 --- src/expression.c | 92 +++++++++++++++++++++++++++++++++++++----------- 1 file changed, 72 insertions(+), 20 deletions(-) diff --git a/src/expression.c b/src/expression.c index 977826ff2879..704a98deb8d6 100644 --- a/src/expression.c +++ b/src/expression.c @@ -9799,6 +9799,7 @@ SliceExp::SliceExp(Loc loc, Expression *e1, Expression *lwr, Expression *upr) this->upr = upr; this->lwr = lwr; lengthVar = NULL; + lowerIsInBounds = false; upperIsInBounds = false; lowerIsLessThanUpper = false; } @@ -9812,10 +9813,11 @@ Expression *SliceExp::syntaxCopy() return se; } -static bool isOpDollar(VarExp* ve) +bool SliceExp::isOpDollar(VarExp* ve) { - VarDeclaration* decl = ve->var->isVarDeclaration(); - return decl && decl->ident == Id::dollar; + return ve->var == this->lengthVar; + // VarDeclaration* decl = ve->var->isVarDeclaration(); + // return decl && decl->ident == Id::dollar; } Expression *SliceExp::semantic(Scope *sc) @@ -10055,57 +10057,107 @@ Expression *SliceExp::semantic(Scope *sc) // avoid slice bounds-checking. TODO use resolveOpDollar? { // lower - bool lwrAtStart = false, lwrIn = false, lwrAtEnd = false; + bool lwrAtStart = false; + bool lwrIn = false; + bool lwrAtEnd = false; + dinteger_t lwrMul = 0; // non-zero means [ $*/wrMul .. _ ] dinteger_t lwrDiv = 0; // non-zero means [ $/lwrDiv .. _ ] if (lwr->op == TOKint64) { lwrAtStart = lwr->toInteger() == 0; - if (lwrAtStart) { lwr->warning("Avoiding boundscheck"); } + if (lwrAtStart) { lwr->warning("Avoiding lower boundscheck"); } + this->lowerIsInBounds = lwrAtStart; } else if (lwr->op == TOKvar) { - VarExp* var = (VarExp*)lwr; // TODO is*Exp predicate? - lwrAtEnd = isOpDollar(var); - if (lwrAtEnd) { lwr->warning("Avoiding boundscheck"); } + VarExp* var = (VarExp*)lwr; + lwrAtEnd = this->isOpDollar(var); + if (lwrAtEnd) { lwr->warning("Avoiding lower boundscheck"); } + this->lowerIsInBounds = lwrAtEnd; } else if (lwr->op == TOKdiv) { - DivExp* div = (DivExp*)lwr; // TODO is*Exp predicate? - if (div->e1->op == TOKvar && isOpDollar((VarExp*)div->e1) && + DivExp* div = (DivExp*)lwr; + if (div->e1->op == TOKvar && this->isOpDollar((VarExp*)div->e1) && // TODO functionize? div->e2->op == TOKint64) { lwrDiv = div->e2->toInteger(); lwrIn = lwrDiv >= 1; - if (lwrIn) { lwr->warning("Avoiding boundscheck"); } + if (lwrIn) { lwr->warning("Avoiding lower boundscheck for $/%ld", lwrDiv); } + this->lowerIsInBounds = lwrIn; + } + else if (div->e1->op == TOKmul && + div->e2->op == TOKint64) + { + MulExp* mul = (MulExp*)div->e1; + const dinteger_t denom = div->e2->toInteger(); + if (mul->e1->op == TOKvar && this->isOpDollar((VarExp*)mul->e1) && // TODO functionize? + mul->e2->op == TOKint64) + { + const dinteger_t numer = mul->e2->toInteger(); + this->lowerIsInBounds = numer <= denom; + if (this->lowerIsInBounds) { lwr->warning("Avoiding lower boundscheck $*%ld/%ld", numer, denom); } + } + else if (mul->e1->op == TOKint64 && + mul->e2->op == TOKvar && this->isOpDollar((VarExp*)mul->e2)) // TODO functionize? + { + const dinteger_t numer = mul->e1->toInteger(); + this->lowerIsInBounds = numer <= denom; + if (this->lowerIsInBounds) { lwr->warning("Avoiding lower boundscheck $*%ld/%ld", numer, denom); } + } } } // upper - bool uprAtStart = false, uprIn = false, uprAtEnd = false; + bool uprAtStart = false; + bool uprIn = false; + bool uprAtEnd = false; + dinteger_t uprMul = 0; // non-zero means [ _ .. $*uprMul ] dinteger_t uprDiv = 0; // non-zero means [ _ .. $/uprDiv ] if (upr->op == TOKint64) { uprAtStart = upr->toInteger() == 0; - if (uprAtStart) { upr->warning("Avoiding boundscheck"); } + if (uprAtStart) { upr->warning("Avoiding upper boundscheck"); } + this->upperIsInBounds = uprAtStart; } else if (upr->op == TOKvar) { - VarExp* var = (VarExp*)upr; // TODO is*Exp predicate? - uprAtEnd = isOpDollar(var); - if (uprAtEnd) { upr->warning("Avoiding boundscheck"); } + VarExp* var = (VarExp*)upr; + uprAtEnd = this->isOpDollar(var); // TODO functionize? + if (uprAtEnd) { upr->warning("Avoiding upper boundscheck"); } this->upperIsInBounds = uprAtEnd; } else if (upr->op == TOKdiv) { - DivExp* div = (DivExp*)upr; // TODO is*Exp predicate? - if (div->e1->op == TOKvar && isOpDollar((VarExp*)div->e1) && + DivExp* div = (DivExp*)upr; + if (div->e1->op == TOKvar && this->isOpDollar((VarExp*)div->e1) && // TODO functionize? div->e2->op == TOKint64) { uprDiv = div->e2->toInteger(); uprIn = uprDiv >= 1; - if (uprIn) { upr->warning("Avoiding boundscheck"); } + if (uprIn) { upr->warning("Avoiding upper boundscheck for $/%ld", uprDiv); } this->upperIsInBounds = uprIn; } + else if (div->e1->op == TOKmul && + div->e2->op == TOKint64) + { + MulExp* mul = (MulExp*)div->e1; + const dinteger_t denom = div->e2->toInteger(); + if (mul->e1->op == TOKvar && this->isOpDollar((VarExp*)mul->e1) && // TODO functionize? + mul->e2->op == TOKint64) + { + const dinteger_t numer = mul->e2->toInteger(); + this->upperIsInBounds = numer <= denom; + if (this->upperIsInBounds) { upr->warning("Avoiding upper boundscheck for $*%ld/%ld", numer, denom); } + } + else if (mul->e1->op == TOKint64 && + mul->e2->op == TOKvar && this->isOpDollar((VarExp*)mul->e2)) // TODO functionize? + { + const dinteger_t numer = mul->e1->toInteger(); + this->upperIsInBounds = numer <= denom; + if (this->upperIsInBounds) { upr->warning("Avoiding upper boundscheck for $*%ld/%ld", numer, denom); } + } + } } // lower and upper @@ -10139,7 +10191,7 @@ Expression *SliceExp::semantic(Scope *sc) else assert(0); - this->lowerIsLessThanUpper = this->lowerIsLessThanUpper || (lwrRange.imax <= uprRange.imin); + this->lowerIsLessThanUpper |= (lwrRange.imax <= uprRange.imin); //printf("upperIsInBounds = %d lowerIsLessThanUpper = %d\n", upperIsInBounds, lowerIsLessThanUpper); } From e23906ce84daa5dd0a3bb2e741e34cef66c118e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Wed, 28 Jan 2015 16:28:32 +0100 Subject: [PATCH 13/35] Simplify --- src/expression.c | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/src/expression.c b/src/expression.c index 704a98deb8d6..137a25ea2c61 100644 --- a/src/expression.c +++ b/src/expression.c @@ -10060,8 +10060,8 @@ Expression *SliceExp::semantic(Scope *sc) bool lwrAtStart = false; bool lwrIn = false; bool lwrAtEnd = false; - dinteger_t lwrMul = 0; // non-zero means [ $*/wrMul .. _ ] - dinteger_t lwrDiv = 0; // non-zero means [ $/lwrDiv .. _ ] + dinteger_t lwrMul = 1; // non-one means [ $*lwrMul .. _ ] + dinteger_t lwrDiv = 1; // non-one means [ $/lwrDiv .. _ ] if (lwr->op == TOKint64) { lwrAtStart = lwr->toInteger() == 0; @@ -10090,20 +10090,20 @@ Expression *SliceExp::semantic(Scope *sc) div->e2->op == TOKint64) { MulExp* mul = (MulExp*)div->e1; - const dinteger_t denom = div->e2->toInteger(); + lwrDiv = div->e2->toInteger(); if (mul->e1->op == TOKvar && this->isOpDollar((VarExp*)mul->e1) && // TODO functionize? mul->e2->op == TOKint64) { - const dinteger_t numer = mul->e2->toInteger(); - this->lowerIsInBounds = numer <= denom; - if (this->lowerIsInBounds) { lwr->warning("Avoiding lower boundscheck $*%ld/%ld", numer, denom); } + lwrMul = mul->e2->toInteger(); + this->lowerIsInBounds = lwrMul <= lwrDiv; + if (this->lowerIsInBounds) { lwr->warning("Avoiding lower boundscheck $*%ld/%ld", lwrMul, lwrDiv); } } else if (mul->e1->op == TOKint64 && mul->e2->op == TOKvar && this->isOpDollar((VarExp*)mul->e2)) // TODO functionize? { - const dinteger_t numer = mul->e1->toInteger(); - this->lowerIsInBounds = numer <= denom; - if (this->lowerIsInBounds) { lwr->warning("Avoiding lower boundscheck $*%ld/%ld", numer, denom); } + lwrMul = mul->e1->toInteger(); + this->lowerIsInBounds = lwrMul <= lwrDiv; + if (this->lowerIsInBounds) { lwr->warning("Avoiding lower boundscheck $*%ld/%ld", lwrMul, lwrDiv); } } } } @@ -10112,8 +10112,8 @@ Expression *SliceExp::semantic(Scope *sc) bool uprAtStart = false; bool uprIn = false; bool uprAtEnd = false; - dinteger_t uprMul = 0; // non-zero means [ _ .. $*uprMul ] - dinteger_t uprDiv = 0; // non-zero means [ _ .. $/uprDiv ] + dinteger_t uprMul = 1; // non-one means [ _ .. $*uprMul ] + dinteger_t uprDiv = 1; // non-one means [ _ .. $/uprDiv ] if (upr->op == TOKint64) { uprAtStart = upr->toInteger() == 0; @@ -10142,20 +10142,20 @@ Expression *SliceExp::semantic(Scope *sc) div->e2->op == TOKint64) { MulExp* mul = (MulExp*)div->e1; - const dinteger_t denom = div->e2->toInteger(); + uprDiv = div->e2->toInteger(); if (mul->e1->op == TOKvar && this->isOpDollar((VarExp*)mul->e1) && // TODO functionize? mul->e2->op == TOKint64) { - const dinteger_t numer = mul->e2->toInteger(); - this->upperIsInBounds = numer <= denom; - if (this->upperIsInBounds) { upr->warning("Avoiding upper boundscheck for $*%ld/%ld", numer, denom); } + uprMul = mul->e2->toInteger(); + this->upperIsInBounds = uprMul <= uprDiv; + if (this->upperIsInBounds) { upr->warning("Avoiding upper boundscheck for $*%ld/%ld", uprMul, uprDiv); } } else if (mul->e1->op == TOKint64 && mul->e2->op == TOKvar && this->isOpDollar((VarExp*)mul->e2)) // TODO functionize? { - const dinteger_t numer = mul->e1->toInteger(); - this->upperIsInBounds = numer <= denom; - if (this->upperIsInBounds) { upr->warning("Avoiding upper boundscheck for $*%ld/%ld", numer, denom); } + uprMul = mul->e1->toInteger(); + this->upperIsInBounds = uprMul <= uprDiv; + if (this->upperIsInBounds) { upr->warning("Avoiding upper boundscheck for $*%ld/%ld", uprMul, uprDiv); } } } } @@ -10163,7 +10163,7 @@ Expression *SliceExp::semantic(Scope *sc) // lower and upper if (lwrAtStart || // [0 .. X] (lwrAtEnd && uprAtEnd) || // [$ .. $] - (lwrDiv != 0 && uprDiv != 0 && lwrDiv >= uprDiv)) // [$/m .. $/n], m >= n + (lwrDiv >= uprDiv)) // [$/m .. $/n], m >= n { lowerIsLessThanUpper = true; } From 4c0afec81b0362b386ea21cbcfff38607df5a765 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Wed, 28 Jan 2015 16:35:24 +0100 Subject: [PATCH 14/35] Working comparison of rational bounds --- src/expression.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/expression.c b/src/expression.c index 137a25ea2c61..6d1c16d34e45 100644 --- a/src/expression.c +++ b/src/expression.c @@ -10163,9 +10163,10 @@ Expression *SliceExp::semantic(Scope *sc) // lower and upper if (lwrAtStart || // [0 .. X] (lwrAtEnd && uprAtEnd) || // [$ .. $] - (lwrDiv >= uprDiv)) // [$/m .. $/n], m >= n + (lwrMul*uprDiv <= uprMul*lwrDiv)) // [$*p/q .. $*r/s], p/q <= r/s => p*s <= r*q { lowerIsLessThanUpper = true; + this->warning("Lower <= upper bound"); } } From 0cb9ae79c3e5f4717a04d1915f4a3a4134f9ac5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Wed, 28 Jan 2015 16:38:13 +0100 Subject: [PATCH 15/35] Be more restrictive --- src/expression.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/expression.c b/src/expression.c index 6d1c16d34e45..9ec47de2c5b7 100644 --- a/src/expression.c +++ b/src/expression.c @@ -10161,9 +10161,11 @@ Expression *SliceExp::semantic(Scope *sc) } // lower and upper - if (lwrAtStart || // [0 .. X] - (lwrAtEnd && uprAtEnd) || // [$ .. $] - (lwrMul*uprDiv <= uprMul*lwrDiv)) // [$*p/q .. $*r/s], p/q <= r/s => p*s <= r*q + if ((this->lowerIsInBounds && + this->upperIsInBounds) && + (lwrAtStart || // [0 .. _] + (lwrAtEnd && uprAtEnd) || // [$ .. $] + (lwrMul*uprDiv <= uprMul*lwrDiv))) // [$*p/q .. $*r/s], p/q <= r/s => p*s <= r*q { lowerIsLessThanUpper = true; this->warning("Lower <= upper bound"); From 10749852fd2de5d8c59a2f8564ae90f95fcd74ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Wed, 28 Jan 2015 16:44:00 +0100 Subject: [PATCH 16/35] Simplify --- src/expression.c | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/src/expression.c b/src/expression.c index 9ec47de2c5b7..4136f7680b1b 100644 --- a/src/expression.c +++ b/src/expression.c @@ -10062,18 +10062,15 @@ Expression *SliceExp::semantic(Scope *sc) bool lwrAtEnd = false; dinteger_t lwrMul = 1; // non-one means [ $*lwrMul .. _ ] dinteger_t lwrDiv = 1; // non-one means [ $/lwrDiv .. _ ] - if (lwr->op == TOKint64) + if (lwr->op == TOKint64 && lwr->toInteger() == 0) { - lwrAtStart = lwr->toInteger() == 0; - if (lwrAtStart) { lwr->warning("Avoiding lower boundscheck"); } - this->lowerIsInBounds = lwrAtStart; + this->lowerIsInBounds = lwrAtStart = true; + lwr->warning("Avoiding lower boundscheck"); } - else if (lwr->op == TOKvar) + else if (lwr->op == TOKvar && this->isOpDollar((VarExp*)lwr)) // TODO functionize? { - VarExp* var = (VarExp*)lwr; - lwrAtEnd = this->isOpDollar(var); - if (lwrAtEnd) { lwr->warning("Avoiding lower boundscheck"); } - this->lowerIsInBounds = lwrAtEnd; + this->lowerIsInBounds = lwrAtEnd = true; + lwr->warning("Avoiding lower boundscheck"); } else if (lwr->op == TOKdiv) { @@ -10114,18 +10111,15 @@ Expression *SliceExp::semantic(Scope *sc) bool uprAtEnd = false; dinteger_t uprMul = 1; // non-one means [ _ .. $*uprMul ] dinteger_t uprDiv = 1; // non-one means [ _ .. $/uprDiv ] - if (upr->op == TOKint64) + if (upr->op == TOKint64 && upr->toInteger() == 0) { - uprAtStart = upr->toInteger() == 0; - if (uprAtStart) { upr->warning("Avoiding upper boundscheck"); } - this->upperIsInBounds = uprAtStart; + this->upperIsInBounds = uprAtStart = true; + upr->warning("Avoiding upper boundscheck"); } - else if (upr->op == TOKvar) + else if (upr->op == TOKvar && this->isOpDollar((VarExp*)upr)) // TODO functionize? { - VarExp* var = (VarExp*)upr; - uprAtEnd = this->isOpDollar(var); // TODO functionize? - if (uprAtEnd) { upr->warning("Avoiding upper boundscheck"); } - this->upperIsInBounds = uprAtEnd; + this->upperIsInBounds = uprAtEnd = true; + upr->warning("Avoiding upper boundscheck"); } else if (upr->op == TOKdiv) { From 3205b61e0b04c7794e6a4438957e50221f3363c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Wed, 28 Jan 2015 23:33:27 +0100 Subject: [PATCH 17/35] Turn lowerIsInBounds to a variable --- src/expression.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/expression.h b/src/expression.h index 9c241ee179a2..e06fb468d1e9 100644 --- a/src/expression.h +++ b/src/expression.h @@ -1007,10 +1007,9 @@ class SliceExp : public UnaExp Expression *upr; // NULL if implicit 0 Expression *lwr; // NULL if implicit [length - 1] VarDeclaration *lengthVar; + bool lowerIsInBounds; // true if lwr <= e1.length bool upperIsInBounds; // true if upr <= e1.length bool lowerIsLessThanUpper; // true if lwr <= upr - bool lowerIsInBounds() const { return (lowerIsLessThanUpper && - upperIsInBounds); }; // true if lwr <= e1.length SliceExp(Loc loc, Expression *e1, Expression *lwr, Expression *upr); Expression *syntaxCopy(); @@ -1020,6 +1019,7 @@ class SliceExp : public UnaExp Expression *toLvalue(Scope *sc, Expression *e); Expression *modifiableLvalue(Scope *sc, Expression *e); int isBool(int result); + bool isOpDollar(VarExp* ve); void accept(Visitor *v) { v->visit(this); } }; From 2599088e628e93ea76d8ab58339cfdd0004559f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Thu, 29 Jan 2015 01:09:50 +0100 Subject: [PATCH 18/35] Break out into SliceExp::Boundness and SliceExp::analyzeRelativeBound() for better reuse --- src/expression.c | 159 ++++++++++++++++++++--------------------------- src/expression.h | 11 ++++ 2 files changed, 79 insertions(+), 91 deletions(-) diff --git a/src/expression.c b/src/expression.c index 4136f7680b1b..34175a03af2a 100644 --- a/src/expression.c +++ b/src/expression.c @@ -9820,6 +9820,62 @@ bool SliceExp::isOpDollar(VarExp* ve) // return decl && decl->ident == Id::dollar; } +SliceExp::Boundness SliceExp::analyzeRelativeBound(Expression* e, + dinteger_t* p, dinteger_t* q) // out arguments +{ + if (e->op == TOKint64 && e->toInteger() == 0) + { + e->warning("Avoiding boundscheck for 0"); + return atStart; + } + else if (e->op == TOKvar && this->isOpDollar((VarExp*)e)) // TODO functionize? + { + e->warning("Avoiding boundscheck for $"); + return atEnd; + } + else if (e->op == TOKdiv) + { + DivExp* div = (DivExp*)e; + if (div->e1->op == TOKvar && this->isOpDollar((VarExp*)div->e1) && // TODO functionize? + div->e2->op == TOKint64) + { + *q = div->e2->toInteger(); + if (*q >= 1) + { + e->warning("Avoiding boundscheck for $/%ld", *q); + return inside; + } + } + else if (div->e1->op == TOKmul && + div->e2->op == TOKint64) + { + MulExp* mul = (MulExp*)div->e1; + *q = div->e2->toInteger(); + if (mul->e1->op == TOKvar && this->isOpDollar((VarExp*)mul->e1) && // TODO functionize? + mul->e2->op == TOKint64) + { + *p = mul->e2->toInteger(); + if (*p <= *q) + { + e->warning("Avoiding boundscheck for $*%ld/%ld", *p, *q); + return inside; + } + } + else if (mul->e1->op == TOKint64 && + mul->e2->op == TOKvar && this->isOpDollar((VarExp*)mul->e2)) // TODO functionize? + { + *p = mul->e1->toInteger(); + if (*p <= *q) + { + e->warning("Avoiding boundscheck for $*%ld/%ld", *p, *q); + return inside; + } + } + } + } + return unknown; +} + Expression *SliceExp::semantic(Scope *sc) { #if LOGSEMANTIC @@ -10057,109 +10113,30 @@ Expression *SliceExp::semantic(Scope *sc) // avoid slice bounds-checking. TODO use resolveOpDollar? { // lower - bool lwrAtStart = false; - bool lwrIn = false; - bool lwrAtEnd = false; dinteger_t lwrMul = 1; // non-one means [ $*lwrMul .. _ ] dinteger_t lwrDiv = 1; // non-one means [ $/lwrDiv .. _ ] - if (lwr->op == TOKint64 && lwr->toInteger() == 0) + const Boundness lwrBoundness = this->analyzeRelativeBound(lwr, &lwrMul, &lwrDiv); + if (lwrBoundness >= inside) { - this->lowerIsInBounds = lwrAtStart = true; - lwr->warning("Avoiding lower boundscheck"); - } - else if (lwr->op == TOKvar && this->isOpDollar((VarExp*)lwr)) // TODO functionize? - { - this->lowerIsInBounds = lwrAtEnd = true; - lwr->warning("Avoiding lower boundscheck"); - } - else if (lwr->op == TOKdiv) - { - DivExp* div = (DivExp*)lwr; - if (div->e1->op == TOKvar && this->isOpDollar((VarExp*)div->e1) && // TODO functionize? - div->e2->op == TOKint64) - { - lwrDiv = div->e2->toInteger(); - lwrIn = lwrDiv >= 1; - if (lwrIn) { lwr->warning("Avoiding lower boundscheck for $/%ld", lwrDiv); } - this->lowerIsInBounds = lwrIn; - } - else if (div->e1->op == TOKmul && - div->e2->op == TOKint64) - { - MulExp* mul = (MulExp*)div->e1; - lwrDiv = div->e2->toInteger(); - if (mul->e1->op == TOKvar && this->isOpDollar((VarExp*)mul->e1) && // TODO functionize? - mul->e2->op == TOKint64) - { - lwrMul = mul->e2->toInteger(); - this->lowerIsInBounds = lwrMul <= lwrDiv; - if (this->lowerIsInBounds) { lwr->warning("Avoiding lower boundscheck $*%ld/%ld", lwrMul, lwrDiv); } - } - else if (mul->e1->op == TOKint64 && - mul->e2->op == TOKvar && this->isOpDollar((VarExp*)mul->e2)) // TODO functionize? - { - lwrMul = mul->e1->toInteger(); - this->lowerIsInBounds = lwrMul <= lwrDiv; - if (this->lowerIsInBounds) { lwr->warning("Avoiding lower boundscheck $*%ld/%ld", lwrMul, lwrDiv); } - } - } + this->lowerIsInBounds = true; } // upper - bool uprAtStart = false; - bool uprIn = false; - bool uprAtEnd = false; - dinteger_t uprMul = 1; // non-one means [ _ .. $*uprMul ] - dinteger_t uprDiv = 1; // non-one means [ _ .. $/uprDiv ] - if (upr->op == TOKint64 && upr->toInteger() == 0) + dinteger_t uprMul = 1; // non-one means [ $*uprMul .. _ ] + dinteger_t uprDiv = 1; // non-one means [ $/uprDiv .. _ ] + const Boundness uprBoundness = this->analyzeRelativeBound(upr, &uprMul, &uprDiv); + if (uprBoundness >= inside) { - this->upperIsInBounds = uprAtStart = true; - upr->warning("Avoiding upper boundscheck"); - } - else if (upr->op == TOKvar && this->isOpDollar((VarExp*)upr)) // TODO functionize? - { - this->upperIsInBounds = uprAtEnd = true; - upr->warning("Avoiding upper boundscheck"); - } - else if (upr->op == TOKdiv) - { - DivExp* div = (DivExp*)upr; - if (div->e1->op == TOKvar && this->isOpDollar((VarExp*)div->e1) && // TODO functionize? - div->e2->op == TOKint64) - { - uprDiv = div->e2->toInteger(); - uprIn = uprDiv >= 1; - if (uprIn) { upr->warning("Avoiding upper boundscheck for $/%ld", uprDiv); } - this->upperIsInBounds = uprIn; - } - else if (div->e1->op == TOKmul && - div->e2->op == TOKint64) - { - MulExp* mul = (MulExp*)div->e1; - uprDiv = div->e2->toInteger(); - if (mul->e1->op == TOKvar && this->isOpDollar((VarExp*)mul->e1) && // TODO functionize? - mul->e2->op == TOKint64) - { - uprMul = mul->e2->toInteger(); - this->upperIsInBounds = uprMul <= uprDiv; - if (this->upperIsInBounds) { upr->warning("Avoiding upper boundscheck for $*%ld/%ld", uprMul, uprDiv); } - } - else if (mul->e1->op == TOKint64 && - mul->e2->op == TOKvar && this->isOpDollar((VarExp*)mul->e2)) // TODO functionize? - { - uprMul = mul->e1->toInteger(); - this->upperIsInBounds = uprMul <= uprDiv; - if (this->upperIsInBounds) { upr->warning("Avoiding upper boundscheck for $*%ld/%ld", uprMul, uprDiv); } - } - } + this->upperIsInBounds = true; } // lower and upper if ((this->lowerIsInBounds && this->upperIsInBounds) && - (lwrAtStart || // [0 .. _] - (lwrAtEnd && uprAtEnd) || // [$ .. $] - (lwrMul*uprDiv <= uprMul*lwrDiv))) // [$*p/q .. $*r/s], p/q <= r/s => p*s <= r*q + (lwrBoundness == atStart || // [0 .. _] + (lwrBoundness == atEnd && + uprBoundness == atEnd) || // [$ .. $] + (lwrMul*uprDiv <= uprMul*lwrDiv))) // [$*p/q .. $*r/s], p/q <= r/s => p*s <= r*q. TODO mul overflows? { lowerIsLessThanUpper = true; this->warning("Lower <= upper bound"); diff --git a/src/expression.h b/src/expression.h index e06fb468d1e9..c14eab2e6c0d 100644 --- a/src/expression.h +++ b/src/expression.h @@ -1011,6 +1011,17 @@ class SliceExp : public UnaExp bool upperIsInBounds; // true if upr <= e1.length bool lowerIsLessThanUpper; // true if lwr <= upr + enum Boundness + { + unknown, + outside, // outside of [0 .. $] + inside, // within [0 .. $] + atStart, // specialization of inside + atEnd, // specialization of inside + }; + + Boundness analyzeRelativeBound(Expression* e, dinteger_t* mul, dinteger_t* div); + SliceExp(Loc loc, Expression *e1, Expression *lwr, Expression *upr); Expression *syntaxCopy(); Expression *semantic(Scope *sc); From 3079e386bc65003f1fe73aa3216a308a87ece5ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Thu, 29 Jan 2015 01:20:38 +0100 Subject: [PATCH 19/35] Comment Boundness --- src/expression.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/expression.h b/src/expression.h index c14eab2e6c0d..d43dc0790eb0 100644 --- a/src/expression.h +++ b/src/expression.h @@ -1011,11 +1011,12 @@ class SliceExp : public UnaExp bool upperIsInBounds; // true if upr <= e1.length bool lowerIsLessThanUpper; // true if lwr <= upr + // Decribes Boundness of a Slice Beginning or End Index. enum Boundness { - unknown, + unknown, // unknown whether inside or outside outside, // outside of [0 .. $] - inside, // within [0 .. $] + inside, // inside of [0 .. $] atStart, // specialization of inside atEnd, // specialization of inside }; From 63d2ac5d6ad7a7eb5720c4eafe89e9d2b036b100 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Thu, 29 Jan 2015 18:03:35 +0100 Subject: [PATCH 20/35] Various feedback fixes --- src/expression.c | 70 +++++++++++++++++++++++++++++++++++------------- 1 file changed, 51 insertions(+), 19 deletions(-) diff --git a/src/expression.c b/src/expression.c index 34175a03af2a..61c1cac4447f 100644 --- a/src/expression.c +++ b/src/expression.c @@ -9813,22 +9813,31 @@ Expression *SliceExp::syntaxCopy() return se; } -bool SliceExp::isOpDollar(VarExp* ve) +bool isOpDollar(Expression* e, VarDeclaration *lengthVar) { - return ve->var == this->lengthVar; - // VarDeclaration* decl = ve->var->isVarDeclaration(); - // return decl && decl->ident == Id::dollar; + return e->op == TOKvar && ((VarExp*)e)->var == lengthVar; } -SliceExp::Boundness SliceExp::analyzeRelativeBound(Expression* e, - dinteger_t* p, dinteger_t* q) // out arguments +// Decribes Boundness of a Slice Beginning or End Index. +enum Boundness +{ + unknown, // unknown whether inside or outside + outside, // outside of [0 .. $] + inside, // inside of [0 .. $] + atStart, // specialization of inside + atEnd, // specialization of inside +}; + +Boundness analyzeRelativeBound(Expression* e, + VarDeclaration *lengthVar, + dinteger_t* p, dinteger_t* q) // out arguments { if (e->op == TOKint64 && e->toInteger() == 0) { e->warning("Avoiding boundscheck for 0"); return atStart; } - else if (e->op == TOKvar && this->isOpDollar((VarExp*)e)) // TODO functionize? + else if (isOpDollar(e, lengthVar)) { e->warning("Avoiding boundscheck for $"); return atEnd; @@ -9836,8 +9845,7 @@ SliceExp::Boundness SliceExp::analyzeRelativeBound(Expression* e, else if (e->op == TOKdiv) { DivExp* div = (DivExp*)e; - if (div->e1->op == TOKvar && this->isOpDollar((VarExp*)div->e1) && // TODO functionize? - div->e2->op == TOKint64) + if (isOpDollar(div->e1, lengthVar) && div->e2->op == TOKint64) { *q = div->e2->toInteger(); if (*q >= 1) @@ -9851,8 +9859,7 @@ SliceExp::Boundness SliceExp::analyzeRelativeBound(Expression* e, { MulExp* mul = (MulExp*)div->e1; *q = div->e2->toInteger(); - if (mul->e1->op == TOKvar && this->isOpDollar((VarExp*)mul->e1) && // TODO functionize? - mul->e2->op == TOKint64) + if (isOpDollar(mul->e1, lengthVar) && mul->e2->op == TOKint64) { *p = mul->e2->toInteger(); if (*p <= *q) @@ -9860,9 +9867,13 @@ SliceExp::Boundness SliceExp::analyzeRelativeBound(Expression* e, e->warning("Avoiding boundscheck for $*%ld/%ld", *p, *q); return inside; } + else if (*p > *q) + { + return outside; + } } else if (mul->e1->op == TOKint64 && - mul->e2->op == TOKvar && this->isOpDollar((VarExp*)mul->e2)) // TODO functionize? + isOpDollar(mul->e2, lengthVar)) { *p = mul->e1->toInteger(); if (*p <= *q) @@ -9870,6 +9881,10 @@ SliceExp::Boundness SliceExp::analyzeRelativeBound(Expression* e, e->warning("Avoiding boundscheck for $*%ld/%ld", *p, *q); return inside; } + else if (*p > *q) + { + return outside; + } } } } @@ -10110,33 +10125,47 @@ Expression *SliceExp::semantic(Scope *sc) lwr = lwr->optimize(WANTvalue); upr = upr->optimize(WANTvalue); - // avoid slice bounds-checking. TODO use resolveOpDollar? + // avoid slice bounds-checking { // lower dinteger_t lwrMul = 1; // non-one means [ $*lwrMul .. _ ] dinteger_t lwrDiv = 1; // non-one means [ $/lwrDiv .. _ ] - const Boundness lwrBoundness = this->analyzeRelativeBound(lwr, &lwrMul, &lwrDiv); + const Boundness lwrBoundness = analyzeRelativeBound(lwr, this->lengthVar, &lwrMul, &lwrDiv); if (lwrBoundness >= inside) { this->lowerIsInBounds = true; } + else if (lwrBoundness == outside) + { + this->error("Lower slice limit $*%lld/%lld is out of bounds", + (unsigned long long)lwrMul, + (unsigned long long)lwrDiv); + } // upper dinteger_t uprMul = 1; // non-one means [ $*uprMul .. _ ] dinteger_t uprDiv = 1; // non-one means [ $/uprDiv .. _ ] - const Boundness uprBoundness = this->analyzeRelativeBound(upr, &uprMul, &uprDiv); + const Boundness uprBoundness = analyzeRelativeBound(upr, this->lengthVar, &uprMul, &uprDiv); if (uprBoundness >= inside) { this->upperIsInBounds = true; } + else if (uprBoundness == outside) + { + this->error("Lower slice limit $*%lld/%lld is out of bounds", + (unsigned long long)lwrMul, + (unsigned long long)lwrDiv); + } // lower and upper + // [$*p/q .. $*r/s], p/q <= r/s => p*s <= r*q. + // TODO do we need to handle mul overflows for (lwrMul*uprDiv <= uprMul*lwrDiv) if ((this->lowerIsInBounds && this->upperIsInBounds) && - (lwrBoundness == atStart || // [0 .. _] + (lwrBoundness == atStart || (lwrBoundness == atEnd && - uprBoundness == atEnd) || // [$ .. $] - (lwrMul*uprDiv <= uprMul*lwrDiv))) // [$*p/q .. $*r/s], p/q <= r/s => p*s <= r*q. TODO mul overflows? + uprBoundness == atEnd) || + (lwrMul*uprDiv <= uprMul*lwrDiv))) { lowerIsLessThanUpper = true; this->warning("Lower <= upper bound"); @@ -10165,7 +10194,10 @@ Expression *SliceExp::semantic(Scope *sc) else assert(0); - this->lowerIsLessThanUpper |= (lwrRange.imax <= uprRange.imin); + if (!this->lowerIsLessThanUpper) + { + this->lowerIsLessThanUpper = (lwrRange.imax <= uprRange.imin); + } //printf("upperIsInBounds = %d lowerIsLessThanUpper = %d\n", upperIsInBounds, lowerIsLessThanUpper); } From f713a38159c2ea5b9e888acd5a01ac7ba75e2ba5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Thu, 29 Jan 2015 18:08:03 +0100 Subject: [PATCH 21/35] Use printf instead of warning --- src/expression.c | 14 ++++++++------ src/expression.h | 13 ------------- 2 files changed, 8 insertions(+), 19 deletions(-) diff --git a/src/expression.c b/src/expression.c index 61c1cac4447f..6010afcb918a 100644 --- a/src/expression.c +++ b/src/expression.c @@ -9828,18 +9828,20 @@ enum Boundness atEnd, // specialization of inside }; +static bool LOG_BOUNDNESS = true; + Boundness analyzeRelativeBound(Expression* e, VarDeclaration *lengthVar, dinteger_t* p, dinteger_t* q) // out arguments { if (e->op == TOKint64 && e->toInteger() == 0) { - e->warning("Avoiding boundscheck for 0"); + if (LOG_BOUNDNESS) printf("Avoiding boundscheck for 0\n"); return atStart; } else if (isOpDollar(e, lengthVar)) { - e->warning("Avoiding boundscheck for $"); + if (LOG_BOUNDNESS) printf("Avoiding boundscheck for $\n"); return atEnd; } else if (e->op == TOKdiv) @@ -9850,7 +9852,7 @@ Boundness analyzeRelativeBound(Expression* e, *q = div->e2->toInteger(); if (*q >= 1) { - e->warning("Avoiding boundscheck for $/%ld", *q); + if (LOG_BOUNDNESS) printf("Avoiding boundscheck for $/%ld\n", *q); return inside; } } @@ -9864,7 +9866,7 @@ Boundness analyzeRelativeBound(Expression* e, *p = mul->e2->toInteger(); if (*p <= *q) { - e->warning("Avoiding boundscheck for $*%ld/%ld", *p, *q); + if (LOG_BOUNDNESS) printf("Avoiding boundscheck for $*%ld/%ld\n", *p, *q); return inside; } else if (*p > *q) @@ -9878,7 +9880,7 @@ Boundness analyzeRelativeBound(Expression* e, *p = mul->e1->toInteger(); if (*p <= *q) { - e->warning("Avoiding boundscheck for $*%ld/%ld", *p, *q); + if (LOG_BOUNDNESS) printf("Avoiding boundscheck for $*%ld/%ld\n", *p, *q); return inside; } else if (*p > *q) @@ -10168,7 +10170,7 @@ Expression *SliceExp::semantic(Scope *sc) (lwrMul*uprDiv <= uprMul*lwrDiv))) { lowerIsLessThanUpper = true; - this->warning("Lower <= upper bound"); + if (LOG_BOUNDNESS) printf("Lower <= upper bound\n"); } } diff --git a/src/expression.h b/src/expression.h index d43dc0790eb0..9c5796017f27 100644 --- a/src/expression.h +++ b/src/expression.h @@ -1011,18 +1011,6 @@ class SliceExp : public UnaExp bool upperIsInBounds; // true if upr <= e1.length bool lowerIsLessThanUpper; // true if lwr <= upr - // Decribes Boundness of a Slice Beginning or End Index. - enum Boundness - { - unknown, // unknown whether inside or outside - outside, // outside of [0 .. $] - inside, // inside of [0 .. $] - atStart, // specialization of inside - atEnd, // specialization of inside - }; - - Boundness analyzeRelativeBound(Expression* e, dinteger_t* mul, dinteger_t* div); - SliceExp(Loc loc, Expression *e1, Expression *lwr, Expression *upr); Expression *syntaxCopy(); Expression *semantic(Scope *sc); @@ -1031,7 +1019,6 @@ class SliceExp : public UnaExp Expression *toLvalue(Scope *sc, Expression *e); Expression *modifiableLvalue(Scope *sc, Expression *e); int isBool(int result); - bool isOpDollar(VarExp* ve); void accept(Visitor *v) { v->visit(this); } }; From 86d09078a9d25f539371c8130f339619948e8a69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Thu, 29 Jan 2015 18:09:49 +0100 Subject: [PATCH 22/35] Disable LOG_BOUNDNESS --- src/expression.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/expression.c b/src/expression.c index 6010afcb918a..c82672748d85 100644 --- a/src/expression.c +++ b/src/expression.c @@ -9828,7 +9828,7 @@ enum Boundness atEnd, // specialization of inside }; -static bool LOG_BOUNDNESS = true; +static bool LOG_BOUNDNESS = false; Boundness analyzeRelativeBound(Expression* e, VarDeclaration *lengthVar, From f37f5b05756db16068389e668ab016213f54e285 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Thu, 29 Jan 2015 23:20:21 +0100 Subject: [PATCH 23/35] Use new function isInteger, move lowerIsInBounds, longer names for Boundness enumerators, and implement partial support for Step 5 --- src/expression.c | 158 ++++++++++++++++++++++++++++++----------------- src/expression.h | 1 - 2 files changed, 100 insertions(+), 59 deletions(-) diff --git a/src/expression.c b/src/expression.c index c82672748d85..9445c7fd62aa 100644 --- a/src/expression.c +++ b/src/expression.c @@ -9799,7 +9799,6 @@ SliceExp::SliceExp(Loc loc, Expression *e1, Expression *lwr, Expression *upr) this->upr = upr; this->lwr = lwr; lengthVar = NULL; - lowerIsInBounds = false; upperIsInBounds = false; lowerIsLessThanUpper = false; } @@ -9818,79 +9817,115 @@ bool isOpDollar(Expression* e, VarDeclaration *lengthVar) return e->op == TOKvar && ((VarExp*)e)->var == lengthVar; } +bool isInteger(Expression* e, dinteger_t* value) +{ + if (e->op == TOKint64) + { + *value = e->toInteger(); + return true; + } + else + { + return false; + } +} + // Decribes Boundness of a Slice Beginning or End Index. enum Boundness { - unknown, // unknown whether inside or outside - outside, // outside of [0 .. $] - inside, // inside of [0 .. $] - atStart, // specialization of inside - atEnd, // specialization of inside + unknownBounds, // unknown whether inside or outside + + insideBounds, // inclusive inside of [0 .. $] + atLowBound, // specialization of inside + atHighBound, // specialization of inside + + outsideBounds, // outside of [0 .. $] + aboveBounds, // specialization of outside + belowBounds, // specialization of outside }; -static bool LOG_BOUNDNESS = false; +bool isInside(Boundness boundness) +{ + return (boundness == insideBounds || + boundness == atLowBound || + boundness == atHighBound); +} + +bool isOutside(Boundness boundness) +{ + return (boundness == outsideBounds || + boundness == aboveBounds || + boundness == belowBounds); +} + +bool LOG_BOUNDNESS = false; -Boundness analyzeRelativeBound(Expression* e, - VarDeclaration *lengthVar, - dinteger_t* p, dinteger_t* q) // out arguments +Boundness analyzeSliceBound(Expression* e, + VarDeclaration *lengthVar, + dinteger_t* p, dinteger_t* q, dinteger_t* off) // out arguments { - if (e->op == TOKint64 && e->toInteger() == 0) + if (e->op == TOKint64 && e->toInteger() == 0) // TODO: isIntegerEqualTo { - if (LOG_BOUNDNESS) printf("Avoiding boundscheck for 0\n"); - return atStart; + if (LOG_BOUNDNESS) e->warning("Avoiding boundscheck for 0"); + return atLowBound; } else if (isOpDollar(e, lengthVar)) { - if (LOG_BOUNDNESS) printf("Avoiding boundscheck for $\n"); - return atEnd; + if (LOG_BOUNDNESS) e->warning("Avoiding boundscheck for $"); + return atHighBound; } else if (e->op == TOKdiv) { DivExp* div = (DivExp*)e; - if (isOpDollar(div->e1, lengthVar) && div->e2->op == TOKint64) + if (isOpDollar(div->e1, lengthVar) && isInteger(div->e2, q)) { - *q = div->e2->toInteger(); if (*q >= 1) { - if (LOG_BOUNDNESS) printf("Avoiding boundscheck for $/%ld\n", *q); - return inside; + if (LOG_BOUNDNESS) e->warning("Avoiding boundscheck for $/%ld", *q); + return insideBounds; } } - else if (div->e1->op == TOKmul && - div->e2->op == TOKint64) + else if (div->e1->op == TOKmul && isInteger(div->e2, q)) { MulExp* mul = (MulExp*)div->e1; - *q = div->e2->toInteger(); - if (isOpDollar(mul->e1, lengthVar) && mul->e2->op == TOKint64) + if (isOpDollar(mul->e1, lengthVar) && isInteger(mul->e2, p)) { - *p = mul->e2->toInteger(); if (*p <= *q) { - if (LOG_BOUNDNESS) printf("Avoiding boundscheck for $*%ld/%ld\n", *p, *q); - return inside; + if (LOG_BOUNDNESS) e->warning("Avoiding boundscheck for $*%ld/%ld", *p, *q); + return insideBounds; } else if (*p > *q) { - return outside; + return outsideBounds; } } - else if (mul->e1->op == TOKint64 && + else if (isInteger(mul->e1, p) && isOpDollar(mul->e2, lengthVar)) { - *p = mul->e1->toInteger(); if (*p <= *q) { - if (LOG_BOUNDNESS) printf("Avoiding boundscheck for $*%ld/%ld\n", *p, *q); - return inside; + if (LOG_BOUNDNESS) e->warning("Avoiding boundscheck for $*%ld/%ld", *p, *q); + return insideBounds; } else if (*p > *q) { - return outside; + return outsideBounds; } } } } - return unknown; + else if (e->op == TOKadd) + { + AddExp* add = (AddExp*)e; + if ((isOpDollar(add->e1, lengthVar) && isInteger(add->e2, off)) || + (isInteger(add->e1, off) && isOpDollar(add->e2, lengthVar))) + { + return outsideBounds; + } + } + + return unknownBounds; } Expression *SliceExp::semantic(Scope *sc) @@ -10130,47 +10165,54 @@ Expression *SliceExp::semantic(Scope *sc) // avoid slice bounds-checking { // lower - dinteger_t lwrMul = 1; // non-one means [ $*lwrMul .. _ ] - dinteger_t lwrDiv = 1; // non-one means [ $/lwrDiv .. _ ] - const Boundness lwrBoundness = analyzeRelativeBound(lwr, this->lengthVar, &lwrMul, &lwrDiv); - if (lwrBoundness >= inside) + dinteger_t lwrM = 1; // non-one means [ $*lwrM .. _ ] + dinteger_t lwrD = 1; // non-one means [ $/lwrD .. _ ] + dinteger_t lwrO = 0; // offset + const Boundness lwrBoundness = analyzeSliceBound(lwr, this->lengthVar, &lwrM, &lwrD, &lwrO); + bool lowerIsInBounds = false; + if (isInside(lwrBoundness)) { - this->lowerIsInBounds = true; + lowerIsInBounds = true; } - else if (lwrBoundness == outside) + else if (isOutside(lwrBoundness)) { - this->error("Lower slice limit $*%lld/%lld is out of bounds", - (unsigned long long)lwrMul, - (unsigned long long)lwrDiv); + this->error("Lower slice limit $*%lld/%lld + %lld is out of bounds", + (unsigned long long)lwrM, + (unsigned long long)lwrD, + (unsigned long long)lwrO); } // upper - dinteger_t uprMul = 1; // non-one means [ $*uprMul .. _ ] - dinteger_t uprDiv = 1; // non-one means [ $/uprDiv .. _ ] - const Boundness uprBoundness = analyzeRelativeBound(upr, this->lengthVar, &uprMul, &uprDiv); - if (uprBoundness >= inside) + dinteger_t uprM = 1; // non-one means [ $*uprM .. _ ] + dinteger_t uprD = 1; // non-one means [ $/uprD .. _ ] + dinteger_t uprO = 0; + const Boundness uprBoundness = analyzeSliceBound(upr, this->lengthVar, &uprM, &uprD, &uprO); + if (isInside(uprBoundness)) { this->upperIsInBounds = true; } - else if (uprBoundness == outside) + else if (isOutside(uprBoundness)) { - this->error("Lower slice limit $*%lld/%lld is out of bounds", - (unsigned long long)lwrMul, - (unsigned long long)lwrDiv); + this->error("Upper slice limit $*%lld/%lld + %lld is out of bounds", + (unsigned long long)uprM, + (unsigned long long)uprD, + (unsigned long long)uprO); } // lower and upper - // [$*p/q .. $*r/s], p/q <= r/s => p*s <= r*q. - // TODO do we need to handle mul overflows for (lwrMul*uprDiv <= uprMul*lwrDiv) - if ((this->lowerIsInBounds && + // [$*p/q .. $*r/s], p/q+o <= r/s+t => p*s + o*q*s <= r*q + t*q*s. + // TODO do we need to handle mul overflows for (lwrM*uprD <= uprM*lwrD) + if ((lowerIsInBounds && this->upperIsInBounds) && - (lwrBoundness == atStart || - (lwrBoundness == atEnd && - uprBoundness == atEnd) || - (lwrMul*uprDiv <= uprMul*lwrDiv))) + (lwrBoundness == atLowBound || + (lwrBoundness == atHighBound && + uprBoundness == atHighBound) || + (lwrM*uprD <= uprM*lwrD && lwrO == 0 && uprO == 0) || + (lwrM == 1 && lwrD == 1 && + uprM == 1 && uprD == 1 && lwrO <= uprO))) { lowerIsLessThanUpper = true; - if (LOG_BOUNDNESS) printf("Lower <= upper bound\n"); + if (LOG_BOUNDNESS) this->warning("Lower <= upper bound"); } } diff --git a/src/expression.h b/src/expression.h index 9c5796017f27..616d56566aa5 100644 --- a/src/expression.h +++ b/src/expression.h @@ -1007,7 +1007,6 @@ class SliceExp : public UnaExp Expression *upr; // NULL if implicit 0 Expression *lwr; // NULL if implicit [length - 1] VarDeclaration *lengthVar; - bool lowerIsInBounds; // true if lwr <= e1.length bool upperIsInBounds; // true if upr <= e1.length bool lowerIsLessThanUpper; // true if lwr <= upr From 0a70d45e593547e88fb7c3d4087bb94e26a2ca71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Fri, 30 Jan 2015 00:13:21 +0100 Subject: [PATCH 24/35] Better dollar offset error checking for (Step 5) --- src/expression.c | 57 +++++++++++++++++++++++++++++------------------- 1 file changed, 34 insertions(+), 23 deletions(-) diff --git a/src/expression.c b/src/expression.c index 9445c7fd62aa..a58f2f15b966 100644 --- a/src/expression.c +++ b/src/expression.c @@ -9840,8 +9840,11 @@ enum Boundness atHighBound, // specialization of inside outsideBounds, // outside of [0 .. $] - aboveBounds, // specialization of outside - belowBounds, // specialization of outside + aboveHighBound, // specialization of outside + belowLowBound, // specialization of outside + + belowHighBound, + aboveLowBound, }; bool isInside(Boundness boundness) @@ -9854,11 +9857,11 @@ bool isInside(Boundness boundness) bool isOutside(Boundness boundness) { return (boundness == outsideBounds || - boundness == aboveBounds || - boundness == belowBounds); + boundness == aboveHighBound || + boundness == belowLowBound); } -bool LOG_BOUNDNESS = false; +bool LOG_BOUNDNESS = true; Boundness analyzeSliceBound(Expression* e, VarDeclaration *lengthVar, @@ -9866,12 +9869,12 @@ Boundness analyzeSliceBound(Expression* e, { if (e->op == TOKint64 && e->toInteger() == 0) // TODO: isIntegerEqualTo { - if (LOG_BOUNDNESS) e->warning("Avoiding boundscheck for 0"); + if (LOG_BOUNDNESS) e->warning("avoiding boundscheck for 0"); return atLowBound; } else if (isOpDollar(e, lengthVar)) { - if (LOG_BOUNDNESS) e->warning("Avoiding boundscheck for $"); + if (LOG_BOUNDNESS) e->warning("avoiding boundscheck for $"); return atHighBound; } else if (e->op == TOKdiv) @@ -9881,7 +9884,7 @@ Boundness analyzeSliceBound(Expression* e, { if (*q >= 1) { - if (LOG_BOUNDNESS) e->warning("Avoiding boundscheck for $/%ld", *q); + if (LOG_BOUNDNESS) e->warning("avoiding boundscheck for $/%ld", *q); return insideBounds; } } @@ -9892,7 +9895,7 @@ Boundness analyzeSliceBound(Expression* e, { if (*p <= *q) { - if (LOG_BOUNDNESS) e->warning("Avoiding boundscheck for $*%ld/%ld", *p, *q); + if (LOG_BOUNDNESS) e->warning("avoiding boundscheck for $*%ld/%ld", *p, *q); return insideBounds; } else if (*p > *q) @@ -9905,7 +9908,7 @@ Boundness analyzeSliceBound(Expression* e, { if (*p <= *q) { - if (LOG_BOUNDNESS) e->warning("Avoiding boundscheck for $*%ld/%ld", *p, *q); + if (LOG_BOUNDNESS) e->warning("avoiding boundscheck for $*%ld/%ld", *p, *q); return insideBounds; } else if (*p > *q) @@ -9921,7 +9924,15 @@ Boundness analyzeSliceBound(Expression* e, if ((isOpDollar(add->e1, lengthVar) && isInteger(add->e2, off)) || (isInteger(add->e1, off) && isOpDollar(add->e2, lengthVar))) { - return outsideBounds; + return aboveHighBound; + } + } + else if (e->op == TOKmin) + { + MinExp* min = (MinExp*)e; + if ((isOpDollar(min->e1, lengthVar) && isInteger(min->e2, off))) + { + return belowHighBound; } } @@ -10054,7 +10065,7 @@ Expression *SliceExp::semantic(Scope *sc) if (e1->op == TOKerror) return e1; error("%s cannot be sliced with []", - t1b->ty == Tvoid ? e1->toChars() : t1b->toChars()); + t1b->ty == Tvoid ? e1->toChars() : t1b->toChars()); return new ErrorExp(); } @@ -10176,10 +10187,10 @@ Expression *SliceExp::semantic(Scope *sc) } else if (isOutside(lwrBoundness)) { - this->error("Lower slice limit $*%lld/%lld + %lld is out of bounds", - (unsigned long long)lwrM, - (unsigned long long)lwrD, - (unsigned long long)lwrO); + lwr->error("slice limit $*%lld/%lld + %lld is out of bounds", + (unsigned long long)lwrM, + (unsigned long long)lwrD, + (unsigned long long)lwrO); } // upper @@ -10193,20 +10204,20 @@ Expression *SliceExp::semantic(Scope *sc) } else if (isOutside(uprBoundness)) { - this->error("Upper slice limit $*%lld/%lld + %lld is out of bounds", - (unsigned long long)uprM, - (unsigned long long)uprD, - (unsigned long long)uprO); + upr->error("slice limit $*%lld/%lld + %lld is out of bounds", + (unsigned long long)uprM, + (unsigned long long)uprD, + (unsigned long long)uprO); } // lower and upper // [$*p/q .. $*r/s], p/q+o <= r/s+t => p*s + o*q*s <= r*q + t*q*s. // TODO do we need to handle mul overflows for (lwrM*uprD <= uprM*lwrD) - if ((lowerIsInBounds && - this->upperIsInBounds) && - (lwrBoundness == atLowBound || + if ((lwrBoundness == atLowBound || (lwrBoundness == atHighBound && uprBoundness == atHighBound) || + (lwrBoundness == belowHighBound && + uprBoundness == atHighBound) || (lwrM*uprD <= uprM*lwrD && lwrO == 0 && uprO == 0) || (lwrM == 1 && lwrD == 1 && uprM == 1 && uprD == 1 && lwrO <= uprO))) From d1997cc9c918a6ff97a6a30f2b33e06f2abc4470 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Fri, 30 Jan 2015 00:13:49 +0100 Subject: [PATCH 25/35] Disable LOG_BOUNDNESS again --- src/expression.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/expression.c b/src/expression.c index a58f2f15b966..da19fea583e0 100644 --- a/src/expression.c +++ b/src/expression.c @@ -9861,7 +9861,7 @@ bool isOutside(Boundness boundness) boundness == belowLowBound); } -bool LOG_BOUNDNESS = true; +bool LOG_BOUNDNESS = false; Boundness analyzeSliceBound(Expression* e, VarDeclaration *lengthVar, From 7e0a57f429db6d68bdf8d288e1e064d06437a3aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Fri, 30 Jan 2015 00:37:22 +0100 Subject: [PATCH 26/35] Bugfix logic that sets lowerIsLessThanUpper --- src/expression.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/expression.c b/src/expression.c index da19fea583e0..971d9552c67e 100644 --- a/src/expression.c +++ b/src/expression.c @@ -146,7 +146,7 @@ Expression *getRightThis(Loc loc, Scope *sc, AggregateDeclaration *ad, if (flag) return NULL; e1->error("this for %s needs to be type %s not type %s", - var->toChars(), ad->toChars(), t->toChars()); + var->toChars(), ad->toChars(), t->toChars()); return new ErrorExp(); } } @@ -9861,7 +9861,7 @@ bool isOutside(Boundness boundness) boundness == belowLowBound); } -bool LOG_BOUNDNESS = false; +bool LOG_BOUNDNESS = true; Boundness analyzeSliceBound(Expression* e, VarDeclaration *lengthVar, @@ -9939,6 +9939,8 @@ Boundness analyzeSliceBound(Expression* e, return unknownBounds; } +#define DINTEGER_UNDEFINED (18446744073709551615ULL) + Expression *SliceExp::semantic(Scope *sc) { #if LOGSEMANTIC @@ -10178,7 +10180,7 @@ Expression *SliceExp::semantic(Scope *sc) // lower dinteger_t lwrM = 1; // non-one means [ $*lwrM .. _ ] dinteger_t lwrD = 1; // non-one means [ $/lwrD .. _ ] - dinteger_t lwrO = 0; // offset + dinteger_t lwrO = DINTEGER_UNDEFINED; // offset const Boundness lwrBoundness = analyzeSliceBound(lwr, this->lengthVar, &lwrM, &lwrD, &lwrO); bool lowerIsInBounds = false; if (isInside(lwrBoundness)) @@ -10196,7 +10198,7 @@ Expression *SliceExp::semantic(Scope *sc) // upper dinteger_t uprM = 1; // non-one means [ $*uprM .. _ ] dinteger_t uprD = 1; // non-one means [ $/uprD .. _ ] - dinteger_t uprO = 0; + dinteger_t uprO = DINTEGER_UNDEFINED; const Boundness uprBoundness = analyzeSliceBound(upr, this->lengthVar, &uprM, &uprD, &uprO); if (isInside(uprBoundness)) { @@ -10213,14 +10215,15 @@ Expression *SliceExp::semantic(Scope *sc) // lower and upper // [$*p/q .. $*r/s], p/q+o <= r/s+t => p*s + o*q*s <= r*q + t*q*s. // TODO do we need to handle mul overflows for (lwrM*uprD <= uprM*lwrD) - if ((lwrBoundness == atLowBound || - (lwrBoundness == atHighBound && - uprBoundness == atHighBound) || - (lwrBoundness == belowHighBound && - uprBoundness == atHighBound) || + if (((lwrBoundness == atLowBound && isInside(uprBoundness)) || + (isInside(lwrBoundness) && uprBoundness == atHighBound) || + (lwrBoundness == belowHighBound && uprBoundness == atHighBound) || + (lwrBoundness == atLowBound && uprBoundness == aboveLowBound) || (lwrM*uprD <= uprM*lwrD && lwrO == 0 && uprO == 0) || (lwrM == 1 && lwrD == 1 && - uprM == 1 && uprD == 1 && lwrO <= uprO))) + uprM == 1 && uprD == 1 && + lwrO != DINTEGER_UNDEFINED && uprO != DINTEGER_UNDEFINED && + lwrO <= uprO))) { lowerIsLessThanUpper = true; if (LOG_BOUNDNESS) this->warning("Lower <= upper bound"); From 9bc701501693256fe39c0a679df4d48cbd0ae2ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Fri, 30 Jan 2015 11:54:31 +0100 Subject: [PATCH 27/35] Disable LOG_BOUNDNESS again --- src/expression.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/expression.c b/src/expression.c index 971d9552c67e..68c663e31f70 100644 --- a/src/expression.c +++ b/src/expression.c @@ -9861,7 +9861,7 @@ bool isOutside(Boundness boundness) boundness == belowLowBound); } -bool LOG_BOUNDNESS = true; +bool LOG_BOUNDNESS = false; Boundness analyzeSliceBound(Expression* e, VarDeclaration *lengthVar, From 64433ae2b0499d32fea5284a03ef8fc7a77e8975 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Fri, 30 Jan 2015 12:17:20 +0100 Subject: [PATCH 28/35] Add boundscheck for p*$, p >= 2 --- src/expression.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/expression.c b/src/expression.c index 68c663e31f70..46c562098d4b 100644 --- a/src/expression.c +++ b/src/expression.c @@ -9861,8 +9861,10 @@ bool isOutside(Boundness boundness) boundness == belowLowBound); } -bool LOG_BOUNDNESS = false; +bool LOG_BOUNDNESS = true; +/** Analyze Expression $(D e) as Slice Bound with Length Variable (Dollar) in $(D lengthVar). + */ Boundness analyzeSliceBound(Expression* e, VarDeclaration *lengthVar, dinteger_t* p, dinteger_t* q, dinteger_t* off) // out arguments @@ -9918,11 +9920,22 @@ Boundness analyzeSliceBound(Expression* e, } } } + else if (e->op == TOKmul) + { + MulExp* mul = (MulExp*)e; + if (((isOpDollar(mul->e1, lengthVar) && isInteger(mul->e2, p)) || + (isInteger(mul->e1, p) && isOpDollar(mul->e2, lengthVar))) && + *p >= 2) + { + return aboveHighBound; // iff $ != 0 + } + } else if (e->op == TOKadd) { AddExp* add = (AddExp*)e; - if ((isOpDollar(add->e1, lengthVar) && isInteger(add->e2, off)) || - (isInteger(add->e1, off) && isOpDollar(add->e2, lengthVar))) + if (((isOpDollar(add->e1, lengthVar) && isInteger(add->e2, off)) || + (isInteger(add->e1, off) && isOpDollar(add->e2, lengthVar))) && + *off >= 1) { return aboveHighBound; } From a571190d4ad2c06cc042cdd39ea68c4d7121f1c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Fri, 30 Jan 2015 19:15:45 +0100 Subject: [PATCH 29/35] Better printing --- src/expression.c | 47 +++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 39 insertions(+), 8 deletions(-) diff --git a/src/expression.c b/src/expression.c index 46c562098d4b..67f8e62d5800 100644 --- a/src/expression.c +++ b/src/expression.c @@ -14,6 +14,7 @@ #include #include #include +#include #include "rmem.h" #include "port.h" @@ -10202,10 +10203,25 @@ Expression *SliceExp::semantic(Scope *sc) } else if (isOutside(lwrBoundness)) { - lwr->error("slice limit $*%lld/%lld + %lld is out of bounds", - (unsigned long long)lwrM, - (unsigned long long)lwrD, - (unsigned long long)lwrO); + char buf[1024]; + int tot = sizeof(buf); + int off = 0; + off += snprintf(&buf[off], tot-off, "lower slice limit "); + off += snprintf(&buf[off], tot-off, "$"); + if (lwrM != 1 && off < tot) + { + off += snprintf(&buf[off], tot-off, "*%lld", (unsigned long long)lwrM); + } + if (lwrD != 1 && off < tot) + { + off += snprintf(&buf[off], tot-off, "/%lld", (unsigned long long)lwrD); + } + if (lwrO != 0 && lwrO != DINTEGER_UNDEFINED && off < tot) + { + off += snprintf(&buf[off], tot-off, "+%lld", (unsigned long long)lwrO); + } + off += snprintf(&buf[off], tot-off, " is out of bounds"); + lwr->error(buf); } // upper @@ -10219,10 +10235,25 @@ Expression *SliceExp::semantic(Scope *sc) } else if (isOutside(uprBoundness)) { - upr->error("slice limit $*%lld/%lld + %lld is out of bounds", - (unsigned long long)uprM, - (unsigned long long)uprD, - (unsigned long long)uprO); + char buf[1024]; + int tot = sizeof(buf); + int off = 0; + off += snprintf(&buf[off], tot-off, "upper slice limit "); + off += snprintf(&buf[off], tot-off, "$"); + if (uprM != 1 && off < tot) + { + off += snprintf(&buf[off], tot-off, "*%lld", (unsigned long long)uprM); + } + if (uprD != 1 && off < tot) + { + off += snprintf(&buf[off], tot-off, "/%lld", (unsigned long long)uprD); + } + if (uprO != 0 && uprO != DINTEGER_UNDEFINED && off < tot) + { + off += snprintf(&buf[off], tot-off, "+%lld", (unsigned long long)uprO); + } + off += snprintf(&buf[off], tot-off, " is out of bounds"); + upr->error(buf); } // lower and upper From 3e7349bc9113d3d86a2c8bb6206c91ede6934b27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Sun, 1 Feb 2015 00:55:10 +0100 Subject: [PATCH 30/35] Add lower limit < 0 --- src/expression.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/expression.c b/src/expression.c index 67f8e62d5800..89d4398e6197 100644 --- a/src/expression.c +++ b/src/expression.c @@ -9862,7 +9862,7 @@ bool isOutside(Boundness boundness) boundness == belowLowBound); } -bool LOG_BOUNDNESS = true; +bool LOG_BOUNDNESS = false; /** Analyze Expression $(D e) as Slice Bound with Length Variable (Dollar) in $(D lengthVar). */ @@ -9870,10 +9870,18 @@ Boundness analyzeSliceBound(Expression* e, VarDeclaration *lengthVar, dinteger_t* p, dinteger_t* q, dinteger_t* off) // out arguments { - if (e->op == TOKint64 && e->toInteger() == 0) // TODO: isIntegerEqualTo + if (e->op == TOKint64) { - if (LOG_BOUNDNESS) e->warning("avoiding boundscheck for 0"); - return atLowBound; + sinteger_t value = e->toInteger(); + if (value == 0) + { + if (LOG_BOUNDNESS) e->warning("avoiding boundscheck for 0"); + return atLowBound; + } + else if (value < 0) + { + return belowLowBound; + } } else if (isOpDollar(e, lengthVar)) { From 7777ca0e576057709be7765d349a9070bf61775e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Sun, 1 Feb 2015 12:28:54 +0100 Subject: [PATCH 31/35] Use isunsigned in analyzeSliceBound --- src/expression.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/expression.c b/src/expression.c index 89d4398e6197..1a4839d58657 100644 --- a/src/expression.c +++ b/src/expression.c @@ -9872,15 +9872,23 @@ Boundness analyzeSliceBound(Expression* e, { if (e->op == TOKint64) { - sinteger_t value = e->toInteger(); + const dinteger_t value = e->toInteger(); if (value == 0) { if (LOG_BOUNDNESS) e->warning("avoiding boundscheck for 0"); return atLowBound; } - else if (value < 0) + if (e->type->isunsigned()) // BUG this evaluates to true for [-1 .. _ ] { - return belowLowBound; + const uinteger_t uvalue = (uinteger_t)value; + } + else + { + const sinteger_t svalue = (sinteger_t)value; + if (svalue < 0) + { + return belowLowBound; + } } } else if (isOpDollar(e, lengthVar)) From 9e61acb4df3c99e4b98f7301efd7826f2e578080 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Sun, 1 Feb 2015 16:29:03 +0100 Subject: [PATCH 32/35] Make all negative bounds an error including minus dollar --- src/expression.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/expression.c b/src/expression.c index 1a4839d58657..2f3d88288468 100644 --- a/src/expression.c +++ b/src/expression.c @@ -9878,17 +9878,10 @@ Boundness analyzeSliceBound(Expression* e, if (LOG_BOUNDNESS) e->warning("avoiding boundscheck for 0"); return atLowBound; } - if (e->type->isunsigned()) // BUG this evaluates to true for [-1 .. _ ] + const sinteger_t svalue = (sinteger_t)value; + if (svalue < 0) { - const uinteger_t uvalue = (uinteger_t)value; - } - else - { - const sinteger_t svalue = (sinteger_t)value; - if (svalue < 0) - { - return belowLowBound; - } + return belowLowBound; } } else if (isOpDollar(e, lengthVar)) @@ -9896,6 +9889,14 @@ Boundness analyzeSliceBound(Expression* e, if (LOG_BOUNDNESS) e->warning("avoiding boundscheck for $"); return atHighBound; } + else if (e->op == TOKneg) + { + NegExp* neg = (NegExp*)e; + if (isOpDollar(neg->e1, lengthVar)) + { + return outsideBounds; + } + } else if (e->op == TOKdiv) { DivExp* div = (DivExp*)e; From 9abf724bbc269522d95f3f7e8dd485a54b478d73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Sun, 1 Feb 2015 22:07:12 +0100 Subject: [PATCH 33/35] Make use of conversion functions isXExp and error checking for - $ * N --- src/expression.c | 85 +++++++++++++++++++++++++++++++++++------------- 1 file changed, 63 insertions(+), 22 deletions(-) diff --git a/src/expression.c b/src/expression.c index 2f3d88288468..468581449421 100644 --- a/src/expression.c +++ b/src/expression.c @@ -9831,6 +9831,31 @@ bool isInteger(Expression* e, dinteger_t* value) } } +AddExp* isAddExp(Expression* e) +{ + return e->op == TOKadd ? (AddExp*)e : NULL; +} + +MinExp* isMinExp(Expression* e) +{ + return e->op == TOKmin ? (MinExp*)e : NULL; +} + +MulExp* isMulExp(Expression* e) +{ + return e->op == TOKmul ? (MulExp*)e : NULL; +} + +DivExp* isDivExp(Expression* e) +{ + return e->op == TOKdiv ? (DivExp*)e : NULL; +} + +NegExp* isNegExp(Expression* e) +{ + return e->op == TOKneg ? (NegExp*)e : NULL; +} + // Decribes Boundness of a Slice Beginning or End Index. enum Boundness { @@ -9868,7 +9893,7 @@ bool LOG_BOUNDNESS = false; */ Boundness analyzeSliceBound(Expression* e, VarDeclaration *lengthVar, - dinteger_t* p, dinteger_t* q, dinteger_t* off) // out arguments + dinteger_t* p, uinteger_t* q, uinteger_t* off) // out arguments { if (e->op == TOKint64) { @@ -9876,12 +9901,15 @@ Boundness analyzeSliceBound(Expression* e, if (value == 0) { if (LOG_BOUNDNESS) e->warning("avoiding boundscheck for 0"); + *off = 0; return atLowBound; } const sinteger_t svalue = (sinteger_t)value; - if (svalue < 0) + if (svalue < 0) // limit max slice bound index to 2^n-1, n=32 on 32-bit and n=64 on 64-bit { - return belowLowBound; + // printf("%s is below low bound: %lld", e->toChars(), svalue); + *off = value; + return belowLowBound; // TODO aboveHighBound instead? } } else if (isOpDollar(e, lengthVar)) @@ -9889,17 +9917,15 @@ Boundness analyzeSliceBound(Expression* e, if (LOG_BOUNDNESS) e->warning("avoiding boundscheck for $"); return atHighBound; } - else if (e->op == TOKneg) + else if (NegExp* neg = isNegExp(e)) { - NegExp* neg = (NegExp*)e; if (isOpDollar(neg->e1, lengthVar)) { return outsideBounds; } } - else if (e->op == TOKdiv) + else if (DivExp* div = isDivExp(e)) { - DivExp* div = (DivExp*)e; if (isOpDollar(div->e1, lengthVar) && isInteger(div->e2, q)) { if (*q >= 1) @@ -9938,19 +9964,28 @@ Boundness analyzeSliceBound(Expression* e, } } } - else if (e->op == TOKmul) + else if (MulExp* mul = isMulExp(e)) { - MulExp* mul = (MulExp*)e; - if (((isOpDollar(mul->e1, lengthVar) && isInteger(mul->e2, p)) || - (isInteger(mul->e1, p) && isOpDollar(mul->e2, lengthVar))) && - *p >= 2) + if (NegExp* neg = isNegExp(mul->e1)) { - return aboveHighBound; // iff $ != 0 + if (isOpDollar(neg->e1, lengthVar) && isInteger(mul->e2, p) && + *p >= 2) + { + return belowLowBound; + } + } + else + { + if (((isOpDollar(mul->e1, lengthVar) && isInteger(mul->e2, p)) || + (isInteger(mul->e1, p) && isOpDollar(mul->e2, lengthVar))) && + *p >= 2) + { + return aboveHighBound; // iff $ != 0 + } } } - else if (e->op == TOKadd) + else if (AddExp* add = isAddExp(e)) { - AddExp* add = (AddExp*)e; if (((isOpDollar(add->e1, lengthVar) && isInteger(add->e2, off)) || (isInteger(add->e1, off) && isOpDollar(add->e2, lengthVar))) && *off >= 1) @@ -9958,9 +9993,8 @@ Boundness analyzeSliceBound(Expression* e, return aboveHighBound; } } - else if (e->op == TOKmin) + else if (MinExp* min = isMinExp(e)) { - MinExp* min = (MinExp*)e; if ((isOpDollar(min->e1, lengthVar) && isInteger(min->e2, off))) { return belowHighBound; @@ -10210,8 +10244,8 @@ Expression *SliceExp::semantic(Scope *sc) { // lower dinteger_t lwrM = 1; // non-one means [ $*lwrM .. _ ] - dinteger_t lwrD = 1; // non-one means [ $/lwrD .. _ ] - dinteger_t lwrO = DINTEGER_UNDEFINED; // offset + uinteger_t lwrD = 1; // non-one means [ $/lwrD .. _ ] + uinteger_t lwrO = DINTEGER_UNDEFINED; // offset const Boundness lwrBoundness = analyzeSliceBound(lwr, this->lengthVar, &lwrM, &lwrD, &lwrO); bool lowerIsInBounds = false; if (isInside(lwrBoundness)) @@ -10224,7 +10258,10 @@ Expression *SliceExp::semantic(Scope *sc) int tot = sizeof(buf); int off = 0; off += snprintf(&buf[off], tot-off, "lower slice limit "); - off += snprintf(&buf[off], tot-off, "$"); + if (lwrM != 1 && lwrD != 1 && off < tot) + { + off += snprintf(&buf[off], tot-off, "$"); + } if (lwrM != 1 && off < tot) { off += snprintf(&buf[off], tot-off, "*%lld", (unsigned long long)lwrM); @@ -10243,8 +10280,8 @@ Expression *SliceExp::semantic(Scope *sc) // upper dinteger_t uprM = 1; // non-one means [ $*uprM .. _ ] - dinteger_t uprD = 1; // non-one means [ $/uprD .. _ ] - dinteger_t uprO = DINTEGER_UNDEFINED; + uinteger_t uprD = 1; // non-one means [ $/uprD .. _ ] + uinteger_t uprO = DINTEGER_UNDEFINED; const Boundness uprBoundness = analyzeSliceBound(upr, this->lengthVar, &uprM, &uprD, &uprO); if (isInside(uprBoundness)) { @@ -10257,6 +10294,10 @@ Expression *SliceExp::semantic(Scope *sc) int off = 0; off += snprintf(&buf[off], tot-off, "upper slice limit "); off += snprintf(&buf[off], tot-off, "$"); + if (uprM != 1 && uprD != 1 && off < tot) + { + off += snprintf(&buf[off], tot-off, "$"); + } if (uprM != 1 && off < tot) { off += snprintf(&buf[off], tot-off, "*%lld", (unsigned long long)uprM); From a371fc23a1f42199fdc24296f741dadad3969f84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Tue, 3 Feb 2015 00:55:51 +0100 Subject: [PATCH 34/35] Make isXExp into virtual members of Expression --- src/expression.c | 37 ++++++------------------------------- src/expression.h | 12 ++++++++++++ 2 files changed, 18 insertions(+), 31 deletions(-) diff --git a/src/expression.c b/src/expression.c index 468581449421..9efcc17371c5 100644 --- a/src/expression.c +++ b/src/expression.c @@ -9831,31 +9831,6 @@ bool isInteger(Expression* e, dinteger_t* value) } } -AddExp* isAddExp(Expression* e) -{ - return e->op == TOKadd ? (AddExp*)e : NULL; -} - -MinExp* isMinExp(Expression* e) -{ - return e->op == TOKmin ? (MinExp*)e : NULL; -} - -MulExp* isMulExp(Expression* e) -{ - return e->op == TOKmul ? (MulExp*)e : NULL; -} - -DivExp* isDivExp(Expression* e) -{ - return e->op == TOKdiv ? (DivExp*)e : NULL; -} - -NegExp* isNegExp(Expression* e) -{ - return e->op == TOKneg ? (NegExp*)e : NULL; -} - // Decribes Boundness of a Slice Beginning or End Index. enum Boundness { @@ -9917,14 +9892,14 @@ Boundness analyzeSliceBound(Expression* e, if (LOG_BOUNDNESS) e->warning("avoiding boundscheck for $"); return atHighBound; } - else if (NegExp* neg = isNegExp(e)) + else if (NegExp* neg = e->isNegExp()) { if (isOpDollar(neg->e1, lengthVar)) { return outsideBounds; } } - else if (DivExp* div = isDivExp(e)) + else if (DivExp* div = e->isDivExp()) { if (isOpDollar(div->e1, lengthVar) && isInteger(div->e2, q)) { @@ -9964,9 +9939,9 @@ Boundness analyzeSliceBound(Expression* e, } } } - else if (MulExp* mul = isMulExp(e)) + else if (MulExp* mul = e->isMulExp()) { - if (NegExp* neg = isNegExp(mul->e1)) + if (NegExp* neg = mul->e1->isNegExp()) { if (isOpDollar(neg->e1, lengthVar) && isInteger(mul->e2, p) && *p >= 2) @@ -9984,7 +9959,7 @@ Boundness analyzeSliceBound(Expression* e, } } } - else if (AddExp* add = isAddExp(e)) + else if (AddExp* add = e->isAddExp()) { if (((isOpDollar(add->e1, lengthVar) && isInteger(add->e2, off)) || (isInteger(add->e1, off) && isOpDollar(add->e2, lengthVar))) && @@ -9993,7 +9968,7 @@ Boundness analyzeSliceBound(Expression* e, return aboveHighBound; } } - else if (MinExp* min = isMinExp(e)) + else if (MinExp* min = e->isMinExp()) { if ((isOpDollar(min->e1, lengthVar) && isInteger(min->e2, off))) { diff --git a/src/expression.h b/src/expression.h index 616d56566aa5..567fa0c8ee4a 100644 --- a/src/expression.h +++ b/src/expression.h @@ -200,6 +200,13 @@ class Expression : public RootObject int isConst() { return ::isConst(this); } virtual int isBool(int result); + + virtual AddExp* isAddExp() { return NULL; } + virtual MinExp* isMinExp() { return NULL; } + virtual MulExp* isMulExp() { return NULL; } + virtual DivExp* isDivExp() { return NULL; } + virtual NegExp* isNegExp() { return NULL; } + Expression *op_overload(Scope *sc) { return ::op_overload(this, sc); @@ -929,6 +936,7 @@ class NegExp : public UnaExp Expression *semantic(Scope *sc); void accept(Visitor *v) { v->visit(this); } + NegExp* isNegExp() { return this; } }; class UAddExp : public UnaExp @@ -1274,6 +1282,7 @@ class AddExp : public BinExp Expression *semantic(Scope *sc); void accept(Visitor *v) { v->visit(this); } + AddExp* isAddExp() { return this; } }; class MinExp : public BinExp @@ -1283,6 +1292,7 @@ class MinExp : public BinExp Expression *semantic(Scope *sc); void accept(Visitor *v) { v->visit(this); } + MinExp* isMinExp() { return this; } }; class CatExp : public BinExp @@ -1301,6 +1311,7 @@ class MulExp : public BinExp Expression *semantic(Scope *sc); void accept(Visitor *v) { v->visit(this); } + MulExp* isMulExp() { return this; } }; class DivExp : public BinExp @@ -1310,6 +1321,7 @@ class DivExp : public BinExp Expression *semantic(Scope *sc); void accept(Visitor *v) { v->visit(this); } + DivExp* isDivExp() { return this; } }; class ModExp : public BinExp From 3b5e9dc1e1371d417b6a3308c64dc747216cf36b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Per=20Nordl=C3=B6w?= Date: Tue, 3 Feb 2015 13:59:06 +0100 Subject: [PATCH 35/35] Add debug print --- src/expression.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/expression.c b/src/expression.c index 9efcc17371c5..8f6ce49c48db 100644 --- a/src/expression.c +++ b/src/expression.c @@ -9882,8 +9882,11 @@ Boundness analyzeSliceBound(Expression* e, const sinteger_t svalue = (sinteger_t)value; if (svalue < 0) // limit max slice bound index to 2^n-1, n=32 on 32-bit and n=64 on 64-bit { - // printf("%s is below low bound: %lld", e->toChars(), svalue); *off = value; + printf("%s is below low bound, e->toInteger() returns %ld, e->toUInteger() returns %ld\n", + e->toChars(), + e->toInteger(), + e->toUInteger()); return belowLowBound; // TODO aboveHighBound instead? } }