Igalia / pflua

Packet filtering in Lua
Other
313 stars 39 forks source link

Simplify 'if A fail fail' to 'fail' #223

Closed mpeterv closed 9 years ago

mpeterv commented 9 years ago

if A fail fail can appear when some checks are conflicting, e.g. tcp and tcp[100] == 1 used to compile to

local lshift = require("bit").lshift
local band = require("bit").band
local cast = require("ffi").cast
return function(P,length)
   if length < 14 then return false end
   local var1 = cast("uint16_t*", P+12)[0]
   if var1 == 8 then
      if length < 54 then return false end
      if P[23] ~= 6 then return false end
      if band(cast("uint16_t*", P+20)[0],65311) ~= 0 then return false end
      local var7 = lshift(band(P[14],15),2)
      if (var7 + 115) > length then return false end
      return P[(var7 + 114)] == 1
   else
      if var1 ~= 56710 then return false end
      if length < 54 then return false end
      local var15 = P[20]
      return false
   end
end

Note the path that always returns false. With this change it compiles to

local lshift = require("bit").lshift
local band = require("bit").band
local cast = require("ffi").cast
return function(P,length)
   if length < 54 then return false end
   if cast("uint16_t*", P+12)[0] ~= 8 then return false end
   if P[23] ~= 6 then return false end
   if band(cast("uint16_t*", P+20)[0],65311) ~= 0 then return false end
   local var7 = lshift(band(P[14],15),2)
   if (var7 + 115) > length then return false end
   return P[(var7 + 114)] == 1
end

Which is same as code for tcp[100] == 1.

Some optimizations are also possible for other cases when true and false branches of an if are identical, but I couldn't find an example of that.

kbara commented 9 years ago

LGTM, nice catch. Waiting on second review.

dpino commented 9 years ago

Same comment as in #220.

wingo commented 9 years ago

LGTM, @mpeterv can you add an entry to doc/ for "tcp and tcp[100] ==1" so that we don't regress? thanks in advance :)

mpeterv commented 9 years ago

@andywingo sorry, could you add it? On my machine I get different results for BPF pipelines, so if I'd do it it would break on yours.

wingo commented 9 years ago

On Tue 30 Jun 2015 16:47, Peter Melnichenko notifications@github.com writes:

@andywingo sorry, could you add it? On my machine I get different results for BPF pipelines, so if I'd do it it would break on yours.

Sure, no problem.

wingo commented 9 years ago

Test added in https://github.com/Igalia/pflua/commit/08c1eb926981131ee9f8bfd7a7f86009bb253ebd