Hi,
I got a - I thought - spurious warning in a development patch. A simplified
reproducer of the warning is [1], which triggers:
<source>: In function 'trigger_warning':
<source>:19:9: warning: array subscript 'struct foo[0]' is partly outside array
bounds of 'unsigned char[13]' [-Warray-bounds=]
19 | foop->len = len;
| ^~
<source>:18:12: note: object of size 13 allocated by 'allocme'
18 | foop = allocme(offsetof(struct foo, data) + sizeof(char) * len);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Compiler returned: 0
This is a pattern that we have in quite a few places.
Note that the warning vanishes if the "flag" element is not present. The
reason for that difference is that when the flag element is present, sizeof(
struct foo) is 16 on a 64bit platform, but we allocate only 13 bytes, as
offsetof(struct foo, data) is 12.
I asked the gcc folks whether they think the warning is spurious or whether
the code is broken. They think it's the code. Their argument is that it always
has to be legal to assign a struct foo * to a struct foo (i.e. do struct foo a
= *foop), and that that may access the trailing padding bytes.
Unfortunately, after staring at the C99 draft, I think the gcc folks are
right, and this undefined behavior. Although it's really hard to tell with the
C99 formulation.
I also looked at C23, and the examples (e.g. ยง 6.7.3.2, paragraph 28) do lend
some additional credence to the position of the gcc folks.
Which isn't great, given how widely we use offsetof(structtype, flex_member)
as the base allocation size.
The historic reason we do that is support for systems that didn't support real
flexible arrays, where we used one element arrays instead. In that case
sizeof(structtype) would obviously return too big a value.
I don't know how likely doing the wrong thing here is to lead to wrong code
generation, but relying on that not to happen doesn't seem great.
I've attached a list of structs that have a flexible array and padding,
generated with
pahole -a --padding_ge=1 --with_flexible_array src/backend/postgres
contrib/*/*.so
which of course is not exhaustive. And will differ between platforms.
One example of where we do this is gtrgm_alloc().
It's not entirely obvious to me what to do here. I think the minimum ought to
to not introduce more uses of offsetof() for flexible-array allocation sizing
in the future. But just leaving the existing uses as-is seems a bit
scary. Fixign them all seems like a lot of fiddly work with some compatibility
hazards :(
Greetings,
Andres Freund
PS: I'm fairly sure that we also are entering UB territory when allocating
space for just the smallest member of a union, as we do a *lot* in numeric.c,
due to all the allocations using NUMERIC_HDRSZ_SHORT. But there's a lot more
standardese to parse to figure out the requirements.
[1]
See also https://godbolt.org/z/98jchG4Ex
/* compiled with gcc -O2 -Warray-bounds */
#include <stddef.h>
struct foo
{
size_t len;
/* warning is only emitted if the "flag" struct member is present */
int flag;
char data[];
};
extern void *allocme(size_t sz) __attribute__((alloc_size(1)));
struct foo* trigger_warning()
{
size_t len = 1;
struct foo *foop;
foop = allocme(offsetof(struct foo, data) + sizeof(char) * len);
foop->len = len;
return foop;
}
struct HeapTupleHeaderData {
union {
HeapTupleFields t_heap; /* 0 12 */
DatumTupleFields t_datum; /* 0 12 */
} t_choice; /* 0 12 */
ItemPointerData t_ctid __attribute__((__aligned__(2))); /*
12 6 */
uint16 t_infomask2; /* 18 2 */
uint16 t_infomask; /* 20 2 */
uint8 t_hoff; /* 22 1 */
bits8 t_bits[]; /* 23 0 */
/* size: 24, cachelines: 1, members: 6 */
/* padding: 1 */
/* forced alignments: 1 */
/* last cacheline: 24 bytes */
} __attribute__((__aligned__(4)));
struct MinimalTupleData {
uint32 t_len; /* 0 4 */
char mt_padding[6]; /* 4 6 */
uint16 t_infomask2; /* 10 2 */
uint16 t_infomask; /* 12 2 */
uint8 t_hoff; /* 14 1 */
bits8 t_bits[]; /* 15 0 */
/* size: 16, cachelines: 1, members: 6 */
/* padding: 1 */
/* last cacheline: 16 bytes */
};
struct BTVacuumPostingData {
IndexTuple itup; /* 0 8 */
OffsetNumber updatedoffset; /* 8 2 */
uint16 ndeletedtids; /* 10 2 */
uint16 deletetids[]; /* 12 0 */
/* size: 16, cachelines: 1, members: 4 */
/* padding: 4 */
/* last cacheline: 16 bytes */
};
struct xl_multixact_create {
MultiXactId mid; /* 0 4 */
/* XXX 4 bytes hole, try to pack */
MultiXactOffset moff; /* 8 8 */
int32 nmembers; /* 16 4 */
MultiXactMember members[]; /* 20 0 */
/* size: 24, cachelines: 1, members: 4 */
/* sum members: 16, holes: 1, sum holes: 4 */
/* padding: 4 */
/* last cacheline: 24 bytes */
};
struct spgxlogVacuumRedirect {
uint16 nToPlaceholder; /* 0 2 */
OffsetNumber firstPlaceholder; /* 2 2 */
TransactionId snapshotConflictHorizon; /* 4 4 */
_Bool isCatalogRel; /* 8 1 */
/* XXX 1 byte hole, try to pack */
OffsetNumber offsets[]; /* 10 0 */
/* size: 12, cachelines: 1, members: 5 */
/* sum members: 9, holes: 1, sum holes: 1 */
/* padding: 2 */
/* last cacheline: 12 bytes */
};
struct TimeZoneAbbrevTable {
Size tblsize; /* 0 8 */
int numabbrevs; /* 8 4 */
datetkn abbrevs[]; /* 12 0 */
/* size: 16, cachelines: 1, members: 3 */
/* padding: 4 */
/* last cacheline: 16 bytes */
};
typedef struct {
char nuls[2]; /* 0 2 */
uint16 len; /* 2 2 */
int32 pid; /* 4 4 */
bits8 flags; /* 8 1 */
char data[]; /* 9 0 */
/* size: 12, cachelines: 1, members: 5 */
/* padding: 3 */
/* last cacheline: 12 bytes */
} PipeProtoHeader;
struct MVDependency {
double degree; /* 0 8 */
AttrNumber nattributes; /* 8 2 */
AttrNumber attributes[]; /* 10 0 */
/* size: 16, cachelines: 1, members: 3 */
/* padding: 6 */
/* last cacheline: 16 bytes */
};
struct ReadStream {
int16 max_ios; /* 0 2 */
int16 io_combine_limit; /* 2 2 */
int16 ios_in_progress; /* 4 2 */
int16 queue_size; /* 6 2 */
int16 max_pinned_buffers; /* 8 2 */
int16 forwarded_buffers; /* 10 2 */
int16 pinned_buffers; /* 12 2 */
int16 distance; /* 14 2 */
int16 initialized_buffers; /* 16 2 */
/* XXX 2 bytes hole, try to pack */
int read_buffers_flags; /* 20 4 */
_Bool sync_mode; /* 24 1 */
_Bool batch_mode; /* 25 1 */
_Bool advice_enabled; /* 26 1 */
_Bool temporary; /* 27 1 */
BlockNumber buffered_blocknum; /* 28 4 */
ReadStreamBlockNumberCB callback; /* 32 8 */
void * callback_private_data; /* 40 8 */
BlockNumber seq_blocknum; /* 48 4 */
BlockNumber seq_until_processed; /* 52 4 */
BlockNumber pending_read_blocknum; /* 56 4 */
int16 pending_read_nblocks; /* 60 2 */
/* XXX 2 bytes hole, try to pack */
/* --- cacheline 1 boundary (64 bytes) --- */
size_t per_buffer_data_size; /* 64 8 */
void * per_buffer_data; /* 72 8 */
InProgressIO * ios; /* 80 8 */
int16 oldest_io_index; /* 88 2 */
int16 next_io_index; /* 90 2 */
_Bool fast_path; /* 92 1 */
/* XXX 1 byte hole, try to pack */
int16 oldest_buffer_index; /* 94 2 */
int16 next_buffer_index; /* 96 2 */
/* XXX 2 bytes hole, try to pack */
Buffer buffers[]; /* 100 0 */
/* size: 104, cachelines: 2, members: 30 */
/* sum members: 93, holes: 4, sum holes: 7 */
/* padding: 4 */
/* last cacheline: 40 bytes */
};
struct shm_mq {
slock_t mq_mutex; /* 0 1 */
/* XXX 7 bytes hole, try to pack */
PGPROC * mq_receiver; /* 8 8 */
PGPROC * mq_sender; /* 16 8 */
pg_atomic_uint64 mq_bytes_read; /* 24 8 */
pg_atomic_uint64 mq_bytes_written; /* 32 8 */
Size mq_ring_size; /* 40 8 */
_Bool mq_detached; /* 48 1 */
uint8 mq_ring_offset; /* 49 1 */
char mq_ring[]; /* 50 0 */
/* size: 56, cachelines: 1, members: 9 */
/* sum members: 43, holes: 1, sum holes: 7 */
/* padding: 6 */
/* last cacheline: 56 bytes */
};
struct StatEntry {
uint32 ndoc; /* 0 4 */
uint32 nentry; /* 4 4 */
struct StatEntry * left; /* 8 8 */
struct StatEntry * right; /* 16 8 */
uint32 lenlexeme; /* 24 4 */
char lexeme[]; /* 28 0 */
/* size: 32, cachelines: 1, members: 6 */
/* padding: 4 */
/* last cacheline: 32 bytes */
};
struct TypeCacheEnumData {
Oid bitmap_base; /* 0 4 */
/* XXX 4 bytes hole, try to pack */
Bitmapset * sorted_values; /* 8 8 */
int num_values; /* 16 4 */
EnumItem enum_values[]; /* 20 0 */
/* size: 24, cachelines: 1, members: 4 */
/* sum members: 16, holes: 1, sum holes: 4 */
/* padding: 4 */
/* last cacheline: 24 bytes */
};
typedef struct {
char nuls[2]; /* 0 2 */
uint16 len; /* 2 2 */
int32 pid; /* 4 4 */
bits8 flags; /* 8 1 */
char data[]; /* 9 0 */
/* size: 12, cachelines: 1, members: 5 */
/* padding: 3 */
/* last cacheline: 12 bytes */
} PipeProtoHeader;
typedef struct {
int32 vl_len_; /* 0 4 */
uint16 numlevel; /* 4 2 */
char data[]; /* 6 0 */
/* size: 8, cachelines: 1, members: 3 */
/* padding: 2 */
/* last cacheline: 8 bytes */
} ltree;
typedef struct {
int32 val; /* 0 4 */
uint16 len; /* 4 2 */
uint8 flag; /* 6 1 */
char name[]; /* 7 0 */
/* size: 8, cachelines: 1, members: 4 */
/* padding: 1 */
/* last cacheline: 8 bytes */
} lquery_variant;
typedef struct {
int32 vl_len_; /* 0 4 */
uint16 numlevel; /* 4 2 */
uint16 firstgood; /* 6 2 */
uint16 flag; /* 8 2 */
char data[]; /* 10 0 */
/* size: 12, cachelines: 1, members: 5 */
/* padding: 2 */
/* last cacheline: 12 bytes */
} lquery;
typedef struct {
int32 vl_len_; /* 0 4 */
uint16 numlevel; /* 4 2 */
char data[]; /* 6 0 */
/* size: 8, cachelines: 1, members: 3 */
/* padding: 2 */
/* last cacheline: 8 bytes */
} ltree;
typedef struct {
int32 vl_len_; /* 0 4 */
uint16 numlevel; /* 4 2 */
uint16 firstgood; /* 6 2 */
uint16 flag; /* 8 2 */
char data[]; /* 10 0 */
/* size: 12, cachelines: 1, members: 5 */
/* padding: 2 */
/* last cacheline: 12 bytes */
} lquery;
typedef struct {
int32 vl_len_; /* 0 4 */
uint16 numlevel; /* 4 2 */
char data[]; /* 6 0 */
/* size: 8, cachelines: 1, members: 3 */
/* padding: 2 */
/* last cacheline: 8 bytes */
} ltree;
typedef struct {
int32 val; /* 0 4 */
uint16 len; /* 4 2 */
uint8 flag; /* 6 1 */
char name[]; /* 7 0 */
/* size: 8, cachelines: 1, members: 4 */
/* padding: 1 */
/* last cacheline: 8 bytes */
} lquery_variant;
typedef struct {
int32 vl_len_; /* 0 4 */
uint16 numlevel; /* 4 2 */
uint16 firstgood; /* 6 2 */
uint16 flag; /* 8 2 */
char data[]; /* 10 0 */
/* size: 12, cachelines: 1, members: 5 */
/* padding: 2 */
/* last cacheline: 12 bytes */
} lquery;
typedef struct {
int32 vl_len_; /* 0 4 */
uint16 numlevel; /* 4 2 */
char data[]; /* 6 0 */
/* size: 8, cachelines: 1, members: 3 */
/* padding: 2 */
/* last cacheline: 8 bytes */
} ltree;
typedef struct {
int32 val; /* 0 4 */
uint16 len; /* 4 2 */
uint8 flag; /* 6 1 */
char name[]; /* 7 0 */
/* size: 8, cachelines: 1, members: 4 */
/* padding: 1 */
/* last cacheline: 8 bytes */
} lquery_variant;
typedef struct {
int32 vl_len_; /* 0 4 */
uint16 numlevel; /* 4 2 */
uint16 firstgood; /* 6 2 */
uint16 flag; /* 8 2 */
char data[]; /* 10 0 */
/* size: 12, cachelines: 1, members: 5 */
/* padding: 2 */
/* last cacheline: 12 bytes */
} lquery;
typedef struct {
int32 vl_len_; /* 0 4 */
uint16 numlevel; /* 4 2 */
char data[]; /* 6 0 */
/* size: 8, cachelines: 1, members: 3 */
/* padding: 2 */
/* last cacheline: 8 bytes */
} ltree;
typedef struct {
int32 val; /* 0 4 */
uint16 len; /* 4 2 */
uint8 flag; /* 6 1 */
char name[]; /* 7 0 */
/* size: 8, cachelines: 1, members: 4 */
/* padding: 1 */
/* last cacheline: 8 bytes */
} lquery_variant;
typedef struct {
int32 vl_len_; /* 0 4 */
uint16 numlevel; /* 4 2 */
uint16 firstgood; /* 6 2 */
uint16 flag; /* 8 2 */
char data[]; /* 10 0 */
/* size: 12, cachelines: 1, members: 5 */
/* padding: 2 */
/* last cacheline: 12 bytes */
} lquery;
typedef struct {
int32 vl_len_; /* 0 4 */
uint16 numlevel; /* 4 2 */
char data[]; /* 6 0 */
/* size: 8, cachelines: 1, members: 3 */
/* padding: 2 */
/* last cacheline: 8 bytes */
} ltree;
typedef struct {
int32 vl_len_; /* 0 4 */
uint16 numlevel; /* 4 2 */
char data[]; /* 6 0 */
/* size: 8, cachelines: 1, members: 3 */
/* padding: 2 */
/* last cacheline: 8 bytes */
} ltree;
typedef struct {
int32 vl_len_; /* 0 4 */
uint16 numlevel; /* 4 2 */
char data[]; /* 6 0 */
/* size: 8, cachelines: 1, members: 3 */
/* padding: 2 */
/* last cacheline: 8 bytes */
} ltree;
typedef struct {
int32 vl_len_; /* 0 4 */
uint8 flag; /* 4 1 */
char data[]; /* 5 0 */
/* size: 8, cachelines: 1, members: 3 */
/* padding: 3 */
/* last cacheline: 8 bytes */
} TRGM;
typedef struct {
int32 vl_len_; /* 0 4 */
uint8 flag; /* 4 1 */
char data[]; /* 5 0 */
/* size: 8, cachelines: 1, members: 3 */
/* padding: 3 */
/* last cacheline: 8 bytes */
} TRGM;
typedef struct {
int32 vl_len_; /* 0 4 */
uint8 flag; /* 4 1 */
char data[]; /* 5 0 */
/* size: 8, cachelines: 1, members: 3 */
/* padding: 3 */
/* last cacheline: 8 bytes */
} TRGM;
typedef struct {
int32 vl_len_; /* 0 4 */
uint8 flag; /* 4 1 */
char data[]; /* 5 0 */
/* size: 8, cachelines: 1, members: 3 */
/* padding: 3 */
/* last cacheline: 8 bytes */
} TRGM;
