Fixes case where terminator instruction is missing at end of function.
If the bitcode parser detects that the last block in the function
is missing a terminator, generate an error message and insert
a terminator instruction.
BUG= https://code.google.com/p/nativeclient/issues/detail?id=4214
R=stichnot@chromium.org
Review URL: https://codereview.chromium.org/1210013005.
diff --git a/pydir/run-pnacl-sz.py b/pydir/run-pnacl-sz.py
index e15b096..787910c 100755
--- a/pydir/run-pnacl-sz.py
+++ b/pydir/run-pnacl-sz.py
@@ -78,6 +78,10 @@
argparser.add_argument('--echo-cmd', required=False,
action='store_true',
help='Trace command that generates ICE instructions')
+ argparser.add_argument('--tbc', required=False, action='store_true',
+ help='Input is textual bitcode (not .ll)')
+ argparser.add_argument('--expect-fail', required=False, action='store_true',
+ help='Negate success of run by using LLVM not')
argparser.add_argument('--args', '-a', nargs=argparse.REMAINDER,
default=[],
help='Remaining arguments are passed to pnacl-sz')
@@ -93,13 +97,24 @@
raise RuntimeError("Can't specify both '--llvm-source' and " +
"'--no-local-syms'")
+ if args.llvm_source and args.tbc:
+ raise RuntimeError("Can't specify both '--tbc' and '--llvm-source'")
+
+ if args.llvm and args.tbc:
+ raise RuntimeError("Can't specify both '--tbc' and '--llvm'")
+
cmd = []
- if not args.llvm_source:
+ if args.tbc:
+ cmd = [os.path.join(pnacl_bin_path, 'pnacl-bcfuzz'), llfile,
+ '-bitcode-as-text', '-output', '-', '|']
+ elif not args.llvm_source:
cmd = [os.path.join(pnacl_bin_path, 'llvm-as'), llfile, '-o', '-', '|',
os.path.join(pnacl_bin_path, 'pnacl-freeze')]
if not args.no_local_syms:
cmd += ['--allow-local-symbol-tables']
cmd += ['|']
+ if args.expect_fail:
+ cmd += [os.path.join(pnacl_bin_path, 'not')]
cmd += [args.pnacl_sz]
cmd += ['--target', args.target]
if args.insts:
diff --git a/src/PNaClTranslator.cpp b/src/PNaClTranslator.cpp
index 5382bcc..fea11d0 100644
--- a/src/PNaClTranslator.cpp
+++ b/src/PNaClTranslator.cpp
@@ -1961,9 +1961,16 @@
};
void FunctionParser::ExitBlock() {
- if (isIRGenerationDisabled()) {
- return;
+ // Check if the last instruction in the function was terminating.
+ if (!InstIsTerminating) {
+ Error("Last instruction in function not terminator");
+ if (isIRGenerationDisabled())
+ return;
+ // Recover by inserting an unreachable instruction.
+ CurrentNode->appendInst(Ice::InstUnreachable::create(Func.get()));
}
+ if (isIRGenerationDisabled())
+ return;
// Before translating, check for blocks without instructions, and
// insert unreachable. This shouldn't happen, but be safe.
size_t Index = 0;
@@ -2256,6 +2263,7 @@
}
case naclbitc::FUNC_CODE_INST_RET: {
// RET: [opval?]
+ InstIsTerminating = true;
if (!isValidRecordSizeInRange(0, 1, "return"))
return;
if (Values.empty()) {
@@ -2270,10 +2278,10 @@
}
CurrentNode->appendInst(Ice::InstRet::create(Func.get(), RetVal));
}
- InstIsTerminating = true;
return;
}
case naclbitc::FUNC_CODE_INST_BR: {
+ InstIsTerminating = true;
if (Values.size() == 1) {
// BR: [bb#]
if (isIRGenerationDisabled())
@@ -2306,7 +2314,6 @@
CurrentNode->appendInst(
Ice::InstBr::create(Func.get(), Cond, ThenBlock, ElseBlock));
}
- InstIsTerminating = true;
return;
}
case naclbitc::FUNC_CODE_INST_SWITCH: {
@@ -2318,6 +2325,7 @@
// unnecesary data fields (i.e. constants 1). These were not
// cleaned up in PNaCl bitcode because the bitcode format was
// already frozen when the problem was noticed.
+ InstIsTerminating = true;
if (!isValidRecordSizeAtLeast(4, "switch"))
return;
@@ -2383,17 +2391,16 @@
if (isIRGenDisabled)
return;
CurrentNode->appendInst(Switch);
- InstIsTerminating = true;
return;
}
case naclbitc::FUNC_CODE_INST_UNREACHABLE: {
// UNREACHABLE: []
+ InstIsTerminating = true;
if (!isValidRecordSize(0, "unreachable"))
return;
if (isIRGenerationDisabled())
return;
CurrentNode->appendInst(Ice::InstUnreachable::create(Func.get()));
- InstIsTerminating = true;
return;
}
case naclbitc::FUNC_CODE_INST_PHI: {
diff --git a/tests_lit/lit.cfg b/tests_lit/lit.cfg
index 10d1df1..3090e60 100644
--- a/tests_lit/lit.cfg
+++ b/tests_lit/lit.cfg
@@ -48,7 +48,7 @@
config.test_format = lit.formats.ShTest()
# suffixes: A list of file extensions to treat as test files.
-config.suffixes = ['.ll']
+config.suffixes = ['.ll', '.test']
# test_source_root: The root path where tests are located.
config.test_source_root = os.path.dirname(__file__)
@@ -106,14 +106,16 @@
config.substitutions.append(('%pnacl_sz', pnacl_sz_tool))
-pnaclbintools = [r"\bFileCheck\b",
- r"\ble32-nacl-objdump\b",
- r"\bllvm-as\b",
- r"\bllvm-mc\b",
- r"\bllvm-readobj\b",
- r"\bnot\b",
- r"\bpnacl-freeze\b",
- r"\bpnacl-bcdis\b"]
+pnaclbintools = [r'\b' + x + r'\b' for x in
+ ['FileCheck',
+ 'le32-nacl-objdump',
+ 'llvm-as',
+ 'llvm-mc',
+ 'llvm-readobj',
+ 'not',
+ 'pnacl-bcdis',
+ 'pnacl-bcfuzz',
+ 'pnacl-freeze']]
for tool in pnaclbintools:
# The re.sub() line is adapted from one of LLVM's lit.cfg files.
diff --git a/tests_lit/llvm2ice_tests/Input/no-terminator-inst.tbc b/tests_lit/llvm2ice_tests/Input/no-terminator-inst.tbc
new file mode 100644
index 0000000..1f89762
--- /dev/null
+++ b/tests_lit/llvm2ice_tests/Input/no-terminator-inst.tbc
@@ -0,0 +1,31 @@
+65535,8,2;
+1,1;
+65535,17,2;
+1,4;
+7,32;
+2;
+21,0,0,0;
+7,1;
+65534;
+8,2,0,0,0;
+65535,19,2;
+5,0;
+65534;
+65535,14,2;
+1,0,102,105,98;
+65534;
+65535,12,2;
+1,3;
+65535,11,2;
+1,0;
+4,2;
+65534;
+28,2,1,36;
+11,1,2,1;
+10,2;
+2,3,2,1;
+34,0,5,0;
+2,1,5,0;
+65534;
+5534;
+65534;
diff --git a/tests_lit/llvm2ice_tests/invalid.test b/tests_lit/llvm2ice_tests/invalid.test
new file mode 100644
index 0000000..e6ec2ed
--- /dev/null
+++ b/tests_lit/llvm2ice_tests/invalid.test
@@ -0,0 +1,7 @@
+; Test that we handle functions that don't end with a terminator instruction.
+; See issue: https://code.google.com/p/nativeclient/issues/detail?id=4214
+
+RUN: %p2i --expect-fail --tbc -i %p/Input/no-terminator-inst.tbc --insts \
+RUN: | FileCheck --check-prefix=NO-TERM-INST %s
+
+; NO-TERM-INST: Last instruction in function not terminator
