derive and expose address keyed NZCV flag liveness - #289
Conversation
2f0d5b2 to
c6c3bb1
Compare
leftbyte
changed the title
leftbyte
changed the title
leftbyte
changed the title
leftbyte
changed the title
leftbyte
changed the title
waskyo
left a comment
waskyo
left a comment
There was a problem hiding this comment.
First pass. I'd like to do a second pass looking at the algo but ran out of time today for that.
| Sequence[Optional[Union["FlagReachingDefFact", | ||
| "ReachingDefFact"]]]], | ||
| names: Optional[Set[str]] = None | ||
| ) -> Tuple[Dict[str, Set[str]], Dict[str, Set[str]]]: |
There was a problem hiding this comment.
This style seems wrong, should follow what CHB already does.
(fwiw, I didn't review the rest of the code for similar issues)
There was a problem hiding this comment.
...fixed? I think you're referring to that weird indentation.
|
|
||
| def _use_kill( | ||
| self, | ||
| get_facts: Callable[ |
There was a problem hiding this comment.
have the docstring document what this is passed and what it is supposed to generate
| if fact is None: | ||
| continue | ||
| name = str(fact.variable) | ||
| if name == "PC": |
There was a problem hiding this comment.
Could you document that PC is always ignored (I think?) in the docstring as part of the names documentation.
| use[iaddr].add(name) | ||
| for d in fact.deflocations: | ||
| da = str(d) | ||
| if da == "init" or da.startswith("F"): |
There was a problem hiding this comment.
what does this check translate to? Maybe you could add a small inline function that does this check and has a docstring explaining it?
There was a problem hiding this comment.
added inline function with some docstring.
| """Block address -> its instruction addresses in execution order. | ||
|
|
||
| Sorted lexicographically, matching how BasicBlock orders its own | ||
| instructions. A numeric (int, 16) key would raise on the analysis's |
There was a problem hiding this comment.
I can't figure out what this sentence means:
A numeric (int, 16) key would raise on the analysis's inlined-instruction addresses (e.g. "F:0x...._0x....").
Is it explaining something about what the function is doing or how it's generating the return value?
There was a problem hiding this comment.
Updated comment to hopefully make it clearer.
| result[ia] = {"live-in": lin, "live-out": lout} | ||
| return result | ||
|
|
||
| def _backward( |
There was a problem hiding this comment.
If you haven't already, it would be good to check with Henny if CHB doesn't already have code to do this traversal (same for the _blocks and _edges functions).
There was a problem hiding this comment.
_edges has an equivalent so that's fixed but the other functionality does not.
62c1d69 to
9c81d60
Compare
Add ASTILiveness, which computes NZCV flag live-in/live-out per instruction address for a function by building per-address use/kill sets from the per-instruction flag-reaching-def facts and running a backward live-variable fixpoint over the CFG. Block instructions are ordered with the same lexicographic sort BasicBlock uses, which also tolerates the analysis's inlined-instruction addresses.
Add a flag-liveness map (keyed by instruction address) to ASTProvenance with the standard getter/setter, and round-trip it through serialize/deserialize alongside the other provenance facts.
Compute flag-liveness in mk_asts, alongside set_ast_provenance, so it flows through the same builder path as the other provenance facts (rather than only on the results-ast command path). The computation is auxiliary and guarded so a failure cannot abort AST generation.
730154a to
9615655
Compare
is_real_def_site excluded any "F"-prefixed def-site as "not a site in this function's CFG", which was wrong. Those instructions execute and define what they define, and their addresses are ordinary keys in fn.blocks and fn.instructions. _use_kill was therefore recording uses at those addresses while discarding their kills.
An "init" def-site means the value was defined on function entry rather than by an instruction. For a registers that is ordinary and an incoming parameter is defined exactly there. This is not normal for a flag because the ABI leaves NZCV undefined on entry to a function.
9615655 to
0c1a707
Compare
2 participants
CodeHawk records per-instruction flag-reaching-def facts but exposes no liveness. This PR derives NZCV flag live-in/live-out for each instruction address and attaches it to the function's provenance, so a consumer can tell whether a flag set by a compare is still needed past a given point.
This metadata is used by the the patcher's predicated-if in-place patching mechanism as a safety check to confirm NZCV are dead at the region exit before replacing a compare.