Xarch Support Broken in Superenv

[GuillaumeDIDIER]

The arg refurbishing logic in superenv's cc does not handle -Xarch_ correctly.
It deletes the arguments which were meant for one architecture, and does not complain.
I stumbled on it some months ago as it caused at the time breakage in Qt (which tended to be fixed when disabling the vector instructions), but the pull request that fixed it was closed due to being quite invasive.
I don't no if that can be fixed, but meanwhile I report it here so that it becomes a known issue.

[GuillaumeDIDIER]

The pull request was #21664
It also contained other things, but if you scroll a bit you'll find the Xarch messing up.

[MikeMcQuaid]

You need to provide a reproducible test case.

[GuillaumeDIDIER]

Does it need to be a formula ?
Is there a way to invoke brew cc without gritting a formula ?
The bug can be seen y reading the superenv source file.
from superenv.rb

      case arg = whittler.next
      when '-arch', /^-Xarch_/
        whittler.next

any argument following -Xarch_xxx will be deleted.

[GuillaumeDIDIER]

Is there a way to use brew cc without writing a formula.

[GuillaumeDIDIER]

Well using brew sh and HOMEBREW_CCFG=sa0 it works, but superenv removes -Werror, so I need to find something else.

[GuillaumeDIDIER]

I've got a test that relies on ssse4.2
It could lead to false positives and negative depending on compiler version (if the compiler defaults to ssse4.2 on) and hardware (no ssse4.2 means error). Current clang version on OSX 10.9.2 works.
Is it OK ?

[GuillaumeDIDIER]

The test is ready
XarchTest, to be run in brew sh:

echo "enabling arg refurbishing"
export HOMEBREW_CCCFG="${HOMEBREW_CCCFG}O" #enable arg refurbishing
echo "removing -march=native from flags (breaks the test)"
export HOMEBREW_OPTFLAGS= #disable -march=native
#clang++ -c -pipe -Wall -W -Werror -Xarch_x86_64 -Wno-error=unused-variable -Xarch_i386 -Wno-error=unused-variable -Qunused-arguments -o unusedWarning.o unusedWarning.cpp #won't work in superenv -Werror is removed
#clang++ -c -pipe -mmmx -o mmx.o mmx.cpp && clang++ -headerpad_max_install_names -o mmx mmx.o -mmmx # clangs enables mms by default
echo "**************"
echo "should work : clang++ -c -pipe -msse4.2 -o sse4_2.o sse4_2.cpp && clang++ -headerpad_max_install_names -o sse4_2 sse4_2.o -msse4.2"
clang++ -c -pipe -msse4.2 -o sse4_2.o sse4_2.cpp && clang++ -headerpad_max_install_names -o sse4_2 sse4_2.o -msse4.2
echo "**************"
echo "should not work : clang++ -c -pipe -o sse4_2.o sse4_2.cpp && clang++ -headerpad_max_install_names -o sse4_2 sse4_2.o"
clang++ -c -pipe -o sse4_2.o sse4_2.cpp && clang++ -headerpad_max_install_names -o sse4_2 sse4_2.o
echo "**************"
echo "should work : clang++ -c -pipe -Xarch_x86_64 -msse4.2 -Xarch_i386 -msse4.2 -o sse4_2.o sse4_2.cpp && clang++ -headerpad_max_install_names -o sse4_2 sse4_2.o -Xarch_x86_64 -msse4.2 -Xarch_i386 -msse4.2"
clang++ -c -pipe -Xarch_x86_64 -msse4.2 -Xarch_i386 -msse4.2 -o sse4_2.o sse4_2.cpp && clang++ -headerpad_max_install_names -o sse4_2 sse4_2.o -Xarch_x86_64 -msse4.2 -Xarch_i386 -msse4.2

ssse4_2.cpp from qt 4.8.6:

/****************************************************************************
**
** Copyright (C) 2014 Digia Plc and/or its subsidiary(-ies).
** Contact: http://www.qt-project.org/legal
**
** This file is part of the config.tests of the Qt Toolkit.
**
** $QT_BEGIN_LICENSE:LGPL$
** Commercial License Usage
** Licensees holding valid commercial Qt licenses may use this file in
** accordance with the commercial license agreement provided with the
** Software or, alternatively, in accordance with the terms contained in
** a written agreement between you and Digia.  For licensing terms and
** conditions see http://qt.digia.com/licensing.  For further information
** use the contact form at http://qt.digia.com/contact-us.
**
** GNU Lesser General Public License Usage
** Alternatively, this file may be used under the terms of the GNU Lesser
** General Public License version 2.1 as published by the Free Software
** Foundation and appearing in the file LICENSE.LGPL included in the
** packaging of this file.  Please review the following information to
** ensure the GNU Lesser General Public License version 2.1 requirements
** will be met: http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html.
**
** In addition, as a special exception, Digia gives you certain additional
** rights.  These rights are described in the Digia Qt LGPL Exception
** version 1.1, included in the file LGPL_EXCEPTION.txt in this package.
**
** GNU General Public License Usage
** Alternatively, this file may be used under the terms of the GNU
** General Public License version 3.0 as published by the Free Software
** Foundation and appearing in the file LICENSE.GPL included in the
** packaging of this file.  Please review the following information to
** ensure the GNU General Public License version 3.0 requirements will be
** met: http://www.gnu.org/copyleft/gpl.html.
**
**
** $QT_END_LICENSE$
**
****************************************************************************/

