Skip to content
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.

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

Refactor Result Classes with Unified Data Container and Improved Extensibility #948

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

zewei-wang
Copy link

@zewei-wang zewei-wang commented Apr 1, 2025

Because the old PR: #941 is accidentally closed and cannot be reopened. I created this new PR with new nits fixed commit.

Description:

This pull request introduces a major refactor to the Result class and its associated data classes. The key improvements and changes are:

1. UML Design: Inheritance → Composition

  • Moved from inheritance-based to composition-based design, eliminating overlapping functionality.
  • Merged duplicated classes (BuildResult and RunResult in builder_runner.py and results.py) to streamline data containers.
    UML Diagram Link

iScreen Shoter - Safari - 250331231950

2. Class Renaming for Clarity

  • Renamed BuildResult, RunResult, and AnalysisResult to:
    • BuildInfo
    • RunInfo
    • AnalysisInfo

3. Migration to New Result Class

  • Replaced the old Result classes:
    • Updated 8–10 files, including:
      • results.py
      • fuzz_target_error.py
      • builder_runner.py
      • run_one_experiment.py
      • semantic_analyzer.py
      • pipeline.py
      • evaluator.py
      • execution_stage.py
      • prototyper.py
      • enhancer.py
    • Approximately 700–800 lines of code modified.
  • Migration:
    • Replaced all old class references to the new Result structure in one comprehensive PR, ensuring consistency and avoiding migration conflicts.

4. New Class Field Semantic Definition

  • Specify semantic definitions in classes, remove unused fields:
    • succeededcompiles in BuildInfo
    • succeededcrashes in RunInfo
  • Introduced read-only methods get_build_script_source() and get_fuzz_target_source() in Result for better access.

5. Crash and Coverage Adaptations

  • Incorporated changes from related PRs:
  • Added new fields to Result:
    • true_bug and insight from CrashResult
    • run_error, crash_func, crash_symptom, crash_stacks from SemanticCheckResult
  • Updated CoverageAnalysis to include:
    • improve_required
    • insight
    • suggestions
  • Added success property to AnalysisInfo.

6. Error Type Consolidation

  • Grouped error types from SemanticCheckResult into an enumeration: FuzzTargetResult.
  • Grouping logic:
    • NORM_ – Normal Situations (e.g., NORM_NO_SEMANTIC_ERR)
    • FP_ – False Positives (e.g., FP_TARGET_CRASH)
    • COV_ – Coverage Issues (e.g., COV_NO_INCREASE)
    • NON_SEC_CRASH_ – Not Security-Related Crashes (e.g., NON_SEC_CRASH_NULL_DEREF, NON_SEC_CRASH_SIGNAL)
    • GEN_ – General Problems (e.g., GEN_OVERWRITE_CONST)
  • Moved error_type to AnalysisInfo for better alignment with analysis stages.

7. Backward Compatibility and New API Methods

  • Introduced methods like is_build_successful() to preserve backward compatibility for current business logic.
  • Ensured that each stage generates and propagates its own Result instance with relevant BuildInfo, RunInfo, or AnalysisInfo.

8. Code Quality and Refactor Considerations

  • Addressed legacy issues such as inappropriate inheritance and template data classes.
  • Improved modularity for result classes by ensuring that the Result class can be safely consumed across multiple stages without boundary errors.

Great thanks to @DonggeLiu for input on refining the UML Design and background context clariying and @oliverchang for reviewing the first version commits.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant