public inbox for gcc@gcc.gnu.org
 help / color / mirror / Atom feed
* Should GCC warn about sizeof(flexible_struct)?
@ 2023-08-11 18:29 Alejandro Colomar
  2023-08-11 18:34 ` Alejandro Colomar
  2023-08-14  6:41 ` Richard Biener
  0 siblings, 2 replies; 10+ messages in thread
From: Alejandro Colomar @ 2023-08-11 18:29 UTC (permalink / raw)
  To: GCC


[-- Attachment #1.1: Type: text/plain, Size: 2872 bytes --]

Hi!

Structures with flexible array members have restrictions about being
used in arrays or within other structures, as the size of the enclosing
aggregate type would be... inconsistent.

In general, sizeof(flexible_struct) is a problematic thing that rarely
means what programmers think it means.  It is not the size of the
structure up to the flexible array member; or expressed using C,
the following can be true:

	sizeof(s) != offsetof(s, fam)

See the program at the bottom that demonstrates how this is problematic.

It's true that if one uses

	malloc(offseof(s, fam) + sizeof_member(s, fam[0]) * N);

and N is very small (0 or 1 usually), the allocation would be smaller
than the object size, which for GCC seems to be fine, but I'm worried the
standard is not clear enough about its validity[1].

[1]: <https://software.codidact.com/posts/287754>

To avoid having UB there, pedantically one would need to call

	malloc(MAX(sizeof(s), offseof(s, fam) + sizeof_member(s, fam[0]) * N));

But I think that's the only correct use of sizeof() with structures
containing flexible array members.  So it seems sizeof() by itself is
a valid thing, but when adding it to something else to get the total size,
or doing any arithmetic with it, that's dubious code.

How about some -Wfam-sizeof-arithmetic that would not warn about taking
sizeof(s) but would warn if that sizeof is used in any arithmetic?

Cheers,
Alex

---

$ cat off.c 
#include <err.h>
#include <stddef.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>


struct s {
	int   i;
	char  c;
	char  fam[];
};


static inline void *xmalloc(size_t size);


int
main(void)
{
	char      *p;
	struct s  *s;

	printf("sizeof: %zu\n", sizeof(struct s));
	printf("offsetof: %zu\n", offsetof(struct s, fam));

	puts("\nWith sizeof():");

	s = xmalloc(sizeof(struct s) + sizeof("Hello, sizeof!"));
	strcpy(s->fam, "Hello, sizeof!");
	p = (char *) s + sizeof(struct s);
	puts(p);
	free(s);

	puts("\nWith offsetof(3):");

	s = xmalloc(offsetof(struct s, fam) + sizeof("Hello, offsetof!"));
	strcpy(s->fam, "Hello, offsetof!");
	p = (char *) s + offsetof(struct s, fam);
	puts(p);
	free(s);

	exit(EXIT_SUCCESS);
}


static inline void *
xmalloc(size_t size)
{
	void  *p;

	p = malloc(size);
	if (p == NULL)
		err(EXIT_FAILURE, "malloc");
	return p;
}
$ gcc-13 -Wall -Wextra -Wpadded -fanalyzer off.c
off.c:12:1: warning: padding struct size to alignment boundary with 3 bytes [-Wpadded]
   12 | };
      | ^


The only warning I know that is triggered in the code above is -Wpadded,
which is related to this problem, but I think there should be something
to warn about sizeof() in this context.


-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Should GCC warn about sizeof(flexible_struct)?
  2023-08-11 18:29 Should GCC warn about sizeof(flexible_struct)? Alejandro Colomar
@ 2023-08-11 18:34 ` Alejandro Colomar
  2023-08-14  6:41 ` Richard Biener
  1 sibling, 0 replies; 10+ messages in thread
From: Alejandro Colomar @ 2023-08-11 18:34 UTC (permalink / raw)
  To: GCC


[-- Attachment #1.1: Type: text/plain, Size: 3302 bytes --]

On 2023-08-11 20:29, Alejandro Colomar wrote:
> Hi!
> 
> Structures with flexible array members have restrictions about being
> used in arrays or within other structures, as the size of the enclosing
> aggregate type would be... inconsistent.
> 
> In general, sizeof(flexible_struct) is a problematic thing that rarely
> means what programmers think it means.  It is not the size of the
> structure up to the flexible array member; or expressed using C,
> the following can be true:
> 
> 	sizeof(s) != offsetof(s, fam)
> 
> See the program at the bottom that demonstrates how this is problematic.
> 
> It's true that if one uses
> 
> 	malloc(offseof(s, fam) + sizeof_member(s, fam[0]) * N);
> 
> and N is very small (0 or 1 usually), the allocation would be smaller
> than the object size, which for GCC seems to be fine, but I'm worried the
> standard is not clear enough about its validity[1].
> 
> [1]: <https://software.codidact.com/posts/287754>
> 
> To avoid having UB there, pedantically one would need to call
> 
> 	malloc(MAX(sizeof(s), offseof(s, fam) + sizeof_member(s, fam[0]) * N));
> 
> But I think that's the only correct use of sizeof() with structures
> containing flexible array members.  So it seems sizeof() by itself is
> a valid thing, but when adding it to something else to get the total size,
> or doing any arithmetic with it, that's dubious code.
> 
> How about some -Wfam-sizeof-arithmetic that would not warn about taking
> sizeof(s) but would warn if that sizeof is used in any arithmetic?
> 
> Cheers,
> Alex
> 
> ---
> 
> $ cat off.c 
> #include <err.h>
> #include <stddef.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> 
> 
> struct s {
> 	int   i;
> 	char  c;
> 	char  fam[];
> };
> 
> 
> static inline void *xmalloc(size_t size);
> 
> 
> int
> main(void)
> {
> 	char      *p;
> 	struct s  *s;
> 
> 	printf("sizeof: %zu\n", sizeof(struct s));
> 	printf("offsetof: %zu\n", offsetof(struct s, fam));
> 
> 	puts("\nWith sizeof():");
> 
> 	s = xmalloc(sizeof(struct s) + sizeof("Hello, sizeof!"));
> 	strcpy(s->fam, "Hello, sizeof!");
> 	p = (char *) s + sizeof(struct s);
> 	puts(p);
> 	free(s);
> 
> 	puts("\nWith offsetof(3):");
> 
> 	s = xmalloc(offsetof(struct s, fam) + sizeof("Hello, offsetof!"));
> 	strcpy(s->fam, "Hello, offsetof!");
> 	p = (char *) s + offsetof(struct s, fam);
> 	puts(p);
> 	free(s);
> 
> 	exit(EXIT_SUCCESS);
> }
> 
> 
> static inline void *
> xmalloc(size_t size)
> {
> 	void  *p;
> 
> 	p = malloc(size);
> 	if (p == NULL)
> 		err(EXIT_FAILURE, "malloc");
> 	return p;
> }
> $ gcc-13 -Wall -Wextra -Wpadded -fanalyzer off.c
> off.c:12:1: warning: padding struct size to alignment boundary with 3 bytes [-Wpadded]
>    12 | };
>       | ^

I forgot to paste the output of the program:)

$ ./a.out 
sizeof: 8
offsetof: 5

With sizeof():
lo, sizeof!

With offsetof(3):
Hello, offsetof!

> 
> 
> The only warning I know that is triggered in the code above is -Wpadded,
> which is related to this problem, but I think there should be something
> to warn about sizeof() in this context.
> 
> 

-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Should GCC warn about sizeof(flexible_struct)?
  2023-08-11 18:29 Should GCC warn about sizeof(flexible_struct)? Alejandro Colomar
  2023-08-11 18:34 ` Alejandro Colomar
@ 2023-08-14  6:41 ` Richard Biener
  2023-08-14  9:51   ` Alejandro Colomar
  2023-08-14 15:10   ` Qing Zhao
  1 sibling, 2 replies; 10+ messages in thread
From: Richard Biener @ 2023-08-14  6:41 UTC (permalink / raw)
  To: Alejandro Colomar; +Cc: GCC

On Fri, Aug 11, 2023 at 8:30 PM Alejandro Colomar via Gcc
<gcc@gcc.gnu.org> wrote:
>
> Hi!
>
> Structures with flexible array members have restrictions about being
> used in arrays or within other structures, as the size of the enclosing
> aggregate type would be... inconsistent.
>
> In general, sizeof(flexible_struct) is a problematic thing that rarely
> means what programmers think it means.  It is not the size of the
> structure up to the flexible array member; or expressed using C,
> the following can be true:
>
>         sizeof(s) != offsetof(s, fam)
>
> See the program at the bottom that demonstrates how this is problematic.
>
> It's true that if one uses
>
>         malloc(offseof(s, fam) + sizeof_member(s, fam[0]) * N);
>
> and N is very small (0 or 1 usually), the allocation would be smaller
> than the object size, which for GCC seems to be fine, but I'm worried the
> standard is not clear enough about its validity[1].
>
> [1]: <https://software.codidact.com/posts/287754>
>
> To avoid having UB there, pedantically one would need to call
>
>         malloc(MAX(sizeof(s), offseof(s, fam) + sizeof_member(s, fam[0]) * N));
>
> But I think that's the only correct use of sizeof() with structures
> containing flexible array members.  So it seems sizeof() by itself is
> a valid thing, but when adding it to something else to get the total size,
> or doing any arithmetic with it, that's dubious code.
>
> How about some -Wfam-sizeof-arithmetic that would not warn about taking
> sizeof(s) but would warn if that sizeof is used in any arithmetic?

