This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] elf.h SHF_EXCLUDE signed int 31 bit shift triggers undefined behaviour.
- From: Mark Wielaard <mjw at redhat dot com>
- To: Szabolcs Nagy <szabolcs dot nagy at arm dot com>
- Cc: "libc-alpha at sourceware dot org" <libc-alpha at sourceware dot org>, Josh Stone <jistone at redhat dot com>
- Date: Tue, 21 Apr 2015 11:20:58 +0200
- Subject: Re: [PATCH] elf.h SHF_EXCLUDE signed int 31 bit shift triggers undefined behaviour.
- Authentication-results: sourceware.org; auth=none
- References: <1427193579-26102-1-git-send-email-mjw at redhat dot com> <55117118 dot 1080706 at arm dot com> <20150324211541 dot GA2318 at blokker dot redhat dot com>
Hi,
On Tue, 2015-03-24 at 22:15 +0100, Mark Wielaard wrote:
> On Tue, Mar 24, 2015 at 02:13:44PM +0000, Szabolcs Nagy wrote:
> > On 24/03/15 10:39, Mark Wielaard wrote:
> > > Any use of SHF_EXCLUDE in code that tries to check it against sh_flags
> > > will trigger undefined behaviour because it is defined as a 31 bit shift
> > > against an signed integer. Fix by explicitly using an unsigned int.
> >
> > there is another proposed patch for this
> >
> > https://sourceware.org/ml/libc-alpha/2015-03/msg00287.html
>
> I missed that one. It does seem more ambitious than what I am proposing.
> It is probably a good idea to change every constant to the appropriate
> unsigned type. But the testing requirements seem hard to satisfy and it
> looks like that patch is stalled because of that.
So I participated in that other thread, but it seems a rewrite of elf.h
to use the "correct type" for each constant is not so tractable as
hoped. So it seems that patch is stalled. Since the SHF_EXCLUDE issue is
real (it keeps being reported over and over again against elfutils) and
easy to fix itself with this one character patch, could you reconsider
just applying this now?
> Could this simpler patch that just fixes the one constant that does
> have a real problem in practice when used be fixed independently?
> I like building my project with gcc -fsanitize=undefined and the
> usage of SHF_EXCLUDE is preventing that atm.
>
> > > ChangeLog | 4 ++++
> > > elf/elf.h | 2 +-
> > > 2 files changed, 5 insertions(+), 1 deletion(-)
> > >
> >
> > i think changelog entries are supposed to be submitted separately
>
> OK. How about the following then?
>
> I would appreciate it if someone could push that for me since I don't
> have glibc commit access.
> 2015-03-24 Mark Wielaard <mjw@redhat.com>
>
> * elf/elf.h (SHF_EXCLUDE): Use unsigned 1 for shift.
diff --git a/elf/elf.h b/elf/elf.h
index 496f08d..960a3c3 100644
--- a/elf/elf.h
+++ b/elf/elf.h
@@ -371,7 +371,7 @@ typedef struct
#define SHF_MASKPROC 0xf0000000 /* Processor-specific */
#define SHF_ORDERED (1 << 30) /* Special ordering requirement
(Solaris). */
-#define SHF_EXCLUDE (1 << 31) /* Section is excluded unless
+#define SHF_EXCLUDE (1U << 31) /* Section is excluded unless
referenced or allocated (Solaris).*/
/* Section group handling. */