PNaCl driver: Add libgcc to mixed native/bitcode links

PNaCl driver: Add libgcc to mixed native/bitcode links

When building with pre-translated object files, calls can be generated
to libgcc/compiler-rt functions, but libgcc was not included on the
bitcode link line, causing undefined references to the libgcc functions.
Include libgcc on the bitcode link line to satisfy these references when
using native objects.

R=mseaborn@chromium.org
BUG= https://code.google.com/p/nativeclient/issues/detail?id=3910

Committed: https://src.chromium.org/viewvc/native_client?view=rev&revision=13602

[Mark Seaborn, 2014-08-07 23:13:02]

LGTM.  s/libcc/libgcc/ in commit message.  Please wait for trybots to pass
before committing. :-/

Would I be right in thinking that there are two linker invocations involved
here?

 * First we invoke the bitcode linker, which produces bitcode (possibly empty),
but also cross-references against native libraries.  Does it produce native
objects too?
 * Then we invoke llc on the bitcode.
 * Then we link all the native objects.

Is that description right?

https://codereview.chromium.org/454593002/diff/20001/tests/toolchain/nacl.scons
File tests/toolchain/nacl.scons (right):

https://codereview.chromium.org/454593002/diff/20001/tests/toolchain/nacl.sco...
tests/toolchain/nacl.scons:559: mixedlink_env = env.Clone()
Maybe add a comment here to say what this is a regression test for?

https://codereview.chromium.org/454593002/diff/20001/tests/toolchain/needs_li...
File tests/toolchain/needs_libgcc.c (right):

https://codereview.chromium.org/454593002/diff/20001/tests/toolchain/needs_li...
tests/toolchain/needs_libgcc.c:6: #include <stdint.h>
Nit: add empty line before this

[Derek Schuff, 2014-08-08 21:11:55]

Your description is right. In practice the output of the bitcode link will
almost never be empty because all the compiler-supplied libraries are bitcode.

https://codereview.chromium.org/454593002/diff/20001/tests/toolchain/nacl.scons
File tests/toolchain/nacl.scons (right):

https://codereview.chromium.org/454593002/diff/20001/tests/toolchain/nacl.sco...
tests/toolchain/nacl.scons:559: mixedlink_env = env.Clone()
On 2014/08/07 23:13:02, Mark Seaborn wrote:
> Maybe add a comment here to say what this is a regression test for?

Done.

https://codereview.chromium.org/454593002/diff/20001/tests/toolchain/needs_li...
File tests/toolchain/needs_libgcc.c (right):

https://codereview.chromium.org/454593002/diff/20001/tests/toolchain/needs_li...
tests/toolchain/needs_libgcc.c:6: #include <stdint.h>
On 2014/08/07 23:13:02, Mark Seaborn wrote:
> Nit: add empty line before this

Done.

[Derek Schuff, 2014-08-09 00:33:30]

It turns out that the expression parser eagerly evaluates both sides of a
ternary expression, which causes a problem with this CL because ARCH is not set
most of the time. It's not a large change to fix that but it does mean changing
the expression parser :/

PTAL

[Mark Seaborn, 2014-08-09 07:11:51]

https://codereview.chromium.org/454593002/diff/40001/pnacl/driver/pnacl-drive...
File pnacl/driver/pnacl-driver.py (right):

https://codereview.chromium.org/454593002/diff/40001/pnacl/driver/pnacl-drive...
pnacl/driver/pnacl-driver.py:126: '${ALLOW_NATIVE ? -L${LIBS_NATIVE_ARCH}} ' +
Might an alternative be to do this below:

if env.getbool('ALLOW_NATIVE'):
  env.append('LD_FLAGS', env.eval('-L${LIBS_NATIVE_ARCH}'))
  env.append('STDLIBS', '-lgcc')

In some of my recent changes to the driver scripts, I've been trying to move
away from the string interpolation that driver_env does, because the evaluation
order is a bit hard to understand.  (See SetUpArch and SetUpLinkOptions in
pnacl-translate.py.)

If you do that, the change is OK with me.  Otherwise I'd need to stare at the
driver_env.py change some more to understand it...

https://codereview.chromium.org/454593002/diff/40001/pnacl/driver/pnacl-drive...
pnacl/driver/pnacl-driver.py:505: if env.getbool('ALLOW_NATIVE') and not
compiling_to_native:
e.g. somewhere around here

[Derek Schuff, 2014-08-11 17:54:17]

https://codereview.chromium.org/454593002/diff/40001/pnacl/driver/pnacl-drive...
File pnacl/driver/pnacl-driver.py (right):

https://codereview.chromium.org/454593002/diff/40001/pnacl/driver/pnacl-drive...
pnacl/driver/pnacl-driver.py:126: '${ALLOW_NATIVE ? -L${LIBS_NATIVE_ARCH}} ' +
On 2014/08/09 07:11:51, Mark Seaborn wrote:
> Might an alternative be to do this below:
> 
> if env.getbool('ALLOW_NATIVE'):
>   env.append('LD_FLAGS', env.eval('-L${LIBS_NATIVE_ARCH}'))
>   env.append('STDLIBS', '-lgcc')
> 
> In some of my recent changes to the driver scripts, I've been trying to move
> away from the string interpolation that driver_env does, because the
evaluation
> order is a bit hard to understand.  (See SetUpArch and SetUpLinkOptions in
> pnacl-translate.py.)
> 
> If you do that, the change is OK with me.  Otherwise I'd need to stare at the
> driver_env.py change some more to understand it...

Done.

https://codereview.chromium.org/454593002/diff/40001/pnacl/driver/pnacl-drive...
pnacl/driver/pnacl-driver.py:505: if env.getbool('ALLOW_NATIVE') and not
compiling_to_native:
On 2014/08/09 07:11:51, Mark Seaborn wrote:
> e.g. somewhere around here

Done.

[Derek Schuff, 2014-08-11 21:46:54]

Committed patchset #4 manually as 13602 (presubmit successful).