Bug 14602 - string/strstr.c is broken
Summary: string/strstr.c is broken
Status: RESOLVED FIXED
Alias: None
Product: glibc
Classification: Unclassified
Component: libc (show other bugs)
Version: 2.17
: P2 normal
Target Milestone: 2.17
Assignee: Maxim Kuvyrkov
URL:
Keywords:
Depends on:
Blocks: 14716
  Show dependency treegraph
 
Reported: 2012-09-20 20:54 UTC by law
Modified: 2012-10-13 18:47 UTC (History)
6 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description law 2012-09-20 20:54:21 UTC
Compile and run the following for older i686 machine.  It claims to find the needle ", enable_shared, " in the haystack ", enable_static, ".

#include <stdio.h>
#include <string.h>

int main(void)
{  
  const char *haystack = ", enable_static, ";
  const char *needle = ", enable_shared, ";

  char *result;
  int retval;

  result = strstr(haystack, needle);
  printf("'%s'\n", result);
  retval = result ? result - haystack : -1; 

  printf("%d\n", retval);                                                       
  return 0;
}


This bug was introduced by this commit:

commit 400726deef57d8da8d6c7cd5c8e7891eaabab041
Author: Maxim Kuvyrkov <maxim@codesourcery.com>
Date:   Mon May 28 22:50:15 2012 -0700

    Detect EOL on-the-fly in strstr, strcasestr and memmem.


For reference, the cpuflags for a machine which can trigger this failure:

flags           : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx lm constant_tsc arch_perfmon pebs bts rep_good nopl aperfmperf pni dtes64 monitor ds_cpl vmx est tm2 ssse3 cx16 xtpr pdcm lahf_lm dts tpr_shadow
Comment 1 H.J. Lu 2012-09-21 22:24:56 UTC
I created hjl/ifunc/test branch.  It allows you to test all IFUNC
implementations supported on your machine when glibc is configured
with --enable-test-multi-arch.
Comment 2 joseph@codesourcery.com 2012-09-21 22:42:07 UTC
On Fri, 21 Sep 2012, hjl.tools at gmail dot com wrote:

> I created hjl/ifunc/test branch.  It allows you to test all IFUNC
> implementations supported on your machine when glibc is configured
> with --enable-test-multi-arch.

We'll need clean patch submissions on libc-alpha, but I think the ability 
to test IFUNC implementations like that is strongly desirable (before 
releases we should try to get people to do so on hardware supporting as 
many as possible of the implementations).
Comment 3 H.J. Lu 2012-09-21 23:37:26 UTC
(In reply to comment #2)
> On Fri, 21 Sep 2012, hjl.tools at gmail dot com wrote:
> 
> > I created hjl/ifunc/test branch.  It allows you to test all IFUNC
> > implementations supported on your machine when glibc is configured
> > with --enable-test-multi-arch.
> 
> We'll need clean patch submissions on libc-alpha, but I think the ability 
> to test IFUNC implementations like that is strongly desirable (before 
> releases we should try to get people to do so on hardware supporting as 
> many as possible of the implementations).

Last time when I tried it:

http://sourceware.org/ml/libc-alpha/2010-11/msg00039.html

it went nowhere.
Comment 4 joseph@codesourcery.com 2012-09-21 23:48:32 UTC
On Fri, 21 Sep 2012, hjl.tools at gmail dot com wrote:

> > We'll need clean patch submissions on libc-alpha, but I think the ability 
> > to test IFUNC implementations like that is strongly desirable (before 
> > releases we should try to get people to do so on hardware supporting as 
> > many as possible of the implementations).
> 
> Last time when I tried it:
> 
> http://sourceware.org/ml/libc-alpha/2010-11/msg00039.html
> 
> it went nowhere.

Try restarting the discussion with a fair self-contained synthesis of the 
various points that were made in the previous thread about the advantages 
and disadvantages of different approaches to this testing (whether or not 
you agree with the points in question), and an explanation of the 
rationale for the approach you are taking this time, with reference to all 
those points and how they are addressed.
Comment 5 H.J. Lu 2012-09-22 00:09:15 UTC
(In reply to comment #4)
> 
> Try restarting the discussion with a fair self-contained synthesis of the 
> various points that were made in the previous thread about the advantages 
> and disadvantages of different approaches to this testing (whether or not 
> you agree with the points in question), and an explanation of the 
> rationale for the approach you are taking this time, with reference to all 
> those points and how they are addressed.

There was a strong disagreement on IFUNC selector. I don't want
to make any changes to IFUNC selector while Roland wants otherwise.
I don't see how we can break the impasse here.
Comment 6 joseph@codesourcery.com 2012-09-23 18:57:49 UTC
On Sat, 22 Sep 2012, hjl.tools at gmail dot com wrote:

> There was a strong disagreement on IFUNC selector. I don't want
> to make any changes to IFUNC selector while Roland wants otherwise.
> I don't see how we can break the impasse here.

Start by not assuming that either your views or those of other 
participants are fated to remain unchanged from those in the previous 
discussion.  Try to understand the arguments for both points of view and 
explain them both fairly when posting your new self-contained analysis of 
the issue.  Maybe start the analysis with the requirements (ability to 
test all the IFUNC versions of functions, easily) and high-level features 
that are desirable if not absolutely required (being able to test them 
without needing a special build of glibc to do so, not making it 
excessively complicated to add new IFUNC versions of functions, not 
affecting the efficiency of those versions) before discussing how the 
possible approaches match up to those requirements and features.
Comment 7 H.J. Lu 2012-09-23 19:10:09 UTC
I am updating hjl/ifunc/test branch. Its goals are

1. Provide an infrastructure to tests all possible implementations
on build machine automatically.
2. No changes to IFUNC selectors.
3. Add a test for a new IFUNC implementation by adding it to the list of
IFUNC functions.

Change IFUNC selector is not the goal of this branch.

I am fixing a few remaining issues and will submit it for comments
after it is finished.
Comment 8 H.J. Lu 2012-10-05 19:21:37 UTC
It is broken on all targets which use string/strstr.c, not just
i686.
Comment 9 Maxim Kuvyrkov 2012-10-10 00:11:26 UTC
Fixed.  Problem was in detecting EOS of haystack.