[prev in list] [next in list] [prev in thread] [next in thread] 

List:       musl
Subject:    Re: [musl] [PATCH] [RFC] cleanup/refactor pthread_arch.h, pthread_impl.h
From:       Rich Felker <dalias () libc ! org>
Date:       2020-08-27 22:38:58
Message-ID: 20200827223858.GS3265 () brightrain ! aerifal ! cx
[Download RAW message or body]

On Wed, Aug 26, 2020 at 11:52:46PM +0200, Bartosz Brachaczek wrote:
> On Tue, Aug 25, 2020 at 6:43 PM Rich Felker <dalias@libc.org> wrote:
> 
> > The attached patch series refactors fragments from pthread_arch.h to
> > reduce redundancy and allow pthread_arch.h to be included at the top
> > of pthread_impl.h, where it can be used to influence the
> > definition/layout of struct __pthread, rather than the middle of the
> > file. This makes it possible to get rid of the duplicate canary and
> > dtv members that existed just to match multiple ABIs silumtaneously.
> >
> > This involved a good deal of manual conversion/deduplication so it's
> > possible there are bugs for some archs. That's why I've posted the
> > series for review rather than just pushing. Reports of success/failure
> > (disassembly of pthread_self.o before/after probably suffice to
> > confirm no regression) would be very helpful.
> >
> 
> I mechanically confirmed for all archs that with the first two patches
> applied, disassembly of pthread_self.o before/after doesn't change. The
> third patch obviously changes the offset for most archs due to change in
> sizeof struct pthread.
> 
> Only other remark: ricv64's __get_tp misses the change of the variable type
> from char * to uintptr_t.

Thanks -- that's very helpful. It also raises a point that we should
probably add -Werror=int-conversion to the default CFLAGS. As far as I
can tell it only catches invalid (constraint-violation) implicit
conversions like the one you found, and does not have any
style-policing component to it.

Rich
[prev in list] [next in list] [prev in thread] [next in thread]