BUILD: drmemory build failure caused by strcasestr declaration error

[xdje42]

Describe the bug

strcasestr's definition is char *(const char *, const char *) whereas drmemory/common/utils.h uses the older
const char *(const char *, const char *).
This causes a build failure on linux (64-bit, recent vintage).

In file included from /tmp/drmemory/common/utils_shared.c:30:
/usr/include/string.h:380:14: error: conflicting types for ‘strcasestr’; have ‘char *(const char *, const char *)’
  380 | extern char *strcasestr (const char *__haystack, const char *__needle)
      |              ^~~~~~~~~~
In file included from /tmp/drmemory/common/utils_shared.c:28:
/tmp/drmemory/common/utils.h:1015:1: note: previous declaration of ‘strcasestr’ with type ‘\
const char *(const char *, const char *)’
 1015 | strcasestr(const char *text, const char *pattern);
      | ^~~~~~~~~~

To Reproduce

Steps to reproduce the behavior:

$ git clone https://github.com/DynamoRIO/drmemory.git
$ cd drmemory
$ ./make/git/dev-setup.sh
$ cd ..
$ mkdir build
$ cd build
$ cmake ../drmemory
$ make -k # other failures may be present: -k keeps going so that strcasestr failure is seen

Versions

• What version of Dr. Memory are you using? git head @bfbc4a118a58a182770d306120cad41b9893c53

[derekbruening]

strcasestr's definition is char *(const char *, const char *) whereas drmemory/common/utils.h uses the older
const char *(const char *, const char *).
This causes a build failure on linux (64-bit, recent vintage).

How much older? If this is changed is it going to break older toolchains?

[xdje42]

Re: older toolchains: I'm not sure, but I suspect it doesn't matter: Note that the fix would presumably be to just copy the check used by drltrace already present in the tree.

[xdje42]

Actually, I was being optimistic. The hits are all over the tree and applying a surgical fix is a lot of work.

I went back through older glibc versions. strcasestr was added here (heh, back when I worked at Cygnus :-) ):

1997-11-26 04:28  Ulrich Drepper  <[email protected]>
[...]
        * string/Makefile (routines): Add strcasestr.
        * string/string.h: Add prototype for strcasestr.
        * sysdeps/generic/strcasestr.c: New file.

I downloaded glibc-2.1.1 from 1999 and it has char* for the return type, not drmemory's const char*.
If we're not ok to just fix drmemory's copy, at least #ifdef LINUX, then that would be disappointing but alas not unexpected.

The original definition (from glibc 2.1.1) was:

#ifdef __USE_GNU
/* Similar to `strstr' but this function ignores the case of both strings.  */
extern char *__strcasestr __P ((__const char *__haystack,
                                __const char *__needle));
extern char *strcasestr __P ((__const char *__haystack,
                              __const char *__needle));
#endif

Today's definition (from glibc 2.40) is:

#ifdef __USE_MISC
/* Similar to `strstr' but this function ignores the case of both strings.  */
# ifdef __CORRECT_ISO_CPP_STRING_H_PROTO
extern "C++" char *strcasestr (char *__haystack, const char *__needle)
     __THROW __asm ("strcasestr") __attribute_pure__ __nonnull ((1, 2));
extern "C++" const char *strcasestr (const char *__haystack,
                                     const char *__needle)
     __THROW __asm ("strcasestr") __attribute_pure__ __nonnull ((1, 2));
# else
extern char *strcasestr (const char *__haystack, const char *__needle)
     __THROW __attribute_pure__ __nonnull ((1, 2));
# endif
#endif

Both still use char* for the return type, not drmemory's const char* (nit: c++ has both).

What do you want to do?

[derekbruening]

Looks like the supplied strcasestr implementation is for Windows. It must have been missing from Linux somewhere? Though if it was there in 1999 that seems doubtful. Seems like making the supplied version Windows-only would be fine.

#2429 has some overlap here. There /usr/include/string.h has a const char* return type with C++ linkage, vs the C linkage. In the glibc 2.40 paste above the const char* overload is still there.

If nothing else but the C++ overload returns const then removing the const from the utils_shared version seems fine too.

[xdje42]

data point: I found this comment in utils_shared.c:

/* Not available in ntdll CRT so we supply our own.
 * It is available on Mac and Android, and we want to avoid it for libs that do not
 * want a libc dependence.
 */
#if !defined(MACOS) && !defined(ANDROID) && !defined(NOLINK_STRCASESTR)
const char *
strcasestr(const char *text, const char *pattern)
{

So it seems we do need to define this for linux for libraries "that do not want a libc dependence".
Still should be able to just remove the const in the return type.

[xdje42]

Open question: Why do current linux builds succeed? Unable to repro the success by replicating by hand, eg, ci-x86.yml.

[derekbruening]

Open question: Why do current linux builds succeed? Unable to repro the success by replicating by hand, eg, ci-x86.yml.

I would assume the GA VM compiler version doesn't give a warning while your local version does.