* [PATCH] dlfcn: Avoid one-element flexible array in Dl_serinfo
@ 2019-05-23 9:34 Florian Weimer
2019-05-23 22:42 ` Paul Eggert
0 siblings, 1 reply; 11+ messages in thread
From: Florian Weimer @ 2019-05-23 9:34 UTC (permalink / raw)
To: libc-alpha
The dls_serpath path field, as an array of length 1, introduces
unexpected array subscript checks with some compilers. Using a
zero-length array (a GNU extension) avoids that. The anonymous union
preserves the original size of the type.
2019-05-23 Florian Weimer <fweimer@redhat.com>
[BZ #24166]
* dlfcn/dlfcn.h (Dl_serinfo): Do not use array of length 1 for
dls_serpath field.
diff --git a/dlfcn/dlfcn.h b/dlfcn/dlfcn.h
index 896ad6fc9b..2ffb13d424 100644
--- a/dlfcn/dlfcn.h
+++ b/dlfcn/dlfcn.h
@@ -180,7 +180,17 @@ 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__
+ /* This avoids 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 /* !__GNUC__ */
Dl_serpath dls_serpath[1]; /* Actually longer, dls_cnt elements. */
+# endif /* !__GNUC__ */
} Dl_serinfo;
#endif /* __USE_GNU */
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] dlfcn: Avoid one-element flexible array in Dl_serinfo
2019-05-23 9:34 [PATCH] dlfcn: Avoid one-element flexible array in Dl_serinfo Florian Weimer
@ 2019-05-23 22:42 ` Paul Eggert
2019-05-23 22:50 ` Joseph Myers
2019-05-24 8:42 ` Florian Weimer
0 siblings, 2 replies; 11+ messages in thread
From: Paul Eggert @ 2019-05-23 22:42 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha
[-- Attachment #1: Type: text/plain, Size: 741 bytes --]
On 5/23/19 2:34 AM, Florian Weimer wrote:
> +# ifdef __GNUC__
> + /* This avoids 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 /* !__GNUC__ */
> Dl_serpath dls_serpath[1]; /* Actually longer, dls_cnt elements. */
> +# endif /* !__GNUC__ */
Since this is actually a flexible array member, shouldn't we be using
C99's support for that if available, instead? Something like the
attached untested patch, say. We've been using a FLEXIBLE_ARRAY_MEMBER
macro in Gnulib for quite some time to do this sort of thing.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: dlfcn.diff --]
[-- Type: text/x-patch; name="dlfcn.diff", Size: 1503 bytes --]
diff --git a/dlfcn/dlfcn.h b/dlfcn/dlfcn.h
index 896ad6fc9b..01e6fdadc3 100644
--- a/dlfcn/dlfcn.h
+++ b/dlfcn/dlfcn.h
@@ -180,7 +180,7 @@ typedef struct
{
size_t dls_size; /* Size in bytes of the whole buffer. */
unsigned int dls_cnt; /* Number of elements in `dls_serpath'. */
- Dl_serpath dls_serpath[1]; /* Actually longer, dls_cnt elements. */
+ Dl_serpath dls_serpath[__GLIBC_FLEXIBLE_ARRAY_MEMBER /* dls_cnt */];
} Dl_serinfo;
#endif /* __USE_GNU */
diff --git a/include/features.h b/include/features.h
index e016b3e5c7..9942984e57 100644
--- a/include/features.h
+++ b/include/features.h
@@ -141,6 +141,7 @@
#undef __KERNEL_STRICT_NAMES
#undef __GLIBC_USE_DEPRECATED_GETS
#undef __GLIBC_USE_DEPRECATED_SCANF
+#undef __GLIBC_FLEXIBLE_ARRAY_MEMBER
/* Suppress kernel-name space pollution unless user expressedly asks
for it. */
@@ -423,6 +424,15 @@
# define __GLIBC_USE_DEPRECATED_SCANF 0
#endif
+/* 'struct { ...; t m[__GLIBC_FLEXIBLE_ARRAY_MEMBER]; }’ declares a
+ structure with a flexible array member m at the end in C99 or later,
+ and a structure with a size-1 array member with earlier compilers. */
+#if defined __STDC_VERSION__ && __STDC_VERSION__ >= 199901L
+# define __GLIBC_FLEXIBLE_ARRAY_MEMBER
+#else
+# define __GLIBC_FLEXIBLE_ARRAY_MEMBER 1
+#endif
+
/* Get definitions of __STDC_* predefined macros, if the compiler has
not preincluded this header automatically. */
#include <stdc-predef.h>
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] dlfcn: Avoid one-element flexible array in Dl_serinfo
2019-05-23 22:42 ` Paul Eggert
@ 2019-05-23 22:50 ` Joseph Myers
2019-05-24 8:42 ` Florian Weimer
1 sibling, 0 replies; 11+ messages in thread
From: Joseph Myers @ 2019-05-23 22:50 UTC (permalink / raw)
To: Paul Eggert; +Cc: Florian Weimer, libc-alpha
On Thu, 23 May 2019, Paul Eggert wrote:
> On 5/23/19 2:34 AM, Florian Weimer wrote:
> > +# ifdef __GNUC__
> > + /* This avoids 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 /* !__GNUC__ */
> > Dl_serpath dls_serpath[1]; /* Actually longer, dls_cnt elements.
> > */
> > +# endif /* !__GNUC__ */
>
> Since this is actually a flexible array member, shouldn't we be using C99's
> support for that if available, instead? Something like the attached untested
> patch, say. We've been using a FLEXIBLE_ARRAY_MEMBER macro in Gnulib for quite
> some time to do this sort of thing.
Since we already have the __flexarr macro in sys/cdefs.h, I don't think
having a slightly different __GLIBC_FLEXIBLE_ARRAY_MEMBER as well seems
like a good idea.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] dlfcn: Avoid one-element flexible array in Dl_serinfo
2019-05-23 22:42 ` Paul Eggert
2019-05-23 22:50 ` Joseph Myers
@ 2019-05-24 8:42 ` Florian Weimer
2019-05-24 23:47 ` Paul Eggert
1 sibling, 1 reply; 11+ messages in thread
From: Florian Weimer @ 2019-05-24 8:42 UTC (permalink / raw)
To: Paul Eggert; +Cc: libc-alpha
* Paul Eggert:
> On 5/23/19 2:34 AM, Florian Weimer wrote:
>> +# ifdef __GNUC__
>> + /* This avoids 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 /* !__GNUC__ */
>> Dl_serpath dls_serpath[1]; /* Actually longer, dls_cnt elements. */
>> +# endif /* !__GNUC__ */
>
> Since this is actually a flexible array member, shouldn't we be using
> C99's support for that if available, instead? Something like the
> attached untested patch, say. We've been using a FLEXIBLE_ARRAY_MEMBER
> macro in Gnulib for quite some time to do this sort of thing.
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.
Thanks,
Florian
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] dlfcn: Avoid one-element flexible array in Dl_serinfo
2019-05-24 8:42 ` Florian Weimer
@ 2019-05-24 23:47 ` Paul Eggert
2019-05-27 19:04 ` Florian Weimer
0 siblings, 1 reply; 11+ messages in thread
From: Paul Eggert @ 2019-05-24 23:47 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] dlfcn: Avoid one-element flexible array in Dl_serinfo
2019-05-24 23:47 ` Paul Eggert
@ 2019-05-27 19:04 ` Florian Weimer
2019-06-01 8:48 ` Paul Eggert
2019-06-03 7:59 ` Florian Weimer
0 siblings, 2 replies; 11+ messages in thread
From: Florian Weimer @ 2019-05-27 19:04 UTC (permalink / raw)
To: Paul Eggert; +Cc: libc-alpha
* 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.
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 */
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] dlfcn: Avoid one-element flexible array in Dl_serinfo
2019-05-27 19:04 ` Florian Weimer
@ 2019-06-01 8:48 ` Paul Eggert
2019-06-03 7:59 ` Florian Weimer
1 sibling, 0 replies; 11+ messages in thread
From: Paul Eggert @ 2019-06-01 8:48 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha
Thanks, it looks OK to me.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] dlfcn: Avoid one-element flexible array in Dl_serinfo
2019-05-27 19:04 ` Florian Weimer
2019-06-01 8:48 ` Paul Eggert
@ 2019-06-03 7:59 ` Florian Weimer
1 sibling, 0 replies; 11+ messages in thread
From: Florian Weimer @ 2019-06-03 7:59 UTC (permalink / raw)
To: libc-alpha
* 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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] dlfcn: Avoid one-element flexible array in Dl_serinfo
@ 2019-06-03 15:37 Florian Weimer
2019-06-18 14:27 ` Florian Weimer
0 siblings, 1 reply; 11+ messages in thread
From: Florian Weimer @ 2019-06-03 15:37 UTC (permalink / raw)
To: libc-alpha
The dls_serpath path field, as an array of length 1, introduces
unexpected array subscript checks with some compilers.
GCC versions before 3.0 treat the nested anonymous union as a
declaration of an unnamed type, and not as a member declaration,
so this construct cannot be used for these compilers.
2019-06-03 Florian Weimer <fweimer@redhat.com>
[BZ #24166]
* dlfcn/dlfcn.h (Dl_serinfo): Do not use array of length 1 for
dls_serpath field.
diff --git a/dlfcn/dlfcn.h b/dlfcn/dlfcn.h
index 896ad6fc9b..c550371999 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'. */
+# if __GNUC_PREREQ (3, 0)
+ /* 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
Dl_serpath dls_serpath[1]; /* Actually longer, dls_cnt elements. */
+# endif
} Dl_serinfo;
#endif /* __USE_GNU */
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] dlfcn: Avoid one-element flexible array in Dl_serinfo
2019-06-03 15:37 Florian Weimer
@ 2019-06-18 14:27 ` Florian Weimer
2019-06-18 22:17 ` Paul Eggert
0 siblings, 1 reply; 11+ messages in thread
From: Florian Weimer @ 2019-06-18 14:27 UTC (permalink / raw)
To: libc-alpha; +Cc: Paul Eggert
* Florian Weimer:
> The dls_serpath path field, as an array of length 1, introduces
> unexpected array subscript checks with some compilers.
>
> GCC versions before 3.0 treat the nested anonymous union as a
> declaration of an unnamed type, and not as a member declaration,
> so this construct cannot be used for these compilers.
>
> 2019-06-03 Florian Weimer <fweimer@redhat.com>
>
> [BZ #24166]
> * dlfcn/dlfcn.h (Dl_serinfo): Do not use array of length 1 for
> dls_serpath field.
>
> diff --git a/dlfcn/dlfcn.h b/dlfcn/dlfcn.h
> index 896ad6fc9b..c550371999 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'. */
> +# if __GNUC_PREREQ (3, 0)
> + /* 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
> Dl_serpath dls_serpath[1]; /* Actually longer, dls_cnt elements. */
> +# endif
> } Dl_serinfo;
> #endif /* __USE_GNU */
>
Ping?
Thanks,
Florian
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] dlfcn: Avoid one-element flexible array in Dl_serinfo
2019-06-18 14:27 ` Florian Weimer
@ 2019-06-18 22:17 ` Paul Eggert
0 siblings, 0 replies; 11+ messages in thread
From: Paul Eggert @ 2019-06-18 22:17 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha
That looks OK to me, thanks. (Sorry, I thought I had already said "LGTM".)
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-06-18 22:17 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-23 9:34 [PATCH] dlfcn: Avoid one-element flexible array in Dl_serinfo Florian Weimer
2019-05-23 22:42 ` Paul Eggert
2019-05-23 22:50 ` Joseph Myers
2019-05-24 8:42 ` Florian Weimer
2019-05-24 23:47 ` Paul Eggert
2019-05-27 19:04 ` Florian Weimer
2019-06-01 8:48 ` Paul Eggert
2019-06-03 7:59 ` Florian Weimer
-- strict thread matches above, loose matches on Subject: below --
2019-06-03 15:37 Florian Weimer
2019-06-18 14:27 ` Florian Weimer
2019-06-18 22:17 ` Paul Eggert
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).