I have been following the "cannot take address of packed record" thread and now I have a similar problem.
I perfectly understand that doing a strcopy is an implicit conversion, but I think that the program gets carried away during checks :-)
I had a VERY complex data structure and it took me quite a while to reduce it to the minimum, please bear with me if it's anyway a bit intricate.
let's have the following definitions:
type
char_arr = array[1..30] of char;
smallrec = RECORD arr : char_arr; END;
largerec = packed RECORD one_rec : smallrec; many_rec : array[1..5] of smallrec; one_array : char_arr; many_arrays : array[1..5] of char_arr; END;
VAR r:largerec; c:char_arr;
The largerec record is packed, and contains an assortment of structures.
A lot of assignments are allowed, like:
r.one_array :=c; r.one_rec.arr :=c; r.many_arrays[1] :=c;
This single one is not allowed: r.many_rec[1].arr:=c;
BTW, assigning a single char is allowed: r.many_rec[1].arr[1]:=#0;
So I guess it must be a check that deals with implicits type conversions.
but:
1) I am not converting strings, I am assigning a char_arr to another char_arr 2) if the "r.one_rec.arr :=c" assignment is allowed, I cannot see how the "r.many_rec[1].arr:=c;" can be more "dangerous" in addressing mode.
I am not trying to be fussy just for the fun of it, but my structure is (as life goes :-) just the single variant that will not work.
I am porting an old source code to gpc, while keeping the data file compatibility, so I will explore the unpack-access-pack solution, (hoping that it will not degrade performances too much)
BTW (and here I go into newbie mode) how can I check if there is after all any physical difference between a packed and an un-packed record? Is there something like a length() to measure a record's size in memory??
Also, do I understand correctly that the proposed solution:
{$pack-struct, maximum-field-alignment 8} foo = record sig : array [ 0..3 ] OF Char; end; {$no-pack-struct, maximum-field-alignment 0}
would change the structure of a "file of foo" ?
Thanks! Francesco Bonomi
Francesco Bonomi Si.lab s.r.l. Via Citille 13 50022 Greve in Chianti FI- Italy
Tel +39 - 055 8544805 Fax +39 - 055 8547418
http//:www.silab.it
Francesco Bonomi wrote:
I have been following the "cannot take address of packed record" thread and now I have a similar problem.
I perfectly understand that doing a strcopy is an implicit conversion, but I think that the program gets carried away during checks :-)
I had a VERY complex data structure and it took me quite a while to reduce it to the minimum, please bear with me if it's anyway a bit intricate.
... snip ...
AFAICS you have no components smaller than a byte in the first place. If you are operating on a byte addressing machine, why bother with the PACKED descriptor? You should simply arrange your records so that the larger items, such as integers, precede the smaller ones, such as chars.
Der friends,
all the comments about possible workarounds to the bug are indeed valid, but I cannot use them because I have to port a large existing application to gpc while keeping binary compatability with existing files.
This would allow me to have a smoother transition from the current program (Oregon Pascal under SCO) to the new program (gpc under Linux). I mean, I can keep the working program running while I port small parts at a time to implement new features up to when I will finish and I will be able to abandon the old program.
As I said in my second message, the structure I am working with is quite complex and I am afraid to break it :-)
For the moment, the solution I think I will have to follow is to change all the instructions like I said in my second message: As I can assign to the whole record but not to its components, I will copy the whole record to a temporary variable, assign the value to its component and then put all the recors back to its place.
DATA.IDX[H1].KEY:= <array of char>;
becomes
TMP:=DATA.IDX[H1]; TMP.KEY:=<array of char>; DATA.IDX[H1]:=TMP;
Very boring and rather un-elegant but it's acceptable for the compiler :-)
Ciao, thanks to all Francesco Bonomi
Si.lab s.r.l. Via Citille 13 50022 Greve in Chianti FI- Italy
Tel +39 - 055 8544805 Fax +39 - 055 8547418
http//:www.silab.it
On 5 Feb 2004 at 18:37, Francesco Bonomi wrote:
Der friends,
all the comments about possible workarounds to the bug are indeed valid, but I cannot use them because I have to port a large existing application to gpc while keeping binary compatability with existing files.
This would allow me to have a smoother transition from the current program (Oregon Pascal under SCO) to the new program (gpc under Linux). I mean, I can keep the working program running while I port small parts at a time to implement new features up to when I will finish and I will be able to abandon the old program.
As I said in my second message, the structure I am working with is quite complex and I am afraid to break it :-)
[...]
I once had a similar problem, and, with Frank's help, I managed to find a relatively easy solution - which was, instead of doing:
type foo = packed record { blah, blah, fieldx, fieldy, etc. } end;
I would use the pack-struct directive both before and after the type declaration, like this: {$ifdef __GPC__}{$pack-struct, maximum-field-alignment 8} {$endif} type foo = record { blah, blah, fieldx, fieldy, etc. } end; {$ifdef __GPC__}{$no-pack-struct, maximum-field-alignment 0}{$endif}
Seems to achieve the same result, without using "packed record".
Best regards, The Chief -------- Prof. Abimbola A. Olowofoyeku (The African Chief) web: http://www.bigfoot.com/~african_chief/
Francesco Bonomi wrote:
[snip]
This single one is not allowed: r.many_rec[1].arr:=c;
Francesco Bonomi wrote: [snip]
This assignment fails:
DATA.IDX[H1].KEY:= <array of char>;
[snip]
I cannot really understand why (and if) this should be safer (from the compiler's poin of view) than doing DATA.IDX[H1].KEY:= <array of char>;
Any thought?
Regardless of all the jaw flapping about how one shouldn't use packed data types, the compiler is broke in this area.
To investigate your problem, I tried a few test programs to see what might be wrong. To keep all the various non-standard extensions out of the picture, I compiled in Extended Pascal compliance mode. For the following program:
program CharArrayTest(input, output); {$extended-pascal} type
char_arr = array[1..30] of char;
smallrec = RECORD arr : char_arr; END;
largerec = packed RECORD one_rec : smallrec; many_rec : array[1..5] of smallrec; one_array : char_arr; many_arrays : array[1..5] of char_arr; END;
VAR r: largerec; c, c1: char_arr; aSmallrec: smallrec; aSmallrecArray: array[1..5] of smallrec; i: integer;
begin for i := 1 to 30 do c[i] := chr(i); c1 := c; aSmallrec.arr := c; aSmallrecArray[1].arr := c; end.
I got these totally bogus errors when trying to compile:
CharArrayTest.pas: In main program: CharArrayTest.pas:28: error: using unpacked character arrays as strings is an extension CharArrayTest.pas:28: error: of Borland Pascal CharArrayTest.pas:28: error: using unpacked character arrays as strings is an extension CharArrayTest.pas:28: error: of Borland Pascal CharArrayTest.pas:29: error: using unpacked character arrays as strings is an extension CharArrayTest.pas:29: error: of Borland Pascal CharArrayTest.pas:29: error: using unpacked character arrays as strings is an extension CharArrayTest.pas:29: error: of Borland Pascal CharArrayTest.pas:30: error: using unpacked character arrays as strings is an extension CharArrayTest.pas:30: error: of Borland Pascal CharArrayTest.pas:30: error: using unpacked character arrays as strings is an extension CharArrayTest.pas:30: error: of Borland Pascal
The errors are totally bogus because there are abolutely no strings in the program and there is nothing requiring string semantics in the program. Per ISO 7185/10206, since there is no 'packed' token in the declaration of the char_arr type, the char_arr type is a plain old array type with char type elements. Since all the assignments have exactly the same type on the left hand and right hand sides (char type or char_arr type which is array type with char element types), they are completely assignment compatible per ISO 7185/10206 paragraph 6.4.6 a).
To get around these bogus errors, the following program uses packed to force a string type:
program PackedCharArrayTest(input, output); {$extended-pascal} type
char_arr = packed array[1..30] of char;
smallrec = RECORD arr : char_arr; END;
largerec = packed RECORD one_rec : smallrec; many_rec : array[1..5] of smallrec; one_array : char_arr; many_arrays : array[1..5] of char_arr; END;
VAR r: largerec; c, c1: char_arr; aSmallrec: smallrec; aSmallrecArray: array[1..5] of smallrec; i: integer;
begin for i := 1 to 30 do c[i] := chr(i); c1 := c; aSmallrec.arr := c; aSmallrecArray[1].arr := c; r.many_rec[1].arr := c; {compiler error but ISO7185/10206 legal} end.
And I got the same error you've been wrestling with:
PackedCharArrayTest.pas: In main program: PackedCharArrayTest.pas:31: error: cannot take address of packed record field `many_rec'
Again this is a bogus error. The assignment statememt "r.many_rec[1].arr := c;" is fully compliant with all requirements of ISO 7185/10206. The type of "r.many_rec[1].arr" is fixed-string-type with a capacity of 30 and "c" is fixed-string-type with capacity and length both 30 so per ISO 7185/10206 paragraph 6.4.6 f) and ISO 10206 paragraph 6.9.2.2 (6.8.2.2 for ISO 7185) the assignment statement is fully compliant with ISO requirements.
I will note that ISO 7185/10206 puts one extra restriction and ONLY one extra restriction on the usage of a variable of packed type (including string types which are or contain packaged array types). From ISO 10206, paragraph 6.7.3.3:
"An actual variable parameter shall not denote a component of a variable where that variable possesses a type that is designated packed. An actual variable parameter shall not denote a component of a string-type."
However, that requirement doesn't even apply to "r.many_rec[1].arr" since the "arr" field is NOT a packed type compontent. Per ISO 10206. paragraph 6.4.3.1:
"The designation of a structured-type as packed shall affect the representation in data-storage of that structured-type only; i.e., if a component is itself structured, the component's representation in data-storage shall be packed only if the type of the component is designated packed."
In other words, even though the largerec type is designated "packed", the "arr" field isn't a packed field because the smallrec type has NOT been designated "packed".
Regardless of the wisdom of using packed data types, the compiler has at least two bugs in the area of character arrays which are ISO 7185/10206 compliance failures. Therefore, those bugs need to be fix.
Russell Whitaker wrote:
- "packed array [ ] of char" is not a string.
You're mistaken on this given the context of the discussion. "packed array [1..N] of char", where N >= 1, is defined to be a fixed-string-type in ISO 10206 and a string-type (and the only string-type) in ISO 7185.
- "packed record". Most cpu's handle data on four byte boundrys.
So what? Francesco isn't complaining about performance. Francesco is complaining about the compiler rejecting a ISO 7185/10206 perfectly legal assignment statement. Francesco complaint is valid and identifies a ISO compliance failure in the compiler.
Also, compared to the baroque CDC Cyber 6600 architecture where Wirth implemented the first Pascal compiler on, today's CPU are pretty much nirvana for compiler implementors. If Wirth could get decent performace in assignment statements involving packed data structures on that machine, surely any compiler implementor worth their salt should be able to do it on today's architecures.
Gale Paeper gpaeper@empirenet.com
Gale Paeper wrote:
Regardless of all the jaw flapping about how one shouldn't use packed data types, the compiler is broke in this area.
To investigate your problem, I tried a few test programs to see what might be wrong. To keep all the various non-standard extensions out of the picture, I compiled in Extended Pascal compliance mode. For the following program:
program CharArrayTest(input, output); {$extended-pascal} type
char_arr = array[1..30] of char;
smallrec = RECORD arr : char_arr; END;
largerec = packed RECORD one_rec : smallrec; many_rec : array[1..5] of smallrec; one_array : char_arr; many_arrays : array[1..5] of char_arr; END;
VAR r: largerec; c, c1: char_arr; aSmallrec: smallrec; aSmallrecArray: array[1..5] of smallrec; i: integer;
begin for i := 1 to 30 do c[i] := chr(i); c1 := c; aSmallrec.arr := c; aSmallrecArray[1].arr := c; end.
I got these totally bogus errors when trying to compile:
CharArrayTest.pas: In main program: CharArrayTest.pas:28: error: using unpacked character arrays as strings
<snip>
The errors are totally bogus because there are abolutely no strings in the program and there is nothing requiring string semantics in the
Agreed. The following patch should fix the problem:
diff -ru tt/gpc-20030830.pp/p/types.c gpc-20030830/p/types.c --- tt/gpc-20030830.pp/p/types.c Wed Aug 13 06:32:06 2003 +++ gpc-20030830/p/types.c Fri Feb 6 05:33:44 2004 @@ -859,9 +859,13 @@ if (TREE_CODE (type) != ARRAY_TYPE || TREE_CODE (TREE_TYPE (type)) != CHAR_TYPE) return 0;
- if (error_flag && !PASCAL_TYPE_PACKED (type)) - chk_dialect ("using unpacked character arrays as strings is", B_D_PASCAL); - + if (!PASCAL_TYPE_PACKED (type)) + { + if (error_flag) + chk_dialect ("using unpacked character arrays as strings is", B_D_PASCAL); + if (co->pascal_dialect & C_E_O_PASCAL) + return 0; + } /* String type low index must be one and nonvarying according to ISO */ if (tree_int_cst_equal (TYPE_MIN_VALUE (TYPE_DOMAIN (type)), integer_one_node)) return 1;
To get around these bogus errors, the following program uses packed to force a string type:
program PackedCharArrayTest(input, output); {$extended-pascal} type
char_arr = packed array[1..30] of char;
smallrec = RECORD arr : char_arr; END;
largerec = packed RECORD one_rec : smallrec; many_rec : array[1..5] of smallrec; one_array : char_arr; many_arrays : array[1..5] of char_arr; END;
VAR r: largerec; c, c1: char_arr; aSmallrec: smallrec; aSmallrecArray: array[1..5] of smallrec; i: integer;
begin for i := 1 to 30 do c[i] := chr(i); c1 := c; aSmallrec.arr := c; aSmallrecArray[1].arr := c; r.many_rec[1].arr := c; {compiler error but ISO7185/10206 legal} end.
And I got the same error you've been wrestling with:
PackedCharArrayTest.pas: In main program: PackedCharArrayTest.pas:31: error: cannot take address of packed record field `many_rec'
Again this is a bogus error. The assignment statememt "r.many_rec[1].arr := c;" is fully compliant with all requirements of ISO 7185/10206. The type of "r.many_rec[1].arr" is fixed-string-type with a capacity of 30 and "c" is fixed-string-type with capacity and length both 30 so per ISO 7185/10206 paragraph 6.4.6 f) and ISO 10206 paragraph 6.9.2.2 (6.8.2.2 for ISO 7185) the assignment statement is fully compliant with ISO requirements.
Agreed.
I will note that ISO 7185/10206 puts one extra restriction and ONLY one extra restriction on the usage of a variable of packed type (including string types which are or contain packaged array types). From ISO 10206, paragraph 6.7.3.3:
"An actual variable parameter shall not denote a component of a variable where that variable possesses a type that is designated packed. An actual variable parameter shall not denote a component of a string-type."
However, that requirement doesn't even apply to "r.many_rec[1].arr" since the "arr" field is NOT a packed type compontent. Per ISO 10206. paragraph 6.4.3.1:
"The designation of a structured-type as packed shall affect the representation in data-storage of that structured-type only; i.e., if a component is itself structured, the component's representation in data-storage shall be packed only if the type of the component is designated packed."
In other words, even though the largerec type is designated "packed", the "arr" field isn't a packed field because the smallrec type has NOT been designated "packed".
Hmm, I do not understand what you are trying to argue here. "arr" field is not used as variable parameter, so 6.7.3.3 does not apply. The compiler message is about "many_rec", which is a packed record field (but the compiler does not see that the reference is legal).
Waldek Hebisch wrote:
Gale Paeper wrote:
[snip]
aSmallrecArray[1].arr := c; end.
I got these totally bogus errors when trying to compile:
CharArrayTest.pas: In main program: CharArrayTest.pas:28: error: using unpacked character arrays as strings
<snip> > The errors are totally bogus because there are abolutely no strings in > the program and there is nothing requiring string semantics in the
Agreed. The following patch should fix the problem:
Waldek, thanks for the timely response. While it doesn't fix the problems Francesco is having, I do appreciate that when compiler problems are discovered they are investigated and the problems get fixed commensurate with the time available and the problems' difficulty.
[snip]
smallrec = RECORD arr : char_arr; END;
[snip]
r.many_rec[1].arr := c; {compiler error but ISO7185/10206 legal} end.
And I got the same error you've been wrestling with:
PackedCharArrayTest.pas: In main program: PackedCharArrayTest.pas:31: error: cannot take address of packed record field `many_rec'
Again this is a bogus error. The assignment statememt "r.many_rec[1].arr := c;" is fully compliant with all requirements of ISO 7185/10206. The type of "r.many_rec[1].arr" is fixed-string-type with a capacity of 30 and "c" is fixed-string-type with capacity and length both 30 so per ISO 7185/10206 paragraph 6.4.6 f) and ISO 10206 paragraph 6.9.2.2 (6.8.2.2 for ISO 7185) the assignment statement is fully compliant with ISO requirements.
Agreed.
Okay. That is really the core of the problems Francesco is having. Although the complier doesn't have to statisfy any guarentees about economies of storage or efficiency of accesses, the compiler does have to handle assignment statements involving packed data structures as legal assignments.
I take it then that the problem has at least been put on the bug list to be investigated and fixed as time and circumstances permit addressing the problem?
I will note that ISO 7185/10206 puts one extra restriction and ONLY one extra restriction on the usage of a variable of packed type (including string types which are or contain packaged array types). From ISO 10206, paragraph 6.7.3.3:
"An actual variable parameter shall not denote a component of a variable where that variable possesses a type that is designated packed. An actual variable parameter shall not denote a component of a string-type."
However, that requirement doesn't even apply to "r.many_rec[1].arr" since the "arr" field is NOT a packed type compontent. Per ISO 10206. paragraph 6.4.3.1:
"The designation of a structured-type as packed shall affect the representation in data-storage of that structured-type only; i.e., if a component is itself structured, the component's representation in data-storage shall be packed only if the type of the component is designated packed."
In other words, even though the largerec type is designated "packed", the "arr" field isn't a packed field because the smallrec type has NOT been designated "packed".
Hmm, I do not understand what you are trying to argue here. "arr" field is not used as variable parameter, so 6.7.3.3 does not apply.
I was trying to agrue that there is only one case where the compiler should be issuing "cannot take address of packed record field" errors. That case being an actual packed component of a data structure being used as the actual parameter to a formal variable parameter. Since the code in question is neither an actual argument for a variable formal parameter or a packed component access, the compiler has no grounds for issuing a "cannot take address of packed record field" error.
(Regardless of the packed-ness of any data structure it is embedded in, there are no packed constraints on accessing the "arr" field since the smallrec record type was not declared packed. Accessing the individual char type elements of the "arr" field would have packed constraints since the char_arr was declared packed; however, the code in question isn't accessing the individual elements of the "arr" field so this isn't an issue.)
... The compiler message is about "many_rec", which is a packed record field (but the compiler does not see that the reference is legal).
That's the problem. The packed-ness of "many_rec" has no involvement in determining the legality of access to the "arr" component nor does it have any involvement in determining whether any of the nested components have packed type charateristics and whether or not the nested components accesses are subject to packed component usage constraints. The packed-ness of "many_rec" is completely out of the picture when it comes to legal accesses to the "arr" component so there is something really wrong with the compiler's handling of packed when it uses the packed charateristic of "many_rec" in determining the legality of accesses to the nested "arr" component.
Gale Paeper gpaeper@empirenet.com
Gale Paeper wrote: <snip>
Although the complier doesn't have to statisfy any guarentees about economies of storage or efficiency of accesses, the compiler does have to handle assignment statements involving packed data structures as legal assignments.
I take it then that the problem has at least been put on the bug list to be investigated and fixed as time and circumstances permit addressing the problem?
While I do not know when Frank put bugs on the official bug list, problems signalled on the mailing list do get attention.
The following patch should fix problems with packed records:
diff -rbu gpc-20030830.orig/p/expressions.c gpc-20030830/p/expressions.c --- gpc-20030830.orig/p/expressions.c Wed Aug 13 06:29:55 2003 +++ gpc-20030830/p/expressions.c Tue Feb 10 02:50:52 2004 @@ -2754,7 +2754,7 @@ int noconvert; { /* No default_conversion here. It causes trouble for ADDR_EXPR. */ - tree arg = xarg, addr, argtype = 0, t; + tree arg = xarg, addr, argtype = 0; enum tree_code typecode = TREE_CODE (TREE_TYPE (arg)); const char *errstring = NULL;
@@ -2851,13 +2851,14 @@ if (TREE_CODE (arg) == ARRAY_REF) { tree index, array = TREE_OPERAND (arg, 0); - +#if 0 tree t = array; if (noconvert) /* Don't let mark_addressable do this, it would complain about packed fields */ while (TREE_CODE (t) == COMPONENT_REF) t = TREE_OPERAND (t, 0); - if (!mark_addressable (t)) +#endif + if (!mark_addressable2 (array, noconvert)) return error_mark_node;
if (!noconvert && PASCAL_TYPE_PACKED (TREE_TYPE (TREE_OPERAND (arg, 0)))) @@ -2925,13 +2926,14 @@ argtype = c_build_type_variant (argtype, TREE_READONLY (arg), TREE_THIS_VOLATILE (arg));
argtype = build_pointer_type (argtype); - +#if 0 t = arg; if (noconvert) /* Don't let mark_addressable do this, it would complain about packed fields */ while (TREE_CODE (t) == COMPONENT_REF) t = TREE_OPERAND (t, 0); - if (!mark_addressable (t)) +#endif + if (!mark_addressable2 (arg, noconvert)) return error_mark_node;
if (TREE_CODE (arg) == BIT_FIELD_REF && PASCAL_TYPE_PACKED (TREE_TYPE (TREE_OPERAND (arg, 0)))) diff -rbu gpc-20030830.orig/p/gpc.h gpc-20030830/p/gpc.h --- gpc-20030830.orig/p/gpc.h Sun Sep 14 01:16:10 2003 +++ gpc-20030830/p/gpc.h Tue Feb 10 01:59:29 2004 @@ -1189,6 +1189,9 @@ extern int lvalue_p PARAMS ((tree)); #ifdef GCC_3_3 extern bool mark_addressable PARAMS ((tree)); +extern bool mark_addressable2 PARAMS ((tree, int)); +#else +extern int mark_addressable2 PARAMS ((tree, int)); #endif #ifndef EGCS extern int mark_addressable PARAMS ((tree)); diff -rbu gpc-20030830.orig/p/typecheck.c gpc-20030830/p/typecheck.c --- gpc-20030830.orig/p/typecheck.c Wed Aug 13 06:27:31 2003 +++ gpc-20030830/p/typecheck.c Tue Feb 10 01:59:38 2004 @@ -2167,12 +2167,24 @@ mark_addressable (exp) tree exp; { + return mark_addressable2 (exp, 0); +} + +#ifdef GCC_3_3 +bool +#else +int +#endif +mark_addressable2 (exp, allow_packed) + tree exp; + int allow_packed; +{ tree x = exp; while (1) switch (TREE_CODE (x)) { case COMPONENT_REF: - if (DECL_PACKED_FIELD (TREE_OPERAND (x, 1))) + if ((DECL_PACKED_FIELD (TREE_OPERAND (x, 1))) && !allow_packed) { error ("cannot take address of packed record field `%s'", IDENTIFIER_NAME (DECL_NAME (TREE_OPERAND (x, 1)))); diff -rbu gpc-20030830.orig/p/types.c gpc-20030830/p/types.c --- gpc-20030830.orig/p/types.c Wed Aug 13 06:32:06 2003 +++ gpc-20030830/p/types.c Tue Feb 10 01:59:52 2004 @@ -2283,7 +2288,7 @@ /* Don't let mark_addressable do this, it would complain about packed fields */ while (TREE_CODE (t) == COMPONENT_REF) t = TREE_OPERAND (t, 0); - if (!mark_addressable (t)) + if (!mark_addressable2 (t, 1)) return error_mark_node; }
Waldek Hebisch wrote:
Gale Paeper wrote:
I take it then that the problem has at least been put on the bug list to be investigated and fixed as time and circumstances permit addressing the problem?
While I do not know when Frank put bugs on the official bug list, problems signalled on the mailing list do get attention. The following patch should fix problems with packed records: [...]
Usually I put them on my copy of the bug list immediately, but I don't always update the web site quickly. Recently I was very busy with other work, so I couldn't look into the issue. Even if Waldek made the fix already, I had to do the administrative work (bring test programs in correct form, write entires for the ChangeLog and (fixed :-) bug list), and I wanted to check for myself that everything is correct. I've done this now (fb[12].pas, gale[34].pas).
BTW, Francesco Bonomi wrote:
BTW (and here I go into newbie mode) how can I check if there is after all any physical difference between a packed and an un-packed record? Is there something like a length() to measure a record's size in memory??
SizeOf
Frank