Enable using the toolchain (llvm) provided FileCheck.

Enable using the toolchain (llvm) provided FileCheck.

C++ test for handling method pointers (struct register lowering),
including debug info.
BUG=NONE
R=dschuff@chromium.org,jfb@chromium.org

Committed: https://chromium.googlesource.com/native_client/src/native_client/+/81b0050efea28aeccd4a5287f17e9dd8d6e367b5

[Mircea Trofin, 2015-03-20 22:14:40]

Patchset #1 (id:1) has been deleted

[Derek Schuff, 2015-03-23 16:25:07]

https://codereview.chromium.org/1021303003/diff/20001/SConstruct
File SConstruct (right):

https://codereview.chromium.org/1021303003/diff/20001/SConstruct#newcode1654
SConstruct:1654:
[env.File('${SCONSTRUCT_DIR}/tools/llvm_file_check_wrapper.sh'),
Why use a wrapper here instead of just invoking ${FILECHECK} directly?

https://codereview.chromium.org/1021303003/diff/20001/site_scons/site_tools/n...
File site_scons/site_tools/naclsdk.py (right):

https://codereview.chromium.org/1021303003/diff/20001/site_scons/site_tools/n...
site_scons/site_tools/naclsdk.py:119: FILECHECK=os.path.join(bin_path,
'%s-FileCheck' % tool_prefix),
The util binaries don't have the tool_prefix on their filenames.

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

https://codereview.chromium.org/1021303003/diff/20001/tests/toolchain/nacl.sc...
tests/toolchain/nacl.scons:516: debug_env.File('dwarf_method_ptr.cc'),
80 col

[Mircea Trofin, 2015-03-23 17:27:21]

https://codereview.chromium.org/1021303003/diff/20001/SConstruct
File SConstruct (right):

https://codereview.chromium.org/1021303003/diff/20001/SConstruct#newcode1654
SConstruct:1654:
[env.File('${SCONSTRUCT_DIR}/tools/llvm_file_check_wrapper.sh'),
On 2015/03/23 16:25:07, Derek Schuff wrote:
> Why use a wrapper here instead of just invoking ${FILECHECK} directly?

Because we want to run FileCheck | <dump command>, but all that as a parameter
to the python test wrapper (see CommandTest). Without a wrapper for FileCheck,
the command interpreter takes everything on the left of '|' and pipes it to
everything on the right, which is not what we want.

https://codereview.chromium.org/1021303003/diff/20001/site_scons/site_tools/n...
File site_scons/site_tools/naclsdk.py (right):

https://codereview.chromium.org/1021303003/diff/20001/site_scons/site_tools/n...
site_scons/site_tools/naclsdk.py:119: FILECHECK=os.path.join(bin_path,
'%s-FileCheck' % tool_prefix),
On 2015/03/23 16:25:07, Derek Schuff wrote:
> The util binaries don't have the tool_prefix on their filenames.

Done.

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

https://codereview.chromium.org/1021303003/diff/20001/tests/toolchain/nacl.sc...
tests/toolchain/nacl.scons:516: debug_env.File('dwarf_method_ptr.cc'),
On 2015/03/23 16:25:07, Derek Schuff wrote:
> 80 col

Done.

https://codereview.chromium.org/1021303003/diff/40001/SConstruct
File SConstruct (right):

https://codereview.chromium.org/1021303003/diff/40001/SConstruct#newcode708
SConstruct:708: 'run_rlimit_test',
Unrelated change. Sync-ed my repo, so this shows as a diff from the previous
change in this review.

[JF, 2015-03-23 17:42:10]

https://codereview.chromium.org/1021303003/diff/20001/SConstruct
File SConstruct (right):

https://codereview.chromium.org/1021303003/diff/20001/SConstruct#newcode1654
SConstruct:1654:
[env.File('${SCONSTRUCT_DIR}/tools/llvm_file_check_wrapper.sh'),
On 2015/03/23 17:27:20, Mircea Trofin wrote:
> On 2015/03/23 16:25:07, Derek Schuff wrote:
> > Why use a wrapper here instead of just invoking ${FILECHECK} directly?
> 
> Because we want to run FileCheck | <dump command>, but all that as a parameter
> to the python test wrapper (see CommandTest). Without a wrapper for FileCheck,
> the command interpreter takes everything on the left of '|' and pipes it to
> everything on the right, which is not what we want.

you should add this as a comment in the wrapper.

[Derek Schuff, 2015-03-23 17:54:03]

LGTM with my and JF's last comments.

https://codereview.chromium.org/1021303003/diff/40001/SConstruct
File SConstruct (right):

https://codereview.chromium.org/1021303003/diff/40001/SConstruct#newcode1657
SConstruct:1657: check_file] + cmd)
does this work on the ARM bots? you might need direct_emulation=False here.

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

https://codereview.chromium.org/1021303003/diff/40001/tests/toolchain/nacl.sc...
tests/toolchain/nacl.scons:519: '-W', True)
make this "True" a kwarg so it's clear that it refers to the use_filecheck
parameter

[Mircea Trofin, 2015-03-24 15:34:02]

Patchset #5 (id:100001) has been deleted

[Mircea Trofin, 2015-03-24 15:34:12]

Patchset #6 (id:140001) has been deleted

[Mircea Trofin, 2015-03-24 15:34:41]

PTAL

https://codereview.chromium.org/1021303003/diff/40001/SConstruct
File SConstruct (right):

https://codereview.chromium.org/1021303003/diff/40001/SConstruct#newcode1657
SConstruct:1657: check_file] + cmd)
On 2015/03/23 17:54:02, Derek Schuff wrote:
> does this work on the ARM bots? you might need direct_emulation=False here.

Done.

[Derek Schuff, 2015-03-24 16:57:13]

https://codereview.chromium.org/1021303003/diff/160001/tests/toolchain/nacl.s...
File tests/toolchain/nacl.scons (right):

https://codereview.chromium.org/1021303003/diff/160001/tests/toolchain/nacl.s...
tests/toolchain/nacl.scons:502: if env.Bit('bitcode'):
add or env.Bit('nacl_clang')

[Mircea Trofin, 2015-03-24 19:41:10]

The CQ bit was checked by mtrofin@chromium.org

[Mircea Trofin, 2015-03-24 19:41:10]

The patchset sent to the CQ was uploaded after l-g-t-m from dschuff@chromium.org
Link to the patchset: https://codereview.chromium.org/1021303003/#ps180001
(title: "nacl_clang")

[commit-bot: I haz the power, 2015-03-24 19:41:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1021303003/180001

[commit-bot: I haz the power, 2015-03-24 19:41:44]

Committed patchset #7 (id:180001) as
https://chromium.googlesource.com/native_client/src/native_client/+/81b0050ef...

[Mircea Trofin, 2015-03-24 21:13:13]

Patchset #8 (id:200001) has been deleted

[Mircea Trofin, 2015-03-24 21:13:20]

Patchset #8 (id:220001) has been deleted