Build problem with ToT GCC
Commit Message
Can you try this change (on branch roland/dl-nns) with that compiler?
I suspect a compile-time constant preventing evaluation of the
expressions doing indexing will avoid the warning. If it doesn't,
then the right thing to do is to put that inside #if DL_NNS > 1.
While I was there I noticed that it's not properly checking for wildly
bogus NSID values that would make that indexing bogus at runtime (in
the SHARED case), so I put that in too.
Thanks,
Roland
2015-04-17 Roland McGrath <roland@hack.frob.com>
* elf/dl-open.c (_dl_open): Use __glibc_unlikely in invalid namespace
check. Reject NSID < 0 and NSID >= dl_nns, and check for DL_NNS==1,
before using NSID as an index.
Comments
On Fri, 2015-04-17 at 12:20 -0700, Roland McGrath wrote:
> Can you try this change (on branch roland/dl-nns) with that compiler?
> I suspect a compile-time constant preventing evaluation of the
> expressions doing indexing will avoid the warning. If it doesn't,
> then the right thing to do is to put that inside #if DL_NNS > 1.
>
> While I was there I noticed that it's not properly checking for wildly
> bogus NSID values that would make that indexing bogus at runtime (in
> the SHARED case), so I put that in too.
>
>
> Thanks,
> Roland
>
>
> 2015-04-17 Roland McGrath <roland@hack.frob.com>
>
> * elf/dl-open.c (_dl_open): Use __glibc_unlikely in invalid namespace
> check. Reject NSID < 0 and NSID >= dl_nns, and check for DL_NNS==1,
> before using NSID as an index.
This patch did fix the problem in dl-open.c but dl-close.c has the same
issue and would need the same fix.
Steve Ellcey
sellcey@imgtec.com
> This patch did fix the problem in dl-open.c but dl-close.c has the same
> issue and would need the same fix.
Please show that error.
On Fri, 2015-04-17 at 12:58 -0700, Roland McGrath wrote:
> > This patch did fix the problem in dl-open.c but dl-close.c has the same
> > issue and would need the same fix.
>
> Please show that error.
dl-close.c: In function '_dl_close_worker':
dl-close.c:136:42: error: array subscript is outside array bounds
[-Werror=array-bounds]
struct link_namespaces *ns = &GL(dl_ns)[nsid];
^
cc1: all warnings being treated as errors
On Fri, 2015-04-17 at 13:02 -0700, Steve Ellcey wrote:
> On Fri, 2015-04-17 at 12:58 -0700, Roland McGrath wrote:
> > > This patch did fix the problem in dl-open.c but dl-close.c has the same
> > > issue and would need the same fix.
> >
> > Please show that error.
>
> dl-close.c: In function '_dl_close_worker':
> dl-close.c:136:42: error: array subscript is outside array bounds
> [-Werror=array-bounds]
> struct link_namespaces *ns = &GL(dl_ns)[nsid];
> ^
> cc1: all warnings being treated as errors
Weird, I assumed that the dl-close.c issue was the same as the dl-open.c
problem. But it looks different. After cutting it down with delta I
get the following small test case and error. I do not see how GCC can
know that nsid is not 0.
Steve Ellcey
sellcey@imgtec.com
extern void bad (const char *__assertion) __attribute__ ((__nothrow__ ))
__attribute__ ((__noreturn__));
struct link_map { long int l_ns; };
extern struct link_namespaces
{
unsigned int _ns_nloaded;
} _dl_ns[1];
void _dl_close_worker (struct link_map *map)
{
long int nsid = map->l_ns;
struct link_namespaces *ns = &_dl_ns[nsid];
(nsid != 0) ? (void) (0) : bad ("nsid != 0");
--ns->_ns_nloaded;
% inst*/bin/*-gcc -O2 -Wall -Werror x.c
x.c: In function '_dl_close_worker':
x.c:10:39: error: array subscript is outside array bounds [-Werror=array-bounds]
struct link_namespaces *ns = &_dl_ns[nsid];
^
x.c:10:39: error: array subscript is outside array bounds [-Werror=array-bounds]
cc1: all warnings being treated as errors
@@ -619,8 +619,14 @@ no more namespaces available for dlmopen()"));
/* Never allow loading a DSO in a namespace which is empty. Such
direct placements is only causing problems. Also don't allow
loading into a namespace used for auditing. */
- else if (__builtin_expect (nsid != LM_ID_BASE && nsid != __LM_ID_CALLER, 0)
- && (GL(dl_ns)[nsid]._ns_nloaded == 0
+ else if (__glibc_unlikely (nsid != LM_ID_BASE && nsid != __LM_ID_CALLER)
+ && (__glibc_unlikely (nsid < 0 || nsid >= GL(dl_nns))
+ /* This prevents the [NSID] index expressions from being
+ evaluated, so the compiler won't think that we are
+ accessing an invalid index here in the !SHARED case where
+ DL_NNS is 1 and so any NSID != 0 is invalid. */
+ || DL_NNS == 1
+ || GL(dl_ns)[nsid]._ns_nloaded == 0
|| GL(dl_ns)[nsid]._ns_loaded->l_auditing))
_dl_signal_error (EINVAL, file, NULL,
N_("invalid target namespace in dlmopen()"));