[acc] Minor improvements to information-flow checker. by jadephilipoom · Pull Request #246 · zerorisc/expo

jadephilipoom · 2026-04-24T11:21:05Z

On top of #245, draft until that PR is merged.

These were some small improvements I made along the way towards a bigger extension to the information-flow checker that I'll post as a follow-up PR; I separated these out for easier review. There's more detail in the commit messages, but in summary:

print out control-flow nodes more nicely for debugging
automatically resolve branches where both sides of the comparison are known constants
allow specifying public values instead of secret values for constant-time checks, which makes it easier to ensure nothing has been forgotten

mkannwischer

Thanks @jadephilipoom. I think you forgot to push something or something went wrong when rebasing.

I'm a little concerned that this still made it through CI even do it seems to break the information flow analysis. ~~Are we not running the constant time tests in CI on every PR? Shouldn't we?~~ The constant-time tests are not depending on this script - I got that wrong.

mkannwischer · 2026-04-29T01:53:16Z

 from shared.decode import decode_elf
 from shared.information_flow import InformationFlowGraph
-from shared.information_flow_analysis import (get_program_iflow,
+from shared.information_flow_analysis import (get_dmem_symbols,


I'm getting a
ImportError: cannot import name 'get_dmem_symbols' from 'shared.information_flow_analysis' here.

~~Given that CI did not flag this, aren't we running the constant-time tests in CI?~~
Given that CI did not flag this, should we extend the CI so this script is exercised? Remind me what the main purpose of the information-flow check is?

We run the check_const_time.py script in CI; this analyze_information_flow.py script uses the same backend but gives more detail. It's run manually if you want to understand what's going on in the code better.

mkannwischer · 2026-04-29T01:55:08Z

    return out
+
+
+def parse_information_flow_node(program: ACCProgram, x: str) -> str:


program is not used in this function - is that intentional?

This is an artifact of detangling these changes from #247; in #247 it gets used. I'll remove it here.

mkannwischer · 2026-04-29T01:59:02Z

+# Deliberately invalid CSR address used by unimp.
+UNIMP_CSR_ADDR = 3072


This is unused? Was that intentional?

Again an artifact from #247, it gets used there to detect unimp calls and not error out on the invalid CSR. Will remove here.

mkannwischer · 2026-04-29T02:33:04Z

                             'program; use --subroutine to analyze a specific '
                             'subroutine.')
-        constants = parse_required_constants(args.constants)
+        constants = parse_required_constants(args.constants, dmem_symbols)


I think you didn't change parse_required_constants? It only takes 1 argument.

Once again a rebase artifact, should be fixed now! Sorry for all the interference, these two were quite intertwined but since #247 is pretty large I wanted to separate out as much as I could.

Small fixup that was helpful to me while debugging. Signed-off-by: Jade Philipoom <jadep@zerorisc.com>

If both sides of a branch are constant, resolve the branch. This helps especially with fine-grained DMEM tracking when different branches write different memory. Signed-off-by: Jade Philipoom <jadep@zerorisc.com>

Signed-off-by: Jade Philipoom <jadep@zerorisc.com>

mkannwischer

Thanks for addressing my comments.
A couple of more nits - feel free to ignore. LGTM.

mkannwischer · 2026-04-29T08:11:19Z

+    # Load the program.
+    program = decode_elf(args.elf)


There is another decode_elf in line 94. Can one be removed?

mkannwischer · 2026-04-29T08:12:09Z

+        raise ValueError('Cannot specify --secret and --public together.')
+
+    # Parse the secrets as information-flow nodes.
+    secret_nodes = set()


secret_nodes is never used, but I guess you will need it later? feel free to ignore.

mkannwischer · 2026-04-29T08:15:00Z

+            if value.startswith('0x'):
+                value = int(value, 16)
+            else:
+                value = int(value)


nit: Could be value = int(value, 0)?

jadephilipoom force-pushed the jadep/iflow-backend-improvements branch 3 times, most recently from 64fceca to bcaf83f Compare April 24, 2026 13:23

jadephilipoom mentioned this pull request Apr 24, 2026

[acc] Extend information-flow tools to track DMEM. #247

Draft

jadephilipoom force-pushed the jadep/iflow-backend-improvements branch from bcaf83f to c3e9482 Compare April 24, 2026 13:42

jadephilipoom mentioned this pull request Apr 27, 2026

[acc] Expand constant-time coverage for ACC tests. #250

Draft

jadephilipoom requested review from mkannwischer and pqcfox April 27, 2026 15:47

jadephilipoom force-pushed the jadep/trigger-fault-common branch from f62909a to b67e603 Compare April 28, 2026 15:22

mkannwischer suggested changes Apr 29, 2026

View reviewed changes

jadephilipoom force-pushed the jadep/trigger-fault-common branch from b67e603 to 9e756e9 Compare April 29, 2026 06:26

jadephilipoom added 2 commits April 29, 2026 08:27

[acc] Better printing of control flow nodes.

bf01fcf

Small fixup that was helpful to me while debugging. Signed-off-by: Jade Philipoom <jadep@zerorisc.com>

[acc] Improve branch resolution for information-flow analysis.

2893c7f

If both sides of a branch are constant, resolve the branch. This helps especially with fine-grained DMEM tracking when different branches write different memory. Signed-off-by: Jade Philipoom <jadep@zerorisc.com>

jadephilipoom force-pushed the jadep/iflow-backend-improvements branch 2 times, most recently from 83be05b to 5726439 Compare April 29, 2026 06:32

[acc] Add secret-tracking starting from a public list.

d6aadb3

Signed-off-by: Jade Philipoom <jadep@zerorisc.com>

jadephilipoom force-pushed the jadep/iflow-backend-improvements branch from 5726439 to d6aadb3 Compare April 29, 2026 06:36

mkannwischer approved these changes Apr 29, 2026

View reviewed changes

mkannwischer mentioned this pull request Jun 1, 2026

[acc] Minor improvements to information-flow checker pavona/pavona#217

Merged

		return out


		def parse_information_flow_node(program: ACCProgram, x: str) -> str:

		# Deliberately invalid CSR address used by unimp.
		UNIMP_CSR_ADDR = 3072

Conversation

jadephilipoom commented Apr 24, 2026

Uh oh!

mkannwischer left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mkannwischer Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mkannwischer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mkannwischer left a comment •

edited

Loading

mkannwischer Apr 29, 2026 •

edited

Loading