From: SourceForge.net <no...@so...> - 2006-08-14 16:37:54
|
Bugs item #1518273, was opened at 2006-07-06 21:22 Message generated for change (Comment added) made by stsp You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1518273&group_id=599 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: msc51(8051) target Group: None Status: Open Resolution: None Priority: 5 Submitted By: Stas Sergeev (stsp) Assigned to: Maarten Brock (maartenbrock) Summary: good code rejected with strange error Initial Comment: Hi. The following code (attached) doesn't compile: --- #define offsetof(TYPE, MEMBER) ((int) &((TYPE *)0)->MEMBER) struct e { unsigned char a; }; struct x { unsigned char b; struct e c[0]; }; unsigned char r(unsigned char n) { return offsetof(struct x, c[n]); } unsigned char main() { return r(5); } --- $ sdcc --std-sdcc99 off.c off.c:14: error 151: cannot use generic pointer <null> to initialize <null> ---------------------------------------------------------------------- >Comment By: Stas Sergeev (stsp) Date: 2006-08-14 20:37 Message: Logged In: YES user_id=501371 A further observation: changing the function r() in my examples to a macro makes the problem to go away. The following compiles fine: --- struct x { unsigned char b,k,l; }; #define r(n) \ ((int)(((struct x *)0) + n)) unsigned char main() { return r(5); } --- Obviously, the difference is that with the macro, the expression is evaluates compile-time. AFAICT (correct me if I am wrong) sdcc does the constant folding on the icode level. So I assume the icode generation is correct, and this is just a codegen bug. Since this all compiles well when the constant folding is possible, but not otherwise, I think it is obviously a bug that have to be fixed. Be it intentional, it wouldn't compile even being a macro. If I manage to code up the patch that will allow such a pointer arith but will complain on an access attempts, unless someone else is going to work on that, will it be considered for inclusion? I myself am quite happy with my current fix, but it would not be a good solution for me to maintain the custom patches, so I can try to code up a new one if there is a hope it will be included. ---------------------------------------------------------------------- Comment By: Stas Sergeev (stsp) Date: 2006-08-11 08:17 Message: Logged In: YES user_id=501371 > If x is a simple > struct containing only char's a,b,c it compiles too. Hmm, no? In my new test-case that struct exactly contains only a,b,c and doesn't compile. It compiles for me only if it has 1-byte size, i.e. contains only "a", or only c[1]. Flexible arrays do not seem to be involved in my case, only the structure size matters it seems. ---------------------------------------------------------------------- Comment By: Maarten Brock (maartenbrock) Date: 2006-08-11 00:23 Message: Logged In: YES user_id=888171 Somehow the flexible array member is involved too. If I give the array c length 1 it compiles ok. If x is a simple struct containing only char's a,b,c it compiles too. struct x { unsigned char a,b,c; } Or even: struct x { unsigned char b; unsigned char c[1]; } But not: struct x { unsigned char b; unsigned char c[0]; } This needs a deeper investigation. ---------------------------------------------------------------------- Comment By: Maarten Brock (maartenbrock) Date: 2006-08-10 23:47 Message: Logged In: YES user_id=888171 Stas, The question is not if it is illegal. The C standard assumes Von Neumann architecture and not Harvard and that is the source of the problem here. If SDCC only generates an error upon dereferencing an unknown storage class pointer chances are you get overwhelmed with the same error if you do so. But it is a choice we can make. I want some input from the other developers on this one. In the meantime I'll fix/hack the stddef.h so that at least works. Maarten ---------------------------------------------------------------------- Comment By: Stas Sergeev (stsp) Date: 2006-08-10 22:58 Message: Logged In: YES user_id=501371 And so I was able to reduce the test-case to this one (attached): --- struct x { unsigned char a,b,c; }; unsigned char r(unsigned char n) { return ((int)(((struct x *)0)+n)); } unsigned char main() { return r(5); } --- Do you think this code is illegal too? I was assuming this works, but it appeared not... ---------------------------------------------------------------------- Comment By: Stas Sergeev (stsp) Date: 2006-08-10 22:47 Message: Logged In: YES user_id=501371 Actually, the "dereference" was probably not the right term here. My point is that the pointer should require to have a storage class only as soon as the memory is read or written via that pointer. As long as the pointer is used only for the pointer arithmetic, it can remain "generic". From that point of view, I see "(int)&gptr->memb" being identical to "(int)(gptr + int_var)". So either both should work (my preference), or both should be prohibited. If you prohibit only "(int)&gptr->memb", then I wonder why "(int)(gptr + int_var)" should still work. ---------------------------------------------------------------------- Comment By: Stas Sergeev (stsp) Date: 2006-08-10 22:21 Message: Logged In: YES user_id=501371 I've googled out something relevant, that supports my point of view: http://c0x.coding-guidelines.com/6.5.3.3.html --- 1104 84) Thus, &*E is equivalent to E (even if E is a null pointer), and &(E1[E2]) to ((E1)+(E2)) --- It doesn't explicitly say about ptr->memb, but I think ptr[offs] is the same thing, and it explicitly says that &(E1[E2]) is just a pointer arithmetic, which I read as "not a dereference". It also explicitly mentions that &*E (is not a deref and) is safe even in case E==NULL. So I think that quote covers our case pretty well. The current offsetof macro does only the pointer arithmetic, which is safe and must always work. It doesn't do any unsafe dereferences. That's my understanding. ---------------------------------------------------------------------- Comment By: Stas Sergeev (stsp) Date: 2006-08-10 20:44 Message: Logged In: YES user_id=501371 > problem seems to be that SDCC does not know which memory > type to choose when casting a number to a generic pointer. Yes, that's what I've figured out too. > For normal compilation that is a good thing I think and we > should not start assuming anything and using a default > memory space. I think it is good to have an error when such a pointer is dereferenced, but not when it is only created. I think creating such a pointers should be perfectly legal. Unfortunately, I am not familiar with the C standard enough to say whether or not the constructs like &ptr->member are a dereferences or not. I think they are not (the value is never read or written), so I think sdcc should not complain. But I need to dig the standards to say for sure, unless someone else knows this right away. :) > A good solution is to > include a memory type in the cast to pointer like this: This is only a good solution if, and I think you imply it is, the &ptr->member is really a dereference. If it is not a dereference, then I am going to assert that the offsetof macro must not be altered, as being perfectly correct. Besides, adding an explicit storage class simply makes no sense there - "code" will work similar to "xdata" etc, so after all it gets unused and therefore smells like a bad hack. But I see your point too. I think we only disagree on whether or not it is a dereference. By the way, thinking about it from that point of view, my patch is not correct. That patch allows a real dereferences, which is not good, as you pointed out. Whether or not it would be easy for me to code up the patch that will allow only that "pseudo-dereferences", is what I doubt in. Hmm... ---------------------------------------------------------------------- Comment By: Maarten Brock (maartenbrock) Date: 2006-08-10 16:55 Message: Logged In: YES user_id=888171 Stas, I haven't looked deeply into your patch yet, but the problem seems to be that SDCC does not know which memory type to choose when casting a number to a generic pointer. For normal compilation that is a good thing I think and we should not start assuming anything and using a default memory space. However in this special case where we always take the address of the created pointer and cast it back to some integer type it gets in our way. A good solution is to include a memory type in the cast to pointer like this: #define offsetof(s,m) (size_t)&(((s __code *)0)->m) I'll see if this works for all ports and adapt stddef.h accordingly. Maarten ---------------------------------------------------------------------- Comment By: Stas Sergeev (stsp) Date: 2006-08-10 08:25 Message: Logged In: YES user_id=501371 Hmm, what category is this - "msc51"? Was it meant to be "mcs51"? A bug? :) I changed the category, as per my analises it is a codegen bug. ---------------------------------------------------------------------- Comment By: Stas Sergeev (stsp) Date: 2006-08-10 08:19 Message: Logged In: YES user_id=501371 Thanks for the pointer, however, removing the macro from my test example and including stddef.h doesn't change anything at all. Your macro works perfectly, but sdcc does not. :-( This is not a parentheses bug - my macro was also taken from a reliable source. ---------------------------------------------------------------------- Comment By: Maarten Brock (maartenbrock) Date: 2006-08-09 23:42 Message: Logged In: YES user_id=888171 Stas, According to the standard this macro should be defined in stddef.h so I put it there already 2 years ago. AFAIK it works perfectly. Only I used another pair of parentheses before taking the address. I would have to look up if your code should give the same result. Maarten ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1518273&group_id=599 |