192975 - Configure does not test supplied compiler options for validity

Status: RESOLVED FIXED

(SeaMonkey :: Build Config, defect, P4)

Description

[tenthumbs]

I was experimenting with a highly optimized build this morning so I said

  ./configure  --disable-mailnews --disable-debug --disable-ldap \
   --enable-optimize="-Wall -W -O3 -minline-all-string-ops -fomit-frame-pointer"

Configure happily did its thing without incident but make immediately failed
because -minline-all-string-ops is incorrect. It should be -minline-all-stringops.

Mozilla should test that the user's options actually work.

Comment 1

[Boris Zbarsky [:bzbarsky]]

> -fomit-frame-pointer

"Don't Do That".  Mozilla won't run.  Last I checked, the build docs said that
pretty clearly...  (not relevant to the actual bug here; just letting you know).

Comment 2

[tenthumbs]

> "Don't Do That"

This is one of the few times I actually know something.

The build docs are wrong. Xptcall used to depend on having valid frame pointers.
Bug 140412 fixed that for Linux. Bug 181464 is about porting the new code to
other platforms. The OS/2 people are also thinking of using the new code.

Once I fixed the typo mozilla really does work.

Comment 3

[hacker formerly known as [e-mail]]

So when do we get a medal for saving the users from themselves?

-fomit-frame-pointer is still not generally recommended as not all platforms
have had their xptcall glue rewritten to avoid the use of fp.  From the sounds
of it, only it only works on recent (1.1+) builds on linux/x86.

Comment 4

[tenthumbs]

> So when do we get a medal for saving the users from themselves?

I'm not a big fan of hand holding but mozilla uses autoconf and every 
other autoconf package tests the compiler + options early on. 
Admittedly, that's using CFLAGS but mozilla has chosen to add an 
additional option for optimizing so I think that mozilla should be 
consistent with the typical, and probably expected, autoconf behavior.

> -fomit-frame-pointer is still not generally recommended as not all 
> platforms ...

Certainly true. I should have been more specific that I know it works
only for x86 Linux.

In any event, this is just a diversion from the point here.

Comment 5

[hacker formerly known as [e-mail]]

Attachment: v1.0

Comment 6

[hacker formerly known as [e-mail]]

Patch has been checked in.