Bug 11120

Changed in git.

For reference, the change was:

commit f69190e74a081f0a35906ff0b9a8dc24e42341e8
Author: Ulrich Drepper <drepper@redhat.com>
Date:   Thu Jan 14 08:09:32 2010 -0800

    Prevent silent errors should x86-64 strncmp be needed outside libc.

Due to a typo repeated several times, this bug hasn't been fixed yet,
despite being closed for more than two years.

I'll submit a patch shortly.

Interesting!
So what was Ulrich's intent in that commit--
to fix the function,
or to make it abort if called from inside ld.so?
I couldn't tell from the commit message
or bug closure comment.

The reason I started looking at this bug
is I was noticing flakiness of getenv() when called inside ld.so;
then when I tried to implement my own my_getenv as a workaround,
I ran into the fact that strncmp() is flaky too
(which is why getenv is flaky-- it uses strncmp),
and I had to implement a my_strncmp as well.

This is really hard to nail down--
sometimes strncmp("abc", "ab", 2) returns 0, and sometimes it returns 1.
It seems consistent within a given compile,
but other than that I don't see much rhyme or reason to it.

I can reproduce the inconsistency by sticking the following
at the top of the _dl_sort_fini function body in elf/dl-fini.c:

    #define PRINT_STRNCMP(a,b,len) _dl_debug_printf("strncmp(\"%s\", \"%s\", %u) returned %u\n", (a), (b), (int)(len), (int)strncmp((a), (b), (len)));
    PRINT_STRNCMP("abc", "ab", 2); // prints correct result 0
    char abc[] = {'a','b','c','\0'};
    char ab[] = {'a','b','\0'};
    PRINT_STRNCMP(abc, ab, 2);     // prints incorrect result 1

It prints something like:
     30747:     strncmp("abc", "ab", 2) = 0
     30747:     strncmp("abc", "ab", 2) = 1

I was about to open a new bug report,
but at this point I'm not sure whether that would be a dup of this one?

(In reply to comment #5)
> Interesting!
> So what was Ulrich's intent in that commit--
> to fix the function,
> or to make it abort if called from inside ld.so?
> I couldn't tell from the commit message
> or bug closure comment.

The commit message says:
"Prevent silent errors should x86-64 strncmp be needed outside libc"
The intent clearly was to make build fail.

The bug is fixed finally:
http://sourceware.org/git/?p=glibc.git;a=commitdiff;h=glibc-2.17-458-g2e0fb52

(In reply to comment #5)
> The reason I started looking at this bug
> is I was noticing flakiness of getenv() when called inside ld.so;

getenv() is not normally called inside rtld.
Are you talking about some patched rtld?

> The commit message says:
> "Prevent silent errors should x86-64 strncmp be needed outside libc"
> The intent clearly was to make build fail.

That commit message reads as completely ambiguous to me,
so thank you for the clarification.

(In reply to comment #7)
> (In reply to comment #5)
> > The reason I started looking at this bug
> > is I was noticing flakiness of getenv() when called inside ld.so;
> 
> getenv() is not normally called inside rtld.
> Are you talking about some patched rtld?

I'm using it for debugging.
(There are also uses of getenv and strncmp throughout the elf subdirectory
which I mistakenly thought were part of rtld... apparently not).

Thanks for the fix.  It will prevent a lot of further wasted time.

Can you comment on the fact that, prior to your latest change
which now prevents calling it altogether,
strncmp was returning inconsistent results?
This is contrary to what I would expect from Mark Seaborn's original
description "If strncmp() is ever used in ld.so on x86-64 it will
silently be implemented as strcmp()".

(In reply to comment #9)
> Can you comment on the fact that, prior to your latest change
> which now prevents calling it altogether,
> strncmp was returning inconsistent results?
> This is contrary to what I would expect from Mark Seaborn's original
> description "If strncmp() is ever used in ld.so on x86-64 it will
> silently be implemented as strcmp()".

There is no code in rtld that directly or indirectly calls strncmp.
The change I committed is fixed safeguards that in case rtld is changed to use strncmp, x86-64 build would fail until strncmp is implemented for rtld.

It's certainly better to let rtld build fail rather than allow strncmp to be implemented as strcmp.

(In reply to comment #10)
> (In reply to comment #9)
> > Can you comment on the fact that, prior to your latest change
> > which now prevents calling it altogether,
> > strncmp was returning inconsistent results?
> > This is contrary to what I would expect from Mark Seaborn's original
> > description "If strncmp() is ever used in ld.so on x86-64 it will
> > silently be implemented as strcmp()".
> 
> There is no code in rtld that directly or indirectly calls strncmp.
> The change I committed is fixed safeguards that in case rtld is changed to use
> strncmp, x86-64 build would fail until strncmp is implemented for rtld.
> 
> It's certainly better to let rtld build fail rather than allow strncmp to be
> implemented as strcmp.

Hi Dmitry,

I understand, and I completely agree with you,
as I've tried to say already.
There is absolutely no disagreement or question there.
After your latest change, I observe that any attempt to call strncmp directly,
or indirectly (e.g. by attempting to call getenv), fails at compile time.
That's a very good thing.  Thank you.

However, you have not answered my question.

My question is about the fact that,
contrary to what both you and Mark Seaborn have said,
prior to your change,
strncmp was *not* acting like strcmp, at least not consistently.
On one call it acted like strcmp, on the next call
(with exactly the same effective input) it didn't.
For a simple example of this,
please look carefully at my test case above (in my note of 2013-03-22 02:48:03).

Is this a surprise or not?
My concern is that your disabling of the function inside rtld
may be pushing more mistakes or rottenness under the rug.

(In reply to comment #11)
> There is absolutely no disagreement or question there.
> After your latest change, I observe that any attempt to call strncmp directly,
> or indirectly (e.g. by attempting to call getenv), fails at compile time.
> That's a very good thing.  Thank you.
> 
> However, you have not answered my question.
> 
> My question is about the fact that,
> contrary to what both you and Mark Seaborn have said,
> prior to your change,
> strncmp was *not* acting like strcmp, at least not consistently.
> On one call it acted like strcmp, on the next call
> (with exactly the same effective input) it didn't.

If you have a look at the implementation, you'll see that it's all the same in the non-libc case no matter by what name it is called.
The reason why you can see different results is that gcc is capable to optimize out some strncmp calls.  For example, in your case

$ echo -e '#include <string.h>\nint foo(void){return strncmp("abc", "ab", 2);}' |gcc -O2 -E - |tail -1
int foo(void){return (__extension__ (__builtin_constant_p (2) && ((__builtin_constant_p ("abc") && strlen ("abc") < ((size_t) (2))) || (__builtin_constant_p ("ab") && strlen ("ab") < ((size_t) (2)))) ? __extension__ ({ size_t __s1_len, __s2_len; (__builtin_constant_p ("abc") && __builtin_constant_p ("ab") && (__s1_len = strlen ("abc"), __s2_len = strlen ("ab"), (!((size_t)(const void *)(("abc") + 1) - (size_t)(const void *)("abc") == 1) || __s1_len >= 4) && (!((size_t)(const void *)(("ab") + 1) - (size_t)(const void *)("ab") == 1) || __s2_len >= 4)) ? __builtin_strcmp ("abc", "ab") : (__builtin_constant_p ("abc") && ((size_t)(const void *)(("abc") + 1) - (size_t)(const void *)("abc") == 1) && (__s1_len = strlen ("abc"), __s1_len < 4) ? (__builtin_constant_p ("ab") && ((size_t)(const void *)(("ab") + 1) - (size_t)(const void *)("ab") == 1) ? __builtin_strcmp ("abc", "ab") : (__extension__ ({ const unsigned char *__s2 = (const unsigned char *) (const char *) ("ab"); register int __result = (((const unsigned char *) (const char *) ("abc"))[0] - __s2[0]); if (__s1_len > 0 && __result == 0) { __result = (((const unsigned char *) (const char *) ("abc"))[1] - __s2[1]); if (__s1_len > 1 && __result == 0) { __result = (((const unsigned char *) (const char *) ("abc"))[2] - __s2[2]); if (__s1_len > 2 && __result == 0) __result = (((const unsigned char *) (const char *) ("abc"))[3] - __s2[3]); } } __result; }))) : (__builtin_constant_p ("ab") && ((size_t)(const void *)(("ab") + 1) - (size_t)(const void *)("ab") == 1) && (__s2_len = strlen ("ab"), __s2_len < 4) ? (__builtin_constant_p ("abc") && ((size_t)(const void *)(("abc") + 1) - (size_t)(const void *)("abc") == 1) ? __builtin_strcmp ("abc", "ab") : (__extension__ ({ const unsigned char *__s1 = (const unsigned char *) (const char *) ("abc"); register int __result = __s1[0] - ((const unsigned char *) (const char *) ("ab"))[0]; if (__s2_len > 0 && __result == 0) { __result = (__s1[1] - ((const unsigned char *) (const char *) ("ab"))[1]); if (__s2_len > 1 && __result == 0) { __result = (__s1[2] - ((const unsigned char *) (const char *) ("ab"))[2]); if (__s2_len > 2 && __result == 0) __result = (__s1[3] - ((const unsigned char *) (const char *) ("ab"))[3]); } } __result; }))) : __builtin_strcmp ("abc", "ab")))); }) : strncmp ("abc", "ab", 2)));}

(In reply to comment #12)
> (In reply to comment #11)
> > 
> > My question is about the fact that,
> > contrary to what both you and Mark Seaborn have said,
> > prior to your change,
> > strncmp was *not* acting like strcmp, at least not consistently.
> > On one call it acted like strcmp, on the next call
> > (with exactly the same effective input) it didn't.
> 
> If you have a look at the implementation, you'll see that it's all the same in
> the non-libc case no matter by what name it is called.
> The reason why you can see different results is that gcc is capable to optimize
> out some strncmp calls.  For example, in your case
> 
> $ echo -e '#include <string.h>\nint foo(void){return strncmp("abc", "ab", 2);}'
> |gcc -O2 -E - |tail -1

(in tcsh, this needs to say /bin/echo so it will understand the -e flag)

Ah!  Thank you, that makes perfect sense to me now.
It must be that __builtin_constant_p (abc) is returning false
but __builtin_constant_p("abc") is returning true,
resulting in two different execution paths taken, one of which actually
calls the function and the other doesn't.

Thanks very much for taking the time to examine and explain this.

Glad to find this, saved quite a bit of head-scratching. Below is my implementation of getenv that doesnt use strncmp. 

<pre>
static
char* getenv2(char **envp, const char* name)
{
  char **ep;
  size_t namelen = strlen(name);
  int i=0;
  for (ep = envp; *ep != NULL; ++ep)
  {
    for (i=0; (*ep)[i] == name[i] && i<namelen; i++)
      ;
    if (i == namelen && (*ep)[namelen] == '=')
      return &(*ep)[namelen+1];
  }
  return NULL;
}
</pre>