There are probably many ways sizeof() plus arithmetic can yield a correct
size for allocation.  After all _all_ uses of FAM requires allocation
and there's
no convenient standard way of calculating the required size (sizeof
(fam-type[n])?).

Iff we want to diagnose anything then possibly a computation that looks like
a size computation but that's actually smaller than required, but
other than that - what
would you suggest to fix such reported warnings?

Richard.

> Cheers,
> Alex
>
> ---
>
> $ cat off.c
> #include <err.h>
> #include <stddef.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
>
>
> struct s {
>         int   i;
>         char  c;
>         char  fam[];
> };
>
>
> static inline void *xmalloc(size_t size);
>
>
> int
> main(void)
> {
>         char      *p;
>         struct s  *s;
>
>         printf("sizeof: %zu\n", sizeof(struct s));
>         printf("offsetof: %zu\n", offsetof(struct s, fam));
>
>         puts("\nWith sizeof():");
>
>         s = xmalloc(sizeof(struct s) + sizeof("Hello, sizeof!"));
>         strcpy(s->fam, "Hello, sizeof!");
>         p = (char *) s + sizeof(struct s);
>         puts(p);
>         free(s);
>
>         puts("\nWith offsetof(3):");
>
>         s = xmalloc(offsetof(struct s, fam) + sizeof("Hello, offsetof!"));
>         strcpy(s->fam, "Hello, offsetof!");
>         p = (char *) s + offsetof(struct s, fam);
>         puts(p);
>         free(s);
>
>         exit(EXIT_SUCCESS);
> }
>
>
> static inline void *
> xmalloc(size_t size)
> {
>         void  *p;
>
>         p = malloc(size);
>         if (p == NULL)
>                 err(EXIT_FAILURE, "malloc");
>         return p;
> }
> $ gcc-13 -Wall -Wextra -Wpadded -fanalyzer off.c
> off.c:12:1: warning: padding struct size to alignment boundary with 3 bytes [-Wpadded]
>    12 | };
>       | ^
>
>
> The only warning I know that is triggered in the code above is -Wpadded,
> which is related to this problem, but I think there should be something
> to warn about sizeof() in this context.
>
>
> --
> <http://www.alejandro-colomar.es/>
> GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Should GCC warn about sizeof(flexible_struct)?
  2023-08-14  6:41 ` Richard Biener
@ 2023-08-14  9:51   ` Alejandro Colomar
  2023-08-14 10:00     ` Martin Uecker
  2023-08-14 15:10   ` Qing Zhao
  1 sibling, 1 reply; 10+ messages in thread
From: Alejandro Colomar @ 2023-08-14  9:51 UTC (permalink / raw)
  To: Richard Biener; +Cc: GCC


[-- Attachment #1.1: Type: text/plain, Size: 2751 bytes --]

Hi Richard,

On 2023-08-14 08:41, Richard Biener wrote:
> On Fri, Aug 11, 2023 at 8:30 PM Alejandro Colomar via Gcc
> <gcc@gcc.gnu.org> wrote:

[...]

>> How about some -Wfam-sizeof-arithmetic that would not warn about taking
>> sizeof(s) but would warn if that sizeof is used in any arithmetic?
> 
> There are probably many ways sizeof() plus arithmetic can yield a correct
> size for allocation.  After all _all_ uses of FAM requires allocation
> and there's
> no convenient standard way of calculating the required size (sizeof
> (fam-type[n])?).

You may be confusing sizeof(struct contains_fam) with sizeof(fam[n]).

Yes, the second is necessary for allocation, but the first is not.  Well,
it is valid for allocation but only because allocating extra bytes is not
a problem.

The size of a flexible structure is calculated as the sum of the offset
of the fam, and the size of the fam.  The size of the structure has nothing
to do.

	struct s {
		int   i;
		char  c;
		char  fam[];
	} s;

	size = offsetof(struct s, fam) + sizeof("foobar"));  // OK: 12 B

	size = sizeof(struct s) + sizeof("foobar"));  // NOK: 15 B; wastes bytes
	       ^~~~ problem here.

