[TinyCC] sizeof of element of struct returned by function
 

Hello,

Is following code correct (ANSI C89) ?
----
01: #include <stdio.h>
02: #include <stdlib.h>
03: struct { char array1 [100]; } * fun1(int x1);
04: struct { char array2 [200]; } fun2(int x2);
05: int main(void) {
06: printf("sizeof(array1) = %lu\n", (unsigned long)(sizeof(fun1(23)-
>array1)));
07: printf("sizeof(array2) = %lu\n", (unsigned long)
(sizeof(fun2(45).array2)));
08: return EXIT_SUCCESS;
09: }
----

If NO then rest of this post may be discarded.

If YES then it seems that I have found an issue
with compiler (tcc version 0.9.26).

Following example is compiled without any error/warning,
but final application generates "Segmentation Fault" in line 31.

I would like to have confirmation about correctness of this code.

--[beg]-----------------------------------------------------
C:\tcc> gawk '{printf("%02u: %s\n", NR, $0);}' main.c
01: #include <stdio.h>
02: #include <stdlib.h>
03: #include <string.h>
04: typedef struct {
05: void * ptr;
06: } bar_t;
07: typedef struct {
08: bar_t bar;
09: int * iptr;
10: } foo_t;
11: bar_t * fun(foo_t * foo_ptr) {
12: bar_t * result = NULL;
13: printf("++ fun(foo_ptr = %p)\n", (void *)foo_ptr);
14: result = &(foo_ptr->bar);
15: printf("-- fun(...) = %p\n", (void *)result);
16: return result;
17: }
18: #define TEST_MACRO(foo_ptr) \
19: ((sizeof(*((1 ? foo_ptr : foo_ptr)->iptr)) == 123) \
20: ? 12 : 34)
21: int main(void) {
22: foo_t foo;
23: memset(&foo, 0, sizeof(foo));
24: foo.bar.ptr = &foo;
25: printf("Hello World\n");
26: printf("foo.bar.ptr = %p\n", foo.bar.ptr);
27: printf("foo.iptr = %p\n", (void *)foo.iptr);
28: printf("fun(&foo) = %p\n", (void *)fun(&foo));
29: printf("fun(&foo)->ptr = %p\n", fun(&foo)->ptr);
30: printf("+++++++++++\n");
31: printf("%d\n", TEST_MACRO(((foo_t *)((fun(&foo))->ptr))));
32: printf("-----------\n");
33: return EXIT_SUCCESS;
34: }
C:\tcc>tcc -c -o main.o main.c
C:\tcc>tcc -o main.exe main.o
C:\tcc>main.exe
Hello World
foo.bar.ptr = 0012FF78
foo.iptr = 00000000
++ fun(foo_ptr = 0012FF78)
-- fun(...) = 0012FF78
fun(&foo) = 0012FF78
++ fun(foo_ptr = 0012FF78)
-- fun(...) = 0012FF78
fun(&foo)->ptr = 0012FF78
+++++++++++
<:seg-fault:>
--[eof]-----------------------------------------------------

NOTE: Simplifying this code for example in line 19:
- ((sizeof(*((1 ? foo_ptr : foo_ptr)->iptr)) == 123) \
+ ((sizeof(*(( foo_ptr )->iptr)) == 123) \
makes that issue disappears.

Regards

--
Maciej Labanowicz

Re: [TinyCC] sizeof of element of struct returned by function
 

Maciej Labanowicz wrote:

> Is following code correct (ANSI C89) ?
> ----
> 01: #include <stdio.h>
> 02: #include <stdlib.h>
> 03: struct { char array1 [100]; } * fun1(int x1);
> 04: struct { char array2 [200]; } fun2(int x2);
> 05: int main(void) {
> 06: printf("sizeof(array1) = %lu\n", (unsigned long)(sizeof(fun1(23)-
>> array1)));
> 07: printf("sizeof(array2) = %lu\n", (unsigned long)
> (sizeof(fun2(45).array2)));
> 08: return EXIT_SUCCESS;
> 09: }
> ----

NB: Adding line numbers makes it impossible to paste as-is.
You could either remove them, or provide them as comments.

> If YES then it seems that I have found an issue
> with compiler (tcc version 0.9.26).

Yes, as far as gcc 4.6.3 can tell :-)
$ gcc -std=c89 -pedantic -Wall -Wextra x.c

However, I don't find it surprising that a compiler whose main goals
are 1) simplicity and 2) compilation speed would stumble on some
corner case.

https://en.wikipedia.org/wiki/Tiny_C_Compiler

Regards.

Re: [TinyCC] sizeof of element of struct returned by function
 

On 02/20/2013 06:54 AM, Maciej Labanowicz wrote:
....
> If YES then it seems that I have found an issue
> with compiler (tcc version 0.9.26).
>
> Following example is compiled without any error/warning,
> but final application generates "Segmentation Fault" in line 31.
>
> I would like to have confirmation about correctness of this code.
>
> --[beg]-----------------------------------------------------
> C:\tcc> gawk '{printf("%02u: %s\n", NR, $0);}' main.c
> 01: #include <stdio.h>
> 02: #include <stdlib.h>
> 03: #include <string.h>
> 04: typedef struct {
> 05: void * ptr;
> 06: } bar_t;
> 07: typedef struct {
> 08: bar_t bar;
> 09: int * iptr;
> 10: } foo_t;
> 11: bar_t * fun(foo_t * foo_ptr) {
> 12: bar_t * result = NULL;

In general, I'd recommend checking whether foo_ptr is null before trying
to dereferencing it. In this particular case, it's trivial to see to
that it's never called with a null pointer argument, but in more
realistic programs, it's a useful safety measure.

Why set 'result' to NULL? You never use 'result' until after it has
already been changed to a different value. Why not simply set it to the
correct value from the beginning?

> 13: printf("++ fun(foo_ptr = %p)\n", (void *)foo_ptr);
> 14: result = &(foo_ptr->bar);
> 15: printf("-- fun(...) = %p\n", (void *)result);
> 16: return result;
> 17: }
> 18: #define TEST_MACRO(foo_ptr) \
> 19: ((sizeof(*((1 ? foo_ptr : foo_ptr)->iptr)) == 123) \
> 20: ? 12 : 34)

In general, it's a good idea to parenthesize the arguments of a macro
wherever they appear it it's expansion. You also have some unnecessary
parenthesis. I'd re-write that as

#define TEST_MACRO(foo_ptr) \
(sizeof (*(1 ? (foo_ptr) : (foo_ptr))->iptr) == 123 ? 12 : 34)

> 21: int main(void) {
> 22: foo_t foo;
> 23: memset(&foo, 0, sizeof(foo));
> 24: foo.bar.ptr = &foo;
> 25: printf("Hello World\n");
> 26: printf("foo.bar.ptr = %p\n", foo.bar.ptr);
> 27: printf("foo.iptr = %p\n", (void *)foo.iptr);

Your memset() of foo caused every byte of foo.iptr to be 0. This is NOT
guaranteed to leave it containing a valid pointer representation. In
particular, it's not guaranteed to be a valid representation of a null
pointer value. You should not be trying to retrieve the value of
foo.iptr until it has been set to a valid pointer value; otherwise the
behavior of your program is undefined. In principle, this issue could
explain your seg-fault below, even though the seg-fault occurs much
later - that's because there are no limits on undefined behavior.
However, in practice, it's unlikely that the representation is invalid,
and even if it were, it would be extremely unlikely that it would cause
such a long-delayed failure.

> 28: printf("fun(&foo) = %p\n", (void *)fun(&foo));
> 29: printf("fun(&foo)->ptr = %p\n", fun(&foo)->ptr);
> 30: printf("+++++++++++\n");
> 31: printf("%d\n", TEST_MACRO(((foo_t *)((fun(&foo))->ptr))));

Several of the parentheses in that macro invocation are unnecessary. The
following should be equivalent:

TEST_MACRO((foo_t*)fun(&foo)->ptr)

> 32: printf("-----------\n");
> 33: return EXIT_SUCCESS;
> 34: }
> C:\tcc>tcc -c -o main.o main.c
> C:\tcc>tcc -o main.exe main.o
> C:\tcc>main.exe
> Hello World
> foo.bar.ptr = 0012FF78
> foo.iptr = 00000000
> ++ fun(foo_ptr = 0012FF78)
> -- fun(...) = 0012FF78
> fun(&foo) = 0012FF78
> ++ fun(foo_ptr = 0012FF78)
> -- fun(...) = 0012FF78
> fun(&foo)->ptr = 0012FF78
> +++++++++++
> <:seg-fault:>

I don't have any good ideas about why that's happening. I don't have
tcc; the code works under gcc.

> --[eof]-----------------------------------------------------
>
> NOTE: Simplifying this code for example in line 19:
> - ((sizeof(*((1 ? foo_ptr : foo_ptr)->iptr)) == 123) \
> + ((sizeof(*(( foo_ptr )->iptr)) == 123) \
> makes that issue disappears.

Nor do I have any idea why that would help. Sorry that I couldn't be
more helpful.
--
James Kuyper

Re: sizeof of element of struct returned by function
 

Oops, this is important issue.

After applying changes:
- ((sizeof(*((1 ? foo_ptr : foo_ptr)->iptr)) == 123) \
+ ((sizeof(*((1 ? (foo_ptr) : (foo_ptr))->iptr)) == 123) \
- memset(&foo, 0, sizeof(foo));
+ foo.iptr = NULL;
"segfault" is still present :)

Thanks for Your comments.

--
Maciej Labanowicz

Re: [TinyCC] sizeof of element of struct returned by function
 

On 2/20/2013 06:54, Maciej Labanowicz wrote:
> Hello,
>

Hello, are you going to follow up on your last thread?

> Is following code correct (ANSI C89) ?

That depends on what you mean. It depends on casts to 'unsigned long'
from 'size_t', which can be criticized.

> ----
> 01: #include <stdio.h>
> 02: #include <stdlib.h>
> 03: struct { char array1 [100]; } * fun1(int x1);
> 04: struct { char array2 [200]; } fun2(int x2);
> 05: int main(void) {
> 06: printf("sizeof(array1) = %lu\n", (unsigned long)(sizeof(fun1(23)->array1)));

You might as well have used 'fun1(0)', since there is no call:

printf(
"sizeof(array1) = %lu\n",
(unsigned long) sizeof fun1(0)->array1
);

> 07: printf("sizeof(array2) = %lu\n", (unsigned long)
> (sizeof(fun2(45).array2)));
> 08: return EXIT_SUCCESS;
> 09: }
> ----
>
> If NO then rest of this post may be discarded.
>

No. The line numbers do not belong. Everyone's got their own opinion,
but your use of line numbers causes someone wishing to copy and paste
your code to perform steps that could be avoided if the line numbers
were omitted. I also find it harder to read, personally.

> If YES then it seems that I have found an issue
> with compiler (tcc version 0.9.26).
>
> Following example is compiled without any error/warning,
> but final application generates "Segmentation Fault" in line 31.
>
> I would like to have confirmation about correctness of this code.
>
> --[beg]-----------------------------------------------------
> C:\tcc> gawk '{printf("%02u: %s\n", NR, $0);}' main.c

I beg you to drop line numbers from your posts. :)

> 01: #include <stdio.h>
> 02: #include <stdlib.h>
> 03: #include <string.h>
> 04: typedef struct {
> 05: void * ptr;
> 06: } bar_t;
> 07: typedef struct {
> 08: bar_t bar;
> 09: int * iptr;
> 10: } foo_t;
> 11: bar_t * fun(foo_t * foo_ptr) {
> 12: bar_t * result = NULL;
> 13: printf("++ fun(foo_ptr = %p)\n", (void *)foo_ptr);
> 14: result = &(foo_ptr->bar);
> 15: printf("-- fun(...) = %p\n", (void *)result);
> 16: return result;
> 17: }
> 18: #define TEST_MACRO(foo_ptr) \
> 19: ((sizeof(*((1 ? foo_ptr : foo_ptr)->iptr)) == 123) \
> 20: ? 12 : 34)

Taking this apart, the macro appears to expand to:

(E1 ? 12 : 34)

E1 is:

(E2 == 123)

E2 is:

sizeof (E3)

Unless VLAs are involved, which they do not appear to be, then E3 should
not involve any side effects.

> 21: int main(void) {
> 22: foo_t foo;
> 23: memset(&foo, 0, sizeof(foo));

Redundant parentheses for the 'sizeof' operator. Why not use the
following?:

((memset)((&foo), (0), (sizeof (foo))));

> 24: foo.bar.ptr = &foo;
> 25: printf("Hello World\n");
> 26: printf("foo.bar.ptr = %p\n", foo.bar.ptr);
> 27: printf("foo.iptr = %p\n", (void *)foo.iptr);

How do you know that 'foo.iptr' has a pointer value stored? Whether or
not your 'memset' did that is implementation-defined.

> 28: printf("fun(&foo) = %p\n", (void *)fun(&foo));
> 29: printf("fun(&foo)->ptr = %p\n", fun(&foo)->ptr);
> 30: printf("+++++++++++\n");
> 31: printf("%d\n", TEST_MACRO(((foo_t *)((fun(&foo))->ptr))));
> 32: printf("-----------\n");
> 33: return EXIT_SUCCESS;
> 34: }
> C:\tcc>tcc -c -o main.o main.c
> C:\tcc>tcc -o main.exe main.o
> C:\tcc>main.exe
> Hello World
> foo.bar.ptr = 0012FF78
> foo.iptr = 00000000
> ++ fun(foo_ptr = 0012FF78)
> -- fun(...) = 0012FF78
> fun(&foo) = 0012FF78
> ++ fun(foo_ptr = 0012FF78)
> -- fun(...) = 0012FF78
> fun(&foo)->ptr = 0012FF78
> +++++++++++
> <:seg-fault:>
> --[eof]-----------------------------------------------------
>
> NOTE: Simplifying this code for example in line 19:
> - ((sizeof(*((1 ? foo_ptr : foo_ptr)->iptr)) == 123) \
> + ((sizeof(*(( foo_ptr )->iptr)) == 123) \
> makes that issue disappears.

As mentioned before, if undefined behaviour is invoked, code changes
which should not have an apparent change in behaviour, sometimes do.

In this case though, perhaps there is a bug. C99 VLA support was just
introduced on Feb. 15th (5 days ago), so it would not be surprising.
Perhaps you could make a more minimal test case and report it to Thomas
Preud'homme.

--
- Shao Miller
--
"Thank you for the kind words; those are the kind of words I like to hear.

Cheerily," -- Richard Harter

Re: sizeof of element of struct returned by function
 

On Wed, 20 Feb 2013 06:23:44 -0800 (PST), Maciej Labanowicz
<m.labanowicz@gmail.com> wrote:

>Oops, this is important issue.
>
>After applying changes:
>- ((sizeof(*((1 ? foo_ptr : foo_ptr)->iptr)) == 123) \
>+ ((sizeof(*((1 ? (foo_ptr) : (foo_ptr))->iptr)) == 123) \
>- memset(&foo, 0, sizeof(foo));
>+ foo.iptr = NULL;
>"segfault" is still present :)

You really should provide the changes in context. All I can tell from
this is you don't have enough right parentheses.

Unless you have hidden a variable length array in there somewhere,
sizeof does not evaluate its operand. What does your sizeof
expression really mean? Do you really think the parentheses change
anything?

Have you verified the trick works with a simple example such as

int main(void){
char c;
double d;
int i1, i2;
i1 = sizeof(1 ? c : d);
i2 = sizeof(0 ? c : d);
printf("%d %d\n", i1, i2);
return 0;
}

If you break each + and - term into its own statement, such as
x = -sizeof(*(1 ? foo...
x += sizeof(*(1 ? (foo...
x -= memset(...
and use a debugger you can determine where the seg fault occurs.

--
Remove del for email

Re: sizeof of element of struct returned by function
 

On 2/20/2013 21:27, Barry Schwarz wrote:
> On Wed, 20 Feb 2013 06:23:44 -0800 (PST), Maciej Labanowicz
> <m.labanowicz@gmail.com> wrote:
>
>> Oops, this is important issue.
>>
>> After applying changes:
>> - ((sizeof(*((1 ? foo_ptr : foo_ptr)->iptr)) == 123) \
>> + ((sizeof(*((1 ? (foo_ptr) : (foo_ptr))->iptr)) == 123) \
>> - memset(&foo, 0, sizeof(foo));
>> + foo.iptr = NULL;
>> "segfault" is still present :)
>
> You really should provide the changes in context. All I can tell from
> this is you don't have enough right parentheses.
>
> Unless you have hidden a variable length array in there somewhere,
> sizeof does not evaluate its operand. What does your sizeof
> expression really mean? Do you really think the parentheses change
> anything?
>
> Have you verified the trick works with a simple example such as
>
> int main(void){
> char c;
> double d;
> int i1, i2;
> i1 = sizeof(1 ? c : d);
> i2 = sizeof(0 ? c : d);
> printf("%d %d\n", i1, i2);
> return 0;
> }
>
> If you break each + and - term into its own statement, such as
> x = -sizeof(*(1 ? foo...
> x += sizeof(*(1 ? (foo...
> x -= memset(...
> and use a debugger you can determine where the seg fault occurs.
>

I'd guess that the TinyCC's 5-day-old VLA support probably caused the
conditional operator to evaluate its operands even if the conditional
expression is the operand of the 'sizeof' operator. Just a guess, though.

#include <stdio.h>

int (* foo(void))[10] {
printf("Uh oh\n");
return 0;
}

int main(void) {
return sizeof *(1 ? foo() : foo());
}

If this outputs "Uh oh", then uh oh.

--
- Shao Miller
--
"Thank you for the kind words; those are the kind of words I like to hear.

Cheerily," -- Richard Harter

Re: sizeof of element of struct returned by function
 

On 02/20/2013 09:27 PM, Barry Schwarz wrote:
> On Wed, 20 Feb 2013 06:23:44 -0800 (PST), Maciej Labanowicz
> <m.labanowicz@gmail.com> wrote:
>
>> Oops, this is important issue.
>>
>> After applying changes:
>> - ((sizeof(*((1 ? foo_ptr : foo_ptr)->iptr)) == 123) \
>> + ((sizeof(*((1 ? (foo_ptr) : (foo_ptr))->iptr)) == 123) \
>> - memset(&foo, 0, sizeof(foo));
>> + foo.iptr = NULL;
>> "segfault" is still present :)
>
> You really should provide the changes in context. All I can tell from
> this is you don't have enough right parentheses.

In the original context, the first line that she changed was part of the
definition of a macro, which was followed by this line:

? 12 : 34)

Which brings the parentheses into balance.

> Unless you have hidden a variable length array in there somewhere,
> sizeof does not evaluate its operand. What does your sizeof
> expression really mean? Do you really think the parentheses change
> anything?

The additional parentheses were my suggestion, as a safety measure, one
that doesn't matter in her example program, but which might matter in a
more complicated one.

My guess is that the original code had all sorts of complexities that
are missing from this version, and that the original version of
TEST_MACRO actually made sense in that environment. Now, after much
simplification, TEST_MACRO()'s only remaining purpose is to trigger the
segfault, so she can try to determine why the segfault is occurring.

> Have you verified the trick works with a simple example such as
>
> int main(void){
> char c;
> double d;
> int i1, i2;
> i1 = sizeof(1 ? c : d);
> i2 = sizeof(0 ? c : d);
> printf("%d %d\n", i1, i2);
> return 0;
> }
>
> If you break each + and - term into its own statement, such as
> x = -sizeof(*(1 ? foo...
> x += sizeof(*(1 ? (foo...
> x -= memset(...
> and use a debugger you can determine where the seg fault occurs.

There's really not much choice in the matter. There's only one tricky
feature that follows the last successfully executed printf() call, and
that's the expansion of TEXT_MACRO(). The result of that expansion is
very complicated, but as I understand it, it should all boil down to
(sizeof(int)==123 ? 12 : 34). Therefore, there's no plausible way for it
to result in a segfault unless someone's misunderstanding something -
either me, or the implementors of tinycc.
--
James Kuyper

Re: sizeof of element of struct returned by function
 

On 2/20/2013 21:50, Shao Miller wrote:
> On 2/20/2013 21:27, Barry Schwarz wrote:
>> On Wed, 20 Feb 2013 06:23:44 -0800 (PST), Maciej Labanowicz
>> <m.labanowicz@gmail.com> wrote:
>>
>>> Oops, this is important issue.
>>>
>>> After applying changes:
>>> - ((sizeof(*((1 ? foo_ptr : foo_ptr)->iptr)) == 123) \
>>> + ((sizeof(*((1 ? (foo_ptr) : (foo_ptr))->iptr)) == 123) \
>>> - memset(&foo, 0, sizeof(foo));
>>> + foo.iptr = NULL;
>>> "segfault" is still present :)
>>
>> You really should provide the changes in context. All I can tell from
>> this is you don't have enough right parentheses.
>>
>> Unless you have hidden a variable length array in there somewhere,
>> sizeof does not evaluate its operand. What does your sizeof
>> expression really mean? Do you really think the parentheses change
>> anything?
>>
>> Have you verified the trick works with a simple example such as
>>
>> int main(void){
>> char c;
>> double d;
>> int i1, i2;
>> i1 = sizeof(1 ? c : d);
>> i2 = sizeof(0 ? c : d);
>> printf("%d %d\n", i1, i2);
>> return 0;
>> }
>>
>> If you break each + and - term into its own statement, such as
>> x = -sizeof(*(1 ? foo...
>> x += sizeof(*(1 ? (foo...
>> x -= memset(...
>> and use a debugger you can determine where the seg fault occurs.
>>
>
> I'd guess that the TinyCC's 5-day-old VLA support probably caused the
> conditional operator to evaluate its operands even if the conditional
> expression is the operand of the 'sizeof' operator. Just a guess, though.
>
> #include <stdio.h>
>
> int (* foo(void))[10] {
> printf("Uh oh\n");
> return 0;
> }
>
> int main(void) {
> return sizeof *(1 ? foo() : foo());
> }
>
> If this outputs "Uh oh", then uh oh.
>

Then again, it gripes that the second operand of the conditional (the
zero) needs to be a pointer, for the code below:

int main(void) {
return sizeof *(1 ? 0 : (int *) 0);
}

So the implementation could use some improvements.

--
- Shao Miller
--
"Thank you for the kind words; those are the kind of words I like to hear.

Cheerily," -- Richard Harter

Re: sizeof of element of struct returned by function
 

On 20 Feb, 20:10, Shao Miller <sha0.mil...@gmail.com> wrote:
> Redundant parentheses for the 'sizeof' operator.

This is a style that I prefer.
My experience shows that sometimes using of parentheses is a good
idea.
It makes that the code review is simpler, example:
- if (a + b * c)
+ if (a + (b * c))
BTW: I remember that my university teacher has always complaining
about the redundant parentheses, he has mentioned that I should
know perfectly priority of operators.
After that I have started to work in private company,
where such practice is forbidden.
Parentheses are your friend.

Another example of the redundant code is the
initialization of 'result' in function 'fun'.
But this is also a style that I prefer.
Any variable should be initialized at begining.
For example, a someone has fixed this code with:
if (foo_ptr) {
result = &(foo_ptr->bar);
}
then function is still correctly working.
In case the initialization is redundant then
an optimizer will remove the code during compilation process.
It is very hard to diagnose bugs that depends on the uninitialized
variable, especially
if it occures only ones per a week and only on client side (taken from
real life).

Regards

--
Maciej Labanowicz