Re: [PATCH] Define GEN_AS_CONST_HEADERS when generating header files [BZ #22792]

[Florian Weimer <fweimer at redhat dot com>]

On 02/21/2018 06:19 PM, H.J. Lu wrote:
> On Wed, Feb 21, 2018 at 7:40 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> > On Tue, Feb 20, 2018 at 11:09 PM, Florian Weimer <fweimer@redhat.com> wrote:
> > > On 02/08/2018 06:58 PM, H.J. Lu wrote:
> > > >          [BZ #22792]
> > > >          * Makerules ($(common-objpfx)%.h): Pass -DGEN_AS_CONST_HEADERS
> > > >          to $(CC).
> > > >          * sysdeps/unix/sysv/linux/i386/lowlevellock.h: Include
> > > >          <tcb-offsets.h> only if GEN_AS_CONST_HEADERS isn't defined.
> > > >          * sysdeps/unix/sysv/linux/x86_64/lowlevellock.h: Don't include
> > > >          <tcb-offsets.h>.
> > >
> > >
> > > It looks to me that a cleaner fix would be to move the definition of the TCB
> > > types to a separate header.
> >
> > I will take a look.
>
> sysdeps/i386/nptl/tcb-offsets.sym has
>
> #include <sysdep.h>
> #include <tls.h>
> #include <kernel-features.h>
>
> RESULT offsetof (struct pthread, result)
> TID offsetof (struct pthread, tid)
> CANCELHANDLING offsetof (struct pthread, cancelhandling)
> CLEANUP_JMP_BUF offsetof (struct pthread, cleanup_jmp_buf)
> MULTIPLE_THREADS_OFFSET offsetof (tcbhead_t, multiple_threads)
> SYSINFO_OFFSET offsetof (tcbhead_t, sysinfo)
> CLEANUP offsetof (struct pthread, cleanup)
> CLEANUP_PREV offsetof (struct _pthread_cleanup_buffer, __prev)
> MUTEX_FUTEX offsetof (pthread_mutex_t, __data.__lock)
> POINTER_GUARD offsetof (tcbhead_t, pointer_guard)
> #ifndef __ASSUME_PRIVATE_FUTEX
> PRIVATE_FUTEX offsetof (tcbhead_t, private_futex)
> #endif
>
> It covers more than just tcbhead_t.  A separate header file for tcbhead_t
> won't help here.

Yes, it's more involved.

> > > The circular dependency is just a cosmetic warning from make, right?
> >
> > It depends on how unlucky you are.  In one case on a many-core machine
> > with make -j60, I had to kill glibc build since it never stopped.

If it fixes an actual build issue, then I think your patch is 
acceptable, although I consider it quite hackish.

But please do add a comment explaining the issue before:

+# ifndef GEN_AS_CONST_HEADERS

Thanks,
Florian