Craig DeForest <[email protected]> writes:
> On Oct 11, 2013, at 3:27 PM, Dima Kogan <[email protected]> wrote:
>> Craig DeForest <[email protected]> writes:
>>> But the code isn't really unsafe under less vigorous optimizing
>>> compilers -- the problem is that whoever wrote that (Tuomas?) used a
>>> massive autogenerated collection of similar-but-slightly-different
>>> structs to simulate dynamically allocated arrays, with a superclass
>>> generic struct that is used to access most of them. The memory is
>>> out-of-bounds for the type to which the struct is cast (the generic
>>> pdl_trans), but the bounds are maintained with runtime variables.
>>
>> Wait... what? I don't quite understand. The code here is fairly
>> unambiguous. We have a
>>
>> struct pdl_trans
>>
>> object and we're trying to access data that's out of bounds. What we
>> tell the compiler it's very clear, unless I'm missing something. Are you
>> saying that the
>>
>> struct pdl_trans*
>>
>> we have here isn't actually pointing to a pdl_trans, but rather to
>> something different that sorta looks like a pdl_trans, but has more
>> elements?
>
>
> Yes, that is exactly right. There are pdl_trans_<foo> objects all over
> the place, autogenerated by PP.pm. They all use that PDL_TRANS_START
> macro to ensure they have the same overall data structure but
> different static autoallocated pdls arrays. They are cast to
> (pdl_trans *) before some of the general purpose manipulations.
Aha. Thanks for confirming this. You are right, the luck that made the
code work was "compile-time" luck, not run-time. I.e. the compiler was
generating "correct" code even though it was being lied to.
The struct pdl_trans definition had the variable-sized array element in
the middle, which means that the variables following this element were
firmly fixed in their memory location. Because of this the compiler
assumed that when we said pdl[1], we MEANT this and NOT pdl[]. I guess
we never accessed those following elements via the generic pdl_trans. If
we tried to access them, we'd see garbage.
If we reorder the data in the pdl_trans to put the variable-length
member at the end, then the compiler no longer assumes that pdl[1] is
what we truly mean, and correct code is generated once again. I can't
find any justification of this anywhere, so I don't want to assume that
this would continue to work in a future gcc release. C99 has a flexible
array construct that exists to handle just the case we're seeing. Using
this construct the original code
#define PDL_TRANS_START(np) \
int magicno; \
short flags; \
pdl_transvtable *vtable; \
void (*freeproc)(struct pdl_trans *); /* Call to free this \
(means whether malloced or not) */ \
pdl *pdls[np]; /* The pdls involved in the transformation */ \
int bvalflag; /* required for binary compatability even if
WITH_BADVAL=0 */ \
int has_badvalue; \
double badvalue; \
int __datatype
would become
#define PDL_TRANS_START(np) \
int magicno; \
short flags; \
pdl_transvtable *vtable; \
void (*freeproc)(struct pdl_trans *); /* Call to free this \
(means whether malloced or not) */ \
int bvalflag; /* required for binary compatability even if
WITH_BADVAL=0 */ \
int has_badvalue; \
double badvalue; \
int __datatype;
pdl *pdls[] /* The pdls involved in the transformation */
Here I moved the trailing variables up, and marked the pdl array as
flexible by not giving it a size. This builds and produces correct code
in gcc4.4 - gcc4.8. I haven't tried others. Do we have an official list
of supported compilers? Can somebody test this construct on other
compilers? I'm mostly just concerned about MSVC, since it's known to be
unfriendly to C99.
If this construct works in MSVC, then this is the right solution. An
added advantage is that when pdl_trans is defined in this way, the
compiler would not allow it to be instantiated; it becomes a "pure
virtual class" in a way.
The proposed code lives in the 'test_flexible_array' branch.
dima
_______________________________________________
Perldl mailing list
[email protected]
http://mailman.jach.hawaii.edu/mailman/listinfo/perldl
