commit 3ddd61e0f7fc81e8067c5ca10eda6c832055b963 Author: Nick Mathewson nickm@torproject.org Date: Fri Sep 26 09:28:10 2014 -0400

Suppress deadcode warnings from static analysis tools

It's intentional that trunnel will generate some dead checks: I deliberately wrote it to _always_ check how many bytes are remaining, even if the check might be dead. The alternative would be to make the code more complex in order to teach it how to sometimes omit checks... but such complexity would run the risk of accidentally omitting a necessary check.

So replace the "if (remaining < foo) goto bar;" checks with a macro, and use an #ifdef to define the macro such that the static tool (probably) won't conclude that check can never be true. --- lib/trunnel/CodeGen.py | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-)

diff --git a/lib/trunnel/CodeGen.py b/lib/trunnel/CodeGen.py index 392783b..9b07e2f 100644 --- a/lib/trunnel/CodeGen.py +++ b/lib/trunnel/CodeGen.py @@ -2406,8 +2406,7 @@ class ParseFnGenerator(CodeGenerator): ntoh = NTOH_FN[width] self.needLabels.add(self.truncatedLabel) self.format(""" - if (remaining < {nbytes}) - goto {truncated}; + CHECK_REMAINING({nbytes}, {truncated}); {element} = {ntoh}(trunnel_get_uint{width}(ptr)); remaining -= {nbytes}; ptr += {nbytes}; """, nbytes=nbytes, truncated=self.truncatedLabel, @@ -2463,8 +2462,7 @@ class ParseFnGenerator(CodeGenerator): if bytesPerElt > 1: multiplier = "%s * " % bytesPerElt self.format(""" - if (remaining < ({multiplier}{width})) - goto {truncated}; + CHECK_REMAINING({multiplier}{width}, {truncated}); memcpy(obj->{c_name}, ptr, {multiplier}{width}); """, c_name=sfa.c_name, multiplier=multiplier, width=sfa.width, truncated=self.truncatedLabel) @@ -2523,7 +2521,7 @@ class ParseFnGenerator(CodeGenerator): # FFFF some of this is kinda cut-and-paste if arrayIsBytes(sva): if sva.widthfield != None: - self.w('if (remaining < %s)\n goto %s;\n' % ( + self.w('CHECK_REMAINING(%s, %s);\n' % ( w, self.truncatedLabel)) else: w = "remaining" @@ -2643,8 +2641,7 @@ class ParseFnGenerator(CodeGenerator): self.format(""" {{ size_t remaining_after; - if ({field} > remaining) - goto {truncated}; + CHECK_REMAINING({field}, {truncated}); remaining_after = remaining - {field}; remaining = {field}; """, field=field_, truncated=self.truncatedLabel) @@ -2652,8 +2649,7 @@ class ParseFnGenerator(CodeGenerator): self.format(""" {{ size_t remaining_after; - if (remaining < {leftafter}) - goto {truncated}; + CHECK_REMAINING({leftafter}, {truncated}); remaining_after = {leftafter}; remaining = remaining - {leftafter}; """, leftafter=sml.leftoverbytes, @@ -2748,6 +2744,22 @@ MODULE_BOILERPLATE = """\ (obj)->trunnel_error_code_ = 1; \ } while (0)

+#if defined(__COVERITY__) || defined(__clang_analyzer__) +/* If we're runnning a static analysis tool, we don't want it to complain + * that some of our remaining-bytes checks are dead-code. */ +int %(csafe_fname)s_deadcode_dummy__ = 0; +#define OR_DEADCODE_DUMMY || %(csafe_fname)s_deadcode_dummy__ +#else +#define OR_DEADCODE_DUMMY +#endif + +#define CHECK_REMAINING(nbytes, label) \ + do { \ + if (remaining < (nbytes) OR_DEADCODE_DUMMY) { \ + goto label; \ + } \ + } while (0) + """

@@ -2765,6 +2777,7 @@ def generate_code(input_fname, extra_options=[], target_dir=None):

c_fname = basename + ".c" h_fname = basename + ".h" + csafe_fname = re.sub(r'[^a-zA-Z]', '', os.path.split(basename)[1])

inp = open(input_fname, 'r') t = trunnel.Grammar.Lexer().tokenize(inp.read()) @@ -2787,6 +2800,7 @@ def generate_code(input_fname, extra_options=[], target_dir=None): 'guard_macro': guard_macro, 'h_fname': os.path.split(h_fname)[1], 'c_fname': os.path.split(c_fname)[1], + 'csafe_fname' : csafe_fname, 'expose_definitions': "".join(expose_definitions), 'version' : trunnel.__version__ }