devsw-prayas wrote:

@zwuis Fair point, and I don't think it's really a nitpick. The whole-function
existence check I have right now is strictly weaker than what -Wuninitialized
already does with its CFG dataflow (which is path sensitive). So as it stands
this doesn't really add anything over what -Wuninitialized already catches
statically.

Here's what I'm thinking as a fix, before I go implement it. Want your take on
the tradeoff first.

Replace the whole-function check with a per-candidate-scalar shadow bit. Each
scalar local that's eligible for this check (same candidacy rules as today:
automatic storage, address never taken) gets a shadow i1 alongside its real
alloca. It resets to false at the point of declaration, which re-executes on
every loop iteration, so loops are handled correctly without needing any
special casing. It gets set to true at every initialization or write to the
variable (initializer, assignment, compound assignment, inc/dec, inline asm
output operand). Then at each flagged read we just load the bit and branch to
the existing trap if it's still false.

That makes the check genuinely path sensitive. Something like
if (cond) x = 1; use(x); would trap exactly when cond was false at runtime,
instead of either always firing or never firing based on the static shape of
the function.

The honest cost here is one extra alloca plus one extra store per write plus
one load and branch per read, for candidate scalars only. Non-candidates like
address-taken or non-scalar locals aren't touched. Since -fsanitize=undefined
is generally expected to stay cheap, unlike say MSan, I wanted to flag this
before writing any of it. Is this overhead fine for inclusion in the
Undefined group as is, or would you rather this sit behind its own flag given
the per-variable runtime cost?

I'll hold off on implementing until we've got agreement here.

https://github.com/llvm/llvm-project/pull/207529
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to