-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Update fuzzing infra to build and use the shared framework #108555
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,7 @@ | ||
<Project> | ||
<PropertyGroup> | ||
<NetCoreAppCurrentVersion>10.0</NetCoreAppCurrentVersion> | ||
<NetCoreAppCurrent>net$(NetCoreAppCurrentVersion)</NetCoreAppCurrent> | ||
<ProductVersion>$(NetCoreAppCurrentVersion).0</ProductVersion> | ||
<TestUtilities>..\..\Common\tests\TestUtilities</TestUtilities> | ||
<IsTestSupportProject>true</IsTestSupportProject> | ||
<TestUtilities>..\..\Common\tests\TestUtilities</TestUtilities> | ||
</PropertyGroup> | ||
</Project> | ||
<Import Project="..\Directory.Build.props" /> | ||
</Project> |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
%~dp0..\..\..\..\artifacts\bin\DotnetFuzzing\Debug\net10.0\win-x64\DotnetFuzzing.exe prepare-onefuzz deployment |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,28 +18,36 @@ Useful links: | |
|
||
### Prerequisites | ||
|
||
Build the runtime with the following arguments: | ||
Build the runtime if you haven't already: | ||
```cmd | ||
./build.cmd clr+libs+packs+host -rc Checked -c Debug | ||
./build.cmd clr+libs -rc release | ||
MihaZupan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
``` | ||
|
||
and install the SharpFuzz command line tool: | ||
```cmd | ||
dotnet tool install --global SharpFuzz.CommandLine | ||
``` | ||
|
||
> [!TIP] | ||
> The project uses a checked runtime + debug libraries configuration by default. | ||
> If you want to use a different configuration, make sure to also adjust the artifact paths in `nuget.config`. | ||
> The project uses a `Release` runtime + `Debug` libraries configuration by default. | ||
> Automated fuzzing runs use a `Checked` runtime + `Debug` libraries configuration by default. | ||
> You can use any configuration locally, but `Checked` is recommended when testing changes in `System.Private.CoreLib`. | ||
|
||
### Fuzzing locally | ||
|
||
The `prepare-onefuzz` command will create separate directories for each fuzzing target, instrument the relevant assemblies, and generate a helper script for running them locally. | ||
Note that this command must be ran on the published artifacts (won't work with `dotnet run`). | ||
Build the `DotnetFuzzing` fuzzing project. It is self-contained, so it will produce `DotnetFuzzing.exe` along with a copy of all required libraries. | ||
|
||
```cmd | ||
cd src/libraries/Fuzzing/DotnetFuzzing | ||
|
||
dotnet publish -o publish; publish/DotnetFuzzing.exe prepare-onefuzz deployment | ||
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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can update with my other PR |
||
When iterating on changes, remember to rebuild the project again: `dotnet build; .\run.bat`. | ||
|
||
```cmd | ||
run.bat | ||
``` | ||
|
||
You can now start fuzzing by running the `local-run.bat` script in the folder of the fuzzer you are interested in. | ||
|
This file was deleted.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 parentDirectory.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