dlfcn: Avoid one-element flexible array in Dl_serinfo
Commit Message
* Paul Eggert:
> On 5/24/19 1:42 AM, Florian Weimer wrote:
>> This changes the size of the type and is not source-code-compatible. I
>> have not investigated whether the change is still reasonably safe, but
>> usually, wo do not make such changes.
>
> OK, in that case I suggest adding a comment explaining the situation,
> since it is a bit of a sore thumb. Something like the following
> perhaps? Or if this problem is likely to occur elsewhere, we could
> package the situation up into a macro and just use the macro here.
>
> /* An array of dls_cnt elements, each of type Dl_serpath. */
> #if 0
> /* With no backward-compatibility concerns we’d use the following
> C99 flexible array member. However, as this data structure
> predates C99 it had to contain a one-element array here, and we
> don't want to change the struct's size now. */
> Dl_serpath dls_serpath[];
> #elif defined __GNUC__
> /* Avoid an unwanted array subscript check by the compiler, while
> preserving the size of the type. */
> __extension__ union
> {
> Dl_serpath dls_serpath[0]; /* Actually longer, dls_cnt elements. */
> Dl_serpath __dls_serpath_pad[1];
> };
> #else
> Dl_serpath dls_serpath[1];
> #endif
The real issue here is that GNU C allows nested flexible array members,
but not in unions.
I still think this is best discussed in the commit message because such
header comments tend not to age well, but I've proposed a patch below.
Thanks,
Florian
dlfcn: Avoid one-element flexible array in Dl_serinfo
The dls_serpath path field, as an array of length 1, introduces
unexpected array subscript checks with some compilers.
2019-05-27 Florian Weimer <fweimer@redhat.com>
[BZ #24166]
* dlfcn/dlfcn.h (Dl_serinfo): Do not use array of length 1 for
dls_serpath field.
Comments
Thanks, it looks OK to me.
* Florian Weimer:
> diff --git a/dlfcn/dlfcn.h b/dlfcn/dlfcn.h
> index 896ad6fc9b..7107b3ea9a 100644
> --- a/dlfcn/dlfcn.h
> +++ b/dlfcn/dlfcn.h
> @@ -180,7 +180,19 @@ typedef struct
> {
> size_t dls_size; /* Size in bytes of the whole buffer. */
> unsigned int dls_cnt; /* Number of elements in `dls_serpath'. */
> +# ifdef __GNUC__
> + /* The zero-length array avoids an unwanted array subscript check by
> + the compiler, while the surrounding anonymous union preserves the
> + historic size of the type. At the time of writing, GNU C does
> + not support structs with flexible array members in unions. */
> + __extension__ union
> + {
> + Dl_serpath dls_serpath[0]; /* Actually longer, dls_cnt elements. */
> + Dl_serpath __dls_serpath_pad[1];
> + };
> +# else /* !__GNUC__ */
> Dl_serpath dls_serpath[1]; /* Actually longer, dls_cnt elements. */
> +# endif /* !__GNUC__ */
> } Dl_serinfo;
> #endif /* __USE_GNU */
>
It turns out that GCC 2.7.2.3 treats this anonymous union as a type
declaration and ignores it. I will try to come up with the appropriate
__GNUC_PREREQ conditional.
(Don't get the wrong idea; we do not test regularly against older GCC
versions.)
Thanks,
Florian
@@ -180,7 +180,19 @@ typedef struct
{
size_t dls_size; /* Size in bytes of the whole buffer. */
unsigned int dls_cnt; /* Number of elements in `dls_serpath'. */
+# ifdef __GNUC__
+ /* The zero-length array avoids an unwanted array subscript check by
+ the compiler, while the surrounding anonymous union preserves the
+ historic size of the type. At the time of writing, GNU C does
+ not support structs with flexible array members in unions. */
+ __extension__ union
+ {
+ Dl_serpath dls_serpath[0]; /* Actually longer, dls_cnt elements. */
+ Dl_serpath __dls_serpath_pad[1];
+ };
+# else /* !__GNUC__ */
Dl_serpath dls_serpath[1]; /* Actually longer, dls_cnt elements. */
+# endif /* !__GNUC__ */
} Dl_serinfo;
#endif /* __USE_GNU */