Skip to content

Commit

Permalink
fix a bounds issue in row_combine which was setting off a segfault. I…
Browse files Browse the repository at this point in the history 
…mplement/fix a lot more tests.
  • Loading branch information
@Whiteknight
Whiteknight committed Aug 19, 2010
1 parent ae283cf commit 95996f9
Show file tree
Hide file tree
Showing 5 changed files with 217 additions and 76 deletions.
2 changes: 1 addition & 1 deletion src/pmc/complexmatrix2d.pmc
View file
Expand Up @@ -1364,7 +1364,7 @@ Swap two rows
const INTVAL cols = attrs->cols;
const INTVAL flags = attrs->flags;
INTVAL i;
if (srcidx < 0 || srcidx >= rows)
if (srcidx < 0 || srcidx >= rows || destidx < 0 || destidx >= rows)
Parrot_ex_throw_from_c_args(INTERP, NULL, EXCEPTION_OUT_OF_BOUNDS,
PLATYPENAME ": Row index out of bounds");
for (i = 0; i < cols; i++) {
Expand Down
12 changes: 6 additions & 6 deletions src/pmc/nummatrix2d.pmc
View file
Expand Up @@ -1192,11 +1192,11 @@ Calculates the matrix equation:

METHOD gemm(FLOATVAL alpha, PMC * A, PMC *B, FLOATVAL beta, PMC *C) {
PMC * const c_out = VTABLE_clone(INTERP, C);
//if (A->vtable->base_type != B->vtable->base_type &&
// C->vtable->base_type != A->vtable->base_type &&
// A->vtable->base_type != enum_class_NumMatrix2D)
// Parrot_ex_throw_from_c_args(INTERP, NULL, EXCEPTION_OUT_OF_BOUNDS,
// PLATYPENAME ": gemm only accepts NumMatrix2D arguments");
if (A->vtable->base_type != B->vtable->base_type &&
C->vtable->base_type != A->vtable->base_type &&
A->vtable->base_type != SELF->vtable->base_type)
Parrot_ex_throw_from_c_args(INTERP, NULL, EXCEPTION_OUT_OF_BOUNDS,
PLATYPENAME ": gemm only accepts NumMatrix2D arguments");
call_gemm(INTERP, alpha, A, B, beta, c_out);
RETURN(PMC* c_out);
}
Expand Down Expand Up @@ -1226,7 +1226,7 @@ Swap two rows
const INTVAL cols = attrs->cols;
const INTVAL flags = attrs->flags;
INTVAL i;
if (srcidx < 0 || srcidx >= rows)
if (srcidx < 0 || srcidx >= rows || destidx < 0 || destidx >= rows)
Parrot_ex_throw_from_c_args(INTERP, NULL, EXCEPTION_OUT_OF_BOUNDS,
PLATYPENAME ": Row index out of bounds");
for (i = 0; i < cols; i++) {
Expand Down
46 changes: 0 additions & 46 deletions t/pmc/nummatrix2d.t
View file
Expand Up @@ -244,49 +244,3 @@ method test_METHOD_gemm_aABbC() {
my $Z := $A.'gemm'(0.5, $A, $B, -10, $C);
assert_equal($Y, $Z, "gemm aAB does not work");
}

# Row operation tests

method test_METHOD_row_combine() {
my $A := self.matrix3x3(1.0, 2.0, 3.0,
4.0, 5.0, 6.0,
7.0, 8.0, 9.0);
$A.row_combine(1, 2, 1.0);
my $B := self.matrix3x3(1.0, 2.0, 3.0,
4.0, 5.0, 6.0,
11.0, 13.0, 15.0);
assert_equal($A, $B, "can add rows");
}

method test_METHOD_row_combine_GAIN() {
my $A := self.matrix3x3(1.0, 2.0, 3.0,
4.0, 5.0, 6.0,
7.0, 8.0, 9.0);
$A.row_combine(0, 1, -4.0);
my $B := self.matrix3x3(1.0, 2.0, 3.0,
0.0, -3.0, -6.0,
7.0, 8.0, 9.0);
assert_equal($A, $B, "can add rows");
}

method test_METHOD_row_scale() {
my $A := self.matrix3x3(1.0, 2.0, 3.0,
4.0, 5.0, 6.0,
7.0, 8.0, 9.0);
$A.row_scale(0, 4);
my $B := self.matrix3x3(4.0, 8.0, 12.0,
4.0, 5.0, 6.0,
7.0, 8.0, 9.0);
assert_equal($A, $B, "can add rows");
}

method test_METHOD_row_swap() {
my $A := self.matrix3x3(1.0, 2.0, 3.0,
4.0, 5.0, 6.0,
7.0, 8.0, 9.0);
$A.row_swap(0, 1);
my $B := self.matrix3x3(4.0, 5.0, 6.0,
1.0, 2.0, 3.0,
7.0, 8.0, 9.0);
assert_equal($A, $B, "can add rows");
}
18 changes: 17 additions & 1 deletion t/testlib/matrixtestbase.nqp
View file
Expand Up @@ -12,7 +12,9 @@ class Pla::Matrix::MatrixTestBase is UnitTest::Testcase {
use('UnitTest::Assertions');
}

method default_loader() { Pla::Matrix::Loader.new; }
method default_loader() {
Pla::Matrix::Loader.new;
}

method RequireOverride($m) {
Exception::MethodNotFound.new(
Expand Down Expand Up @@ -85,6 +87,20 @@ class Pla::Matrix::MatrixTestBase is UnitTest::Testcase {
return ($m);
}

method defaultmatrix3x3() {
return self.matrix3x3(
self.defaultvalue(),
self.defaultvalue(),
self.defaultvalue(),
self.defaultvalue(),
self.defaultvalue(),
self.defaultvalue(),
self.defaultvalue(),
self.defaultvalue(),
self.defaultvalue()
);
}

method AssertSize($m, $rows, $cols) {
my $real_rows := pir::getattribute__PPS($m, "rows");
my $real_cols := pir::getattribute__PPS($m, "cols");
Expand Down
215 changes: 193 additions & 22 deletions t/testlib/numericmatrixtest.nqp
View file
Expand Up @@ -324,25 +324,196 @@ class Pla::Matrix::NumericMatrixTest is Pla::Matrix::MatrixTest {
method test_METHOD_gemm_AB() { self.RequireOverride("test_METHOD_gemm_AB"); }
method test_METHOD_gemm_aAB() { self.RequireOverride("test_METHOD_gemm_aAB"); }
method test_METHOD_gemm_aABbC() { self.RequireOverride("test_METHOD_gemm_aABbC"); }
method test_METHOD_gemm_BADTYPE() { self.RequireOverride("test_METHOD_gemm_BADTYPE");}
method test_METHOD_gemm_BADSIZE_A() { self.RequireOverride("test_METHOD_gemm_BADSIZE_A"); }
method test_METHOD_gemm_BADSIZE_B() { self.RequireOverride("test_METHOD_gemm_BADSIZE_B"); }
method test_METHOD_gemm_BADSIZE_C() { self.RequireOverride("test_METHOD_gemm_BADSIZE_C"); }
method test_METHOD_row_combine() { self.RequireOverride("test_METHOD_row_combine"); }
method test_METHOD_row_combine_GAIN() { self.RequireOverride("test_METHOD_row_combine_GAIN"); }
method test_METHOD_row_combine_NEGINDICES_A() { self.RequireOverride("test_METHOD_row_combine_NEGINDICES_A"); }
method test_METHOD_row_combine_BOUNDS_A() { self.RequireOverride("test_METHOD_row_combine_BOUNDS_A"); }
method test_METHOD_row_combine_NEGINDICES_B() { self.RequireOverride("test_METHOD_row_combine_NEGINDICES_B"); }
method test_METHOD_row_combine_BOUNDS_B() { self.RequireOverride("test_METHOD_row_combine_BOUNDS_B"); }
method test_METHOD_row_swap() { todo("Write this"); }
method test_METHOD_row_swap_NEGINDICES_A() { todo("Write this"); }
method test_METHOD_row_swap_BOUNDS_A() { todo("Write this"); }
method test_METHOD_row_swap_NEGINDICES_B() { todo("Write this"); }
method test_METHOD_row_swap_BOUNDS_B() { todo("Write this"); }
method test_METHOD_row_scale() { self.RequireOverride("test_METHOD_row_scale"); }
method test_METHOD_row_scale_NEGINDICES() { self.RequireOverride("test_METHOD_row_scale_NEGINDICES"); }
method test_METHOD_row_scale_BOUNDS() { self.RequireOverride("test_METHOD_row_scale_BOUNDS"); }
}
method test_METHOD_gemm_BADTYPE_A() {
assert_throws(Exception::OutOfBounds, "A is bad type",
{
my $A := "foobar";
my $B := self.defaultmatrix3x3();
my $C := self.defaultmatrix3x3();
$B.gemm(1.0, $A, $B, 1.0, $C);
});
}

method test_METHOD_gemm_BADTYPE_B() {
assert_throws(Exception::OutOfBounds, "B is bad type",
{
my $A := self.defaultmatrix3x3();
my $B := "foobar";
my $C := self.defaultmatrix3x3();
$A.gemm(1.0, $A, $B, 1.0, $C);
});
}

method test_METHOD_gemm_BADTYPE_C() {
assert_throws(Exception::OutOfBounds, "C is bad type",
{
my $A := self.defaultmatrix3x3();
my $B := self.defaultmatrix3x3();
my $C := "foobar";
$A.gemm(1.0, $A, $B, 1.0, $C);
});
}

method test_METHOD_gemm_BADSIZE_A() {
assert_throws(Exception::OutOfBounds, "A has incorrect size",
{
my $A := self.defaultmatrix2x2();
my $B := self.defaultmatrix3x3();
my $C := self.defaultmatrix3x3();
$A.gemm(1.0, $A, $B, 1.0, $C);
});
}

method test_METHOD_gemm_BADSIZE_B() {
assert_throws(Exception::OutOfBounds, "B has incorrect size",
{
my $A := self.defaultmatrix3x3();
my $B := self.defaultmatrix2x2();
my $C := self.defaultmatrix3x3();
$A.gemm(1.0, $A, $B, 1.0, $C);
});
}

method test_METHOD_gemm_BADSIZE_C() {
assert_throws(Exception::OutOfBounds, "C has incorrect size",
{
my $A := self.defaultmatrix3x3();
my $B := self.defaultmatrix3x3();
my $C := self.defaultmatrix2x2();
$A.gemm(1.0, $A, $B, 1.0, $C);
});
}

method test_METHOD_row_combine() {
my $A := self.fancymatrix2x2();
my $B := self.matrix2x2(self.fancyvalue(0) + self.fancyvalue(2), self.fancyvalue(1) + self.fancyvalue(3),
self.fancyvalue(2), self.fancyvalue(3));
$A.row_combine(1, 0, 1);
assert_equal($A, $B, "cannot row_combine");
}

method test_METHOD_row_combine_GAIN() {
my $A := self.fancymatrix2x2();
my $B := self.matrix2x2(self.fancyvalue(0) + self.fancyvalue(2) * self.fancyvalue(0),
self.fancyvalue(1) + self.fancyvalue(3) * self.fancyvalue(0),
self.fancyvalue(2), self.fancyvalue(3));
$A.row_combine(1, 0, self.fancyvalue(0));
assert_equal($A, $B, "cannot row_combine");
}

method test_METHOD_row_combine_NEGINDICES_A() {
assert_throws(Exception::OutOfBounds, "Index A is out of bounds",
{
my $A := self.defaultmatrix3x3();
$A.row_combine(-1, 1, 1);
});
}

method test_METHOD_row_combine_BOUNDS_A() {
assert_throws(Exception::OutOfBounds, "Index A is out of bounds",
{
my $A := self.defaultmatrix3x3();
$A.row_combine(7, 1, 1);
});
}

method test_METHOD_row_combine_NEGINDICES_B() {
assert_throws(Exception::OutOfBounds, "Index B is out of bounds",
{
my $A := self.defaultmatrix3x3();
$A.row_combine(1, -1, 1);
});
}

method test_METHOD_row_combine_BOUNDS_B() {
assert_throws(Exception::OutOfBounds, "Index B is out of bounds",
{
my $A := self.defaultmatrix3x3();
$A.row_combine(1, 7, 1);
});
}

method test_METHOD_row_swap() {
my $A := self.matrix();
$A.initialize_from_args(3, 3,
self.fancyvalue(0), self.fancyvalue(0), self.fancyvalue(0),
self.fancyvalue(1), self.fancyvalue(1), self.fancyvalue(1),
self.fancyvalue(2), self.fancyvalue(2), self.fancyvalue(2));

my $B := self.matrix();
$B.initialize_from_args(3, 3,
self.fancyvalue(1), self.fancyvalue(1), self.fancyvalue(1),
self.fancyvalue(2), self.fancyvalue(2), self.fancyvalue(2),
self.fancyvalue(0), self.fancyvalue(0), self.fancyvalue(0));
$A.row_swap(0, 2);
$A.row_swap(0, 1);
assert_equal($A, $B, "cannot row_swap");
}

method test_METHOD_row_swap_NEGINDICES_A() {
assert_throws(Exception::OutOfBounds, "Index A is out of bounds",
{
my $A := self.defaultmatrix3x3();
$A.row_swap(-1, 1);
});
}

method test_METHOD_row_swap_BOUNDS_A() {
assert_throws(Exception::OutOfBounds, "Index A is out of bounds",
{
my $A := self.defaultmatrix3x3();
$A.row_swap(7, 1);
});
}

method test_METHOD_row_swap_NEGINDICES_B() {
assert_throws(Exception::OutOfBounds, "Index B is out of bounds",
{
my $A := self.defaultmatrix3x3();
$A.row_swap(1, -1);
});
}

method test_METHOD_row_swap_BOUNDS_B() {
assert_throws(Exception::OutOfBounds, "Index B is out of bounds",
{
my $A := self.defaultmatrix3x3();
$A.row_swap(1, 7);
});
}

method test_METHOD_row_scale() {
my $A := self.matrix();
$A.initialize_from_args(3, 3,
self.fancyvalue(0), self.fancyvalue(0), self.fancyvalue(0),
self.fancyvalue(1), self.fancyvalue(1), self.fancyvalue(1),
self.fancyvalue(2), self.fancyvalue(2), self.fancyvalue(2));

my $B := self.matrix();
$B.initialize_from_args(3, 3,
self.fancyvalue(0) * 2, self.fancyvalue(0) * 2, self.fancyvalue(0) * 2,
self.fancyvalue(1) * 3, self.fancyvalue(1) * 3, self.fancyvalue(1) * 3,
self.fancyvalue(2) * 4, self.fancyvalue(2) * 4, self.fancyvalue(2) * 4);
$A.row_scale(0, 2);
$A.row_scale(1, 3);
$A.row_scale(2, 4);
assert_equal($A, $B, "cannot scale rows");
}

method test_METHOD_row_scale_NEGINDICES() {
assert_throws(Exception::OutOfBounds, "index is negative",
{
my $A := self.defaultmatrix3x3();
$A.row_scale(-1, 1);
});
}

method test_METHOD_row_scale_BOUNDS() {
assert_throws(Exception::OutOfBounds, "index is negative",
{
my $A := self.defaultmatrix3x3();
$A.row_scale(7, 1);
});
}
}

0 comments on commit 95996f9

Please sign in to comment.