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

[Marcin Kościelnicki]

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.
>
>
>