#include <smmintrin.h>

int main(int, char**)
{
    volatile __m128i a = _mm_setzero_si128();
    volatile __m128i b = _mm_set1_epi32(42);
    volatile __m128i result = _mm_cmpestrm(a, 16, b, 16, 0);
    (void)result;
    return 0;
}

[GuillaumeDIDIER]

I had to remove -march=native from the arguments brew adds because it causes clang to enable ssse4.2, which I don't want to be enable by default but only enabled through the -mssse4.2 flag.

[MikeMcQuaid]

We need a formula in Homebrew or one of the taps that is currently broken without these changes and without you modifying superenv to e.g. remove -march=native. Provide a brew install line that does the wrong thing, please.

[GuillaumeDIDIER]

I don't no if any of the current formulae is broken due to this issue, I just wanted to report a caveat which might cause some breakage.

[GuillaumeDIDIER]

Actually if you are looking for a breakage investigate qt + universal binary.
Qt uses Xarch for some args and removing those args leads to interesting failures.
The complete logs with -j1 are coming.

[GuillaumeDIDIER]

Due to the little amount of people that use Qt Universal binary nowadays, it might not be worth it fixing that, but we should still remember that this is broken.
The qt universal cc logs clearly show that superenv removes those args.
eg:

g++-4.2 called with: -c -pipe -I/opt/X11/include -Xarch_i386 -mmacosx-version-min=10.4 -Xarch_x86_64 -mmacosx-version-min=10.5 -O2 -fPIC -arch i386 -arch x86_64 -Xarch_x86_64 -mmacosx-version-min=10.5 -Wall -W -DQT_BOOTSTRAPPED -DQT_LITE_UNICODE -DQT_NO_CAST_FROM_ASCII -DQT_NO_CAST_TO_ASCII -DQT_NO_CODECS -DQT_NO_DATASTREAM -DQT_NO_GEOM_VARIANT -DQT_NO_LIBRARY -DQT_NO_QOBJECT -DQT_NO_STL -DQT_NO_SYSTEMLOCALE -DQT_NO_TEXTSTREAM -DQT_NO_THREAD -DQT_NO_UNICODETABLES -DQT_NO_USING_NAMESPACE -DQT_NO_DEPRECATED -D_LARGEFILE64_SOURCE -D_LARGEFILE_SOURCE -I../../../mkspecs/macx-g++ -I. -I../../../include -I../../../include/QtCore -I../../../include/QtXml -o .obj/release-static/qfilesystemengine.o ../../corelib/io/qfilesystemengine.cpp
superenv removed:  -I/opt/X11/include -Xarch_i386 -mmacosx-version-min=10.4 -Xarch_x86_64 -mmacosx-version-min=10.5 -O2 -Xarch_x86_64 -mmacosx-version-min=10.5 -Wall -W
superenv added:    -w -Os -isystem/usr/local/include -isystem/usr/include/libxml2 -isystem/System/Library/Frameworks/OpenGL.framework/Versions/Current/Headers
superenv executed: g++-4.2 -pipe -w -Os -arch i386 -arch x86_64 -c -pipe -fPIC -DQT_BOOTSTRAPPED -DQT_LITE_UNICODE -DQT_NO_CAST_FROM_ASCII -DQT_NO_CAST_TO_ASCII -DQT_NO_CODECS -DQT_NO_DATASTREAM -DQT_NO_GEOM_VARIANT -DQT_NO_LIBRARY -DQT_NO_QOBJECT -DQT_NO_STL -DQT_NO_SYSTEMLOCALE -DQT_NO_TEXTSTREAM -DQT_NO_THREAD -DQT_NO_UNICODETABLES -DQT_NO_USING_NAMESPACE -DQT_NO_DEPRECATED -D_LARGEFILE64_SOURCE -D_LARGEFILE_SOURCE -I../../../mkspecs/macx-g++ -I. -I../../../include -I../../../include/QtCore -I../../../include/QtXml -o .obj/release-static/qfilesystemengine.o ../../corelib/io/qfilesystemengine.cpp -isystem/usr/local/include -isystem/usr/include/libxml2 -isystem/System/Library/Frameworks/OpenGL.framework/Versions/Current/Headers

In this example it is harmless to remove those args, but when it comes to -Xarch_x86_64 -msse4.2 -Xarch_i386 -msse4.2, or something like that, when sse4.2 is not enable by default, we get a breakage.

[MikeMcQuaid]

Closing this. I'll reopen if you provide a formula with arguments that demonstrates that it actually does the wrong thing.

[GuillaumeDIDIER]

Is dakcarto's patched qt formula a good example ?
cf. #28859

[MikeMcQuaid]

Again: provide a brew install line for something in core or a tap that generates incorrect architectures (rather than just passes flags you don't agree with).

[GuillaumeDIDIER]

Well currently I can't find a formula that is broken due to that bug, but some of qt failure in the PR mentioned higher up where due to it.
As -Xarch is not a common argument, it is not a bug that could break many formula.
I juster wanted to have this bug recorded somewhere, in case.
NB -Xarch_ is not about wrong architecture -Xarch_x86_64 -an_argumentmeans that the argument is only applied to x86_64.
Superenv behavior is -an_argument is removed.
In most formula -Xarch isn't used and Qt uses it only for vector instructions, which are either disabled, or enabled by -march=native for clang.

[GuillaumeDIDIER]

By the way, when using gcc, messing up with -march=native is not required to show failure in my first test.
After commenting the second line of XarchTest
HOMERBREW_CC="gcc" brew sh
then ./XarchTest
ends up with failure : the third compiler invocation should work, but it does not in super env due to arg removal.
And indeed this kind of compiler invocation are not common, and the one seen in Qt + universal have been disabled.
No formula are currently broken, but superenv is doing something which could break things.

[GuillaumeDIDIER]

Perhaps that Max Howell could tell us why he chose to disable -Xarch_ in the first place.

[MikeMcQuaid]

Please stop posting to the issue tracker about this; it is not an important issue. If you want to talk about it further please take it to the mailing list: homebrew@librelist.com

[watersb]

This command line FAILS for me:

brew install homebrew/versions/ruby193 --universal

The build and config logs pass the -arch options, but the executable is 64-bit only. From the 02-make.cc log:

superenv removed:  -I../.././ext/syck -O3 -ggdb -Wall -Wextra -Wpointer-arith -Wwrite-strings -Wdeclaration-after-statement -Wshorten-64-to-32 -Wimplicit-function-declaration -arch x86_64 -arch i386

So superenv is removing the -arch args, and the resulting binaries are single-architecture.

mb09:~$ brew install homebrew/versions/ruby193 --universal 
==> Installing dependencies for ruby193: gdbm, libyaml
==> Installing ruby193 dependency: gdbm
==> Downloading http://ftpmirror.gnu.org/gdbm/gdbm-1.11.tar.gz
Already downloaded: /Library/Caches/Homebrew/gdbm-1.11.tar.gz
==> ./configure --prefix=/usr/local/Cellar/gdbm/1.11 --mandir=/usr/local/Cellar/gdbm/1.11/share/man --infodir=/usr/loc
==> make install
🍺  /usr/local/Cellar/gdbm/1.11: 17 files, 760K, built in 38 seconds
==> Installing ruby193 dependency: libyaml
==> Downloading http://pyyaml.org/download/libyaml/yaml-0.1.6.tar.gz
Already downloaded: /Library/Caches/Homebrew/libyaml-0.1.6.tar.gz
==> ./configure --prefix=/usr/local/Cellar/libyaml/0.1.6
==> make install
🍺  /usr/local/Cellar/libyaml/0.1.6: 7 files, 592K, built in 27 seconds
==> Installing ruby193
==> Downloading http://cache.ruby-lang.org/pub/ruby/1.9/ruby-1.9.3-p547.tar.bz2
Already downloaded: /Library/Caches/Homebrew/ruby193-1.9.3-p547.tar.bz2
==> ./configure --prefix=/usr/local/Cellar/ruby193/1.9.3-p547_2 --enable-shared --with-arch=x86_64,i386 --disable-tclt
==> make
==> make install
🍺  /usr/local/Cellar/ruby193/1.9.3-p547_2: 795 files, 17M, built in 4.5 minutes
mb09:~$ file /usr/local/lib/libruby.1.9.1.dylib
/usr/local/lib/libruby.1.9.1.dylib: Mach-O 64-bit dynamically linked shared library x86_64
mb09:~$ file /usr/local/Cellar/ruby193/1.9.3-p547_2/bin/ruby 
/usr/local/Cellar/ruby193/1.9.3-p547_2/bin/ruby: Mach-O 64-bit executable x86_64

I see that this bug report is some months old, and also has been closed as "unimportant", so I will see if I'm missing something obvious.

[watersb]

From Bug #14845, I learned to try --env=std. That seemed to work around this problem.

mb09:~$ !file
file /usr/local/Cellar/ruby193/1.9.3-p547_2/bin/ruby 
/usr/local/Cellar/ruby193/1.9.3-p547_2/bin/ruby: Mach-O universal binary with 2 architectures
/usr/local/Cellar/ruby193/1.9.3-p547_2/bin/ruby (for architecture x86_64):  Mach-O 64-bit executable x86_64
/usr/local/Cellar/ruby193/1.9.3-p547_2/bin/ruby (for architecture i386):    Mach-O executable i386

[GuillaumeDIDIER]

@watersb
The issues I mentioned here were fixed more recently, by c7409f3 and 83ceef1
Your issue arises from the fact that neither env. permit_arch_flags or env.universal_binary are specified in the formula, which leads to superenv building for your architecture only.
I think that this should be reported on the tap issue tracker.

[watersb]

@GuillaumeDIDIER
THANKS VERY MUCH! 👍