ARM strchr fails to convert c to char

Bug #842258 reported by Colin Watson on 2011-09-06
This bug affects 1 person
Affects Status Importance Assigned to Milestone
eglibc (Ubuntu)
Dr. David Alan Gilbert

Bug Description

C99 says:

  "The strchr function locates the first occurrence of c (converted to a char) in the string pointed to by s."

The current ARM strchr implementation in eglibc (2.13-17ubuntu2) starts off like this:

  ldrb r2,[r0],#1
  cmp r2,r1

This loads a byte from the address pointed to by the first argument (s), zero-extends it to 32 bits, and then compares it directly against the second argument (c). If c is negative, this fails.

I think that this function should first convert c to a char, e.g. by zeroing the top 24 bits. char is unsigned on this platform, so (char) -1 == (int) 255.

Here's a test program. By my reading of C99, it should return 0. On Ubuntu 11.10 armel, it currently returns 1. (This is the root cause of bug 791274, although it's easily worked around by passing the anyway less obtuse value of 255 rather than -1.)

#include <string.h>
int main(int argc, char **argv) {
        const char *s = "\xff";
        if (strchr (s, -1) == s)
                return 0;
                return 1;

Related branches

Changed in eglibc (Ubuntu):
assignee: nobody → Dr. David Alan Gilbert (davidgil-uk)

Hi Colin,
  Thanks for that (very clear) report - I've just created a branch that appears to do the trick:

and added you as a reviewer on it.


Changed in eglibc (Ubuntu):
status: New → In Progress
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package eglibc - 2.13-20ubuntu1

eglibc (2.13-20ubuntu1) oneiric; urgency=low

  [ Colin Watson ]
  * Revert change from 2.13-17ubuntu2 now that data.tar.xz support is
    deployed in Launchpad. Add Pre-Depends: dpkg (>= 1.15.6) to affected

  [ Dr. David Alan Gilbert ]
  * ARM strchr: mask r1 to char (LP: #842258)

  [ Matthias Klose ]
  * Merge with Debian (r4955).

eglibc (2.13-20) unstable; urgency=low

  * debian/ call /bin/mv with --version so
    that it doesn't return an error. Closes: #640872.

eglibc (2.13-19) unstable; urgency=low

  [ Aurelien Jarno ]
  * Change libc_rtlddir to /lib on s390x.
  * Add debian/patches/submitted-glob_h-ifdef.diff to fix an undefined
    preprocessor symbol in some rare conditions. Closes: #639213.
  * debian/sysdeps/ re-enable multiarch similarly to what
    has been done on sparc.
  * debian/ remove Breaks: on perl. Closes: #640300.
  * debian/patches/localedata/locale-C.diff: Don't include ISO14651
    collation rules in C.UTF-8 locale.
  * Update debian/patches/svn-updates to revision 15228:
    - Drop debian/patches/any/cvs-dl-deps.diff (merged upstream).
    - Drop debian/patches/arm/cvs-align-constant-pool.diff (merged upstream).
  * debian/ get the dynamic linker from /bin/mv
    instead of /bin/true. Closes: #640753.

  [ Jeremie Koenig ]
  * New patches to improve the signal code on Hurd:
  * Update testsuite accordingly.
  * Remove patches/hurd-i386/submitted-PTRACE_CONTINUE.diff, now covered by
  * libc0.3.symbols.hurd-i386: Add version for global-disposition functions.

  [ Samuel Thibault ]
  * Add patches/hurd-i386/submitted-libc_stack_end.diff to fix ruby1.9.1 stack
  * Add patches/hurd-i386/submitted-ttyname_ERANGE.diff to fix ttyname error
  * Add patches/hurd-i386/submitted-DEV_BSIZE.diff to add DEV_BSIZE.

  [ Petr Salinger ]
  * kfreebsd/local-sysdeps.diff: update to revision 3697 (from glibc-bsd).
    - fixes location used inside ldd on kfreebsd-amd64. Closes: #640156.
    - wrap faccessat() X_OK testing for superuser. Closes: #640325.

eglibc (2.13-18) unstable; urgency=low

  * On s390x the PI is /lib/, so we don't need to move from /lib to /lib64.
 -- Matthias Klose <email address hidden> Fri, 09 Sep 2011 13:44:41 +0200

Changed in eglibc (Ubuntu):
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers