Re: [PATCH][BZ 18960] setlocale.c: Mark *_used symbols as unaligned.

[Marcin KoÅcielnicki <koriakin at 0x04 dot net>]

On 28/09/15 16:17, Carlos O'Donell wrote:
> On 09/28/2015 09:16 AM, Florian Weimer wrote:
> > On 09/27/2015 03:26 PM, Marcin KoÅcielnicki wrote:
> > > This ensures that compiler doesn't get the values of these symbols
> > > using instructions that have alignment requirements (eg. larl on s390).
> >
> > The commit message should perhaps say âthe address of these variablesâ
> > because the trigger for this issue is in C code, so it makes sense to
> > use C terms.
> >
> > > ---
> > >   locale/setlocale.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/locale/setlocale.c b/locale/setlocale.c
> > > index ead030d..028496d 100644
> > > --- a/locale/setlocale.c
> > > +++ b/locale/setlocale.c
> > > @@ -35,7 +35,7 @@
> > >      Also use a weak reference for the _nl_current_CATEGORY thread variable.  */
> > >
> > >   # define DEFINE_CATEGORY(category, category_name, items, a) \
> > > -    extern char _nl_current_##category##_used; \
> > > +    extern char _nl_current_##category##_used __attribute__((__aligned__(1))); \
> >
> > This is a very gray area as far as GCC is concerned.  This side effect
> > of attributed âalignedâ is not documented, and I'm not sure if we can
> > rely on it.  It's a bit like making a non-weak function symbol zero.

FWIW, it seems to be quite purposeful, but s390 specific: 
https://github.com/gcc-mirror/gcc/blob/gcc_5_2_0_release/gcc/config/s390/s390.c#L11798
> > Maybe the better approach would be to change 1 to 8 or 16.
>
> Agreed.
>
> See locale/localeinfo.h and adjust the asm to use 16, and see if that fixes
> the problem. If it does, you'll need a large comment there explaining why
> 16 is important and should not be changed.

Yeah, that's another way to fix the problem. I picked the aligned 
attribute since I also had to fix a similiar problem in gold testsuite, 
which actually relied on symbols having unaligned values. But "use 16" 
seems just fine for glibc.
> I'm surpised there aren't other instances where this doesn't cause problems,
> but there are indeed few of these cases where data addresses are used, and
> AFAICT only one which is type 'char', this one. The other instance is _dl_rtld_map
> which has a larger alignment.

There's one major problem: GNU ld does not emit any diagnostics about 
misaligned larl target, it just happily forces the LSB to 0. So there 
may be a lot of problems that noone noticed.

As for _dl_rtld_map, that'd mean miscalculating _end or _edata by 1 if 
they're unaligned (which is likely quite rare). Sounds rather hard to 
notice.
> A function address would have all the requirements for alignment that you would
> need to avoid this problem.

Sorry, I don't get what you're talking about.
> Cheers,
> Carlos.