Skip to content

Stop analysis at illegal instructions #13

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jserv merged 1 commit into from
Aug 3, 2026
+16 −8
Merged

Stop analysis at illegal instructions #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,15 @@ RVOPT_BAD_ENC := \
ebreak:05d00893,00100073 \
ecall-with-rd:05d00893,000000f3 \
ecall-with-rs1:05d00893,00008073
# The jump-over cases exercise reachability, not the decode gate: an illegal
# word off the reachable path must not change what the reachable path lowers to.
RVOPT_OK_ENC := \
JALR:00800093,00008067,05d00893,00000073 \
ret:00c000ef,05d00893,00000073,00008067 \
ecall-exit:05d00893,00000073 \
ecall-write:04000893,00000073
ecall-write:04000893,00000073 \
jump-over-reserved-JALR:0100006f,01400093,00009067,00008067,05d00893,00000073 \
jump-over-CSR:0100006f,01400093,00001073,00008067,05d00893,00000073
Comment on lines +77 to +78

@jserv jserv Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good regression coverage: both cases reject without the rvopt.c change (unsupported op at pc 20) and accept with it, so they pin the behavior.

PACK_WORDS = python3 -c 'import sys, struct; sys.stdout.buffer.write(b"".join(struct.pack("<I", int(w, 16)) for w in sys.argv[1].split(",")))'

# Standalone native-image emission: run a hand-written smoke image (MOVE, SUBLEQ
Expand Down Expand Up @@ -110,13 +114,15 @@ verify-mux: $(BIN) $(RVOPT) ## Verify wide 32-bit-cell native emission.
else $(PRINTF) "verify-mux: FENCE reject/accept [SKIP: no python3]\n"; fi
$(Q)if command -v python3 >/dev/null 2>&1; then \
for c in $(RVOPT_BAD_ENC); do \
$(PACK_WORDS) "$${c#*:}" > $(TMPDIR)/rvopt-enc.bin; \
$(PACK_WORDS) "$${c#*:}" > $(TMPDIR)/rvopt-enc.bin \
|| { echo "verify-mux: cannot encode $${c%%:*}"; exit 1; }; \
! $(RVOPT) mux $(TMPDIR)/rvopt-enc.bin >/dev/null 2>$(TMPDIR)/rvopt-enc.err \
&& grep -q 'unsupported op' $(TMPDIR)/rvopt-enc.err \
|| { echo "verify-mux: accepted $${c%%:*}"; exit 1; }; \
done; \
for c in $(RVOPT_OK_ENC); do \
$(PACK_WORDS) "$${c#*:}" > $(TMPDIR)/rvopt-enc.bin; \
$(PACK_WORDS) "$${c#*:}" > $(TMPDIR)/rvopt-enc.bin \
|| { echo "verify-mux: cannot encode $${c%%:*}"; exit 1; }; \
$(RVOPT) mux $(TMPDIR)/rvopt-enc.bin >/dev/null 2>&1 \
|| { echo "verify-mux: rejected $${c%%:*}"; exit 1; }; \
done; \
Expand Down
12 changes: 7 additions & 5 deletions rvopt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,11 +321,13 @@ static int addr2node(const struct graph *g, uint32_t pc)
return (idx >= 1 && idx < g->count) ? idx : NONE;
}

/* A control transfer ends a basic block; its successor starts a new one. */
/* A control transfer ends a basic block, and so does an illegal word (nothing
* runs past one); its successor starts a new one.
*/
static bool is_block_end(int kind)
{
return kind == K_JAL || kind == K_JALR || kind == K_BRANCH ||
kind == K_SYSTEM;
return kind == K_ILL || kind == K_JAL || kind == K_JALR ||
kind == K_BRANCH || kind == K_SYSTEM;
}

/* Build the def-use lists from the vd1/vd2 producer edges: a flat pool indexed
Expand Down Expand Up @@ -813,7 +815,7 @@ static void resolve_jalr(struct graph *g)
* terminates) plus a branch/jump target. Writes up to 2 node indexes into
* succ[] and returns the count. Only 'jal ra' and resolved 'jalr ra,...' have a
* return site (matching the ret model); 'j', 'jal x5', and runtime JALR do not
* fall through; an ecall exit terminates.
* fall through; an ecall exit terminates, and so does an illegal word.
*/
static int successors(const struct graph *g,
const struct sysinfo *sys,
Expand All @@ -826,7 +828,7 @@ static int successors(const struct graph *g,
const int rd = (nd->word >> 7) & 31;
const bool link = rd == 1 && (nd->kind == K_JAL ||
(nd->kind == K_JALR && nd->target != NONE));
const bool terminates = (nd->kind == K_JAL && !link) ||
const bool terminates = nd->kind == K_ILL || (nd->kind == K_JAL && !link) ||

@jserv jserv Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes the illegal-word instance of a more general issue rather than the root cause. The underlying trigger is that resolve_jalr marks the target of any resolved JALR as a leader without checking that the JALR is reachable, and analyze_syscalls then clears cprop at that leader. So an unreachable ret with a constant base can corrupt a reachable node's cprop by leaking a constant across any intervening straight-line instruction, not only an illegal one: replace the illegal word in the new test with a plain NOP (00000013) in the same layout and rvopt still rejects the reachable exit path, with and without this change. This PR is a good, targeted fix for the illegal-word case; a follow-up could gate JALR-target leader marking on reachability (iterate resolve_jalr and mark_reachable to a fixpoint) to close the general case.

@thc1006 thc1006 Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed. Replacing the illegal word with either 00000013 (nop) or 00000033 (add x0, x0, x0) reproduces the same rejection on both main and this branch, confirming that leader marking from unreachable JALRs is the general issue.

I'll keep this PR scoped to treating K_ILL as a control-flow barrier and address reachability-gated JALR target leader marking in a separate follow-up.

(nd->kind == K_JALR && !link) ||
(nd->kind == K_SYSTEM && sys[i].kind == SYS_EXIT);
if (!terminates && fall != NONE)
Expand Down