Re: [PATCH 4/4] arm: Enable ARM mode for armv6 strlen

[Phil Blundell <pb at pbcl dot net>]

On Fri, 2018-04-13 at 13:42 -0300, Adhemerval Zanella wrote:
> My understanding is glibc try support kernel with missing
> functionalities but either using fallback implementations (either
> subpar with underlying issues as for old pipe2), emulation (as for
> copy_file_range which tries to emulate the kernel behaviour), or by
> just warning userland the kernel do not provide the underlying
> support (for instance set_robust_list).

It's true that glibc has tried (with varying levels of success) to
provide compatibility implementations so that new APIs can be used on
older kernels which lack some or other functionality.  But this doesn't
extend to working around arbitrarily broken kernel configurations.

The cost of enabling CONFIG_ARM_THUMB in your kernel if you're building
for ARMv7 anyway is so close to zero as to be completely negligible. 
There is simply no rational reason to not have it included.  It would
be interesting to ask the guy who filed that bugzilla ticket whether he
has turned it off deliberately, and if so why.  I suspect he probably
just didn't realise it was needed, rather than actively wanting it off.

> I really think just saying 'go fix your kernel' is not the correct
> answer, even more when the configuration used is supported upstream
> (it not an experimental or out-of-tree one). 

There are any number of other ways in which you can break your system
by configuring your kernel wrongly.  The fact that the kernel config
system lets you turn a given option on or off doesn't mean you can just
flip switches at random and expect things to work.  That said, in the
particular case of CONFIG_ARM_THUMB, I think the kernel people should
simply force this on when CONFIG_CPU_V7 is selected.

> Also for specific case, my wild guess is using ARM code path should
> not shown in much difference and will prevent such possible issues.

In this particular case you're right, the ARM implementation is
probably not going to perform very much worse, and now that you've
fixed at least the obvious bugs I think it will probably work fine.  

My concern is about the precedent it sets for the future.  Right now,
it clearly is not true that building glibc for ARMv7 with -marm will
use only A32 instructions.  Further, as far as I can tell it has never
been true, so there cannot be any existing users who are depending on
this behaviour.  If we fix this bug in the way that you are proposing,
we would be making an implicit promise that any future ARMv7-optimised
assembly code is also sensitive to being compiled under -marm and will
avoid Thumb2 instructions in that situation. 

So, all in all it seems we have a choice between:

- we just tell this one guy "sorry, you have to enable CONFIG_ARM_THUMB
in your kernel for glibc to work on ARMv7", he enables the option, it
doesn't cost him anything, and everyone moves on; or

- we fix glibc to avoid Thumb2 instructions in these assembly files,
which means we now have an extra variant that we have to maintain and
test, we need to remember to add the same checks to any future assembly
code that might be added for v7 in the future, and we essentially can't
ever stop doing that for fear that users might have become dependent on
it

I would prefer to do the former.

p.