Skip to content

Define a project-level error-handling policy #1079

@elazarg

Description

@elazarg

Problem

Prevail needs a project-level policy for distinguishing error categories. The important categories are currently
implicit:

  • malformed input;
  • unsupported input;
  • verifier rejection;
  • invalid configuration or caller misuse;
  • I/O failure;
  • internal invariant violation.

Current mechanisms include:

  • verification failures carried as AnalysisResult::failed plus VerificationError;
  • unmarshal failures returned as variant<InstructionSeq, string> / expected-style values;
  • loader and ELF/BTF failures thrown as UnmarshalError / MalformedElf;
  • preparation failures thrown as InvalidControlFlow;
  • invalid caller configuration thrown as std::invalid_argument;
  • internal invariant failures represented by assert, CRAB_ERROR, std::logic_error, std::runtime_error, and a few
    bare std::exception() throws;
  • tests that often treat unmarshal failure, verifier rejection, and runtime exceptions as equivalent "rejected"
    outcomes.

Several of these styles have local justification. The missing piece is a rule for which style belongs at which boundary.

CRAB_ERROR is also still used as the generic internal-error primitive. It comes from the original CRAB library style
and
does not fit Prevail's current codebase well: it throws std::runtime_error, prefixes messages with CRAB ERROR, and
does not encode whether the failure is a verifier bug, malformed input, unsupported feature, or caller misuse.

Direction

Write a short project-level policy that answers:

  • Which failures are normal verifier results rather than exceptions?
  • Which failures should be value-return errors at phase boundaries?
  • Which failures should throw typed exceptions?
  • Which failures are internal invariant violations?
  • When is assert acceptable, and when must the check remain active in release builds?
  • What should command-line tools print, and which stream should receive diagnostics?
  • What should tests assert: "failed somehow" or a specific failure phase/category?

The policy should preserve the good existing direction: a program failing verification should remain normal result data,
not an exception.

It should also define a replacement direction for CRAB_ERROR. That does not have to be a large framework. A small
helper
or typed exception for internal verifier invariants would be enough as a first step, as long as new code has a clear
rule
to follow.

Candidate Policy Shape

This is not meant to prescribe the exact final API, but the policy probably needs categories along these lines:

  • VerificationFailure: analyzer proved the program unsafe or could not establish a required assertion; represented in
    AnalysisResult.
  • InputError: malformed ELF, BTF, CO-RE, bytecode, textual IR, or YAML.
  • UnsupportedFeature: input is well-formed but outside the implemented verifier/platform surface.
  • InvalidConfiguration: caller or CLI supplied invalid verifier options.
  • IoError: filesystem or output failure.
  • InternalError: violated verifier invariant or impossible state.

The category should not be inferred only from message text such as rejected:, not implemented:, or
internal error:.
Message text is useful for humans, but code and tests should have a structured way to distinguish categories where it
matters.

PR-sized Closures

  • Inventory existing throw/catch/result sites and classify them by intended category.
  • First cleanup candidate: replace bare throw std::exception() sites with meaningful internal invariant errors.
  • First cleanup candidate: classify existing InvalidControlFlow uses as unsupported feature, invalid program
    structure, policy rejection, or internal preparation bug.
  • Define the replacement convention for CRAB_ERROR and migrate a small representative subsystem.
  • Make unmarshal's error contract complete: expected bytecode decode failures should consistently use the documented
    value-return channel.
  • Enrich or split InvalidControlFlow based on the classification above.
  • Narrow broad catch sites that convert all std::exception failures into malformed input.
  • Adjust representative tests so they assert the expected failure phase/category instead of only "some rejection".
  • Update CLI output conventions for error streams and message prefixes.

Related Existing Issues

  • #1027 is relevant because error-channel choices interact with the C++23
    migration and public/private use of std::expected.
  • #1010 is related to diagnostics/reporting, but this issue is
    lower-level:
    it is about classifying failures before deciding how to present them.
  • #680 is closed, but relevant historical API context: callers want
    queryable
    verifier results, which requires distinguishing verifier failures from other errors.

Related PRs

  • #1074 moves unmarshal toward std::expected. That is a useful local
    improvement, but the broader policy should decide which phase boundaries should use value-return errors and which
    should
    use typed exceptions.
  • #1062 introduces reusable analysis-facing abstractions. Those should
    avoid
    baking in the current broad bool/exception conflation.

Notes

This should be treated as an auditability issue, not a style cleanup. Error handling affects whether future maintainers
can tell the difference between "the program was rejected", "the input is malformed", and "the verifier hit an internal
bug".

The first PR should probably be policy plus one or two uncontroversial cleanups, not a repo-wide migration. Good first
cleanups are the bare std::exception() replacements and an InvalidControlFlow classification inventory.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions