error: ‘::memmem’ has not been declared

REOPENED
Assigned to

Status

Firefox Build System
General
REOPENED
7 years ago
3 months ago

People

(Reporter: anant, Assigned: jesup)

Tracking

Trunk
mozilla15
x86_64
Mac OS X

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

7 years ago
Building mozilla-central on a 64-bit Mac on OS X 10.7 with Xcode 3 installed (10.6 sdk) gives the following error:

    /usr/local/bin/ccache g++-4.2 -o nsCRT.o -c  -fvisibility=hidden -DMOZILLA_INTERNAL_API -DOSTYPE=\"Darwin11.1.0\" -DOSARCH=Darwin -D_IMPL_NS_COM -I/Users/anant/Code/alder/xpcom/ds/../io -I/Users/anant/Code/alder/xpcom/ds -I. -I../../dist/include -I../../dist/include/nsprpub  -I/Users/anant/Code/alder/objdir/dist/include/nspr -I/Users/anant/Code/alder/objdir/dist/include/nss      -fPIC  -fno-rtti -fno-exceptions -Wall -Wpointer-arith -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wcast-align -Wno-invalid-offsetof -Wno-variadic-macros -Werror=return-type -fno-strict-aliasing -fno-common -fshort-wchar -pthread -DNO_X11 -pipe  -DDEBUG -D_DEBUG -DTRACING -g -O3 -fno-omit-frame-pointer   -DMOZILLA_CLIENT -include ../../mozilla-config.h -MD -MF .deps/nsCRT.pp /Users/anant/Code/alder/xpcom/ds/nsCRT.cpp
    /Users/anant/Code/alder/xpcom/ds/nsCRT.cpp: In static member function ‘static const char* nsCRT::memmem(const char*, PRUint32, const char*, PRUint32)’:
    /Users/anant/Code/alder/xpcom/ds/nsCRT.cpp:175: error: ‘::memmem’ has not been declared 

The autoconf check for memmem succeeds, but the function isn't actually present in the OS X 10.6 SDK.
(In reply to Anant Narayanan [:anant] from comment #0)
> The autoconf check for memmem succeeds, but the function isn't actually
> present in the OS X 10.6 SDK.

Er, how is that happening?
Our theory is that Anant's machine has a memmem implementation in /usr/lib/ but /usr/include/string.h doesn't define it. Therefore the autoconf test, which just checks that the linker can find the symbol, succeeds, but the C++ compiler generates an error because it can't find a declaration in the global scope.

Anant, can you verify this is the case, e.g. with:

nm /usr/lib/system/libsystem_c.dylib | grep memmem

This may be corruption on Anant's (10.6/Xcode3) system, since my (MacOS 10.7/Xcode 4) system has memmem only in the 10.7 sdk (and /usr/lib) and not in the 10.6 sdk. Nevertheless, checking for the declaration in string.h before defining HAVE_MEMMEM would have found the issue and saved three of us an hour of debugging time.
Created attachment 567931 [details]
Quick program I used to debug the issue

Despite the manpage dating from 2005, Apple appears to have only added memmem in MacOS 10.6:

glaucomys:tmp giles$ g++ -g -Wall -o memmem-test memmem-test.cpp glaucomys:tmp giles$ g++ -g -Wall -isysroot / -o memmem-test memmem-test.cpp 
glaucomys:tmp giles$ g++ -g -Wall -isysroot /Developer/SDKs/MacOSX10.7.sdk -o memmem-test memmem-test.cpp 
glaucomys:tmp giles$ g++ -g -Wall -isysroot /Developer/SDKs/MacOSX10.6.sdk -o memmem-test memmem-test.cpp 
memmem-test.cpp: In function 'int main(int, char**)':
memmem-test.cpp:8: error: 'memmem' was not declared in this scope
(Reporter)

Comment 4

7 years ago
$ nm /usr/lib/system/libsystem_c.dylib | grep memmem
0000000000007956 T _memmem

Ralph's analysis seems to fit!

So, this is only a problem for those folks with OS X 10.7 but an older Xcode 3 i.e. compiling against the 10.6 SDK.
Lovely.

There was a reason I used AC_CHECK_FUNC originally, but it escapes me at the moment.

We removed the only use of memmem in the tree though, so we could just rip this stuff out.
AC_CHECK_FUNC is certainly faster and less code in configure.in.

The error is for http://mxr.mozilla.org/mozilla-central/source/xpcom/ds/nsCRT.cpp#174 which definitely calls memmem when HAVE_MEMMEM is defined. Did you mean just to remove the configure check and always use our fallback implementation?

(and that should be, "Apple appears to have only added memmem in MacOS 10.7.")
No, what I'm saying is that the only caller of nsCRT::memmem was removed, so we can remove nsCRT::memmem, and the accompanying configure logic.

I would r+ a patch that does this ...
(Reporter)

Comment 8

7 years ago
Created attachment 568110 [details] [diff] [review]
Remove memmem from nsCRT
Assignee: nobody → anant
Status: NEW → ASSIGNED
Attachment #568110 - Flags: review?(khuey)
Great. Less code is even better.
(Assignee)

Comment 10

6 years ago
Can we land this as soon as the tree opens again?  Just had a contributor (enda) run into the same problem; luckily I did a search for memmem in bugzilla and we didn't waste 3 hours ourselves.

Thanks
(Reporter)

Updated

6 years ago
Keywords: checkin-needed
(Reporter)

Comment 11

6 years ago
Created attachment 617878 [details] [diff] [review]
Remove memmem from nsCRT - v2

This patch is identical to attachment 568110 [details] [diff] [review] but is correctly formatted as a hg patch and should be used to check-in.
(Reporter)

Updated

6 years ago
Whiteboard: [autoland-try:617878]

Updated

6 years ago
Whiteboard: [autoland-try:617878] → [autoland-in-queue]

Comment 12

6 years ago
Autoland Patchset:
	Patches: 617878
	Branch: mozilla-central => try
	Destination: http://hg.mozilla.org/try/pushloghtml?changeset=4e89a9a3b9ce
Try run started, revision 4e89a9a3b9ce. To cancel or monitor the job, see: https://tbpl.mozilla.org/?tree=Try&rev=4e89a9a3b9ce

Comment 13

6 years ago
Try run for 4e89a9a3b9ce is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=4e89a9a3b9ce
Results (out of 15 total builds):
    success: 15
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/autolanduser@mozilla.com-4e89a9a3b9ce

Updated

6 years ago
Whiteboard: [autoland-in-queue]
Attachment #568110 - Attachment is obsolete: true
https://hg.mozilla.org/integration/mozilla-inbound/rev/48213b973bfb
Flags: in-testsuite-
Keywords: checkin-needed
Target Milestone: --- → mozilla15
I had to back this out due to OSX asserts causing orange.
https://hg.mozilla.org/integration/mozilla-inbound/rev/314adec5e515

Assertion failure: OnLionOrLater() || gCriticalAddress.mAddr != __null, at ../../../xpcom/base/nsStackWalk.cpp:175

Comment 16

6 years ago
https://hg.mozilla.org/mozilla-central/rev/48213b973bfb
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED

Comment 17

6 years ago
Backed out: https://hg.mozilla.org/mozilla-central/rev/314adec5e515
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Reporter)

Comment 18

6 years ago
Doing a try run to double check the orange.
Whiteboard: [autoland-try:617878:-b do -p all -u all -t all]

Updated

6 years ago
Whiteboard: [autoland-try:617878:-b do -p all -u all -t all] → [autoland-in-queue]

Comment 19

6 years ago
Autoland Patchset:
	Patches: 617878
	Branch: mozilla-central => try
	Destination: http://hg.mozilla.org/try/pushloghtml?changeset=dc6a1233bb13
Try run started, revision dc6a1233bb13. To cancel or monitor the job, see: https://tbpl.mozilla.org/?tree=Try&rev=dc6a1233bb13
(Reporter)

Comment 20

6 years ago
Hmm, tons of oranges on that try run. Not sure why this patch would be causing them but definitely deserves investigation.
Whiteboard: [autoland-in-queue]
(Assignee)

Comment 21

4 years ago
Currently used by:
media/webrtc/trunk/webrtc/system_wrappers/source/droid-cpu-features.c
and
gfx/skia/trunk/src/core/SkUtilsArm.cpp

both for parsing ARM cpu features
Assignee: anant → rjesup
(Assignee)

Comment 22

4 years ago
https://tbpl.mozilla.org/?tree=Try&rev=5e80787f36eb

Fails on Android 2.3: (likely the arm cpu features stuff above, or maybe the configure change):

/builds/slave/try-and-d-00000000000000000000/build/mozglue/linker/ElfLoader.h:32:5: error: conflicting declaration 'typedef struct Dl_info Dl_info'
/builds/slave/try-and-d-00000000000000000000/build/android-ndk/platforms/android-9/arch-arm/usr/include/dlfcn.h:44:3: error: 'Dl_info' has a previous declaration as 'typedef struct Dl_info Dl_info'
make[5]: *** [BaseElf.o] Error 1

Updated

3 months ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.