> 
> Iff we want to diagnose anything then possibly a computation that looks like
> a size computation but that's actually smaller than required,

It's actually the other way around.  The problem is that the computation may
give a value larger than the expected one.  This can be usually a benign bug
and nothing bad will happen.  But when a programmer relies on that size for
reading or writing, which I've seen happen, you better make sure that the
structure has no padding, or you'll be reading/writing at `fam + padding`.

Example, using the allocation above:

	strcpy((char *) s + sizeof(struct s), "foobar");  // NOK, writes after padding
	puts(s->fam);  // OK, reads the fam, but surprise

	// Unpredictable; prints 3 uninitialized bytes, then (if no previous '\0') "foobar"

	strcpy(s->fam, "foobar");  // OK, writes at the fam
	puts((char *) s + sizeof(struct s));  // NOK, reads after padding

	// prints: "bar"

	strcpy(s->fam, "foobar");  // OK
	puts((char *) s + offsetof(struct s, fam));  // OK; with offsetof, equivalent to s->fam

	// prints: "foobar"

	strcpy((char *) s + offsetof(struct s, fam), "foobar");  // Also OK
	puts(s->fam);  // OK

	// prints: "foobar"

> but
> other than that - what
> would you suggest to fix such reported warnings?

To fix the warnings, replace all invocations of `sizeof(struct contains_fam)`
by `offsetof(struct contains_fam, fam)`.

Cheers,
Alex

-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Should GCC warn about sizeof(flexible_struct)?
  2023-08-14  9:51   ` Alejandro Colomar
@ 2023-08-14 10:00     ` Martin Uecker
  2023-08-14 10:21       ` Alejandro Colomar
  0 siblings, 1 reply; 10+ messages in thread
From: Martin Uecker @ 2023-08-14 10:00 UTC (permalink / raw)
  To: Alejandro Colomar, Richard Biener; +Cc: GCC

Am Montag, dem 14.08.2023 um 11:51 +0200 schrieb Alejandro Colomar via Gcc:
> Hi Richard,
> 
> On 2023-08-14 08:41, Richard Biener wrote:
> > On Fri, Aug 11, 2023 at 8:30 PM Alejandro Colomar via Gcc
> > <gcc@gcc.gnu.org> wrote:
> 
> [...]
> 
> > > How about some -Wfam-sizeof-arithmetic that would not warn about taking
> > > sizeof(s) but would warn if that sizeof is used in any arithmetic?
> > 
> > There are probably many ways sizeof() plus arithmetic can yield a correct
> > size for allocation.  After all _all_ uses of FAM requires allocation
> > and there's
> > no convenient standard way of calculating the required size (sizeof
> > (fam-type[n])?).
> 
> You may be confusing sizeof(struct contains_fam) with sizeof(fam[n]).
> 
> Yes, the second is necessary for allocation, but the first is not.  Well,
> it is valid for allocation but only because allocating extra bytes is not
> a problem.
> 
> The size of a flexible structure is calculated as the sum of the offset
> of the fam, and the size of the fam.  The size of the structure has nothing
> to do.
> 
> 	struct s {
> 		int   i;
> 		char  c;
> 		char  fam[];
> 	} s;
> 
> 	size = offsetof(struct s, fam) + sizeof("foobar"));  // OK: 12 B
> 
> 	size = sizeof(struct s) + sizeof("foobar"));  // NOK: 15 B; wastes bytes
> 	       ^~~~ problem here.
> 

The first formula may give a result smaller than sizeof(struct s), so
could also be dangerous.

Martin



> > 
> > Iff we want to diagnose anything then possibly a computation that looks like
> > a size computation but that's actually smaller than required,
> 
> It's actually the other way around.  The problem is that the computation may
> give a value larger than the expected one.  This can be usually a benign bug
> and nothing bad will happen.  But when a programmer relies on that size for
> reading or writing, which I've seen happen, you better make sure that the
> structure has no padding, or you'll be reading/writing at `fam + padding`.
> 
> Example, using the allocation above:
> 
> 	strcpy((char *) s + sizeof(struct s), "foobar");  // NOK, writes after padding
> 	puts(s->fam);  // OK, reads the fam, but surprise
> 
> 	// Unpredictable; prints 3 uninitialized bytes, then (if no previous '\0') "foobar"
> 
> 	strcpy(s->fam, "foobar");  // OK, writes at the fam
> 	puts((char *) s + sizeof(struct s));  // NOK, reads after padding
> 
> 	// prints: "bar"
> 
> 	strcpy(s->fam, "foobar");  // OK
> 	puts((char *) s + offsetof(struct s, fam));  // OK; with offsetof, equivalent to s->fam
> 
> 	// prints: "foobar"
> 
> 	strcpy((char *) s + offsetof(struct s, fam), "foobar");  // Also OK
> 	puts(s->fam);  // OK
> 
> 	// prints: "foobar"
> 
> > but
> > other than that - what
> > would you suggest to fix such reported warnings?
> 
> To fix the warnings, replace all invocations of `sizeof(struct contains_fam)`
> by `offsetof(struct contains_fam, fam)`.
> 
> Cheers,
> Alex
> 



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Should GCC warn about sizeof(flexible_struct)?
  2023-08-14 10:00     ` Martin Uecker
@ 2023-08-14 10:21       ` Alejandro Colomar
  2023-08-14 16:49         ` Martin Uecker
  0 siblings, 1 reply; 10+ messages in thread
From: Alejandro Colomar @ 2023-08-14 10:21 UTC (permalink / raw)
  To: Martin Uecker, Richard Biener; +Cc: GCC


[-- Attachment #1.1: Type: text/plain, Size: 1717 bytes --]

Hi Martin,

On 2023-08-14 12:00, Martin Uecker wrote:
> Am Montag, dem 14.08.2023 um 11:51 +0200 schrieb Alejandro Colomar via Gcc:
>> Hi Richard,

[...]

>>
>> The size of a flexible structure is calculated as the sum of the offset
>> of the fam, and the size of the fam.  The size of the structure has nothing
>> to do.
>>
>> 	struct s {
>> 		int   i;
>> 		char  c;
>> 		char  fam[];
>> 	} s;
>>
>> 	size = offsetof(struct s, fam) + sizeof("foobar"));  // OK: 12 B
>>
>> 	size = sizeof(struct s) + sizeof("foobar"));  // NOK: 15 B; wastes bytes
>> 	       ^~~~ problem here.
>>
> 
> The first formula may give a result smaller than sizeof(struct s), so
> could also be dangerous.

That's true, which is why the pedantic way involves MAX(sizeof(struct s), ...).

<https://gustedt.wordpress.com/2011/03/14/flexible-array-member/>

Would you mind chiming in to this question?:
<https://software.codidact.com/posts/287754>

The standard could be more clear about it.  And GCC could also specify what
happens if you don't allocate trailing padding for a structure.

Especially, I don't think the compiler is free to write to the padding, since
the padding is part of the FAM, and so it would be a visible change (unless
the compiler knows the FAM is uninitialized, in which case it could decide that

	s->c = 'r';

can be implemented by copying an entire 4 bytes, as the padding permits).

Anyway, that's why warning on sizeof(struct s) would be a bad idea.  Warning
when any arithmetic is applied to that sizeof() would be good, though, I think.

Cheers,
Alex

-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Should GCC warn about sizeof(flexible_struct)?
  2023-08-14  6:41 ` Richard Biener
  2023-08-14  9:51   ` Alejandro Colomar
