public inbox for linux-m68k@lists.linux-m68k.org
 help / color / mirror / Atom feed
From: David Laight <David.Laight@ACULAB.COM>
To: 'Linus Torvalds' <torvalds@linux-foundation.org>,
	Guenter Roeck <linux@roeck-us.net>
Cc: Rasmus Villemoes <rasmus.villemoes@prevas.dk>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	"Jason A. Donenfeld" <Jason@zx2c4.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-kbuild@vger.kernel.org" <linux-kbuild@vger.kernel.org>,
	"linux-arch@vger.kernel.org" <linux-arch@vger.kernel.org>,
	"linux-toolchains@vger.kernel.org"
	<linux-toolchains@vger.kernel.org>,
	Masahiro Yamada <masahiroy@kernel.org>,
	Kees Cook <keescook@chromium.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"linux-m68k@lists.linux-m68k.org"
	<linux-m68k@lists.linux-m68k.org>
Subject: RE: [PATCH v2] kbuild: treat char as always unsigned
Date: Thu, 22 Dec 2022 10:41:52 +0000	[thread overview]
Message-ID: <b2144334261246aa8dc5004c5f1a58c9@AcuMS.aculab.com> (raw)
In-Reply-To: <CAHk-=wj7FMFLr9AOW9Aa9ZMt1-Lu01_X8vLiaKosPyF2H-+ujA@mail.gmail.com>

From: Linus Torvalds
> Sent: 21 December 2022 17:07
> 
> On Wed, Dec 21, 2022 at 7:56 AM Guenter Roeck <linux@roeck-us.net> wrote:
> >
> > The above assumes an unsigned char as input to strcmp(). I consider that
> > a hypothetical problem because "comparing" strings with upper bits
> > set doesn't really make sense in practice (How does one compare Günter
> > against Gunter ? And how about Gǖnter ?). On the other side, the problem
> > observed here is real and immediate.
> 
> POSIX does actually specify "Günter" vs "Gunter".
> 
> The way strcmp is supposed to work is to return the sign of the
> difference between the byte values ("unsigned char").
> 
> But that sign has to be computed in 'int', not in 'signed char'.
> 
> So yes, the m68k implementation is broken regardless, but with a
> signed char it just happened to work for the US-ASCII case that the
> crypto case tested.
> 
> I think the real fix is to just remove that broken implementation
> entirely, and rely on the generic one.

I wonder how much slower it is - m68k is likely to be microcoded
and I don't think instruction timings are actually available.
The fastest version probably uses subx (with carry) to generate
0/-1 and leaves +delta for the other result - but getting the
compares and branches in the right order is hard.

I believe some of the other m68k asm functions are also missing
the "memory" 'clobber' and so could get mis-optimised.
While I can write (or rather have written) m68k asm I don't have
a compiler.

I also suspect that any x86 code that uses 'rep scas' is going
to be slow on anything modern.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

  parent reply	other threads:[~2022-12-22 10:42 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <Y1BcpXAjR4tmV6RQ@zx2c4.com>
     [not found] ` <20221019203034.3795710-1-Jason@zx2c4.com>
2022-12-21 14:53   ` [PATCH v2] kbuild: treat char as always unsigned Guenter Roeck
2022-12-21 15:05     ` Geert Uytterhoeven
2022-12-21 15:23       ` Guenter Roeck
2022-12-21 15:29       ` Rasmus Villemoes
2022-12-21 15:56         ` Guenter Roeck
2022-12-21 17:06           ` Linus Torvalds
2022-12-21 17:19             ` Guenter Roeck
2022-12-21 18:46               ` Linus Torvalds
2022-12-21 19:08                 ` Linus Torvalds
2022-12-21 21:01                 ` Guenter Roeck
2022-12-22 13:05                 ` Geert Uytterhoeven
2022-12-22 10:41             ` David Laight [this message]
     [not found]               ` <f02e0ac7f2d805020a7ba66803aaff3e31b5eeff.camel@t-online.de>
2022-12-24  9:47                 ` Geert Uytterhoeven
2022-12-30 11:39                 ` David Laight
2022-12-30 13:13                   ` David Laight
2023-01-02  8:29                   ` Geert Uytterhoeven
2022-12-21 17:49           ` Andreas Schwab
2022-12-21 16:57         ` Geert Uytterhoeven

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b2144334261246aa8dc5004c5f1a58c9@AcuMS.aculab.com \
    --to=david.laight@aculab.com \
    --cc=Jason@zx2c4.com \
    --cc=akpm@linux-foundation.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=geert@linux-m68k.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=keescook@chromium.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-m68k@lists.linux-m68k.org \
    --cc=linux-toolchains@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=masahiroy@kernel.org \
    --cc=rasmus.villemoes@prevas.dk \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox