Skip to content

Conversation

@buyaa-n
Copy link
Contributor

@buyaa-n buyaa-n commented Oct 4, 2024

Fuzzing infra is currently broken because of #106599, because it is partially integrated with the repo workflow. In order to fix this failure, we need to integrate it completely, that means need to fix #107204:

  • Make the fuzzing solution a framework dependent app and use test shared framework
  • Fixed and/or suppressed all findings of different analyzer runs
  • Updated the README.md to apply the changes made
  • Ended up removing nuget.config as it became same as the repo config in the root, also removed Directory.Build.targets that was empty

Further MihuBot need to be updated as needed, might also need to update onefuzz deployment scripts CC @MihaZupan

@ghost ghost added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Oct 4, 2024
@buyaa-n buyaa-n added area-Meta and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Oct 4, 2024
Copy link
Member

@MihaZupan MihaZupan left a comment

Choose a reason for hiding this comment

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

Thank you

@MihaZupan
Copy link
Member

MihaZupan commented Oct 4, 2024

The bot's also working again now MihuBot/runtime-utils@5466698
And updating the pipeline: buyaa-n#19

MihaZupan and others added 3 commits October 4, 2024 23:27
Co-authored-by: Miha Zupan <mihazupan.zupan1@gmail.com>
Update deploy-to-onefuzz pipeline
@buyaa-n buyaa-n merged commit 0372b50 into dotnet:main Oct 5, 2024
@buyaa-n buyaa-n deleted the fuzz-infra branch October 5, 2024 02:49
flAllocationType: VirtualAllocAllocationType.MEM_RESERVE | VirtualAllocAllocationType.MEM_COMMIT,
flProtect: VirtualAllocProtection.PAGE_NOACCESS);
VirtualAllocHandle handle;
checked
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this checked added due to a bug? It seems like it would only throw on 32-bit platforms.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not due to a bug, after changing the project build with current runtime settings many analyzer diagnostics found that are fixed or suppressed with the PR.

CA2020 Starting with .NET 7 the explicit conversion '(IntPtr)Int64' will not throw when overflowing in an unchecked context. Wrap the expression with a 'checked' statement to restore the .NET 6 behavior.

Flagged at row 38: dwSize: (IntPtr)totalBytesToAllocate /* cast throws OverflowException if out of range */,

Given that this type is for testing, and also base on the comment I assumed it is important to restore the overflowing behavior

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suppressed have all other warnings for this BoundedMemory type, now after moving <IsTestSupportProject>true</IsTestSupportProject> property above importing parent Directory.Build.props this type is not being analyzed anymore, same as all other test types. I can revert this and other suppressions with my other PR

dotnet build
```

Now you can run `run.bat`, which will create separate directories for each fuzzing target, instrument the relevant assemblies, and generate a helper script for running them locally.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: "Now you can" and "You can now" a little below seems unnecessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can update with my other PR

sirntar pushed a commit to sirntar/runtime that referenced this pull request Oct 8, 2024
…8555)

* Update fuzzing infra to build and use the shared framework

* Update deploy-to-onefuzz pipeline

* Apply suggestions from code review

Co-authored-by: Miha Zupan <mihazupan.zupan1@gmail.com>

---------

Co-authored-by: Miha Zupan <mihazupan.zupan1@gmail.com>
@github-actions github-actions bot locked and limited conversation to collaborators Nov 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fuzzing workflow is not friendly to rebuild and incremental changes.

3 participants