@ 2023-08-14 15:10   ` Qing Zhao
  1 sibling, 0 replies; 10+ messages in thread
From: Qing Zhao @ 2023-08-14 15:10 UTC (permalink / raw)
  To: Richard Biener, Martin Uecker; +Cc: Alejandro Colomar, GCC



> On Aug 14, 2023, at 2:41 AM, Richard Biener via Gcc <gcc@gcc.gnu.org> wrote:
> 
> On Fri, Aug 11, 2023 at 8:30 PM Alejandro Colomar via Gcc
> <gcc@gcc.gnu.org> wrote:
>> 
>> Hi!
>> 
>> Structures with flexible array members have restrictions about being
>> used in arrays or within other structures, as the size of the enclosing
>> aggregate type would be... inconsistent.
>> 
>> In general, sizeof(flexible_struct) is a problematic thing that rarely
>> means what programmers think it means.  It is not the size of the
>> structure up to the flexible array member; or expressed using C,
>> the following can be true:
>> 
>>        sizeof(s) != offsetof(s, fam)
>> 
>> See the program at the bottom that demonstrates how this is problematic.
>> 
>> It's true that if one uses
>> 
>>        malloc(offseof(s, fam) + sizeof_member(s, fam[0]) * N);
>> 
>> and N is very small (0 or 1 usually), the allocation would be smaller
>> than the object size, which for GCC seems to be fine, but I'm worried the
>> standard is not clear enough about its validity[1].
>> 
>> [1]: <https://software.codidact.com/posts/287754>
>> 
>> To avoid having UB there, pedantically one would need to call
>> 
>>        malloc(MAX(sizeof(s), offseof(s, fam) + sizeof_member(s, fam[0]) * N));
>> 
>> But I think that's the only correct use of sizeof() with structures
>> containing flexible array members.  So it seems sizeof() by itself is
>> a valid thing, but when adding it to something else to get the total size,
>> or doing any arithmetic with it, that's dubious code.
>> 
>> How about some -Wfam-sizeof-arithmetic that would not warn about taking
>> sizeof(s) but would warn if that sizeof is used in any arithmetic?
> 
> There are probably many ways sizeof() plus arithmetic can yield a correct
> size for allocation.  After all _all_ uses of FAM requires allocation
> and there's
> no convenient standard way of calculating the required size (sizeof
> (fam-type[n])?).
> 
> Iff we want to diagnose anything then possibly a computation that looks like
> a size computation but that's actually smaller than required,

Yes, I think that an warning on insufficient allocation might be useful in general, 
which might be combined with Martin’s previous proposed patch together: -Walloc_type?
https://gcc.gnu.org/pipermail/gcc-patches/2023-July/625172.html

Another thing is: is there a place in GCC’s doc where we can add some clarification
or suggestion to the users on how to allocate space for structures with FAM?
Looks like that this is a very confusion area..

thanks.

Qing

> but
> other than that - what
> would you suggest to fix such reported warnings?
> 
> Richard.
> 
>> Cheers,
>> Alex
>> 
>> ---
>> 
>> $ cat off.c
>> #include <err.h>
>> #include <stddef.h>
>> #include <stdio.h>
>> #include <stdlib.h>
>> #include <string.h>
>> 
>> 
>> struct s {
>>        int   i;
>>        char  c;
>>        char  fam[];
>> };
>> 
>> 
>> static inline void *xmalloc(size_t size);
>> 
>> 
>> int
>> main(void)
>> {
>>        char      *p;
>>        struct s  *s;
>> 
>>        printf("sizeof: %zu\n", sizeof(struct s));
>>        printf("offsetof: %zu\n", offsetof(struct s, fam));
>> 
>>        puts("\nWith sizeof():");
>> 
>>        s = xmalloc(sizeof(struct s) + sizeof("Hello, sizeof!"));
>>        strcpy(s->fam, "Hello, sizeof!");
>>        p = (char *) s + sizeof(struct s);
>>        puts(p);
>>        free(s);
>> 
>>        puts("\nWith offsetof(3):");
>> 
>>        s = xmalloc(offsetof(struct s, fam) + sizeof("Hello, offsetof!"));
>>        strcpy(s->fam, "Hello, offsetof!");
>>        p = (char *) s + offsetof(struct s, fam);
>>        puts(p);
>>        free(s);
>> 
>>        exit(EXIT_SUCCESS);
>> }
>> 
>> 
>> static inline void *
>> xmalloc(size_t size)
>> {
>>        void  *p;
>> 
>>        p = malloc(size);
>>        if (p == NULL)
>>                err(EXIT_FAILURE, "malloc");
>>        return p;
>> }
>> $ gcc-13 -Wall -Wextra -Wpadded -fanalyzer off.c
>> off.c:12:1: warning: padding struct size to alignment boundary with 3 bytes [-Wpadded]
>>   12 | };
>>      | ^
>> 
>> 
>> The only warning I know that is triggered in the code above is -Wpadded,
>> which is related to this problem, but I think there should be something
>> to warn about sizeof() in this context.
>> 
>> 
>> --
>> <http://www.alejandro-colomar.es/>
>> GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Should GCC warn about sizeof(flexible_struct)?
  2023-08-14 10:21       ` Alejandro Colomar
@ 2023-08-14 16:49         ` Martin Uecker
  2023-08-14 17:11           ` Alejandro Colomar
  0 siblings, 1 reply; 10+ messages in thread
From: Martin Uecker @ 2023-08-14 16:49 UTC (permalink / raw)
  To: Alejandro Colomar, Richard Biener; +Cc: GCC

Am Montag, dem 14.08.2023 um 12:21 +0200 schrieb Alejandro Colomar:
> Hi Martin,
> 
> On 2023-08-14 12:00, Martin Uecker wrote:
> > Am Montag, dem 14.08.2023 um 11:51 +0200 schrieb Alejandro Colomar via Gcc:
> > > Hi Richard,
> 
> [...]
> 
> > > 
> > > The size of a flexible structure is calculated as the sum of the offset
> > > of the fam, and the size of the fam.  The size of the structure has nothing
> > > to do.
> > > 
> > > 	struct s {
> > > 		int   i;
> > > 		char  c;
> > > 		char  fam[];
> > > 	} s;
> > > 
> > > 	size = offsetof(struct s, fam) + sizeof("foobar"));  // OK: 12 B
> > > 
> > > 	size = sizeof(struct s) + sizeof("foobar"));  // NOK: 15 B; wastes bytes
> > > 	       ^~~~ problem here.
> > > 
> > 
> > The first formula may give a result smaller than sizeof(struct s), so
> > could also be dangerous.
> 
> That's true, which is why the pedantic way involves MAX(sizeof(struct s), ...).
> 
> <https://gustedt.wordpress.com/2011/03/14/flexible-array-member/>
> 
> Would you mind chiming in to this question?:
> <https://software.codidact.com/posts/287754>

Unclear. It is probably UB by omission.  But this is possibly
different from the FAM case.

I any case, I am not so concerned about the whether this UB, 
but that a programmer might do:

struct s = { .. }
struct s* p = malloc(...)
memcpy(p, &s, sizeof s); // copy header. 


Martin


> The standard could be more clear about it.  And GCC could also specify what
> happens if you don't allocate trailing padding for a structure.
> 
> Especially, I don't think the compiler is free to write to the padding, since
> the padding is part of the FAM, and so it would be a visible change (unless
> the compiler knows the FAM is uninitialized, in which case it could decide that
> 
> 	s->c = 'r';
> 
> can be implemented by copying an entire 4 bytes, as the padding permits).
> 
> Anyway, that's why warning on sizeof(struct s) would be a bad idea.  Warning
> when any arithmetic is applied to that sizeof() would be good, though, I think.
> 
> Cheers,
> Alex
> 



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Should GCC warn about sizeof(flexible_struct)?
  2023-08-14 16:49         ` Martin Uecker
@ 2023-08-14 17:11           ` Alejandro Colomar
  2023-08-14 17:16             ` Alejandro Colomar
  0 siblings, 1 reply; 10+ messages in thread
From: Alejandro Colomar @ 2023-08-14 17:11 UTC (permalink / raw)
  To: Martin Uecker, Richard Biener; +Cc: GCC


[-- Attachment #1.1: Type: text/plain, Size: 1954 bytes --]

Hi Martin,

On 2023-08-14 18:49, Martin Uecker wrote:
> Am Montag, dem 14.08.2023 um 12:21 +0200 schrieb Alejandro Colomar:
[...]

>> Would you mind chiming in to this question?:
>> <https://software.codidact.com/posts/287754>
> 
> Unclear. It is probably UB by omission.

Agree.

>  But this is possibly
> different from the FAM case.

I don't think I agree on this.  To me it really looks like the same thing.
BTW, there was a mention there to the FAM case in a comment:
<https://software.codidact.com/comments/thread/7169#comment-21254>

> 
> I any case, I am not so concerned about the whether this UB, 
> but that a programmer might do:
> 
> struct s = { .. }
> struct s* p = malloc(...)
> memcpy(p, &s, sizeof s); // copy header. 

That's (or could be) a bug, and just another manifestation of why
sizeof(s) is wrong.  Let's see a couple of ways how this can go wrong:


$ cat memcpy.c 
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

struct s {
	int   i;
	char  c;
	char  fam[];
};

struct h {
	int   i;
	char  c;
};

int
main(void)
{
	char      *f;
	struct h  h = { .i = 42, .c = 3 };
	struct s  *p;

	p = malloc(sizeof(struct s) + sizeof("foobar"));

	strcpy(p->fam, "foobar");
	/*
	 * since we're copying the header, it shouldn't matter if we
	 * copy it after copying the fam itself, no?  They're at
	 * different locations... or are they?
	 */
	memcpy(p, &h, sizeof(struct s));
	puts(p->fam);
	free(p);

	p = malloc(sizeof(struct s) + sizeof("foobar"));

	f = mempcpy(p, &h, sizeof(struct s));
	/*
	 * We could reuse the pointer from mempcpy(3) to get the location
	 * of just after the header, right?  Heh.
	 */
	strcpy(f, "foobar");
	puts(p->fam);
	free(p);
}
$ cc -Wall -Wextra memcpy.c -D_GNU_SOURCE
$ ./a.out 


$


Cheers,
Alex

-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Should GCC warn about sizeof(flexible_struct)?
  2023-08-14 17:11           ` Alejandro Colomar
