KhronosGroup / KhronosGroup/SPIRV-Tools

Add validity checks to the reducer test cases, and fix any identified issues.

Open
#4,256 3 comments 0 reactions 0 assignees View on GitHub
component:reducer
Dominant language
C++
Stars
1.4k
Forks
709
Avg merge
1d 22h
Merged PRs (30d)
28

Description

In #4255 @Vasniktel commented:

> Also, I've noticed that if you add `CheckValid` to unit tests to all the places it's missing from (e.g. when the initial binary is constructed, after every `TryToApply` etc), a bunch of tests fail. I also have doubts about whether `CheckValid` ensures that the module is well-formed (i.e. all analyses are correct) - maybe `fuzzerutil::IsValidAndWellFormed` can help with that?

It would be good to fix this.

If we want to reuse fuzzerutil::IsValidAndWellFormed then we would have to move it somewhere that's visible to both fuzz and reduce.

It could be moved to reduce since fuzz can see reduce but that doesn't seem the natural place for it.

Perhaps it should be made an instance method of IRContext.

The current signature is:

```
bool IsValidAndWellFormed(const opt::IRContext* ir_context,
spv_validator_options validator_options,
MessageConsumer consumer)
```

The method does two things: it invokes the validator, and then does some extra well-formedness checks.

It seems nasty for IRContext to invoke the validator, because this would mean that IRContext would need to know how to create a Tools object.

However, IRContext could own the extra checks - that every block has a parent function, and that the ids defined in the module are unique.

So maybe we could give IRContext and IsWellFormed method. The IsValidAndWellFormed could stay in fuzzerutil and could invoke its local IsValid, and then ir_context->IsWellFormed(), and the reducer could do something similar.

@Vasniktel what do you think?

Contributor guide

Open the contributing guide

Research direction

Start with the reducer unit tests and the existing CheckValid calls, then read fuzzerutil::IsValidAndWellFormed and IRContext to understand the validation and well-formedness responsibilities. Add validity checks at the missing test points, resolve the identified failures, and ensure the reducer tests pass with the intended checks.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
compilers, testing-qa
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.