Prev: Is this ambiguous ? Templates parameter deduction.
Next: member iterator - request for comment
From: nmm1 on 20 Jun 2010 04:57

I have some code where executing some completely irrelevant calls in
cstdio causes gslice to throw a segmentation fault; a previous code
(which was buggy) caused the use of gslice to cause cerr/cout to
throw away all their output! Now, the most likely explanation is
that I have missed some critical constraint on the use of gslice
arrays, but blowed if I can spot one.

This was under Linux, and I shall retry under Microsoft tomorrow
(if our local systems have enough compiler support). The original
program was solid with checking, so it's not a simple bounds error.
The appended code is really stripped down and fails if the argument
count is greater than 1 but not otherwise.

Any suggestions welcome.

Regards,
Nick Maclaren.



life.cpp:

using namespace std;
#include <valarray>
#include <cstdio>



class Matrix {
private:
valarray<signed char> *base;
int stride, offset;
public:
Matrix (int lwb1, int lim1, int lwb2, int lim2);
gslice_array<signed char> slice (int lwb1, int lim1, int lwb2, int lim2);
};

Matrix::Matrix (int lwb1, int lim1, int lwb2, int lim2) {
this->base = new valarray<signed char>((size_t)((lim1-lwb1)*(lim2-lwb2)));
this->stride = lim2-lwb2;
this->offset = lwb1*this->stride+lwb2;
}

gslice_array<signed char> Matrix::slice (int lwb1, int lim1,
int lwb2, int lim2) {
static valarray<size_t> length(2), stride(2);
length[0] = lim1-lwb1;
length[1] = lim2-lwb2;
stride[0] = this->stride;
stride[1] = 1;
return (*this->base)[gslice(lwb1*this->stride+lwb2-offset,length,stride)];
}

typedef class Matrix matrix;



int main (int argc, char *argv[]) {
int size = 8, i, n;
FILE *file;
matrix *total = NULL;

file = fopen("beacon.life","r");
total = new matrix(-1,size+1,-1,size+1);
(*total).slice(-1,0,-1,size+1) = 0;
(*total).slice(-1,size+1,-1,0) = 0;
(*total).slice(size,size+1,-1,size+1) = 0;
(*total).slice(-1,size+1,size,size+1) = 0;

if (argc > 1) {
for (n = 0; n < size; ++n) {
for (i = 0; i < size; ++i) getc(file);
getc(file);
}
getc(file);
fclose(file); /* This rarely detects errors, anyway */
}

(*total).slice(-1,0,-1,size+1) = 0;
(*total).slice(-1,size+1,-1,0) = 0;
(*total).slice(size,size+1,-1,size+1) = 0;
(*total).slice(-1,size+1,size,size+1) = 0;

return 0;
}



beacon.life:

8
.........
.........
...**....
...**....
.....**..
.....**..
.........
.........

--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

From: Ulrich Eckhardt on 22 Jun 2010 08:54
nmm1(a)cam.ac.uk wrote:
> I have some code where executing some completely irrelevant calls in
> cstdio causes gslice to throw a segmentation fault; a previous code
> (which was buggy) caused the use of gslice to cause cerr/cout to
> throw away all their output!

That sounds like severely buggy code.

> class Matrix {
> private:
> valarray<signed char> *base;
> int stride, offset;
> public:
> Matrix (int lwb1, int lim1, int lwb2, int lim2);
> gslice_array<signed char> slice (int lwb1, int lim1, int lwb2, int
> lim2);
> };

The first thing I notice here is that you don't handle/prevent copying and
assignment, not even a destructor for cleanup!

> Matrix::Matrix (int lwb1, int lim1, int lwb2, int lim2) {
> this->base = new valarray<signed
> char>((size_t)((lim1-lwb1)*(lim2-lwb2))); this->stride = lim2-lwb2;
> this->offset = lwb1*this->stride+lwb2;
> }



> gslice_array<signed char> Matrix::slice (int lwb1, int lim1,
> int lwb2, int lim2) {
> static valarray<size_t> length(2), stride(2);

A function-static object like this here is just asking for trouble...

> typedef class Matrix matrix;

Why?

> matrix *total = NULL;
[...]
> total = new matrix(-1,size+1,-1,size+1);

Why use dynamic allocation here? This is not Java!

> (*total).slice(-1,0,-1,size+1) = 0;

For your info: "(*total).xyz" is usually written as "total->xyz" instead, as
you otherwise also use.

Sorry, but especially the careless use of raw pointers is a good way to
shoot yourself in the foot. Fix that and your program might run already.

Uli

--
Sator Laser GmbH
Geschäftsführer: Thorsten Föcking, Amtsgericht Hamburg HR B62 932


[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

From: nmm1 on 22 Jun 2010 23:06
In article <cfd6f7-4sg.ln1(a)satorlaser.homedns.org>,
Ulrich Eckhardt <eckhardt(a)satorlaser.com> wrote:

Thanks for your response. I am afraid that we are considerably at
cross-purposes. The code that I had left was NOT intended to be
an example of good style - for heaven's sake, I had removed all
of the comments! This was the result of a proper debugging process,
and iteration on the actual use to try and locate why the program
worked perfectly sometimes and not others. Please don't bother
telling me about the usual reasons, as I do know about them.

I was asking about whether there are any hidden (or at least badly
documented) constraints or assumptions on the use of the STL that I
had fallen foul of. Specifically about whether valarray and
gslice_array objects have rvalue semantics (i.e. can be copied and
returned from functions). The standard does not make that clear
in their specification, though it may be answered as a generic rule
somewhere in its 1325 pages ....

>> I have some code where executing some completely irrelevant calls in
>> cstdio causes gslice to throw a segmentation fault; a previous code
>> (which was buggy) caused the use of gslice to cause cerr/cout to
>> throw away all their output!
>
>That sounds like severely buggy code.

Do you mean in the STL implementation or my code? If the former,
I am a bit surprised, as gcc is heavily used. If the latter, I
cannot see what, and rather doubt it, though one can never be sure.

The test works perfectly under Visual Studio, incidentally.

The really crazy thing is that it works until it crashes, deep in
the valarray code. And, yes, of course, there was consistency
checking and diagnostics in the original to prove that - I removed
them for clarity in the example. It looks like I am triggering a
bug that causes the valarray code to corrupt itself.

>> class Matrix {
>> private:
>> valarray<signed char> *base;
>> int stride, offset;
>> public:
>> Matrix (int lwb1, int lim1, int lwb2, int lim2);
>> gslice_array<signed char> slice (int lwb1, int lim1, int lwb2, int
>> lim2);
>> };
>
>The first thing I notice here is that you don't handle/prevent copying and
>assignment, not even a destructor for cleanup!

No, deliberately. As far as copying and assignment go, what does the
default do wrong that I need to fix up? I positively don't want deep
copying semantics, so a simple structure copy is precisely right.

My program doesn't have a destructor, because its original used a
valarray object (not a pointer to one) in the Matrix class and,
again, the default destructor semantics are precisely what I want.
Unless, of course, they are broken ....

>> gslice_array<signed char> Matrix::slice (int lwb1, int lim1,
>> int lwb2, int lim2) {
>> static valarray<size_t> length(2), stride(2);
>
>A function-static object like this here is just asking for trouble...

Why? If you look, those objects are NOT returned from the function,
and are used solely as read-only arguments to a constructor - i.e.
are simply scratch values. One of the things that the standard does
not state is whether the arguments to gslice are required to be
persistent or not - I assumed not. And, if they are not, there is
nothing wrong with this sort of use.

>> typedef class Matrix matrix;
>
>Why?

I forget exactly why now, but it helped to clarify the use of the
class. Something to do with namespaces, if I recall. It's not
important, anyway - or shouldn't be. If I can resolve the problem
that has defeated me, I will be revisiting it, but I can't manage
to get returning a gslice_array object to behave reliably.

>> matrix *total = NULL;
>[...]
>> total = new matrix(-1,size+1,-1,size+1);
>
>Why use dynamic allocation here? This is not Java!

Because the implementation crashed without it! That is probably
related to my question, because what seems to be failing is the
reliance on rvalue semantics of valarray and gslice_array objects.
Unfortunately, the standard doesn't state whether they do or don't
have them, and one has to guess the answer.

>> (*total).slice(-1,0,-1,size+1) = 0;
>
>For your info: "(*total).xyz" is usually written as "total->xyz" instead, as
>you otherwise also use.

Yes, I know. You have noticed one of the relics of me changing from
assuming rvalue semantics of the valarray object to using dynamic
allocation. A simple global exchange on the text of the program!

>Sorry, but especially the careless use of raw pointers is a good way to
>shoot yourself in the foot. Fix that and your program might run already.

Er, no. See above for why.


Regards,
Nick Maclaren.

--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

From: Daniel Krügler on 23 Jun 2010 21:45
On 20 Jun., 21:57, n...(a)cam.ac.uk wrote:
> I have some code where executing some completely irrelevant calls in
> cstdio causes gslice to throw a segmentation fault; a previous code
> (which was buggy) caused the use of gslice to cause cerr/cout to
> throw away all their output! Now, the most likely explanation is
> that I have missed some critical constraint on the use of gslice
> arrays, but blowed if I can spot one.
>
> This was under Linux, and I shall retry under Microsoft tomorrow
> (if our local systems have enough compiler support). The original
> program was solid with checking, so it's not a simple bounds error.
> The appended code is really stripped down and fails if the argument
> count is greater than 1 but not otherwise.
>
> Any suggestions welcome.

I checked your code analytically and I found only
little, nevertheless, I hope it might help you.
Let me first ask a question: According to which
standard (C++98, C++03, C++0x) did you compile your
code? (See below, why)?

> life.cpp:
>
> using namespace std;

I'm not kidding, but did you really write this using-
directive *in front* of the std headers and w/o any
previous namespace std definition? If so, your
compiler must be very tolerant (it should not even
be allowed in C++98 to use a using-directive w/o
a previous namespace definition) and I strongly suggest
to move this directive after the include's.

> #include <valarray>
> #include <cstdio>
>
> class Matrix {
> private:
> valarray<signed char> *base;
> int stride, offset;
> public:
> Matrix (int lwb1, int lim1, int lwb2, int lim2);
> gslice_array<signed char> slice (int lwb1, int lim1, int lwb2, int lim2);
> };
>
> Matrix::Matrix (int lwb1, int lim1, int lwb2, int lim2) {
> this->base = new valarray<signed char>((size_t)((lim1-lwb1)*(lim2-lwb2)));
> this->stride = lim2-lwb2;
> this->offset = lwb1*this->stride+lwb2;
> }
>
> gslice_array<signed char> Matrix::slice (int lwb1, int lim1,
> int lwb2, int lim2) {
> static valarray<size_t> length(2), stride(2);
> length[0] = lim1-lwb1;
> length[1] = lim2-lwb2;
> stride[0] = this->stride;
> stride[1] = 1;
> return (*this->base)[gslice(lwb1*this->stride+lwb2-offset,length,stride)];
> }

As of C++98/03 this code should be ill-formed, because
std::gslice_array is not copyable. You should look
carefully at this, because if the compiler accepts
this, it might just work or - with some bad luck - it
might produce something nasty. This restriction has
been removed as of C++0x, though.

Let me note that I also looked for some particular
problem which might hit you - at least theoretically:
The standard says in all three versions that you meet
undefined behaviour, if the non-const overload of
valarray's operator[](const gslice&) sees degenerate
indices. I did calculate your indices by hand, but
I didn't find a violation of this restriction.

I also checked for normal boundary violations but I
didn't find any. This doesn't mean that there aren't any,
but I want just to give you the best information at
your hands that are available to me.

Last but not least I tried your code with the Visual
Studio 2008 compiler with a lot of runtime diagnostic
activated, but did not hit your problem (But I needed to
move your using directive after the std header includes
to get it accepted by the compiler).

Sorry, this is all I found. My best guess is that this
is indeed a defect in your valarray implementation.

Good luck & Greetings from Bremen,

Daniel Kr�gler


--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

From: nmm1 on 1 Jul 2010 00:03
In article <b17b6daf-48e2-464d-bcb5-f1e0c71d28f6(a)b35g2000yqi.googlegroups.com>,
=?ISO-8859-1?Q?Daniel_Kr=FCgler?= <daniel.kruegler(a)googlemail.com> wrote:
>
>> I have some code where executing some completely irrelevant calls in
>> cstdio causes gslice to throw a segmentation fault; a previous code
>> (which was buggy) caused the use of gslice to cause cerr/cout to
>> throw away all their output! Now, the most likely explanation is
>> that I have missed some critical constraint on the use of gslice
>> arrays, but blowed if I can spot one.
>>
>> ...
>
>I checked your code analytically and I found only
>little, nevertheless, I hope it might help you.
>Let me first ask a question: According to which
>standard (C++98, C++03, C++0x) did you compile your
>code? (See below, why)?

C++0x, I am afraid :-( But I checked against examples, tutorials etc.
on the Web and a couple of books to avoid using too much new stuff. Not
ideal, I know, and I really should get hold of a copy of C++03.

>> life.cpp:
>>
>> using namespace std;
>
>I'm not kidding, but did you really write this using-
>directive *in front* of the std headers and w/o any
>previous namespace std definition? If so, your
>compiler must be very tolerant (it should not even
>be allowed in C++98 to use a using-directive w/o
>a previous namespace definition) and I strongly suggest
>to move this directive after the include's.

Thanks for the advice. I can't find that restriction at all in C++0x,
and I can think of no reason that it's necessary, but it costs
nothing to change.

>As of C++98/03 this code should be ill-formed, because
>std::gslice_array is not copyable. You should look
>carefully at this, because if the compiler accepts
>this, it might just work or - with some bad luck - it
>might produce something nasty. This restriction has
>been removed as of C++0x, though.

That is almost certainly the problem, then. It's thoroughly shoddy to
generate code that doesn't work with no comment, of a nature that can
be detected statically.

None of the gslice tutorials and descriptions I saw mentioned that
gslice_array is not copyable, which isn't good of them, either.

>The standard says in all three versions that you meet
>undefined behaviour, if the non-const overload of
>valarray's operator[](const gslice&) sees degenerate
>indices. I did calculate your indices by hand, but
>I didn't find a violation of this restriction.
>
>I also checked for normal boundary violations but I
>didn't find any. This doesn't mean that there aren't any,
>but I want just to give you the best information at
>your hands that are available to me.

That is very kind - thanks very much. The reason that there aren't
any is that my base code contains checking against both of those;
I removed it for the example I posted.


Regards,
Nick Maclaren.

--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

 |  Next  |  Last
Pages: 1 2
Prev: Is this ambiguous ? Templates parameter deduction.
Next: member iterator - request for comment