@ 2023-08-14 17:16             ` Alejandro Colomar
  0 siblings, 0 replies; 10+ messages in thread
From: Alejandro Colomar @ 2023-08-14 17:16 UTC (permalink / raw)
  To: Martin Uecker, Richard Biener; +Cc: GCC


[-- Attachment #1.1: Type: text/plain, Size: 2366 bytes --]

On 2023-08-14 19:11, Alejandro Colomar wrote:
> Hi Martin,
> 
> On 2023-08-14 18:49, Martin Uecker wrote:
>> Am Montag, dem 14.08.2023 um 12:21 +0200 schrieb Alejandro Colomar:
> [...]
> 
>>> Would you mind chiming in to this question?:
>>> <https://software.codidact.com/posts/287754>
>>
>> Unclear. It is probably UB by omission.
> 
> Agree.
> 
>>  But this is possibly
>> different from the FAM case.
> 
> I don't think I agree on this.  To me it really looks like the same thing.
> BTW, there was a mention there to the FAM case in a comment:
> <https://software.codidact.com/comments/thread/7169#comment-21254>
> 
>>
>> I any case, I am not so concerned about the whether this UB, 
>> but that a programmer might do:
>>
>> struct s = { .. }
>> struct s* p = malloc(...)
>> memcpy(p, &s, sizeof s); // copy header. 
> 
> That's (or could be) a bug, and just another manifestation of why
> sizeof(s) is wrong.  Let's see a couple of ways how this can go wrong:
> 
> 
> $ cat memcpy.c 
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> 
> struct s {
> 	int   i;
> 	char  c;
> 	char  fam[];
> };
> 
> struct h {
> 	int   i;
> 	char  c;
> };
> 
> int
> main(void)
> {
> 	char      *f;
> 	struct h  h = { .i = 42, .c = 3 };
> 	struct s  *p;
> 
> 	p = malloc(sizeof(struct s) + sizeof("foobar"));
> 
> 	strcpy(p->fam, "foobar");
> 	/*
> 	 * since we're copying the header, it shouldn't matter if we
> 	 * copy it after copying the fam itself, no?  They're at
> 	 * different locations... or are they?
> 	 */
> 	memcpy(p, &h, sizeof(struct s));

But if you did here

	memcpy(p, &h, offsetof(struct s, fam));

> 	puts(p->fam);
> 	free(p);
> 
> 	p = malloc(sizeof(struct s) + sizeof("foobar"));
> 
> 	f = mempcpy(p, &h, sizeof(struct s));

and here

	f = mempcpy(p, &h, offsetof(struct s, fam));

Then it magically works as expected:

$ ./a.out 
foobar
foobar


> 	/*
> 	 * We could reuse the pointer from mempcpy(3) to get the location
> 	 * of just after the header, right?  Heh.
> 	 */
> 	strcpy(f, "foobar");
> 	puts(p->fam);
> 	free(p);
> }
> $ cc -Wall -Wextra memcpy.c -D_GNU_SOURCE
> $ ./a.out 
> 
> 
> $
> 
> 
> Cheers,
> Alex
> 

-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-08-14 17:16 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-11 18:29 Should GCC warn about sizeof(flexible_struct)? Alejandro Colomar
2023-08-11 18:34 ` Alejandro Colomar
2023-08-14  6:41 ` Richard Biener
2023-08-14  9:51   ` Alejandro Colomar
2023-08-14 10:00     ` Martin Uecker
2023-08-14 10:21       ` Alejandro Colomar
2023-08-14 16:49         ` Martin Uecker
2023-08-14 17:11           ` Alejandro Colomar
2023-08-14 17:16             ` Alejandro Colomar
2023-08-14 15:10   ` Qing Zhao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).